diff --git a/config/confighttp/client.go b/config/confighttp/client.go index ec16cc293d5..9822841824c 100644 --- a/config/confighttp/client.go +++ b/config/confighttp/client.go @@ -14,6 +14,7 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel" + "go.uber.org/zap" "golang.org/x/net/http2" "golang.org/x/net/publicsuffix" @@ -24,6 +25,7 @@ import ( "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/confmap" ) const ( @@ -67,30 +69,10 @@ type ClientConfig struct { // Advanced configuration options for the Compression CompressionParams configcompression.CompressionParams `mapstructure:"compression_params,omitempty"` - // MaxIdleConns is used to set a limit to the maximum idle HTTP connections the client can keep open. - // By default, it is set to 100. Zero means no limit. - MaxIdleConns int `mapstructure:"max_idle_conns"` - - // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connections the host can keep open. - // If zero, [net/http.DefaultMaxIdleConnsPerHost] is used. - MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host,omitempty"` - // MaxConnsPerHost limits the total number of connections per host, including connections in the dialing, // active, and idle states. Default is 0 (unlimited). MaxConnsPerHost int `mapstructure:"max_conns_per_host,omitempty"` - // IdleConnTimeout is the maximum amount of time a connection will remain open before closing itself. - // By default, it is set to 90 seconds. - IdleConnTimeout time.Duration `mapstructure:"idle_conn_timeout"` - - // DisableKeepAlives, if true, disables HTTP keep-alives and will only use the connection to the server - // for a single HTTP request. - // - // WARNING: enabling this option can result in significant overhead establishing a new HTTP(S) - // connection for every request. Before enabling this option please consider whether changes - // to idle connection settings can achieve your goal. - DisableKeepAlives bool `mapstructure:"disable_keep_alives,omitempty"` - // This is needed in case you run into // https://github.com/golang/go/issues/59690 // https://github.com/golang/go/issues/36026 @@ -112,6 +94,92 @@ type ClientConfig struct { // Middleware handlers are called in the order they appear in this list, // with the first middleware becoming the outermost handler. Middlewares []configmiddleware.Config `mapstructure:"middlewares,omitempty"` + + // Keepalive configuration. + Keepalive configoptional.Optional[KeepaliveClientConfig] `mapstructure:"keepalive,omitempty"` + + warnings []renamedField +} + +type renamedField struct { + old string + new string +} + +func (f *renamedField) Log(logger *zap.Logger) { + logger.Warn("Use of deprecated configuration option", + zap.String("old name", f.old), + zap.String("new name", f.new), + ) +} + +func (cc *ClientConfig) Unmarshal(conf *confmap.Conf) error { + type oldFields struct { + IdleConnTimeout time.Duration `mapstructure:"idle_conn_timeout"` + MaxIdleConns int `mapstructure:"max_idle_conns"` + MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host,omitempty"` + DisableKeepAlives bool `mapstructure:"disable_keep_alives,omitempty"` + } + + type clientConfigFields ClientConfig + + type legacyConfig struct { + clientConfigFields `mapstructure:",squash"` + oldFields `mapstructure:",squash"` + } + + var cfg legacyConfig + cfg.clientConfigFields = clientConfigFields(*cc) + defaultTransport := http.DefaultTransport.(*http.Transport) + cfg.oldFields = oldFields{ + MaxIdleConns: defaultTransport.MaxIdleConns, + IdleConnTimeout: defaultTransport.IdleConnTimeout, + DisableKeepAlives: false, + } + if err := conf.Unmarshal(&cfg, confmap.WithIgnoreUnused()); err != nil { + return err + } + + deprecatedFields := []renamedField{ + {"idle_conn_timeout", "keepalive::idle_conn_timeout"}, + {"max_idle_conns", "keepalive::max_idle_conns"}, + {"disable_keep_alives", "keepalive::enabled"}, + {"max_idle_conns_per_host", "keepalive::max_idle_conns_per_host"}, + } + + var warnings []renamedField + for _, field := range deprecatedFields { + if conf.IsSet(field.old) { + warnings = append(warnings, field) + } + } + + if len(warnings) > 0 && conf.IsSet("keepalive") { + return errors.New("confighttp.ClientConfig: cannot use legacy fields and new 'keepalive' section") + } + + if cfg.DisableKeepAlives { + cfg.Keepalive = configoptional.None[KeepaliveClientConfig]() + } else { + // should never happen with default values + if !cfg.Keepalive.HasValue() { + cfg.Keepalive = configoptional.Some(KeepaliveClientConfig{}) + } + + if conf.IsSet("idle_conn_timeout") { + cfg.Keepalive.Get().IdleConnTimeout = cfg.IdleConnTimeout + } + if conf.IsSet("max_idle_conns") { + cfg.Keepalive.Get().MaxIdleConns = cfg.MaxIdleConns + } + if conf.IsSet("max_idle_conns_per_host") { + cfg.Keepalive.Get().MaxIdleConnsPerHost = cfg.MaxIdleConnsPerHost + } + } + + *cc = ClientConfig(cfg.clientConfigFields) + cc.warnings = warnings + return nil } // CookiesConfig defines the configuration of the HTTP client regarding cookies served by the server. @@ -119,6 +187,23 @@ type CookiesConfig struct { _ struct{} } +// KeepaliveClientConfig describes the keepalive configuration. +type KeepaliveClientConfig struct { + _ struct{} + + // IdleConnTimeout is the maximum amount of time an iddle (keep-alive) connection will remain open before closing itself. + // By default, it is set to 90 seconds. + IdleConnTimeout time.Duration `mapstructure:"idle_conn_timeout"` + + // MaxIdleConns is used to set a limit to the maximum idle HTTP connections the client can keep open. + // By default, it is set to 100. Zero means no limit. + MaxIdleConns int `mapstructure:"max_idle_conns"` + + // MaxIdleConnsPerHost is used to set a limit to the maximum idle HTTP connections the host can keep open. + // If zero, [net/http.DefaultMaxIdleConnsPerHost] is used. + MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host,omitempty"` +} + // NewDefaultClientConfig returns ClientConfig type object with // the default values of 'MaxIdleConns' and 'IdleConnTimeout', as well as [http.DefaultTransport] values. // Other config options are not added as they are initialized with 'zero value' by GoLang as default. @@ -128,8 +213,10 @@ func NewDefaultClientConfig() ClientConfig { defaultTransport := http.DefaultTransport.(*http.Transport) return ClientConfig{ - MaxIdleConns: defaultTransport.MaxIdleConns, - IdleConnTimeout: defaultTransport.IdleConnTimeout, + Keepalive: configoptional.Some(KeepaliveClientConfig{ + MaxIdleConns: defaultTransport.MaxIdleConns, + IdleConnTimeout: defaultTransport.IdleConnTimeout, + }), ForceAttemptHTTP2: true, } } @@ -156,7 +243,11 @@ type ToClientOption interface { // the `extensions` argument should be the output of `host.GetExtensions()`. // It may also be `nil` in tests where no such extension is expected to be used. func (cc *ClientConfig) ToClient(ctx context.Context, extensions map[component.ID]component.Component, settings component.TelemetrySettings, _ ...ToClientOption) (*http.Client, error) { - tlsCfg, err := cc.TLS.LoadTLSConfig(ctx) + for _, field := range cc.warnings { + field.Log(settings.Logger) + } + + tlsCfg, err := cc.TLS.LoadTLSConfig(ctx) if err != nil { return nil, err } @@ -171,12 +262,13 @@ func (cc *ClientConfig) ToClient(ctx context.Context, extensions map[component.I transport.WriteBufferSize = cc.WriteBufferSize } - transport.MaxIdleConns = cc.MaxIdleConns - transport.MaxIdleConnsPerHost = cc.MaxIdleConnsPerHost + if kaCfg := cc.Keepalive.Get(); cc.Keepalive.HasValue() { + transport.MaxIdleConns = kaCfg.MaxIdleConns + transport.MaxIdleConnsPerHost = kaCfg.MaxIdleConnsPerHost + transport.IdleConnTimeout = kaCfg.IdleConnTimeout + } transport.MaxConnsPerHost = cc.MaxConnsPerHost - transport.IdleConnTimeout = cc.IdleConnTimeout transport.ForceAttemptHTTP2 = cc.ForceAttemptHTTP2 - // Setting the Proxy URL if cc.ProxyURL != "" { proxyURL, parseErr := url.ParseRequestURI(cc.ProxyURL) @@ -186,7 +278,7 @@ func (cc *ClientConfig) ToClient(ctx context.Context, extensions map[component.I transport.Proxy = http.ProxyURL(proxyURL) } - transport.DisableKeepAlives = cc.DisableKeepAlives + transport.DisableKeepAlives = !cc.Keepalive.HasValue() if cc.HTTP2ReadIdleTimeout > 0 { transport2, transportErr := http2.ConfigureTransports(transport) diff --git a/config/confighttp/client_test.go b/config/confighttp/client_test.go index 65423c76bb2..42f132f374d 100644 --- a/config/confighttp/client_test.go +++ b/config/confighttp/client_test.go @@ -65,15 +65,15 @@ func TestAllHTTPClientSettings(t *testing.T) { TLS: configtls.ClientConfig{ Insecure: false, }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - MaxIdleConns: maxIdleConns, - MaxIdleConnsPerHost: maxIdleConnsPerHost, - MaxConnsPerHost: maxConnsPerHost, - IdleConnTimeout: idleConnTimeout, - Compression: "", - DisableKeepAlives: true, - Cookies: configoptional.Some(CookiesConfig{}), + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxConnsPerHost: maxConnsPerHost, + Compression: "", + Keepalive: configoptional.Some(KeepaliveClientConfig{ + MaxIdleConns: maxIdleConns, + MaxIdleConnsPerHost: maxIdleConnsPerHost, + IdleConnTimeout: idleConnTimeout, + }), Cookies: configoptional.Some(CookiesConfig{}), HTTP2ReadIdleTimeout: idleConnTimeout, HTTP2PingTimeout: http2PingTimeout, }, @@ -86,15 +86,16 @@ func TestAllHTTPClientSettings(t *testing.T) { TLS: configtls.ClientConfig{ Insecure: false, }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - MaxIdleConns: maxIdleConns, - MaxIdleConnsPerHost: maxIdleConnsPerHost, - MaxConnsPerHost: maxConnsPerHost, - ForceAttemptHTTP2: true, - IdleConnTimeout: idleConnTimeout, - Compression: "", - DisableKeepAlives: true, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxConnsPerHost: maxConnsPerHost, + ForceAttemptHTTP2: true, + Compression: "", + Keepalive: configoptional.Some(KeepaliveClientConfig{ + MaxIdleConns: maxIdleConns, + MaxIdleConnsPerHost: maxIdleConnsPerHost, + IdleConnTimeout: idleConnTimeout, + }), Cookies: configoptional.Some(CookiesConfig{}), HTTP2ReadIdleTimeout: idleConnTimeout, HTTP2PingTimeout: http2PingTimeout, @@ -108,14 +109,16 @@ func TestAllHTTPClientSettings(t *testing.T) { TLS: configtls.ClientConfig{ Insecure: false, }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - MaxIdleConns: maxIdleConns, - MaxIdleConnsPerHost: maxIdleConnsPerHost, - MaxConnsPerHost: maxConnsPerHost, - IdleConnTimeout: idleConnTimeout, - Compression: "none", - DisableKeepAlives: true, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxConnsPerHost: maxConnsPerHost, + + Compression: "none", + Keepalive: configoptional.Some(KeepaliveClientConfig{ + MaxIdleConns: maxIdleConns, + MaxIdleConnsPerHost: maxIdleConnsPerHost, + IdleConnTimeout: idleConnTimeout, + }), HTTP2ReadIdleTimeout: idleConnTimeout, HTTP2PingTimeout: http2PingTimeout, }, @@ -128,14 +131,15 @@ func TestAllHTTPClientSettings(t *testing.T) { TLS: configtls.ClientConfig{ Insecure: false, }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - MaxIdleConns: maxIdleConns, - MaxIdleConnsPerHost: maxIdleConnsPerHost, - MaxConnsPerHost: maxConnsPerHost, - IdleConnTimeout: idleConnTimeout, - Compression: "gzip", - DisableKeepAlives: true, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxConnsPerHost: maxConnsPerHost, + Compression: "gzip", + Keepalive: configoptional.Some(KeepaliveClientConfig{ + MaxIdleConns: maxIdleConns, + MaxIdleConnsPerHost: maxIdleConnsPerHost, + IdleConnTimeout: idleConnTimeout, + }), HTTP2ReadIdleTimeout: idleConnTimeout, HTTP2PingTimeout: http2PingTimeout, }, @@ -148,14 +152,15 @@ func TestAllHTTPClientSettings(t *testing.T) { TLS: configtls.ClientConfig{ Insecure: false, }, - ReadBufferSize: 1024, - WriteBufferSize: 512, - MaxIdleConns: maxIdleConns, - MaxIdleConnsPerHost: maxIdleConnsPerHost, - MaxConnsPerHost: maxConnsPerHost, - IdleConnTimeout: idleConnTimeout, - Compression: "gzip", - DisableKeepAlives: true, + ReadBufferSize: 1024, + WriteBufferSize: 512, + MaxConnsPerHost: maxConnsPerHost, + Compression: "gzip", + Keepalive: configoptional.Some(KeepaliveClientConfig{ + MaxIdleConns: maxIdleConns, + MaxIdleConnsPerHost: maxIdleConnsPerHost, + IdleConnTimeout: idleConnTimeout, + }), HTTP2ReadIdleTimeout: idleConnTimeout, HTTP2PingTimeout: http2PingTimeout, }, @@ -181,7 +186,7 @@ func TestAllHTTPClientSettings(t *testing.T) { assert.Equal(t, 40, transport.MaxIdleConnsPerHost) assert.Equal(t, 45, transport.MaxConnsPerHost) assert.Equal(t, 30*time.Second, transport.IdleConnTimeout) - assert.True(t, transport.DisableKeepAlives) + assert.False(t, transport.DisableKeepAlives) case *compressRoundTripper: assert.EqualValues(t, "gzip", transport.compressionType) } @@ -208,6 +213,7 @@ func TestPartialHTTPClientSettings(t *testing.T) { }, ReadBufferSize: 1024, WriteBufferSize: 512, + Keepalive: configoptional.Some(KeepaliveClientConfig{}), }, shouldError: false, }, @@ -233,8 +239,8 @@ func TestPartialHTTPClientSettings(t *testing.T) { func TestDefaultHTTPClientSettings(t *testing.T) { httpClientSettings := NewDefaultClientConfig() - assert.Equal(t, 100, httpClientSettings.MaxIdleConns) - assert.Equal(t, 90*time.Second, httpClientSettings.IdleConnTimeout) + assert.Equal(t, 100, httpClientSettings.Keepalive.Get().MaxIdleConns) + assert.Equal(t, 90*time.Second, httpClientSettings.Keepalive.Get().IdleConnTimeout) } func TestProxyURL(t *testing.T) { @@ -566,10 +572,12 @@ func TestHttpTransportOptions(t *testing.T) { settings.TracerProvider = nil clientConfig := NewDefaultClientConfig() - clientConfig.MaxIdleConns = 100 - clientConfig.IdleConnTimeout = time.Duration(100) + clientConfig.Keepalive = configoptional.Some(KeepaliveClientConfig{ + MaxIdleConns: 100, + IdleConnTimeout: time.Duration(100), + MaxIdleConnsPerHost: 100, + }) clientConfig.MaxConnsPerHost = 100 - clientConfig.MaxIdleConnsPerHost = 100 client, err := clientConfig.ToClient(context.Background(), nil, settings) require.NoError(t, err) transport, ok := client.Transport.(*http.Transport) @@ -580,10 +588,11 @@ func TestHttpTransportOptions(t *testing.T) { require.Equal(t, 100, transport.MaxIdleConnsPerHost) clientConfig = NewDefaultClientConfig() - clientConfig.MaxIdleConns = 0 - clientConfig.IdleConnTimeout = 0 - clientConfig.MaxConnsPerHost = 0 - clientConfig.IdleConnTimeout = time.Duration(0) + clientConfig.Keepalive = configoptional.Some(KeepaliveClientConfig{ + MaxIdleConns: 0, + IdleConnTimeout: 0, + MaxIdleConnsPerHost: 0, + }) client, err = clientConfig.ToClient(context.Background(), nil, settings) require.NoError(t, err) transport, ok = client.Transport.(*http.Transport) diff --git a/config/confighttp/server.go b/config/confighttp/server.go index 57a7feebdf7..6ac3fe0ad79 100644 --- a/config/confighttp/server.go +++ b/config/confighttp/server.go @@ -26,6 +26,7 @@ import ( "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/extension/extensionauth" ) @@ -83,20 +84,84 @@ type ServerConfig struct { // A zero or negative value means there will be no timeout. WriteTimeout time.Duration `mapstructure:"write_timeout"` - // IdleTimeout is the maximum amount of time to wait for the - // next request when keep-alives are enabled. If IdleTimeout - // is zero, the value of ReadTimeout is used. If both are - // zero, there is no timeout. - IdleTimeout time.Duration `mapstructure:"idle_timeout"` - // Middlewares are used to add custom functionality to the HTTP server. // Middleware handlers are called in the order they appear in this list, // with the first middleware becoming the outermost handler. Middlewares []configmiddleware.Config `mapstructure:"middlewares,omitempty"` - // KeepAlivesEnabled controls whether HTTP keep-alives are enabled. + // Keepalive controls HTTP keep-alives. // By default, keep-alives are always enabled. Only very resource-constrained environments should disable them. - KeepAlivesEnabled bool `mapstructure:"keep_alives_enabled,omitempty"` + Keepalive configoptional.Optional[KeepaliveServerConfig] `mapstructure:"keepalive,omitempty"` + + renamedFields []renamedField +} + +func (sc *ServerConfig) Unmarshal(conf *confmap.Conf) error { + type oldFields struct { + IdleTimeout time.Duration `mapstructure:"idle_timeout"` + KeepAlivesEnabled bool `mapstructure:"keep_alives_enabled,omitempty"` + } + + type serverConfigFields ServerConfig + + type legacyConfig struct { + serverConfigFields `mapstructure:",squash"` + oldFields `mapstructure:",squash"` + } + + var cfg legacyConfig + cfg.serverConfigFields = serverConfigFields(*sc) + cfg.oldFields = oldFields{ + IdleTimeout: 1 * time.Minute, + KeepAlivesEnabled: true, + } + if err := conf.Unmarshal(&cfg, confmap.WithIgnoreUnused()); err != nil { + return err + } + + deprecatedFields := []struct { + old, new string + }{ + {"idle_timeout", "keepalive::idle_timeout"}, + {"keep_alives_enabled", "keepalive::enabled"}, + } + + var warnings []renamedField + for _, field := range deprecatedFields { + if conf.IsSet(field.old) { + warnings = append(warnings, field) + } + } + + if len(warnings) > 0 && conf.IsSet("keepalive") { + return errors.New("confighttp.ClientConfig: cannot use legacy fields and new 'keepalive' section") + } + + if !cfg.KeepAlivesEnabled { + // should never happen with default values + cfg.Keepalive = configoptional.None[KeepaliveServerConfig]() + } else { + if !cfg.Keepalive.HasValue() { + cfg.Keepalive = configoptional.Some(KeepaliveServerConfig{}) + } + if conf.IsSet("idle_timeout") { + cfg.Keepalive.Get().IdleTimeout = cfg.IdleTimeout + } + } + + *sc = ServerConfig(cfg.serverConfigFields) + sc.renamedFields = warnings + return nil +} + +type KeepaliveServerConfig struct { + _ struct{} + + // IdleTimeout is the maximum amount of time to wait for the + // next request when keep-alives are enabled. If IdleTimeout + // is zero, the value of ReadTimeout is used. If both are + // zero, there is no timeout. + IdleTimeout time.Duration `mapstructure:"idle_timeout"` } // NewDefaultServerConfig returns ServerConfig type object with default values. @@ -105,8 +170,9 @@ func NewDefaultServerConfig() ServerConfig { return ServerConfig{ WriteTimeout: 30 * time.Second, ReadHeaderTimeout: 1 * time.Minute, - IdleTimeout: 1 * time.Minute, - KeepAlivesEnabled: true, + Keepalive: configoptional.Some(KeepaliveServerConfig{ + IdleTimeout: 1 * time.Minute, + }), } } @@ -174,7 +240,11 @@ func WithDecoder(key string, dec func(body io.ReadCloser) (io.ReadCloser, error) // the `extensions` argument should be the output of `host.GetExtensions()`. // It may also be `nil` in tests where no such extension is expected to be used. func (sc *ServerConfig) ToServer(ctx context.Context, extensions map[component.ID]component.Component, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { - serverOpts := &toServerOptions{} + for _, field := range sc.renamedFields { + field.Log(settings.Logger) + } + + serverOpts := &toServerOptions{} serverOpts.Apply(opts...) if sc.MaxRequestBodySize <= 0 { @@ -283,17 +353,22 @@ func (sc *ServerConfig) ToServer(ctx context.Context, extensions map[component.I return nil, err // If an error occurs while creating the logger, return nil and the error } + var idleTimeout time.Duration + if kaCfg := sc.Keepalive.Get(); sc.Keepalive.HasValue() { + idleTimeout = kaCfg.IdleTimeout + } + server := &http.Server{ Handler: handler, ReadTimeout: sc.ReadTimeout, ReadHeaderTimeout: sc.ReadHeaderTimeout, WriteTimeout: sc.WriteTimeout, - IdleTimeout: sc.IdleTimeout, + IdleTimeout: idleTimeout, ErrorLog: errorLog, } // Set keep-alives enabled/disabled - server.SetKeepAlivesEnabled(sc.KeepAlivesEnabled) + server.SetKeepAlivesEnabled(sc.Keepalive.HasValue()) return server, err } diff --git a/config/confighttp/server_test.go b/config/confighttp/server_test.go index 5db0af79a81..f6a56fe931f 100644 --- a/config/confighttp/server_test.go +++ b/config/confighttp/server_test.go @@ -929,11 +929,11 @@ func TestDefaultHTTPServerSettings(t *testing.T) { httpServerSettings := NewDefaultServerConfig() assert.NotNil(t, httpServerSettings.CORS) assert.NotNil(t, httpServerSettings.TLS) - assert.Equal(t, 1*time.Minute, httpServerSettings.IdleTimeout) assert.Equal(t, 30*time.Second, httpServerSettings.WriteTimeout) assert.Equal(t, time.Duration(0), httpServerSettings.ReadTimeout) assert.Equal(t, 1*time.Minute, httpServerSettings.ReadHeaderTimeout) - assert.True(t, httpServerSettings.KeepAlivesEnabled) // Default should be true (keep-alives enabled by default) + require.True(t, httpServerSettings.Keepalive.HasValue()) // Default should be true (keep-alives enabled by default) + assert.Equal(t, 1*time.Minute, httpServerSettings.Keepalive.Get().IdleTimeout) } func TestHTTPServerKeepAlives(t *testing.T) { @@ -956,9 +956,15 @@ func TestHTTPServerKeepAlives(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + var kaCfg configoptional.Optional[KeepaliveServerConfig] + if tt.expectedKeepAlives { + kaCfg = configoptional.Some(KeepaliveServerConfig{ + IdleTimeout: 1 * time.Minute, + }) + } sc := &ServerConfig{ - Endpoint: "localhost:0", - KeepAlivesEnabled: tt.keepAlivesEnabled, + Endpoint: "localhost:0", + Keepalive: kaCfg, } server, err := sc.ToServer( @@ -991,8 +997,6 @@ func TestHTTPServerKeepAlives(t *testing.T) { require.NotNil(t, resp) _ = resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) - - assert.Equal(t, tt.keepAlivesEnabled, sc.KeepAlivesEnabled) }) } } @@ -1095,8 +1099,7 @@ func TestServerUnmarshalYAMLComprehensiveConfig(t *testing.T) { assert.Equal(t, 30*time.Second, serverConfig.ReadTimeout) assert.Equal(t, 10*time.Second, serverConfig.ReadHeaderTimeout) assert.Equal(t, 30*time.Second, serverConfig.WriteTimeout) - assert.Equal(t, 120*time.Second, serverConfig.IdleTimeout) - assert.True(t, serverConfig.KeepAlivesEnabled) // Should be true as configured in config.yaml + assert.Equal(t, configoptional.Some(KeepaliveServerConfig{IdleTimeout: 120 * time.Second}), serverConfig.Keepalive) assert.Equal(t, int64(33554432), serverConfig.MaxRequestBodySize) assert.True(t, serverConfig.IncludeMetadata) diff --git a/config/confighttp/testdata/config/client/legacy_all_fields.yaml b/config/confighttp/testdata/config/client/legacy_all_fields.yaml new file mode 100644 index 00000000000..63f0c790e38 --- /dev/null +++ b/config/confighttp/testdata/config/client/legacy_all_fields.yaml @@ -0,0 +1,4 @@ +endpoint: "http://localhost:4318" +idle_conn_timeout: 90s +max_idle_conns: 100 +max_idle_conns_per_host: 10 diff --git a/config/confighttp/testdata/config/client/legacy_partial_fields.yaml b/config/confighttp/testdata/config/client/legacy_partial_fields.yaml new file mode 100644 index 00000000000..59b704a5cd4 --- /dev/null +++ b/config/confighttp/testdata/config/client/legacy_partial_fields.yaml @@ -0,0 +1,2 @@ +endpoint: "http://localhost:4318" +idle_conn_timeout: 60s diff --git a/config/confighttp/testdata/config/client/legacy_with_disable.yaml b/config/confighttp/testdata/config/client/legacy_with_disable.yaml new file mode 100644 index 00000000000..a1bed8d9357 --- /dev/null +++ b/config/confighttp/testdata/config/client/legacy_with_disable.yaml @@ -0,0 +1,4 @@ +endpoint: "http://localhost:4318" +idle_conn_timeout: 90s +max_idle_conns: 100 +disable_keep_alives: true diff --git a/config/confighttp/testdata/config/client/mixed_fields_error.yaml b/config/confighttp/testdata/config/client/mixed_fields_error.yaml new file mode 100644 index 00000000000..b9f2d053e1c --- /dev/null +++ b/config/confighttp/testdata/config/client/mixed_fields_error.yaml @@ -0,0 +1,4 @@ +endpoint: "http://localhost:4318" +idle_conn_timeout: 90s +keepalive: + idle_conn_timeout: 60s diff --git a/config/confighttp/testdata/config/client/new_keepalive.yaml b/config/confighttp/testdata/config/client/new_keepalive.yaml new file mode 100644 index 00000000000..c0b5274f399 --- /dev/null +++ b/config/confighttp/testdata/config/client/new_keepalive.yaml @@ -0,0 +1,5 @@ +endpoint: "http://localhost:4318" +keepalive: + idle_conn_timeout: 60s + max_idle_conns: 50 + max_idle_conns_per_host: 5 diff --git a/config/confighttp/testdata/config/client/no_keepalive_fields.yaml b/config/confighttp/testdata/config/client/no_keepalive_fields.yaml new file mode 100644 index 00000000000..ccf50bd5831 --- /dev/null +++ b/config/confighttp/testdata/config/client/no_keepalive_fields.yaml @@ -0,0 +1,2 @@ +endpoint: "http://localhost:4318" +timeout: 30s diff --git a/config/confighttp/testdata/config/server/legacy_disabled.yaml b/config/confighttp/testdata/config/server/legacy_disabled.yaml new file mode 100644 index 00000000000..a107746facc --- /dev/null +++ b/config/confighttp/testdata/config/server/legacy_disabled.yaml @@ -0,0 +1,3 @@ +endpoint: "0.0.0.0:4318" +idle_timeout: 120s +keep_alives_enabled: false diff --git a/config/confighttp/testdata/config/server/legacy_no_idle_timeout.yaml b/config/confighttp/testdata/config/server/legacy_no_idle_timeout.yaml new file mode 100644 index 00000000000..444a1d7a9f2 --- /dev/null +++ b/config/confighttp/testdata/config/server/legacy_no_idle_timeout.yaml @@ -0,0 +1,2 @@ +endpoint: "0.0.0.0:4318" +keep_alives_enabled: true diff --git a/config/confighttp/testdata/config/server/legacy_with_idle_timeout.yaml b/config/confighttp/testdata/config/server/legacy_with_idle_timeout.yaml new file mode 100644 index 00000000000..709c10498ed --- /dev/null +++ b/config/confighttp/testdata/config/server/legacy_with_idle_timeout.yaml @@ -0,0 +1,3 @@ +endpoint: "0.0.0.0:4318" +idle_timeout: 120s +keep_alives_enabled: true diff --git a/config/confighttp/testdata/config/server/mixed_fields_error.yaml b/config/confighttp/testdata/config/server/mixed_fields_error.yaml new file mode 100644 index 00000000000..ec14c06589d --- /dev/null +++ b/config/confighttp/testdata/config/server/mixed_fields_error.yaml @@ -0,0 +1,4 @@ +endpoint: "0.0.0.0:4318" +idle_timeout: 120s +keepalive: + idle_timeout: 90s diff --git a/config/confighttp/testdata/config/server/new_keepalive.yaml b/config/confighttp/testdata/config/server/new_keepalive.yaml new file mode 100644 index 00000000000..c428a21872c --- /dev/null +++ b/config/confighttp/testdata/config/server/new_keepalive.yaml @@ -0,0 +1,3 @@ +endpoint: "0.0.0.0:4318" +keepalive: + idle_timeout: 90s diff --git a/config/confighttp/testdata/config/server/no_keepalive_fields.yaml b/config/confighttp/testdata/config/server/no_keepalive_fields.yaml new file mode 100644 index 00000000000..fe791849d95 --- /dev/null +++ b/config/confighttp/testdata/config/server/no_keepalive_fields.yaml @@ -0,0 +1,2 @@ +endpoint: "0.0.0.0:4318" +read_timeout: 30s diff --git a/config/confighttp/unmarshal_test.go b/config/confighttp/unmarshal_test.go new file mode 100644 index 00000000000..1a832b3897d --- /dev/null +++ b/config/confighttp/unmarshal_test.go @@ -0,0 +1,169 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package confighttp + +import ( + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.opentelemetry.io/collector/config/configoptional" + "go.opentelemetry.io/collector/confmap/confmaptest" +) + +func TestClientConfigUnmarshal(t *testing.T) { + tests := []struct { + name string + configFile string + expectError bool + expectWarnings bool + want configoptional.Optional[KeepaliveClientConfig] + }{ + { + name: "legacy_all_fields", + configFile: "client/legacy_all_fields.yaml", + expectWarnings: true, + want: configoptional.Some(KeepaliveClientConfig{ + IdleConnTimeout: 90 * time.Second, + MaxIdleConns: 100, + MaxIdleConnsPerHost: 10, + }), + }, + { + name: "legacy_partial_fields", + configFile: "client/legacy_partial_fields.yaml", + expectWarnings: true, + want: configoptional.Some(KeepaliveClientConfig{ + IdleConnTimeout: 60 * time.Second, + MaxIdleConns: 100, + }), + }, + { + name: "legacy_with_disable", + configFile: "client/legacy_with_disable.yaml", + expectWarnings: true, + }, + { + name: "new_keepalive", + configFile: "client/new_keepalive.yaml", + want: configoptional.Some(KeepaliveClientConfig{ + IdleConnTimeout: 60 * time.Second, + MaxIdleConns: 50, + MaxIdleConnsPerHost: 5, + }), + }, + { + name: "mixed_fields_error", + configFile: "client/mixed_fields_error.yaml", + expectError: true, + }, + { + name: "no_keepalive_fields", + configFile: "client/no_keepalive_fields.yaml", + want: configoptional.Some(KeepaliveClientConfig{ + IdleConnTimeout: 90 * time.Second, + MaxIdleConns: 100, + }), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config", tt.configFile)) + require.NoError(t, err) + + cfg := NewDefaultClientConfig() + err = cm.Unmarshal(&cfg) + + if tt.expectError { + require.Error(t, err) + return + } + + require.NoError(t, err) + + if tt.expectWarnings { + require.NotEmpty(t, cfg.warnings) + } + assert.Equal(t, tt.want, cfg.Keepalive) + }) + } +} + +func TestServerConfigUnmarshal(t *testing.T) { + tests := []struct { + name string + configFile string + expectError bool + expectWarnings bool + want configoptional.Optional[KeepaliveServerConfig] + }{ + { + name: "legacy_with_idle_timeout", + configFile: "server/legacy_with_idle_timeout.yaml", + expectWarnings: true, + want: configoptional.Some(KeepaliveServerConfig{ + IdleTimeout: 120 * time.Second, + }), + }, + { + name: "legacy_no_idle_timeout", + configFile: "server/legacy_no_idle_timeout.yaml", + expectWarnings: true, + want: configoptional.Some(KeepaliveServerConfig{ + IdleTimeout: 60 * time.Second, + }), + }, + { + name: "legacy_disabled", + configFile: "server/legacy_disabled.yaml", + expectWarnings: true, + }, + { + name: "new_keepalive", + configFile: "server/new_keepalive.yaml", + want: configoptional.Some(KeepaliveServerConfig{ + IdleTimeout: 90 * time.Second, + }), + }, + { + name: "mixed_fields_error", + configFile: "server/mixed_fields_error.yaml", + expectError: true, + }, + { + name: "no_keepalive_fields", + configFile: "server/no_keepalive_fields.yaml", + want: configoptional.Some(KeepaliveServerConfig{ + IdleTimeout: 60 * time.Second, + }), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config", tt.configFile)) + require.NoError(t, err) + + cfg := NewDefaultServerConfig() + err = cm.Unmarshal(&cfg) + + if tt.expectError { + require.Error(t, err) + return + } + + require.NoError(t, err) + + if tt.expectWarnings { + require.NotEmpty(t, cfg.renamedFields) + } + + assert.Equal(t, tt.want, cfg.Keepalive) + }) + } +} diff --git a/exporter/otlphttpexporter/config_test.go b/exporter/otlphttpexporter/config_test.go index 1957053163a..a55362d6009 100644 --- a/exporter/otlphttpexporter/config_test.go +++ b/exporter/otlphttpexporter/config_test.go @@ -80,15 +80,17 @@ func TestUnmarshalConfig(t *testing.T) { }, Insecure: true, }, - ReadBufferSize: 123, - WriteBufferSize: 345, - Timeout: time.Second * 10, - Compression: "gzip", - MaxIdleConns: defaultMaxIdleConns, - MaxIdleConnsPerHost: defaultMaxIdleConnsPerHost, - MaxConnsPerHost: defaultMaxConnsPerHost, - IdleConnTimeout: defaultIdleConnTimeout, - ForceAttemptHTTP2: true, + ReadBufferSize: 123, + WriteBufferSize: 345, + Timeout: time.Second * 10, + Compression: "gzip", + Keepalive: configoptional.Some(confighttp.KeepaliveClientConfig{ + MaxIdleConns: defaultMaxIdleConns, + MaxIdleConnsPerHost: defaultMaxIdleConnsPerHost, + IdleConnTimeout: defaultIdleConnTimeout, + }), + MaxConnsPerHost: defaultMaxConnsPerHost, + ForceAttemptHTTP2: true, }, ProfilesEndpoint: "https://custom.profiles.endpoint:8080/v1development/profiles", }, cfg) diff --git a/receiver/otlpreceiver/factory.go b/receiver/otlpreceiver/factory.go index 1b6f483a1e3..bbe67a5c70a 100644 --- a/receiver/otlpreceiver/factory.go +++ b/receiver/otlpreceiver/factory.go @@ -51,7 +51,7 @@ func createDefaultConfig() component.Config { httpCfg.TLS = configoptional.None[configtls.ServerConfig]() httpCfg.WriteTimeout = 0 httpCfg.ReadHeaderTimeout = 0 - httpCfg.IdleTimeout = 0 + httpCfg.Keepalive.Get().IdleTimeout = 0 return &Config{ Protocols: Protocols{