chore: update dotnet instr. to command-line (open-telemetry#2690)

* chore: update `dotnet` instr. to command-line

* fix: `.NET` pkg instrumentation upgrade

* fix: remove `debug` logs

* fix: pod mutator tests for `.NET` instrumentations

* fix: using capital N for `DotNet`

Author: luizhlelis

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

internal/config/main.go

@@ -45,6 +45,7 @@ type Config struct {
 	createRBACPermissions               bool
 	enableMultiInstrumentation          bool
 	enableApacheHttpdInstrumentation    bool
+	enableDotNetInstrumentation         bool
 	autoInstrumentationDotNetImage      string
 	autoInstrumentationGoImage          string
 	autoInstrumentationApacheHttpdImage string
@@ -79,6 +80,7 @@ func New(opts ...Option) Config {
 		createRBACPermissions:               o.createRBACPermissions,
 		enableMultiInstrumentation:          o.enableMultiInstrumentation,
 		enableApacheHttpdInstrumentation:    o.enableApacheHttpdInstrumentation,
+		enableDotNetInstrumentation:         o.enableDotNetInstrumentation,
 		targetAllocatorImage:                o.targetAllocatorImage,
 		operatorOpAMPBridgeImage:            o.operatorOpAMPBridgeImage,
 		targetAllocatorConfigMapEntry:       o.targetAllocatorConfigMapEntry,
@@ -118,10 +120,16 @@ func (c *Config) EnableMultiInstrumentation() bool {
 	return c.enableMultiInstrumentation
 }
 
+// EnableApacheHttpdAutoInstrumentation is true when the operator supports ApacheHttpd auto instrumentation.
 func (c *Config) EnableApacheHttpdAutoInstrumentation() bool {
 	return c.enableApacheHttpdInstrumentation
 }
 
+// EnableDotNetAutoInstrumentation is true when the operator supports dotnet auto instrumentation.
+func (c *Config) EnableDotNetAutoInstrumentation() bool {
+	return c.enableDotNetInstrumentation
+}
+
 // CollectorConfigMapEntry represents the configuration file name for the collector. Immutable.
 func (c *Config) CollectorConfigMapEntry() string {
 	return c.collectorConfigMapEntry

internal/config/options.go

@@ -44,6 +44,7 @@ type options struct {
 	createRBACPermissions               bool
 	enableMultiInstrumentation          bool
 	enableApacheHttpdInstrumentation    bool
+	enableDotNetInstrumentation         bool
 	targetAllocatorConfigMapEntry       string
 	operatorOpAMPBridgeConfigMapEntry   string
 	targetAllocatorImage                string
@@ -92,6 +93,11 @@ func WithEnableApacheHttpdInstrumentation(s bool) Option {
 		o.enableApacheHttpdInstrumentation = s
 	}
 }
+func WithEnableDotNetInstrumentation(s bool) Option {
+	return func(o *options) {
+		o.enableDotNetInstrumentation = s
+	}
+}
 func WithTargetAllocatorConfigMapEntry(s string) Option {
 	return func(o *options) {
 		o.targetAllocatorConfigMapEntry = s

main.go

@@ -109,6 +109,7 @@ func main() {
 		createRBACPermissions            bool
 		enableMultiInstrumentation       bool
 		enableApacheHttpdInstrumentation bool
+		enableDotNetInstrumentation      bool
 		collectorImage                   string
 		targetAllocatorImage             string
 		operatorOpAMPBridgeImage         string
@@ -132,7 +133,8 @@ func main() {
 			"Enabling this will ensure there is only one active controller manager.")
 	pflag.BoolVar(&createRBACPermissions, "create-rbac-permissions", false, "Automatically create RBAC permissions needed by the processors")
 	pflag.BoolVar(&enableMultiInstrumentation, "enable-multi-instrumentation", false, "Controls whether the operator supports multi instrumentation")
-	pflag.BoolVar(&enableApacheHttpdInstrumentation, constants.FlagApacheHttpd, true, "controls whether the operator supports Apache HTTPD auto-instrumentation")
+	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")
 	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.")
@@ -172,6 +174,7 @@ func main() {
 		"labels-filter", labelsFilter,
 		"enable-multi-instrumentation", enableMultiInstrumentation,
 		"enable-apache-httpd-instrumentation", enableApacheHttpdInstrumentation,
+		"enable-dotnet-instrumentation", enableDotNetInstrumentation,
 	)
 
 	restConfig := ctrl.GetConfigOrDie()
@@ -190,6 +193,7 @@ func main() {
 		config.WithCreateRBACPermissions(createRBACPermissions),
 		config.WithEnableMultiInstrumentation(enableMultiInstrumentation),
 		config.WithEnableApacheHttpdInstrumentation(enableApacheHttpdInstrumentation),
+		config.WithEnableDotNetInstrumentation(enableDotNetInstrumentation),
 		config.WithTargetAllocatorImage(targetAllocatorImage),
 		config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage),
 		config.WithAutoInstrumentationJavaImage(autoInstrumentationJava),

pkg/constants/env.go

@@ -36,4 +36,5 @@ const (
 	EnvNodeName = "OTEL_RESOURCE_ATTRIBUTES_NODE_NAME"
 
 	FlagApacheHttpd = "enable-apache-httpd-instrumentation"
+	FlagDotNet      = "enable-dotnet-instrumentation"
 )

pkg/featuregate/featuregate.go

@@ -25,12 +25,6 @@ const (
 )
 
 var (
-	EnableDotnetAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
-		"operator.autoinstrumentation.dotnet",
-		featuregate.StageBeta,
-		featuregate.WithRegisterDescription("controls whether the operator supports .NET auto-instrumentation"),
-		featuregate.WithRegisterFromVersion("v0.76.1"),
-	)
 	EnablePythonAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
 		"operator.autoinstrumentation.python",
 		featuregate.StageBeta,

pkg/instrumentation/podmutator.go

@@ -265,7 +265,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.EnableDotnetAutoInstrumentationSupport.IsEnabled() || inst == nil {
+	if pm.config.EnableDotNetAutoInstrumentation() || inst == nil {
 		insts.DotNet.Instrumentation = inst
 		insts.DotNet.AdditionalAnnotations = map[string]string{annotationDotNetRuntime: annotationValue(ns.ObjectMeta, pod.ObjectMeta, annotationDotNetRuntime)}
 	} else {

pkg/instrumentation/podmutator_test.go

@@ -1811,6 +1811,7 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
+			config: config.New(config.WithEnableDotNetInstrumentation(true)),
 		},
 		{
 			name: "dotnet injection, by namespace annotations",
@@ -1990,6 +1991,7 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
+			config: config.New(config.WithEnableDotNetInstrumentation(true)),
 		},
 		{
 			name: "dotnet injection multiple containers, true",
@@ -2265,6 +2267,7 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
+			config: config.New(config.WithEnableDotNetInstrumentation(true)),
 		},
 		{
 			name: "dotnet injection feature gate disabled",
@@ -2343,13 +2346,7 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
-			setFeatureGates: func(t *testing.T) {
-				originalVal := featuregate.EnableDotnetAutoInstrumentationSupport.IsEnabled()
-				require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableDotnetAutoInstrumentationSupport.ID(), false))
-				t.Cleanup(func() {
-					require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableDotnetAutoInstrumentationSupport.ID(), originalVal))
-				})
-			},
+			config: config.New(config.WithEnableDotNetInstrumentation(false)),
 		},
 		{
 			name: "go injection, true",
@@ -3845,7 +3842,10 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
-			config: config.New(config.WithEnableMultiInstrumentation(true)),
+			config: config.New(
+				config.WithEnableMultiInstrumentation(true),
+				config.WithEnableDotNetInstrumentation(true),
+			),
 		},
 		{
 			name: "multi instrumentation for multiple containers feature gate enabled, container-names not used",
@@ -4503,7 +4503,10 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
-			config: config.New(config.WithEnableMultiInstrumentation(true)),
+			config: config.New(
+				config.WithEnableMultiInstrumentation(true),
+				config.WithEnableDotNetInstrumentation(true),
+			),
 		},
 		{
 			name: "multi instrumentation for multiple containers feature gate disabled, multiple instrumentation annotations set",
@@ -4980,7 +4983,10 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
-			config: config.New(config.WithEnableMultiInstrumentation(true)),
+			config: config.New(
+				config.WithEnableMultiInstrumentation(true),
+				config.WithEnableDotNetInstrumentation(true),
+			),
 		},
 		{
 			name: "multi instrumentation feature gate disabled, instrumentation feature gate disabled and annotation set, multiple specific containers set",
@@ -5080,14 +5086,10 @@ func TestMutatePod(t *testing.T) {
 					},
 				},
 			},
-			config: config.New(config.WithEnableMultiInstrumentation(true)),
-			setFeatureGates: func(t *testing.T) {
-				originalValDotNetInstr := featuregate.EnableDotnetAutoInstrumentationSupport.IsEnabled()
-				require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableDotnetAutoInstrumentationSupport.ID(), false))
-				t.Cleanup(func() {
-					require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableDotnetAutoInstrumentationSupport.ID(), originalValDotNetInstr))
-				})
-			},
+			config: config.New(
+				config.WithEnableMultiInstrumentation(true),
+				config.WithEnableDotNetInstrumentation(false),
+			),
 		},
 	}
 

pkg/instrumentation/upgrade/upgrade.go

@@ -35,7 +35,6 @@ var (
 		constants.AnnotationDefaultAutoInstrumentationJava:   featuregate.EnableJavaAutoInstrumentationSupport,
 		constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport,
 		constants.AnnotationDefaultAutoInstrumentationPython: featuregate.EnablePythonAutoInstrumentationSupport,
-		constants.AnnotationDefaultAutoInstrumentationDotNet: featuregate.EnableDotnetAutoInstrumentationSupport,
 		constants.AnnotationDefaultAutoInstrumentationGo:     featuregate.EnableGoAutoInstrumentationSupport,
 		constants.AnnotationDefaultAutoInstrumentationNginx:  featuregate.EnableNginxAutoInstrumentationSupport,
 	}
@@ -62,7 +61,8 @@ type InstrumentationUpgrade struct {
 
 func NewInstrumentationUpgrade(client client.Client, logger logr.Logger, recorder record.EventRecorder, cfg config.Config) *InstrumentationUpgrade {
 	defaultAnnotationToConfig := map[string]autoInstConfig{
-		constants.AnnotationDefaultAutoInstrumentationApacheHttpd: autoInstConfig{constants.FlagApacheHttpd, cfg.EnableApacheHttpdAutoInstrumentation()},
+		constants.AnnotationDefaultAutoInstrumentationApacheHttpd: {constants.FlagApacheHttpd, cfg.EnableApacheHttpdAutoInstrumentation()},
+		constants.AnnotationDefaultAutoInstrumentationDotNet:      {constants.FlagDotNet, cfg.EnableDotNetAutoInstrumentation()},
 	}
 
 	return &InstrumentationUpgrade{
@@ -127,6 +127,11 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
 						upgraded.Spec.ApacheHttpd.Image = u.DefaultAutoInstApacheHttpd
 						upgraded.Annotations[annotation] = u.DefaultAutoInstApacheHttpd
 					}
+				case constants.AnnotationDefaultAutoInstrumentationDotNet:
+					if inst.Spec.DotNet.Image == autoInst {
+						upgraded.Spec.DotNet.Image = u.DefaultAutoInstDotNet
+						upgraded.Annotations[annotation] = u.DefaultAutoInstDotNet
+					}
 				}
 			} else {
 				u.Logger.Error(nil, "autoinstrumentation not enabled for this language", "flag", config.id)
@@ -155,11 +160,6 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
 						upgraded.Spec.Python.Image = u.DefaultAutoInstPython
 						upgraded.Annotations[annotation] = u.DefaultAutoInstPython
 					}
-				case constants.AnnotationDefaultAutoInstrumentationDotNet:
-					if inst.Spec.DotNet.Image == autoInst {
-						upgraded.Spec.DotNet.Image = u.DefaultAutoInstDotNet
-						upgraded.Annotations[annotation] = u.DefaultAutoInstDotNet
-					}
 				case constants.AnnotationDefaultAutoInstrumentationGo:
 					if inst.Spec.Go.Image == autoInst {
 						upgraded.Spec.Go.Image = u.DefaultAutoInstGo

pkg/instrumentation/upgrade/upgrade_test.go

@@ -79,6 +79,7 @@ func TestUpgrade(t *testing.T) {
 			config.WithAutoInstrumentationApacheHttpdImage("apache-httpd:1"),
 			config.WithAutoInstrumentationNginxImage("nginx:1"),
 			config.WithEnableApacheHttpdInstrumentation(true),
+			config.WithEnableDotNetInstrumentation(true),
 		),
 	).Default(context.Background(), inst)
 	assert.Nil(t, err)
@@ -101,6 +102,7 @@ func TestUpgrade(t *testing.T) {
 		config.WithAutoInstrumentationApacheHttpdImage("apache-httpd:2"),
 		config.WithAutoInstrumentationNginxImage("nginx:2"),
 		config.WithEnableApacheHttpdInstrumentation(true),
+		config.WithEnableDotNetInstrumentation(true),
 	)
 	up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg)