Avoid using go embedded messages in Config (#12718)

This PR:
* Helps with
#12709,
to avoid the situation where we need to add Unmarshal for a struct and
if used as embedded will not work.
* Helps with avoiding name conflicts between variables. Will allow for
example to have a "Timeout" member in both QueueConfig and ClientConfig,
even though that was a valid YAML config since `QueueConfig` is under
"seding_queue".
* This is a breaking change only if devs are created manually the OTLP
configs which should be very rare, and unfortunately this is hard to do
using deprecation steps.

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

Author: bogdandrutu

.chloggen/avoid-embedings-in-conifg.yaml

@@ -0,0 +1,25 @@
+# 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. otlpreceiver)
+component: otlpreceiver/otlpexporter/otlphttpexporter
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Avoid using go embedded messages in Config
+
+# One or more tracking issues or pull requests related to the change
+issues: [12718]
+
+# (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:
+
+# 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]

exporter/otlpexporter/config.go

@@ -19,15 +19,14 @@ import (
 
 // Config defines configuration for OTLP exporter.
 type Config struct {
-	exporterhelper.TimeoutConfig `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
-	exporterhelper.QueueConfig   `mapstructure:"sending_queue"`
-	RetryConfig                  configretry.BackOffConfig `mapstructure:"retry_on_failure"`
+	TimeoutConfig exporterhelper.TimeoutConfig `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
+	QueueConfig   exporterhelper.QueueConfig   `mapstructure:"sending_queue"`
+	RetryConfig   configretry.BackOffConfig    `mapstructure:"retry_on_failure"`
+	ClientConfig  configgrpc.ClientConfig      `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
 
 	// Experimental: This configuration is at the early stage of development and may change without backward compatibility
 	// until https://github.com/open-telemetry/opentelemetry-collector/issues/8122 is resolved
 	BatcherConfig exporterhelper.BatcherConfig `mapstructure:"batcher"`
-
-	configgrpc.ClientConfig `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
 }
 
 func (c *Config) Validate() error {
@@ -50,15 +49,15 @@ func (c *Config) Validate() error {
 
 func (c *Config) sanitizedEndpoint() string {
 	switch {
-	case strings.HasPrefix(c.Endpoint, "http://"):
-		return strings.TrimPrefix(c.Endpoint, "http://")
-	case strings.HasPrefix(c.Endpoint, "https://"):
-		return strings.TrimPrefix(c.Endpoint, "https://")
-	case strings.HasPrefix(c.Endpoint, "dns://"):
+	case strings.HasPrefix(c.ClientConfig.Endpoint, "http://"):
+		return strings.TrimPrefix(c.ClientConfig.Endpoint, "http://")
+	case strings.HasPrefix(c.ClientConfig.Endpoint, "https://"):
+		return strings.TrimPrefix(c.ClientConfig.Endpoint, "https://")
+	case strings.HasPrefix(c.ClientConfig.Endpoint, "dns://"):
 		r := regexp.MustCompile("^dns://[/]?")
-		return r.ReplaceAllString(c.Endpoint, "")
+		return r.ReplaceAllString(c.ClientConfig.Endpoint, "")
 	default:
-		return c.Endpoint
+		return c.ClientConfig.Endpoint
 	}
 }
 

exporter/otlpexporter/config_test.go

@@ -144,17 +144,17 @@ func TestUnmarshalInvalidConfig(t *testing.T) {
 func TestValidDNSEndpoint(t *testing.T) {
 	factory := NewFactory()
 	cfg := factory.CreateDefaultConfig().(*Config)
-	cfg.Endpoint = "dns://authority/backend.example.com:4317"
+	cfg.ClientConfig.Endpoint = "dns://authority/backend.example.com:4317"
 	assert.NoError(t, cfg.Validate())
 }
 
 func TestSanitizeEndpoint(t *testing.T) {
 	factory := NewFactory()
 	cfg := factory.CreateDefaultConfig().(*Config)
-	cfg.Endpoint = "dns://authority/backend.example.com:4317"
+	cfg.ClientConfig.Endpoint = "dns://authority/backend.example.com:4317"
 	assert.Equal(t, "authority/backend.example.com:4317", cfg.sanitizedEndpoint())
-	cfg.Endpoint = "dns:///backend.example.com:4317"
+	cfg.ClientConfig.Endpoint = "dns:///backend.example.com:4317"
 	assert.Equal(t, "backend.example.com:4317", cfg.sanitizedEndpoint())
-	cfg.Endpoint = "dns:////backend.example.com:4317"
+	cfg.ClientConfig.Endpoint = "dns:////backend.example.com:4317"
 	assert.Equal(t, "/backend.example.com:4317", cfg.sanitizedEndpoint())
 }

exporter/otlpexporter/factory_test.go

@@ -34,7 +34,7 @@ func TestCreateDefaultConfig(t *testing.T) {
 	assert.Equal(t, configretry.NewDefaultBackOffConfig(), ocfg.RetryConfig)
 	assert.Equal(t, exporterhelper.NewDefaultQueueConfig(), ocfg.QueueConfig)
 	assert.Equal(t, exporterhelper.NewDefaultTimeoutConfig(), ocfg.TimeoutConfig)
-	assert.Equal(t, configcompression.TypeGzip, ocfg.Compression)
+	assert.Equal(t, configcompression.TypeGzip, ocfg.ClientConfig.Compression)
 }
 
 func TestCreateMetrics(t *testing.T) {

exporter/otlphttpexporter/config.go

@@ -45,9 +45,9 @@ func (e *EncodingType) UnmarshalText(text []byte) error {
 
 // Config defines configuration for OTLP/HTTP exporter.
 type Config struct {
-	confighttp.ClientConfig    `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
-	exporterhelper.QueueConfig `mapstructure:"sending_queue"`
-	RetryConfig                configretry.BackOffConfig `mapstructure:"retry_on_failure"`
+	ClientConfig confighttp.ClientConfig    `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
+	QueueConfig  exporterhelper.QueueConfig `mapstructure:"sending_queue"`
+	RetryConfig  configretry.BackOffConfig  `mapstructure:"retry_on_failure"`
 
 	// The URL to send traces to. If omitted the Endpoint + "/v1/traces" will be used.
 	TracesEndpoint string `mapstructure:"traces_endpoint"`
@@ -66,7 +66,7 @@ var _ component.Config = (*Config)(nil)
 
 // Validate checks if the exporter configuration is valid
 func (cfg *Config) Validate() error {
-	if cfg.Endpoint == "" && cfg.TracesEndpoint == "" && cfg.MetricsEndpoint == "" && cfg.LogsEndpoint == "" {
+	if cfg.ClientConfig.Endpoint == "" && cfg.TracesEndpoint == "" && cfg.MetricsEndpoint == "" && cfg.LogsEndpoint == "" {
 		return errors.New("at least one endpoint must be specified")
 	}
 	return nil

exporter/otlphttpexporter/factory.go

@@ -63,13 +63,13 @@ func composeSignalURL(oCfg *Config, signalOverrideURL string, signalName string,
 			return "", fmt.Errorf("%s_endpoint must be a valid URL", signalName)
 		}
 		return signalOverrideURL, nil
-	case oCfg.Endpoint == "":
+	case oCfg.ClientConfig.Endpoint == "":
 		return "", fmt.Errorf("either endpoint or %s_endpoint must be specified", signalName)
 	default:
-		if strings.HasSuffix(oCfg.Endpoint, "/") {
-			return oCfg.Endpoint + signalVersion + "/" + signalName, nil
+		if strings.HasSuffix(oCfg.ClientConfig.Endpoint, "/") {
+			return oCfg.ClientConfig.Endpoint + signalVersion + "/" + signalName, nil
 		}
-		return oCfg.Endpoint + "/" + signalVersion + "/" + signalName, nil
+		return oCfg.ClientConfig.Endpoint + "/" + signalVersion + "/" + signalName, nil
 	}
 }
 

exporter/otlphttpexporter/factory_test.go

@@ -37,7 +37,7 @@ func TestCreateDefaultConfig(t *testing.T) {
 	assert.Equal(t, 30*time.Second, ocfg.RetryConfig.MaxInterval, "default retry MaxInterval")
 	assert.True(t, ocfg.QueueConfig.Enabled, "default sending queue is enabled")
 	assert.Equal(t, EncodingProto, ocfg.Encoding)
-	assert.Equal(t, configcompression.TypeGzip, ocfg.Compression)
+	assert.Equal(t, configcompression.TypeGzip, ocfg.ClientConfig.Compression)
 }
 
 func TestCreateMetrics(t *testing.T) {

exporter/otlphttpexporter/otlp.go

@@ -60,8 +60,8 @@ const (
 func newExporter(cfg component.Config, set exporter.Settings) (*baseExporter, error) {
 	oCfg := cfg.(*Config)
 
-	if oCfg.Endpoint != "" {
-		_, err := url.Parse(oCfg.Endpoint)
+	if oCfg.ClientConfig.Endpoint != "" {
+		_, err := url.Parse(oCfg.ClientConfig.Endpoint)
 		if err != nil {
 			return nil, errors.New("endpoint must be a valid URL")
 		}

internal/e2e/otlphttp_test.go

@@ -319,7 +319,7 @@ func startLogs(t *testing.T, baseURL string, overrideURL string) exporter.Logs {
 
 func createConfig(baseURL string, defaultCfg component.Config) *otlphttpexporter.Config {
 	cfg := defaultCfg.(*otlphttpexporter.Config)
-	cfg.Endpoint = baseURL
+	cfg.ClientConfig.Endpoint = baseURL
 	cfg.QueueConfig.Enabled = false
 	cfg.RetryConfig.Enabled = false
 	return cfg
@@ -351,7 +351,7 @@ func startLogsReceiver(t *testing.T, addr string, next consumer.Logs) {
 
 func createReceiverConfig(addr string, defaultCfg component.Config) *otlpreceiver.Config {
 	cfg := defaultCfg.(*otlpreceiver.Config)
-	cfg.HTTP.Endpoint = addr
+	cfg.HTTP.ServerConfig.Endpoint = addr
 	cfg.GRPC = nil
 	return cfg
 }

receiver/otlpreceiver/config.go

@@ -22,7 +22,7 @@ const (
 )
 
 type HTTPConfig struct {
-	*confighttp.ServerConfig `mapstructure:",squash"`
+	ServerConfig confighttp.ServerConfig `mapstructure:",squash"`
 
 	// The URL path to receive traces on. If omitted "/v1/traces" will be used.
 	TracesURLPath string `mapstructure:"traces_url_path,omitempty"`

receiver/otlpreceiver/config_test.go

@@ -119,7 +119,7 @@ func TestUnmarshalConfig(t *testing.T) {
 					},
 				},
 				HTTP: &HTTPConfig{
-					ServerConfig: &confighttp.ServerConfig{
+					ServerConfig: confighttp.ServerConfig{
 						Auth: &confighttp.AuthConfig{
 							Authentication: configauth.Authentication{
 								AuthenticatorID: component.MustNewID("test"),
@@ -164,7 +164,7 @@ func TestUnmarshalConfigUnix(t *testing.T) {
 					Keepalive:      configgrpc.NewDefaultKeepaliveServerConfig(),
 				},
 				HTTP: &HTTPConfig{
-					ServerConfig: &confighttp.ServerConfig{
+					ServerConfig: confighttp.ServerConfig{
 						Endpoint:        "/tmp/http_otlp.sock",
 						CORS:            confighttp.NewDefaultCORSConfig(),
 						ResponseHeaders: map[string]configopaque.String{},

receiver/otlpreceiver/factory.go

@@ -58,7 +58,7 @@ func createDefaultConfig() component.Config {
 		Protocols: Protocols{
 			GRPC: grpcCfg,
 			HTTP: &HTTPConfig{
-				ServerConfig:   &httpCfg,
+				ServerConfig:   httpCfg,
 				TracesURLPath:  defaultTracesURLPath,
 				MetricsURLPath: defaultMetricsURLPath,
 				LogsURLPath:    defaultLogsURLPath,

receiver/otlpreceiver/factory_test.go

@@ -39,7 +39,7 @@ func TestCreateSameReceiver(t *testing.T) {
 	factory := NewFactory()
 	cfg := factory.CreateDefaultConfig().(*Config)
 	cfg.GRPC.NetAddr.Endpoint = testutil.GetAvailableLocalAddress(t)
-	cfg.HTTP.Endpoint = testutil.GetAvailableLocalAddress(t)
+	cfg.HTTP.ServerConfig.Endpoint = testutil.GetAvailableLocalAddress(t)
 
 	core, observer := observer.New(zapcore.DebugLevel)
 	attrs := attribute.NewSet(
@@ -91,7 +91,7 @@ func TestCreateTraces(t *testing.T) {
 	defaultServerConfig := confighttp.NewDefaultServerConfig()
 	defaultServerConfig.Endpoint = testutil.GetAvailableLocalAddress(t)
 	defaultHTTPSettings := &HTTPConfig{
-		ServerConfig:   &defaultServerConfig,
+		ServerConfig:   defaultServerConfig,
 		TracesURLPath:  defaultTracesURLPath,
 		MetricsURLPath: defaultMetricsURLPath,
 		LogsURLPath:    defaultLogsURLPath,
@@ -136,7 +136,7 @@ func TestCreateTraces(t *testing.T) {
 				Protocols: Protocols{
 					GRPC: defaultGRPCSettings,
 					HTTP: &HTTPConfig{
-						ServerConfig: &confighttp.ServerConfig{
+						ServerConfig: confighttp.ServerConfig{
 							Endpoint: "localhost:112233",
 						},
 						TracesURLPath: defaultTracesURLPath,
@@ -185,7 +185,7 @@ func TestCreateMetric(t *testing.T) {
 	defaultServerConfig := confighttp.NewDefaultServerConfig()
 	defaultServerConfig.Endpoint = "127.0.0.1:0"
 	defaultHTTPSettings := &HTTPConfig{
-		ServerConfig:   &defaultServerConfig,
+		ServerConfig:   defaultServerConfig,
 		TracesURLPath:  defaultTracesURLPath,
 		MetricsURLPath: defaultMetricsURLPath,
 		LogsURLPath:    defaultLogsURLPath,
@@ -230,7 +230,7 @@ func TestCreateMetric(t *testing.T) {
 				Protocols: Protocols{
 					GRPC: defaultGRPCSettings,
 					HTTP: &HTTPConfig{
-						ServerConfig: &confighttp.ServerConfig{
+						ServerConfig: confighttp.ServerConfig{
 							Endpoint: "327.0.0.1:1122",
 						},
 						MetricsURLPath: defaultMetricsURLPath,
@@ -279,7 +279,7 @@ func TestCreateLogs(t *testing.T) {
 	defaultServerConfig := confighttp.NewDefaultServerConfig()
 	defaultServerConfig.Endpoint = testutil.GetAvailableLocalAddress(t)
 	defaultHTTPSettings := &HTTPConfig{
-		ServerConfig:   &defaultServerConfig,
+		ServerConfig:   defaultServerConfig,
 		TracesURLPath:  defaultTracesURLPath,
 		MetricsURLPath: defaultMetricsURLPath,
 		LogsURLPath:    defaultLogsURLPath,
@@ -324,7 +324,7 @@ func TestCreateLogs(t *testing.T) {
 				Protocols: Protocols{
 					GRPC: defaultGRPCSettings,
 					HTTP: &HTTPConfig{
-						ServerConfig: &confighttp.ServerConfig{
+						ServerConfig: confighttp.ServerConfig{
 							Endpoint: "327.0.0.1:1122",
 						},
 						LogsURLPath: defaultLogsURLPath,
@@ -373,7 +373,7 @@ func TestCreateProfiles(t *testing.T) {
 	defaultServerConfig := confighttp.NewDefaultServerConfig()
 	defaultServerConfig.Endpoint = testutil.GetAvailableLocalAddress(t)
 	defaultHTTPSettings := &HTTPConfig{
-		ServerConfig:   &defaultServerConfig,
+		ServerConfig:   defaultServerConfig,
 		TracesURLPath:  defaultTracesURLPath,
 		MetricsURLPath: defaultMetricsURLPath,
 		LogsURLPath:    defaultLogsURLPath,
@@ -418,7 +418,7 @@ func TestCreateProfiles(t *testing.T) {
 				Protocols: Protocols{
 					GRPC: defaultGRPCSettings,
 					HTTP: &HTTPConfig{
-						ServerConfig: &confighttp.ServerConfig{
+						ServerConfig: confighttp.ServerConfig{
 							Endpoint: "localhost:112233",
 						},
 					},

receiver/otlpreceiver/otlp.go

@@ -166,7 +166,7 @@ func (r *otlpReceiver) startHTTPServer(ctx context.Context, host component.Host)
 	}
 
 	var err error
-	if r.serverHTTP, err = r.cfg.HTTP.ToServer(ctx, host, r.settings.TelemetrySettings, httpMux, confighttp.WithErrorHandler(errorHandler)); err != nil {
+	if r.serverHTTP, err = r.cfg.HTTP.ServerConfig.ToServer(ctx, host, r.settings.TelemetrySettings, httpMux, confighttp.WithErrorHandler(errorHandler)); err != nil {
 		return err
 	}
 

receiver/otlpreceiver/otlp_test.go

@@ -755,7 +755,7 @@ func TestHTTPInvalidTLSCredentials(t *testing.T) {
 	cfg := &Config{
 		Protocols: Protocols{
 			HTTP: &HTTPConfig{
-				ServerConfig: &confighttp.ServerConfig{
+				ServerConfig: confighttp.ServerConfig{
 					Endpoint: testutil.GetAvailableLocalAddress(t),
 					TLSSetting: &configtls.ServerConfig{
 						Config: configtls.Config{
@@ -788,7 +788,7 @@ func testHTTPMaxRequestBodySize(t *testing.T, path string, contentType string, p
 	cfg := &Config{
 		Protocols: Protocols{
 			HTTP: &HTTPConfig{
-				ServerConfig: &confighttp.ServerConfig{
+				ServerConfig: confighttp.ServerConfig{
 					Endpoint:           addr,
 					MaxRequestBodySize: int64(size),
 				},
@@ -833,7 +833,7 @@ func newGRPCReceiver(t *testing.T, settings component.TelemetrySettings, endpoin
 
 func newHTTPReceiver(t *testing.T, settings component.TelemetrySettings, endpoint string, c consumertest.Consumer) component.Component {
 	cfg := createDefaultConfig().(*Config)
-	cfg.HTTP.Endpoint = endpoint
+	cfg.HTTP.ServerConfig.Endpoint = endpoint
 	cfg.GRPC = nil
 	return newReceiver(t, settings, cfg, otlpReceiverID, c)
 }
@@ -1015,7 +1015,7 @@ func TestShutdown(t *testing.T) {
 	factory := NewFactory()
 	cfg := factory.CreateDefaultConfig().(*Config)
 	cfg.GRPC.NetAddr.Endpoint = endpointGrpc
-	cfg.HTTP.Endpoint = endpointHTTP
+	cfg.HTTP.ServerConfig.Endpoint = endpointHTTP
 	set := receivertest.NewNopSettings(metadata.Type)
 	set.ID = otlpReceiverID
 	r, err := NewFactory().CreateTraces(