From 7aaf176263561b0586d02456d65f54985ee00aae Mon Sep 17 00:00:00 2001 From: Avadhut Pisal Date: Wed, 19 Oct 2022 20:18:51 +0530 Subject: [PATCH] Skip .NET auto-instrumentation if OTEL_DOTNET_AUTO_HOME env var is already set (#1177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * skips env var injection and sdk configurations if agent injection is skipped * mutate container at the last of SDK injection step * validate first and then mutate the container with env variables * fixes go lint issues * incorporates review comments * fixes go lint issue * removes return statement in case of failed instrumentation * skips auto-instrumentation if OTEL_DOTNET_AUTO_HOME is already set * use errors.New() to get the error object use errors.New() to get the error object instead of fmt.Errorf() Co-authored-by: Robert Pająk * use errors.New() to get the error object use errors.New() to get the error object instead of fmt.Errorf() Co-authored-by: Robert Pająk * replaces fmt.Errorf with error.New Co-authored-by: Robert Pająk --- pkg/instrumentation/dotnet.go | 13 +++++++ pkg/instrumentation/dotnet_test.go | 60 +++++++++++++++++++++++++++--- pkg/instrumentation/python_test.go | 1 + 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/pkg/instrumentation/dotnet.go b/pkg/instrumentation/dotnet.go index 47263bb91a..c86614a47a 100644 --- a/pkg/instrumentation/dotnet.go +++ b/pkg/instrumentation/dotnet.go @@ -15,6 +15,7 @@ package instrumentation import ( + "errors" "fmt" corev1 "k8s.io/api/core/v1" @@ -49,6 +50,18 @@ func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int) (cor return pod, err } + // check if OTEL_DOTNET_AUTO_HOME env var is already set in the container + // if it is already set, then we assume that .NET Auto-instrumentation is already configured for this container + if getIndexOfEnv(container.Env, envDotNetOTelAutoHome) > -1 { + return pod, errors.New("OTEL_DOTNET_AUTO_HOME environment variable is already set in the container") + } + + // check if OTEL_DOTNET_AUTO_HOME env var is already set in the .NET instrumentatiom spec + // if it is already set, then we assume that .NET Auto-instrumentation is already configured for this container + if getIndexOfEnv(dotNetSpec.Env, envDotNetOTelAutoHome) > -1 { + return pod, errors.New("OTEL_DOTNET_AUTO_HOME environment variable is already set in the .NET instrumentation spec") + } + // inject .NET instrumentation spec env vars. for _, env := range dotNetSpec.Env { idx := getIndexOfEnv(container.Env, env.Name) diff --git a/pkg/instrumentation/dotnet_test.go b/pkg/instrumentation/dotnet_test.go index aa0978c985..eab7fb85c2 100644 --- a/pkg/instrumentation/dotnet_test.go +++ b/pkg/instrumentation/dotnet_test.go @@ -108,7 +108,7 @@ func TestInjectDotNetSDK(t *testing.T) { err: nil, }, { - name: "CORECLR_ENABLE_PROFILING, CORECLR_PROFILER, CORECLR_PROFILER_PATH, DOTNET_STARTUP_HOOKS, DOTNET_ADDITIONAL_DEPS, DOTNET_SHARED_STORE, OTEL_DOTNET_AUTO_HOME defined", + name: "CORECLR_ENABLE_PROFILING, CORECLR_PROFILER, CORECLR_PROFILER_PATH, DOTNET_STARTUP_HOOKS, DOTNET_ADDITIONAL_DEPS, DOTNET_SHARED_STORE defined", DotNet: v1alpha1.DotNet{Image: "foo/bar:1"}, pod: corev1.Pod{ Spec: corev1.PodSpec{ @@ -139,10 +139,6 @@ func TestInjectDotNetSDK(t *testing.T) { Name: envDotNetSharedStore, Value: "/foo:/bar", }, - { - Name: envDotNetOTelAutoHome, - Value: "/foo:/bar", - }, }, }, }, @@ -204,7 +200,7 @@ func TestInjectDotNetSDK(t *testing.T) { }, { Name: envDotNetOTelAutoHome, - Value: "/foo:/bar", + Value: dotNetOTelAutoHomePath, }, }, }, @@ -312,6 +308,58 @@ func TestInjectDotNetSDK(t *testing.T) { }, err: fmt.Errorf("the container defines env var value via ValueFrom, envVar: %s", envDotNetSharedStore), }, + { + name: "OTEL_DOTNET_AUTO_HOME already set in the container", + DotNet: v1alpha1.DotNet{Image: "foo/bar:1"}, + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: envDotNetOTelAutoHome, + Value: "/otel-dotnet-auto", + }, + }, + }, + }, + }, + }, + expected: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: envDotNetOTelAutoHome, + Value: "/otel-dotnet-auto", + }, + }, + }, + }, + }, + }, + err: fmt.Errorf("OTEL_DOTNET_AUTO_HOME environment variable is already set in the container"), + }, + { + name: "OTEL_DOTNET_AUTO_HOME already set in the .NET instrumentation spec", + DotNet: v1alpha1.DotNet{Image: "foo/bar:1", Env: []corev1.EnvVar{{Name: envDotNetOTelAutoHome, Value: dotNetOTelAutoHomePath}}}, + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {}, + }, + }, + }, + expected: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {}, + }, + }, + }, + err: fmt.Errorf("OTEL_DOTNET_AUTO_HOME environment variable is already set in the .NET instrumentation spec"), + }, } for _, test := range tests { diff --git a/pkg/instrumentation/python_test.go b/pkg/instrumentation/python_test.go index 35e8e4d34e..910a35ba61 100644 --- a/pkg/instrumentation/python_test.go +++ b/pkg/instrumentation/python_test.go @@ -221,6 +221,7 @@ func TestInjectPythonSDK(t *testing.T) { }, }, }, + err: nil, }, { name: "OTEL_METRICS_EXPORTER defined",