Deprecate Apply in favor of Set (#7018)

Trying to keep the API closer to some standard libraries like FlagsSet. If a getter is needed we should name it Lookup.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

.chloggen/depapply.yaml

@@ -0,0 +1,11 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: deprecation
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: featuregate
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Deprecate Apply in favor of Set
+
+# One or more tracking issues or pull requests related to the change
+issues: [7018]

featuregate/registry.go

@@ -73,19 +73,12 @@ func WithRegisterRemovalVersion(version string) RegistryOption {
 	})
 }
 
-// Apply a configuration in the form of a map of Gate identifiers to boolean values.
-// Sets only those values provided in the map, other gate values are not changed.
+// Deprecated: [v0.71.0] use Set.
 func (r *Registry) Apply(cfg map[string]bool) error {
-	for id, val := range cfg {
-		v, ok := r.gates.Load(id)
-		if !ok {
-			return fmt.Errorf("feature gate %s is unregistered", id)
+	for id, enabled := range cfg {
+		if err := r.Set(id, enabled); err != nil {
+			return err
 		}
-		g := v.(*Gate)
-		if g.stage == StageStable {
-			return fmt.Errorf("feature gate %s is stable, can not be modified", id)
-		}
-		g.enabled.Store(val)
 	}
 	return nil
 }
@@ -146,6 +139,20 @@ func (r *Registry) RegisterID(id string, stage Stage, opts ...RegistryOption) er
 	return err
 }
 
+// Set the enabled valued for a Gate identified by the given id.
+func (r *Registry) Set(id string, enabled bool) error {
+	v, ok := r.gates.Load(id)
+	if !ok {
+		return fmt.Errorf("no such feature gate -%v", id)
+	}
+	g := v.(*Gate)
+	if g.stage == StageStable {
+		return fmt.Errorf("feature gate %s is stable, can not be modified", id)
+	}
+	g.enabled.Store(enabled)
+	return nil
+}
+
 // List returns a slice of copies of all registered Gates.
 func (r *Registry) List() []Gate {
 	var ret []Gate

featuregate/registry_test.go

@@ -28,7 +28,7 @@ func TestGlobalRegistry(t *testing.T) {
 func TestRegistry(t *testing.T) {
 	r := NewRegistry()
 
-	id := "foo"
+	const id = "foo"
 
 	assert.Empty(t, r.List())
 
@@ -37,7 +37,7 @@ func TestRegistry(t *testing.T) {
 	assert.Len(t, r.List(), 1)
 	assert.True(t, g.IsEnabled())
 
-	assert.NoError(t, r.Apply(map[string]bool{id: false}))
+	assert.NoError(t, r.Set(id, false))
 	assert.False(t, g.IsEnabled())
 
 	_, err = r.Register(id, StageBeta)
@@ -49,18 +49,18 @@ func TestRegistry(t *testing.T) {
 
 func TestRegistryApplyError(t *testing.T) {
 	r := NewRegistry()
-	assert.Error(t, r.Apply(map[string]bool{"foo": true}))
+	assert.Error(t, r.Set("foo", true))
 	r.MustRegister("bar", StageAlpha)
-	assert.Error(t, r.Apply(map[string]bool{"foo": true}))
+	assert.Error(t, r.Set("foo", true))
 	r.MustRegister("foo", StageStable, WithRegisterRemovalVersion("next"))
-	assert.Error(t, r.Apply(map[string]bool{"foo": true}))
+	assert.Error(t, r.Set("foo", true))
 }
 
 func TestRegistryApply(t *testing.T) {
 	r := NewRegistry()
 	fooGate := r.MustRegister("foo", StageAlpha, WithRegisterDescription("Test Gate"))
 	assert.False(t, fooGate.IsEnabled())
-	assert.NoError(t, r.Apply(map[string]bool{fooGate.ID(): true}))
+	assert.NoError(t, r.Set(fooGate.ID(), true))
 	assert.True(t, fooGate.IsEnabled())
 }
 

otelcol/command.go

@@ -44,8 +44,10 @@ func NewCommand(set CollectorSettings) *cobra.Command {
 }
 
 func newCollectorWithFlags(set CollectorSettings, flags *flag.FlagSet) (*Collector, error) {
-	if err := featuregate.GlobalRegistry().Apply(getFeatureGatesFlag(flags)); err != nil {
-		return nil, err
+	for id, enabled := range getFeatureGatesFlag(flags) {
+		if err := featuregate.GlobalRegistry().Set(id, enabled); err != nil {
+			return nil, err
+		}
 	}
 	if set.ConfigProvider == nil {
 		configFlags := getConfigFlag(flags)

otelcol/config_test.go

@@ -255,9 +255,9 @@ func TestConfigValidate(t *testing.T) {
 		},
 	}
 
-	require.NoError(t, featuregate.GlobalRegistry().Apply(map[string]bool{connectorsFeatureGate.ID(): true}))
+	require.NoError(t, featuregate.GlobalRegistry().Set(connectorsFeatureGate.ID(), true))
 	defer func() {
-		require.NoError(t, featuregate.GlobalRegistry().Apply(map[string]bool{connectorsFeatureGate.ID(): false}))
+		require.NoError(t, featuregate.GlobalRegistry().Set(connectorsFeatureGate.ID(), false))
 	}()
 	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {

otelcol/otelcoltest/config_test.go

@@ -75,9 +75,9 @@ func TestLoadConfigAndValidate(t *testing.T) {
 	factories, err := NopFactories()
 	assert.NoError(t, err)
 
-	require.NoError(t, featuregate.GlobalRegistry().Apply(map[string]bool{"otelcol.enableConnectors": true}))
+	require.NoError(t, featuregate.GlobalRegistry().Set("otelcol.enableConnectors", true))
 	defer func() {
-		require.NoError(t, featuregate.GlobalRegistry().Apply(map[string]bool{"otelcol.enableConnectors": false}))
+		require.NoError(t, featuregate.GlobalRegistry().Set("otelcol.enableConnectors", false))
 	}()
 	cfgValidate, errValidate := LoadConfigAndValidate(filepath.Join("testdata", "config.yaml"), factories)
 	require.NoError(t, errValidate)