Support configuring java runtime from configmap or secret (env.valueF…

…rom) Signed-off-by: Pavol Loffay <[email protected]>
open-telemetry · Oct 22, 2024 · ce7f1f9 · ce7f1f9
1 parent 70dff69
commit ce7f1f9
Show file tree

Hide file tree

Showing 12 changed files with 97 additions and 26 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,8 @@
 resources:
 - manager.yaml
-
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+images:
+- name: controller
+  newName: docker.io/pavolloffay/opentelemetry-operator
+  newTag: dev-70dff694-1729607411
diff --git a/pkg/instrumentation/javaagent.go b/pkg/instrumentation/javaagent.go
@@ -24,7 +24,7 @@ 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"
@@ -34,11 +34,6 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.
 	// 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)
@@ -53,14 +48,13 @@ 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 {
+		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:      javaVolumeName,
@@ -102,5 +96,5 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.
 		}
 
 	}
-	return pod, err
+	return pod, nil
 }
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"
@@ -211,7 +210,11 @@ 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,
 								},
 							},
 						},
@@ -222,7 +225,7 @@ func TestInjectJavaagent(t *testing.T) {
 		},
 		{
 			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,19 +242,50 @@ 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),
 		},
 	}
 

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-instrumentation/instrumentation-java/chainsaw-test.yaml b/tests/e2e-instrumentation/instrumentation-java/chainsaw-test.yaml
@@ -5,6 +5,7 @@ metadata:
   creationTimestamp: null
   name: instrumentation-java
 spec:
+  skipDelete: false
   steps:
   - name: step-00
     try:

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