Skip to content

Commit 2068d0d

Browse files
bric3bric3
authored
Prevent duplicate class inclusion in runtime artifacts (#11853)
fix(build): Stop publishing extra source-set output twice The `TracerJavaExtension.addSourceSetFor` added the extra source-set output to the `implementation` configuration. A project dependency on the runtime classpath resolves the producer's `runtimeElements` variant. That variant publishes the project jar and runtime dependencies derived from `implementation`, so consumers (the instrumentation project in particular) received the same classes twice: 1. once from the jar configured with `jar.from(mainForJavaVersionSourceSet.output)`, 2. once from the raw class directory published as a dependency This change keeps the additional source set local to the producer module's _compile_ and _test_ classpaths respectively with `compileOnly` and `testImplementation`. The jar remains the runtime artifact consumed by downstream aggregating projects, and it still contains the additional source-set classes. What was considered: dropping the `jar.from(output)` config, it would remove the duplicate, but it would make the consumed module jar incomplete and leave consuming projects relying on class directories instead of the artifact. This would also a the side effect of making Gradle to track individual classes for up-to-date tracking, which consumes more memory. This keeps the existing "jar contract" and only prevent publishing the additional source-set output through `runtimeElements`. This removes the exception-profiling and instrumentation duplicate exclusions that were needed in the instrumentation `shadowJar`. fix(build): Stop publishing agent-installer source-set output twice The `agent-installer` project added its Java-version-specific source-set outputs to the `runtimeOnly` configuration. A project dependency on the runtime classpath resolves the producer's `runtimeElements` variant. That variant publishes the project jar and runtime dependencies derived from `runtimeOnly`, so consumers (the instrumentation project in particular) received the same classes twice: 1. once from the jar configured with `from sourceSets.main_java11.output` and `from sourceSets.main_java25.output`, 2. once from the raw class directories published as runtime dependencies This change keeps the Java-version-specific source sets local to the producer module's test classpath with `testImplementation`. The jar remains the runtime artifact consumed by downstream aggregating projects, and it still contains the Java 11 and Java 25 source-set classes. Dropping the jar configuration would remove the duplicate, but it would make the consumed `agent-installer` jar incomplete and leave consuming projects relying on class directories instead of the artifact. This keeps the existing "jar contract" and only prevents publishing the Java-version-specific source-set output through `runtimeElements`. This removes the agent-installer duplicate exclusions that were needed in the instrumentation `shadowJar`. fix(build): Keep servlet5 relocated jar off `runtimeElements` The `jakarta-servlet-5.0` project added the relocated javax-to-jakarta advice jar to the `implementation` configuration. And a project dependency on the runtime classpath resolves the producer's `runtimeElements` variant. That variant publishes the project jar and its runtime dependencies that are derived from `implementation`, so consumers (the instrumentation project in particular) received the same classes twice: 1. once from the module jar configured with `from zipTree(relocatedJavaxJar.outputs.files.asPath)`, 2. once from the relocated jar published as a runtime dependency This change keeps the relocated jar local to the producer module's _compile_ and _test_ classpaths with `compileOnly` and `testImplementation` respectively. The module jar remains the runtime artifact consumed by downstream projects, and it still contains the relocated servlet5 advice classes. With this, the last duplicate is gone, so drop the remaining `filesMatching { EXCLUDE }` workaround and refresh the comment. chore(build): Use lazy API on jakarta-servlet-5.0 shadow jar task fix(build): Muzzle supports projects having jars outside runtimeClasspath Teach `MuzzleTask` to include an extra classpath (if provided). The goal is to allow it to see the relocated servlet5 jar. It is done by introducing `extraAgentClasspath` on the `MuzzelTask`. Another approach would have been to include the relcated jar on the runtimeClasspath: `sourceSets.main.runtimeClasspath += files(relocatedJavaxJarFile)` However this approach creates confusion ; anything that consume this configuration would see the relocated jar (e.g. during debug). Co-authored-by: brice.dutheil <brice.dutheil@datadoghq.com>
1 parent bb6b14a commit 2068d0d

5 files changed

Lines changed: 78 additions & 32 deletions

File tree

‎buildSrc/src/main/kotlin/datadog/gradle/plugin/muzzle/tasks/MuzzleTask.kt‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import datadog.gradle.plugin.muzzle.MuzzleExtension
77
import datadog.gradle.plugin.muzzle.allMainSourceSet
88
import org.gradle.api.Project
99
import org.gradle.api.artifacts.Configuration
10+
import org.gradle.api.file.ConfigurableFileCollection
1011
import org.gradle.api.file.FileCollection
1112
import org.gradle.api.file.RegularFileProperty
1213
import org.gradle.api.invocation.BuildInvocationDetails
@@ -59,6 +60,10 @@ abstract class MuzzleTask @Inject constructor(
5960
@get:Classpath
6061
protected val agentClassPath = providers.provider { createAgentClassPath(project) }
6162

63+
@get:InputFiles
64+
@get:Classpath
65+
abstract val extraAgentClasspath: ConfigurableFileCollection
66+
6267
@get:InputFiles
6368
@get:Classpath
6469
protected val muzzleClassPath = providers.provider { createMuzzleClassPath(project, name) }
@@ -119,7 +124,7 @@ abstract class MuzzleTask @Inject constructor(
119124
buildStartedTime.set(invocationDetails.buildStartedTime)
120125
bootstrapClassPath.setFrom(muzzleBootstrap)
121126
toolingClassPath.setFrom(muzzleTooling)
122-
instrumentationClassPath.setFrom(agentClassPath.get())
127+
instrumentationClassPath.setFrom(agentClassPath.get(), extraAgentClasspath)
123128
testApplicationClassPath.setFrom(muzzleClassPath.get())
124129
if (muzzleDirective != null) {
125130
assertPass.set(muzzleDirective.assertPass)

‎dd-java-agent/agent-installer/build.gradle‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,26 @@ tasks.named("compileMain_java25Java", JavaCompile) {
4242
}
4343

4444
dependencies {
45+
// Wire the Java-version-specific source sets into this project without publishing
46+
// their raw output:
47+
//
48+
// - each additional source set compiles against the main output and only the
49+
// dependencies it needs
50+
// - their compiled outputs stay visible to tests through `testImplementation`
51+
// - their compiled outputs are folded into this project's jar by the jar task below
52+
//
53+
// Main code loads these classes reflectively at agent runtime, so downstream
54+
// consumers should get them from the agent-installer jar. Do not add these
55+
// outputs to `runtimeOnly`: `runtimeElements` publishes both the jar artifact
56+
// and runtime dependencies. Publishing the raw outputs there would expose the
57+
// same classes twice, once from the jar and once from the class directories.
4558
main_java11CompileOnly project(':dd-java-agent:agent-tooling')
4659
main_java11CompileOnly sourceSets.main.output
60+
testImplementation sourceSets.main_java11.output
61+
4762
main_java25CompileOnly project(':dd-trace-core')
4863
main_java25CompileOnly sourceSets.main.output
49-
runtimeOnly sourceSets.main_java11.output
50-
runtimeOnly sourceSets.main_java25.output
64+
testImplementation sourceSets.main_java25.output
5165
}
5266

5367
tasks.named("jar", Jar) {

‎dd-java-agent/instrumentation/build.gradle‎

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -119,31 +119,15 @@ if (project.gradle.startParameter.taskNames.any { it.endsWith("generateMuzzleRep
119119
}
120120

121121
tasks.named('shadowJar', ShadowJar) {
122-
duplicatesStrategy = DuplicatesStrategy.FAIL
123-
// Shadow 8.x silently wrote duplicate class (130+) entries into the instrumentation jar.
124-
// Those duplicate entries did not survive as duplicates in the final agent jar,
125-
// because `:dd-java-agent:expandAgentShadowJarInst` expands the instrumentation jar
126-
// with a `Sync` task, and the specific `inst` expansion keeps the first duplicate path.
127-
// (explicit `duplicatesStrategy = DuplicatesStrategy.EXCLUDE` on the `inst` prefix).
122+
// Keep duplicate detection enabled on the aggregate instrumentation jar.
128123
//
129-
// Shadow 9 refactored deeply it's "copy" pipeline via https://github.com/GradleUp/shadow/pull/1233
130-
// by `Project.zipTree` and can now honor `DuplicatesStrategy`, see in particular
131-
// * https://github.com/GradleUp/shadow/issues/1223 - Duplicates from jars are not handled per duplicatesStrategy
132-
// * https://github.com/GradleUp/shadow/issues/488 - DuplicatesStrategy.FAIL doesn't work with transform
124+
// The previous source-set fixes and the servlet5 relocated-jar fix keep extra classes in
125+
// producer jars without also publishing their raw output through `runtimeElements`. The old
126+
// per-path `EXCLUDE` workaround is no longer needed.
133127
//
134-
// Keeping `duplicatesStrategy = FAIL` only, Shadow 9 now fails while creating the instrumentation jar.
135-
filesMatching([
136-
// agent-installer also publishes this Java 11 source-set output at runtime.
137-
'datadog/trace/agent/tooling/bytebuddy/DDJava9ClassFileTransformer*.class',
138-
// agent-installer also publishes this Java 25 source-set output at runtime.
139-
'datadog/trace/agent/tooling/servicediscovery/MemFDUnixWriterFFM*.class',
140-
// aggregate instrumentation includes some classes directly and via jars.
141-
'datadog/trace/instrumentation/**/*.class',
142-
// same aggregate duplication, for Java 11 exception profiling advice.
143-
'datadog/exceptions/instrumentation/**/*.class',
144-
]) {
145-
duplicatesStrategy = DuplicatesStrategy.EXCLUDE
146-
}
128+
// Shadow 9 honors `DuplicatesStrategy.FAIL` for these inputs. If a producer publishes the same
129+
// classes from both its jar and a runtime file or directory, this task should fail again.
130+
duplicatesStrategy = DuplicatesStrategy.FAIL
147131
dependencies {
148132
// the tracer is now in a separate shadow jar
149133
exclude(project(":dd-trace-core"))

‎dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/build.gradle‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
2+
import datadog.gradle.plugin.muzzle.tasks.MuzzleTask
23

34
plugins {
45
id 'java-test-fixtures'
@@ -30,7 +31,7 @@ configurations {
3031
javaxClassesToRelocate
3132
}
3233

33-
tasks.register('relocatedJavaxJar', ShadowJar) {
34+
def relocatedJavaxJar = tasks.register('relocatedJavaxJar', ShadowJar) {
3435
relocate 'javax.servlet', 'jakarta.servlet'
3536
relocate 'datadog.trace.instrumentation.servlet3', 'datadog.trace.instrumentation.servlet5'
3637
relocate 'datadog.trace.instrumentation.servlet', 'datadog.trace.instrumentation.servlet5'
@@ -49,9 +50,24 @@ tasks.register('relocatedJavaxJar', ShadowJar) {
4950

5051
includeEmptyDirs = false
5152
}
53+
def relocatedJavaxJarFile = relocatedJavaxJar.flatMap { it.archiveFile }
5254

5355
dependencies {
54-
implementation files(relocatedJavaxJar.outputs.files)
56+
// Wire the relocated javax->jakarta advice into this project without publishing
57+
// the relocated jar as runtime output:
58+
//
59+
// - keep the relocated jar visible to main compilation through `compileOnly`
60+
// - keep the relocated jar visible to tests through `testImplementation`
61+
// - include the relocated classes into this project's jar (jar task config)
62+
//
63+
// NOTE:
64+
// Do not add the relocated jar to `implementation`: `runtimeElements` publishes
65+
// both the jar artifact and runtime dependencies derived from `implementation`.
66+
// Publishing the relocated jar there would expose the same servlet5 advice classes
67+
// twice to downstream projects, once from the module jar and once from the
68+
// relocated jar.
69+
compileOnly files(relocatedJavaxJarFile)
70+
testImplementation files(relocatedJavaxJarFile)
5571
compileOnly group: 'jakarta.servlet', name: 'jakarta.servlet-api', version: '5.0.0'
5672
testImplementation group: 'jakarta.servlet', name: 'jakarta.servlet-api', version: '5.0.0'
5773
testImplementation group: 'jakarta.servlet.jsp', name: 'jakarta.servlet.jsp-api', version: '3.0.0'
@@ -76,5 +92,12 @@ dependencies {
7692
}
7793

7894
tasks.named("jar", Jar) {
79-
from zipTree(relocatedJavaxJar.outputs.files.asPath)
95+
from zipTree(relocatedJavaxJarFile)
96+
}
97+
98+
// Make the relocated jar visible to muzzle through `extraAgentClasspath`.
99+
// Muzzle uses the runtime classpath of a project, and this jar is produced
100+
// outside of it.
101+
tasks.withType(MuzzleTask).configureEach {
102+
extraAgentClasspath.from(relocatedJavaxJarFile)
80103
}

‎gradle/java_no_deps.gradle‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,33 @@ class TracerJavaExtension {
105105
}
106106
}
107107

108-
// "socket-utils" is only set to compileOnly because the implementation dependency incorrectly adds Java17 classes to all jar prefixes.
109-
// This causes the AgentJarIndex to search for other non-Java17 classes in the wrong prefix location and fail to resolve class names.
110108
if (sourceSetConfig.compileOnly.orElse(false).get()) {
109+
// The ":utils:socket-utils" is only set to `compileOnly` because the `implementation` dependency
110+
// adds Java17 classes to all jar prefixes, which is incorrect for this project.
111+
// This causes the AgentJarIndex to search for other non-Java17 classes in the wrong prefix
112+
// location and to fail to resolve class names.
111113
project.dependencies.add("compileOnly", mainForJavaVersionSourceSet.output)
112114
} else {
115+
// Wire this additional source-set into this project without publishing its raw output
116+
// (handled by the jar configuration). The following
117+
//
118+
// * make this additional source-set's _compile classpath_ visible to **main compilation** through `compileOnly`
119+
// * make this additional source-set's _compiled output_ visible to **main compilation** through `compileOnly`
120+
// * make this additional source-set's _compiled output_ visible to tests through `testImplementation`
121+
//
122+
// The jar task produces the artifact that consumer wants to see, and it is configured
123+
// below to add classes of this additional source-set.
124+
//
125+
// NOTE:
126+
// We don't want to add this source-set's output to the `implementation` configuration!
127+
// Otherwise this additional source-set output directory (where classes are) would end up
128+
// being published as a runtime dependency through `runtimeElements`.
129+
// The `runtimeElements` configuration publishes the jar artifact by default ; thus
130+
// `runtimeElements` would have both the jar, and the added output folder.
131+
// In short this makes classes of this extra source-set in both the jar and the directory.
113132
project.dependencies.add("compileOnly", project.files(mainForJavaVersionSourceSet.compileClasspath))
114-
project.dependencies.add("implementation", mainForJavaVersionSourceSet.output)
133+
project.dependencies.add("compileOnly", mainForJavaVersionSourceSet.output)
134+
project.dependencies.add("testImplementation", mainForJavaVersionSourceSet.output)
115135
}
116136

117137
project.tasks.named("jar", Jar) {

0 commit comments

Comments
 (0)