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

hprajapati-splunk · 2021-12-06T12:50:37Z

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:

This issue is created to track the progress mentioned above.

The text was updated successfully, but these errors were encountered:

jpkrohling · 2022-02-09T15:19:12Z

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

github-actions · 2022-11-08T03:47:39Z

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 · 2022-11-14T21:39:32Z

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 · 2022-11-15T01:15:46Z

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 · 2022-11-15T23:50:42Z

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?

Signed-off-by: Bogdan Drutu <[email protected]> Signed-off-by: Bogdan Drutu <[email protected]>

github-actions · 2023-01-16T03:31:17Z

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 · 2023-01-17T05:17:58Z

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 · 2023-03-20T03:30:39Z

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 · 2023-05-29T03:30:05Z

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 · 2023-06-26T19:26:55Z

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

dmitryax · 2023-12-07T19:12:44Z

This is still relevant

atoulme · 2023-12-14T17:41:18Z

$>  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 · 2023-12-14T17:42:26Z

**Description:** Modernize the logic of validation by moving to config Validate from the Start function Use the HTTPClientDefaultSettings defaults when setting the config. **Link to tracking Issue:** #6641 **Testing:** Updated tests.

**Description:** Use confighttp.HTTPDefaultClientSettings when configuring the HTTPClientSettings for the httpforwarder extension. **Link to tracking Issue:** #6641

…metry#29931) **Description:** Modernize the logic of validation by moving to config Validate from the Start function Use the HTTPClientDefaultSettings defaults when setting the config. **Link to tracking Issue:** open-telemetry#6641 **Testing:** Updated tests.

…metry#29887) **Description:** Use confighttp.HTTPDefaultClientSettings when configuring the HTTPClientSettings for the httpforwarder extension. **Link to tracking Issue:** open-telemetry#6641

github-actions · 2024-02-13T03:29:22Z

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 · 2024-02-29T13:05:51Z

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.

…Config` (#31510) Use `confighttp.NewDefaultClientConfig` instead of `confighttp.ClientConfig{...}`. **Link to tracking Issue:** #6641 **Testing:** Unit tests

…Config` (open-telemetry#31510) Use `confighttp.NewDefaultClientConfig` instead of `confighttp.ClientConfig{...}`. **Link to tracking Issue:** open-telemetry#6641 **Testing:** Unit tests

github-actions · 2024-04-30T03:29:32Z

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 · 2024-06-29T05:19:27Z

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

jpkrohling transferred this issue from open-telemetry/opentelemetry-collector Dec 8, 2021

jpkrohling added the good first issue Good for newcomers label Dec 8, 2021

github-actions bot added the Stale label Nov 8, 2022

fatsheep9146 removed the Stale label Nov 8, 2022

povilasv referenced this issue in coralogix/opentelemetry-collector-contrib Dec 19, 2022

Prefix auth funcs/types with Client. Add *Func types (#6641)

5f49d45

Signed-off-by: Bogdan Drutu <[email protected]> Signed-off-by: Bogdan Drutu <[email protected]>

github-actions bot added the Stale label Jan 16, 2023

fatsheep9146 removed the Stale label Jan 17, 2023

github-actions bot added the Stale label Mar 20, 2023

jpkrohling removed the Stale label Mar 27, 2023

github-actions bot added the Stale label May 29, 2023

jpkrohling closed this as completed Jun 26, 2023

dmitryax reopened this Dec 7, 2023

dmitryax added help wanted Extra attention is needed and removed Stale labels Dec 7, 2023

This was referenced Dec 14, 2023

[extension/httpforwarder] use default http client settings #29887

Merged

[exporter/zipkin] use confighttp.DefaultHTTPClientSettings #29931

Merged

github-actions bot added the Stale label Feb 13, 2024

andrzej-stencel removed the Stale label Feb 16, 2024

andrzej-stencel changed the title ~~Use DefaultHTTPClientSettings function to create object of HTTPClientSettings~~ Use confighttp.DefaultClientConfig function to create object of confighttp.ClientConfig Feb 29, 2024

andrzej-stencel changed the title ~~Use confighttp.DefaultClientConfig function to create object of confighttp.ClientConfig~~ Use confighttp.NewDefaultClientConfig function to create object of confighttp.ClientConfig Feb 29, 2024

andrzej-stencel mentioned this issue Feb 29, 2024

[chore] refactor(sumologicexporter): use confighttp.NewDefaultClientConfig #31510

Merged

github-actions bot added the Stale label Apr 30, 2024

github-actions bot added the closed as inactive label Jun 29, 2024

github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hprajapati-splunk commented Dec 6, 2021

jpkrohling commented Feb 9, 2022

github-actions bot commented Nov 8, 2022

mattsains commented Nov 14, 2022 •

edited

Loading

mattsains commented Nov 15, 2022

mattsains commented Nov 15, 2022

github-actions bot commented Jan 16, 2023

mattsains commented Jan 17, 2023

github-actions bot commented Mar 20, 2023

github-actions bot commented May 29, 2023

jpkrohling commented Jun 26, 2023

dmitryax commented Dec 7, 2023

atoulme commented Dec 14, 2023

atoulme commented Dec 14, 2023

github-actions bot commented Feb 13, 2024

andrzej-stencel commented Feb 29, 2024

github-actions bot commented Apr 30, 2024

github-actions bot commented Jun 29, 2024

Use confighttp.NewDefaultClientConfig function to create object of confighttp.ClientConfig #6641

Use confighttp.NewDefaultClientConfig function to create object of confighttp.ClientConfig #6641

Comments

hprajapati-splunk commented Dec 6, 2021

jpkrohling commented Feb 9, 2022

github-actions bot commented Nov 8, 2022

mattsains commented Nov 14, 2022 • edited Loading

mattsains commented Nov 15, 2022

mattsains commented Nov 15, 2022

github-actions bot commented Jan 16, 2023

mattsains commented Jan 17, 2023

github-actions bot commented Mar 20, 2023

github-actions bot commented May 29, 2023

jpkrohling commented Jun 26, 2023

dmitryax commented Dec 7, 2023

atoulme commented Dec 14, 2023

atoulme commented Dec 14, 2023

github-actions bot commented Feb 13, 2024

andrzej-stencel commented Feb 29, 2024

github-actions bot commented Apr 30, 2024

github-actions bot commented Jun 29, 2024

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

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

mattsains commented Nov 14, 2022 •

edited

Loading