From 7a4afa84b140ae06d4e9f3b40d66ade2d9b26d47 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 9 Aug 2021 16:27:40 -0700 Subject: [PATCH 1/4] Limit Netty exception capture to Netty span --- .../AbstractChannelHandlerContextInstrumentation.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java index 4e61e6bf708b..7ce5e6bea1fb 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java @@ -15,9 +15,7 @@ import io.opentelemetry.instrumentation.netty.v4_1.AttributeKeys; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import io.opentelemetry.javaagent.instrumentation.netty.v4_1.client.NettyHttpClientTracer; -import io.opentelemetry.javaagent.instrumentation.netty.v4_1.server.NettyHttpServerTracer; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -51,9 +49,10 @@ public static void onEnter( Attribute clientContextAttr = channelContext.channel().attr(AttributeKeys.CLIENT_CONTEXT); NettyHttpClientTracer.tracer().endExceptionally(clientContextAttr.get(), throwable); - } else { - NettyHttpServerTracer.tracer() - .onException(Java8BytecodeBridge.currentContext(), throwable); + } else if (channelContext.channel().hasAttr(AttributeKeys.SERVER_SPAN)) { + Attribute clientContextAttr = + channelContext.channel().attr(AttributeKeys.SERVER_SPAN); + NettyHttpClientTracer.tracer().onException(clientContextAttr.get(), throwable); } } } From d0c83c69bfa3a740fe4b69658178cd0e6939165f Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 9 Aug 2021 17:18:38 -0700 Subject: [PATCH 2/4] Rename constant --- ...ctChannelHandlerContextInstrumentation.java | 18 ++++++++---------- .../v4_1/server/NettyHttpServerTracer.java | 4 ++-- .../netty/v4_1/AttributeKeys.java | 2 +- .../ratpack/TracingHandler.java | 2 +- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java index 7ce5e6bea1fb..d78159956aad 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java @@ -44,16 +44,14 @@ public static class InvokeExceptionCaughtAdvice { public static void onEnter( @Advice.This ChannelHandlerContext channelContext, @Advice.Argument(0) Throwable throwable) { - if (throwable != null) { - if (channelContext.channel().hasAttr(AttributeKeys.CLIENT_CONTEXT)) { - Attribute clientContextAttr = - channelContext.channel().attr(AttributeKeys.CLIENT_CONTEXT); - NettyHttpClientTracer.tracer().endExceptionally(clientContextAttr.get(), throwable); - } else if (channelContext.channel().hasAttr(AttributeKeys.SERVER_SPAN)) { - Attribute clientContextAttr = - channelContext.channel().attr(AttributeKeys.SERVER_SPAN); - NettyHttpClientTracer.tracer().onException(clientContextAttr.get(), throwable); - } + if (channelContext.channel().hasAttr(AttributeKeys.CLIENT_CONTEXT)) { + Attribute clientContextAttr = + channelContext.channel().attr(AttributeKeys.CLIENT_CONTEXT); + NettyHttpClientTracer.tracer().endExceptionally(clientContextAttr.get(), throwable); + } else if (channelContext.channel().hasAttr(AttributeKeys.SERVER_CONTEXT)) { + Attribute clientContextAttr = + channelContext.channel().attr(AttributeKeys.SERVER_CONTEXT); + NettyHttpClientTracer.tracer().onException(clientContextAttr.get(), throwable); } } } diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java index 81e085452a60..d70453df0cab 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java @@ -43,12 +43,12 @@ protected int responseStatus(HttpResponse httpResponse) { @Override protected void attachServerContext(Context context, Channel channel) { - channel.attr(AttributeKeys.SERVER_SPAN).set(context); + channel.attr(AttributeKeys.SERVER_CONTEXT).set(context); } @Override public Context getServerContext(Channel channel) { - return channel.attr(AttributeKeys.SERVER_SPAN).get(); + return channel.attr(AttributeKeys.SERVER_CONTEXT).get(); } @Override diff --git a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/AttributeKeys.java b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/AttributeKeys.java index 83b3bda6ac29..068548dc6dc8 100644 --- a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/AttributeKeys.java +++ b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/AttributeKeys.java @@ -16,7 +16,7 @@ public final class AttributeKeys { // this is the context that has the server span // // note: this attribute key is also used by ratpack instrumentation - public static final AttributeKey SERVER_SPAN = + public static final AttributeKey SERVER_CONTEXT = AttributeKey.valueOf(AttributeKeys.class, "server-span"); public static final AttributeKey CLIENT_CONTEXT = diff --git a/instrumentation/ratpack-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/TracingHandler.java b/instrumentation/ratpack-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/TracingHandler.java index d52fec8db171..d736a11f031e 100644 --- a/instrumentation/ratpack-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/TracingHandler.java +++ b/instrumentation/ratpack-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/TracingHandler.java @@ -21,7 +21,7 @@ public final class TracingHandler implements Handler { @Override public void handle(Context ctx) { Attribute spanAttribute = - ctx.getDirectChannelAccess().getChannel().attr(AttributeKeys.SERVER_SPAN); + ctx.getDirectChannelAccess().getChannel().attr(AttributeKeys.SERVER_CONTEXT); io.opentelemetry.context.Context serverSpanContext = spanAttribute.get(); // Must use context from channel, as executor instrumentation is not accurate - Ratpack From 7e3d9f58a46966d78aa2cc3aff67a328b41931bb Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 10 Aug 2021 12:14:22 -0700 Subject: [PATCH 3/4] Fix var name --- .../v4_1/AbstractChannelHandlerContextInstrumentation.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java index d78159956aad..9a17f6337325 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AbstractChannelHandlerContextInstrumentation.java @@ -49,9 +49,9 @@ public static void onEnter( channelContext.channel().attr(AttributeKeys.CLIENT_CONTEXT); NettyHttpClientTracer.tracer().endExceptionally(clientContextAttr.get(), throwable); } else if (channelContext.channel().hasAttr(AttributeKeys.SERVER_CONTEXT)) { - Attribute clientContextAttr = + Attribute serverContextAttr = channelContext.channel().attr(AttributeKeys.SERVER_CONTEXT); - NettyHttpClientTracer.tracer().onException(clientContextAttr.get(), throwable); + NettyHttpClientTracer.tracer().onException(serverContextAttr.get(), throwable); } } } From 043a8fe12c78965ce3630b2e1f9e7060fd5e2b07 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 10 Aug 2021 12:19:31 -0700 Subject: [PATCH 4/4] Apply to netty-4.0 also --- ...tChannelHandlerContextInstrumentation.java | 23 ++++++++++--------- .../netty/v4_0/AttributeKeys.java | 4 ++-- .../v4_0/server/NettyHttpServerTracer.java | 4 ++-- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AbstractChannelHandlerContextInstrumentation.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AbstractChannelHandlerContextInstrumentation.java index 580d6bde2959..280be5ee672b 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AbstractChannelHandlerContextInstrumentation.java +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AbstractChannelHandlerContextInstrumentation.java @@ -15,7 +15,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import io.opentelemetry.javaagent.instrumentation.netty.v4_0.client.NettyHttpClientTracer; import io.opentelemetry.javaagent.instrumentation.netty.v4_0.server.NettyHttpServerTracer; import net.bytebuddy.asm.Advice; @@ -49,16 +48,18 @@ public static class InvokeExceptionCaughtAdvice { public static void onEnter( @Advice.This ChannelHandlerContext channelContext, @Advice.Argument(0) Throwable throwable) { - if (throwable != null) { - Attribute clientContextAttr = - channelContext.channel().attr(AttributeKeys.CLIENT_CONTEXT); - Context context = clientContextAttr.get(); - if (context != null) { - NettyHttpClientTracer.tracer().endExceptionally(context, throwable); - } else { - NettyHttpServerTracer.tracer() - .onException(Java8BytecodeBridge.currentContext(), throwable); - } + Attribute clientContextAttr = + channelContext.channel().attr(AttributeKeys.CLIENT_CONTEXT); + Context clientContext = clientContextAttr.get(); + if (clientContext != null) { + NettyHttpClientTracer.tracer().endExceptionally(clientContext, throwable); + return; + } + Attribute serverContextAttr = + channelContext.channel().attr(AttributeKeys.SERVER_CONTEXT); + Context serverContext = serverContextAttr.get(); + if (serverContext != null) { + NettyHttpServerTracer.tracer().onException(serverContext, throwable); } } } diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AttributeKeys.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AttributeKeys.java index 70547a6b0c9c..22df1bee415c 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AttributeKeys.java +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AttributeKeys.java @@ -24,8 +24,8 @@ protected ConcurrentMap> computeValue(Class type) { attributeKey(AttributeKeys.class.getName() + ".write-context"); // this is the context that has the server span - public static final AttributeKey SERVER_SPAN = - attributeKey(AttributeKeys.class.getName() + ".server-span"); + public static final AttributeKey SERVER_CONTEXT = + attributeKey(AttributeKeys.class.getName() + ".server-context"); public static final AttributeKey CLIENT_CONTEXT = attributeKey(AttributeKeys.class.getName() + ".client-context"); diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java index 033f7b7a86bd..7399f707d9ca 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java @@ -43,12 +43,12 @@ protected int responseStatus(HttpResponse httpResponse) { @Override protected void attachServerContext(Context context, Channel channel) { - channel.attr(AttributeKeys.SERVER_SPAN).set(context); + channel.attr(AttributeKeys.SERVER_CONTEXT).set(context); } @Override public Context getServerContext(Channel channel) { - return channel.attr(AttributeKeys.SERVER_SPAN).get(); + return channel.attr(AttributeKeys.SERVER_CONTEXT).get(); } @Override