Introduce module-kind convention plugins#11620
Conversation
bric3
added
comp: tooling
This comment has been minimized.
This comment has been minimized.
bric3
requested review from
claponcet,
dd-oleksii,
dudikeleti,
jandro996,
leoromanovsky and
ygree
and removed request for
a team
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
|
|
@amarziali Good spot, I'll update the doc. |
|
Sorry for the late feedback, I was struggling to find some time to get back at it.
Most of the existing modules will fit under lib, or api if they have a dedicated one. |
|
No problem @PerfectSlayer, thank you very much for the review.
I propose
That's a good point, at that time they are all under
Indeed those are covered.
Yes and no, while I was aiming for few conventions, it appeared to me that modules were sufficiently "different" in their purpose to warrant specific conventions. Also, e.g. those that apply these conventions:
Moreover, the end game it to remove script plugins, and by experience it's not a only have lower level (technical) convention plugins. THat's why identifying the purpose of a module is sensible. The project is large, and that is reflected in various purpose this project has. |
But it won't use the
Make sure to have a separate plugin for the ones under
I wonder if we should focus on setting modules up for where we want to go, or what we have right now? |
Yes, I was thinking like
I asked the same question, but I think it's gonna be easier to refactor if we have convention modules that identify modules for what they are today. Since they convey some expectation like you mentioned for components, we can easily spot modules for what they or what they aren't. |
Alright, just keep in mind we will have duplicate as |
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.
Rename the `sub-agent` module kind to `agent-product` and reserve it for shipped agent products. Add `bootstrap-component` and `testing-support` module kinds.
bric3
force-pushed
the
bdu/introduce-first-module-conventions
branch
from
2ea7e84 to
8beb2c7
Compare
Replace the `module.internal-component` module plugin with explicit internal module kinds. * Use `module.internal-api` for product and remote-config API surfaces. * Use `module.internal-library` for product implementation modules and shared internal libraries such as `:communication`, `:telemetry`, `:utils`, and agent helper modules. * Use `module.internal-platform-component` only for modules under `:components` so their stricter platform dependency and testing constraints can evolve separately. Currently, they still delegate to the shared Java convention; in essence this commit expand the module taxonomy precision, not the underlying build behavior.
bric3
force-pushed
the
bdu/introduce-first-module-conventions
branch
from
8beb2c7 to
0a0226c
Compare
…-module-conventions
…-module-conventions # Conflicts: # dd-smoke-tests/springboot-openliberty-20/build.gradle # dd-smoke-tests/springboot-openliberty-23/build.gradle
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-componentIt is for code whose artifact is intentionally part of the agent's bootstrap surface. It is expected to be included at the root of the final agent jar, currently directly through
shadowInclude.dd-trace-java/dd-java-agent/build.gradle
Lines 395 to 402 in 0a0226c
The classes in these modules (and their dependencies) are visible through the JVM bootstrap classloader once the agent jar is appended to the bootstrap search path. This the strictest category because the code can run very early and can be referenced from instrumented application classes.
Note, the dependencies of these modules do not use this convention.
The bootstrap modules and their dependencies happens to be excluded from the
shared/prefixdd-trace-java/dd-java-agent/build.gradle
Lines 267 to 282 in 0a0226c
dd-trace-java.module.distributable.api(here the dot is intended, as the idea is to have adistributable.agent)dd-trace-java.module.instrumentationdd-trace-java.module.internal-libraryThis is for internal implementation libraries. They are regular shared implementation code, generally intended to be consumed by other agent modules and often packaged inside
shared/,trace/, or a product-specific directory in the final jar. Some of them can still land at the jar root (bootstrap) transitively when a bootstrap component depends on them, for exampleinternal-apiormetrics-agent; that does not necessarily make them bootstrap components. It just means a bootstrap component needs them.dd-trace-java.module.internal-platform-componentIt is for low-level platform building blocks shared by several parts of the agent: context, json, annotations, HTTP primitives, etc. These modules are not agent
products and are not feature implementations. They are reusable infrastructure. They may end up bootstrap-visible when a bootstrap module depends on them, but that is a consequence of who consumes them.
dd-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]