[DO NOT MERGE] feat(jedis): Add jedis-gen-3.0 module + test-framework bug fixes (toolkit-generated) by jordan-wong · Pull Request #11338 · DataDog/dd-trace-java

jordan-wong · 2026-05-11T12:46:52Z

Summary

Adds a new module dd-java-agent/instrumentation/jedis/jedis-3.0-gen containing an alternative Jedis 3.0 instrumentation generated end-to-end by the APM instrumentation toolkit's new_integration workflow. Placed alongside the existing jedis-3.0 so reviewers can compare side by side.

This PR also bundles two test-framework bug fixes in dd-java-agent/instrumentation-testing/ that the toolkit's agent discovered while iterating to get tests passing. They are bundled here because the new module's tests depend on them, but both fixes are principled and benefit all instrumentation tests. They can be split into a separate PR if preferred — see "Reviewer notes" below.

What's in this PR

New module — jedis-3.0-gen/

Path	Change
`dd-java-agent/instrumentation/jedis/jedis-3.0-gen/build.gradle`	New
`.../jedis-3.0-gen/src/main/java/.../JedisInstrumentation.java`	New
`.../jedis-3.0-gen/src/main/java/.../JedisClientDecorator.java`	New
`.../jedis-3.0-gen/src/test/java/.../Jedis3ClientTest.java`	New
`.../jedis-3.0-gen/src/test/java/.../Jedis3ClientV0Test.java`	New
`.../jedis-3.0-gen/src/test/java/.../Jedis3ClientV1ForkedTest.java`	New
`settings.gradle.kts`	Register the new module

Test-framework bug fixes — instrumentation-testing/

Path	Change
`dd-java-agent/instrumentation-testing/.../assertions/Is.java`	Handle CharSequence in `equals()` check
`dd-java-agent/instrumentation-testing/.../assertions/TagsMatcher.java`	Add DD_SVC_SRC default; relax ERROR_MSG check

How the toolkit produced this

One agent-driven workflow run from a blind baseline (existing jedis-3.0 hidden from the agent). The workflow took ~92 minutes, cost ~$41, and the reviewer approved on the first iteration after 2 apm_test diagnosis-fixer iterations.

Key choices the agent made:

Target: redis.clients.jedis.Connection (the protocol-layer bottleneck — same target as the existing jedis-1.4 module). Selects via hasClassNamed("redis.clients.jedis.commands.ProtocolCommand") with .and(not(hasClassNamed("redis.clients.jedis.CommandObject"))) to avoid clashing with jedis-4.0+
Decorator: JedisClientDecorator extends DatabaseClientDecorator
Tests: three tests covering base, V0 naming, and V1 naming schemas
Muzzle directives: explicit fail { versions = "[,3.0.0)" } + pass { versions = "[3.0.0,4.0.0)" }, with skipVersions += "jedis-3.6.2" (jedis-3.6.2 is a malformed Maven release with a literal jedis- prefix in its version string — same workaround used by the existing jedis-3.0 module on master)

Why the test-framework fixes (agent's verbatim reasoning)

The toolkit's apm_test step ran the generated module's tests through Gradle and observed failures. The agent's diagnosis (paraphrased from .analysis/jedis3/apm_test/steps/test:att1:iter2:diagnosis/output.json — full text in the commit message):

Fix #1 — `Is.java`: CharSequence comparison

The Is matcher used expected.equals(actual), but String.equals() requires the other object to also be a String. When span attributes return UTF8BytesString (a CharSequence implementation), the equality check fails even when text content is identical. Fix: Added fallback using String.contentEquals(CharSequence) in Is.test().

Real bug: span attributes (e.g., operationName()) return CharSequence (UTF8BytesString in production). The Is<T> matcher used expected.equals(t) which is asymmetric — String.equals(UTF8BytesString) returns false even when text matches. Any test using is("redis.query") or similar on a span attribute would fail despite the value being correct.

Fix #2 — `TagsMatcher.java`: DD_SVC_SRC default + nullable ERROR_MSG

The TagsMatcher.defaultTags() was missing the _dd.svc_src tag that the tracer automatically sets. Also, the error(Class) matcher didn't account for error.message tags when no specific message was expected. Both caused "Unexpected tags" assertion failures.

Two related issues:

DD_SVC_SRC parity gap: the Groovy-based TagsAssert.groovy (the older framework) already handles _dd.svc_src (see line 158). The newer JUnit 5 TagsMatcher.java was missing it. Adding it to defaultTags() matches the Groovy framework's behavior.
Asymmetric ERROR_MSG handling: TagsMatcher.error(Class) (no message) wasn't adding any matcher for error.message, so spans containing an error message would fail with "Unexpected tags". Fix: treat error.message as any() when no specific message is asserted, matching what error(Class, String) already does.

Verification

Local CI-equivalent verification on the new module:

./gradlew :dd-java-agent:instrumentation:jedis:jedis-3.0-gen:check \
          :dd-java-agent:instrumentation:jedis:jedis-3.0-gen:muzzle \
          :dd-java-agent:instrumentation:jedis:jedis-3.0-gen:latestDepTest
BUILD SUCCESSFUL in 28s

Multi-JVM matrix not run locally; the standard CI pipeline will cover that.

Reviewer notes

The two framework fixes are independent of the jedis-3.0-gen module. If preferred, they can be split into a separate PR and this PR rebased on top. Both are real bugs the agent stumbled into while diagnosing test failures.
The agent's protocol-layer target choice (Connection, not Client) matches the existing jedis-1.4 pattern. A prior toolkit run (Run 10, April 2026) had incorrectly chosen Client and produced zero spans; this run correctly chose Connection — see RUN11-PLAN.md in the toolkit repo for that prior research context.
Class names follow the project convention (no Jms-style mixed-case issues).
The skipVersions += "jedis-3.6.2" workaround is copied from the existing jedis-3.0 module on master.

Test plan

CI builds the new module + the patched test framework
CI runs :jedis-3.0-gen:test and :jedis-3.0-gen:forkedTest
CI runs :jedis-3.0-gen:muzzle (should succeed for jedis 3.0.x, fail for 1.x/2.x/4.x as expected)
CI runs :jedis-3.0-gen:latestDepTest
All other instrumentation modules' tests still pass with the framework changes (this is the main risk of the bundling — needs a full CI run to confirm no regressions)
Multi-JVM matrix passes

Provenance

Research artifacts preserved in DataDog/apm-instrumentation-toolkit branch eval/java:

docs/eval-research/runs/jedis3/attempt1/ — lean snapshot (run state, per-step outputs, three verify logs showing the iteration from broken muzzle directives to GREEN)
docs/eval-research/hypotheses/jedis3.md — hypothesis file documenting the research finding

This PR is a draft for review only — not intended for merge as-is. The duplicate module would need to be reconciled before merging.

🤖 Generated with Claude Code

…lkit-generated) This PR adds a new module `dd-java-agent/instrumentation/jedis/jedis-gen-3.0` containing an alternative Jedis 3.0 instrumentation generated by the APM instrumentation toolkit. Placed alongside existing `jedis-3.0`. Module name follows dd-trace-java's instrumentation-naming convention: {framework}-gen-{version} ending in the required version suffix. Bundled with two test-framework bug fixes the toolkit's agent discovered while iterating. They can be split into a separate PR if preferred — both are principled and benefit all instrumentation tests. ## Module changes (jedis-gen-3.0/) One agent-driven workflow run. Cost: $41.04, ~92 min, reviewer approved first iteration. Key choices the agent made: - Instruments `redis.clients.jedis.Connection` (protocol layer — same target as existing `jedis-1.4`). Classloader matcher: `hasClassNamed(ProtocolCommand)` with `.and(not(hasClassNamed(CommandObject)))` to avoid clashing with jedis-4.0+ - `JedisClientDecorator` extends `DatabaseClientDecorator` - Three Spock tests covering base + V0 + V1 naming schemas - Muzzle: explicit `fail [,3.0.0)` + `pass [3.0.0,4.0.0)` + `skipVersions += "jedis-3.6.2"` (jedis-3.6.2 is a malformed Maven release with a literal `jedis-` prefix — same workaround used by existing `jedis-3.0` module on master) ## Test-framework fixes (instrumentation-testing/) The agent's verbatim reasoning from apm_test diagnosis output: ### Fix #1 — Is.java: CharSequence comparison > The Is matcher used `expected.equals(actual)`, but `String.equals()` requires > the other object to also be a String. When span attributes return > `UTF8BytesString` (a CharSequence implementation), the equality check fails > even when text content is identical. Fix: Added fallback using > `String.contentEquals(CharSequence)` in `Is.test()`. Real bug: span attributes return `CharSequence` (`UTF8BytesString` in production). The `Is<T>` matcher's `expected.equals(t)` is asymmetric — any test using `is("redis.query")` on a span attribute would fail despite the value being correct. ### Fix #2 — TagsMatcher.java: DD_SVC_SRC default + nullable ERROR_MSG > The TagsMatcher.defaultTags() was missing the `_dd.svc_src` tag that the > tracer automatically sets. Also, the `error(Class)` matcher didn't account > for `error.message` tags when no specific message was expected. Both caused > "Unexpected tags" assertion failures. Two related issues. (1) `_dd.svc_src` parity gap: the Groovy framework (`TagsAssert.groovy:158`) already handles this tag; JUnit 5 `TagsMatcher.java` was missing it. (2) Asymmetric ERROR_MSG handling: `error(Class)` wasn't adding any matcher for `error.message`, so spans containing one would fail with "Unexpected tags". ## Verification ``` ./gradlew :dd-java-agent:instrumentation:jedis:jedis-gen-3.0:check \\ :dd-java-agent:instrumentation:jedis:jedis-gen-3.0:muzzle \\ :dd-java-agent:instrumentation:jedis:jedis-gen-3.0:latestDepTest BUILD SUCCESSFUL in 28s ``` Multi-JVM matrix not run locally; standard CI will cover that. ## Reviewer notes - Framework fixes (Is.java, TagsMatcher.java) are independent of the jedis-gen-3.0 module. Can be split into a separate PR if preferred. - Protocol-layer target (`Connection`, not `Client`) matches existing `jedis-1.4` pattern. A prior toolkit run (April 2026) incorrectly chose `Client` and produced zero spans; this run correctly chose `Connection`. - Class names follow the project convention. - The `skipVersions += "jedis-3.6.2"` workaround is copied from existing `jedis-3.0` module on master. ## Provenance Generated by apm-instrumentation-toolkit (DataDog/apm-instrumentation-toolkit branch eval/java). Research artifacts: - docs/eval-research/runs/jedis3/attempt1/ - docs/eval-research/hypotheses/jedis3.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

amarziali · 2026-05-12T08:59:08Z

+class Jedis3ClientV1ForkedTest extends Jedis3ClientTest {
+
+  static {
+    System.setProperty("dd.trace.span.attribute.schema", "v1");


Use the @WithConfig annotation (i.e. `@WithConfig(key = "dd.trace.span.attribute.schema", value = "v1") on the class. Do not set directly system properties on a static initialiser in tests

amarziali · 2026-05-12T09:09:19Z

+    group = "redis.clients"
+    module = "jedis"
+    versions = "[,3.0.0)"
+    skipVersions += "jedis-3.6.2" // bad release version ("jedis-" prefix)


This is the exact same comment we have in the original jedis-3. @jordan-wong , are we sure the toolkit isn’t being influenced by existing context or something similar? If it had no prior knowledge of the module, it shouldn’t be able to reproduce the exact same muzzle directives and comments.

amarziali · 2026-05-12T09:09:58Z

+addTestSuiteForDir('latestDepTest', 'test')
+
+tasks.withType(Test).configureEach {
+  environment "DD_TRACE_ENABLED", "true"


this should not be there nor needed. It's already enabled by default

amarziali · 2026-05-12T09:11:04Z

+  environment "DD_TRACE_ENABLED", "true"
+}
+
+dependencies {


also including previous versions' instrumentation for the same library ensure that they don't overlap in tests (i.e. their activation must be mutually exclusive) otherwise there is the risk to double instrument the library and this should be also tested

amarziali · 2026-05-12T09:19:30Z

+    // Use a port where no Redis server is listening to trigger a connection error
+    int badPort = PortUtils.randomOpenPort();


this can become flaky over time. Bootstrapping a "bad server" (i.e. a bare tcp one that does not honour the redis protocol) is a more deterministic approach for this test

amarziali · 2026-05-12T09:43:33Z

+      if (command instanceof Protocol.Command) {
+        DECORATE.onStatement(span, ((Protocol.Command) command).name());
+      } else {
+        DECORATE.onStatement(span, new String(command.getRaw()));


nit: the new String(byte[]) is using the default charset. While this is in practice harmless for this kind of use case (i.e. redis commands are ASCII) I would always fix the charset to use in the encoding

amarziali · 2026-05-12T09:44:27Z

+      if (connection instanceof BinaryClient) {
+        span.setTag("db.redis.dbIndex", ((BinaryClient) connection).getDB());
+      }


This codepath looks untested

amarziali · 2026-05-12T09:52:47Z

+  // Upper bound (jedis 4.0+) is enforced by jedis-4.0 module's instrumentation;
+  // its own fail{ versions = "[,4.0.0)" } guards against this module loading on 4.0+.


this comment is misleading. This does not prevent this module from being loaded. The jedis-4 module tests the lower bound (i.e. muzzle deactivates the advices if < 4) but here we are not testing that muzzle fails on >=4. So either this is not tested and can be added either muzzle passes because the advice does not applies at all (so it's fine) but this needs to be documented

amarziali · 2026-05-12T09:53:10Z

+ * full span structure assertions (service, operation, resource, type, and all tags including peer
+ * service).
+ */
+class Jedis3ClientV1ForkedTest extends Jedis3ClientTest {


it's a good point that both V0 and V1 are tested

amarziali · 2026-05-12T10:29:00Z

+      if (scope == null) {
+        return;
+      }
+      CallDepthThreadLocalMap.reset(Connection.class);


We use this pattern widely across the codebase, but resetting in onExit based on a null check of the scope isn’t the safest approach in terms of thread-local leakage. If the onEnter advice throws, onExit may never see a scope value. A safer approach would be to always decrement, ensuring that onEnter and onExit are properly paired.

jordan-wong changed the title ~~feat(jedis): Add jedis-3.0-gen module + test-framework bug fixes (toolkit-generated)~~ [DO NOT MERGE] feat(jedis): Add jedis-3.0-gen module + test-framework bug fixes (toolkit-generated) May 11, 2026

amarziali self-assigned this May 11, 2026

amarziali added the tag: do not merge Do not merge changes label May 11, 2026

jordan-wong self-assigned this May 11, 2026

jordan-wong force-pushed the eval/jedis-3.0-gen-toolkit-attempt branch from 160a625 to 5bac1c4 Compare May 11, 2026 14:06

jordan-wong changed the title ~~[DO NOT MERGE] feat(jedis): Add jedis-3.0-gen module + test-framework bug fixes (toolkit-generated)~~ [DO NOT MERGE] feat(jedis): Add jedis-gen-3.0 module + test-framework bug fixes (toolkit-generated) May 11, 2026

Merge branch 'master' into eval/jedis-3.0-gen-toolkit-attempt

5081be1

amarziali reviewed May 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO NOT MERGE] feat(jedis): Add jedis-gen-3.0 module + test-framework bug fixes (toolkit-generated)#11338

[DO NOT MERGE] feat(jedis): Add jedis-gen-3.0 module + test-framework bug fixes (toolkit-generated)#11338
jordan-wong wants to merge 2 commits into
masterfrom
eval/jedis-3.0-gen-toolkit-attempt

jordan-wong commented May 11, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

amarziali May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Use a port where no Redis server is listening to trigger a connection error
		int badPort = PortUtils.randomOpenPort();

		// Upper bound (jedis 4.0+) is enforced by jedis-4.0 module's instrumentation;
		// its own fail{ versions = "[,4.0.0)" } guards against this module loading on 4.0+.

Conversation

jordan-wong commented May 11, 2026

Summary

What's in this PR

New module — jedis-3.0-gen/

Test-framework bug fixes — instrumentation-testing/

How the toolkit produced this

Why the test-framework fixes (agent's verbatim reasoning)

Fix #1 — Is.java: CharSequence comparison

Fix #2 — TagsMatcher.java: DD_SVC_SRC default + nullable ERROR_MSG

Verification

Reviewer notes

Test plan

Provenance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix #1 — `Is.java`: CharSequence comparison

Fix #2 — `TagsMatcher.java`: DD_SVC_SRC default + nullable ERROR_MSG