Merge branch 'main' into httpd_safe_conf

open-telemetry · Oct 26, 2024 · 1b6b970 · 1b6b970
2 parents eb8c97e + 2b36f0d
commit 1b6b970
Show file tree

Hide file tree

Showing 12 changed files with 97 additions and 43 deletions.
diff --git a/.chloggen/1814-java-configmap.yaml b/.chloggen/1814-java-configmap.yaml
@@ -0,0 +1,19 @@
+# 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. collector, target allocator, auto-instrumentation, opamp, github action)
+component: auto-instrumentation
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Support configuring Java auto-instrumentation when runtime configuration is provided from configmap or secret.
+
+# One or more tracking issues related to the change
+issues: [1814]
+
+# (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: |
+  This change allows users to configure JAVA_TOOL_OPTIONS in config map or secret.
+  The operator in this case set another JAVA_TOOL_OPTIONS that references the original value 
+  e.g. `JAVA_TOOL_OPTIONS=$(JAVA_TOOL_OPTIONS) -javaagent:/otel-auto-instrumentation-java/javaagent.jar`.
diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml
@@ -1,3 +1,2 @@
 resources:
 - manager.yaml
-
diff --git a/pkg/instrumentation/javaagent.go b/pkg/instrumentation/javaagent.go
@@ -24,23 +24,17 @@ import (
 
 const (
 	envJavaToolsOptions   = "JAVA_TOOL_OPTIONS"
-	javaAgent             = " -javaagent:/otel-auto-instrumentation-java/javaagent.jar"
+	javaAgent             = "-javaagent:/otel-auto-instrumentation-java/javaagent.jar"
 	javaInitContainerName = initContainerName + "-java"
 	javaVolumeName        = volumeName + "-java"
 	javaInstrMountPath    = "/otel-auto-instrumentation-java"
 )
 
-func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.Pod, error) {
+func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) corev1.Pod {
 	volume := instrVolume(javaSpec.VolumeClaimTemplate, javaVolumeName, javaSpec.VolumeSizeLimit)
-
 	// caller checks if there is at least one container.
 	container := &pod.Spec.Containers[index]
 
-	err := validateContainerEnv(container.Env, envJavaToolsOptions)
-	if err != nil {
-		return pod, err
-	}
-
 	// inject Java instrumentation spec env vars.
 	for _, env := range javaSpec.Env {
 		idx := getIndexOfEnv(container.Env, env.Name)
@@ -55,14 +49,14 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.
 	}
 
 	idx := getIndexOfEnv(container.Env, envJavaToolsOptions)
-	if idx == -1 {
-		container.Env = append(container.Env, corev1.EnvVar{
-			Name:  envJavaToolsOptions,
-			Value: javaJVMArgument,
-		})
-	} else {
-		container.Env[idx].Value = container.Env[idx].Value + javaJVMArgument
+	if idx != -1 {
+		// https://kubernetes.io/docs/tasks/inject-data-application/define-interdependent-environment-variables/
+		javaJVMArgument = fmt.Sprintf("$(%s) %s", envJavaToolsOptions, javaJVMArgument)
 	}
+	container.Env = append(container.Env, corev1.EnvVar{
+		Name:  envJavaToolsOptions,
+		Value: javaJVMArgument,
+	})
 
 	container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
 		Name:      volume.Name,
@@ -97,5 +91,5 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.
 		}
 
 	}
-	return pod, err
+	return pod
 }
diff --git a/pkg/instrumentation/javaagent_test.go b/pkg/instrumentation/javaagent_test.go
@@ -15,7 +15,6 @@
 package instrumentation
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -30,7 +29,6 @@ func TestInjectJavaagent(t *testing.T) {
 		v1alpha1.Java
 		pod      corev1.Pod
 		expected corev1.Pod
-		err      error
 	}{
 		{
 			name: "JAVA_TOOL_OPTIONS not defined",
@@ -83,7 +81,6 @@ func TestInjectJavaagent(t *testing.T) {
 					},
 				},
 			},
-			err: nil,
 		},
 		{
 			name: "add extensions to JAVA_TOOL_OPTIONS",
@@ -157,7 +154,6 @@ func TestInjectJavaagent(t *testing.T) {
 					},
 				},
 			},
-			err: nil,
 		},
 		{
 			name: "JAVA_TOOL_OPTIONS defined",
@@ -211,18 +207,21 @@ func TestInjectJavaagent(t *testing.T) {
 							Env: []corev1.EnvVar{
 								{
 									Name:  "JAVA_TOOL_OPTIONS",
-									Value: "-Dbaz=bar" + javaAgent,
+									Value: "-Dbaz=bar",
+								},
+								{
+									Name:  "JAVA_TOOL_OPTIONS",
+									Value: "$(JAVA_TOOL_OPTIONS) " + javaAgent,
 								},
 							},
 						},
 					},
 				},
 			},
-			err: nil,
 		},
 		{
 			name: "JAVA_TOOL_OPTIONS defined as ValueFrom",
-			Java: v1alpha1.Java{Image: "foo/bar:1"},
+			Java: v1alpha1.Java{Image: "foo/bar:1", Resources: testResourceRequirements},
 			pod: corev1.Pod{
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{
@@ -239,27 +238,57 @@ func TestInjectJavaagent(t *testing.T) {
 			},
 			expected: corev1.Pod{
 				Spec: corev1.PodSpec{
+					Volumes: []corev1.Volume{
+						{
+							Name: "opentelemetry-auto-instrumentation-java",
+							VolumeSource: corev1.VolumeSource{
+								EmptyDir: &corev1.EmptyDirVolumeSource{
+									SizeLimit: &defaultVolumeLimitSize,
+								},
+							},
+						},
+					},
+					InitContainers: []corev1.Container{
+						{
+							Name:    "opentelemetry-auto-instrumentation-java",
+							Image:   "foo/bar:1",
+							Command: []string{"cp", "/javaagent.jar", "/otel-auto-instrumentation-java/javaagent.jar"},
+							VolumeMounts: []corev1.VolumeMount{{
+								Name:      "opentelemetry-auto-instrumentation-java",
+								MountPath: "/otel-auto-instrumentation-java",
+							}},
+							Resources: testResourceRequirements,
+						},
+					},
 					Containers: []corev1.Container{
 						{
+							VolumeMounts: []corev1.VolumeMount{
+								{
+									Name:      "opentelemetry-auto-instrumentation-java",
+									MountPath: "/otel-auto-instrumentation-java",
+								},
+							},
 							Env: []corev1.EnvVar{
 								{
 									Name:      "JAVA_TOOL_OPTIONS",
 									ValueFrom: &corev1.EnvVarSource{},
 								},
+								{
+									Name:  "JAVA_TOOL_OPTIONS",
+									Value: "$(JAVA_TOOL_OPTIONS) " + javaAgent,
+								},
 							},
 						},
 					},
 				},
 			},
-			err: fmt.Errorf("the container defines env var value via ValueFrom, envVar: %s", envJavaToolsOptions),
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			pod, err := injectJavaagent(test.Java, test.pod, 0)
+			pod := injectJavaagent(test.Java, test.pod, 0)
 			assert.Equal(t, test.expected, pod)
-			assert.Equal(t, test.err, err)
 		})
 	}
 }
diff --git a/pkg/instrumentation/sdk.go b/pkg/instrumentation/sdk.go
@@ -59,7 +59,6 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations
 	}
 	if insts.Java.Instrumentation != nil {
 		otelinst := *insts.Java.Instrumentation
-		var err error
 		i.logger.V(1).Info("injecting Java instrumentation into pod", "otelinst-namespace", otelinst.Namespace, "otelinst-name", otelinst.Name)
 
 		if len(insts.Java.Containers) == 0 {
@@ -68,14 +67,10 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations
 
 		for _, container := range insts.Java.Containers {
 			index := getContainerIndex(container, pod)
-			pod, err = injectJavaagent(otelinst.Spec.Java, pod, index)
-			if err != nil {
-				i.logger.Info("Skipping javaagent injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name)
-			} else {
-				pod = i.injectCommonEnvVar(otelinst, pod, index)
-				pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index)
-				pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, javaInitContainerName)
-			}
+			pod = injectJavaagent(otelinst.Spec.Java, pod, index)
+			pod = i.injectCommonEnvVar(otelinst, pod, index)
+			pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index)
+			pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, javaInitContainerName)
 		}
 	}
 	if insts.NodeJS.Instrumentation != nil {

diff --git a/tests/e2e-instrumentation/instrumentation-java-multicontainer/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-multicontainer/01-assert.yaml
@@ -25,7 +25,7 @@ spec:
     - name: SPLUNK_PROFILER_ENABLED
       value: "false"
     - name: JAVA_TOOL_OPTIONS
-      value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar'
+      value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar'
     - name: OTEL_TRACES_EXPORTER
       value: otlp
     - name: OTEL_EXPORTER_OTLP_ENDPOINT
@@ -75,7 +75,7 @@ spec:
     - name: SPLUNK_PROFILER_ENABLED
       value: "false"
     - name: JAVA_TOOL_OPTIONS
-      value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar'
+      value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar'
     - name: OTEL_TRACES_EXPORTER
       value: otlp
     - name: OTEL_EXPORTER_OTLP_ENDPOINT

diff --git a/tests/e2e-instrumentation/instrumentation-java-multicontainer/02-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-multicontainer/02-assert.yaml
@@ -36,7 +36,7 @@ spec:
     - name: SPLUNK_PROFILER_ENABLED
       value: "false"
     - name: JAVA_TOOL_OPTIONS
-      value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar'
+      value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar'
     - name: OTEL_TRACES_EXPORTER
       value: otlp
     - name: OTEL_EXPORTER_OTLP_ENDPOINT

diff --git a/tests/e2e-instrumentation/instrumentation-java-other-ns/03-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-other-ns/03-assert.yaml
@@ -24,7 +24,7 @@ spec:
     - name: SPLUNK_PROFILER_ENABLED
       value: "false"
     - name: JAVA_TOOL_OPTIONS
-      value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar'
+      value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar'
     - name: OTEL_TRACES_EXPORTER
       value: otlp
     - name: OTEL_EXPORTER_OTLP_ENDPOINT

diff --git a/tests/e2e-instrumentation/instrumentation-java-tls/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java-tls/01-assert.yaml
@@ -17,7 +17,7 @@ spec:
         fieldRef:
           fieldPath: status.podIP
     - name: JAVA_TOOL_OPTIONS
-      value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar'
+      value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar'
     - name: OTEL_SERVICE_NAME
       value: my-java
     - name: OTEL_EXPORTER_OTLP_ENDPOINT

diff --git a/tests/e2e-instrumentation/instrumentation-java/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-java/01-assert.yaml
@@ -17,14 +17,19 @@ spec:
       valueFrom:
         fieldRef:
           fieldPath: status.podIP
+    - name: JAVA_TOOL_OPTIONS
+      valueFrom:
+        configMapKeyRef:
+          name: config-java
+          key: system-properties
     - name: OTEL_JAVAAGENT_DEBUG
       value: "true"
     - name: OTEL_INSTRUMENTATION_JDBC_ENABLED
       value: "false"
     - name: SPLUNK_PROFILER_ENABLED
       value: "false"
     - name: JAVA_TOOL_OPTIONS
-      value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar'
+      value: '$(JAVA_TOOL_OPTIONS) -javaagent:/otel-auto-instrumentation-java/javaagent.jar'
     - name: OTEL_TRACES_EXPORTER
       value: otlp
     - name: OTEL_EXPORTER_OTLP_ENDPOINT

diff --git a/tests/e2e-instrumentation/instrumentation-java/01-install-app.yaml b/tests/e2e-instrumentation/instrumentation-java/01-install-app.yaml
@@ -1,3 +1,10 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config-java
+data:
+  system-properties: "-Xmx256m -Xms64m"
+---
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -22,6 +29,12 @@ spec:
       containers:
       - name: myapp
         image: ghcr.io/open-telemetry/opentelemetry-operator/e2e-test-app-java:main
+        env:
+        - name: JAVA_TOOL_OPTIONS
+          valueFrom:
+            configMapKeyRef:
+              name: config-java
+              key: system-properties
         securityContext:
           allowPrivilegeEscalation: false
           capabilities:

diff --git a/tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer/01-assert.yaml b/tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer/01-assert.yaml
@@ -89,7 +89,7 @@ spec:
     - name: OTEL_SERVICE_NAME
       value: javaapp
     - name: JAVA_TOOL_OPTIONS
-      value: ' -javaagent:/otel-auto-instrumentation-java/javaagent.jar'
+      value: '-javaagent:/otel-auto-instrumentation-java/javaagent.jar'
     - name: OTEL_TRACES_SAMPLER
       value: parentbased_traceidratio
     - name: OTEL_TRACES_SAMPLER_ARG