Skip to content

Commit b741811

Browse files
authored
Fix netty connection keep alive (#385)
* fix netty connection keep alive context
1 parent 777d4b1 commit b741811

File tree

2 files changed

+100
-0
lines changed

2 files changed

+100
-0
lines changed

instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/HttpServerResponseTracingHandler.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,14 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) {
103103
span.setStatus(code >= 100 && code < 500 ? StatusCode.UNSET : StatusCode.ERROR);
104104
}
105105
if (msg instanceof LastHttpContent) {
106+
// When we end the span, we should set the server context and request attr to null so that
107+
// for the next request a new context and request is made and stored in channel.
108+
// Else, when using a Connection: keep-alive header, the same server context and request attr
109+
// is used in the subsequent requests.
110+
ctx.channel()
111+
.attr(io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys.SERVER_CONTEXT)
112+
.set(null);
113+
ctx.channel().attr(AttributeKeys.REQUEST).set(null);
106114
span.end();
107115
}
108116
}

instrumentation/netty/netty-4.1/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/AbstractNetty41ServerInstrumentationTest.java

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,4 +206,96 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio
206206
.getAttributes()
207207
.get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_BODY)));
208208
}
209+
210+
@Test
211+
public void connectionKeepAlive() throws IOException, TimeoutException, InterruptedException {
212+
Request request =
213+
new Request.Builder()
214+
.url(String.format("http://localhost:%d/post", port))
215+
.header(REQUEST_HEADER_NAME, REQUEST_HEADER_VALUE)
216+
.header("first", "1st")
217+
.header("connection", "keep-alive")
218+
.get()
219+
.build();
220+
221+
try (Response response = httpClient.newCall(request).execute()) {
222+
Assertions.assertEquals(200, response.code());
223+
Assertions.assertEquals(RESPONSE_BODY, response.body().string());
224+
}
225+
226+
List<List<SpanData>> traces = TEST_WRITER.getTraces();
227+
TEST_WRITER.waitForTraces(1);
228+
Assertions.assertEquals(1, traces.size());
229+
List<SpanData> trace = traces.get(0);
230+
Assertions.assertEquals(1, trace.size());
231+
SpanData spanData = trace.get(0);
232+
233+
Assertions.assertEquals(
234+
REQUEST_HEADER_VALUE,
235+
spanData
236+
.getAttributes()
237+
.get(HypertraceSemanticAttributes.httpRequestHeader(REQUEST_HEADER_NAME)));
238+
Assertions.assertEquals(
239+
"1st",
240+
spanData.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader("first")));
241+
Assertions.assertEquals(
242+
"keep-alive",
243+
spanData.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader("connection")));
244+
Assertions.assertEquals(
245+
RESPONSE_HEADER_VALUE,
246+
spanData
247+
.getAttributes()
248+
.get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_HEADER_NAME)));
249+
Assertions.assertNull(
250+
spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY));
251+
Assertions.assertEquals(
252+
RESPONSE_BODY,
253+
spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY));
254+
255+
RequestBody requestBody = blockedRequestBody(true, 3000, 75);
256+
Request request2 =
257+
new Request.Builder()
258+
.url(String.format("http://localhost:%d/post", port))
259+
.header(REQUEST_HEADER_NAME, "REQUEST_HEADER_VALUE")
260+
.header("second", "2nd")
261+
.header("connection", "keep-alive")
262+
.post(requestBody)
263+
.build();
264+
265+
try (Response response = httpClient.newCall(request2).execute()) {
266+
Assertions.assertEquals(403, response.code());
267+
Assertions.assertTrue(response.body().string().isEmpty());
268+
}
269+
270+
List<List<SpanData>> traces2 = TEST_WRITER.getTraces();
271+
TEST_WRITER.waitForTraces(2);
272+
Assertions.assertEquals(2, traces2.size());
273+
List<SpanData> trace2 = traces2.get(1);
274+
Assertions.assertEquals(1, trace2.size());
275+
SpanData spanData2 = trace2.get(0);
276+
277+
Assertions.assertEquals(
278+
"REQUEST_HEADER_VALUE",
279+
spanData2
280+
.getAttributes()
281+
.get(HypertraceSemanticAttributes.httpRequestHeader(REQUEST_HEADER_NAME)));
282+
Assertions.assertEquals(
283+
"2nd",
284+
spanData2.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader("second")));
285+
Assertions.assertEquals(
286+
"keep-alive",
287+
spanData2
288+
.getAttributes()
289+
.get(HypertraceSemanticAttributes.httpRequestHeader("connection")));
290+
Assertions.assertNull(
291+
spanData2.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader("first")));
292+
Assertions.assertNull(
293+
spanData2
294+
.getAttributes()
295+
.get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_HEADER_NAME)));
296+
Assertions.assertNull(
297+
spanData2
298+
.getAttributes()
299+
.get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_BODY)));
300+
}
209301
}

0 commit comments

Comments
 (0)