feat(feign-10.8): toolkit-generated feign HTTP client instrumentation by jordan-wong · Pull Request #11709 · DataDog/dd-trace-java

jordan-wong · 2026-06-23T13:47:35Z

Summary

Toolkit-generated greenfield instrumentation for Feign HTTP client (io.github.openfeign:feign-core 10.8+). Part of the dd-trace-java APM toolkit eval program. ⚠️ DO NOT MERGE. Open for review;
reviewer feedback feeds back into toolkit R-rules.

Wins from prior reviewer feedback

@mcculls reviewed the prior toolkit feign attempt on #10579 (CLOSED). This regeneration addresses three of his key concerns:

✅ Lambda-in-advice replaced with named helper class (SpanFinishingCallback) — fixes invokedynamic-on-unprepared-class footgun
✅ @Advice.Return(readOnly = false) CompletableFuture future correctly assigned back via future = future.whenComplete(...)
✅ Async CallDepthThreadLocalMap.reset(...) happens on the calling thread (not the completion thread)
## Regressions / concerns vs prior review and master conventions

build.gradle:1-7 — assertInverse = true dropped from muzzle. @mcculls explicitly praised this on DRAFT: APM AI TOOLKIT New Integration: Feign Client Instrumentation #10579 ("something people often forget"). Clean regression.
Two @autoservice(InstrumenterModule.class) classes (FeignClientInstrumentation + FeignAsyncClientInstrumentation) instead of one aggregator FeignModule with typeInstrumentations(). @mcculls
asked for the ReactorCoreModule pattern explicitly on DRAFT: APM AI TOOLKIT New Integration: Feign Client Instrumentation #10579.
AsyncClient.Pseudo double-span. Sync advice uses Client.class as the CallDepthThreadLocalMap key; async uses AsyncClient.class. AsyncClient.Pseudo delegates to Client.Default — different
keys → two HTTP spans per request. Tests assert this rather than fix it. Use a shared sentinel.
RequestHeaderInjectAdapter.inject() hardcodes StandardCharsets.UTF_8 — drops original.charset(). Non-UTF-8 callers silently get bodies rewritten.
FeignClientDecorator missing sourceUrl(...) and service() overrides — convention drift vs OkHttpClientDecorator / ApacheHttpClientDecorator.
Span lifecycle order inverted at FeignClientInstrumentation.java:109-112 — scope.close() before span.finish(). Skill convention (R8) is the inverse.
DD_TRACE_FEIGN_ENABLED defaults to false — most HTTP-client integrations default-enabled.
Tests bypass shared HttpClientTest base — bespoke com.sun.net.httpserver. Loses cross-tracer behavioral parity.

What was structurally correct

HttpClientDecorator<Request, Response> shape matches master HTTP-client conventions. Module path dd-java-agent/instrumentation/feign/feign-10.8-generated/ follows version-suffix + parent-dir
naming.

CI

All substantive jobs pass (build, instrumentationTest, latestDepTest, muzzle 8/8, smokeTest, profiling). Only failing check is dd-gitlab/validate_supported_configurations_v2_local_file —
external registry validator requires DD_TRACE_FEIGN_* keys to be pre-registered out-of-band. Not a code issue.

What reviewers should do

Decide on the aggregator-module pattern and the Pseudo double-span fix (both rooted in @mcculls's prior feedback).
Flag charset-loss and default-enabled concerns for product decision.
Coordinate registry registration before merge.

Toolkit-generated greenfield instrumentation for Feign HTTP client v10.8+. Run details: - Branch (toolkit): eval/java @ 757979b9 - Branch (worktree): eval/feign-blind-attempt1-20260623 in /Users/jordan.wong/dd-trace-java-eval-feign - Workflow: new_integration, completed 2026-06-23 - Cost: $26.95, 78min, reviewer approved=True - Maven coordinates: io.github.openfeign:feign-core:10.8 Generated module: dd-java-agent/instrumentation/feign/feign-10.8-generated/ - FeignClientInstrumentation (sync Client.execute) - FeignAsyncClientInstrumentation (async AsyncClient.execute) - FeignClientDecorator (HttpClientDecorator) - RequestHeaderInjectAdapter (header injection for distributed tracing) - SpanFinishingCallback (async future completion) - Java tests (FeignClientTest, FeignAsyncClientTest) per R20 Supersedes #10855 (Feign 8.0, stale) and closed #10980 (Feign 10.8, never independently verified). ## Research integrity note `metadata/supported-configurations.json` (4 entries: DD_TRACE_FEIGN_10_8_ENABLED, DD_TRACE_FEIGN_ANALYTICS_ENABLED, DD_TRACE_FEIGN_ANALYTICS_SAMPLE_RATE, DD_TRACE_FEIGN_ENABLED) was **hand-added** by the human operator AFTER the toolkit run; the toolkit's R29 rule was added later in the same session (see commit 757979b9 on eval/java). This is documented in docs/eval-research/generated/feign-20260623/caveats.md. `settings.gradle.kts` line addition is a faithful replay of what the toolkit produced in the eval worktree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dd-octo-sts · 2026-06-23T14:14:34Z

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite	Status
Startup	🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results

Scenario	Candidate	master	Δ (95% CI of mean)
startup:insecure-bank:iast:Agent	13.95 s	13.93 s	[-0.6%; +0.8%] (no difference)
startup:insecure-bank:tracing:Agent	12.92 s	12.97 s	[-1.1%; +0.2%] (no difference)
startup:petclinic:appsec:Agent	16.86 s	16.52 s	[+1.0%; +3.1%] (significantly worse)
startup:petclinic:iast:Agent	16.95 s	16.96 s	[-1.0%; +0.8%] (no difference)
startup:petclinic:profiling:Agent	16.76 s	16.95 s	[-2.2%; -0.2%] (maybe better)
startup:petclinic:sca:Agent	16.97 s	16.88 s	[-0.6%; +1.7%] (no difference)
startup:petclinic:tracing:Agent	15.96 s	16.07 s	[-1.7%; +0.3%] (no difference)

Commit: 1a6fd6d2 · CI Pipeline · Benchmarking Platform UI

Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

… name The checkDecoratorAnalyticsConfigurations task requires ANALYTICS entries for each instrumentation name returned by Decorator.instrumentationNames(), not just the shared name. FeignClientDecorator returns ['feign', 'feign-10.8']; the 'feign' analytics keys were already added, but 'feign-10.8' was missing. Adds: - DD_TRACE_FEIGN_10_8_ANALYTICS_ENABLED - DD_TRACE_FEIGN_10_8_ANALYTICS_SAMPLE_RATE (type: decimal) Locally verified ./gradlew checkConfigurations BUILD SUCCESSFUL. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

R31 — drop assertInverse from muzzle: versions = '[10.8,)' with assertInverse = true was failing because feign 10.6.0 actually passes muzzle (FeignClientInstrumentation sync class doesn't need 10.8 features; only FeignAsyncClientInstrumentation does). assertInverse asserts inverse versions FAIL muzzle — but 10.6.0 PASSES, so the auto-test failed. Conservative omission of assertInverse is correct here. The min version remains 10.8 (compileOnly + muzzle pass range). Fixes: muzzle: [2/8] CI failure (muzzle-AssertFail-io.github.openfeign-feign-core-10.6.0). R32 — '-generated' exclusion: Added 'feign-10.8-generated' to instrumentationNaming.exclusions in dd-java-agent/instrumentation/build.gradle. Fixes check-instrumentation-naming. Local verification: ./gradlew :dd-java-agent:instrumentation:feign:feign-10.8-generated:muzzle BUILD SUCCESSFUL ./gradlew checkInstrumentationNaming BUILD SUCCESSFUL Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Spotless moved the trailing comment 'org-json does not use semver' to its own line. Fixes dd-gitlab/spotless CI failure introduced by the R32 edit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

jordan-wong · 2026-06-24T14:01:28Z

Only fails CI check dd-gitlab/validate_supported_configurations_v2_local_file because of config keys arent registered as this is a completely new instrumentation. We can assume CI checks are passing

dd-octo-sts · 2026-06-25T00:36:37Z

Hi! 👋 Thanks for your pull request! 🎉

To help us review it, please make sure to:

Add at least one type, and one component or instrumentation label to the pull request

If you need help, please check our contributing guidelines.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1a6fd6d284

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-25T00:43:41Z

+      }
+
+      if (future != null) {
+        future = future.whenComplete(new SpanFinishingCallback(span));


Preserve cancellation on async Feign futures

When an AsyncClient implementation returns a cancellable CompletableFuture (for example Feign's async clients use cancellation to abort in-flight requests), replacing it with the dependent stage from whenComplete breaks that contract: cancelling the future returned to user code only cancels the dependent stage and leaves the original request running. Register the callback without assigning it back, or otherwise propagate cancellation to the original future.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-06-25T00:43:41Z

+    implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice {
+
+  public FeignClientInstrumentation() {
+    super("feign", "feign-10.8");


Keep generated Feign instrumentation opt-in

This generated/eval module is documented in the new metadata as disabled by default, but InstrumenterModule.Tracing uses defaultEnabled() from the base class unless it is overridden, so this constructor enables the sync and async Feign instrumentations for every Feign 10.8+ application by default. If this module is meant to be opt-in, override defaultEnabled() in both instrumentation classes; otherwise the metadata default is misleading and users will get spans unless they explicitly opt out.

Useful? React with 👍 / 👎.

ValentinZakharov · 2026-07-06T21:29:09Z

+    implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice {
+
+  public FeignAsyncClientInstrumentation() {
+    super("feign", "feign-10.8");


Since there's only one Feign module, the second name is redundant - super("feign") is enough. Each name here becomes its own enable flag (DD_TRACE_FEIGN_ENABLED + DD_TRACE_FEIGN_10_8_ENABLED), so the extra name just doubles the config surface for no gain. The group-name + versioned-name form (like super("okhttp", "okhttp-3")) is only needed when multiple version-specific modules exist and we want one shared switch across them.

ValentinZakharov · 2026-07-06T21:57:47Z

+      {
+        "version": "A",
+        "type": "boolean",
+        "default": "false",


All our HTTP client integrations default to true (see e.g. DD_TRACE_OKHTTP_ENABLED), so this should be true as well. This field must also match the code: the module doesn't override defaultEnabled(), so it's actually enabled by default - false here is inconsistent. If we do want Feign off by default, override defaultEnabled() to return false in both instrumentation classes.

ValentinZakharov · 2026-07-06T22:11:38Z

+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static AgentScope methodEnter(
+        @Advice.Argument(value = 0, readOnly = false) Request request) {
+      final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(AsyncClient.class);


Instrumenting both the async and sync layers produces two spans per request, and call-depth can't reliably dedupe them.

AsyncClient.Pseudo delegates synchronously to Client.Default, which FeignClientInstrumentation also instruments. Since async increments AsyncClient.class and sync increments Client.class, the counters are independent - the inner Client.execute still sees depth 0 and starts its own span.

Since every Feign call ultimately goes through feign.Client.execute, the cleaner design is to instrument a single layer - most likely just the sync Client - giving exactly one span in all cases except a fully custom async client that bypasses feign.Client.

…ep 4.4 Port reviewer-encoded rules from the toolkit's synced derivative (`apm-integrations` skill in DataDog/apm-instrumentation-toolkit) back to the canonical `add-apm-integrations` skill. The toolkit accumulated 21+ numbered reviewer rules during eval research (commits flowing through PR #11337 jedis-3.0, PR #11562 sparkjava-2.3, PR #11717 commons-httpclient, PR #11709 feign, and PR #11506 RxJava 3). These rules encode real failure modes observed during agent-generated PRs and the fixes reviewers asked for. Sections added: - **Step 4.4** — Library category: span-creating vs context-propagation. New axis that routes Cat B libraries (reactive, async, executors, futures, actors) into a `context_propagation` codegen path producing `InstrumenterModule.ContextTracking` instead of span-creating advice. Includes the Flowable subscribe(FlowableSubscriber) overload rule (hook the framework-internal overload, not the public wrapper). - **Step 4.5** — Java naming consistency (module-name conventions). R-rule placements: - Step 4: R32 (dir name must end with version OR allowed suffix) - Step 5: R13 (no single-type helper class for CallDepthThreadLocalMap) R30 (preserve master's integration name when regenerating) - Step 7: R15/R16/R17 (single delegate method, not all overloads) R33 (no NullPointerException catches; use null-check guards) - Step 9: R14 (test error/exception scenarios + spotless) R18 (muzzle excludes incompatible majors) R19 (latestDepTestImplementation range matches instrumented) R20 (Java tests only; no new .groovy files) R28 (compileOnly/testImplementation version split rationale) R29 (register names in supported-configurations.json) R31 (assertInverse only when declared min is true min) - Step 4.4: R21-R27 (the Cat A vs Cat B classification + Cat B schema) This PR pairs with toolkit-side PR DataDog/apm-instrumentation-toolkit#472 which adds the same content to the toolkit's synced derivative copy. Both copies should stay in sync — this is the canonical home. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com>

@mcculls

…o sub references (#11760) skill(add-apm-integrations): R13-R33 + Cat B (context-propagation) Step 4.4 Port reviewer-encoded rules from the toolkit's synced derivative (`apm-integrations` skill in DataDog/apm-instrumentation-toolkit) back to the canonical `add-apm-integrations` skill. The toolkit accumulated 21+ numbered reviewer rules during eval research (commits flowing through PR #11337 jedis-3.0, PR #11562 sparkjava-2.3, PR #11717 commons-httpclient, PR #11709 feign, and PR #11506 RxJava 3). These rules encode real failure modes observed during agent-generated PRs and the fixes reviewers asked for. Sections added: - **Step 4.4** — Library category: span-creating vs context-propagation. New axis that routes Cat B libraries (reactive, async, executors, futures, actors) into a `context_propagation` codegen path producing `InstrumenterModule.ContextTracking` instead of span-creating advice. Includes the Flowable subscribe(FlowableSubscriber) overload rule (hook the framework-internal overload, not the public wrapper). - **Step 4.5** — Java naming consistency (module-name conventions). R-rule placements: - Step 4: R32 (dir name must end with version OR allowed suffix) - Step 5: R13 (no single-type helper class for CallDepthThreadLocalMap) R30 (preserve master's integration name when regenerating) - Step 7: R15/R16/R17 (single delegate method, not all overloads) R33 (no NullPointerException catches; use null-check guards) - Step 9: R14 (test error/exception scenarios + spotless) R18 (muzzle excludes incompatible majors) R19 (latestDepTestImplementation range matches instrumented) R20 (Java tests only; no new .groovy files) R28 (compileOnly/testImplementation version split rationale) R29 (register names in supported-configurations.json) R31 (assertInverse only when declared min is true min) - Step 4.4: R21-R27 (the Cat A vs Cat B classification + Cat B schema) This PR pairs with toolkit-side PR DataDog/apm-instrumentation-toolkit#472 which adds the same content to the toolkit's synced derivative copy. Both copies should stay in sync — this is the canonical home. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> skill(add-apm-integrations): clean up R-numbering and sub-step naming Two cosmetic passes on the rule encoding ported from the toolkit's synced derivative copy. Substantive content unchanged. Pass 1 — strip R-numbering. The R13-R33 numbers are toolkit-internal traceability tags that tie each rule to a specific reviewer comment on a specific generated-PR review. They are meaningful to the toolkit-side eval research workflow but meaningless to readers of the canonical skill, who have no R1-R12 context here. Removes: - '#### R<NN> — <title>' prefixes (14 sub-section headings) - Inline '(R<NN>)', '(R<NN>/R<NN>/R<NN>)' parentheticals on existing bullets that already convey the rule in their wording - A stale cross-reference 'see R29 below' rewritten to 'see "Register new integration names"' The toolkit-side copy keeps the R-numbering — it remains the eval-research home where the traceability matters. This is a one-way port convention. Pass 2 — renumber half-steps so the decimals make sense. Was: Step 4 → Step 4.4 → Step 4.5 → Step 5 implying missing 4.1, 4.2, 4.3. The original numbering was an artifact of an earlier toolkit-side draft that had a 4.1-4.4 enumeration which got collapsed. Renumbered to: Step 4 → Step 4.1 → Step 4.2 → Step 5 Main integer steps (1-12) unchanged. Step 7.1 (Multiple advice classes and @AppliesOn) left as-is since its decimal already makes sense relative to Step 7. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> refactor(skill): extract Category B routing to references/ Move the 62-line "Step 4.1 – Library category" section from SKILL.md into references/category-b-context-propagation.md. SKILL.md keeps a 5-line stub linking to the reference file — enough context to know when to read it, not enough to bury the rest of Step 4. Preserves all content verbatim; no wording changes. Follows the existing dd-trace-java skill convention of tracking specific per-skill files (git add -f, matching the precedent set by .claude/skills/migrate-groovy-to-java/QUALITY_RULES.md). Part 1 of a refactor to slim the 794-line SKILL.md into a routing overview (~250 lines target) with topic-oriented reference files. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> refactor(skill): extract naming conventions to references/ Move two related naming rules from SKILL.md into references/naming-conventions.md: - Step 4's module-directory-name rule (must end with version or "-common"/"-stubs"/"-iast" suffix) - Step 4.2's Java filename ↔ class-name matching rule (with the sanity-check script) They belong together because both are enforcement rules for names. SKILL.md keeps short stubs linking to the reference file. Preserves all content verbatim. Part 2 of the SKILL.md slim. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> refactor(skill): extract InstrumenterModule guidance to references/ Move Step 5's 103-line body from SKILL.md into references/instrumenter-module.md, covering: - @autoservice + narrow-interface preferences (ForSingleType > ForKnownTypes > ForTypeHierarchy) with the interface-only API JAR exception (JMS, JPA, JDBC, etc.) - 'Must NOT do' — no static constants for one-shot methods - instrumentationNames() version-qualified alias rule - No helper class for single-target CallDepthThreadLocalMap - Preserve master's integration name on regeneration - Advanced: grouping multiple instrumentations under one module SKILL.md keeps Step 5 as a 6-line summary + link. Preserves all content verbatim; no wording changes. Part 3 of the SKILL.md slim. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> refactor(skill): extract Advice class guidance to references/ Move Step 7's 149-line body from SKILL.md into references/advice-class.md. This was the largest single-step body in the skill and the highest-risk area to get wrong. Reference file covers: - Advice method annotations + parameter kinds - Span lifecycle (enter/exit order) - onExit resilience to onEnter throwing - Explicit charset for byte[] to String - No NullPointerException catches (SpotBugs enforces) - Single-delegate-method instrumentation (not all overloads) - @AppliesOn + multiple advice classes - 'Must NOT do' list (no loggers, no lambdas, no inline=false, etc.) SKILL.md keeps Step 7 as a summary + link. Preserves all content verbatim. Part 4 of the SKILL.md slim. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> refactor(skill): extract tests guidance to references/ Move Step 9's 'Instrumentation test' section + all its sub-rules (no .groovy files, supported-configurations.json registration, compileOnly/testImplementation version-split rationale, prior-version-module inclusion) from SKILL.md into references/tests.md. Muzzle content stays in place for now — it's a separate concern and gets its own reference file next. SKILL.md keeps Step 9.1 as a summary + link. Preserves all content verbatim. Part 5 of the SKILL.md slim. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> refactor(skill): extract muzzle guidance to references/ Move Step 9's 'Muzzle directives' section + all its sub-rules (assertInverse gotchas, incompatible-major-version exclusion, skipVersions for malformed release versions) from SKILL.md into references/muzzle.md. SKILL.md keeps Step 9.2 as a summary + link. Preserves all content verbatim; no wording changes. Part 6 of the SKILL.md slim. Final state: SKILL.md 794 → ~215 lines, split into 6 topic-oriented reference files under references/. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> style(skill): add missing blank lines before Step/subsection headings Cosmetic fixup after the section extractions. Three headings lost their preceding blank line during the awk-based edits — restoring them so the rendered Markdown reads cleanly. No content changes. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> refactor(skill): drop toolkit-internal 'Category A/B' language The 'Category A' / 'Category B' labels came from toolkit-side research where they were shorthand for the 'target_kind' Pydantic enum values. They have no meaning in dd-trace-java on their own — a contributor reading the skill has no context for what 'Category B' refers to. Replace with the descriptive terms that already exist in dd-trace-java: - 'span-creating instrumentation' — extends InstrumenterModule.Tracing - 'context-tracking instrumentation' — extends InstrumenterModule.ContextTracking (matches the class name + TargetSystem.CONTEXT_TRACKING enum) Changes: - Rename references/category-b-context-propagation.md → references/context-tracking.md - Rewrite Step 4.1 stub in SKILL.md to drop Category A/B and 'target_kind' - Rewrite context-tracking.md body from 'Category B target shape' Pydantic- field enumeration to 'What a context-tracking instrumentation captures', described in Java terms (boundary type, capture/restore points, wrapper class, wrapper methods) instead of toolkit Pydantic field names - Fix advice-class.md's stray 'context-propagation logic' → 'context-tracking logic' to match dd-trace-java's TargetSystem.CONTEXT_TRACKING naming No substantive guidance changed. Reference still points at rxjava-2.0 as the canonical example. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> refactor(skill): remove toolkit-workflow language from reference files Two remaining spots reframed from LLM-agent-workflow perspective to dd-trace-java human-contributor perspective: - muzzle.md 'Background' paragraph: 'a typical greenfield generation produces...' + 'the agent picks the higher version...' → 'this failure mode is common when a module has both a sync and async instrumentation class' + 'declaring the higher version as the muzzle min...'. Same technical content, no LLM-agent workflow assumption. - tests.md 'How to discover' step: 'run the sample app' → 'run your instrumentation test'. 'Sample app' was ambiguous ('the toolkit's sample-app workflow step' vs 'your own test app'); the concrete dd-trace-java term is 'instrumentation test'. No substantive guidance changed. Preserves all rules verbatim. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> fix(skill): address review comments on #11760 Nine fixes from Copilot bot + @mcculls review comments: SKILL.md - Step 4 source layout: 'src/test/groovy/ — Spock tests' → 'src/test/java/ — JUnit 5 tests'. Contradicted Step 9.1's Java-only policy. (Copilot) references/tests.md - Rewrite the error-test example: 'List<List<SpanData>> traces = ...' used OpenTelemetry's SpanData type (won't compile against dd-trace-java's TEST_WRITER, which returns List<List<DDSpan>>). Now uses AgentSpan and span.getTag() per mcculls's guidance that AgentSpan is enough for tests. - Replace 'checkNewGroovyFiles' (unverifiable bot name) with the real workflow: 'Enforce Groovy Migration' (.github/workflows/enforce-groovy-migration.yaml). Both places. - Default value in supported-configurations.json: change 'false' to 'true' per mcculls — ~83% of typical integrations default to true; 'false' is reserved for modules that override defaultEnabled() (OpenTelemetry, Hazelcast, sparkjava). Add a note calling out the branching. references/naming-conventions.md - Remove gRPCInstrumentation as an example — it doesn't exist in the codebase; the gRPC integration uses Grpc* (GrpcClientDecorator etc). Reframe the section to acknowledge acronym casing is not uniform across dd-trace-java and to defer to a reference instrumentation. (Copilot) - Drop the sanity-check bash script entirely. mcculls flagged that its regex only matched 'class', missing enum/interface/@interface, and would produce false MISMATCH lines for any such file (LogHandler.java, ParameterCollector.java, etc.). references/advice-class.md - Rewrite the 'onExit resilient to onEnter throwing' section — the claim that 'onThrowable = Throwable.class ensures exit fires even on onEnter exception' was factually wrong. Per docs/how_instrumentations_work.md:532-552, 'if the OnMethodEnter method throws an exception, the OnMethodExit method is not invoked' — unconditionally; onThrowable cannot override it. onThrowable controls exit-on-target-method-throw, not exit-on-enter-throw. (mcculls) - Add inline note that java.nio.charset.StandardCharsets is a java.nio.* type and forbidden in bootstrap instrumentations (per the same file's Must NOT list). In bootstrap advice, use the string charset name ('UTF-8') instead. (Copilot) references/context-tracking.md - Soften the 'rxjava-2.0 hooks subscribe(Observer)' statement. The module's actual matcher is named('subscribe').and(takesArguments(1)), matching any single-arg subscribe overload with the argument typed as the base callback interface. Direct the reader at the module source instead of copying overload names. (Copilot) Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> fix(skill): trim bootstrap note on charset to a single line Copilot's suggestion was 'add an explicit note here'; the initial fix was a full paragraph. Trimming to a single-sentence pointer since the Must NOT list already carries the details. Signed-off-by: Jordan Wong <jordan.wong@datadoghq.com> review feedback Fix docs inconsistencies Rename skill to apm-integrations remove redundant note Renamed .claude/skills to .agents/skills and added symlink-style redirects for the old location Align NIO docs advice with skill Simplify SKILL Fix constructor advice best-practice Address codex comments Co-authored-by: mcculls <stuart.mcculloch@datadoghq.com> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>

jordan-wong mentioned this pull request Jun 23, 2026

feat(commons-httpclient-2.0): toolkit-generated regeneration #11717

Open

jordan-wong and others added 2 commits June 23, 2026 21:42

fix(build): spotlessApply on instrumentation/build.gradle

1a6fd6d

Spotless moved the trailing comment 'org-json does not use semver' to its own line. Fixes dd-gitlab/spotless CI failure introduced by the R32 edit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

jordan-wong added the tag: do not merge Do not merge changes label Jun 24, 2026

jordan-wong marked this pull request as ready for review June 25, 2026 00:36

jordan-wong requested review from a team as code owners June 25, 2026 00:36

jordan-wong requested review from bric3 and ygree and removed request for a team June 25, 2026 00:36

dd-octo-sts Bot added the tag: ai generated Largely based on code generated by an AI or LLM label Jun 25, 2026

jordan-wong removed request for bric3 and ygree June 25, 2026 00:39

chatgpt-codex-connector Bot reviewed Jun 25, 2026

View reviewed changes

This was referenced Jun 25, 2026

feat(sparkjava-2.3): toolkit-generated sparkjava-2.3 #11708

Open

skill(apm-integrations): add rules from recent reviews + refactor into sub references #11760

Merged

jordan-wong changed the title ~~feat(feign-10.8): toolkit-generated feign HTTP client instrumentation [DO NOT MERGE]~~ feat(feign-10.8): toolkit-generated feign HTTP client instrumentation Jun 29, 2026

jordan-wong added inst: others All other instrumentations type: feature Enhancements and improvements labels Jun 29, 2026

PerfectSlayer added the tag: apm integration toolkit Changes generated by DataDog/apm-instrumentation-toolkit as part of IDM AIT experimentation label Jul 1, 2026

ValentinZakharov reviewed Jul 6, 2026

View reviewed changes

jordan-wong mentioned this pull request Jul 13, 2026

skill(apm-integrations): additional rules from recent HTTP-category PR reviews #11927

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(feign-10.8): toolkit-generated feign HTTP client instrumentation#11709

feat(feign-10.8): toolkit-generated feign HTTP client instrumentation#11709
jordan-wong wants to merge 4 commits into
masterfrom
eval/feign-10.8-toolkit-attempt

jordan-wong commented Jun 23, 2026 •

edited

Loading

Uh oh!

dd-octo-sts Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

jordan-wong commented Jun 24, 2026

Uh oh!

dd-octo-sts Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 25, 2026

Uh oh!

chatgpt-codex-connector Bot Jun 25, 2026

Uh oh!

ValentinZakharov Jul 6, 2026

Uh oh!

ValentinZakharov Jul 6, 2026

Uh oh!

ValentinZakharov Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jordan-wong commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Wins from prior reviewer feedback

What was structurally correct

CI

What reviewers should do

Related

Uh oh!

dd-octo-sts Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟢 Java Benchmark SLOs — All performance SLOs passed

Uh oh!

jordan-wong commented Jun 24, 2026

Uh oh!

dd-octo-sts Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

ValentinZakharov Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

ValentinZakharov Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

ValentinZakharov Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jordan-wong commented Jun 23, 2026 •

edited

Loading

dd-octo-sts Bot commented Jun 23, 2026 •

edited

Loading

dd-octo-sts Bot commented Jun 25, 2026 •

edited

Loading