Increase splunkObservability and splunkPlatform timeouts to 20s #438
Conversation
jvoravong
changed the title
…splunkPlatform timeouts to 20s
| @@ -146,6 +146,9 @@ | |||
| "profilingEnabled": { | |||
| "description": "Whether profiling data is enabled for Splunk Observability", | |||
| "type": "boolean" | |||
| }, | |||
| "timeout": { | |||
| "type": "string" | |||
There was a problem hiding this comment.
mind adding a description?
| @@ -108,6 +108,9 @@ splunkObservability: | |||
| # If you don't use AlwaysOn Profiling for Splunk APM, you can disable it. | |||
| profilingEnabled: false | |||
|
|
|||
| # HTTP timeout for sending data. | |||
| timeout: 20s | |||
There was a problem hiding this comment.
20s is pretty high, so maybe we should keep the default of 10 with the new value until/unless we receive reports of it being exceeded (wasn't the linked issue just for exceeded platform timeouts)?
There was a problem hiding this comment.
I agree with Ryan. 20s for HTTP connection is too high. We shouldn't change it
There was a problem hiding this comment.
Thank you for your input.
I rarely see the same context error in my k8s clusters that upload data to Splunk Observability, it probably only happens when backend or middle issues are present.
I'm okay with not adding a splunkObservability.timeout config, I thought it might be useful for Observability users.
There was a problem hiding this comment.
I don't think it's reasonable option to have. We should merge open-telemetry/opentelemetry-collector-contrib#6803 and expose full set of http settings instead. 10s timeout is already very high. I doubt that users really need it
|
I just realized that all the http settings that will be added in open-telemetry/opentelemetry-collector-contrib#6803 are squashed, so this config option won't break it. Probably we can add timeout as done in this PR if it's really needed, but I'm apposed to bumping the defaults unless we are confident that this is needed |
|
I'm going to close this PR for now since we should take a different approach to addressing the related issue. |
Related Issue: signalfx/splunk-otel-collector#1454