[confmap] confmap honors Unmarshal methods on config embedded structs. (#9635)

**Description:**
This implements support for calling `Unmarshal` on embedded structs of
structs being decoded.

**Link to tracking Issue:**
Fixes #6671

**Testing:**
Unit tests.

Contrib fix is open:
open-telemetry/opentelemetry-collector-contrib#31406

Author: atoulme

.chloggen/support_embedded_structs_confmap.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: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: confmap honors `Unmarshal` methods on config embedded structs.
+
+# One or more tracking issues or pull requests related to the change
+issues: [6671]
+
+# (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: []

component/config_test.go

@@ -11,6 +11,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"go.opentelemetry.io/collector/confmap"
 )
 
 var _ fmt.Stringer = Type{}
@@ -417,3 +419,28 @@ 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)
+}

confmap/confmap.go

@@ -157,6 +157,9 @@ func decodeConfig(m *Conf, result any, errorUnused bool) error {
 			mapstructure.StringToTimeDurationHookFunc(),
 			mapstructure.TextUnmarshallerHookFunc(),
 			unmarshalerHookFunc(result),
+			// after the main unmarshaler hook is called,
+			// we unmarshal the embedded structs if present to merge with the result:
+			unmarshalerEmbeddedStructsHookFunc(),
 			zeroSliceHookFunc(),
 		),
 	}
@@ -261,6 +264,42 @@ func mapKeyStringToMapKeyTextUnmarshalerHookFunc() mapstructure.DecodeHookFuncTy
 	}
 }
 
+// unmarshalerEmbeddedStructsHookFunc provides a mechanism for embedded structs to define their own unmarshal logic,
+// by implementing the Unmarshaler interface.
+func unmarshalerEmbeddedStructsHookFunc() mapstructure.DecodeHookFuncValue {
+	return func(from reflect.Value, to reflect.Value) (any, error) {
+		if to.Type().Kind() != reflect.Struct {
+			return from.Interface(), nil
+		}
+		fromAsMap, ok := from.Interface().(map[string]any)
+		if !ok {
+			return from.Interface(), nil
+		}
+		for i := 0; i < to.Type().NumField(); i++ {
+			// embedded structs passed in via `squash` cannot be pointers. We just check if they are structs:
+			if to.Type().Field(i).IsExported() && to.Type().Field(i).Anonymous {
+				if unmarshaler, ok := to.Field(i).Addr().Interface().(Unmarshaler); ok {
+					if err := unmarshaler.Unmarshal(NewFromStringMap(fromAsMap)); err != nil {
+						return nil, err
+					}
+					// the struct we receive from this unmarshaling only contains fields related to the embedded struct.
+					// we merge this partially unmarshaled struct with the rest of the result.
+					// note we already unmarshaled the main struct earlier, and therefore merge with it.
+					conf := New()
+					if err := conf.Marshal(unmarshaler); err != nil {
+						return nil, err
+					}
+					resultMap := conf.ToStringMap()
+					for k, v := range resultMap {
+						fromAsMap[k] = v
+					}
+				}
+			}
+		}
+		return fromAsMap, nil
+	}
+}
+
 // Provides a mechanism for individual structs to define their own unmarshal logic,
 // by implementing the Unmarshaler interface.
 func unmarshalerHookFunc(result any) mapstructure.DecodeHookFuncValue {

confmap/confmap_test.go

@@ -309,8 +309,78 @@ func newConfFromFile(t testing.TB, fileName string) map[string]any {
 }
 
 type testConfig struct {
-	Next    *nextConfig `mapstructure:"next"`
-	Another string      `mapstructure:"another"`
+	Next            *nextConfig `mapstructure:"next"`
+	Another         string      `mapstructure:"another"`
+	EmbeddedConfig  `mapstructure:",squash"`
+	EmbeddedConfig2 `mapstructure:",squash"`
+}
+
+type testConfigWithoutUnmarshaler struct {
+	Next            *nextConfig `mapstructure:"next"`
+	Another         string      `mapstructure:"another"`
+	EmbeddedConfig  `mapstructure:",squash"`
+	EmbeddedConfig2 `mapstructure:",squash"`
+}
+
+type testConfigWithEmbeddedError struct {
+	Next                    *nextConfig `mapstructure:"next"`
+	Another                 string      `mapstructure:"another"`
+	EmbeddedConfigWithError `mapstructure:",squash"`
+}
+
+type testConfigWithMarshalError struct {
+	Next                           *nextConfig `mapstructure:"next"`
+	Another                        string      `mapstructure:"another"`
+	EmbeddedConfigWithMarshalError `mapstructure:",squash"`
+}
+
+func (tc *testConfigWithEmbeddedError) Unmarshal(component *Conf) error {
+	if err := component.Unmarshal(tc, WithIgnoreUnused()); err != nil {
+		return err
+	}
+	return nil
+}
+
+type EmbeddedConfig struct {
+	Some string `mapstructure:"some"`
+}
+
+func (ec *EmbeddedConfig) Unmarshal(component *Conf) error {
+	if err := component.Unmarshal(ec, WithIgnoreUnused()); err != nil {
+		return err
+	}
+	ec.Some += " is also called"
+	return nil
+}
+
+type EmbeddedConfig2 struct {
+	Some2 string `mapstructure:"some_2"`
+}
+
+func (ec *EmbeddedConfig2) Unmarshal(component *Conf) error {
+	if err := component.Unmarshal(ec, WithIgnoreUnused()); err != nil {
+		return err
+	}
+	ec.Some2 += " also called2"
+	return nil
+}
+
+type EmbeddedConfigWithError struct {
+}
+
+func (ecwe *EmbeddedConfigWithError) Unmarshal(_ *Conf) error {
+	return errors.New("embedded error")
+}
+
+type EmbeddedConfigWithMarshalError struct {
+}
+
+func (ecwe EmbeddedConfigWithMarshalError) Marshal(_ *Conf) error {
+	return errors.New("marshaling error")
+}
+
+func (ecwe EmbeddedConfigWithMarshalError) Unmarshal(_ *Conf) error {
+	return nil
 }
 
 func (tc *testConfig) Unmarshal(component *Conf) error {
@@ -340,12 +410,59 @@ func TestUnmarshaler(t *testing.T) {
 			"string": "make sure this",
 		},
 		"another": "make sure this",
+		"some":    "make sure this",
+		"some_2":  "this better be",
 	})
 
 	tc := &testConfig{}
 	assert.NoError(t, cfgMap.Unmarshal(tc))
 	assert.Equal(t, "make sure this", tc.Another)
 	assert.Equal(t, "make sure this is called", tc.Next.String)
+	assert.Equal(t, "make sure this is also called", tc.EmbeddedConfig.Some)
+	assert.Equal(t, "this better be also called2", tc.EmbeddedConfig2.Some2)
+}
+
+func TestEmbeddedUnmarshaler(t *testing.T) {
+	cfgMap := NewFromStringMap(map[string]any{
+		"next": map[string]any{
+			"string": "make sure this",
+		},
+		"another": "make sure this",
+		"some":    "make sure this",
+		"some_2":  "this better be",
+	})
+
+	tc := &testConfigWithoutUnmarshaler{}
+	assert.NoError(t, cfgMap.Unmarshal(tc))
+	assert.Equal(t, "make sure this", tc.Another)
+	assert.Equal(t, "make sure this is called", tc.Next.String)
+	assert.Equal(t, "make sure this is also called", tc.EmbeddedConfig.Some)
+	assert.Equal(t, "this better be also called2", tc.EmbeddedConfig2.Some2)
+}
+
+func TestEmbeddedUnmarshalerError(t *testing.T) {
+	cfgMap := NewFromStringMap(map[string]any{
+		"next": map[string]any{
+			"string": "make sure this",
+		},
+		"another": "make sure this",
+		"some":    "make sure this",
+	})
+
+	tc := &testConfigWithEmbeddedError{}
+	assert.EqualError(t, cfgMap.Unmarshal(tc), "error decoding '': embedded error")
+}
+
+func TestEmbeddedMarshalerError(t *testing.T) {
+	cfgMap := NewFromStringMap(map[string]any{
+		"next": map[string]any{
+			"string": "make sure this",
+		},
+		"another": "make sure this",
+	})
+
+	tc := &testConfigWithMarshalError{}
+	assert.EqualError(t, cfgMap.Unmarshal(tc), "error decoding '': error running encode hook: marshaling error")
 }
 
 func TestUnmarshalerKeepAlreadyInitialized(t *testing.T) {