From b9918ab513f498bb96ea124d265cfe4cd2a47c54 Mon Sep 17 00:00:00 2001
From: Trask Stalnaker <trask.stalnaker@gmail.com>
Date: Mon, 6 Feb 2023 08:16:11 -0800
Subject: [PATCH] End netty client span before callbacks (#7737)

netty 3.8 already ends client span before callbacks:


https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/b17f20c08e9def0cc2d342e7acd313dd7e52d677/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientResponseTracingHandler.java#L33-L46
---
 .../client/HttpClientResponseTracingHandler.java   | 14 +++++++-------
 .../client/HttpClientResponseTracingHandler.java   | 12 ++++++------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/HttpClientResponseTracingHandler.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/HttpClientResponseTracingHandler.java
index f56be5374726..f8202cb846d8 100644
--- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/HttpClientResponseTracingHandler.java
+++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/HttpClientResponseTracingHandler.java
@@ -50,6 +50,13 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
       requestAttr.remove();
     }
 
+    if (msg instanceof FullHttpResponse) {
+      instrumenter().end(context, request, (HttpResponse) msg, null);
+    } else if (msg instanceof LastHttpContent) {
+      HttpResponse response = ctx.channel().attr(AttributeKeys.CLIENT_RESPONSE).getAndRemove();
+      instrumenter().end(context, request, response, null);
+    }
+
     // We want the callback in the scope of the parent, not the client span
     if (parentContext != null) {
       try (Scope ignored = parentContext.makeCurrent()) {
@@ -58,12 +65,5 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
     } else {
       super.channelRead(ctx, msg);
     }
-
-    if (msg instanceof FullHttpResponse) {
-      instrumenter().end(context, request, (HttpResponse) msg, null);
-    } else if (msg instanceof LastHttpContent) {
-      HttpResponse response = ctx.channel().attr(AttributeKeys.CLIENT_RESPONSE).getAndRemove();
-      instrumenter().end(context, request, response, null);
-    }
   }
 }
diff --git a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/client/HttpClientResponseTracingHandler.java b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/client/HttpClientResponseTracingHandler.java
index a9819a8da8e5..bf2fcc778ed6 100644
--- a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/client/HttpClientResponseTracingHandler.java
+++ b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/client/HttpClientResponseTracingHandler.java
@@ -66,6 +66,12 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
       requestAttr.set(null);
     }
 
+    if (msg instanceof FullHttpResponse) {
+      instrumenter.end(context, request, (HttpResponse) msg, null);
+    } else if (msg instanceof LastHttpContent) {
+      instrumenter.end(context, request, ctx.channel().attr(HTTP_RESPONSE).getAndSet(null), null);
+    }
+
     // We want the callback in the scope of the parent, not the client span
     if (parentContext != null) {
       try (Scope ignored = parentContext.makeCurrent()) {
@@ -74,11 +80,5 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
     } else {
       super.channelRead(ctx, msg);
     }
-
-    if (msg instanceof FullHttpResponse) {
-      instrumenter.end(context, request, (HttpResponse) msg, null);
-    } else if (msg instanceof LastHttpContent) {
-      instrumenter.end(context, request, ctx.channel().attr(HTTP_RESPONSE).getAndSet(null), null);
-    }
   }
 }