Introduce module-kind convention plugins#11620
Conversation
Introduce module-kind convention plugins that describe what a Gradle
project is, rather than only how it is wired.
These conventions keep the existing script plugins as the source of
truth for now, but let modules apply them through plugins {}.
This starts a gradual migration path away from direct apply from usage
without changing existing lower level plugin scripts at this time.
This only changes the application path for `gradle/java.gradle`.
It introduces module conventions for instrumentation, internal
components, smoke tests, sub-agents, annotation processors, and
distributable API modules.
The change also normalizes plugin declarations in touched build files so
dd-trace-java.* and core Gradle plugins are declared in plugins {} where
possible.
Gradle documents convention plugins from `buildSrc` or `build-logic` as
the recommended way to share build logic, while script plugin
application via `apply()` is listed as legacy/not recommended:
* https://docs.gradle.org/9.5.1/userguide/sharing_build_logic_between_subprojects.html#sec:sharing_logic_via_convention_plugins
* https://docs.gradle.org/9.5.1/userguide/plugins_intermediate.html#sec:script_plugins
Disable Spotless' Groovy Java exclusion after the local targets are declared. This preserves the previous scoped formatting behavior now that scala is applied from the plugins block.
Keep the instrumentation aggregator non-Java while restoring archive conventions and the root muzzle report task used by CI.
Keep the module-kind hook, but leave the Java-based configurations and muzzle report behavior in place for now.
Keep instrumentation modules on the same forbidden API signature set that the
script-plugin ordering used before module conventions. Applying `java.gradle`
from `plugins {}` otherwise adds the global `main` filter to these tasks.
Checked with
* `:dd-java-agent:instrumentation:liberty:liberty-20.0:forbiddenApisMain --info`
* `:dd-java-agent:instrumentation:java:java-lang:java-lang-1.8:forbiddenApisMain --info`
* `:components:context:forbiddenApisMain --info`
* ...
on `master` and this branch. On `master` the tasks only reads
`gradle/forbiddenApiFilters/instrumentation.txt` on instrumantation
modules, while the modification in this branch, before this commit,
read `main.txt` + `instrumentation.txt`.
The ordering issue is subtle: the old append on `master` happened before
`forbiddenapis.gradle` installed `main.txt` on the extension.
The `forbidden-apis` 3.10 plugin uses convention mapping rather than a
live Provider-backed task property, so that early task actually read
captured the "then-empty" default when adding `instruentation.txt`
and the later extension assignment did not appended `main.txt` into the
task value.
The purpose of this branch is to keep the convention-plugin migration
behavior-neutral. A follow-up PR should restore the intended rule set,
i.e. `main.txt` + `instrumentation.txt`, because the current mechanism
is broken and allowed some forbidden APIs only covered by `main.txt`
to leak into instrumentation modules.
Apply the shadow plugin before the distributable API convention so `publish.gradle` sees the same plugin state it saw on `master`. This keeps `dd-trace-ot` on the shadow publication path and avoids publishing metadata that points at internal components.
This comment has been minimized.
This comment has been minimized.
|
Going back to draft, as some sub-agent modules convention are just not right. |
Rename the `sub-agent` module kind to `agent-product` and reserve it for shipped agent products. Add `bootstrap-component` and `testing-support` module kinds.
7c6df25 to
b5b937b
Compare
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 9 metrics, 6 unstable metrics. See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (350.188 µs) : 290, 410
. : milestone, 350,
basic (300.036 µs) : 292, 308
. : milestone, 300,
loop (8.985 ms) : 8980, 8991
. : milestone, 8985,
section candidate
noprobe (331.036 µs) : 307, 355
. : milestone, 331,
basic (296.931 µs) : 291, 303
. : milestone, 297,
loop (8.994 ms) : 8989, 8998
. : milestone, 8994,
|
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
| signaturesFiles += subProj.files("$rootDir/gradle/forbiddenApiFilters/instrumentation.txt") | ||
| } | ||
| // Preserve the historical instrumentation-only forbiddenApi rules. | ||
| subProj.tasks.withType(CheckForbiddenApis).configureEach { | ||
| signaturesFiles = subProj.files("$rootDir/gradle/forbiddenApiFilters/instrumentation.txt") |
There was a problem hiding this comment.
todo: This will be fixed by #11623 (at this time it needs to explicitly include main.text), I'll rebase this PR once it's merged.
Kafka / producer-benchmarkParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics. See unchanged results
|
Kafka / consumer-benchmarkParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics. See unchanged results
|
What Does This Do
Introduces module-kind convention plugins for Java-backed Gradle modules and applies them from
plugins {}while keeping the existing script plugins as the behavior source.The basic change is usually removing the
apply fromof the java script plugin, and use the modern recommended plugins block with the identified module kind :In particular these are the identified module kind, in this PR, it's likely some kind are not represented in this PR, and that's not the goal to be exhaustive at this stage, it can be refined later (for example for the shared modules in the agent jar).
dd-trace-java.module.agent-productdd-trace-java.module.annotation-processordd-trace-java.module.bootstrap-componentdd-trace-java.module.distributable.api(here the dot is intended)dd-trace-java.module.instrumentationdd-trace-java.module.internal-componentdd-trace-java.module.smoke-testdd-trace-java.module.testing-supportI chose
moduleprefix as a way to convey the "higher level" module kind. While there might be other lower level technical conventions.Small point of attention to the
:dd-java-agent:instrumentationmodule, this PR replaces this checkBy checking if the sub project has the
dd-trace-java.module.instrumentationconvention, if it does it applies the necessary conventions for the instrumentation, so it just wraps existing code within the following closure:Motivation
Prepare a gradual migration from lower-level script plugin application to higher-level conventions that describe what each module is.
Gradle references used for this migration direction:
Additional Details
The current forbidden-apis wiring for instrumentation modules is preserved behaviorally, but the investigation found it is not the intended rule set. On
master, instrumentation tasks effectively read onlyinstrumentation.txt; the expected rule set should bemain.txtplusinstrumentation.txt. The old+=ordering withforbidden-apis3.10 convention mapping let APIs covered only bymain.txtleak into instrumentation modules. Restoring the intended combined signature set should be a follow-up PR.:dd-java-agent:instrumentationitself is intentionally not migrated further here. Its aggregator-specific wiring, especially around muzzle reports, needs preparatory work before moving more of that project into convention-plugin logic.jardiffwas used to compare the agent jar frommasterand this branch. The agent jar contains both.classand.classdataentries, so the stricter check includes both and coalesces.classdataas class-like bytecode:Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: [N/A]