From 2c90e5c6bd689c61c99bdce86a91305f4d59c686 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Thu, 30 Mar 2023 13:01:40 -0600 Subject: [PATCH 1/3] Add ability to use feature-gates in the operator --- go.mod | 3 +- go.sum | 5 ++- main.go | 4 ++ pkg/flags/flags.go | 32 +++++++++++++++ pkg/flags/flags_test.go | 89 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 pkg/flags/flags.go create mode 100644 pkg/flags/flags_test.go diff --git a/go.mod b/go.mod index 3d87781251..824d0bbbaf 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/prometheus/prometheus v1.8.2-0.20210621150501-ff58416a0b02 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.2 + go.opentelemetry.io/collector/featuregate v0.74.0 go.opentelemetry.io/otel v1.14.0 gopkg.in/yaml.v2 v2.4.0 k8s.io/api v0.26.3 @@ -109,7 +110,7 @@ require ( go.opencensus.io v0.23.0 // indirect go.opentelemetry.io/otel/trace v1.14.0 // indirect go.uber.org/atomic v1.8.0 // indirect - go.uber.org/multierr v1.6.0 // indirect + go.uber.org/multierr v1.10.0 // indirect go.uber.org/zap v1.24.0 // indirect golang.org/x/crypto v0.1.0 // indirect golang.org/x/net v0.7.0 // indirect diff --git a/go.sum b/go.sum index a056d09dd4..99c0d160fa 100644 --- a/go.sum +++ b/go.sum @@ -1039,6 +1039,8 @@ go.opencensus.io v0.22.4/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.5/go.mod h1:5pWMHQbX5EPX2/62yrJeAkowc+lfs/XD7Uxpq3pI6kk= go.opencensus.io v0.23.0 h1:gqCw0LfLxScz8irSi8exQc7fyQ0fKQU/qnC/X8+V/1M= go.opencensus.io v0.23.0/go.mod h1:XItmlyltB5F7CS4xOC1DcqMoFqwtC6OG2xF7mCv7P7E= +go.opentelemetry.io/collector/featuregate v0.74.0 h1:hzkzhi6pvjqEK5+CkVBJX69wpEEYqgtTFMHGlZFsQyE= +go.opentelemetry.io/collector/featuregate v0.74.0/go.mod h1:pmVMr98Ps6QKyEHiVPN7o3Qd8K//M2NapfOv5BMWvA0= go.opentelemetry.io/otel v1.14.0 h1:/79Huy8wbf5DnIPhemGB+zEPVwnN6fuQybr/SRXa6hM= go.opentelemetry.io/otel v1.14.0/go.mod h1:o4buv+dJzx8rohcUeRmWUZhqupFvzWis188WlggnNeU= go.opentelemetry.io/otel/trace v1.14.0 h1:wp2Mmvj41tDsyAJXiWDWpfNsOiIyd38fy85pyKcFq/M= @@ -1057,8 +1059,9 @@ go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/ go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.4.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.5.0/go.mod h1:FeouvMocqHpRaaGuG9EjoKcStLC43Zu/fmqdUMPcKYU= -go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= +go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= +go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee/go.mod h1:vJERXedbb3MVM5f9Ejo0C68/HhF8uaILCdgjnY+goOA= go.uber.org/zap v1.9.1/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= diff --git a/main.go b/main.go index 0d677edbc1..503894b69f 100644 --- a/main.go +++ b/main.go @@ -26,6 +26,7 @@ import ( routev1 "github.com/openshift/api/route/v1" "github.com/spf13/pflag" + "go.opentelemetry.io/collector/featuregate" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sruntime "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -47,6 +48,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/webhookhandler" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" collectorupgrade "github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade" + "github.com/open-telemetry/opentelemetry-operator/pkg/flags" "github.com/open-telemetry/opentelemetry-operator/pkg/instrumentation" instrumentationupgrade "github.com/open-telemetry/opentelemetry-operator/pkg/instrumentation/upgrade" "github.com/open-telemetry/opentelemetry-operator/pkg/sidecar" @@ -74,8 +76,10 @@ func init() { func main() { // registers any flags that underlying libraries might use opts := zap.Options{} + flagset := flags.Flags(featuregate.GlobalRegistry()) opts.BindFlags(flag.CommandLine) pflag.CommandLine.AddGoFlagSet(flag.CommandLine) + pflag.CommandLine.AddGoFlagSet(flagset) v := version.Get() diff --git a/pkg/flags/flags.go b/pkg/flags/flags.go new file mode 100644 index 0000000000..6a5aecab5f --- /dev/null +++ b/pkg/flags/flags.go @@ -0,0 +1,32 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package flags + +import ( + "flag" + + "go.opentelemetry.io/collector/featuregate" +) + +const ( + featureGatesFlag = "feature-gates" +) + +func Flags(reg *featuregate.Registry) *flag.FlagSet { + flagSet := new(flag.FlagSet) + flagSet.Var(featuregate.NewFlag(reg), featureGatesFlag, + "Comma-delimited list of feature gate identifiers. Prefix with '-' to disable the feature. '+' or no prefix will enable the feature.") + return flagSet +} diff --git a/pkg/flags/flags_test.go b/pkg/flags/flags_test.go new file mode 100644 index 0000000000..eef0462b45 --- /dev/null +++ b/pkg/flags/flags_test.go @@ -0,0 +1,89 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package flags + +import ( + "testing" + + "go.opentelemetry.io/collector/featuregate" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + basicGate = "basic" + advancedGate = "advanced" + falseGate = "false" +) + +func TestSetFlag(t *testing.T) { + featuregate.GlobalRegistry().MustRegister(basicGate, featuregate.StageAlpha) + featuregate.GlobalRegistry().MustRegister(advancedGate, featuregate.StageBeta) + featuregate.GlobalRegistry().MustRegister(falseGate, featuregate.StageStable, featuregate.WithRegisterRemovalVersion("v0.0.1")) + tests := []struct { + name string + args []string + expectedTrue []string + expectedFalse []string + expectedErr string + }{ + { + name: "simple set", + args: []string{"--feature-gates=basic"}, + expectedTrue: []string{basicGate}, + }, + { + name: "two flags", + args: []string{"--feature-gates=basic,advanced"}, + expectedTrue: []string{basicGate, advancedGate}, + }, + { + name: "one true one false", + args: []string{"--feature-gates=basic,-advanced"}, + expectedTrue: []string{basicGate}, + expectedFalse: []string{advancedGate}, + }, + { + name: "invalid set", + args: []string{"--feature-gates=01"}, + expectedErr: `no such feature gate -01`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + flgs := Flags(featuregate.GlobalRegistry()) + err := flgs.Parse(tt.args) + if tt.expectedErr != "" { + require.Error(t, err) + } else { + require.NoError(t, err) + } + featuregate.GlobalRegistry().VisitAll(func(gate *featuregate.Gate) { + for _, id := range tt.expectedTrue { + if gate.ID() == id { + assert.True(t, gate.IsEnabled()) + } + } + for _, id := range tt.expectedFalse { + if gate.ID() == id { + assert.False(t, gate.IsEnabled()) + } + } + }) + }) + } +} From dd9592b5fcdfa72740e3b525695cca0f240f3393 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Thu, 30 Mar 2023 13:05:43 -0600 Subject: [PATCH 2/3] Add changelog --- .chloggen/add-featuregates.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100755 .chloggen/add-featuregates.yaml diff --git a/.chloggen/add-featuregates.yaml b/.chloggen/add-featuregates.yaml new file mode 100755 index 0000000000..920d5664d2 --- /dev/null +++ b/.chloggen/add-featuregates.yaml @@ -0,0 +1,16 @@ +# 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. operator, target allocator, github action) +component: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add ability to use feature gates in the operator + +# One or more tracking issues related to the change +issues: [1619] + +# (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: From 78e980da6efcc45a7cd017e586790389ddace608 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Fri, 31 Mar 2023 09:46:31 -0600 Subject: [PATCH 3/3] Apply feedback --- main.go | 6 +++--- pkg/{flags/flags.go => featuregate/featuregate.go} | 3 ++- .../flags_test.go => featuregate/featuregate_test.go} | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) rename pkg/{flags/flags.go => featuregate/featuregate.go} (87%) rename pkg/{flags/flags_test.go => featuregate/featuregate_test.go} (99%) diff --git a/main.go b/main.go index 503894b69f..ea09f38071 100644 --- a/main.go +++ b/main.go @@ -26,7 +26,7 @@ import ( routev1 "github.com/openshift/api/route/v1" "github.com/spf13/pflag" - "go.opentelemetry.io/collector/featuregate" + colfeaturegate "go.opentelemetry.io/collector/featuregate" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sruntime "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -48,7 +48,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/webhookhandler" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" collectorupgrade "github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade" - "github.com/open-telemetry/opentelemetry-operator/pkg/flags" + "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" "github.com/open-telemetry/opentelemetry-operator/pkg/instrumentation" instrumentationupgrade "github.com/open-telemetry/opentelemetry-operator/pkg/instrumentation/upgrade" "github.com/open-telemetry/opentelemetry-operator/pkg/sidecar" @@ -76,7 +76,7 @@ func init() { func main() { // registers any flags that underlying libraries might use opts := zap.Options{} - flagset := flags.Flags(featuregate.GlobalRegistry()) + flagset := featuregate.Flags(colfeaturegate.GlobalRegistry()) opts.BindFlags(flag.CommandLine) pflag.CommandLine.AddGoFlagSet(flag.CommandLine) pflag.CommandLine.AddGoFlagSet(flagset) diff --git a/pkg/flags/flags.go b/pkg/featuregate/featuregate.go similarity index 87% rename from pkg/flags/flags.go rename to pkg/featuregate/featuregate.go index 6a5aecab5f..da877c882e 100644 --- a/pkg/flags/flags.go +++ b/pkg/featuregate/featuregate.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package flags +package featuregate import ( "flag" @@ -24,6 +24,7 @@ const ( featureGatesFlag = "feature-gates" ) +// Flags creates a new FlagSet that represents the available featuregate flags using the supplied featuregate registry. func Flags(reg *featuregate.Registry) *flag.FlagSet { flagSet := new(flag.FlagSet) flagSet.Var(featuregate.NewFlag(reg), featureGatesFlag, diff --git a/pkg/flags/flags_test.go b/pkg/featuregate/featuregate_test.go similarity index 99% rename from pkg/flags/flags_test.go rename to pkg/featuregate/featuregate_test.go index eef0462b45..1321bfc34f 100644 --- a/pkg/flags/flags_test.go +++ b/pkg/featuregate/featuregate_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package flags +package featuregate import ( "testing"