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.

[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.

[jpkrohling]

I'm closing this, as it's unlikely to be still relevant. If there's still interest in working on this, please reopen.

[dmitryax]

This is still relevant

[atoulme]

$>  git grep -l HTTPClientSettings | xargs dirname | sort | uniq > using_http_client_settings.txt
$> git grep -l DefaultHTTPClientSettings | xargs dirname | sort | uniq > using_default_http_client_settings.txt
$> diff using_http_client_settings.txt using_default_http_client_settings.txt

Gives me:

2,15d1
< cmd/opampsupervisor/specification
< cmd/otelcontribcol
< exporter/alertmanagerexporter
< exporter/datadogexporter
< exporter/dynatraceexporter
< exporter/dynatraceexporter/config
< exporter/elasticsearchexporter
< exporter/f5cloudexporter
< exporter/honeycombmarkerexporter
< exporter/influxdbexporter
< exporter/instanaexporter
< exporter/logicmonitorexporter
< exporter/logzioexporter
< exporter/lokiexporter
18,21d3
< exporter/prometheusremotewriteexporter
< exporter/signalfxexporter
< exporter/signalfxexporter/internal/correlation
< exporter/splunkhecexporter
23,28d4
< exporter/tanzuobservabilityexporter
< exporter/zipkinexporter
< extension/httpforwarder
< extension/oauth2clientauthextension
< extension/observer/ecstaskobserver
< internal/aws/ecsutil
30d5
< processor/resourcedetectionprocessor/internal/aws/ec2
32,51d6
< receiver/apachereceiver
< receiver/apachesparkreceiver
< receiver/awscontainerinsightreceiver/internal/ecsInfo
< receiver/awsecscontainermetricsreceiver
< receiver/bigipreceiver
< receiver/cloudfoundryreceiver
< receiver/couchdbreceiver
< receiver/elasticsearchreceiver
< receiver/expvarreceiver
< receiver/flinkmetricsreceiver
< receiver/gitproviderreceiver/internal/scraper/githubscraper
< receiver/haproxyreceiver
< receiver/httpcheckreceiver
< receiver/nginxreceiver
< receiver/nsxtreceiver
< receiver/purefareceiver
< receiver/purefbreceiver
< receiver/rabbitmqreceiver
< receiver/riakreceiver
< receiver/simpleprometheusreceiver
53d7
< testbed/mockdatasenders/mockdatadogagentexporter

Quite a few components need to change to accomodate that change.

[atoulme]

Usage to review:

• cmd/opampsupervisor/specification
• cmd/otelcontribcol
• exporter/alertmanagerexporter
• exporter/datadogexporter
• exporter/dynatraceexporter
• exporter/dynatraceexporter/config
• exporter/elasticsearchexporter
• exporter/f5cloudexporter
• exporter/honeycombmarkerexporter
• exporter/influxdbexporter
• exporter/instanaexporter
• exporter/logicmonitorexporter
• exporter/logzioexporter
• exporter/lokiexporter
• exporter/prometheusremotewriteexporter
• exporter/signalfxexporter
• exporter/signalfxexporter/internal/correlation
• exporter/splunkhecexporter
• exporter/tanzuobservabilityexporter
• exporter/zipkinexporter
• extension/httpforwarder
• extension/oauth2clientauthextension
• extension/observer/ecstaskobserver
• internal/aws/ecsutil
• processor/resourcedetectionprocessor/internal/aws/ec2
• receiver/apachereceiver
• receiver/apachesparkreceiver
• receiver/awscontainerinsightreceiver/internal/ecsInfo
• receiver/awsecscontainermetricsreceiver
• receiver/bigipreceiver
• receiver/cloudfoundryreceiver
• receiver/couchdbreceiver
• receiver/elasticsearchreceiver
• receiver/expvarreceiver
• receiver/flinkmetricsreceiver
• receiver/gitproviderreceiver/internal/scraper/githubscraper
• receiver/haproxyreceiver
• receiver/httpcheckreceiver
• receiver/nginxreceiver
• receiver/nsxtreceiver
• receiver/purefareceiver
• receiver/purefbreceiver
• receiver/rabbitmqreceiver
• receiver/riakreceiver
• receiver/simpleprometheusreceiver
• testbed/mockdatasenders/mockdatadogagentexporter

[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.

[andrzej-stencel]

Here's an example pull request, this one fixes the Sumo Logic exporter: #31510

Similar pull requests are needed for all other components that use the confighttp.ClientConfig{...} initialization.

[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.

[github-actions]

This issue has been closed as inactive because it has been stale for 120 days with no activity.