diff --git a/.chloggen/mx-psi_configoptional-enabled-impl.yaml b/.chloggen/mx-psi_configoptional-enabled-impl.yaml new file mode 100644 index 00000000000..e2550eae160 --- /dev/null +++ b/.chloggen/mx-psi_configoptional-enabled-impl.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: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: pkg/config/configoptional + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Adds new `configoptional.AddEnabledField` feature gate that allows users to explicitly disable a `configoptional.Optional` through a new `enabled` field. + +# One or more tracking issues or pull requests related to the change +issues: [14021] + +# (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/config/configoptional/go.mod b/config/configoptional/go.mod index cd953b1cfea..514bf8480a3 100644 --- a/config/configoptional/go.mod +++ b/config/configoptional/go.mod @@ -6,6 +6,7 @@ require ( github.com/stretchr/testify v1.11.1 go.opentelemetry.io/collector/confmap v1.44.0 go.opentelemetry.io/collector/confmap/xconfmap v0.138.0 + go.opentelemetry.io/collector/featuregate v1.44.0 go.uber.org/goleak v1.3.0 ) @@ -20,7 +21,6 @@ require ( github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - go.opentelemetry.io/collector/featuregate v1.44.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect diff --git a/config/configoptional/optional.go b/config/configoptional/optional.go index c950487953f..c510a021b37 100644 --- a/config/configoptional/optional.go +++ b/config/configoptional/optional.go @@ -11,6 +11,7 @@ import ( "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/confmap/xconfmap" + "go.opentelemetry.io/collector/featuregate" ) type flavor int @@ -26,6 +27,7 @@ const ( // It supports a third flavor for struct types: Default(defaultVal). // // For struct types, it supports unmarshaling from a configuration source. +// For struct types, it supports an 'enabled' field to explicitly disable a section. // The zero value of Optional is None. type Optional[T any] struct { // value is the value of the Optional. @@ -165,6 +167,17 @@ func (o *Optional[T]) GetOrInsertDefault() *T { var _ confmap.Unmarshaler = (*Optional[any])(nil) +var ( + addEnabledFieldFeatureGateID = "configoptional.AddEnabledField" + addEnabledFieldFeatureGate = featuregate.GlobalRegistry().MustRegister( + addEnabledFieldFeatureGateID, + featuregate.StageAlpha, + featuregate.WithRegisterFromVersion("v0.138.0"), + featuregate.WithRegisterDescription("Allows optional fields to be toggled via an 'enabled' field."), + featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector/issues/14021"), + ) +) + // Unmarshal the configuration into the Optional value. // // The behavior of this method depends on the state of the Optional: @@ -173,6 +186,11 @@ var _ confmap.Unmarshaler = (*Optional[any])(nil) // - Default[T](val), equivalent to unmarshaling into a field of type T with base value val, // using val without overrides from the configuration if the configuration is nil. // +// (Under the `configoptional.AddEnabledField` feature gate) +// If the configuration contains an 'enabled' field: +// - if enabled is true: the Optional becomes Some after unmarshaling. +// - if enabled is false: the Optional becomes None regardless of other configuration values. +// // T must be derefenceable to a type with struct kind and not have an 'enabled' field. // Scalar values are not supported. func (o *Optional[T]) Unmarshal(conf *confmap.Conf) error { @@ -186,11 +204,26 @@ func (o *Optional[T]) Unmarshal(conf *confmap.Conf) error { return nil } + isEnabled := true + if addEnabledFieldFeatureGate.IsEnabled() && conf.IsSet("enabled") { + enabled := conf.Get("enabled") + conf.Delete("enabled") + var ok bool + if isEnabled, ok = enabled.(bool); !ok { + return fmt.Errorf("unexpected type %T for 'enabled': got '%v' value expected 'true' or 'false'", enabled, enabled) + } + } + if err := conf.Unmarshal(&o.value); err != nil { return err } - o.flavor = someFlavor + if isEnabled { + o.flavor = someFlavor + } else { + o.flavor = noneFlavor + } + return nil } diff --git a/config/configoptional/optional_test.go b/config/configoptional/optional_test.go index 619576ea9be..ff301b51814 100644 --- a/config/configoptional/optional_test.go +++ b/config/configoptional/optional_test.go @@ -14,6 +14,7 @@ import ( "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/confmap/confmaptest" "go.opentelemetry.io/collector/confmap/xconfmap" + "go.opentelemetry.io/collector/featuregate" ) type Config[T any] struct { @@ -377,6 +378,174 @@ func TestUnmarshalOptional(t *testing.T) { } } +func TestAddFieldEnabledFeatureGate(t *testing.T) { + tests := []struct { + name string + config map[string]any + defaultCfg Config[Sub] + expectedSub bool + expectedFoo string + }{ + { + name: "none_with_enabled_true", + config: map[string]any{ + "sub": map[string]any{ + "enabled": true, + "foo": "bar", + }, + }, + defaultCfg: Config[Sub]{ + Sub1: None[Sub](), + }, + expectedSub: true, + expectedFoo: "bar", + }, + { + name: "none_with_enabled_false", + config: map[string]any{ + "sub": map[string]any{ + "enabled": false, + "foo": "bar", + }, + }, + defaultCfg: Config[Sub]{ + Sub1: None[Sub](), + }, + expectedSub: false, + }, + { + name: "none_with_enabled_false_no_other_config", + config: map[string]any{ + "sub": map[string]any{ + "enabled": false, + }, + }, + defaultCfg: Config[Sub]{ + Sub1: None[Sub](), + }, + expectedSub: false, + }, + { + name: "default_with_enabled_true", + config: map[string]any{ + "sub": map[string]any{ + "enabled": true, + "foo": "bar", + }, + }, + defaultCfg: Config[Sub]{ + Sub1: Default(subDefault), + }, + expectedSub: true, + expectedFoo: "bar", + }, + { + name: "default_with_enabled_false", + config: map[string]any{ + "sub": map[string]any{ + "enabled": false, + "foo": "bar", + }, + }, + defaultCfg: Config[Sub]{ + Sub1: Default(subDefault), + }, + expectedSub: false, + }, + { + name: "default_with_enabled_false_no_other_config", + config: map[string]any{ + "sub": map[string]any{ + "enabled": false, + }, + }, + defaultCfg: Config[Sub]{ + Sub1: Default(subDefault), + }, + expectedSub: false, + }, + { + name: "some_with_enabled_true", + config: map[string]any{ + "sub": map[string]any{ + "enabled": true, + "foo": "baz", + }, + }, + defaultCfg: Config[Sub]{ + Sub1: Some(Sub{ + Foo: "foobar", + }), + }, + expectedSub: true, + expectedFoo: "baz", + }, + { + name: "some_with_enabled_false", + config: map[string]any{ + "sub": map[string]any{ + "enabled": false, + "foo": "baz", + }, + }, + defaultCfg: Config[Sub]{ + Sub1: Some(Sub{ + Foo: "foobar", + }), + }, + expectedSub: false, + }, + { + name: "some_with_enabled_false_no_other_config", + config: map[string]any{ + "sub": map[string]any{ + "enabled": false, + }, + }, + defaultCfg: Config[Sub]{ + Sub1: Some(Sub{ + Foo: "foobar", + }), + }, + expectedSub: false, + }, + } + + oldVal := addEnabledFieldFeatureGate.IsEnabled() + require.NoError(t, featuregate.GlobalRegistry().Set(addEnabledFieldFeatureGateID, true)) + defer func() { require.NoError(t, featuregate.GlobalRegistry().Set(addEnabledFieldFeatureGateID, oldVal)) }() + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cfg := test.defaultCfg + conf := confmap.NewFromStringMap(test.config) + require.NoError(t, conf.Unmarshal(&cfg)) + require.Equal(t, test.expectedSub, cfg.Sub1.HasValue()) + if test.expectedSub { + require.Equal(t, test.expectedFoo, cfg.Sub1.Get().Foo) + } + }) + } +} + +func TestUnmarshalErrorEnabledInvalidType(t *testing.T) { + oldVal := addEnabledFieldFeatureGate.IsEnabled() + require.NoError(t, featuregate.GlobalRegistry().Set(addEnabledFieldFeatureGateID, true)) + defer func() { require.NoError(t, featuregate.GlobalRegistry().Set(addEnabledFieldFeatureGateID, oldVal)) }() + + cm := confmap.NewFromStringMap(map[string]any{ + "sub": map[string]any{ + "enabled": "something", + "foo": "bar", + }, + }) + cfg := Config[Sub]{ + Sub1: None[Sub](), + } + err := cm.Unmarshal(&cfg) + require.ErrorContains(t, err, "unexpected type string for 'enabled': got 'something' value expected 'true' or 'false'") +} + func TestUnmarshalErrorEnabledField(t *testing.T) { cm := confmap.NewFromStringMap(map[string]any{ "enabled": true,