Skip to content

Fix Netty 4.1 server span completion for streaming responses#11941

Open
ygree wants to merge 5 commits into
ygree/fix-netty41-http11-pipelining-context-queuefrom
ygree/fix-netty41-chunked-tracing
Open

Fix Netty 4.1 server span completion for streaming responses#11941
ygree wants to merge 5 commits into
ygree/fix-netty41-http11-pipelining-context-queuefrom
ygree/fix-netty41-chunked-tracing

Conversation

@ygree

@ygree ygree commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Updates Netty 4.1 server instrumentation to keep request spans open until the HTTP response is actually complete.

It tracks pending HTTP/1.1 request contexts in FIFO order, finishes streaming/chunked responses on LastHttpContent, finishes header-complete responses such as 204/205/304, HEAD, CONNECT, Content-Length: 0, and WebSocket upgrades at the response headers, keeps spans open across interim 1xx responses (e.g. 100 Continue, 103 Early Hints), finishes connection-close-delimited responses (no Content-Length, not chunked) normally when the channel closes, and marks a response as an error only when it declared a body that never completed before the channel closed.

Motivation

Netty 4.1 server spans could finish when the first response headers/chunk were written instead of when the response completed. That made streaming response durations too short and could misattribute spans on keep-alive or pipelined HTTP/1.1 connections.

Additional Notes

The fix avoids relying on a single channel-level streaming context because that is not sufficient for pipelined requests. Response writes are matched to the queued request context instead.

Contributor Checklist

Jira ticket: APMS-20050

@ygree ygree self-assigned this Jul 14, 2026
@ygree ygree added type: bug fix Bug fix inst: netty Netty instrumentation tag: ai generated Largely based on code generated by an AI or LLM labels Jul 14, 2026
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 57.03% (-12.70%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: efe7144 | Docs | Datadog PR Page | Give us feedback!

@dd-octo-sts

dd-octo-sts Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🟢 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.97 s 13.95 s [-0.6%; +0.9%] (no difference)
startup:insecure-bank:tracing:Agent 12.89 s 13.05 s [-2.0%; -0.4%] (maybe better)
startup:petclinic:appsec:Agent 16.96 s 16.86 s [-0.5%; +1.8%] (no difference)
startup:petclinic:iast:Agent 16.84 s 16.44 s [-2.1%; +7.0%] (no difference)
startup:petclinic:profiling:Agent 16.81 s 16.96 s [-1.9%; +0.2%] (no difference)
startup:petclinic:sca:Agent 16.92 s 16.94 s [-1.1%; +0.8%] (no difference)
startup:petclinic:tracing:Agent 16.06 s 16.01 s [-0.6%; +1.2%] (no difference)

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

@ygree ygree force-pushed the ygree/fix-netty41-chunked-tracing branch from 5e8efd4 to 8a32be2 Compare July 14, 2026 05:39
Keep server spans open until terminal response writes, finish incomplete
responses on channel close, and use queued request contexts to avoid
keep-alive/pipelining misattribution. Cover chunked, bodyless, HEAD,
CONNECT, interim, and WebSocket upgrade response cases.
@ygree ygree force-pushed the ygree/fix-netty41-chunked-tracing branch from 8a32be2 to f65d4a4 Compare July 14, 2026 06:10
Keep Netty 4.1 server spans open until terminal response writes while
preserving request-ended callback timing for AppSec/IAST. Track one-shot
beforeFinish execution separately from span finish so streaming spans
get
correct duration without dropping request-end security tags.
@ygree ygree marked this pull request as ready for review July 14, 2026 18:01
@ygree ygree requested a review from a team as a code owner July 14, 2026 18:01
@ygree ygree requested review from jordan-wong and removed request for a team July 14, 2026 18:01

@datadog-datadog-prod-us1-2 datadog-datadog-prod-us1-2 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: PASS

More details

The PR correctly refactors Netty 4.1 server span lifecycle to keep spans open until response completion instead of finishing on first headers. A FIFO queue ensures pipelined requests match responses correctly, and guards protect against double-finishing. All critical scenarios (chunked responses, bodyless responses, pipelined requests, connection drops) are tested and pass validation. No behavioral regressions found in diff analysis or test coverage review.

Was this helpful? React 👍 or 👎

📊 Validated against 7 scenarios · Open Bits AI session

🤖 Datadog Autotest · Commit de23316 · What is Autotest? · Any feedback? Reach out in #autotest

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de233160c2

ℹ️ 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".

@ygree ygree marked this pull request as draft July 14, 2026 21:55
ygree added 2 commits July 14, 2026 16:49
- Defer beforeFinish until the response actually completes.
- Finish close-delimited responses without error.
- Guard Content-Length parsing.
- Skip AppSec response analysis for all interim 1xx responses in
  MaybeBlockResponseHandler, not just 100 Continue.

Add a regression test for close-delimited responses.
@ygree ygree marked this pull request as ready for review July 15, 2026 02:19

@datadog-datadog-prod-us1-2 datadog-datadog-prod-us1-2 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: PASS

More details

The PR's isTerminalResponse logic was traced through 15 concrete response type/method combinations (FullHttpResponse, chunked, 1xx, 204/205/304, HEAD, CONNECT, WebSocket, close-delimited, Content-Length:0) and the channelInactive handler was verified against 6 channel-close scenarios. No path produces a span leak, a double-reported span under normal conditions, or a misclassified terminal/non-terminal response. The PR also silently fixes two pre-existing bugs: 103 Early Hints was incorrectly treated as terminal in the old code (consuming the one-shot AppSec analysis slot), and Upgrade: WebSocket with a capital W was not recognised as a WebSocket upgrade due to a case-sensitive string comparison.

Was this helpful? React 👍 or 👎

📊 Validated against 26 scenarios · Open Bits AI session

🤖 Datadog Autotest · Commit efe7144 · What is Autotest? · Any feedback? Reach out in #autotest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inst: netty Netty instrumentation tag: ai generated Largely based on code generated by an AI or LLM type: bug fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant