Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support configuring java runtime from configmap or secret (env.valueFrom) #3379

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Support configuring java runtime from configmap or secret (env.valueFrom) #3379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .chloggen/1814-java-configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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`.
1 change: 0 additions & 1 deletion config/manager/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
resources:
- manager.yaml

26 changes: 10 additions & 16 deletions pkg/instrumentation/javaagent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -97,5 +91,5 @@ func injectJavaagent(javaSpec v1alpha1.Java, pod corev1.Pod, index int) (corev1.
}

}
return pod, err
return pod
}
49 changes: 39 additions & 10 deletions pkg/instrumentation/javaagent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package instrumentation

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -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",
Expand Down Expand Up @@ -83,7 +81,6 @@ func TestInjectJavaagent(t *testing.T) {
},
},
},
err: nil,
},
{
name: "add extensions to JAVA_TOOL_OPTIONS",
Expand Down Expand Up @@ -157,7 +154,6 @@ func TestInjectJavaagent(t *testing.T) {
},
},
},
err: nil,
},
{
name: "JAVA_TOOL_OPTIONS defined",
Expand Down Expand Up @@ -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{
Expand All @@ -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)
})
}
}
13 changes: 4 additions & 9 deletions pkg/instrumentation/sdk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e-instrumentation/instrumentation-java-multicontainer/01-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e-instrumentation/instrumentation-java-multicontainer/02-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e-instrumentation/instrumentation-java-other-ns/03-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e-instrumentation/instrumentation-java-tls/01-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion tests/e2e-instrumentation/instrumentation-java/01-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we able to verify that this works? I didn't think that additive variables with the same name work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(verify in the test)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it works, we are even using it for getting the pod name and namespace to resource attributes env var

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://kubernetes.io/docs/tasks/inject-data-application/define-interdependent-environment-variables/

Copy link
Contributor

@swiatekm swiatekm Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Jacob was asking if defining the same variable twice works as one would intuitively expect here.

I tested it, and it does work, but we should really leave a comment in the code explaining this. It seems very weird at first glance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the link into the source code.

As I mention we already use it for other env vars

- name: OTEL_TRACES_EXPORTER
value: otlp
- name: OTEL_EXPORTER_OTLP_ENDPOINT
Expand Down
13 changes: 13 additions & 0 deletions tests/e2e-instrumentation/instrumentation-java/01-install-app.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: config-java
data:
system-properties: "-Xmx256m -Xms64m"
---
apiVersion: apps/v1
kind: Deployment
metadata:
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer/01-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading