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 👍 / 👎.

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(add-apm-integrations): R13-R33 + Cat B (context-propagation) rules #11760

Draft

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: enhancement 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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 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