Upgrade Shadow plugin to 9.4.2#11730
Conversation
* Adapt custom ShadowJar wiring to the lazy API. Configuration lists now use providers around `configurations.named(...)`, and Shadow resolves `Configuration` values directly. GradleUp/shadow#1044 GradleUp/shadow#1045 * Keep filtering explicit. Shadow 9 changed `DependencyFilter` overloads, so shared filters run in Shadow `dependencies` blocks. Empty module/version fields no longer act as broad wildcards, so group-wide excludes use full group/module/version regex notation. That keeps dependencies meant to stay external out of shaded jars; otherwise downstream jars can get partly relocated API copies, such as SLF4J, and lose their real binding. https://gradleup.com/shadow/configuration/dependencies/#using-regex-patterns-to-filter-dependencies GradleUp/shadow#1328 * Keep `mergeServiceFiles` users on `DuplicatesStrategy.INCLUDE` before transformers run, then drop unrelated duplicate resources with file matching. Shadow 9 honors duplicate strategies during transforms. GradleUp/shadow#1233 GradleUp/shadow#1617 * Disable automatic Multi-Release manifest inheritance for the agent jar to keep the previous manifest shape. GradleUp/shadow#1239 GradleUp/shadow#1675 * Note the relocation output change. Shadow 9.2+ rewrites more descriptor-shaped string constants, which explains the observed `LoggerRuntime` LDC descriptor change. GradleUp/shadow#1714 GradleUp/shadow#1731 * Apply the same API and resource handling changes to smoke-test modules that run in the main Gradle build.
There was a problem hiding this comment.
LGTM from CIVis side! AFAIK we don't use JaCoCo's LoggerRuntime at all, we rely on jacoco for parsing/analyzing coverage reports, and our smoke tests already cover the expected behavior for the instrumentation. So agreed it shouldn't be a regression
AlexeyKuznetsov-DD
left a comment
There was a problem hiding this comment.
LGTM, nice improvement!
Left minor comments.
| filesNotMatching([ | ||
| 'META-INF/services/**', | ||
| 'META-INF/spring.handlers', | ||
| 'META-INF/spring.schemas', | ||
| 'META-INF/spring.tooling', | ||
| 'META-INF/spring.factories', | ||
| ]) { | ||
| duplicatesStrategy = DuplicatesStrategy.EXCLUDE | ||
| } |
There was a problem hiding this comment.
Q: this pattern looks like repeating several times, any ideas how to reuse code?
There was a problem hiding this comment.
For now I think it's fine, this is happening in smoke tests because these projects are using the main build.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What Does This Do
Upgrades the Gradle Shadow plugin from 8.3.9 to 9.4.2 in the main build.
The build wiring is updated for Shadow 9 APIs and behavior:
ShadowJarconfiguration inputs where practicalMotivation
Shadow 9 brings several build-pipeline fixes that are useful for this project:
Migrate
ShadowJarto using lazy properties GradleUp/shadow#1044Resolve
Configurationdirectly inDependencyFilterGradleUp/shadow#1045Refactor file visiting logic in StreamAction GradleUp/shadow#1233
Default
duplicatesStrategyback toEXCLUDEGradleUp/shadow#1617Merge dependency and project overloads in
DependencyFilterGradleUp/shadow#1328Support relocating list of types in
RelocatorRemapperGradleUp/shadow#1714Update
RelocatorRemapperclass pattern to cover more Java method descriptors GradleUp/shadow#1731Additional Notes
bric3/jardiff was used between a master 11646d3 agent jar and this this branch's agent jar using ASM textifier output.
The difference is in a string constant inside JaCoCo’s
LoggerRuntime, not inPatchLogger.javaitself.Shadow 9.2+ started relocating descriptor-shaped string constants. So this
LDCchanged:That comes from the
org.jacoco.core.runtime.LoggerRuntimeclass included via:dd-java-agent:agent-ci-visibility.Before Shadow 9, relocation had already changed the method owner to
datadog/trace/bootstrap/PatchLogger, but the descriptor string still said the method returnedjava.util.logging.Logger. That describes a method that does not exist:PatchLogger.getLogger(String):Logger. Shadow 9 rewrites the descriptor too, so generated bytecode calls the real method:PatchLogger.getLogger(String):PatchLogger, which matchesdatadog.trace.bootstrapPatchLoggerin:dd-java-agent:agent-bootstrap.https://github.com/DataDog/dd-trace-java/blob/987a0eba81a1121ea037005640cd0f407aaf5389/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/PatchLogger.java#L16-L19
In my opinion this looks like a correctness fix for that generated-accessor path, not a behavioral regression. It is also aligned with the agent relocation rule that intentionally redirects
java.util.logging.LoggertoPatchLoggerto avoid JUL initialization during premain:https://github.com/DataDog/dd-trace-java/blob/987a0eba81a1121ea037005640cd0f407aaf5389/dd-java-agent/build.gradle#L114-L116
Coverage-wise:
The Maven and Gradle smoke tests cover the real CI Visibility JaCoCo integration path: they enable the JaCoCo plugin version and assert received coverages/reports.
dd-trace-java/dd-smoke-tests/maven/src/test/java/datadog/smoketest/MavenSmokeTest.java
Lines 84 to 102 in 11646d3
dd-trace-java/dd-smoke-tests/gradle/src/test/java/datadog/smoketest/GradleDaemonSmokeTest.java
Lines 96 to 112 in 11646d3
That path uses the external JaCoCo agent runtime, which our instrumentation targets via
org.jacoco.agent.rt.internal...:dd-trace-java/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/InstrumenterInstrumentation.java
Lines 28 to 35 in 11646d3
The shaded
org.jacoco.coredependency inagent-ci-visibilityappears used for parsing/analyzing coverage data and reports, not forLoggerRuntime:dd-trace-java/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/JacocoCoverageProcessor.java
Lines 233 to 234 in 11646d3
dd-trace-java/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java
Lines 27 to 29 in 11646d3
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issue/merge.Jira ticket: [PROJ-IDENT]