Use `confighttp.NewDefaultClientConfig` function to create object of `confighttp.ClientConfig`

[hprajapati-splunk]

In config/confighttp/confighttp.go file 4 new pointer type variables (MaxIdleConns, MaxIdleConnsPerHost, MaxConnsPerHost, IdleConnTimeout) has been introduced. The pointer type is used in order to distinguish the user provided input with no-input as discussed here.

In long run, we want to change those variable types to non-pointers as discussed here, so we have introduced DefaultHTTPClientSettings functions. Once, all the users' of below two repos start to use DefaultHTTPClientSettings function to create the object of HTTPClientSettings, we'll change those 4 variables's type to non-pointers:

• opentelemetry-collector
• opentelemetry-collector-contrib

This issue is created to track the progress mentioned above.

[jpkrohling]

@vvydier, this one is also up for grabs, in case you are interested.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

[mattsains]

I'm working on this issue. I've noticed that a lot of code in contrib defines different timeout values, and a lot of the values are the same across packages, so I'm thinking a lot of the explicit timeout setting is just copy-pasting. Anyway, I'm going to preserve these changes in my PR because I can't know whether these timeouts are intentional or not.

I have a lot of work to do to update all the tests since they mostly assume the timeouts will be set to zero/nil whereas now they are set to the defaults in NewDefaultHTTPClientSettings()

[mattsains]

here's the progress I've made so far. The vast majority of the work is tests, which I'll continue burning through tomorrow:

https://github.com/open-telemetry/opentelemetry-collector-contrib/compare/main...mattsains:opentelemetry-collector-contrib:6641?expand=1

Please let me know if you see any issues with this because I'm repeating the same pattern hundreds of times so it would be good to know earlier if it's wrong :D

[mattsains]

worried about uses of HTTPClientSettings like this one:

opentelemetry-collector-contrib/exporter/tanzuobservabilityexporter/config.go

Line 25 in c3fb1c1

confighttp.HTTPClientSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.

I think that results in uninitialized struct values if those values are not present in the config when it is unmarshaled here: https://github.com/open-telemetry/opentelemetry-collector/blob/22d3a4d483e970e01cbc71cfee07df3dcd20dda6/confmap/confmap.go#L114

I'm not really familiar with the code, but would it be reasonable to implement the Unmarshaler interface on DefaultHTTPClientSettings?

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

[mattsains]

I think this is still relevant because the config object still exists and there is a path forward to fixing it, just trying to get some buy in before doing the work.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.