[confmap] Embedded structs fail to unmarshal

[bogdandrutu]

Component(s)

confmap

What happened?

type Config struct {
	FooConfig `mapstructure:"foo"`
	Encoding  string  `mapstructure:"encoding"`
}

type FooConfig struct {
   MyString "my_foo_string"
} 

Now, if we define the "Unmarshal" on "FooConfig" we will get an error that encoding is not defined. This is a problem that will stop development for FooConfig.

Collector version

v0.122.0

[github-actions]

Pinging code owners:

• confmap: @mx-psi @evan-bradley

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[bogdandrutu]

Probably the easiest solution is to disallow "embedded" structs that are not "squashed", but would be good for someone to think about this.

[mattsains]

I ran into this issue myself when trying to upgrade to 0.123.0

It seems that using an embedded struct also brings in all of its implementations, so Config ends up having the unmarshaler of FooConfig.

This was surprising to me as someone who isn't a go expert.

[songy23]

Not sure if it's the same issue but I found the error message quite misleading:

Consider this simple test

type MyExpCfg struct {
	Timeout time.Duration `mapstructure:"timeout"`
	QueueConfig `mapstructure:",squash"`
}

func TestUnmarshalConfig(t *testing.T) {
	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml"))
	require.NoError(t, err)
	sub, err := cm.Sub(component.MustNewID("my_exp").String())
	require.NoError(t, err)
	got := MyExpCfg{}
	require.NoError(t, sub.Unmarshal(&got))
	assert.NoError(t, xconfmap.Validate(got))
}

Test config

my_exp:
  timeout: 10s
  queue_size: 100

This used to work fine until v0.123.0. In v0.123.0 the above returns an error:

decoding failed due to the following error(s):
error decoding '': decoding failed due to the following error(s):
'' has invalid keys: timeout

...but the issue has nothing to do with timeout, the fix is instead to move QueueConfig to a dedicated sub-section like

type MyExpCfg struct {
	Timeout time.Duration `mapstructure:"timeout"`
	QueueConfig `mapstructure:"sending_queue"`
}

This should be an API breaking change, though I cannot find any reference in the changelog

[songy23]

Probably the easiest solution is to disallow "embedded" structs that are not "squashed"

In the example above if you squash the embedded struct, it leads to an unmarshal error.