diff --git a/.chloggen/change-instrumentation-feature-gates.yaml b/.chloggen/change-instrumentation-feature-gates.yaml new file mode 100755 index 0000000000..50e93eace9 --- /dev/null +++ b/.chloggen/change-instrumentation-feature-gates.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'breaking' + +# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) +component: 'operator' + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: remove featuregate `operator.autoinstrumentation.nodejs`. Use command line flag `--enable-nodejs-instrumentation` instead + +# One or more tracking issues related to the change +issues: [2674] + +# (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: diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index b09eafa381..d8bf4d629a 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -35,9 +35,9 @@ jobs: - e2e-metadata-filters include: - group: e2e-instrumentation - setup: "add-operator-arg OPERATOR_ARG=--enable-go-instrumentation prepare-e2e" + setup: "add-instrumentation-params prepare-e2e" - group: e2e-multi-instrumentation - setup: "add-operator-arg OPERATOR_ARG=--enable-multi-instrumentation prepare-e2e" + setup: "add-multi-instrumentation-params prepare-e2e" - group: e2e-metadata-filters setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=*filter.out --labels=*filter.out' prepare-e2e" diff --git a/Makefile b/Makefile index e55cb145be..d169d3c38b 100644 --- a/Makefile +++ b/Makefile @@ -145,6 +145,14 @@ add-operator-arg: manifests kustomize add-image-targetallocator: @$(MAKE) add-operator-arg OPERATOR_ARG=--target-allocator-image=$(TARGETALLOCATOR_IMG) +.PHONY: add-instrumentation-params +add-instrumentation-params: + @$(MAKE) add-operator-arg OPERATOR_ARG=--enable-go-instrumentation=true + +.PHONY: add-multi-instrumentation-params +add-multi-instrumentation-params: + @$(MAKE) add-operator-arg OPERATOR_ARG=--enable-multi-instrumentation + .PHONY: add-image-opampbridge add-image-opampbridge: @$(MAKE) add-operator-arg OPERATOR_ARG=--operator-opamp-bridge-image=$(OPERATOROPAMPBRIDGE_IMG) diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index c8116daa19..eaea7882c1 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -498,7 +498,6 @@ spec: - --enable-leader-election - --zap-log-level=info - --zap-time-encoding=rfc3339nano - - --feature-gates=+operator.autoinstrumentation.go - --enable-nginx-instrumentation=true image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.98.0 livenessProbe: diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 76eb71ac95..a329ad65ab 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -32,7 +32,6 @@ spec: - "--enable-leader-election" - "--zap-log-level=info" - "--zap-time-encoding=rfc3339nano" - - "--feature-gates=+operator.autoinstrumentation.go" - "--enable-nginx-instrumentation=true" image: controller name: manager diff --git a/internal/config/main.go b/internal/config/main.go index 27ff489a3b..4430f3d618 100644 --- a/internal/config/main.go +++ b/internal/config/main.go @@ -50,6 +50,7 @@ type Config struct { enableGoInstrumentation bool enableNginxInstrumentation bool enablePythonInstrumentation bool + enableNodeJSInstrumentation bool enableJavaInstrumentation bool autoInstrumentationDotNetImage string autoInstrumentationGoImage string @@ -94,6 +95,7 @@ func New(opts ...Option) Config { enableGoInstrumentation: o.enableGoInstrumentation, enableNginxInstrumentation: o.enableNginxInstrumentation, enablePythonInstrumentation: o.enablePythonInstrumentation, + enableNodeJSInstrumentation: o.enableNodeJSInstrumentation, enableJavaInstrumentation: o.enableJavaInstrumentation, targetAllocatorImage: o.targetAllocatorImage, operatorOpAMPBridgeImage: o.operatorOpAMPBridgeImage, @@ -172,6 +174,11 @@ func (c *Config) EnablePythonAutoInstrumentation() bool { return c.enablePythonInstrumentation } +// EnableNodeJSAutoInstrumentation is true when the operator supports dotnet auto instrumentation. +func (c *Config) EnableNodeJSAutoInstrumentation() bool { + return c.enableNodeJSInstrumentation +} + // CollectorConfigMapEntry represents the configuration file name for the collector. Immutable. func (c *Config) CollectorConfigMapEntry() string { return c.collectorConfigMapEntry diff --git a/internal/config/options.go b/internal/config/options.go index f6a3a160de..40ce1dade9 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -49,6 +49,7 @@ type options struct { enableGoInstrumentation bool enableNginxInstrumentation bool enablePythonInstrumentation bool + enableNodeJSInstrumentation bool enableJavaInstrumentation bool targetAllocatorConfigMapEntry string operatorOpAMPBridgeConfigMapEntry string @@ -125,6 +126,11 @@ func WithEnablePythonInstrumentation(s bool) Option { o.enablePythonInstrumentation = s } } +func WithEnableNodeJSInstrumentation(s bool) Option { + return func(o *options) { + o.enableNodeJSInstrumentation = s + } +} func WithTargetAllocatorConfigMapEntry(s string) Option { return func(o *options) { o.targetAllocatorConfigMapEntry = s diff --git a/main.go b/main.go index 83bc6ef1a2..fb12b5eccf 100644 --- a/main.go +++ b/main.go @@ -115,6 +115,7 @@ func main() { enableGoInstrumentation bool enablePythonInstrumentation bool enableNginxInstrumentation bool + enableNodeJSInstrumentation bool enableJavaInstrumentation bool collectorImage string targetAllocatorImage string @@ -145,6 +146,7 @@ func main() { pflag.BoolVar(&enableGoInstrumentation, constants.FlagGo, false, "Controls whether the operator supports Go auto-instrumentation") pflag.BoolVar(&enablePythonInstrumentation, constants.FlagPython, true, "Controls whether the operator supports python auto-instrumentation") pflag.BoolVar(&enableNginxInstrumentation, constants.FlagNginx, false, "Controls whether the operator supports nginx auto-instrumentation") + pflag.BoolVar(&enableNodeJSInstrumentation, constants.FlagNodeJS, true, "Controls whether the operator supports nodejs auto-instrumentation") pflag.BoolVar(&enableJavaInstrumentation, constants.FlagJava, true, "Controls whether the operator supports java auto-instrumentation") stringFlagOrEnv(&collectorImage, "collector-image", "RELATED_IMAGE_COLLECTOR", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-collector-releases/opentelemetry-collector:%s", v.OpenTelemetryCollector), "The default OpenTelemetry collector image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&targetAllocatorImage, "target-allocator-image", "RELATED_IMAGE_TARGET_ALLOCATOR", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/target-allocator:%s", v.TargetAllocator), "The default OpenTelemetry target allocator image. This image is used when no image is specified in the CustomResource.") @@ -191,6 +193,7 @@ func main() { "enable-go-instrumentation", enableGoInstrumentation, "enable-python-instrumentation", enablePythonInstrumentation, "enable-nginx-instrumentation", enableNginxInstrumentation, + "enable-nodejs-instrumentation", enableNodeJSInstrumentation, "enable-java-instrumentation", enableJavaInstrumentation, ) @@ -214,6 +217,7 @@ func main() { config.WithEnableGoInstrumentation(enableGoInstrumentation), config.WithEnableNginxInstrumentation(enableNginxInstrumentation), config.WithEnablePythonInstrumentation(enablePythonInstrumentation), + config.WithEnableNodeJSInstrumentation(enableNodeJSInstrumentation), config.WithEnableJavaInstrumentation(enableJavaInstrumentation), config.WithTargetAllocatorImage(targetAllocatorImage), config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage), diff --git a/pkg/constants/env.go b/pkg/constants/env.go index fb15aa6ca4..8ebd1bb5d9 100644 --- a/pkg/constants/env.go +++ b/pkg/constants/env.go @@ -42,5 +42,6 @@ const ( FlagGo = "enable-go-instrumentation" FlagPython = "enable-python-instrumentation" FlagNginx = "enable-nginx-instrumentation" + FlagNodeJS = "enable-nodejs-instrumentation" FlagJava = "enable-java-instrumentation" ) diff --git a/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index 2e2798cc85..2d633c5276 100644 --- a/pkg/featuregate/featuregate.go +++ b/pkg/featuregate/featuregate.go @@ -25,25 +25,6 @@ const ( ) var ( - EnableNodeJSAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister( - "operator.autoinstrumentation.nodejs", - featuregate.StageBeta, - featuregate.WithRegisterDescription("controls whether the operator supports NodeJS auto-instrumentation"), - featuregate.WithRegisterFromVersion("v0.76.1"), - ) - EnableNginxAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister( - "operator.autoinstrumentation.nginx", - featuregate.StageAlpha, - featuregate.WithRegisterDescription("controls whether the operator supports Nginx auto-instrumentation"), - featuregate.WithRegisterFromVersion("v0.86.0"), - ) - EnableGoAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister( - "operator.autoinstrumentation.go", - featuregate.StageAlpha, - featuregate.WithRegisterDescription("controls whether the operator supports Golang auto-instrumentation"), - featuregate.WithRegisterFromVersion("v0.77.0"), - ) - // PrometheusOperatorIsAvailable is the feature gate that enables features associated to the Prometheus Operator. PrometheusOperatorIsAvailable = featuregate.GlobalRegistry().MustRegister( "operator.observability.prometheus", diff --git a/pkg/instrumentation/podmutator.go b/pkg/instrumentation/podmutator.go index d82564cde3..37d1be99b7 100644 --- a/pkg/instrumentation/podmutator.go +++ b/pkg/instrumentation/podmutator.go @@ -29,7 +29,6 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/webhook/podmutation" - "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) var ( @@ -241,7 +240,7 @@ func (pm *instPodMutator) Mutate(ctx context.Context, ns corev1.Namespace, pod c logger.Error(err, "failed to select an OpenTelemetry Instrumentation instance for this pod") return pod, err } - if featuregate.EnableNodeJSAutoInstrumentationSupport.IsEnabled() || inst == nil { + if pm.config.EnableNodeJSAutoInstrumentation() || inst == nil { insts.NodeJS.Instrumentation = inst } else { logger.Error(nil, "support for NodeJS auto instrumentation is not enabled") diff --git a/pkg/instrumentation/podmutator_test.go b/pkg/instrumentation/podmutator_test.go index ffc69463a8..0166ee0451 100644 --- a/pkg/instrumentation/podmutator_test.go +++ b/pkg/instrumentation/podmutator_test.go @@ -22,14 +22,12 @@ import ( "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - colfeaturegate "go.opentelemetry.io/collector/featuregate" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) func TestMutatePod(t *testing.T) { @@ -815,6 +813,7 @@ func TestMutatePod(t *testing.T) { }, }, }, + config: config.New(config.WithEnableNodeJSInstrumentation(true)), }, { name: "nodejs injection multiple containers, true", @@ -1086,6 +1085,7 @@ func TestMutatePod(t *testing.T) { }, }, }, + config: config.New(config.WithEnableNodeJSInstrumentation(true)), }, { name: "nodejs injection feature gate disabled", @@ -1168,13 +1168,6 @@ func TestMutatePod(t *testing.T) { }, }, }, - setFeatureGates: func(t *testing.T) { - originalVal := featuregate.EnableNodeJSAutoInstrumentationSupport.IsEnabled() - require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNodeJSAutoInstrumentationSupport.ID(), false)) - t.Cleanup(func() { - require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNodeJSAutoInstrumentationSupport.ID(), originalVal)) - }) - }, }, { name: "python injection, true", @@ -4201,6 +4194,7 @@ func TestMutatePod(t *testing.T) { config.WithEnableMultiInstrumentation(true), config.WithEnablePythonInstrumentation(true), config.WithEnableDotNetInstrumentation(true), + config.WithEnableNodeJSInstrumentation(true), ), }, { @@ -4991,6 +4985,7 @@ func TestMutatePod(t *testing.T) { config.WithEnableMultiInstrumentation(true), config.WithEnableDotNetInstrumentation(true), config.WithEnablePythonInstrumentation(true), + config.WithEnableNodeJSInstrumentation(true), ), }, { @@ -5145,7 +5140,7 @@ func TestMutatePod(t *testing.T) { }, }, }, - config: config.New(config.WithEnableMultiInstrumentation(false)), + config: config.New(config.WithEnableMultiInstrumentation(false), config.WithEnableJavaInstrumentation(false)), }, { name: "multi instrumentation feature gate enabled, multiple instrumentation annotations set, no containers", @@ -5289,7 +5284,7 @@ func TestMutatePod(t *testing.T) { }, }, }, - config: config.New(config.WithEnableMultiInstrumentation(true)), + config: config.New(config.WithEnableMultiInstrumentation(true), config.WithEnableJavaInstrumentation(false)), }, { name: "multi instrumentation feature gate enabled, single instrumentation annotation set, no containers", @@ -5590,6 +5585,7 @@ func TestMutatePod(t *testing.T) { config: config.New( config.WithEnableMultiInstrumentation(true), config.WithEnableDotNetInstrumentation(false), + config.WithEnableNodeJSInstrumentation(false), ), }, } diff --git a/pkg/instrumentation/upgrade/upgrade.go b/pkg/instrumentation/upgrade/upgrade.go index aeedf119e6..948a8e7288 100644 --- a/pkg/instrumentation/upgrade/upgrade.go +++ b/pkg/instrumentation/upgrade/upgrade.go @@ -20,20 +20,12 @@ import ( "reflect" "github.com/go-logr/logr" - featuregate2 "go.opentelemetry.io/collector/featuregate" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/pkg/constants" - "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" -) - -var ( - defaultAnnotationToGate = map[string]*featuregate2.Gate{ - constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport, - } ) type autoInstConfig struct { @@ -62,6 +54,7 @@ func NewInstrumentationUpgrade(client client.Client, logger logr.Logger, recorde constants.AnnotationDefaultAutoInstrumentationGo: {constants.FlagGo, cfg.EnableGoAutoInstrumentation()}, constants.AnnotationDefaultAutoInstrumentationNginx: {constants.FlagNginx, cfg.EnableNginxAutoInstrumentation()}, constants.AnnotationDefaultAutoInstrumentationPython: {constants.FlagPython, cfg.EnablePythonAutoInstrumentation()}, + constants.AnnotationDefaultAutoInstrumentationNodeJS: {constants.FlagNodeJS, cfg.EnableNodeJSAutoInstrumentation()}, constants.AnnotationDefaultAutoInstrumentationJava: {constants.FlagJava, cfg.EnableJavaAutoInstrumentation()}, } @@ -147,43 +140,23 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru upgraded.Spec.Python.Image = u.DefaultAutoInstPython upgraded.Annotations[annotation] = u.DefaultAutoInstPython } + case constants.AnnotationDefaultAutoInstrumentationNodeJS: + if inst.Spec.NodeJS.Image == autoInst { + upgraded.Spec.NodeJS.Image = u.DefaultAutoInstNodeJS + upgraded.Annotations[annotation] = u.DefaultAutoInstNodeJS + } case constants.AnnotationDefaultAutoInstrumentationJava: if inst.Spec.Java.Image == autoInst { upgraded.Spec.Java.Image = u.DefaultAutoInstJava upgraded.Annotations[annotation] = u.DefaultAutoInstJava } } + } else { u.Logger.V(4).Info("autoinstrumentation not enabled for this language", "flag", instCfg.id) } } } - for annotation, gate := range defaultAnnotationToGate { - autoInst := upgraded.Annotations[annotation] - if autoInst != "" { - if gate.IsEnabled() { - switch annotation { - case constants.AnnotationDefaultAutoInstrumentationNodeJS: - if inst.Spec.NodeJS.Image == autoInst { - upgraded.Spec.NodeJS.Image = u.DefaultAutoInstNodeJS - upgraded.Annotations[annotation] = u.DefaultAutoInstNodeJS - } - case constants.AnnotationDefaultAutoInstrumentationPython: - if inst.Spec.Python.Image == autoInst { - upgraded.Spec.Python.Image = u.DefaultAutoInstPython - upgraded.Annotations[annotation] = u.DefaultAutoInstPython - } - case constants.AnnotationDefaultAutoInstrumentationGo: - if inst.Spec.Go.Image == autoInst { - upgraded.Spec.Go.Image = u.DefaultAutoInstGo - upgraded.Annotations[annotation] = u.DefaultAutoInstGo - } - } - } else { - u.Logger.V(4).Info("autoinstrumentation not enabled for this language", "flag", gate.ID()) - } - } - } return upgraded } diff --git a/pkg/instrumentation/upgrade/upgrade_test.go b/pkg/instrumentation/upgrade/upgrade_test.go index bc52a4d3a4..f7a401a201 100644 --- a/pkg/instrumentation/upgrade/upgrade_test.go +++ b/pkg/instrumentation/upgrade/upgrade_test.go @@ -69,6 +69,7 @@ func TestUpgrade(t *testing.T) { config.WithEnableGoInstrumentation(true), config.WithEnableNginxInstrumentation(true), config.WithEnablePythonInstrumentation(true), + config.WithEnableNodeJSInstrumentation(true), config.WithEnableJavaInstrumentation(true), ), ).Default(context.Background(), inst) @@ -96,6 +97,7 @@ func TestUpgrade(t *testing.T) { config.WithEnableGoInstrumentation(true), config.WithEnableNginxInstrumentation(true), config.WithEnablePythonInstrumentation(true), + config.WithEnableNodeJSInstrumentation(true), config.WithEnableJavaInstrumentation(true), ) up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg) diff --git a/tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml b/tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml index 12afe3893b..cc8a9cac64 100644 --- a/tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml +++ b/tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml @@ -8317,7 +8317,6 @@ spec: - --enable-leader-election - --zap-log-level=info - --zap-time-encoding=rfc3339nano - - --feature-gates=+operator.autoinstrumentation.go image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.86.0 livenessProbe: httpGet: