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.

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

Conversation

pavolloffay
Copy link
Member

@pavolloffay pavolloffay commented Oct 22, 2024

Description:

Supersedes #3373

This PR is much simpler it does not require fetching configmaps/secrets.

If the JAVA_TOOL_OPTIONS is set (e.g. from configmap the operator sets another JAVA_TOOL_OPTIONS=$(JAVA_TOOL_OPTIONS) -javaagent

Link to tracking Issue(s):

Testing:

Documentation:

config/manager/kustomization.yaml Outdated Show resolved Hide resolved
- 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.

Copy link
Contributor

@swiatekm swiatekm Oct 23, 2024

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

…rom)

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
@pavolloffay
Copy link
Member Author

PR updated

@pavolloffay pavolloffay merged commit 2b36f0d into open-telemetry:main Oct 25, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting JAVA_TOOL_OPTIONS via configmap is not possible (replaced,ignored)
3 participants