Skip to content

Commit

Permalink
Deprecate Apply in favor of Set
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
bogdandrutu committed Jan 25, 2023
1 parent fec9cd3 commit a02aa9e
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 25 deletions.
12 changes: 12 additions & 0 deletions .chloggen/depapply.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# 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]

29 changes: 18 additions & 11 deletions featuregate/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions featuregate/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand All @@ -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)
Expand All @@ -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())
}

Expand Down
6 changes: 4 additions & 2 deletions otelcol/collector_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ func (s *windowsService) start(elog *eventlog.Log, colErrorChannel chan error) e
return err
}

if err := featuregate.GlobalRegistry().Apply(getFeatureGatesFlag(s.flags)); err != nil {
return err
for id, enabled := range getFeatureGatesFlag(flagSet) {
if err := featuregate.GlobalRegistry().Set(id, enabled); err != nil {
return err
}
}
var err error
s.col, err = newWithWindowsEventLogCore(s.settings, s.flags, elog)
Expand Down
6 changes: 4 additions & 2 deletions otelcol/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ func NewCommand(set CollectorSettings) *cobra.Command {
Version: set.BuildInfo.Version,
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
if err := featuregate.GlobalRegistry().Apply(getFeatureGatesFlag(flagSet)); err != nil {
return err
for id, enabled := range getFeatureGatesFlag(flagSet) {
if err := featuregate.GlobalRegistry().Set(id, enabled); err != nil {
return err
}
}
if set.ConfigProvider == nil {
var err error
Expand Down
4 changes: 2 additions & 2 deletions otelcol/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions otelcol/otelcoltest/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,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)
Expand Down

0 comments on commit a02aa9e

Please sign in to comment.