Change nginx instrumentation into command line (#2777)

* feat: change nginx instrumentation to cli flag

* set nginx instrumentation to default false

* update config/manager

* enable nginx in config/manager

* update chloggen

.chloggen/autoinstrumentation-nginx-cli-flag.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. 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: change nginx instrumentation feature gate operator.autoinstrumentation.nginx into command line flag --enable-nginx-instrumentation
+
+# One or more tracking issues related to the change
+issues: [2582, 2676]
+
+# (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:

bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml

@@ -99,7 +99,7 @@ metadata:
     categories: Logging & Tracing,Monitoring
     certified: "false"
     containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
-    createdAt: "2024-03-11T13:32:19Z"
+    createdAt: "2024-03-25T08:38:05Z"
     description: Provides the OpenTelemetry components, including the Collector
     operators.operatorframework.io/builder: operator-sdk-v1.29.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
@@ -487,7 +487,8 @@ spec:
                 - --enable-leader-election
                 - --zap-log-level=info
                 - --zap-time-encoding=rfc3339nano
-                - --feature-gates=+operator.autoinstrumentation.go,+operator.autoinstrumentation.nginx
+                - --feature-gates=+operator.autoinstrumentation.go
+                - --enable-nginx-instrumentation=true
                 image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.96.0
                 livenessProbe:
                   httpGet:

config/manager/manager.yaml

@@ -32,7 +32,8 @@ spec:
         - "--enable-leader-election"
         - "--zap-log-level=info"
         - "--zap-time-encoding=rfc3339nano"
-        - "--feature-gates=+operator.autoinstrumentation.go,+operator.autoinstrumentation.nginx"
+        - "--feature-gates=+operator.autoinstrumentation.go"
+        - "--enable-nginx-instrumentation=true"
         image: controller
         name: manager
         livenessProbe:

internal/config/main.go

@@ -46,6 +46,7 @@ type Config struct {
 	enableMultiInstrumentation          bool
 	enableApacheHttpdInstrumentation    bool
 	enableDotNetInstrumentation         bool
+	enableNginxInstrumentation          bool
 	enablePythonInstrumentation         bool
 	autoInstrumentationDotNetImage      string
 	autoInstrumentationGoImage          string
@@ -83,6 +84,7 @@ func New(opts ...Option) Config {
 		enableMultiInstrumentation:          o.enableMultiInstrumentation,
 		enableApacheHttpdInstrumentation:    o.enableApacheHttpdInstrumentation,
 		enableDotNetInstrumentation:         o.enableDotNetInstrumentation,
+		enableNginxInstrumentation:          o.enableNginxInstrumentation,
 		enablePythonInstrumentation:         o.enablePythonInstrumentation,
 		targetAllocatorImage:                o.targetAllocatorImage,
 		operatorOpAMPBridgeImage:            o.operatorOpAMPBridgeImage,
@@ -134,6 +136,11 @@ func (c *Config) EnableDotNetAutoInstrumentation() bool {
 	return c.enableDotNetInstrumentation
 }
 
+// EnableNginxAutoInstrumentation is true when the operator supports nginx auto instrumentation.
+func (c *Config) EnableNginxAutoInstrumentation() bool {
+	return c.enableNginxInstrumentation
+}
+
 // EnablePythonAutoInstrumentation is true when the operator supports dotnet auto instrumentation.
 func (c *Config) EnablePythonAutoInstrumentation() bool {
 	return c.enablePythonInstrumentation

internal/config/options.go

@@ -45,6 +45,7 @@ type options struct {
 	enableMultiInstrumentation          bool
 	enableApacheHttpdInstrumentation    bool
 	enableDotNetInstrumentation         bool
+	enableNginxInstrumentation          bool
 	enablePythonInstrumentation         bool
 	targetAllocatorConfigMapEntry       string
 	operatorOpAMPBridgeConfigMapEntry   string
@@ -100,6 +101,11 @@ func WithEnableDotNetInstrumentation(s bool) Option {
 		o.enableDotNetInstrumentation = s
 	}
 }
+func WithEnableNginxInstrumentation(s bool) Option {
+	return func(o *options) {
+		o.enableNginxInstrumentation = s
+	}
+}
 func WithEnablePythonInstrumentation(s bool) Option {
 	return func(o *options) {
 		o.enablePythonInstrumentation = s

main.go

@@ -113,6 +113,7 @@ func main() {
 		enableApacheHttpdInstrumentation bool
 		enableDotNetInstrumentation      bool
 		enablePythonInstrumentation      bool
+		enableNginxInstrumentation       bool
 		collectorImage                   string
 		targetAllocatorImage             string
 		operatorOpAMPBridgeImage         string
@@ -140,6 +141,7 @@ func main() {
 	pflag.BoolVar(&enableApacheHttpdInstrumentation, constants.FlagApacheHttpd, true, "Controls whether the operator supports Apache HTTPD auto-instrumentation")
 	pflag.BoolVar(&enableDotNetInstrumentation, constants.FlagDotNet, true, "Controls whether the operator supports dotnet 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")
 	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.")
 	stringFlagOrEnv(&operatorOpAMPBridgeImage, "operator-opamp-bridge-image", "RELATED_IMAGE_OPERATOR_OPAMP_BRIDGE", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/operator-opamp-bridge:%s", v.OperatorOpAMPBridge), "The default OpenTelemetry Operator OpAMP Bridge image. This image is used when no image is specified in the CustomResource.")
@@ -183,6 +185,7 @@ func main() {
 		"enable-apache-httpd-instrumentation", enableApacheHttpdInstrumentation,
 		"enable-dotnet-instrumentation", enableDotNetInstrumentation,
 		"enable-python-instrumentation", enablePythonInstrumentation,
+		"enable-nginx-instrumentation", enableNginxInstrumentation,
 	)
 
 	restConfig := ctrl.GetConfigOrDie()
@@ -202,6 +205,7 @@ func main() {
 		config.WithEnableMultiInstrumentation(enableMultiInstrumentation),
 		config.WithEnableApacheHttpdInstrumentation(enableApacheHttpdInstrumentation),
 		config.WithEnableDotNetInstrumentation(enableDotNetInstrumentation),
+		config.WithEnableNginxInstrumentation(enableNginxInstrumentation),
 		config.WithEnablePythonInstrumentation(enablePythonInstrumentation),
 		config.WithTargetAllocatorImage(targetAllocatorImage),
 		config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage),

pkg/constants/env.go

@@ -38,4 +38,5 @@ const (
 	FlagApacheHttpd = "enable-apache-httpd-instrumentation"
 	FlagDotNet      = "enable-dotnet-instrumentation"
 	FlagPython      = "enable-python-instrumentation"
+	FlagNginx       = "enable-nginx-instrumentation"
 )

pkg/featuregate/featuregate.go

@@ -43,12 +43,6 @@ var (
 		featuregate.WithRegisterDescription("controls whether the operator supports Golang auto-instrumentation"),
 		featuregate.WithRegisterFromVersion("v0.77.0"),
 	)
-	EnableNginxAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
-		"operator.autoinstrumentation.nginx",
-		featuregate.StageAlpha,
-		featuregate.WithRegisterDescription("controls whether the operator supports Nginx auto-instrumentation"),
-		featuregate.WithRegisterFromVersion("v0.86.0"),
-	)
 	// EnableTargetAllocatorRewrite is the feature gate that controls whether the collector's configuration should
 	// automatically be rewritten when the target allocator is enabled.
 	EnableTargetAllocatorRewrite = featuregate.GlobalRegistry().MustRegister(

pkg/instrumentation/podmutator.go

@@ -302,7 +302,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.EnableNginxAutoInstrumentationSupport.IsEnabled() || inst == nil {
+	if pm.config.EnableNginxAutoInstrumentation() || inst == nil {
 		insts.Nginx.Instrumentation = inst
 	} else {
 		logger.Error(nil, "support for Nginx auto instrumentation is not enabled")

pkg/instrumentation/podmutator_test.go

@@ -2972,13 +2972,7 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
-			setFeatureGates: func(t *testing.T) {
-				originalVal := featuregate.EnableNginxAutoInstrumentationSupport.IsEnabled()
-				require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), true))
-				t.Cleanup(func() {
-					require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), originalVal))
-				})
-			},
+			config: config.New(config.WithEnableNginxInstrumentation(true)),
 		},
 		{
 			name: "nginx injection feature gate disabled",
@@ -3059,13 +3053,7 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
-			setFeatureGates: func(t *testing.T) {
-				originalVal := featuregate.EnableNginxAutoInstrumentationSupport.IsEnabled()
-				require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), false))
-				t.Cleanup(func() {
-					require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), originalVal))
-				})
-			},
+			config: config.New(config.WithEnableDotNetInstrumentation(false)),
 		},
 
 		{

pkg/instrumentation/upgrade/upgrade.go

@@ -35,7 +35,6 @@ var (
 		constants.AnnotationDefaultAutoInstrumentationJava:   featuregate.EnableJavaAutoInstrumentationSupport,
 		constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport,
 		constants.AnnotationDefaultAutoInstrumentationGo:     featuregate.EnableGoAutoInstrumentationSupport,
-		constants.AnnotationDefaultAutoInstrumentationNginx:  featuregate.EnableNginxAutoInstrumentationSupport,
 	}
 )
 
@@ -62,6 +61,7 @@ func NewInstrumentationUpgrade(client client.Client, logger logr.Logger, recorde
 	defaultAnnotationToConfig := map[string]autoInstConfig{
 		constants.AnnotationDefaultAutoInstrumentationApacheHttpd: {constants.FlagApacheHttpd, cfg.EnableApacheHttpdAutoInstrumentation()},
 		constants.AnnotationDefaultAutoInstrumentationDotNet:      {constants.FlagDotNet, cfg.EnableDotNetAutoInstrumentation()},
+		constants.AnnotationDefaultAutoInstrumentationNginx:       {constants.FlagNginx, cfg.EnableNginxAutoInstrumentation()},
 		constants.AnnotationDefaultAutoInstrumentationPython:      {constants.FlagPython, cfg.EnablePythonAutoInstrumentation()},
 	}
 
@@ -132,6 +132,11 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
 						upgraded.Spec.DotNet.Image = u.DefaultAutoInstDotNet
 						upgraded.Annotations[annotation] = u.DefaultAutoInstDotNet
 					}
+				case constants.AnnotationDefaultAutoInstrumentationNginx:
+					if inst.Spec.Nginx.Image == autoInst {
+						upgraded.Spec.Nginx.Image = u.DefaultAutoInstNginx
+						upgraded.Annotations[annotation] = u.DefaultAutoInstNginx
+					}
 				case constants.AnnotationDefaultAutoInstrumentationPython:
 					if inst.Spec.Python.Image == autoInst {
 						upgraded.Spec.Python.Image = u.DefaultAutoInstPython
@@ -170,11 +175,6 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
 						upgraded.Spec.Go.Image = u.DefaultAutoInstGo
 						upgraded.Annotations[annotation] = u.DefaultAutoInstGo
 					}
-				case constants.AnnotationDefaultAutoInstrumentationNginx:
-					if inst.Spec.Nginx.Image == autoInst {
-						upgraded.Spec.Nginx.Image = u.DefaultAutoInstNginx
-						upgraded.Annotations[annotation] = u.DefaultAutoInstNginx
-					}
 				}
 			} else {
 				u.Logger.Error(nil, "autoinstrumentation not enabled for this language", "flag", gate.ID())

pkg/instrumentation/upgrade/upgrade_test.go

@@ -42,12 +42,6 @@ func TestUpgrade(t *testing.T) {
 		require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableGoAutoInstrumentationSupport.ID(), originalVal))
 	})
 
-	originalVal = featuregate.EnableNginxAutoInstrumentationSupport.IsEnabled()
-	require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), true))
-	t.Cleanup(func() {
-		require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), originalVal))
-	})
-
 	nsName := strings.ToLower(t.Name())
 	err := k8sClient.Create(context.Background(), &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -80,6 +74,7 @@ func TestUpgrade(t *testing.T) {
 			config.WithAutoInstrumentationNginxImage("nginx:1"),
 			config.WithEnableApacheHttpdInstrumentation(true),
 			config.WithEnableDotNetInstrumentation(true),
+			config.WithEnableNginxInstrumentation(true),
 			config.WithEnablePythonInstrumentation(true),
 		),
 	).Default(context.Background(), inst)
@@ -104,6 +99,7 @@ func TestUpgrade(t *testing.T) {
 		config.WithAutoInstrumentationNginxImage("nginx:2"),
 		config.WithEnableApacheHttpdInstrumentation(true),
 		config.WithEnableDotNetInstrumentation(true),
+		config.WithEnableNginxInstrumentation(true),
 		config.WithEnablePythonInstrumentation(true),
 	)
 	up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg)

tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml

@@ -8317,7 +8317,7 @@ spec:
             - --enable-leader-election
             - --zap-log-level=info
             - --zap-time-encoding=rfc3339nano
-            - --feature-gates=+operator.autoinstrumentation.go,+operator.autoinstrumentation.nginx
+            - --feature-gates=+operator.autoinstrumentation.go
           image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.86.0
           livenessProbe:
             httpGet: