Consolidate otelcol configunmarshaler into generic implementation

.chloggen/generic-configunmarshaler.yaml

@@ -0,0 +1,16 @@
+# 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: configunmarshaler
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Consolidate package into generic implementation
+
+# One or more tracking issues or pull requests related to the change
+issues: [6801]
+
+# (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:

otelcol/internal/configunmarshaler/configs.go

@@ -22,49 +22,49 @@ import (
 	"go.opentelemetry.io/collector/confmap"
 )
 
-type Configs struct {
+type Configs[F component.Factory] struct {
 	cfgs map[component.ID]component.Config
 
-	factories map[component.Type]component.Factory
+	factories map[component.Type]F
 }
 
-func NewConfigs(factories map[component.Type]component.Factory) *Configs {
-	return &Configs{factories: factories}
+func NewConfigs[F component.Factory](factories map[component.Type]F) *Configs[F] {
+	return &Configs[F]{factories: factories}
 }
 
-func (p *Configs) Unmarshal(conf *confmap.Conf) error {
-	rawProcs := make(map[component.ID]map[string]interface{})
-	if err := conf.Unmarshal(&rawProcs, confmap.WithErrorUnused()); err != nil {
+func (c *Configs[F]) Unmarshal(conf *confmap.Conf) error {
+	rawCfgs := make(map[component.ID]map[string]interface{})
+	if err := conf.Unmarshal(&rawCfgs, confmap.WithErrorUnused()); err != nil {
 		return err
 	}
 
 	// Prepare resulting map.
-	p.cfgs = make(map[component.ID]component.Config)
-	// Iterate over processors and create a config for each.
-	for id, value := range rawProcs {
-		// Find processor factory based on "type" that we read from config source.
-		factory := p.factories[id.Type()]
-		if factory == nil {
-			return errorUnknownType(id, reflect.ValueOf(p.factories).MapKeys())
+	c.cfgs = make(map[component.ID]component.Config)
+	// Iterate over raw configs and create a config for each.
+	for id, value := range rawCfgs {
+		// Find factory based on component kind and type that we read from config source.
+		factory, ok := c.factories[id.Type()]
+		if !ok {
+			return errorUnknownType(id, reflect.ValueOf(c.factories).MapKeys())
 		}
 
-		// Create the default config for this processor.
-		processorCfg := factory.CreateDefaultConfig()
+		// Create the default config for this component.
+		cfg := factory.CreateDefaultConfig()
 
 		// Now that the default config struct is created we can Unmarshal into it,
 		// and it will apply user-defined config on top of the default.
-		if err := component.UnmarshalConfig(confmap.NewFromStringMap(value), processorCfg); err != nil {
+		if err := component.UnmarshalConfig(confmap.NewFromStringMap(value), cfg); err != nil {
 			return errorUnmarshalError(id, err)
 		}
 
-		p.cfgs[id] = processorCfg
+		c.cfgs[id] = cfg
 	}
 
 	return nil
 }
 
-func (p *Configs) Configs() map[component.ID]component.Config {
-	return p.cfgs
+func (c *Configs[F]) Configs() map[component.ID]component.Config {
+	return c.cfgs
 }
 
 func errorUnknownType(id component.ID, factories []reflect.Value) error {

otelcol/internal/configunmarshaler/configs_test.go

@@ -23,94 +23,125 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/confmap"
 	"go.opentelemetry.io/collector/exporter/exportertest"
+	"go.opentelemetry.io/collector/extension/extensiontest"
 	"go.opentelemetry.io/collector/processor/processortest"
+	"go.opentelemetry.io/collector/receiver/receivertest"
 )
 
-func TestProcessorsUnmarshal(t *testing.T) {
-	factories := map[component.Type]component.Factory{
-		"nop": processortest.NewNopFactory(),
-	}
-
-	procs := NewConfigs(factories)
-	conf := confmap.NewFromStringMap(map[string]interface{}{
-		"nop":             nil,
-		"nop/myprocessor": nil,
-	})
-	require.NoError(t, procs.Unmarshal(conf))
-
-	cfgWithName := factories["nop"].CreateDefaultConfig()
-	assert.Equal(t, map[component.ID]component.Config{
-		component.NewID("nop"):                        factories["nop"].CreateDefaultConfig(),
-		component.NewIDWithName("nop", "myprocessor"): cfgWithName,
-	}, procs.cfgs)
-}
-
-func TestProcessorsUnmarshalError(t *testing.T) {
-	var testCases = []struct {
-		name string
-		conf *confmap.Conf
-		// string that the error must contain
-		expectedError string
-	}{
-		{
-			name: "invalid-type",
-			conf: confmap.NewFromStringMap(map[string]interface{}{
-				"nop":     nil,
-				"/custom": nil,
-			}),
-			expectedError: "the part before / should not be empty",
+var testKinds = []struct {
+	kind      string
+	factories map[component.Type]component.Factory
+}{
+	{
+		kind: "receiver",
+		factories: map[component.Type]component.Factory{
+			"nop": receivertest.NewNopFactory(),
 		},
-		{
-			name: "invalid-name-after-slash",
-			conf: confmap.NewFromStringMap(map[string]interface{}{
-				"nop":  nil,
-				"nop/": nil,
-			}),
-			expectedError: "the part after / should not be empty",
+	},
+	{
+		kind: "processor",
+		factories: map[component.Type]component.Factory{
+			"nop": processortest.NewNopFactory(),
 		},
-		{
-			name: "unknown-type",
-			conf: confmap.NewFromStringMap(map[string]interface{}{
-				"nosuchprocessor": nil,
-			}),
-			expectedError: "unknown type: \"nosuchprocessor\"",
+	},
+	{
+		kind: "exporter",
+		factories: map[component.Type]component.Factory{
+			"nop": exportertest.NewNopFactory(),
 		},
-		{
-			name: "duplicate",
-			conf: confmap.NewFromStringMap(map[string]interface{}{
-				"nop /exp ": nil,
-				" nop/ exp": nil,
-			}),
-			expectedError: "duplicate name",
+	},
+	{
+		kind: "extension",
+		factories: map[component.Type]component.Factory{
+			"nop": extensiontest.NewNopFactory(),
 		},
-		{
-			name: "invalid-section",
-			conf: confmap.NewFromStringMap(map[string]interface{}{
-				"nop": map[string]interface{}{
-					"unknown_section": "processor",
-				},
-			}),
-			expectedError: "error reading configuration for \"nop\"",
-		},
-		{
-			name: "invalid-processor-sub-config",
-			conf: confmap.NewFromStringMap(map[string]interface{}{
-				"nop": "tests",
-			}),
-			expectedError: "'[nop]' expected a map, got 'string'",
-		},
-	}
+	},
+}
 
-	factories := map[component.Type]component.Factory{
-		"nop": exportertest.NewNopFactory(),
+func TestUnmarshal(t *testing.T) {
+	for _, tk := range testKinds {
+		t.Run(tk.kind, func(t *testing.T) {
+			cfgs := NewConfigs(tk.factories)
+			conf := confmap.NewFromStringMap(map[string]interface{}{
+				"nop":              nil,
+				"nop/my" + tk.kind: nil,
+			})
+			require.NoError(t, cfgs.Unmarshal(conf))
+
+			assert.Equal(t, map[component.ID]component.Config{
+				component.NewID("nop"):                       tk.factories["nop"].CreateDefaultConfig(),
+				component.NewIDWithName("nop", "my"+tk.kind): tk.factories["nop"].CreateDefaultConfig(),
+			}, cfgs.Configs())
+		})
 	}
+}
+
+func TestUnmarshalError(t *testing.T) {
+	for _, tk := range testKinds {
+		t.Run(tk.kind, func(t *testing.T) {
+			var testCases = []struct {
+				name string
+				conf *confmap.Conf
+				// string that the error must contain
+				expectedError string
+			}{
+				{
+					name: "invalid-type",
+					conf: confmap.NewFromStringMap(map[string]interface{}{
+						"nop":     nil,
+						"/custom": nil,
+					}),
+					expectedError: "the part before / should not be empty",
+				},
+				{
+					name: "invalid-name-after-slash",
+					conf: confmap.NewFromStringMap(map[string]interface{}{
+						"nop":  nil,
+						"nop/": nil,
+					}),
+					expectedError: "the part after / should not be empty",
+				},
+				{
+					name: "unknown-type",
+					conf: confmap.NewFromStringMap(map[string]interface{}{
+						"nosuch" + tk.kind: nil,
+					}),
+					expectedError: "unknown type: \"nosuch" + tk.kind + "\"",
+				},
+				{
+					name: "duplicate",
+					conf: confmap.NewFromStringMap(map[string]interface{}{
+						"nop /my" + tk.kind + " ": nil,
+						" nop/ my" + tk.kind:      nil,
+					}),
+					expectedError: "duplicate name",
+				},
+				{
+					name: "invalid-section",
+					conf: confmap.NewFromStringMap(map[string]interface{}{
+						"nop": map[string]interface{}{
+							"unknown_section": tk.kind,
+						},
+					}),
+					expectedError: "error reading configuration for \"nop\"",
+				},
+				{
+					name: "invalid-sub-config",
+					conf: confmap.NewFromStringMap(map[string]interface{}{
+						"nop": "tests",
+					}),
+					expectedError: "'[nop]' expected a map, got 'string'",
+				},
+			}
 
-	for _, tt := range testCases {
-		t.Run(tt.name, func(t *testing.T) {
-			procs := NewConfigs(factories)
-			err := procs.Unmarshal(tt.conf)
-			require.Error(t, err)
-			assert.Contains(t, err.Error(), tt.expectedError)
+			for _, tt := range testCases {
+				t.Run(tt.name, func(t *testing.T) {
+					cfgs := NewConfigs(tk.factories)
+					err := cfgs.Unmarshal(tt.conf)
+					require.Error(t, err)
+					assert.Contains(t, err.Error(), tt.expectedError)
+				})
+			}
 		})
 	}
 }

otelcol/unmarshaler.go

@@ -17,31 +17,34 @@ package otelcol // import "go.opentelemetry.io/collector/otelcol"
 import (
 	"go.uber.org/zap/zapcore"
 
-	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configtelemetry"
 	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/exporter"
+	"go.opentelemetry.io/collector/extension"
 	"go.opentelemetry.io/collector/otelcol/internal/configunmarshaler"
+	"go.opentelemetry.io/collector/processor"
+	"go.opentelemetry.io/collector/receiver"
 	"go.opentelemetry.io/collector/service"
 	"go.opentelemetry.io/collector/service/telemetry"
 )
 
 type configSettings struct {
-	Receivers  *configunmarshaler.Configs `mapstructure:"receivers"`
-	Processors *configunmarshaler.Configs `mapstructure:"processors"`
-	Exporters  *configunmarshaler.Configs `mapstructure:"exporters"`
-	Extensions *configunmarshaler.Configs `mapstructure:"extensions"`
-	Service    service.ConfigService      `mapstructure:"service"`
+	Receivers  *configunmarshaler.Configs[receiver.Factory]  `mapstructure:"receivers"`
+	Processors *configunmarshaler.Configs[processor.Factory] `mapstructure:"processors"`
+	Exporters  *configunmarshaler.Configs[exporter.Factory]  `mapstructure:"exporters"`
+	Extensions *configunmarshaler.Configs[extension.Factory] `mapstructure:"extensions"`
+	Service    service.ConfigService                         `mapstructure:"service"`
 }
 
 // unmarshal the configSettings from a confmap.Conf.
 // After the config is unmarshalled, `Validate()` must be called to validate.
 func unmarshal(v *confmap.Conf, factories Factories) (*configSettings, error) {
 	// Unmarshal top level sections and validate.
 	cfg := &configSettings{
-		Receivers:  configunmarshaler.NewConfigs(toFactoryMap(factories.Receivers)),
-		Processors: configunmarshaler.NewConfigs(toFactoryMap(factories.Processors)),
-		Exporters:  configunmarshaler.NewConfigs(toFactoryMap(factories.Exporters)),
-		Extensions: configunmarshaler.NewConfigs(toFactoryMap(factories.Extensions)),
+		Receivers:  configunmarshaler.NewConfigs(factories.Receivers),
+		Processors: configunmarshaler.NewConfigs(factories.Processors),
+		Exporters:  configunmarshaler.NewConfigs(factories.Exporters),
+		Extensions: configunmarshaler.NewConfigs(factories.Extensions),
 		// TODO: Add a component.ServiceFactory to allow this to be defined by the Service.
 		Service: service.ConfigService{
 			Telemetry: telemetry.Config{
@@ -69,11 +72,3 @@ func unmarshal(v *confmap.Conf, factories Factories) (*configSettings, error) {
 
 	return cfg, v.Unmarshal(&cfg, confmap.WithErrorUnused())
 }
-
-func toFactoryMap[F component.Factory](factories map[component.Type]F) map[component.Type]component.Factory {
-	ret := make(map[component.Type]component.Factory, len(factories))
-	for k := range factories {
-		ret[k] = factories[k]
-	}
-	return ret
-}