-
Notifications
You must be signed in to change notification settings - Fork 419
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
Publish a docker image containing Java autoinstrumentation files. #475
Conversation
@@ -0,0 +1 @@ | |||
1.7.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For other languages, this will be a version number in something like a package.json
. I tried to provide a similar "edit a file to update the version" experience for Java using this txt file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative approach would be to put the version in https://github.com/open-telemetry/opentelemetry-operator/blob/main/versions.txt and publish the image on operator release.
However, I like your approach more as it allows us to release the java docker image without operator release. I still need the version being defined in the root version.txt to use it as a default version of the javaagent when the image is not specified in the CR (I will do this on a follow up PR once the initial implementation is merged).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - yeah since instrumentation will be published at various cycles it's probably good to decouple those release cycles.
Added the version to top level to allow operator to reference the most recent one (which will generally have already been published before the operator is released)
/cc @pavolloffay |
@@ -0,0 +1 @@ | |||
1.7.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative approach would be to put the version in https://github.com/open-telemetry/opentelemetry-operator/blob/main/versions.txt and publish the image on operator release.
However, I like your approach more as it allows us to release the java docker image without operator release. I still need the version being defined in the root version.txt to use it as a default version of the javaagent when the image is not specified in the CR (I will do this on a follow up PR once the initial implementation is merged).
@VineethReddy02 @jpkrohling could you please take a look and move this forward? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I think after iterating over Juraci comments. We can go ahead with the merge :)
@anuraaga the CI failed |
@VineethReddy02, can you look at the failure? Looks like it's not related to this PR. |
This is the CI failure === RUN Test0_36_0Upgrade
v0_36_0_test.go:119:
Error Trace: v0_36_0_test.go:119
Error: Not equal:
expected: "upgrade to v0.36.0 has changed the tls_settings field name to tls in grpc protocol of otlp/mtls receiver"
actual : "upgrade to v0.36.0 has changed the tls_settings field name to tls in http protocol of otlp/mtls receiver"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-upgrade to v0.36.0 has changed the tls_settings field name to tls in grpc protocol of otlp/mtls receiver
+upgrade to v0.36.0 has changed the tls_settings field name to tls in http protocol of otlp/mtls receiver
Test: Test0_36_0Upgrade
v0_36_0_test.go:120:
Error Trace: v0_36_0_test.go:120
Error: Not equal:
expected: "upgrade to v0.36.0 has changed the tls_settings field name to tls in http protocol of otlp/mtls receiver"
actual : "upgrade to v0.36.0 has changed the tls_settings field name to tls in grpc protocol of otlp/mtls receiver"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-upgrade to v0.36.0 has changed the tls_settings field name to tls in http protocol of otlp/mtls receiver
+upgrade to v0.36.0 has changed the tls_settings field name to tls in grpc protocol of otlp/mtls receiver
Test: Test0_36_0Upgrade
--- FAIL: Test0_36_0Upgrade (0.00s) Indeed not related to this PR. |
Here is fix #494 |
This PR needs a rebase, to get the fix for the flaky test. |
@jpkrohling CI is green now |
…en-telemetry#475) * Publish a docker image containing Java autoinstrumentation files. * Push to anuraaga temporarily * Push to open-telemetry * Allow yaml files for autoinstrumentation to be grouped together * Add reference to java instrumentation version for use in operator * Quay
…en-telemetry#475) * Publish a docker image containing Java autoinstrumentation files. * Push to anuraaga temporarily * Push to open-telemetry * Allow yaml files for autoinstrumentation to be grouped together * Add reference to java instrumentation version for use in operator * Quay
To allow auto-instrumentation from the operator, we need to package the actual instrumentation into a docker image for copying into the user's container. This may be a java agent, or a directory of node_modules, or python pip installed packages, etc.
This adds
ghcr.io/open-telemetry/autoinstrumentation-java
as an image to contain the java instrumentation and is what would be used in conjunction with the pod mutator in https://github.com/open-telemetry/opentelemetry-operator/pull/464/files#diff-8792ce6e230133d6b38ced97a2fdbb6f6282a7c8e5f939cc7783521be7823232R59Note that I discussed with the Java maintainers, and we didn't find there to be enough value in a base image for end users to consume to publish it there, so instead this is meant to be more of an internal detail of the operator. And it would make even less sense for something like Python, where end users just use pip anyways for example (javaagent users don't have such a package management approach in general making it slightly unique).
Ran the workflow on my fork to produce https://github.com/anuraaga/opentelemetry-operator/pkgs/container/opentelemetry-operator%2Fautoinstrumentation-java