diff --git a/.chloggen/support_embedded_structs_confmap.yaml b/.chloggen/support_embedded_structs_confmap.yaml new file mode 100755 index 00000000000..7d010e9524d --- /dev/null +++ b/.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: [] diff --git a/cmd/mdatagen/metricdata.go b/cmd/mdatagen/metricdata.go index f4515136aae..5bf77985cc9 100644 --- a/cmd/mdatagen/metricdata.go +++ b/cmd/mdatagen/metricdata.go @@ -125,7 +125,7 @@ func (d *gauge) Unmarshal(parser *confmap.Conf) error { if err := d.MetricValueType.Unmarshal(parser); err != nil { return err } - return parser.Unmarshal(d) + return parser.Unmarshal(d, confmap.WithIgnoreUnused()) } func (d gauge) Type() string { @@ -155,7 +155,7 @@ func (d *sum) Unmarshal(parser *confmap.Conf) error { if err := d.MetricValueType.Unmarshal(parser); err != nil { return err } - return parser.Unmarshal(d) + 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 diff --git a/component/config_test.go b/component/config_test.go index fcddc343d20..2359dc09419 100644 --- a/component/config_test.go +++ b/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) +} diff --git a/confmap/confmap.go b/confmap/confmap.go index 213e4a332d9..be8515a8faf 100644 --- a/confmap/confmap.go +++ b/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 { diff --git a/confmap/confmap_test.go b/confmap/confmap_test.go index 6680a1bc517..20659fd29f3 100644 --- a/confmap/confmap_test.go +++ b/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) {