From 5b1d32c2071ec87b3d7a6ff62c79073724ebf6e5 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Tue, 25 Apr 2023 05:15:44 -0600 Subject: [PATCH] [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 | 16 ++++ README.md | 15 +++ main.go | 4 +- pkg/featuregate/featuregate.go | 11 ++- pkg/instrumentation/podmutator.go | 13 ++- pkg/instrumentation/podmutator_test.go | 107 ++++++++++++++++++++-- pkg/instrumentation/upgrade/upgrade.go | 16 +++- 7 files changed, 166 insertions(+), 16 deletions(-) create mode 100755 .chloggen/add-featuregate-for-dotnet.yaml diff --git a/.chloggen/add-featuregate-for-dotnet.yaml b/.chloggen/add-featuregate-for-dotnet.yaml new file mode 100755 index 0000000000..a2a2bdb57c --- /dev/null +++ b/.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: diff --git a/README.md b/README.md index 04430352c3..397a58ab7f 100644 --- a/README.md +++ b/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: diff --git a/main.go b/main.go index ea09f38071..7245737a25 100644 --- a/main.go +++ b/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) })) diff --git a/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index da877c882e..9324faa9b1 100644 --- a/pkg/featuregate/featuregate.go +++ b/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 } diff --git a/pkg/instrumentation/podmutator.go b/pkg/instrumentation/podmutator.go index c18c661326..f3d3184437 100644 --- a/pkg/instrumentation/podmutator.go +++ b/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,7 +52,7 @@ 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, @@ -57,6 +60,7 @@ func NewMutator(logger logr.Logger, client client.Client) *instPodMutator { 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 diff --git a/pkg/instrumentation/podmutator_test.go b/pkg/instrumentation/podmutator_test.go index 64a6e1fb0b..f3816488aa 100644 --- a/pkg/instrumentation/podmutator_test.go +++ b/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() { diff --git a/pkg/instrumentation/upgrade/upgrade.go b/pkg/instrumentation/upgrade/upgrade.go index b42d9cb908..a0a041f974 100644 --- a/pkg/instrumentation/upgrade/upgrade.go +++ b/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