[confighttp] Move keepalive config into a dedicated optional section

.chloggen/feat_confighttp_config-optional.yaml

@@ -0,0 +1,36 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: deprecation
+
+# The name of the component, or a single word describing the area of concern, (e.g. receiver/otlp)
+component: pkg/confighttp
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Deprecate flat keepalive fields in client and server config in favor of the new `keepalive` section.
+
+# One or more tracking issues or pull requests related to the change
+issues: [14020]
+
+# (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: |
+  The following client configuration fields are deprecated in favor of the `keepalive` section:
+    - `idle_conn_timeout` -> `keepalive::idle_conn_timeout`
+    - `max_idle_conns` -> `keepalive::max_idle_conns`
+    - `max_idle_conns_per_host` -> `keepalive::max_idle_conns_per_host`
+    - `disable_keep_alives: true` -> `keepalive::enabled: false`
+  The following server configuration fields are deprecated in favor of the `keepalive` section:
+    - `idle_timeout` -> `keepalive::idle_timeout`
+    - `keep_alives_enabled: false` -> `keepalive::enabled: false`
+  Deprecated fields set in the configuration keep working exactly as before and produce a
+  deprecation warning when the client or server is created. Setting deprecated fields together
+  with the new `keepalive` section is an error.
+
+# 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: [user, api]

config/confighttp/README.md

@@ -51,12 +51,15 @@ README](../configtls/README.md).
       No compression levels supported yet
     - `x-snappy-framed` (When feature gate `confighttp.framedSnappy` is enabled)
       No compression levels supported yet
-- [`max_idle_conns`](https://golang.org/pkg/net/http/#Transport)
-- [`max_idle_conns_per_host`](https://golang.org/pkg/net/http/#Transport)
 - [`max_conns_per_host`](https://golang.org/pkg/net/http/#Transport)
-- [`idle_conn_timeout`](https://golang.org/pkg/net/http/#Transport)
+- `keepalive`: Keepalive settings for the HTTP client.
+  - `enabled`: Set to `false` to disable keep-alives. Default: `true`
+  - [`idle_conn_timeout`](https://golang.org/pkg/net/http/#Transport): Default: `90s`
+  - [`max_idle_conns`](https://golang.org/pkg/net/http/#Transport): Default: `100`
+  - [`max_idle_conns_per_host`](https://golang.org/pkg/net/http/#Transport)
 - [`auth`](../configauth/README.md)
-- [`disable_keep_alives`](https://golang.org/pkg/net/http/#Transport)
+- **Deprecated** — `idle_conn_timeout`, `max_idle_conns`, `max_idle_conns_per_host`: use the `keepalive` section instead.
+- **Deprecated** — `disable_keep_alives`: use `keepalive:\n  enabled: false` instead.
 - [`force_attempt_http2`](https://golang.org/pkg/net/http/#Transport)
 - [`http2_read_idle_timeout`](https://pkg.go.dev/golang.org/x/net/http2#Transport)
 - [`http2_ping_timeout`](https://pkg.go.dev/golang.org/x/net/http2#Transport)
@@ -119,8 +122,11 @@ will not be enabled.
 - `read_timeout`: maximum duration for reading the entire request, including the body. A zero or negative value means there will be no timeout. Default: `0` (no timeout)
 - `read_header_timeout`: amount of time allowed to read request headers. If zero, the value of `read_timeout` is used. If both are zero, there is no timeout. Default: `1m`
 - `write_timeout`: maximum duration before timing out writes of the response. A zero or negative value means there will be no timeout. Default: `30s`
-- `idle_timeout`: maximum amount of time to wait for the next request when keep-alives are enabled. If zero, the value of `read_timeout` is used. If both are zero, there is no timeout. Default: `1m`
-- `keep_alives_enabled`: controls whether HTTP keep-alives are enabled. Default: `true`
+- `keepalive`: Keepalive settings for the HTTP server.
+  - `enabled`: Set to `false` to disable keep-alives. Default: `true`
+  - `idle_timeout`: maximum amount of time to wait for the next request when keep-alives are enabled. If zero, the value of `read_timeout` is used. If both are zero, there is no timeout. Default: `1m`
+- **Deprecated** — `idle_timeout`: use `keepalive::idle_timeout` instead.
+- **Deprecated** — `keep_alives_enabled: false`: use `keepalive:\n  enabled: false` instead.
 - [`tls`](../configtls/README.md)
 - [`auth`](../configauth/README.md)
   - `request_params`: a list of query parameter names to add to the auth context, along with the HTTP headers

config/confighttp/client.go

@@ -25,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 (
@@ -68,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
@@ -113,13 +94,47 @@ 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. When set, it takes precedence over the
+	// deprecated flat fields below.
+	Keepalive configoptional.Optional[KeepaliveClientConfig] `mapstructure:"keepalive,omitempty"`
+
+	// Deprecated: use Keepalive.IdleConnTimeout instead.
+	IdleConnTimeout time.Duration `mapstructure:"idle_conn_timeout,omitempty"`
+	// Deprecated: use Keepalive.MaxIdleConns instead.
+	MaxIdleConns int `mapstructure:"max_idle_conns,omitempty"`
+	// Deprecated: use Keepalive.MaxIdleConnsPerHost instead.
+	MaxIdleConnsPerHost int `mapstructure:"max_idle_conns_per_host,omitempty"`
+	// Deprecated: set 'keepalive::enabled' to false to disable keep-alives.
+	DisableKeepAlives bool `mapstructure:"disable_keep_alives,omitempty"`
+
+	// deprecationWarnings records use of deprecated fields observed while
+	// unmarshaling; ToClient logs them, as no logger is available here.
+	deprecationWarnings []string
 }
 
 // CookiesConfig defines the configuration of the HTTP client regarding cookies served by the server.
 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.
@@ -129,12 +144,73 @@ func NewDefaultClientConfig() ClientConfig {
 	defaultTransport := http.DefaultTransport.(*http.Transport)
 
 	return ClientConfig{
-		MaxIdleConns:      defaultTransport.MaxIdleConns,
-		IdleConnTimeout:   defaultTransport.IdleConnTimeout,
+		// The deprecated flat fields keep carrying the defaults so that
+		// configurations and code which still use them behave as before.
+		MaxIdleConns:    defaultTransport.MaxIdleConns,
+		IdleConnTimeout: defaultTransport.IdleConnTimeout,
+		Keepalive: configoptional.Default(KeepaliveClientConfig{
+			MaxIdleConns:    defaultTransport.MaxIdleConns,
+			IdleConnTimeout: defaultTransport.IdleConnTimeout,
+		}),
 		ForceAttemptHTTP2: true,
 	}
 }
 
+var _ confmap.Unmarshaler = (*ClientConfig)(nil)
+
+// Unmarshal implements confmap.Unmarshaler. It rejects configurations mixing the
+// deprecated keepalive fields with the 'keepalive' section and records deprecation
+// warnings for ToClient to log. Only keys present in the configuration count:
+// values set programmatically (e.g. by a component's default config) are neither
+// deprecated usage nor a conflict.
+func (cc *ClientConfig) Unmarshal(conf *confmap.Conf) error {
+	prevKeepalive := cc.Keepalive
+	// Read before unmarshaling: decoding the Optional consumes the 'enabled' key.
+	keepaliveDisabled := conf.Get("keepalive::enabled") == false
+
+	// WithIgnoreUnused is needed because ClientConfig is commonly squash-embedded
+	// into component configs, in which case conf also holds the parent's sibling fields.
+	if err := conf.Unmarshal(cc, confmap.WithIgnoreUnused()); err != nil {
+		return err
+	}
+
+	// A null 'keepalive' key carries no settings, but decodes as an enabled
+	// section. Marshaling produces it for an unset Keepalive, so treat it as
+	// unset to keep marshaled configurations loadable.
+	keepaliveSet := conf.IsSet("keepalive") && conf.Get("keepalive") != nil
+	if !keepaliveSet {
+		cc.Keepalive = prevKeepalive
+	}
+
+	// Values which match the field's zero value are no-ops in the legacy logic,
+	// so they neither conflict with the 'keepalive' section nor deserve a warning.
+	var deprecated []string
+	if conf.IsSet("idle_conn_timeout") && cc.IdleConnTimeout != 0 {
+		deprecated = append(deprecated, "'idle_conn_timeout' is deprecated; use 'keepalive::idle_conn_timeout' instead")
+	}
+	if conf.IsSet("max_idle_conns") && cc.MaxIdleConns != 0 {
+		deprecated = append(deprecated, "'max_idle_conns' is deprecated; use 'keepalive::max_idle_conns' instead")
+	}
+	if conf.IsSet("max_idle_conns_per_host") && cc.MaxIdleConnsPerHost != 0 {
+		deprecated = append(deprecated, "'max_idle_conns_per_host' is deprecated; use 'keepalive::max_idle_conns_per_host' instead")
+	}
+	if conf.IsSet("disable_keep_alives") && cc.DisableKeepAlives {
+		deprecated = append(deprecated, "'disable_keep_alives' is deprecated; set 'keepalive::enabled' to false to disable keep-alives")
+	}
+	if keepaliveSet && len(deprecated) > 0 {
+		return errors.New("confighttp.ClientConfig: cannot use deprecated keepalive fields (idle_conn_timeout, max_idle_conns, max_idle_conns_per_host, disable_keep_alives) alongside the 'keepalive' section; migrate to the 'keepalive' section")
+	}
+	cc.deprecationWarnings = deprecated
+
+	// 'keepalive::enabled: false' leaves the Optional without a value, which is
+	// indistinguishable from an unset section; carry the intent in the flat
+	// field, which the resolution in ToClient falls back to.
+	if keepaliveDisabled {
+		cc.DisableKeepAlives = true
+	}
+	return nil
+}
+
 func (cc *ClientConfig) Validate() error {
 	if cc.Compression.IsCompressed() {
 		if err := cc.Compression.ValidateParams(cc.CompressionParams); err != nil {
@@ -157,6 +233,10 @@ 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) {
+	for _, warning := range cc.deprecationWarnings {
+		settings.Logger.Warn(warning)
+	}
+
 	tlsCfg, err := cc.TLS.LoadTLSConfig(ctx)
 	if err != nil {
 		return nil, err
@@ -172,12 +252,20 @@ 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(); kaCfg != nil {
+		transport.MaxIdleConns = kaCfg.MaxIdleConns
+		transport.MaxIdleConnsPerHost = kaCfg.MaxIdleConnsPerHost
+		transport.IdleConnTimeout = kaCfg.IdleConnTimeout
+	} else {
+		// The 'keepalive' section is not in use; apply the deprecated flat
+		// fields exactly as the code before the section's introduction did.
+		transport.DisableKeepAlives = cc.DisableKeepAlives
+		transport.MaxIdleConns = cc.MaxIdleConns
+		transport.MaxIdleConnsPerHost = cc.MaxIdleConnsPerHost
+		transport.IdleConnTimeout = cc.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)
@@ -187,8 +275,6 @@ func (cc *ClientConfig) ToClient(ctx context.Context, extensions map[component.I
 		transport.Proxy = http.ProxyURL(proxyURL)
 	}
 
-	transport.DisableKeepAlives = cc.DisableKeepAlives
-
 	if cc.HTTP2ReadIdleTimeout > 0 {
 		transport2, transportErr := http2.ConfigureTransports(transport)
 		if transportErr != nil {