Finish HTTP client spans and close scopes when AppSec blocks requests by amarziali · Pull Request #12611 · DataDog/dd-trace-java

amarziali · 2026-09-23T13:50:46Z

What Does This Do

AppSec blocking could leave an HTTP client span unfinished and its scope active in:

JDK HTTP client (send and sendAsync)
Apache HttpClient 4
Commons HttpClient 2

These instrumentations create and activate a client span before calling DECORATE.onRequest(). If AppSec throws BlockingException, entry advice aborts before handing the scope to exit advice. For async requests, no completion callback has been registered either.

Consequently, nobody finishes the client span or closes its scope. The unfinished client span cannot be reported as completed, and the lingering active context interferes with enclosing Spring and servlet scope cleanup. This does not necessarily prevent other finished spans in the trace from being published.

This PR cleans up the span and scope in the blocking path before rethrowing the exception, preserving call-depth reset and blocking behavior.

flowchart LR
  A[Create span and activate scope] --> B[AppSec blocks]
  B -->|Before| C[Entry advice aborts]
  C --> D[Unfinished span and active scope]
  B -->|Fixed| E[Decorate blocking error]
  E --> F[Close scope and restore parent]
  F --> G[Finish client span and rethrow]

  classDef broken fill:#fee2e2,stroke:#dc2626
  classDef healthy fill:#dcfce7,stroke:#16a34a
  class C,D broken
  class E,F,G healthy

Motivation

Instrumentation tests did not exercise the blocking callback path. Existing AppSec smoke tests checked the blocking response and security event, which could succeed despite incomplete tracing cleanup.

Enabling scope diagnostics in smoke tests exposed the missing cleanup.

Additional Notes

Contributor Checklist

Format the title according to the contribution guidelines
Assign the type: and (comp: or inst:) labels in addition to any other useful labels
Avoid using close, fix, or any linking keywords when referencing an issue
Use solves instead, and assign the PR milestone to the issue
Update the CODEOWNERS file on source file addition, migration, or deletion
Update public documentation with any new configuration flags or behaviors
Once approved, use merge queue to merge the PR

Jira ticket: [PROJ-IDENT]

chatgpt-codex-connector · 2026-09-23T13:50:58Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-23T13:55:32.744092Z	`04c2d1a`	PR opened
🔒 Security Review	✅ Completed 2026-09-23T13:57:44.731829Z	`04c2d1a`	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

datadog-datadog-prod-us1 · 2026-09-23T14:06:31Z

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 68.82% (+9.52%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 04c2d1a | Docs | Give us feedback!}

datadog-datadog-prod-us1

More details

The blocking paths now reset call depth, restore the parent context, close the scope, and finish the client span before they throw the blocking exception.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit 04c2d1a · @DataDog review to ask questions}

dd-octo-sts · 2026-09-23T14:13:26Z

🟢 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.93 s	13.89 s	[-0.5%; +1.2%] (no difference)
startup:insecure-bank:tracing:Agent	12.88 s	12.91 s	[-0.9%; +0.4%] (no difference)
startup:petclinic:appsec:Agent	17.10 s	16.93 s	[+0.1%; +1.9%] (maybe worse)
startup:petclinic:iast:Agent	16.93 s	17.05 s	[-1.7%; +0.3%] (no difference)
startup:petclinic:profiling:Agent	16.63 s	16.91 s	[-2.6%; -0.7%] (maybe better)
startup:petclinic:sca:Agent	16.99 s	16.85 s	[-0.1%; +1.7%] (no difference)
startup:petclinic:tracing:Agent	16.20 s	16.21 s	[-1.0%; +0.8%] (no difference)

Commit: 04c2d1a1 · 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.

jandro996 · 2026-09-23T14:42:02Z

Hey @amarziali not sure if this is part of a group of PRs to solve this kind of issues but I found that apache-httpclient-5.0 has the identical span/scope leak on BlockingException. activateHttpSpan() has no try/catch around DECORATE.onRequest(), and HelperMethods.onBlockingRequest() only resets CallDepthThreadLocalMap, never touches the span/scope. Worth a follow-up PR (or confirm it's intentionally out of scope here) since it's the same bug, one HTTP client version up.

jandro996

LGTM!

amarziali · 2026-09-23T14:44:08Z

Hey @amarziali not sure if this is part of a group of PRs to solve this kind of issues but I found that apache-httpclient-5.0 has the identical span/scope leak on BlockingException. activateHttpSpan() has no try/catch around DECORATE.onRequest(), and HelperMethods.onBlockingRequest() only resets CallDepthThreadLocalMap, never touches the span/scope. Worth a follow-up PR (or confirm it's intentionally out of scope here) since it's the same bug, one HTTP client version up.

Good catch. Actually the diagnostics is plugged for trial on #12609 but can only audit things covered by a smoke tests. So yes it deserves a followup PR and a smoke tests so

PerfectSlayer · 2026-09-24T07:23:50Z

+      DECORATE.afterStart(span);
+      DECORATE.onRequest(span, request);
+    } catch (BlockingException e) {
+      try {


❔ question: ‏Should we have try / catch block here?
The only exception allowed is now a BlockingException since decorator methods are no longer allowed to raised anything else.

Finish HTTP client spans and close scopes when AppSec blocks requests

04c2d1a

amarziali added the type: bug fix Bug fix label Sep 23, 2026

amarziali requested review from a team as code owners September 23, 2026 13:50

amarziali requested review from PerfectSlayer and removed request for a team September 23, 2026 13:50

amarziali added inst: others All other instrumentations comp: asm waf Application Security Management (WAF) tag: ai generated Largely based on code generated by an AI or LLM labels Sep 23, 2026

amarziali requested review from jordan-wong and removed request for a team September 23, 2026 13:50

amarziali requested a review from jandro996 September 23, 2026 13:51

amarziali added the tag: override groovy enforcement Override the "Enforce Groovy Migration" check label Sep 23, 2026

datadog-datadog-prod-us1 Bot reviewed Sep 23, 2026

View reviewed changes

jandro996 approved these changes Sep 23, 2026

View reviewed changes

amarziali mentioned this pull request Sep 23, 2026

Enforce continuation diagnostics on smoke tests #12609

Draft

PerfectSlayer reviewed Sep 24, 2026

View reviewed changes

Navigation Menu

Finish HTTP client spans and close scopes when AppSec blocks requests - #12611

Finish HTTP client spans and close scopes when AppSec blocks requests#12611
amarziali wants to merge 1 commit into
masterfrom
andrea.marziali/fix-http-blocking-scope-cleanup

amarziali commented Sep 23, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 23, 2026 •

edited

Loading

Uh oh!

datadog-datadog-prod-us1 Bot commented Sep 23, 2026

Uh oh!

datadog-datadog-prod-us1 Bot left a comment

Uh oh!

dd-octo-sts Bot commented Sep 23, 2026

Uh oh!

jandro996 commented Sep 23, 2026

Uh oh!

jandro996 left a comment

Uh oh!

amarziali commented Sep 23, 2026

Uh oh!

PerfectSlayer Sep 24, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

amarziali commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Does This Do

Motivation

Additional Notes

Contributor Checklist

Uh oh!

chatgpt-codex-connector Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

datadog-datadog-prod-us1 Bot commented Sep 23, 2026

Uh oh!

datadog-datadog-prod-us1 Bot left a comment

Choose a reason for hiding this comment

Uh oh!

dd-octo-sts Bot commented Sep 23, 2026

🟢 Java Benchmark SLOs — All performance SLOs passed

Uh oh!

jandro996 commented Sep 23, 2026

Uh oh!

jandro996 left a comment

Choose a reason for hiding this comment

Uh oh!

amarziali commented Sep 23, 2026

Uh oh!

PerfectSlayer Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amarziali commented Sep 23, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 23, 2026 •

edited

Loading