Skip to content

Commit e27f234

Browse files
authored
Fix the gap in the build time instrumentation plugin to allow processing additional folder (#10735)
fix(build): TODO non-deterministic OTel class embedding in otel-bootstrap shadow jar The problem was that `./gradlew shadowJar --rerun-tasks` execution was non-deterministic depending on the state of the build. This resulted in `otel-bootstrap` jar sometimes embedded the full set of OTel semconv classes (~189), and sometimes only the Java-compiled ones (~4). ### Root cause `unpackJars` and `compileJava` shared the same output directory (`compileJava.destinationDirectory` = `build/classes/java/main/`). After a first successful build, `main/` contained **instrumented** OTel classes (modified in-place by the ByteBuddy `doLast` action). But on the next execution **with** `--rerun-tasks`: 1. `unpackJars` re-ran and wrote **original** (un-instrumented) OTel classes to `main/`, replacing the previously-instrumented versions. 2. The Java compiler detected that files in its output directory had changed (instrumented → original) and switched to **non-incremental mode**, in the info log: Not incremental: removing prior outputs" thus deleting **all** prior outputs from `main/` — including the OTel classes just placed by `unpackJars`. 3. In the end only the project's compiled and processed classes files remained. The bug was not manifested on a clean state because `main/` had no prior files for the Java compiler to compare against. --- This commit fixes the situation by filling a gap in the build time instrumentation plugin: - `unpackJars`'s output is now a dedicated `build/otel-unpacked/` directory to avoid overlap with `compileJava.destinationDirectory`. - Build time instrumentation plugin allow to declare a `includeClassDirectories` used as task input, its classes will be included for post processing. Because `build/otel-unpacked/` is now only written to by `unpackJars`, Gradle's stale-output cleanup for `compileJava` never touches it. chore: Rename unpackJars to unpackOtelJars fix(build): Properly wire the unpackJars task The `unpackJars` task dependency do not have to be declared as a dependency of the compile task, however it should be declared as an included input of the build time instrumentation. Also, the `unpackJars` is now a `Sync` task to deletes files that are no longer in the source JARs. style: sourcesDir to classesDir Merge branch 'master' into bdu/allow-build-time-instrumentation-to-process-additional-classes Merge branch 'master' into bdu/allow-build-time-instrumentation-to-process-additional-classes Co-authored-by: brice.dutheil <brice.dutheil@datadoghq.com>
1 parent 77900be commit e27f234

6 files changed

Lines changed: 42 additions & 27 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package datadog.gradle.plugin.instrument
22

3+
import org.gradle.api.file.ConfigurableFileCollection
34
import org.gradle.api.file.DirectoryProperty
45
import org.gradle.api.provider.ListProperty
56

67
abstract class BuildTimeInstrumentationExtension {
78
abstract ListProperty<String> getPlugins()
89
abstract ListProperty<DirectoryProperty> getAdditionalClasspath()
10+
abstract ConfigurableFileCollection getIncludeClassDirectories()
911
}

buildSrc/src/main/groovy/datadog/gradle/plugin/instrument/BuildTimeInstrumentationPlugin.groovy

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ class BuildTimeInstrumentationPlugin implements Plugin<Project> {
126126
it.inputs.property("javaVersion", javaVersion)
127127
it.inputs.property("plugins", extension.plugins)
128128
it.inputs.files(extension.additionalClasspath)
129+
it.inputs.files(extension.includeClassDirectories)
129130

130131
// Temporary location for raw (un-instrumented) classes
131132
DirectoryProperty tmpUninstrumentedClasses = project.objects.directoryProperty().value(
@@ -149,7 +150,8 @@ class BuildTimeInstrumentationPlugin implements Plugin<Project> {
149150
extension.plugins,
150151
instrumentingClassPath,
151152
it.destinationDirectory,
152-
tmpUninstrumentedClasses
153+
tmpUninstrumentedClasses,
154+
extension.includeClassDirectories
153155
)
154156
)
155157
logger.info("[BuildTimeInstrumentationPlugin] Configured post-compile instrumentation for $compileTaskName for source-set $sourceSetName")

buildSrc/src/main/groovy/datadog/gradle/plugin/instrument/InstrumentAction.groovy

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ abstract class InstrumentAction implements WorkAction<InstrumentWorkParameters>
4444
from(classesDirectory)
4545
into(tmpUninstrumentedDir)
4646
}
47+
// Merge any additional class directories (e.g. unpacked dependency JARs) to be processed
48+
parameters.includeClassDirectories.files.each { classesDir ->
49+
if (classesDir.exists()) {
50+
fileSystemOperations.copy {
51+
from(classesDir)
52+
into(tmpUninstrumentedDir)
53+
}
54+
}
55+
}
4756
fileSystemOperations.delete {
4857
delete(objects.fileTree().from(classesDirectory))
4958
}

buildSrc/src/main/groovy/datadog/gradle/plugin/instrument/InstrumentPostProcessingAction.groovy

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,30 +35,33 @@ abstract class InstrumentPostProcessingAction implements Action<AbstractCompile>
3535
final FileCollection instrumentingClassPath
3636
final DirectoryProperty compilerOutputDirectory
3737
final DirectoryProperty tmpDirectory
38+
final FileCollection includeClassDirectories
3839

3940
@Inject
4041
InstrumentPostProcessingAction(
4142
String javaVersion,
4243
ListProperty<String> plugins,
4344
FileCollection instrumentingClassPath,
4445
DirectoryProperty compilerOutputDirectory,
45-
DirectoryProperty tmpDirectory
46+
DirectoryProperty tmpDirectory,
47+
FileCollection includeClassDirectories
4648
) {
4749
this.javaVersion = javaVersion == BuildTimeInstrumentationPlugin.DEFAULT_JAVA_VERSION ? JavaLanguageVersion.current() : JavaLanguageVersion.of(javaVersion)
4850
this.plugins = plugins
4951
this.instrumentingClassPath = instrumentingClassPath
5052
this.compilerOutputDirectory = compilerOutputDirectory
5153
this.tmpDirectory = tmpDirectory
54+
this.includeClassDirectories = includeClassDirectories
5255
}
5356

5457
@Override
5558
void execute(AbstractCompile task) {
5659
logger.info(
5760
"""
58-
[InstrumentPostProcessingAction] About to instrument classes
59-
javaVersion=${javaVersion},
60-
plugins=${plugins.get()},
61-
instrumentingClassPath=${instrumentingClassPath.files},
61+
[InstrumentPostProcessingAction] About to instrument classes
62+
javaVersion=${javaVersion},
63+
plugins=${plugins.get()},
64+
instrumentingClassPath=${instrumentingClassPath.files},
6265
rawClassesDirectory=${compilerOutputDirectory.get().asFile}
6366
""".stripIndent()
6467
)
@@ -72,6 +75,7 @@ abstract class InstrumentPostProcessingAction implements Action<AbstractCompile>
7275
parameters.instrumentingClassPath.setFrom(postCompileAction.instrumentingClassPath)
7376
parameters.compilerOutputDirectory.set(postCompileAction.compilerOutputDirectory)
7477
parameters.tmpDirectory.set(postCompileAction.tmpDirectory)
78+
parameters.includeClassDirectories.setFrom(postCompileAction.includeClassDirectories)
7579
})
7680
}
7781

buildSrc/src/main/groovy/datadog/gradle/plugin/instrument/InstrumentWorkParameters.groovy

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,5 @@ interface InstrumentWorkParameters extends WorkParameters {
1414
ConfigurableFileCollection getInstrumentingClassPath()
1515
DirectoryProperty getCompilerOutputDirectory()
1616
DirectoryProperty getTmpDirectory()
17+
ConfigurableFileCollection getIncludeClassDirectories()
1718
}

dd-java-agent/agent-otel/otel-bootstrap/build.gradle

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,26 @@ configurations {
2828
}
2929
}
3030

31+
// Unpack the embeddedClasspath jars into otelUnpackedDir so that OtelShimGradlePlugin can
32+
// instrument them (e.g. inject the OTel shim).
33+
def unpackOtelJars = tasks.register('unpackOtelJars', Sync) {
34+
exclude 'META-INF/'
35+
exclude '**/module-info.class'
36+
exclude '**/package-info.class'
37+
38+
from(configurations.named('embeddedClasspath').map {
39+
it.collect {
40+
zipTree(it)
41+
}
42+
})
43+
// Note: they are intentionally kept in a separate directory from compileJava's output so that
44+
// Gradle's stale-output cleanup on a full rebuild does not delete them before instrumentation.
45+
into layout.buildDirectory.dir("otel-unpacked")
46+
}
47+
3148
buildTimeInstrumentation {
3249
plugins.addAll('datadog.opentelemetry.tooling.shim.OtelShimGradlePlugin')
50+
includeClassDirectories.from(unpackOtelJars)
3351
}
3452

3553
minimumInstructionCoverage = 0.0
@@ -56,27 +74,6 @@ dependencies {
5674

5775
compileOnly project(':dd-java-agent:agent-bootstrap')
5876
implementation project(':dd-java-agent:agent-otel:otel-shim')
59-
60-
// buildTimeInstrumentationPlugin project(path: ':dd-java-agent:agent-otel:otel-tooling', configuration: 'buildTimeInstrumentationToolingPlugins')
61-
}
62-
63-
// unpack embeddedClasspath to same path as compiled classes so it can get instrumented
64-
tasks.register('unpackJars', Copy) {
65-
dependsOn configurations.named('embeddedClasspath')
66-
exclude 'META-INF/'
67-
exclude '**/module-info.class'
68-
exclude '**/package-info.class'
69-
70-
from(configurations.named('embeddedClasspath').map {
71-
it.collect {
72-
zipTree(it)
73-
}
74-
})
75-
into compileJava.destinationDirectory
76-
}
77-
78-
tasks.named('compileJava') {
79-
dependsOn 'unpackJars'
8077
}
8178

8279
tasks.named("shadowJar", ShadowJar) {

0 commit comments

Comments
 (0)