From 135d7237c36e0036004928a6a7da51d51459bfa1 Mon Sep 17 00:00:00 2001 From: Yang Song Date: Thu, 14 Mar 2024 11:01:00 -0400 Subject: [PATCH] [exporter/datadog] Respect confighttp configs and use better defaults (#31733) **Description:** Datadog exporter respects a subset of settings in confighttp client configs and uses a consistent default HTTP transport settings as Datadog Agent (https://github.com/DataDog/datadog-agent/blob/f9ae7f4b842f83b23b2dfe3f15d31f9e6b12e857/pkg/util/http/transport.go#L91-L106). --- .chloggen/ddog-config-change.yaml | 27 ++++++ .chloggen/ddog-http-defaults.yaml | 27 ++++++ .chloggen/ddog-respect-confighttp.yaml | 27 ++++++ exporter/datadogexporter/config.go | 54 ++++++++---- exporter/datadogexporter/config_test.go | 84 ++++++++++++++++++- .../datadogexporter/examples/collector.yaml | 21 +++++ exporter/datadogexporter/examples/logs.yaml | 1 + exporter/datadogexporter/factory.go | 12 +-- exporter/datadogexporter/factory_test.go | 30 +++---- exporter/datadogexporter/go.mod | 10 +-- exporter/datadogexporter/hostmetadata.go | 2 +- .../internal/clientutil/api.go | 6 +- .../internal/clientutil/http.go | 59 +++++++++---- .../internal/clientutil/http_test.go | 74 ++++++++++++++++ .../internal/hostmetadata/config.go | 6 +- .../internal/hostmetadata/metadata.go | 2 +- .../datadogexporter/internal/logs/sender.go | 6 +- .../internal/logs/sender_test.go | 5 +- exporter/datadogexporter/logs_exporter.go | 5 +- exporter/datadogexporter/metrics_exporter.go | 5 +- exporter/datadogexporter/traces_exporter.go | 5 +- 21 files changed, 387 insertions(+), 81 deletions(-) create mode 100644 .chloggen/ddog-config-change.yaml create mode 100644 .chloggen/ddog-http-defaults.yaml create mode 100644 .chloggen/ddog-respect-confighttp.yaml diff --git a/.chloggen/ddog-config-change.yaml b/.chloggen/ddog-config-change.yaml new file mode 100644 index 000000000000..9d6c23d43994 --- /dev/null +++ b/.chloggen/ddog-config-change.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: datadogexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Remove config structs `LimitedClientConfig` and `LimitedTLSClientSettings`" + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [31733] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: This should have no impact to end users as long as they do not import Datadog exporter config structs in their source code. + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/.chloggen/ddog-http-defaults.yaml b/.chloggen/ddog-http-defaults.yaml new file mode 100644 index 000000000000..2c703b081e28 --- /dev/null +++ b/.chloggen/ddog-http-defaults.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: datadogexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Datadog exporter uses the same default HTTP settings as Datadog Agent HTTP transport + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [31733] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/.chloggen/ddog-respect-confighttp.yaml b/.chloggen/ddog-respect-confighttp.yaml new file mode 100644 index 000000000000..292fe7bec465 --- /dev/null +++ b/.chloggen/ddog-respect-confighttp.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: datadogexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Datadog exporter respects a subset of settings in confighttp client configs + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [31733] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: "Currently the following configs are respected: `read_buffer_size`, `write_buffer_size`, `timeout`, `max_idle_conns`, `max_idle_conns_per_host`, `max_conns_per_host`, `idle_conn_timeout`, `disable_keep_alives` and `tls.insecure_skip_verify`." + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/exporter/datadogexporter/config.go b/exporter/datadogexporter/config.go index fd6c64a8a520..9ebd11698016 100644 --- a/exporter/datadogexporter/config.go +++ b/exporter/datadogexporter/config.go @@ -11,6 +11,7 @@ import ( "strings" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configretry" @@ -390,24 +391,11 @@ type HostMetadataConfig struct { Tags []string `mapstructure:"tags"` } -// LimitedTLSClientSetting is a subset of TLSClientSetting, see LimitedClientConfig for more details -type LimitedTLSClientSettings struct { - // InsecureSkipVerify controls whether a client verifies the server's - // certificate chain and host name. - InsecureSkipVerify bool `mapstructure:"insecure_skip_verify"` -} - -type LimitedClientConfig struct { - TLSSetting LimitedTLSClientSettings `mapstructure:"tls,omitempty"` -} - // Config defines configuration for the Datadog exporter. type Config struct { - exporterhelper.TimeoutSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct. - exporterhelper.QueueSettings `mapstructure:"sending_queue"` - configretry.BackOffConfig `mapstructure:"retry_on_failure"` - - LimitedClientConfig `mapstructure:",squash"` + confighttp.ClientConfig `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct. + exporterhelper.QueueSettings `mapstructure:"sending_queue"` + configretry.BackOffConfig `mapstructure:"retry_on_failure"` TagsConfig `mapstructure:",squash"` @@ -450,6 +438,10 @@ var _ component.Config = (*Config)(nil) // Validate the configuration for errors. This is required by component.Config. func (c *Config) Validate() error { + if err := validateClientConfig(c.ClientConfig); err != nil { + return err + } + if c.OnlyMetadata && (!c.HostMetadata.Enabled || c.HostMetadata.HostnameSource != HostnameSourceFirstResource) { return errNoMetadata } @@ -490,6 +482,36 @@ func (c *Config) Validate() error { return nil } +func validateClientConfig(cfg confighttp.ClientConfig) error { + var unsupported []string + if cfg.Auth != nil { + unsupported = append(unsupported, "auth") + } + if cfg.Endpoint != "" { + unsupported = append(unsupported, "endpoint") + } + if cfg.Compression != "" { + unsupported = append(unsupported, "compression") + } + if cfg.ProxyURL != "" { + unsupported = append(unsupported, "proxy_url") + } + if cfg.Headers != nil { + unsupported = append(unsupported, "headers") + } + if cfg.HTTP2ReadIdleTimeout != 0 { + unsupported = append(unsupported, "http2_read_idle_timeout") + } + if cfg.HTTP2PingTimeout != 0 { + unsupported = append(unsupported, "http2_ping_timeout") + } + + if len(unsupported) > 0 { + return fmt.Errorf("these confighttp client configs are currently not respected by Datadog exporter: %s", strings.Join(unsupported, ", ")) + } + return nil +} + var _ error = (*renameError)(nil) // renameError is an error related to a renamed setting. diff --git a/exporter/datadogexporter/config_test.go b/exporter/datadogexporter/config_test.go index c5e21a3a452c..62d244f7ca03 100644 --- a/exporter/datadogexporter/config_test.go +++ b/exporter/datadogexporter/config_test.go @@ -5,12 +5,25 @@ package datadogexporter import ( "testing" + "time" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configauth" + "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/configopaque" + "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/confmap" ) func TestValidate(t *testing.T) { + idleConnTimeout := 30 * time.Second + maxIdleConn := 300 + maxIdleConnPerHost := 150 + maxConnPerHost := 250 + ty, err := component.NewType("ty") + assert.NoError(t, err) + auth := configauth.Authentication{AuthenticatorID: component.NewID(ty)} tests := []struct { name string @@ -94,8 +107,8 @@ func TestValidate(t *testing.T) { name: "TLS settings are valid", cfg: &Config{ API: APIConfig{Key: "notnull"}, - LimitedClientConfig: LimitedClientConfig{ - TLSSetting: LimitedTLSClientSettings{ + ClientConfig: confighttp.ClientConfig{ + TLSSetting: configtls.ClientConfig{ InsecureSkipVerify: true, }, }, @@ -115,6 +128,40 @@ func TestValidate(t *testing.T) { Traces: TracesConfig{PeerTags: []string{"tag1", "tag2"}}, }, }, + { + name: "With confighttp client configs", + cfg: &Config{ + API: APIConfig{Key: "notnull"}, + ClientConfig: confighttp.ClientConfig{ + ReadBufferSize: 100, + WriteBufferSize: 200, + Timeout: 10 * time.Second, + IdleConnTimeout: &idleConnTimeout, + MaxIdleConns: &maxIdleConn, + MaxIdleConnsPerHost: &maxIdleConnPerHost, + MaxConnsPerHost: &maxConnPerHost, + DisableKeepAlives: true, + TLSSetting: configtls.ClientConfig{InsecureSkipVerify: true}, + }, + }, + }, + + { + name: "unsupported confighttp client configs", + cfg: &Config{ + API: APIConfig{Key: "notnull"}, + ClientConfig: confighttp.ClientConfig{ + Endpoint: "endpoint", + Compression: "gzip", + ProxyURL: "proxy", + Auth: &auth, + Headers: map[string]configopaque.String{"key": "val"}, + HTTP2ReadIdleTimeout: 250, + HTTP2PingTimeout: 200, + }, + }, + err: "these confighttp client configs are currently not respected by Datadog exporter: auth, endpoint, compression, proxy_url, headers, http2_read_idle_timeout, http2_ping_timeout", + }, } for _, testInstance := range tests { t.Run(testInstance.name, func(t *testing.T) { @@ -129,10 +176,26 @@ func TestValidate(t *testing.T) { } func TestUnmarshal(t *testing.T) { + cfgWithHTTPConfigs := NewFactory().CreateDefaultConfig().(*Config) + idleConnTimeout := 30 * time.Second + maxIdleConn := 300 + maxIdleConnPerHost := 150 + maxConnPerHost := 250 + cfgWithHTTPConfigs.ReadBufferSize = 100 + cfgWithHTTPConfigs.WriteBufferSize = 200 + cfgWithHTTPConfigs.Timeout = 10 * time.Second + cfgWithHTTPConfigs.MaxIdleConns = &maxIdleConn + cfgWithHTTPConfigs.MaxIdleConnsPerHost = &maxIdleConnPerHost + cfgWithHTTPConfigs.MaxConnsPerHost = &maxConnPerHost + cfgWithHTTPConfigs.IdleConnTimeout = &idleConnTimeout + cfgWithHTTPConfigs.DisableKeepAlives = true + cfgWithHTTPConfigs.TLSSetting.InsecureSkipVerify = true + cfgWithHTTPConfigs.warnings = nil + tests := []struct { name string configMap *confmap.Conf - cfg Config + cfg *Config err string }{ { @@ -264,6 +327,21 @@ func TestUnmarshal(t *testing.T) { }), err: "\"metrics::sums::initial_cumulative_monotonic_value\" can only be configured when \"metrics::sums::cumulative_monotonic_mode\" is set to \"to_delta\"", }, + { + name: "unmarshall confighttp client configs", + configMap: confmap.NewFromStringMap(map[string]any{ + "read_buffer_size": 100, + "write_buffer_size": 200, + "timeout": "10s", + "max_idle_conns": 300, + "max_idle_conns_per_host": 150, + "max_conns_per_host": 250, + "disable_keep_alives": true, + "idle_conn_timeout": "30s", + "tls": map[string]any{"insecure_skip_verify": true}, + }), + cfg: cfgWithHTTPConfigs, + }, } f := NewFactory() diff --git a/exporter/datadogexporter/examples/collector.yaml b/exporter/datadogexporter/examples/collector.yaml index c0a3ceff9108..ec0ae11ae1bb 100644 --- a/exporter/datadogexporter/examples/collector.yaml +++ b/exporter/datadogexporter/examples/collector.yaml @@ -214,6 +214,27 @@ exporters: ## @param tls - boolean - optional - default: false # insecure_skip_verify: false + ## @param read_buffer_size - integer - optional + ## @param write_buffer_size - integer - optional + ## @param timeout - duration - optional + ## @param max_idle_conns - integer - optional + ## @param max_idle_conns_per_host - integer - optional + ## @param max_conns_per_host - integer - optional + ## @param idle_conn_timeout - duration - optional + ## @param disable_keep_alives - boolean - optional + ## + ## Below are a subset of configs in confighttp that are respected by Datadog exporter. + ## See https://pkg.go.dev/go.opentelemetry.io/collector/config/confighttp for details on these configs. + ## + # read_buffer_size: 0 + # write_buffer_size: 0 + # timeout: 15s + # max_idle_conns: 100 + # max_idle_conns_per_host: 0 + # max_conns_per_host: 0 + # idle_conn_timeout: 0s + # disable_keep_alives: false + ## @param metrics - custom object - optional ## Metric exporter specific configuration. # diff --git a/exporter/datadogexporter/examples/logs.yaml b/exporter/datadogexporter/examples/logs.yaml index 46d56acc690e..15d9ba66c001 100644 --- a/exporter/datadogexporter/examples/logs.yaml +++ b/exporter/datadogexporter/examples/logs.yaml @@ -23,6 +23,7 @@ processors: exporters: datadog: + idle_conn_timeout: 10s api: site: ${env:DD_SITE} key: ${env:DD_API_KEY} diff --git a/exporter/datadogexporter/factory.go b/exporter/datadogexporter/factory.go index cb9bcaf55697..26372229c40a 100644 --- a/exporter/datadogexporter/factory.go +++ b/exporter/datadogexporter/factory.go @@ -19,6 +19,7 @@ import ( "github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes" "github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes/source" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configretry" "go.opentelemetry.io/collector/consumer" @@ -170,8 +171,9 @@ func NewFactory() exporter.Factory { return newFactoryWithRegistry(featuregate.GlobalRegistry()) } -func defaulttimeoutSettings() exporterhelper.TimeoutSettings { - return exporterhelper.TimeoutSettings{ +func defaultClientConfig() confighttp.ClientConfig { + // do not use NewDefaultClientConfig for backwards-compatibility + return confighttp.ClientConfig{ Timeout: 15 * time.Second, } } @@ -179,9 +181,9 @@ func defaulttimeoutSettings() exporterhelper.TimeoutSettings { // createDefaultConfig creates the default exporter configuration func (f *factory) createDefaultConfig() component.Config { return &Config{ - TimeoutSettings: defaulttimeoutSettings(), - BackOffConfig: configretry.NewDefaultBackOffConfig(), - QueueSettings: exporterhelper.NewDefaultQueueSettings(), + ClientConfig: defaultClientConfig(), + BackOffConfig: configretry.NewDefaultBackOffConfig(), + QueueSettings: exporterhelper.NewDefaultQueueSettings(), API: APIConfig{ Site: "datadoghq.com", diff --git a/exporter/datadogexporter/factory_test.go b/exporter/datadogexporter/factory_test.go index 7e82bf1c6c7f..5e65c366a279 100644 --- a/exporter/datadogexporter/factory_test.go +++ b/exporter/datadogexporter/factory_test.go @@ -71,9 +71,9 @@ func TestCreateDefaultConfig(t *testing.T) { cfg := factory.CreateDefaultConfig() assert.Equal(t, &Config{ - TimeoutSettings: defaulttimeoutSettings(), - BackOffConfig: configretry.NewDefaultBackOffConfig(), - QueueSettings: exporterhelper.NewDefaultQueueSettings(), + ClientConfig: defaultClientConfig(), + BackOffConfig: configretry.NewDefaultBackOffConfig(), + QueueSettings: exporterhelper.NewDefaultQueueSettings(), API: APIConfig{ Site: "datadoghq.com", @@ -132,9 +132,9 @@ func TestLoadConfig(t *testing.T) { { id: component.NewIDWithName(metadata.Type, "default"), expected: &Config{ - TimeoutSettings: defaulttimeoutSettings(), - BackOffConfig: configretry.NewDefaultBackOffConfig(), - QueueSettings: exporterhelper.NewDefaultQueueSettings(), + ClientConfig: defaultClientConfig(), + BackOffConfig: configretry.NewDefaultBackOffConfig(), + QueueSettings: exporterhelper.NewDefaultQueueSettings(), API: APIConfig{ Key: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Site: "datadoghq.com", @@ -180,9 +180,9 @@ func TestLoadConfig(t *testing.T) { { id: component.NewIDWithName(metadata.Type, "api"), expected: &Config{ - TimeoutSettings: defaulttimeoutSettings(), - BackOffConfig: configretry.NewDefaultBackOffConfig(), - QueueSettings: exporterhelper.NewDefaultQueueSettings(), + ClientConfig: defaultClientConfig(), + BackOffConfig: configretry.NewDefaultBackOffConfig(), + QueueSettings: exporterhelper.NewDefaultQueueSettings(), TagsConfig: TagsConfig{ Hostname: "customhostname", }, @@ -235,9 +235,9 @@ func TestLoadConfig(t *testing.T) { { id: component.NewIDWithName(metadata.Type, "api2"), expected: &Config{ - TimeoutSettings: defaulttimeoutSettings(), - BackOffConfig: configretry.NewDefaultBackOffConfig(), - QueueSettings: exporterhelper.NewDefaultQueueSettings(), + ClientConfig: defaultClientConfig(), + BackOffConfig: configretry.NewDefaultBackOffConfig(), + QueueSettings: exporterhelper.NewDefaultQueueSettings(), TagsConfig: TagsConfig{ Hostname: "customhostname", }, @@ -620,9 +620,9 @@ func TestOnlyMetadata(t *testing.T) { factory := NewFactory() ctx := context.Background() cfg := &Config{ - TimeoutSettings: defaulttimeoutSettings(), - BackOffConfig: configretry.NewDefaultBackOffConfig(), - QueueSettings: exporterhelper.NewDefaultQueueSettings(), + ClientConfig: defaultClientConfig(), + BackOffConfig: configretry.NewDefaultBackOffConfig(), + QueueSettings: exporterhelper.NewDefaultQueueSettings(), API: APIConfig{Key: "notnull"}, Metrics: MetricsConfig{TCPAddrConfig: confignet.TCPAddrConfig{Endpoint: server.URL}}, diff --git a/exporter/datadogexporter/go.mod b/exporter/datadogexporter/go.mod index 64b4ce32df3a..9a8e953aec6c 100644 --- a/exporter/datadogexporter/go.mod +++ b/exporter/datadogexporter/go.mod @@ -18,6 +18,7 @@ require ( github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.21.0 github.com/aws/aws-sdk-go v1.50.27 github.com/cenkalti/backoff/v4 v4.2.1 + github.com/google/go-cmp v0.6.0 github.com/open-telemetry/opentelemetry-collector-contrib/connector/datadogconnector v0.96.0 github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/ecsutil v0.96.0 github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.96.0 @@ -35,9 +36,13 @@ require ( github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver v0.96.0 github.com/stretchr/testify v1.9.0 go.opentelemetry.io/collector/component v0.96.1-0.20240306115632-b2693620eff6 + go.opentelemetry.io/collector/config/configauth v0.96.1-0.20240306115632-b2693620eff6 + go.opentelemetry.io/collector/config/configcompression v0.96.1-0.20240306115632-b2693620eff6 + go.opentelemetry.io/collector/config/confighttp v0.96.1-0.20240306115632-b2693620eff6 go.opentelemetry.io/collector/config/confignet v0.96.1-0.20240306115632-b2693620eff6 go.opentelemetry.io/collector/config/configopaque v1.3.1-0.20240306115632-b2693620eff6 go.opentelemetry.io/collector/config/configretry v0.96.1-0.20240306115632-b2693620eff6 + go.opentelemetry.io/collector/config/configtls v0.96.1-0.20240306115632-b2693620eff6 go.opentelemetry.io/collector/confmap v0.96.1-0.20240306115632-b2693620eff6 go.opentelemetry.io/collector/connector v0.96.1-0.20240306115632-b2693620eff6 go.opentelemetry.io/collector/consumer v0.96.1-0.20240306115632-b2693620eff6 @@ -129,7 +134,6 @@ require ( github.com/golang/protobuf v1.5.3 // indirect github.com/golang/snappy v0.0.4 // indirect github.com/google/gnostic-models v0.6.8 // indirect - github.com/google/go-cmp v0.6.0 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/google/s2a-go v0.1.7 // indirect @@ -232,12 +236,8 @@ require ( github.com/zorkian/go-datadog-api v2.30.0+incompatible // indirect go.opencensus.io v0.24.0 // indirect go.opentelemetry.io/collector v0.96.1-0.20240306115632-b2693620eff6 // indirect - go.opentelemetry.io/collector/config/configauth v0.96.1-0.20240306115632-b2693620eff6 // indirect - go.opentelemetry.io/collector/config/configcompression v0.96.1-0.20240306115632-b2693620eff6 // indirect go.opentelemetry.io/collector/config/configgrpc v0.96.1-0.20240306115632-b2693620eff6 // indirect - go.opentelemetry.io/collector/config/confighttp v0.96.1-0.20240306115632-b2693620eff6 // indirect go.opentelemetry.io/collector/config/configtelemetry v0.96.1-0.20240306115632-b2693620eff6 // indirect - go.opentelemetry.io/collector/config/configtls v0.96.1-0.20240306115632-b2693620eff6 // indirect go.opentelemetry.io/collector/config/internal v0.96.1-0.20240306115632-b2693620eff6 // indirect go.opentelemetry.io/collector/confmap/converter/expandconverter v0.96.1-0.20240306115632-b2693620eff6 // indirect go.opentelemetry.io/collector/confmap/provider/envprovider v0.96.1-0.20240306115632-b2693620eff6 // indirect diff --git a/exporter/datadogexporter/hostmetadata.go b/exporter/datadogexporter/hostmetadata.go index 82fa74aa7dd5..1338bed2fb54 100644 --- a/exporter/datadogexporter/hostmetadata.go +++ b/exporter/datadogexporter/hostmetadata.go @@ -16,7 +16,7 @@ func newMetadataConfigfromConfig(cfg *Config) hostmetadata.PusherConfig { APIKey: string(cfg.API.Key), UseResourceMetadata: cfg.HostMetadata.HostnameSource == HostnameSourceFirstResource, InsecureSkipVerify: cfg.TLSSetting.InsecureSkipVerify, - TimeoutSettings: cfg.TimeoutSettings, + ClientConfig: cfg.ClientConfig, RetrySettings: cfg.BackOffConfig, } } diff --git a/exporter/datadogexporter/internal/clientutil/api.go b/exporter/datadogexporter/internal/clientutil/api.go index 616627d9b9f0..60bbaeb36dec 100644 --- a/exporter/datadogexporter/internal/clientutil/api.go +++ b/exporter/datadogexporter/internal/clientutil/api.go @@ -11,7 +11,7 @@ import ( "github.com/DataDog/datadog-api-client-go/v2/api/datadogV1" "github.com/DataDog/datadog-api-client-go/v2/api/datadogV2" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/exporter/exporterhelper" + "go.opentelemetry.io/collector/config/confighttp" "go.uber.org/zap" zorkian "gopkg.in/zorkian/go-datadog-api.v2" ) @@ -20,10 +20,10 @@ import ( var GZipSubmitMetricsOptionalParameters = datadogV2.NewSubmitMetricsOptionalParameters().WithContentEncoding(datadogV2.METRICCONTENTENCODING_GZIP) // CreateAPIClient creates a new Datadog API client -func CreateAPIClient(buildInfo component.BuildInfo, endpoint string, settings exporterhelper.TimeoutSettings, insecureSkipVerify bool) *datadog.APIClient { +func CreateAPIClient(buildInfo component.BuildInfo, endpoint string, hcs confighttp.ClientConfig) *datadog.APIClient { configuration := datadog.NewConfiguration() configuration.UserAgent = UserAgent(buildInfo) - configuration.HTTPClient = NewHTTPClient(settings, insecureSkipVerify) + configuration.HTTPClient = NewHTTPClient(hcs) configuration.Compress = true configuration.Servers = datadog.ServerConfigurations{ { diff --git a/exporter/datadogexporter/internal/clientutil/http.go b/exporter/datadogexporter/internal/clientutil/http.go index 2b19d3c22d67..4b69ebdef6e3 100644 --- a/exporter/datadogexporter/internal/clientutil/http.go +++ b/exporter/datadogexporter/internal/clientutil/http.go @@ -11,7 +11,7 @@ import ( "time" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/exporter/exporterhelper" + "go.opentelemetry.io/collector/config/confighttp" ) var ( @@ -27,21 +27,50 @@ var ( } ) -// NewHTTPClient returns a http.Client configured with the Agent options. -func NewHTTPClient(settings exporterhelper.TimeoutSettings, insecureSkipVerify bool) *http.Client { +// NewHTTPClient returns a http.Client configured with a subset of the confighttp.ClientConfig options. +func NewHTTPClient(hcs confighttp.ClientConfig) *http.Client { + transport := http.Transport{ + Proxy: http.ProxyFromEnvironment, + // Default values consistent with https://github.com/DataDog/datadog-agent/blob/f9ae7f4b842f83b23b2dfe3f15d31f9e6b12e857/pkg/util/http/transport.go#L91-L106 + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + // Enables TCP keepalives to detect broken connections + KeepAlive: 30 * time.Second, + // Disable RFC 6555 Fast Fallback ("Happy Eyeballs") + FallbackDelay: -1 * time.Nanosecond, + }).DialContext, + MaxIdleConns: 100, + MaxIdleConnsPerHost: 5, + // This parameter is set to avoid connections sitting idle in the pool indefinitely + IdleConnTimeout: 45 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + // Not supported by intake + ForceAttemptHTTP2: false, + TLSClientConfig: &tls.Config{InsecureSkipVerify: hcs.TLSSetting.InsecureSkipVerify}, + } + if hcs.ReadBufferSize > 0 { + transport.ReadBufferSize = hcs.ReadBufferSize + } + if hcs.WriteBufferSize > 0 { + transport.WriteBufferSize = hcs.WriteBufferSize + } + if hcs.MaxIdleConns != nil { + transport.MaxIdleConns = *hcs.MaxIdleConns + } + if hcs.MaxIdleConnsPerHost != nil { + transport.MaxIdleConnsPerHost = *hcs.MaxIdleConnsPerHost + } + if hcs.MaxConnsPerHost != nil { + transport.MaxConnsPerHost = *hcs.MaxConnsPerHost + } + if hcs.IdleConnTimeout != nil { + transport.IdleConnTimeout = *hcs.IdleConnTimeout + } + transport.DisableKeepAlives = hcs.DisableKeepAlives return &http.Client{ - Timeout: settings.Timeout, - Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, - DialContext: (&net.Dialer{ - // Disable RFC 6555 Fast Fallback ("Happy Eyeballs") - FallbackDelay: -1 * time.Nanosecond, - }).DialContext, - MaxIdleConns: 100, - // Not supported by intake - ForceAttemptHTTP2: false, - TLSClientConfig: &tls.Config{InsecureSkipVerify: insecureSkipVerify}, - }, + Timeout: hcs.Timeout, + Transport: &transport, } } diff --git a/exporter/datadogexporter/internal/clientutil/http_test.go b/exporter/datadogexporter/internal/clientutil/http_test.go index db6e02360a46..db2eea93cb50 100644 --- a/exporter/datadogexporter/internal/clientutil/http_test.go +++ b/exporter/datadogexporter/internal/clientutil/http_test.go @@ -4,11 +4,18 @@ package clientutil // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter/internal/clientutil" import ( + "crypto/tls" "net/http" "testing" + "time" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configcompression" + "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/configtls" ) var ( @@ -18,6 +25,73 @@ var ( } ) +func TestNewHTTPClient(t *testing.T) { + hcsEmpty := confighttp.ClientConfig{} + client1 := NewHTTPClient(hcsEmpty) + defaultTransport := &http.Transport{ + MaxIdleConns: 100, + MaxIdleConnsPerHost: 5, + IdleConnTimeout: 45 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + ForceAttemptHTTP2: false, + TLSClientConfig: &tls.Config{InsecureSkipVerify: false}, + } + if diff := cmp.Diff( + defaultTransport, + client1.Transport.(*http.Transport), + cmpopts.IgnoreUnexported(http.Transport{}, tls.Config{}), + cmpopts.IgnoreFields(http.Transport{}, "Proxy", "DialContext")); diff != "" { + t.Errorf("Mismatched transports -want +got %s", diff) + } + assert.Equal(t, time.Duration(0), client1.Timeout) + + idleConnTimeout := 30 * time.Second + maxIdleConn := 300 + maxIdleConnPerHost := 150 + maxConnPerHost := 250 + hcs := confighttp.ClientConfig{ + ReadBufferSize: 100, + WriteBufferSize: 200, + Timeout: 10 * time.Second, + IdleConnTimeout: &idleConnTimeout, + MaxIdleConns: &maxIdleConn, + MaxIdleConnsPerHost: &maxIdleConnPerHost, + MaxConnsPerHost: &maxConnPerHost, + DisableKeepAlives: true, + TLSSetting: configtls.ClientConfig{InsecureSkipVerify: true}, + + // The rest are ignored + Endpoint: "endpoint", + ProxyURL: "proxy", + Compression: configcompression.TypeSnappy, + HTTP2ReadIdleTimeout: 15 * time.Second, + HTTP2PingTimeout: 20 * time.Second, + } + client2 := NewHTTPClient(hcs) + expectedTransport := &http.Transport{ + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + ReadBufferSize: 100, + WriteBufferSize: 200, + MaxIdleConns: maxIdleConn, + MaxIdleConnsPerHost: maxIdleConnPerHost, + MaxConnsPerHost: maxConnPerHost, + IdleConnTimeout: idleConnTimeout, + DisableKeepAlives: true, + ForceAttemptHTTP2: false, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + if diff := cmp.Diff( + expectedTransport, + client2.Transport.(*http.Transport), + cmpopts.IgnoreUnexported(http.Transport{}, tls.Config{}), + cmpopts.IgnoreFields(http.Transport{}, "Proxy", "DialContext")); diff != "" { + t.Errorf("Mismatched transports -want +got %s", diff) + } + assert.Equal(t, 10*time.Second, client2.Timeout) +} + func TestUserAgent(t *testing.T) { assert.Equal(t, UserAgent(buildInfo), "otelcontribcol/1.0") diff --git a/exporter/datadogexporter/internal/hostmetadata/config.go b/exporter/datadogexporter/internal/hostmetadata/config.go index 330e792abcc4..d5c4a3e34160 100644 --- a/exporter/datadogexporter/internal/hostmetadata/config.go +++ b/exporter/datadogexporter/internal/hostmetadata/config.go @@ -4,8 +4,8 @@ package hostmetadata // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter/internal/hostmetadata" import ( + "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/configretry" - "go.opentelemetry.io/collector/exporter/exporterhelper" ) // PusherConfig is the configuration for the metadata pusher goroutine. @@ -22,8 +22,8 @@ type PusherConfig struct { UseResourceMetadata bool // InsecureSkipVerify is the value of `tls.insecure_skip_verify` on the configuration. InsecureSkipVerify bool - // TimeoutSettings of exporter. - TimeoutSettings exporterhelper.TimeoutSettings + // ClientConfig of exporter. + ClientConfig confighttp.ClientConfig // RetrySettings of exporter. RetrySettings configretry.BackOffConfig } diff --git a/exporter/datadogexporter/internal/hostmetadata/metadata.go b/exporter/datadogexporter/internal/hostmetadata/metadata.go index ed4ea8626c77..700f20269127 100644 --- a/exporter/datadogexporter/internal/hostmetadata/metadata.go +++ b/exporter/datadogexporter/internal/hostmetadata/metadata.go @@ -144,7 +144,7 @@ func NewPusher(params exporter.CreateSettings, pcfg PusherConfig) inframetadata. params: params, pcfg: pcfg, retrier: clientutil.NewRetrier(params.Logger, pcfg.RetrySettings, scrub.NewScrubber()), - httpClient: clientutil.NewHTTPClient(pcfg.TimeoutSettings, pcfg.InsecureSkipVerify), + httpClient: clientutil.NewHTTPClient(pcfg.ClientConfig), } } diff --git a/exporter/datadogexporter/internal/logs/sender.go b/exporter/datadogexporter/internal/logs/sender.go index 016bc68da510..7aa6d26cec4d 100644 --- a/exporter/datadogexporter/internal/logs/sender.go +++ b/exporter/datadogexporter/internal/logs/sender.go @@ -8,7 +8,7 @@ import ( "github.com/DataDog/datadog-api-client-go/v2/api/datadog" "github.com/DataDog/datadog-api-client-go/v2/api/datadogV2" - "go.opentelemetry.io/collector/exporter/exporterhelper" + "go.opentelemetry.io/collector/config/confighttp" "go.uber.org/zap" "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter/internal/clientutil" @@ -27,7 +27,7 @@ type Sender struct { const logsV2 = "v2.LogsApi.SubmitLog" // NewSender creates a new Sender -func NewSender(endpoint string, logger *zap.Logger, s exporterhelper.TimeoutSettings, insecureSkipVerify, verbose bool, apiKey string) *Sender { +func NewSender(endpoint string, logger *zap.Logger, hcs confighttp.ClientConfig, verbose bool, apiKey string) *Sender { cfg := datadog.NewConfiguration() logger.Info("Logs sender initialized", zap.String("endpoint", endpoint)) cfg.OperationServers[logsV2] = datadog.ServerConfigurations{ @@ -35,7 +35,7 @@ func NewSender(endpoint string, logger *zap.Logger, s exporterhelper.TimeoutSett URL: endpoint, }, } - cfg.HTTPClient = clientutil.NewHTTPClient(s, insecureSkipVerify) + cfg.HTTPClient = clientutil.NewHTTPClient(hcs) cfg.AddDefaultHeader("DD-API-KEY", apiKey) apiClient := datadog.NewAPIClient(cfg) return &Sender{ diff --git a/exporter/datadogexporter/internal/logs/sender_test.go b/exporter/datadogexporter/internal/logs/sender_test.go index fe95e1360162..cb74c4abf785 100644 --- a/exporter/datadogexporter/internal/logs/sender_test.go +++ b/exporter/datadogexporter/internal/logs/sender_test.go @@ -12,7 +12,8 @@ import ( "github.com/DataDog/datadog-api-client-go/v2/api/datadog" "github.com/DataDog/datadog-api-client-go/v2/api/datadogV2" "github.com/stretchr/testify/assert" - "go.opentelemetry.io/collector/exporter/exporterhelper" + "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap/zaptest" "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter/internal/testutil" @@ -191,7 +192,7 @@ func TestSubmitLogs(t *testing.T) { } }) defer server.Close() - s := NewSender(server.URL, logger, exporterhelper.TimeoutSettings{Timeout: time.Second * 10}, true, true, "") + s := NewSender(server.URL, logger, confighttp.ClientConfig{Timeout: time.Second * 10, TLSSetting: configtls.ClientConfig{InsecureSkipVerify: true}}, true, "") if err := s.SubmitLogs(context.Background(), tt.payload); err != nil { t.Fatal(err) } diff --git a/exporter/datadogexporter/logs_exporter.go b/exporter/datadogexporter/logs_exporter.go index 58c8de9275f2..37d0a52b1675 100644 --- a/exporter/datadogexporter/logs_exporter.go +++ b/exporter/datadogexporter/logs_exporter.go @@ -56,8 +56,7 @@ func newLogsExporter( apiClient := clientutil.CreateAPIClient( params.BuildInfo, cfg.Metrics.TCPAddrConfig.Endpoint, - cfg.TimeoutSettings, - cfg.LimitedClientConfig.TLSSetting.InsecureSkipVerify) + cfg.ClientConfig) go func() { errchan <- clientutil.ValidateAPIKey(ctx, string(cfg.API.Key), params.Logger, apiClient) }() } else { client := clientutil.CreateZorkianClient(string(cfg.API.Key), cfg.Metrics.TCPAddrConfig.Endpoint) @@ -74,7 +73,7 @@ func newLogsExporter( if err != nil { return nil, fmt.Errorf("failed to create logs translator: %w", err) } - s := logs.NewSender(cfg.Logs.TCPAddrConfig.Endpoint, params.Logger, cfg.TimeoutSettings, cfg.LimitedClientConfig.TLSSetting.InsecureSkipVerify, cfg.Logs.DumpPayloads, string(cfg.API.Key)) + s := logs.NewSender(cfg.Logs.TCPAddrConfig.Endpoint, params.Logger, cfg.ClientConfig, cfg.Logs.DumpPayloads, string(cfg.API.Key)) return &logsExporter{ params: params, diff --git a/exporter/datadogexporter/metrics_exporter.go b/exporter/datadogexporter/metrics_exporter.go index 332bffd9ed14..94709256d1b5 100644 --- a/exporter/datadogexporter/metrics_exporter.go +++ b/exporter/datadogexporter/metrics_exporter.go @@ -126,14 +126,13 @@ func newMetricsExporter( apiClient := clientutil.CreateAPIClient( params.BuildInfo, cfg.Metrics.TCPAddrConfig.Endpoint, - cfg.TimeoutSettings, - cfg.LimitedClientConfig.TLSSetting.InsecureSkipVerify) + cfg.ClientConfig) go func() { errchan <- clientutil.ValidateAPIKey(ctx, string(cfg.API.Key), params.Logger, apiClient) }() exporter.metricsAPI = datadogV2.NewMetricsApi(apiClient) } else { client := clientutil.CreateZorkianClient(string(cfg.API.Key), cfg.Metrics.TCPAddrConfig.Endpoint) client.ExtraHeader["User-Agent"] = clientutil.UserAgent(params.BuildInfo) - client.HttpClient = clientutil.NewHTTPClient(cfg.TimeoutSettings, cfg.LimitedClientConfig.TLSSetting.InsecureSkipVerify) + client.HttpClient = clientutil.NewHTTPClient(cfg.ClientConfig) go func() { errchan <- clientutil.ValidateAPIKeyZorkian(params.Logger, client) }() exporter.client = client } diff --git a/exporter/datadogexporter/traces_exporter.go b/exporter/datadogexporter/traces_exporter.go index d64af8958b67..05088b2f3db9 100644 --- a/exporter/datadogexporter/traces_exporter.go +++ b/exporter/datadogexporter/traces_exporter.go @@ -73,8 +73,7 @@ func newTracesExporter( apiClient := clientutil.CreateAPIClient( params.BuildInfo, cfg.Metrics.TCPAddrConfig.Endpoint, - cfg.TimeoutSettings, - cfg.LimitedClientConfig.TLSSetting.InsecureSkipVerify) + cfg.ClientConfig) go func() { errchan <- clientutil.ValidateAPIKey(ctx, string(cfg.API.Key), params.Logger, apiClient) }() exp.metricsAPI = datadogV2.NewMetricsApi(apiClient) } else { @@ -207,7 +206,7 @@ func newTraceAgentConfig(ctx context.Context, params exporter.CreateSettings, cf acfg.Ignore["resource"] = cfg.Traces.IgnoreResources acfg.ReceiverPort = 0 // disable HTTP receiver acfg.AgentVersion = fmt.Sprintf("datadogexporter-%s-%s", params.BuildInfo.Command, params.BuildInfo.Version) - acfg.SkipSSLValidation = cfg.LimitedClientConfig.TLSSetting.InsecureSkipVerify + acfg.SkipSSLValidation = cfg.ClientConfig.TLSSetting.InsecureSkipVerify acfg.ComputeStatsBySpanKind = cfg.Traces.ComputeStatsBySpanKind acfg.PeerServiceAggregation = cfg.Traces.PeerServiceAggregation acfg.PeerTagsAggregation = cfg.Traces.PeerTagsAggregation