Skip to content

Commit f359744

Browse files
gtukmachevclaude
andcommitted
Add Javadoc to test methods and improve connection-drop test synchronization
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 01e6ec9 commit f359744

1 file changed

Lines changed: 28 additions & 5 deletions

File tree

dd-java-agent/instrumentation/netty/netty-4.1/src/test/java/datadog/trace/instrumentation/netty41/server/NettyChunkedResponseTest.java

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ void stopServer() {
9191
}
9292
}
9393

94+
/**
95+
* Verifies that the span for a chunked HTTP response covers the full streaming duration, not just
96+
* the time to send headers. Without the fix in HttpServerResponseTracingHandler, the span would
97+
* finish when HttpResponse (headers) is written (~0ms), ignoring the time spent writing
98+
* HttpContent chunks and LastHttpContent.
99+
*/
94100
@Test
95101
void chunkedResponseSpanIncludesFullStreamDuration() throws Exception {
96102
String body = doGet("/chunked");
@@ -119,6 +125,11 @@ void chunkedResponseSpanIncludesFullStreamDuration() throws Exception {
119125
tag("peer.ipv4", any()))));
120126
}
121127

128+
/**
129+
* Regression test: a non-chunked FullHttpResponse must still finish the span immediately. This
130+
* ensures the instanceof ordering fix (FullHttpResponse checked before HttpResponse and
131+
* LastHttpContent) does not break the standard single-message response path.
132+
*/
122133
@Test
123134
void fullResponseStillFinishesSpanImmediately() throws Exception {
124135
String body = doGet("/full");
@@ -145,6 +156,13 @@ void fullResponseStillFinishesSpanImmediately() throws Exception {
145156
tag("peer.ipv4", any()))));
146157
}
147158

159+
/**
160+
* Verifies that sequential chunked requests on the same keep-alive connection each get their own
161+
* span with correct duration. Without the STREAMING_CONTEXT_KEY fix, Netty's event loop can
162+
* process channelRead for the next request (overwriting CONTEXT_ATTRIBUTE_KEY) before the pending
163+
* write of the previous response's LastHttpContent runs — causing handleLastHttpContent to finish
164+
* the wrong span.
165+
*/
148166
@Test
149167
void keepAliveSequentialChunkedRequestsGetCorrectSpans() throws Exception {
150168
URL url = new URL("http://localhost:" + port + "/chunked");
@@ -202,18 +220,23 @@ void keepAliveSequentialChunkedRequestsGetCorrectSpans() throws Exception {
202220
tag("peer.ipv4", any()))));
203221
}
204222

223+
/**
224+
* Verifies that a streaming span is properly finished (not leaked) when the client disconnects
225+
* mid-stream. Without the channelInactive fix in HttpServerRequestTracingHandler, the span stored
226+
* in STREAMING_CONTEXT_KEY would never be finished if LastHttpContent is never written because
227+
* the connection dropped.
228+
*/
205229
@Test
206230
void connectionDropDuringChunkedResponseFinishesSpan() throws Exception {
207-
// Open a raw socket, send the request, then close before the server finishes streaming.
208-
// This tests the channelInactive handler: the span in STREAMING_CONTEXT_KEY must be
209-
// finished when the channel becomes inactive, not leaked.
210231
try (Socket socket = new Socket("localhost", port)) {
232+
socket.setSoTimeout(5000);
211233
socket
212234
.getOutputStream()
213235
.write("GET /slow-chunked HTTP/1.1\r\nHost: localhost\r\n\r\n".getBytes());
214236
socket.getOutputStream().flush();
215-
// Wait for headers + first chunk, then close before streaming completes
216-
Thread.sleep(800);
237+
// Read until we get at least the first chunk — synchronization point before closing
238+
byte[] buf = new byte[512];
239+
socket.getInputStream().read(buf);
217240
}
218241
// Socket closed — channelInactive should fire and finish the streaming span
219242

0 commit comments

Comments
 (0)