[receiver/otlpreceiver] Use configoptional type (#13119)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

<!-- Issue number if applicable -->

Uses `configoptional.Optional` for fields in `protocols` section.

Removes `Unmarshal` method since it is no longer needed.

These are both breaking changes, I think it would be a bit difficult to
do this in two steps, I am happy to work on fixing contrib after this is
merged.

#### Link to tracking issue
Fixes #12980

Author: mx-psi

.chloggen/mx-psi_otlpreciever-configoptional.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
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Use `configoptional.Optional` to define optional configuration sections in the OTLP receiver. Remove `Unmarshal` method.
+
+# One or more tracking issues or pull requests related to the change
+issues: [13119]
+
+# (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]

cmd/builder/internal/builder/main_test.go

@@ -50,6 +50,7 @@ var replaceModules = []string{
 	"/config/configmiddleware",
 	"/config/confignet",
 	"/config/configopaque",
+	"/config/configoptional",
 	"/config/configretry",
 	"/config/configtelemetry",
 	"/config/configtls",

cmd/otelcorecol/builder-config.yaml

@@ -49,6 +49,7 @@ replaces:
   - go.opentelemetry.io/collector/config/configmiddleware => ../../config/configmiddleware
   - go.opentelemetry.io/collector/config/confignet => ../../config/confignet
   - go.opentelemetry.io/collector/config/configopaque => ../../config/configopaque
+  - go.opentelemetry.io/collector/config/configoptional => ../../config/configoptional
   - go.opentelemetry.io/collector/config/configretry => ../../config/configretry
   - go.opentelemetry.io/collector/config/configtelemetry => ../../config/configtelemetry
   - go.opentelemetry.io/collector/config/configtls => ../../config/configtls

cmd/otelcorecol/go.mod

@@ -95,6 +95,7 @@ require (
 	go.opentelemetry.io/collector/config/configmiddleware v0.127.0 // indirect
 	go.opentelemetry.io/collector/config/confignet v1.33.0 // indirect
 	go.opentelemetry.io/collector/config/configopaque v1.33.0 // indirect
+	go.opentelemetry.io/collector/config/configoptional v0.0.0-00010101000000-000000000000 // indirect
 	go.opentelemetry.io/collector/config/configretry v1.33.0 // indirect
 	go.opentelemetry.io/collector/config/configtelemetry v0.127.0 // indirect
 	go.opentelemetry.io/collector/config/configtls v1.33.0 // indirect
@@ -197,6 +198,8 @@ replace go.opentelemetry.io/collector/config/confignet => ../../config/confignet
 
 replace go.opentelemetry.io/collector/config/configopaque => ../../config/configopaque
 
+replace go.opentelemetry.io/collector/config/configoptional => ../../config/configoptional
+
 replace go.opentelemetry.io/collector/config/configretry => ../../config/configretry
 
 replace go.opentelemetry.io/collector/config/configtelemetry => ../../config/configtelemetry

internal/e2e/consume_contract_test.go

@@ -7,10 +7,14 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configgrpc"
+	"go.opentelemetry.io/collector/config/configoptional"
 	"go.opentelemetry.io/collector/config/configretry"
 	"go.opentelemetry.io/collector/config/configtls"
+	"go.opentelemetry.io/collector/confmap"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
 	"go.opentelemetry.io/collector/exporter/exportertest"
 	"go.opentelemetry.io/collector/exporter/otlpexporter"
@@ -19,6 +23,19 @@ import (
 	"go.opentelemetry.io/collector/receiver/otlpreceiver"
 )
 
+// GetOrInsertDefault is a helper function to get or insert a default value for a configoptional.Optional type.
+func GetOrInsertDefault[T any](t *testing.T, opt *configoptional.Optional[T]) *T {
+	if opt.HasValue() {
+		return opt.Get()
+	}
+
+	empty := confmap.NewFromStringMap(map[string]any{})
+	require.NoError(t, empty.Unmarshal(opt))
+	val := opt.Get()
+	require.NotNil(t, "Expected a default value to be set for %T", val)
+	return val
+}
+
 func testExporterConfig(endpoint string) component.Config {
 	retryConfig := configretry.NewDefaultBackOffConfig()
 	retryConfig.InitialInterval = time.Millisecond // interval is short for the test purposes
@@ -34,10 +51,9 @@ func testExporterConfig(endpoint string) component.Config {
 	}
 }
 
-func testReceiverConfig(endpoint string) component.Config {
+func testReceiverConfig(t *testing.T, endpoint string) component.Config {
 	cfg := otlpreceiver.NewFactory().CreateDefaultConfig()
-	cfg.(*otlpreceiver.Config).HTTP = nil
-	cfg.(*otlpreceiver.Config).GRPC.NetAddr.Endpoint = endpoint
+	GetOrInsertDefault(t, &cfg.(*otlpreceiver.Config).GRPC).NetAddr.Endpoint = endpoint
 	return cfg
 }
 
@@ -52,7 +68,7 @@ func TestConsumeContractOtlpLogs(t *testing.T) {
 		Signal:               pipeline.SignalLogs,
 		ExporterConfig:       testExporterConfig(addr),
 		ReceiverFactory:      otlpreceiver.NewFactory(),
-		ReceiverConfig:       testReceiverConfig(addr),
+		ReceiverConfig:       testReceiverConfig(t, addr),
 	})
 }
 
@@ -65,7 +81,7 @@ func TestConsumeContractOtlpTraces(t *testing.T) {
 		ExporterFactory:      otlpexporter.NewFactory(),
 		ExporterConfig:       testExporterConfig(addr),
 		ReceiverFactory:      otlpreceiver.NewFactory(),
-		ReceiverConfig:       testReceiverConfig(addr),
+		ReceiverConfig:       testReceiverConfig(t, addr),
 	})
 }
 
@@ -78,6 +94,6 @@ func TestConsumeContractOtlpMetrics(t *testing.T) {
 		Signal:               pipeline.SignalMetrics,
 		ExporterConfig:       testExporterConfig(addr),
 		ReceiverFactory:      otlpreceiver.NewFactory(),
-		ReceiverConfig:       testReceiverConfig(addr),
+		ReceiverConfig:       testReceiverConfig(t, addr),
 	})
 }

internal/e2e/go.mod

@@ -13,6 +13,7 @@ require (
 	go.opentelemetry.io/collector/config/confighttp v0.127.0
 	go.opentelemetry.io/collector/config/confignet v1.33.0
 	go.opentelemetry.io/collector/config/configopaque v1.33.0
+	go.opentelemetry.io/collector/config/configoptional v0.0.0-00010101000000-000000000000
 	go.opentelemetry.io/collector/config/configretry v1.33.0
 	go.opentelemetry.io/collector/config/configtelemetry v0.127.0
 	go.opentelemetry.io/collector/config/configtls v1.33.0
@@ -151,6 +152,8 @@ replace go.opentelemetry.io/collector => ../..
 
 replace go.opentelemetry.io/collector/config/configopaque => ../../config/configopaque
 
+replace go.opentelemetry.io/collector/config/configoptional => ../../config/configoptional
+
 replace go.opentelemetry.io/collector/config/configgrpc => ../../config/configgrpc
 
 replace go.opentelemetry.io/collector/config/confignet => ../../config/confignet

internal/e2e/otlphttp_test.go

@@ -327,32 +327,31 @@ func createConfig(baseURL string, defaultCfg component.Config) *otlphttpexporter
 
 func startTracesReceiver(t *testing.T, addr string, next consumer.Traces) {
 	factory := otlpreceiver.NewFactory()
-	cfg := createReceiverConfig(addr, factory.CreateDefaultConfig())
+	cfg := createReceiverConfig(t, addr, factory.CreateDefaultConfig())
 	recv, err := factory.CreateTraces(context.Background(), receivertest.NewNopSettings(factory.Type()), cfg, next)
 	require.NoError(t, err)
 	startAndCleanup(t, recv)
 }
 
 func startMetricsReceiver(t *testing.T, addr string, next consumer.Metrics) {
 	factory := otlpreceiver.NewFactory()
-	cfg := createReceiverConfig(addr, factory.CreateDefaultConfig())
+	cfg := createReceiverConfig(t, addr, factory.CreateDefaultConfig())
 	recv, err := factory.CreateMetrics(context.Background(), receivertest.NewNopSettings(factory.Type()), cfg, next)
 	require.NoError(t, err)
 	startAndCleanup(t, recv)
 }
 
 func startLogsReceiver(t *testing.T, addr string, next consumer.Logs) {
 	factory := otlpreceiver.NewFactory()
-	cfg := createReceiverConfig(addr, factory.CreateDefaultConfig())
+	cfg := createReceiverConfig(t, addr, factory.CreateDefaultConfig())
 	recv, err := factory.CreateLogs(context.Background(), receivertest.NewNopSettings(factory.Type()), cfg, next)
 	require.NoError(t, err)
 	startAndCleanup(t, recv)
 }
 
-func createReceiverConfig(addr string, defaultCfg component.Config) *otlpreceiver.Config {
+func createReceiverConfig(t *testing.T, addr string, defaultCfg component.Config) *otlpreceiver.Config {
 	cfg := defaultCfg.(*otlpreceiver.Config)
-	cfg.HTTP.ServerConfig.Endpoint = addr
-	cfg.GRPC = nil
+	GetOrInsertDefault(t, &cfg.HTTP).ServerConfig.Endpoint = addr
 	return cfg
 }
 

receiver/otlpreceiver/config.go

@@ -13,13 +13,7 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configgrpc"
 	"go.opentelemetry.io/collector/config/confighttp"
-	"go.opentelemetry.io/collector/confmap"
-)
-
-const (
-	// Protocol values.
-	protoGRPC = "protocols::grpc"
-	protoHTTP = "protocols::http"
+	"go.opentelemetry.io/collector/config/configoptional"
 )
 
 type SanitizedURLPath string
@@ -58,8 +52,8 @@ type HTTPConfig struct {
 
 // Protocols is the configuration for the supported protocols.
 type Protocols struct {
-	GRPC *configgrpc.ServerConfig `mapstructure:"grpc"`
-	HTTP *HTTPConfig              `mapstructure:"http"`
+	GRPC configoptional.Optional[configgrpc.ServerConfig] `mapstructure:"grpc"`
+	HTTP configoptional.Optional[HTTPConfig]              `mapstructure:"http"`
 	// prevent unkeyed literal initialization
 	_ struct{}
 }
@@ -70,34 +64,12 @@ type Config struct {
 	Protocols `mapstructure:"protocols"`
 }
 
-var (
-	_ component.Config    = (*Config)(nil)
-	_ confmap.Unmarshaler = (*Config)(nil)
-)
+var _ component.Config = (*Config)(nil)
 
 // Validate checks the receiver configuration is valid
 func (cfg *Config) Validate() error {
-	if cfg.GRPC == nil && cfg.HTTP == nil {
+	if !cfg.GRPC.HasValue() && !cfg.HTTP.HasValue() {
 		return errors.New("must specify at least one protocol when using the OTLP receiver")
 	}
 	return nil
 }
-
-// Unmarshal a confmap.Conf into the config struct.
-func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
-	// first load the config normally
-	err := conf.Unmarshal(cfg)
-	if err != nil {
-		return err
-	}
-
-	if !conf.IsSet(protoGRPC) {
-		cfg.GRPC = nil
-	}
-
-	if !conf.IsSet(protoHTTP) {
-		cfg.HTTP = nil
-	}
-
-	return nil
-}

receiver/otlpreceiver/config_test.go

@@ -17,19 +17,36 @@ import (
 	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/confignet"
 	"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/confmap/confmaptest"
 	"go.opentelemetry.io/collector/confmap/xconfmap"
 )
 
+// GetOrInsertDefault is a helper function to get or insert a default value for a configoptional.Optional type.
+func GetOrInsertDefault[T any](t *testing.T, opt *configoptional.Optional[T]) *T {
+	if opt.HasValue() {
+		return opt.Get()
+	}
+
+	empty := confmap.NewFromStringMap(map[string]any{})
+	require.NoError(t, empty.Unmarshal(opt))
+	val := opt.Get()
+	require.NotNil(t, "Expected a default value to be set for %T", val)
+	return val
+}
+
 func TestUnmarshalDefaultConfig(t *testing.T) {
 	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "default.yaml"))
 	require.NoError(t, err)
 	factory := NewFactory()
 	cfg := factory.CreateDefaultConfig()
 	require.NoError(t, cm.Unmarshal(&cfg))
-	assert.Equal(t, factory.CreateDefaultConfig(), cfg)
+	expectedCfg := factory.CreateDefaultConfig().(*Config)
+	GetOrInsertDefault(t, &expectedCfg.GRPC)
+	GetOrInsertDefault(t, &expectedCfg.HTTP)
+	assert.Equal(t, expectedCfg, cfg)
 }
 
 func TestUnmarshalConfigOnlyGRPC(t *testing.T) {
@@ -40,7 +57,7 @@ func TestUnmarshalConfigOnlyGRPC(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&cfg))
 
 	defaultOnlyGRPC := factory.CreateDefaultConfig().(*Config)
-	defaultOnlyGRPC.HTTP = nil
+	GetOrInsertDefault(t, &defaultOnlyGRPC.GRPC)
 	assert.Equal(t, defaultOnlyGRPC, cfg)
 }
 
@@ -52,7 +69,7 @@ func TestUnmarshalConfigOnlyHTTP(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&cfg))
 
 	defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
-	defaultOnlyHTTP.GRPC = nil
+	GetOrInsertDefault(t, &defaultOnlyHTTP.HTTP)
 	assert.Equal(t, defaultOnlyHTTP, cfg)
 }
 
@@ -64,7 +81,7 @@ func TestUnmarshalConfigOnlyHTTPNull(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&cfg))
 
 	defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
-	defaultOnlyHTTP.GRPC = nil
+	GetOrInsertDefault(t, &defaultOnlyHTTP.HTTP)
 	assert.Equal(t, defaultOnlyHTTP, cfg)
 }
 
@@ -76,7 +93,7 @@ func TestUnmarshalConfigOnlyHTTPEmptyMap(t *testing.T) {
 	require.NoError(t, cm.Unmarshal(&cfg))
 
 	defaultOnlyHTTP := factory.CreateDefaultConfig().(*Config)
-	defaultOnlyHTTP.GRPC = nil
+	GetOrInsertDefault(t, &defaultOnlyHTTP.HTTP)
 	assert.Equal(t, defaultOnlyHTTP, cfg)
 }
 
@@ -89,7 +106,7 @@ func TestUnmarshalConfig(t *testing.T) {
 	assert.Equal(t,
 		&Config{
 			Protocols: Protocols{
-				GRPC: &configgrpc.ServerConfig{
+				GRPC: configoptional.Some(configgrpc.ServerConfig{
 					NetAddr: confignet.AddrConfig{
 						Endpoint:  "localhost:4317",
 						Transport: confignet.TransportTypeTCP,
@@ -117,8 +134,8 @@ func TestUnmarshalConfig(t *testing.T) {
 							PermitWithoutStream: true,
 						},
 					},
-				},
-				HTTP: &HTTPConfig{
+				}),
+				HTTP: configoptional.Some(HTTPConfig{
 					ServerConfig: confighttp.ServerConfig{
 						Auth: &confighttp.AuthConfig{
 							Config: configauth.Config{
@@ -141,7 +158,7 @@ func TestUnmarshalConfig(t *testing.T) {
 					TracesURLPath:  "/traces",
 					MetricsURLPath: "/v2/metrics",
 					LogsURLPath:    "/log/ingest",
-				},
+				}),
 			},
 		}, cfg)
 }
@@ -155,15 +172,15 @@ func TestUnmarshalConfigUnix(t *testing.T) {
 	assert.Equal(t,
 		&Config{
 			Protocols: Protocols{
-				GRPC: &configgrpc.ServerConfig{
+				GRPC: configoptional.Some(configgrpc.ServerConfig{
 					NetAddr: confignet.AddrConfig{
 						Endpoint:  "/tmp/grpc_otlp.sock",
 						Transport: confignet.TransportTypeUnix,
 					},
 					ReadBufferSize: 512 * 1024,
 					Keepalive:      configgrpc.NewDefaultKeepaliveServerConfig(),
-				},
-				HTTP: &HTTPConfig{
+				}),
+				HTTP: configoptional.Some(HTTPConfig{
 					ServerConfig: confighttp.ServerConfig{
 						Endpoint:        "/tmp/http_otlp.sock",
 						CORS:            confighttp.NewDefaultCORSConfig(),
@@ -172,7 +189,7 @@ func TestUnmarshalConfigUnix(t *testing.T) {
 					TracesURLPath:  defaultTracesURLPath,
 					MetricsURLPath: defaultMetricsURLPath,
 					LogsURLPath:    defaultLogsURLPath,
-				},
+				}),
 			},
 		}, cfg)
 }