Skip to content

Commit

Permalink
Revert "Make the option WithErrorUnused enabled by default when unm…
Browse files Browse the repository at this point in the history
…arshaling configuration (open-telemetry#9154)"

This reverts commit 6155cc2. I propose reverting this change as it's blocking the release. It's causing more problems and i'm not sure if there's a way to fix these tests as they call UnmarshalConfig which doesn't provide a way to pass in WithIgnoreUnused https://github.com/open-telemetry/opentelemetry-collector/blob/6a726c955a137f05f5d5ad402098f3220f42ceb6/component/config.go#L30-L36

See
https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/7466429082/job/20317807463?pr=30167
  • Loading branch information
Alex Boten committed Jan 9, 2024
1 parent 6a726c9 commit ac1747c
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 50 deletions.
27 changes: 0 additions & 27 deletions .chloggen/make_error_unused_default.yaml

This file was deleted.

2 changes: 1 addition & 1 deletion component/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func UnmarshalConfig(conf *confmap.Conf, intoCfg Config) error {
return cu.Unmarshal(conf)
}

return conf.Unmarshal(intoCfg)
return conf.Unmarshal(intoCfg, confmap.WithErrorUnused())
}

// ConfigValidator defines an optional interface for configurations to implement to do validation.
Expand Down
18 changes: 4 additions & 14 deletions confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,15 @@ type UnmarshalOption interface {
}

type unmarshalOption struct {
ignoreUnused bool
errorUnused bool
}

// WithErrorUnused sets an option to error when there are existing
// keys in the original Conf that were unused in the decoding process
// (extra keys). This option is enabled by default and can be disabled with `WithIgnoreUnused`.
// Deprecated: [v0.92.0] this is now enabled by default. Use `WithIgnoreUnused` to disable.
func WithErrorUnused() UnmarshalOption {
return unmarshalOptionFunc(func(uo *unmarshalOption) {
uo.ignoreUnused = false
})
}

// WithIgnoreUnused sets an option to ignore errors if existing
// keys in the original Conf were unused in the decoding process
// (extra keys).
func WithIgnoreUnused() UnmarshalOption {
func WithErrorUnused() UnmarshalOption {
return unmarshalOptionFunc(func(uo *unmarshalOption) {
uo.ignoreUnused = true
uo.errorUnused = true
})
}

Expand All @@ -86,7 +76,7 @@ func (l *Conf) Unmarshal(result any, opts ...UnmarshalOption) error {
for _, opt := range opts {
opt.apply(&set)
}
return decodeConfig(l, result, !set.ignoreUnused)
return decodeConfig(l, result, set.errorUnused)
}

type marshalOption struct{}
Expand Down
3 changes: 1 addition & 2 deletions confmap/confmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ func TestUnmarshalWithErrorUnused(t *testing.T) {
"string": "this is a string",
}
conf := NewFromStringMap(stringMap)
assert.Error(t, conf.Unmarshal(&TestIDConfig{}))
assert.NoError(t, conf.Unmarshal(&TestIDConfig{}, WithIgnoreUnused()))
assert.Error(t, conf.Unmarshal(&TestIDConfig{}, WithErrorUnused()))
}

type TestConfig struct {
Expand Down
2 changes: 1 addition & 1 deletion exporter/loggingexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
return fmt.Errorf("'loglevel' and 'verbosity' are incompatible. Use only 'verbosity' instead")
}

if err := conf.Unmarshal(cfg); err != nil {
if err := conf.Unmarshal(cfg, confmap.WithErrorUnused()); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion otelcol/internal/configunmarshaler/configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func NewConfigs[F component.Factory](factories map[component.Type]F) *Configs[F]

func (c *Configs[F]) Unmarshal(conf *confmap.Conf) error {
rawCfgs := make(map[component.ID]map[string]any)
if err := conf.Unmarshal(&rawCfgs); err != nil {
if err := conf.Unmarshal(&rawCfgs, confmap.WithErrorUnused()); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion otelcol/unmarshaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,5 @@ func unmarshal(v *confmap.Conf, factories Factories) (*configSettings, error) {
},
}

return cfg, v.Unmarshal(&cfg)
return cfg, v.Unmarshal(&cfg, confmap.WithErrorUnused())
}
4 changes: 2 additions & 2 deletions otelcol/unmarshaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func TestPipelineConfigUnmarshalError(t *testing.T) {
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
pips := new(pipelines.Config)
err := tt.conf.Unmarshal(&pips)
err := tt.conf.Unmarshal(&pips, confmap.WithErrorUnused())
require.Error(t, err)
assert.Contains(t, err.Error(), tt.expectError)
})
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestServiceUnmarshalError(t *testing.T) {

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
err := tt.conf.Unmarshal(&service.Config{})
err := tt.conf.Unmarshal(&service.Config{}, confmap.WithErrorUnused())
require.Error(t, err)
assert.Contains(t, err.Error(), tt.expectError)
})
Expand Down
2 changes: 1 addition & 1 deletion receiver/otlpreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (cfg *Config) Validate() error {
// Unmarshal a confmap.Conf into the config struct.
func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
// first load the config normally
err := conf.Unmarshal(cfg)
err := conf.Unmarshal(cfg, confmap.WithErrorUnused())
if err != nil {
return err
}
Expand Down

0 comments on commit ac1747c

Please sign in to comment.