Uh oh!
There was an error while loading. Please reload this page.
Make Config / InstrumenterConfig modifiable via a load-time test agent - #11397
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 8 commits intoMay 20, 2026
Conversation
…agent The JUnit 5 WithConfigExtension used ByteBuddy retransformation at @BeforeAll to make Config and InstrumenterConfig INSTANCE fields public/volatile/non-final. Retransformation is not guaranteed when the class has been loaded before the extension runs (e.g. through CoreTracer), which caused intermittent failures on at least HotSpot 1.8, IBM J9, and OpenJ9 Semeru jobs. This commit switches to a load-time javaagent that rewrites the INSTANCE fields upon class cload. It introduces in particular `dd-trace-java.modifiable-config` convention plugin whose role is to configure the test jvm with the java agent to every Test task. This convention plugin is applied automatically via dd-trace-java.configure-tests which is itself applied via java_no_deps.gradle. `WithConfigExtension` no longer installs a ByteBuddy agent programmatically; instead it just verifies the fields are already modifiable.
…ntees writable Config fields
With modifiable-config-agent attached to every test JVM, the `INSTANCE`
fields of `Config` and `InstrumenterConfig` are always
`public`/`volatile`/non-`final`
by the time `WithConfigExtension` is loaded. The runtime state
machine that previously guarded the ByteBuddy retransform is therefore
dead code.
Collapse it:
- Move reflective lookups of the INSTANCE fields and default constructors
into a static {} block that first calls
ensureConfigInstrumentationHasBeenApplied() and then caches the
handles as static finals. Failure throws ExceptionInInitializerError,
which surfaces as a clear test-class load failure pointing at the
Gradle convention plugin.
- Remove the isConfigInstanceModifiable / configModificationFailed /
configTransformerInstalled flags and every guard they fed.
- Drop makeConfigInstanceModifiable(), checkConfigTransformation(),
and checkWritable() — all redundant with the static-init check.
- Drop the corresponding JUnit assert* static imports.
- beforeAll/beforeEach/afterEach/afterAll now call rebuildConfig()
unconditionally.
- Update the class Javadoc to reflect the new mechanism.
Co-Authored-By: Claude <noreply@anthropic.com>[ci: NON_DEFAULT_JVMS] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
[ci: NON_DEFAULT_JVMS]
…avaagent
The `dd-trace-java.modifiable-config` convention plugin unconditionally
attached a second `-javaagent:modifiable-config-agent.jar` to every Test
JVM. That breaks the few modules whose tests already launch with the real
dd-java-agent attached (currently only `:dd-java-agent`'s integration
tests, via the `doFirst` at `dd-java-agent/build.gradle:454-457`):
* `AgentBootstrap.installAgentJar` tries the class' CodeSource first,
then `getAgentFileFromJavaagentArg`, then a ClassLoader-resource
lookup. On the failing matrix the CodeSource is null for the
bootstrap class, so it falls through to the `-javaagent:` arg.
* `getAgentFileFromJavaagentArg` refuses to pick when more than one
`-javaagent:` is present ("multiple javaagents specified") and
returns null.
* The ClassLoader fallback then throws `IllegalArgumentException: URI
is not absolute` for the bootstrap class URL.
Net effect: the tracing agent never installs and suites like
`OpenTracingTest` / `ShadowPackageRenamingTest` fail across non-default
JVMs (ibm8, semeru11/17, zulu11, tip).
Alternatives considered and rejected:
* Manifest scan in `getAgentFileFromJavaagentArg` to pick the entry
whose `Main-Class` is `AgentBootstrap`: meaningful bootstrap-
performance cost (open every `-javaagent:` jar, read MANIFEST.MF)
on a hot path that runs on every JVM startup.
* `-Ddd.agent.jar.path` / `DD_AGENT_JAR_PATH`: no equivalent knob
exists today, adds an attacker-controllable file path, and only
helps when the CLI is already ambiguous — not worth the surface.
Skipping is safe for `:dd-java-agent`'s tests: nothing under
`dd-java-agent/src/test` (Java or Groovy) references
`Config` / `InstrumenterConfig` `INSTANCE`, and nothing uses
`WithConfigExtension`. The tests rely on the real agent's startup to
populate `Config.INSTANCE`, not on swapping the singleton mid-test.
Implementation: defer the attach to a `doFirst` that inspects
`allJvmArgs` for an existing non-modifiable-config `-javaagent:` entry.
Gradle prepends each new `doFirst`, so the plugin's check runs after
the project's own agent-attach `doFirst` and sees the real arg before
the JVM forks. When skipping, emit an `INFO`-level log so the decision
is visible under `--info`.
[ci: NON_DEFAULT_JVMS]
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>bric3force-pushed
the
bdu/moves-with-config-extension-agent-to-commandline-via-convention-plugin
branch
from
May 18, 2026 12:21
6d3aea1 to
9a48fb5CompareConfig / InstrumenterConfig modifiable via a load-time test agentjpbempelforce-pushed
the
bdu/moves-with-config-extension-agent-to-commandline-via-convention-plugin
branch
7 times, most recently
from
May 19, 2026 08:55
b2ec041 to
4d1114eComparefilling the delaying buffer for test bufferFullYieldsImmediateWrite takes more than 5 seconds on ibm8 and CI [ci: NON_DEFAULT_JVMS]
jpbempelforce-pushed
the
bdu/moves-with-config-extension-agent-to-commandline-via-convention-plugin
branch
from
May 19, 2026 08:56
4d1114e to
eb901d7Comparejpbempel
marked this pull request as ready for review
May 19, 2026 11:05
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:eb901d7c1e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+29
to
+32
| val foreignAgent = allJvmArgs.firstOrNull { | ||
| it.startsWith("-javaagent:") && !it.contains("modifiable-config-agent") | ||
| } | ||
| if (foreignAgent != null) { |
There was a problem hiding this comment.
When a test task already adds the Datadog agent, this branch skips the only transformer that now makes Config/InstrumenterConfig.INSTANCE writable. I checked dd-java-agent/build.gradle lines 454-456, where every dd-java-agent test task adds -javaagent:${shadowJar...} in doFirst, and there are many @WithConfig JUnit tests under dd-java-agent/instrumentation; those tests will load WithConfigExtension, hit its new static check, and fail because the modifiable-config agent was deliberately not attached. The JVM supports multiple -javaagent arguments, so this should not skip just because another agent is present.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
…ommandline-via-convention-plugin
bric3
enabled auto-merge
May 19, 2026 18:37
jpbempel
approved these changes
May 20, 2026
Contributor
/merge |
View all feedbacks in Devflow UI.
The expected merge time in
|
github-merge-queueBot
removed this pull request from the merge queue due to failed status checks
May 20, 2026
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
gh-worker-dd-mergequeue-cf854dBot
deleted the
bdu/moves-with-config-extension-agent-to-commandline-via-convention-plugin
branch
May 20, 2026 06:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
Replaces the runtime ByteBuddy retransformation done by
WithConfigExtensionwith a load-time Java agent that rewrites theINSTANCEfield ofdatadog.trace.api.Configanddatadog.trace.api.InstrumenterConfigto bepublic/static/volatile/ non-finalbefore the config classes are first defined.It creates a new convention plugin
buildSrc/modifiable-config-agentthat applies an agent that modifies the modifiers ofINSTANCEfor these two config classes for each gradle test tasks. Every module that appliesjava_no_deps.gradlebenefit from it.WithConfigExtension:static {}block verifies the agent did its job (ensureConfigInstrumentationHasBeenApplied) and caches the reflective field/constructor.Supersedes #11388.
Motivation
WithConfigExtensionmade the singleton config classes mutable by using ByteBuddy retransformation from@BeforeAll. Retransforming a class that has already been loaded and touched is not guaranteed by the JVM — it depends on the vendor, the JVM version, and the redefinition capabilities advertised by the JVMTI environment. Several entry points (CoreTracer, etc.) touchConfigorInstrumenterConfigbeforeWithConfigExtensionis applied — for exampleParentBasedAlwaysOnSamplerTestbuilds aCoreTracerthat accessesInstrumenterConfigwithout ever activatingWithConfigExtension, so on a later test that relies onInstrumenterConfig.INSTANCEbeing instrumented, the agent failed silently, and was producing an error when the field was modified by the junit extension.This shows up in CI as recurring
test_basefailures on:test_flaky: [:baseTest, 3/4]test_base: [ibm8, 3/4]test_base: [semeru17, 3/4]test_base: [semeru8, 3/4]test_base: [semeru11, 3/4]Instead with a load-time agent that intercepts the class before it is defined, prevents scenarios were the config classes are loaded before
WithConfigExtension.Note
Regarding the
PendingTraceBufferTest#bufferFullYieldsImmediateWrite, it was found that on IBM J9 1.8 the test case fixture is actually slower than other JVMs, the buffer capacity is 4096, and this loop can take ~8s, we found that it was ok to just bump the timeout for this method.Additional Notes
Testtask already runs its tests in a separate JVM, so attaching-javaagentdo not create an addition JVM. Note theForkedTest(meansforkEvery = 1, that is for each test case a new JVM is created).WithConfigExtension: the agent rewrites the fields regardless of whether the test uses the extension. So tests likeParentBasedAlwaysOnSamplerTestnow run against a modifiable config, at build time the field remainsfinal.WithConfigExtensionfails early if for some reason the config instance or not "writable", and points to the agent / gradle convention plugin being not setup.call-site-instrumentation-plugin, however it's not the best practice here and a follow might be needed.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueSupersedes #11388.
Jira ticket: [PROJ-IDENT]