Use confighttp.NewDefaultClientConfig instead of manually creating struct #11274
Comments
bogdandrutu
pushed a commit
that referenced
this issue
Sep 26, 2024
…ually creating struct (#11273) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR makes usage of `NewDefaultClientConfig` instead of manually creating the `confighttp.ClientConfig` struct. The updates to defaults are maintained (Timeout, Compression, WriteBufferSize) whereas the fields that match the default are not manually set anymore (Endpoint, Headers). It also sets to nil the fields that are set in default but weren't set previously (MaxIdleConns, MaxIdleConnsPerHost, MaxConnsPerHost, IdleConnTimeout) to retain the same behaviour. I am on the fence about this one, should we switch to using the defaults ? <!-- Issue number if applicable --> #### Link to tracking issue #11274 <!--Describe what testing was performed and which tests were added.--> #### Testing <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.-->
|
Can you please separate this into 2 different issues: one for core components and one for contrib? |
|
Opened following issue for contrib: open-telemetry/opentelemetry-collector-contrib#35457 |
jackgopack4
pushed a commit
to jackgopack4/opentelemetry-collector
that referenced
this issue
Oct 8, 2024
…ually creating struct (open-telemetry#11273) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR makes usage of `NewDefaultClientConfig` instead of manually creating the `confighttp.ClientConfig` struct. The updates to defaults are maintained (Timeout, Compression, WriteBufferSize) whereas the fields that match the default are not manually set anymore (Endpoint, Headers). It also sets to nil the fields that are set in default but weren't set previously (MaxIdleConns, MaxIdleConnsPerHost, MaxConnsPerHost, IdleConnTimeout) to retain the same behaviour. I am on the fence about this one, should we switch to using the defaults ? <!-- Issue number if applicable --> #### Link to tracking issue open-telemetry#11274 <!--Describe what testing was performed and which tests were added.--> #### Testing <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.-->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Related to second point in: #9478 (comment).
Move away from manually creating
confighttp.ClientConfigin favor of usingconfighttp.NewDefaultClientConfig.This is necessary for the following components:
This will be done following conditions:
NewDefaultClientConfigFor the third point, I am unsure of which approach to follow. An example is the
otlphttpexporter(#11273), where previouslyMaxIdleConns,MaxIdleConnsPerHost,MaxConnsPerHostandIdleConnTimeoutwere unset (equivalent ofnil), which is different than the default which has values.The text was updated successfully, but these errors were encountered: