[pkg/instrumentation] Add feature gate for dotnet instrumentation (#1629

)

* Add feature gate for dotnet instrumentation

* changelog

* Apply feedback

* Move gate into featuregate pkg

* Add featuregates to info log

* Add README entry for the new gate

* Write event if feature gate is disabled

* Add back error log

.chloggen/add-featuregate-for-dotnet.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: pkg/instrumentation
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add dotnet instrumentation capability behind a feature gate which is enabled by default.
+
+# One or more tracking issues related to the change
+issues: [1629]
+
+# (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:

README.md

@@ -334,6 +334,21 @@ You can configure the OpenTelemetry SDK for applications which can't currently b
 instrumentation.opentelemetry.io/inject-sdk: "true"
 ```
 
+#### Controlling Instrumentation Capabilities
+
+The operator allows specifying, via the feature gates,  which languages the Instrumentation resource may instrument.
+These feature gates must be passed to the operator via the `--feature-gates` flag.
+The flag allows for a comma-delimited list of feature gate identifiers.
+Prefix a gate with '-' to disable support for the corresponding language.
+Prefixing a gate with '+' or no prefix will enable support for the corresponding language.
+If a language is enabled by default its gate only needs to be supplied when disabling the gate.
+
+| Language | Gate                                  | Default Value   |
+|----------|---------------------------------------|-----------------|
+| .NET     | `operator.autoinstrumentation.dotnet` | enabled         |
+
+Language not specified in the table are always supported and cannot be disabled.
+
 ### Target Allocator
 
 The OpenTelemetry Operator comes with an optional component, the Target Allocator (TA). When creating an OpenTelemetryCollector Custom Resource (CR) and setting the TA as enabled, the Operator will create a new deployment and service to serve specific `http_sd_config` directives for each Collector pod as part of that CR. It will also change the Prometheus receiver configuration in the CR, so that it uses the [http_sd_config](https://prometheus.io/docs/prometheus/latest/http_sd/) from the TA. The following example shows how to get started with the Target Allocator:

main.go

@@ -130,6 +130,7 @@ func main() {
 		"auto-instrumentation-nodejs", autoInstrumentationNodeJS,
 		"auto-instrumentation-python", autoInstrumentationPython,
 		"auto-instrumentation-dotnet", autoInstrumentationDotNet,
+		"feature-gates", flagset.Lookup(featuregate.FeatureGatesFlag).Value.String(),
 		"build-date", v.BuildDate,
 		"go-version", v.Go,
 		"go-arch", runtime.GOARCH,
@@ -242,7 +243,7 @@ func main() {
 			Handler: webhookhandler.NewWebhookHandler(cfg, ctrl.Log.WithName("pod-webhook"), mgr.GetClient(),
 				[]webhookhandler.PodMutator{
 					sidecar.NewMutator(logger, cfg, mgr.GetClient()),
-					instrumentation.NewMutator(logger, mgr.GetClient()),
+					instrumentation.NewMutator(logger, mgr.GetClient(), mgr.GetEventRecorderFor("opentelemetry-operator")),
 				}),
 		})
 	} else {
@@ -297,6 +298,7 @@ func addDependencies(_ context.Context, mgr ctrl.Manager, cfg config.Config, v v
 			DefaultAutoInstPython: cfg.AutoInstrumentationPythonImage(),
 			DefaultAutoInstDotNet: cfg.AutoInstrumentationDotNetImage(),
 			Client:                mgr.GetClient(),
+			Recorder:              mgr.GetEventRecorderFor("opentelemetry-operator"),
 		}
 		return u.ManagedInstances(c)
 	}))

pkg/featuregate/featuregate.go

@@ -21,13 +21,20 @@ import (
 )
 
 const (
-	featureGatesFlag = "feature-gates"
+	FeatureGatesFlag = "feature-gates"
+)
+
+var (
+	EnableDotnetAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
+		"operator.autoinstrumentation.dotnet",
+		featuregate.StageBeta,
+		featuregate.WithRegisterDescription("controls whether the operator supports .NET auto-instrumentation"))
 )
 
 // 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,
+	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
 }

pkg/instrumentation/podmutator.go

@@ -22,10 +22,12 @@ import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"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/webhookhandler"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
 )
 
 var (
@@ -37,6 +39,7 @@ type instPodMutator struct {
 	Client      client.Client
 	sdkInjector *sdkInjector
 	Logger      logr.Logger
+	Recorder    record.EventRecorder
 }
 
 type languageInstrumentations struct {
@@ -49,14 +52,15 @@ type languageInstrumentations struct {
 
 var _ webhookhandler.PodMutator = (*instPodMutator)(nil)
 
-func NewMutator(logger logr.Logger, client client.Client) *instPodMutator {
+func NewMutator(logger logr.Logger, client client.Client, recorder record.EventRecorder) *instPodMutator {
 	return &instPodMutator{
 		Logger: logger,
 		Client: client,
 		sdkInjector: &sdkInjector{
 			logger: logger,
 			client: client,
 		},
+		Recorder: recorder,
 	}
 }
 
@@ -102,7 +106,12 @@ 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
 	}
-	insts.DotNet = inst
+	if featuregate.EnableDotnetAutoInstrumentationSupport.IsEnabled() {
+		insts.DotNet = inst
+	} else {
+		logger.Error(nil, "support for .NET auto instrumentation is not enabled")
+		pm.Recorder.Event(pod.DeepCopy(), "Warning", "InstrumentationRequestRejected", "support for .NET auto instrumentation is not enabled")
+	}
 
 	if inst, err = pm.getInstrumentationInstance(ctx, ns, pod, annotationInjectSdk); err != nil {
 		// we still allow the pod to be created, but we log a message to the operator's logs

pkg/instrumentation/podmutator_test.go

@@ -22,23 +22,27 @@ 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/pkg/featuregate"
 )
 
 func TestMutatePod(t *testing.T) {
-	mutator := NewMutator(logr.Discard(), k8sClient)
+	mutator := NewMutator(logr.Discard(), k8sClient, record.NewFakeRecorder(1))
 	require.NotNil(t, mutator)
 
 	tests := []struct {
-		name     string
-		err      string
-		pod      corev1.Pod
-		expected corev1.Pod
-		inst     v1alpha1.Instrumentation
-		ns       corev1.Namespace
+		name            string
+		err             string
+		pod             corev1.Pod
+		expected        corev1.Pod
+		inst            v1alpha1.Instrumentation
+		ns              corev1.Namespace
+		setFeatureGates func(t *testing.T)
 	}{
 		{
 			name: "javaagent injection, true",
@@ -746,6 +750,91 @@ func TestMutatePod(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "dotnet injection feature gate disabled",
+			ns: corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "dotnet-disabled",
+				},
+			},
+			inst: v1alpha1.Instrumentation{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "example-inst",
+					Namespace: "dotnet-disabled",
+				},
+				Spec: v1alpha1.InstrumentationSpec{
+					DotNet: v1alpha1.DotNet{
+						Image: "otel/dotnet:1",
+						Env: []corev1.EnvVar{
+							{
+								Name:  "OTEL_LOG_LEVEL",
+								Value: "debug",
+							},
+							{
+								Name:  "OTEL_EXPORTER_OTLP_ENDPOINT",
+								Value: "http://localhost:4317",
+							},
+						},
+					},
+					Exporter: v1alpha1.Exporter{
+						Endpoint: "http://collector:12345",
+					},
+					Env: []corev1.EnvVar{
+						{
+							Name:  "OTEL_EXPORTER_OTLP_TIMEOUT",
+							Value: "20",
+						},
+						{
+							Name:  "OTEL_TRACES_SAMPLER",
+							Value: "parentbased_traceidratio",
+						},
+						{
+							Name:  "OTEL_TRACES_SAMPLER_ARG",
+							Value: "0.85",
+						},
+						{
+							Name:  "SPLUNK_TRACE_RESPONSE_HEADER_ENABLED",
+							Value: "true",
+						},
+					},
+				},
+			},
+			pod: corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationInjectDotNet: "true",
+					},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name: "app",
+						},
+					},
+				},
+			},
+			expected: corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						annotationInjectDotNet: "true",
+					},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name: "app",
+						},
+					},
+				},
+			},
+			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))
+				})
+			},
+		},
 		{
 			name: "missing annotation",
 			ns: corev1.Namespace{
@@ -877,6 +966,10 @@ func TestMutatePod(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			if test.setFeatureGates != nil {
+				test.setFeatureGates(t)
+			}
+
 			err := k8sClient.Create(context.Background(), &test.ns)
 			require.NoError(t, err)
 			defer func() {

pkg/instrumentation/upgrade/upgrade.go

@@ -20,14 +20,17 @@ import (
 	"reflect"
 
 	"github.com/go-logr/logr"
+	"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/pkg/featuregate"
 )
 
 type InstrumentationUpgrade struct {
 	Client                client.Client
 	Logger                logr.Logger
+	Recorder              record.EventRecorder
 	DefaultAutoInstJava   string
 	DefaultAutoInstNodeJS string
 	DefaultAutoInstPython string
@@ -95,10 +98,15 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
 	}
 	autoInstDotnet := inst.Annotations[v1alpha1.AnnotationDefaultAutoInstrumentationDotNet]
 	if autoInstDotnet != "" {
-		// upgrade the image only if the image matches the annotation
-		if inst.Spec.DotNet.Image == autoInstDotnet {
-			inst.Spec.DotNet.Image = u.DefaultAutoInstDotNet
-			inst.Annotations[v1alpha1.AnnotationDefaultAutoInstrumentationDotNet] = u.DefaultAutoInstDotNet
+		if featuregate.EnableDotnetAutoInstrumentationSupport.IsEnabled() {
+			// upgrade the image only if the image matches the annotation
+			if inst.Spec.DotNet.Image == autoInstDotnet {
+				inst.Spec.DotNet.Image = u.DefaultAutoInstDotNet
+				inst.Annotations[v1alpha1.AnnotationDefaultAutoInstrumentationDotNet] = u.DefaultAutoInstDotNet
+			}
+		} else {
+			u.Logger.Error(nil, "support for .NET auto instrumentation is not enabled")
+			u.Recorder.Event(inst.DeepCopy(), "Warning", "InstrumentationUpgradeRejected", "support for .NET auto instrumentation is not enabled")
 		}
 	}
 	return inst