From 1b5b986965b85defed71c0e670aaa37f1cd3913c Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 22 Oct 2024 18:37:50 +0200 Subject: [PATCH] Support configuring java runtime from configmap or secret (env.valueFrom) Signed-off-by: Pavol Loffay --- .chloggen/1814-java-configmap.yaml | 19 +++++++ config/manager/kustomization.yaml | 1 - pkg/instrumentation/javaagent.go | 26 ++++------ pkg/instrumentation/javaagent_test.go | 49 +++++++++++++++---- pkg/instrumentation/sdk.go | 13 ++--- .../01-assert.yaml | 4 +- .../02-assert.yaml | 2 +- .../03-assert.yaml | 2 +- .../instrumentation-java-tls/01-assert.yaml | 2 +- .../instrumentation-java/01-assert.yaml | 7 ++- .../instrumentation-java/01-install-app.yaml | 13 +++++ .../01-assert.yaml | 2 +- 12 files changed, 97 insertions(+), 43 deletions(-) create mode 100755 .chloggen/1814-java-configmap.yaml diff --git a/.chloggen/1814-java-configmap.yaml b/.chloggen/1814-java-configmap.yaml new file mode 100755 index 0000000000..dbdddd5786 --- /dev/null +++ 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 index 372a75ae43..5c5f0b84cb 100644 --- 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 index ef91d296d8..1dafcd9cd7 100644 --- 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 index ea8d81305d..f52beb20a3 100644 --- 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 index 0033f70566..e47c358fb0 100644 --- 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 index a4dca94976..09b2a5687b 100644 --- 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 index 03c002d2d8..5bfa1ceff3 100644 --- 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 index 0b6ea1db84..ef36aa4c46 100644 --- 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 index 7ddecadb47..6cb4d2d206 100644 --- 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 index cd8a8a37fe..f1af6b5218 100644 --- a/tests/e2e-instrumentation/instrumentation-java/01-assert.yaml +++ b/tests/e2e-instrumentation/instrumentation-java/01-assert.yaml @@ -17,6 +17,11 @@ 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 @@ -24,7 +29,7 @@ spec: - 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 index 4655644b5b..c3204ec290 100644 --- 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 index 3ba921ada1..250223271b 100644 --- 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