[config] Deprecate config.UnmarshalConfig

.chloggen/embedded_struct_unmarshaler.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: component
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Deprecate `component.UnmarshalConfig`, use `conf.Unmarshal(&intoCfg)` instead.
+
+# One or more tracking issues or pull requests related to the change
+issues: [7102]
+
+# (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/mdatagen/loader.go

@@ -124,11 +124,7 @@ func (m *metric) Unmarshal(parser *confmap.Conf) error {
 	if !parser.IsSet("enabled") {
 		return errors.New("missing required field: `enabled`")
 	}
-	err := parser.Unmarshal(m)
-	if err != nil {
-		return err
-	}
-	return nil
+	return parser.Unmarshal(m)
 }
 func (m metric) Data() MetricData {
 	if m.Sum != nil {

cmd/mdatagen/loader_test.go

@@ -249,7 +249,7 @@ func TestLoadMetadata(t *testing.T) {
 		},
 		{
 			name:    "testdata/unknown_value_type.yaml",
-			wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[system.cpu.time]': 1 error(s) decoding:\n\n* error decoding 'sum': 1 error(s) decoding:\n\n* error decoding 'value_type': invalid value_type: \"unknown\"",
+			wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[system.cpu.time]': 1 error(s) decoding:\n\n* error decoding 'sum': invalid value_type: \"unknown\"",
 		},
 		{
 			name:    "testdata/no_aggregation.yaml",
@@ -259,7 +259,7 @@ func TestLoadMetadata(t *testing.T) {
 		{
 			name:    "testdata/invalid_aggregation.yaml",
 			want:    metadata{},
-			wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[default.metric]': 1 error(s) decoding:\n\n* error decoding 'sum': 1 error(s) decoding:\n\n* error decoding 'aggregation_temporality': invalid aggregation: \"invalidaggregation\"",
+			wantErr: "1 error(s) decoding:\n\n* error decoding 'metrics[default.metric]': 1 error(s) decoding:\n\n* error decoding 'sum': invalid aggregation: \"invalidaggregation\"",
 		},
 		{
 			name:    "testdata/invalid_type_attr.yaml",

cmd/mdatagen/metricdata.go

@@ -28,18 +28,20 @@ type MetricData interface {
 type AggregationTemporality struct {
 	// Aggregation describes if the aggregator reports delta changes
 	// since last report time, or cumulative changes since a fixed start time.
-	Aggregation pmetric.AggregationTemporality
+	Aggregation pmetric.AggregationTemporality `mapstructure:"aggregation_temporality"`
 }
 
-// UnmarshalText implements the encoding.TextUnmarshaler interface.
-func (agg *AggregationTemporality) UnmarshalText(text []byte) error {
-	switch vtStr := string(text); vtStr {
+func (agg *AggregationTemporality) Unmarshal(parser *confmap.Conf) error {
+	if !parser.IsSet("aggregation_temporality") {
+		return errors.New("missing required field: `aggregation_temporality`")
+	}
+	switch vt := parser.Get("aggregation_temporality"); vt {
 	case "cumulative":
 		agg.Aggregation = pmetric.AggregationTemporalityCumulative
 	case "delta":
 		agg.Aggregation = pmetric.AggregationTemporalityDelta
 	default:
-		return fmt.Errorf("invalid aggregation: %q", vtStr)
+		return fmt.Errorf("invalid aggregation: %q", vt)
 	}
 	return nil
 }
@@ -73,36 +75,31 @@ func (mit MetricInputType) String() string {
 // MetricValueType defines the metric number type.
 type MetricValueType struct {
 	// ValueType is type of the metric number, options are "double", "int".
-	ValueType pmetric.NumberDataPointValueType
+	ValueType pmetric.NumberDataPointValueType `mapstructure:"value_type"`
 }
 
 func (mvt *MetricValueType) Unmarshal(parser *confmap.Conf) error {
 	if !parser.IsSet("value_type") {
 		return errors.New("missing required field: `value_type`")
 	}
-	return nil
-}
-
-// UnmarshalText implements the encoding.TextUnmarshaler interface.
-func (mvt *MetricValueType) UnmarshalText(text []byte) error {
-	switch vtStr := string(text); vtStr {
+	switch vt := parser.Get("value_type"); vt {
 	case "int":
 		mvt.ValueType = pmetric.NumberDataPointValueTypeInt
 	case "double":
 		mvt.ValueType = pmetric.NumberDataPointValueTypeDouble
 	default:
-		return fmt.Errorf("invalid value_type: %q", vtStr)
+		return fmt.Errorf("invalid value_type: %q", vt)
 	}
 	return nil
 }
 
 // Type returns name of the datapoint type.
-func (mvt MetricValueType) String() string {
+func (mvt *MetricValueType) String() string {
 	return mvt.ValueType.String()
 }
 
 // BasicType returns name of a golang basic type for the datapoint type.
-func (mvt MetricValueType) BasicType() string {
+func (mvt *MetricValueType) BasicType() string {
 	switch mvt.ValueType {
 	case pmetric.NumberDataPointValueTypeInt:
 		return "int64"
@@ -116,18 +113,10 @@ func (mvt MetricValueType) BasicType() string {
 }
 
 type gauge struct {
-	MetricValueType `mapstructure:"value_type"`
+	MetricValueType `mapstructure:",squash"`
 	MetricInputType `mapstructure:",squash"`
 }
 
-// Unmarshal is a custom unmarshaler for gauge. Needed mostly to avoid MetricValueType.Unmarshal inheritance.
-func (d *gauge) Unmarshal(parser *confmap.Conf) error {
-	if err := d.MetricValueType.Unmarshal(parser); err != nil {
-		return err
-	}
-	return parser.Unmarshal(d, confmap.WithIgnoreUnused())
-}
-
 func (d gauge) Type() string {
 	return "Gauge"
 }
@@ -141,23 +130,12 @@ func (d gauge) HasAggregated() bool {
 }
 
 type sum struct {
-	AggregationTemporality `mapstructure:"aggregation_temporality"`
+	AggregationTemporality `mapstructure:",squash"`
 	Mono                   `mapstructure:",squash"`
-	MetricValueType        `mapstructure:"value_type"`
+	MetricValueType        `mapstructure:",squash"`
 	MetricInputType        `mapstructure:",squash"`
 }
 
-// Unmarshal is a custom unmarshaler for sum. Needed mostly to avoid MetricValueType.Unmarshal inheritance.
-func (d *sum) Unmarshal(parser *confmap.Conf) error {
-	if !parser.IsSet("aggregation_temporality") {
-		return errors.New("missing required field: `aggregation_temporality`")
-	}
-	if err := d.MetricValueType.Unmarshal(parser); err != nil {
-		return err
-	}
-	return parser.Unmarshal(d, confmap.WithIgnoreUnused())
-}
-
 // TODO: Currently, this func will not be called because of https://github.com/open-telemetry/opentelemetry-collector/issues/6671. Uncomment function and
 // add a test case to Test_loadMetadata for file no_monotonic.yaml once the issue is solved.
 //

cmd/mdatagen/templates/component_test.go.tmpl

@@ -88,7 +88,7 @@ func TestComponentLifecycle(t *testing.T) {
 	cfg := factory.CreateDefaultConfig()
 	sub, err := cm.Sub("tests::config")
 	require.NoError(t, err)
-	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, sub.Unmarshal(&cfg))
 
 	for _, test := range tests {
 		{{- if not .Tests.SkipShutdown }}
@@ -179,7 +179,7 @@ func TestComponentLifecycle(t *testing.T) {
 	cfg := factory.CreateDefaultConfig()
 	sub, err := cm.Sub("tests::config")
 	require.NoError(t, err)
-	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, sub.Unmarshal(&cfg))
 
 	for _, test := range tests {
 		{{- if not .Tests.SkipShutdown }}
@@ -268,7 +268,7 @@ func TestComponentLifecycle(t *testing.T) {
 	cfg := factory.CreateDefaultConfig()
 	sub, err := cm.Sub("tests::config")
 	require.NoError(t, err)
-	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, sub.Unmarshal(&cfg))
 
 	for _, test := range tests {
 		{{- if not .Tests.SkipShutdown }}
@@ -307,7 +307,7 @@ func TestComponentLifecycle(t *testing.T) {
 	cfg := factory.CreateDefaultConfig()
 	sub, err := cm.Sub("tests::config")
 	require.NoError(t, err)
-	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, sub.Unmarshal(&cfg))
 
 	{{- if not .Tests.SkipShutdown }}
 	t.Run("shutdown", func(t *testing.T) {
@@ -421,7 +421,7 @@ func TestComponentLifecycle(t *testing.T) {
 	cfg := factory.CreateDefaultConfig()
 	sub, err := cm.Sub("tests::config")
 	require.NoError(t, err)
-	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, sub.Unmarshal(&cfg))
 
 	for _, test := range tests {
 		{{- if not .Tests.SkipShutdown }}

component/config.go

@@ -27,13 +27,8 @@ type Config any
 var configValidatorType = reflect.TypeOf((*ConfigValidator)(nil)).Elem()
 
 // UnmarshalConfig helper function to UnmarshalConfig a Config.
-// It checks if the config implements confmap.Unmarshaler and uses that if available,
-// otherwise uses Map.UnmarshalExact, erroring if a field is nonexistent.
+// Deprecated: Use conf.Unmarshal(&intoCfg)
 func UnmarshalConfig(conf *confmap.Conf, intoCfg Config) error {
-	if cu, ok := intoCfg.(confmap.Unmarshaler); ok {
-		return cu.Unmarshal(conf)
-	}
-
 	return conf.Unmarshal(intoCfg)
 }
 

component/config_test.go

@@ -11,8 +11,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	"go.opentelemetry.io/collector/confmap"
 )
 
 var _ fmt.Stringer = Type{}
@@ -419,28 +417,3 @@ func TestNewType(t *testing.T) {
 		})
 	}
 }
-
-type configWithEmbeddedStruct struct {
-	String string `mapstructure:"string"`
-	Num    int    `mapstructure:"num"`
-	embeddedUnmarshallingConfig
-}
-
-type embeddedUnmarshallingConfig struct {
-}
-
-func (euc *embeddedUnmarshallingConfig) Unmarshal(_ *confmap.Conf) error {
-	return nil // do nothing.
-}
-func TestStructWithEmbeddedUnmarshaling(t *testing.T) {
-	t.Skip("Skipping, to be fixed with https://github.com/open-telemetry/opentelemetry-collector/issues/7102")
-	cfgMap := confmap.NewFromStringMap(map[string]any{
-		"string": "foo",
-		"num":    123,
-	})
-	tc := &configWithEmbeddedStruct{}
-	err := UnmarshalConfig(cfgMap, tc)
-	require.NoError(t, err)
-	assert.Equal(t, "foo", tc.String)
-	assert.Equal(t, 123, tc.Num)
-}