Skip .NET auto-instrumentation if OTEL_DOTNET_AUTO_HOME env var is al…

…ready set (open-telemetry#1177) * 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 <[email protected]> * 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 <[email protected]> * replaces fmt.Errorf with error.New Co-authored-by: Robert Pająk <[email protected]>
ItielOlenick · Oct 19, 2022 · 1ba3217 · 1ba3217
1 parent 19c4a11
commit 1ba3217
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 6 deletions.
diff --git 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
@@ -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
@@ -221,6 +221,7 @@ func TestInjectPythonSDK(t *testing.T) {
 					},
 				},
 			},
+			err: nil,
 		},
 		{
 			name:   "OTEL_METRICS_EXPORTER defined",
-Original file line number
+Diff line change
@@ Expand Up / @@ -221,6 +221,7 @@ func TestInjectPythonSDK(t *testing.T) { @@
     					},
     				},
     			},
+    			err: nil,
     		},
     		{
     			name:   "OTEL_METRICS_EXPORTER defined",
@@ Expand Down @@