open-telemetry / opentelemetry-operator

Kubernetes Operator for OpenTelemetry Collector
Apache License 2.0
1.2k stars 438 forks source link

Support opentelemetry Javaagent extensions #1758

Closed dreaminghk closed 7 months ago

dreaminghk commented 1 year ago

Hi everyone! We are looking into implement an custom propagator to propagate x-request-id for ISTIO as an opentelemetry extension.

As instructed in this page, we can implement that, built it as a jar and build a custom java instrumentation image that include that jar.

But the OpentelemetryOperator has to instruct the client to use it by adding -Dotel.javaagent.extensions

java -javaagent:path/to/opentelemetry-javaagent.jar \
     -Dotel.javaagent.extensions=build/libs/opentelemetry-java-instrumentation-extension-demo-1.0-all.jar
     -jar myapp.jar

I have tried something like the following in the Instrumentation Object:

  java:
    image: ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-java:latest
    env:
      - name: JAVA_TOOL_OPTIONS
        value: "$(JAVA_TOOL_OPTIONS) -Dotel.javaagent.extensions=build/libs/opentelemetry-java-instrumentation-extension-demo-1.0-all.jar"

However, only env names starting with OTEL and SPLUNK are supported.

Does anyone has any idea on how to instruct the application to use a opentelemetry extension? Thank you!

jaronoff97 commented 1 year ago

If you have a custom image with your code already, all you should need to do is change the java image in the instrumentation object rather than change the environment variables. (docs)

example:

  java:
    image: <your-custom-image>:<tag>
dreaminghk commented 1 year ago

Thanks @jaronoff97 for your reply! In that case, looks like we will need to build the propagator into the OpenTelemetry Java Agent to produce a single jar file.

pavolloffay commented 1 year ago

Let's reopen this, as the java extension questions come up often. We should consider supporting it and document it.

We can still require users to provide a custom image with java agent but additionally copy the extension as well and allow configuring it.

crossoverJie commented 8 months ago
FROM busybox

ADD open-telemetry/opentelemetry-javaagent.jar /javaagent.jar

# Copy extensions to specify a path.
ADD open-telemetry/ext-1.0.0.jar /ext-1.0.0.jar

RUN chmod -R go+r /javaagent.jar
RUN chmod -R go+r /ext-1.0.0.jar

apiVersion: opentelemetry.io/v1alpha1
kind: Instrumentation
metadata:
  name: my-instrumentation
spec:
  java:
    image: custom-image:1.0.0
    extensions: /ext-1.0.0.jar
    env:
    # If extension.jar already exists in the container, you can only specify a specific path with this environment variable.
      - name: OTEL_EXTENSIONS_DIR
        value: /custom-dir

We can copy extension.jar to the specified directory in a custom image, and Operator will copy jars from the ${java.extensions} directory to /otel-auto-instrumentation/.

JVM parameters will also be added to the JAVA_TOOL_OPTIONS environment variable: -Dotel.javaagent.extensions=/otel-auto-instrumentation/${java.extensions}.jar.

At the same time, add an environment variable OTEL_EXTENSIONS_DIR, the default value of which can be /otel-auto-instrumentation/extensions.

If extension.jar already exists in the container, we can only specify a specific path with this environment variable.

Are there any other better solutions?

cb645j commented 8 months ago

I need this as well and was working on a draft very similiar to @crossoverJie solution. Can a PR be opened?

crossoverJie commented 8 months ago

I need this as well and was working on a draft very similiar to @crossoverJie solution. Can a PR be opened?

I'm preparing a code implementation, and I'll submit a PR after it's done.

pavolloffay commented 8 months ago

@crossoverJie @cb645j does the extension jar has to be rebuilt for every javaagent release?

pavolloffay commented 8 months ago

I would prefer if the extension Jar would be separate from the auto-instrumentation image.

I am thinking of something like this:

apiVersion: opentelemetry.io/v1alpha1
kind: Instrumentation
metadata:
  name: my-instrumentation
spec:
  java:
    extensions:
      - image: pavolloffay/java-auto-instrumentation-extension1
        dir: /opt/java-extensions

The operator would create an init-container per extension and copy the content from extensions.dir to a shared volume where the agent is copied as well and it would configure the ENV var to load the extension

crossoverJie commented 8 months ago

I would prefer if the extension Jar would be separate from the auto-instrumentation image.

This is a good suggestion, and it is easier to maintain after separation.

Please assign to me.

crossoverJie commented 8 months ago
    extensions:
      - image: pavolloffay/java-auto-instrumentation-extension1
        dir: /opt/java-extensions

@pavolloffay Maybe we don't need an array here, and we can put extensions together when we build a custom image.

cb645j commented 7 months ago

I would prefer if the extension Jar would be separate from the auto-instrumentation image.

This is a good suggestion, and it is easier to maintain after separation.

Please assign to me.

i agrree

cb645j commented 7 months ago

@pavolloffay @crossoverJie i dont see a need for an array, also do we need to specifiy the dir? I think it should just go in the same place as the agent and for agent we dont specify directory that im aware of... My suggestion would be to keep it simple. Please link PR or draft to this ticket once you have

spec:
  java:
    image: com.abc/java-agent
    extensions:
       image: com.abc/java-agent

or

spec:
  java:
    image: com.abc/java-agent
    extensionsImage: com.abc/java-agent
crossoverJie commented 7 months ago

i dont see a need for an array,

I think using arrays makes it easier to integrate third-party extensions. https://github.com/open-telemetry/opentelemetry-operator/pull/2761/files/924d6e3e2bacd2c79a7be56045849b9e3a7f6cc0


also do we need to specifiy the dir? I think it should just go in the same place as the agent and for agent we dont specify directory that im aware of... My suggestion would be to keep it simple.

I also agree with keeping it simple, but we still need to separate the directory of extensions from the directory of agent, otherwise we cannot distinguish which one is agent and which ones are extensions. https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/93e06678168c5bce2c2a892685b06cceb8c1519d/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ExtensionClassLoader.java#L154

So maybe we can make dir an optional field with default value of /extensions. @pavolloffay What's your opinion?