Change the default javaagent artifact to have exporters #4106
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -265,6 +265,18 @@ jobs: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| with: | ||
| upload_url: ${{ steps.create_release.outputs.upload_url }} | ||
| asset_path: javaagent/build/libs/opentelemetry-javaagent-${{ github.event.inputs.version }}.jar | ||
| asset_name: opentelemetry-javaagent.jar | ||
| asset_content_type: application/java-archive | ||
|
|
||
| # TODO (trask) delete this after the 1.7.0 release (to make sure it is used for any 1.6.x patches) | ||
| - name: Upload Release Asset (backwards compatible "all" artifact) | ||
| id: upload-release-asset | ||
| uses: actions/[email protected] | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| with: | ||
| upload_url: ${{ steps.create_release.outputs.upload_url }} | ||
| asset_path: javaagent/build/libs/opentelemetry-javaagent-${{ github.event.inputs.version }}.jar | ||
| asset_name: opentelemetry-javaagent-all.jar | ||
| asset_content_type: application/java-archive | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,6 +173,18 @@ jobs: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| with: | ||
| upload_url: ${{ steps.create_release.outputs.upload_url }} | ||
| asset_path: javaagent/build/libs/opentelemetry-javaagent-${{ github.event.inputs.version }}.jar | ||
| asset_name: opentelemetry-javaagent.jar | ||
| asset_content_type: application/java-archive | ||
|
|
||
| # TODO (trask) delete this after the 1.7.0 release | ||
| - name: Upload Release Asset (backwards compatible "all" artifact) | ||
| id: upload-release-asset | ||
| uses: actions/[email protected] | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| with: | ||
| upload_url: ${{ steps.create_release.outputs.upload_url }} | ||
| asset_path: javaagent/build/libs/opentelemetry-javaagent-${{ github.event.inputs.version }}.jar | ||
| asset_name: opentelemetry-javaagent-all.jar | ||
| asset_content_type: application/java-archive | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,7 @@ tasks { | |
| } | ||
|
|
||
| named("jmh") { | ||
| dependsOn(":javaagent:shadowJar") | ||
| } | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,11 +18,6 @@ or they might want to manually create spans for their own custom code. | |
|
|
||
| # Dependencies | ||
|
|
||
|
Comment on lines
-21
to
-25
There was a problem hiding this comment. Are we sure about that? Have we tested that? There was a problem hiding this comment. we have a smoke test that verifies interop with opentelemetry-api 1.0.0, and #3044 to track doing better than that with muzzle (also, this is old doc, I'm removing it, see #4106 (comment)) |
||
| You'll need to add a dependency on the `opentelemetry-api` library to get started; if you intend to | ||
| use the `@WithSpan` annotation, also include the `opentelemetry-extension-annotations` dependency. | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,9 +35,14 @@ val exporterLibs by configurations.creating { | |
| isCanBeResolved = true | ||
| isCanBeConsumed = false | ||
| } | ||
| // this configuration collects just exporter libs for slim artifact (also placed in the agent classloader & isolated from the instrumented application) | ||
| val exporterSlimLibs by configurations.creating { | ||
| isCanBeResolved = true | ||
| isCanBeConsumed = false | ||
| } | ||
|
|
||
| // exclude dependencies that are to be placed in bootstrap from agent libs - they won't be added to inst/ | ||
| listOf(javaagentLibs, exporterLibs, exporterSlimLibs).forEach { | ||
| it.run { | ||
| exclude("org.slf4j") | ||
| exclude("io.opentelemetry", "opentelemetry-api") | ||
|
|
@@ -73,6 +78,10 @@ dependencies { | |
|
|
||
| exporterLibs(project(":javaagent-exporters")) | ||
|
|
||
| exporterSlimLibs("io.opentelemetry:opentelemetry-exporter-otlp") | ||
| exporterSlimLibs("io.opentelemetry:opentelemetry-exporter-otlp-metrics") | ||
| exporterSlimLibs("io.grpc:grpc-okhttp:1.41.0") | ||
|
There was a problem hiding this comment. Just realized that because this is using There was a problem hiding this comment. Well assuming it doesn't cause dependency hell, wow that's old https://github.com/grpc/grpc-java/blob/master/build.gradle#L158 There was a problem hiding this comment. Err - nevermind, yeah it wouldn't cause dependency hell since it's a different artifact So scratch the almost-no-additional-bytecode idea. There was a problem hiding this comment. By the way, just some random trivia grpc/grpc-java#6119 (comment) There was a problem hiding this comment.
this thread helped my mental picture 👍 There was a problem hiding this comment. It appears that we can exclude okhttp 2 from the transitive dependencies open-telemetry/opentelemetry-java#3678 though I added a comment on the grpc-java issue to see if there is anything obviously bad with that. Currently I guess we effectively have two copies of okhttp2, the forked-into-grpc one and the transitive dependency which we don't seem to need. There was a problem hiding this comment. I added #4240 to discuss/track |
||
|
|
||
| // We only have compileOnly dependencies on these to make sure they don't leak into POMs. | ||
| licenseReportDependencies("com.github.ben-manes.caffeine:caffeine") { | ||
| isTransitive = false | ||
|
|
@@ -143,12 +152,23 @@ tasks { | |
| archiveFileName.set("exporterLibs-relocated.jar") | ||
| } | ||
|
|
||
| val relocateExporterSlimLibs by registering(ShadowJar::class) { | ||
| configurations = listOf(exporterSlimLibs) | ||
|
|
||
| archiveFileName.set("exporterSlimLibs-relocated.jar") | ||
| } | ||
|
|
||
| // Includes everything needed for OOTB experience | ||
| val shadowJar by existing(ShadowJar::class) { | ||
| configurations = listOf(bootstrapLibs) | ||
|
|
||
| // without an explicit dependency on jar here, :javaagent:test fails on CI because :javaagent:jar | ||
| // runs after :javaagent:shadowJar and loses (at least) the manifest entries | ||
| dependsOn(jar, relocateJavaagentLibs, relocateExporterLibs) | ||
| isolateClasses(relocateJavaagentLibs.get().outputs.files) | ||
| isolateClasses(relocateExporterLibs.get().outputs.files) | ||
|
|
||
| duplicatesStrategy = DuplicatesStrategy.EXCLUDE | ||
|
|
||
| archiveClassifier.set("") | ||
|
|
||
|
|
@@ -164,17 +184,15 @@ tasks { | |
| } | ||
| } | ||
|
|
||
| // Includes instrumentations, but not exporters | ||
|
There was a problem hiding this comment. Slim does include OTLP exporter, right? |
||
| val slimShadowJar by registering(ShadowJar::class) { | ||
| configurations = listOf(bootstrapLibs) | ||
|
|
||
| dependsOn(relocateJavaagentLibs, relocateExporterSlimLibs) | ||
| isolateClasses(relocateJavaagentLibs.get().outputs.files) | ||
| isolateClasses(relocateExporterSlimLibs.get().outputs.files) | ||
|
|
||
| archiveClassifier.set("slim") | ||
|
|
||
| manifest { | ||
| attributes(shadowJar.get().manifest.attributes) | ||
|
|
@@ -207,18 +225,18 @@ tasks { | |
| } | ||
|
|
||
| assemble { | ||
| dependsOn(shadowJar, slimShadowJar, baseJavaagentJar) | ||
| } | ||
|
|
||
| withType<Test>().configureEach { | ||
| dependsOn(shadowJar) | ||
| inputs.file(shadowJar.get().archiveFile) | ||
|
|
||
| jvmArgs("-Dotel.javaagent.debug=true") | ||
|
|
||
| doFirst { | ||
| // Defining here to allow jacoco to be first on the command line. | ||
| jvmArgs("-javaagent:${shadowJar.get().archiveFile.get().asFile}") | ||
| } | ||
|
|
||
| testLogging { | ||
|
|
@@ -237,7 +255,7 @@ tasks { | |
| publishing { | ||
| publications { | ||
| named<MavenPublication>("maven") { | ||
| artifact(slimShadowJar) | ||
| } | ||
| } | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I agree with the rename but it's sort of a breaking change for e.g., Dockerfiles that may be curling the "latest release" URL. Should we upload both at least for one release?
We may also consider whether it is bad practice in general to point at an unversioned URL - if so we can probably update our README to not point at the latest version URL to discourage usage.
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.
makes sense to me, I added attaching backwards compatible "all" artifact step to both release and patch release builds
I like explicit versions in docs (and keeping it updated), IIRC we were stuck on how to automate this? maybe we can tack this problem on to #3516 @iNikem?