Fix Netty HTTP span lifecycle for chunked/streaming responses

[gtukmachev]

What Does This Do

Fixes HttpServerResponseTracingHandler to correctly handle chunked/streaming HTTP responses. The span now covers the full response duration (from headers to LastHttpContent) instead of closing at header-send time.

Four issues addressed:

1. Wrong instanceof order — FullHttpResponse must be checked before HttpResponse and LastHttpContent since it extends both
2. Keep-alive race condition — added STREAMING_CONTEXT_KEY channel attribute to prevent the next request's channelRead from overwriting the span context before LastHttpContent is written
3. Streaming span leak on disconnect — channelInactive now finishes spans stored in STREAMING_CONTEXT_KEY when the channel closes mid-stream
4. WebSocket/bodyless span leak — WebSocket upgrades and bodyless responses (204, 205, 304) finish immediately since they never produce LastHttpContent

Motivation

Applications using chunked HTTP responses (e.g. Ktor's respondOutputStream) report near-zero latency in APM — the span closes when response headers are sent, not when the stream finishes.

call.respondOutputStream {
    repeat(5) { writeSomeChunk(); delay(1000) }  // 5s total — DD reports ~0ms
}

This affects any Netty-backed server that sends HttpResponse + multiple HttpContent + LastHttpContent separately (i.e. chunked transfer encoding).

Additional Notes

Files Changed

• netty-4.1/.../HttpServerResponseTracingHandler.java — message type routing, streaming context key, websocket + bodyless fix
• netty-4.1/.../HttpServerRequestTracingHandler.java — channelInactive streaming span cleanup
• netty-common/.../AttributeKeys.java — added STREAMING_CONTEXT_KEY
• netty-4.1/.../NettyChunkedResponseTest.java — new JUnit 5 test (4 test cases)

Tests

New test class NettyChunkedResponseTest with a real Netty server that writes chunked responses manually (HttpResponse + HttpContent* + LastHttpContent):

Test	What it verifies
chunkedResponseSpanIncludesFullStreamDuration	Span duration ≥ full stream time (5×200ms), not ~0ms
fullResponseStillFinishesSpanImmediately	FullHttpResponse regression — span finishes immediately
keepAliveSequentialChunkedRequestsGetCorrectSpans	Sequential keep-alive requests each get correct spans
connectionDropDuringChunkedResponseFinishesSpan	Span finished with error on client disconnect mid-stream

Contributor Checklist

• Format the title according to the contribution guidelines
• Assign the type: and (comp: or inst:) labels — external contributor, no permission to add labels; please add type: bug, inst: netty, tag: ai generated
• Avoid using linking keywords when referencing an issue
• No new source files requiring CODEOWNERS update (test file only)
• No new configuration flags or behaviors requiring documentation update

[gtukmachev]

@ygree (original comment):

Thanks for the contribution! Could you please add tests? Also, I noticed there's a possibility of a span leak when LastHttpContent is not received during chunked/streaming responses, which needs to be addressed.

[gtukmachev]

@gtukmachev (original reply):

🔄 Re: tests and span leak

Tests will be added shortly. I'll also look into the span leak scenario you mentioned for cases when LastHttpContent is not received.

Thank you for the review!

[gtukmachev]

Hi @ygree, this PR has been updated with all the changes since your initial review on #10656 (now closed):

Tests added — new JUnit 5 test class NettyChunkedResponseTest with 4 test cases exercising the chunked response path (HttpResponse → HttpContent* → LastHttpContent) using a real Netty server.

Span leak fix — channelInactive in HttpServerRequestTracingHandler now checks STREAMING_CONTEXT_KEY and finishes spans when the channel closes mid-stream.

Additional fix — WebSocket upgrades and bodyless responses (204, 205, 304) finish the span immediately instead of deferring to LastHttpContent.

Could you please also add the required labels? As an external contributor I can't assign them:

• type: bug
• inst: netty
• tag: ai generated

Ready for review. Thanks!

[gtukmachev]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77cc414ea5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Handle HEAD responses without waiting for chunks

When a Netty handler answers a HEAD request by writing only a DefaultHttpResponse/headers, no LastHttpContent is necessarily written because the response has no body. This branch only treats 204/205/304 as bodyless, so a 200 HEAD response is moved to STREAMING_CONTEXT_KEY and CONTEXT_ATTRIBUTE_KEY is removed; on a keep-alive connection the span stays open until the channel closes and can be reported late/as an error instead of finishing with the response headers. Consider using the stored request method (or another marker) so HEAD/header-only responses finish immediately.

Useful? React with 👍 / 👎.

[gtukmachev]

Not an issue — Netty's HttpObjectEncoder.encode() always writes LastHttpContent.EMPTY_LAST_CONTENT after a non-full HttpResponse, including HEAD responses. The handleLastHttpContent path will fire and finish the span correctly. The truly bodyless status codes (204, 205, 304) are already handled with immediate finish.

[gtukmachev]

Hey DataDog team! Hope you're doing great!

Could we please resume communication on this PR?

This fix is very important for my company.

We are transitioning more of our backend endpoints to a streaming approach and are currently forced either to lose DD APM tracking or maintain our own custom version of the DD Java agent.

CC @AlexeyKuznetsov-DD

Sorry for pinging you directly — I found you as the latest committer today 😄

Would you please recommend the correct way to request a review for this PR, or point me to the right person to contact regarding it?

[AlexeyKuznetsov-DD]

Hey DataDog team! Hope you're doing great!
Could we please resume communication on this PR?
This fix is very important for my company.
We are transitioning more of our backend endpoints to a streaming approach and are currently forced either to lose DD APM tracking or maintain our own custom version of the DD Java agent.

CC @AlexeyKuznetsov-DD
Sorry for pinging you directly — I found you as the latest committer today 😄
Would you please recommend the correct way to request a review for this PR, or point me to the right person to contact regarding it?

@gtukmachev Thank you for your contribution, I will ask the team to take a look.

[vandonr]

thanks for the PR, I asked @ygree if he could take a second look since he had the context from the previous PR.
Also, we'll need to duplicate this PR in our name to have it run on our CI. I'll take care of that tomorrow.

[ygree]

The PR fixes streamed response duration for common keep-alive / read-ahead timing after headers have already been written, but it does not make Netty HTTP/1.1 tracing fully pipeline-correct. A proper solution would need per-request/per-response correlation, likely a FIFO queue on the channel for HTTP/1.1 response order.

[gtukmachev]

@ygree, thanks for coming back to the PR.

Regarding your last comment:

The PR fixes streamed response duration for common keep-alive / read-ahead timing after headers have already been written, but it does not make Netty HTTP/1.1 tracing fully pipeline-correct. A proper solution would need per-request/per-response correlation, likely a FIFO queue on the channel for HTTP/1.1 response order.

What should the next step be to resolve this issue?

1. Will the Datadog team prioritise fixing it?
2. Or is it expected that I continue working on it and implement the suggested approach ("a FIFO queue on the channel for HTTP/1.1 response order")?

If it's the latter, @ygree, could you please provide more specific guidance on the expected implementation? That would help ensure we're aligned on the direction before I continue.

P/S - I'm fine with any of the options.

[ygree]

@ygree, thanks for coming back to the PR.

Regarding your last comment:

The PR fixes streamed response duration for common keep-alive / read-ahead timing after headers have already been written, but it does not make Netty HTTP/1.1 tracing fully pipeline-correct. A proper solution would need per-request/per-response correlation, likely a FIFO queue on the channel for HTTP/1.1 response order.

What should the next step be to resolve this issue?

1. Will the Datadog team prioritise fixing it?
2. Or is it expected that I continue working on it and implement the suggested approach ("a FIFO queue on the channel for HTTP/1.1 response order")?

If it's the latter, @ygree, could you please provide more specific guidance on the expected implementation? That would help ensure we're aligned on the direction before I continue.

P/S - I'm fine with any of the options.

Thanks for your patience! I opened a pull request (#11937) that addresses the underlying pipelining issue by introducing a dequeue of contexts. The current request context being responded to is at the front of the queue, and the last request context is at the back of the queue. This can be used as a basis for supporting chunked transfer encoding tracing.

I hope this helps unblock your change. Please let us know if you need further assistance.