From a5a36bd2036139e9ecf3a4560c51ba39ad080655 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 24 Nov 2020 12:07:10 -0800 Subject: [PATCH] Use namespace for attributes put into user classes (#1739) * Use namespace for attributes put into user classes * Add dependency to remove duplication --- .../api/tracer/HttpServerTracer.java | 4 +++- .../v2_2/TracingExecutionInterceptor.java | 8 ++++++-- .../netty/v4_0/AttributeKeys.java | 12 +++++------- .../ChannelFutureListenerInstrumentation.java | 2 +- .../NettyChannelPipelineInstrumentation.java | 2 +- .../HttpClientRequestTracingHandler.java | 2 +- .../netty/v4_1/AttributeKeys.java | 19 ++++++------------- .../ChannelFutureListenerInstrumentation.java | 2 +- .../NettyChannelPipelineInstrumentation.java | 2 +- .../HttpClientRequestTracingHandler.java | 2 +- .../ratpack-1.4/ratpack-1.4.gradle | 3 ++- .../ratpack/RatpackInstrumentationModule.java | 2 ++ .../ratpack/TracingHandler.java | 13 ++----------- .../ReactorNettyInstrumentationModule.java | 9 ++++++--- .../spring/webflux/server/AdviceUtils.java | 3 +-- 15 files changed, 39 insertions(+), 46 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java index f8daa05517..6073c1d0a6 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java @@ -30,7 +30,9 @@ public abstract class HttpServerTracer e private static final Logger log = LoggerFactory.getLogger(HttpServerTracer.class); - public static final String CONTEXT_ATTRIBUTE = "io.opentelemetry.instrumentation.context"; + // the class name is part of the attribute name, so that it will be shaded when used in javaagent + // instrumentation, and won't conflict with usage outside javaagent instrumentation + public static final String CONTEXT_ATTRIBUTE = HttpServerTracer.class.getName() + ".Context"; protected static final String USER_AGENT = "User-Agent"; diff --git a/instrumentation/aws-sdk/aws-sdk-2.2/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java b/instrumentation/aws-sdk/aws-sdk-2.2/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java index 497c446175..2e84b052a3 100644 --- a/instrumentation/aws-sdk/aws-sdk-2.2/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java +++ b/instrumentation/aws-sdk/aws-sdk-2.2/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java @@ -27,11 +27,15 @@ import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute; /** AWS request execution interceptor. */ final class TracingExecutionInterceptor implements ExecutionInterceptor { + // the class name is part of the attribute name, so that it will be shaded when used in javaagent + // instrumentation, and won't conflict with usage outside javaagent instrumentation static final ExecutionAttribute CONTEXT_ATTRIBUTE = - new ExecutionAttribute<>("io.opentelemetry.auto.Context"); + new ExecutionAttribute<>(TracingExecutionInterceptor.class.getName() + ".Context"); + // the class name is part of the attribute name, so that it will be shaded when used in javaagent + // instrumentation, and won't conflict with usage outside javaagent instrumentation static final ExecutionAttribute REQUEST_TYPE_ATTRIBUTE = - new ExecutionAttribute<>("io.opentelemetry.auto.aws.RequestType"); + new ExecutionAttribute<>(TracingExecutionInterceptor.class.getName() + ".RequestType"); static final String COMPONENT_NAME = "java-aws-sdk"; diff --git a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AttributeKeys.java b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AttributeKeys.java index a42ceb210e..3ee86150c5 100644 --- a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AttributeKeys.java +++ b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/AttributeKeys.java @@ -9,8 +9,6 @@ import io.netty.util.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.javaagent.instrumentation.api.WeakMap; -import io.opentelemetry.javaagent.instrumentation.netty.v4_0.client.HttpClientTracingHandler; -import io.opentelemetry.javaagent.instrumentation.netty.v4_0.server.HttpServerTracingHandler; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -26,17 +24,17 @@ public class AttributeKeys { } }; - public static final AttributeKey PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY = - attributeKey("io.opentelemetry.javaagent.instrumentation.netty.v4_0.parent.connect.context"); + public static final AttributeKey CONNECT_CONTEXT_ATTRIBUTE_KEY = + attributeKey(AttributeKeys.class.getName() + ".connect.context"); public static final AttributeKey SERVER_ATTRIBUTE_KEY = - attributeKey(HttpServerTracingHandler.class.getName() + ".context"); + attributeKey(AttributeKeys.class.getName() + ".context"); public static final AttributeKey CLIENT_ATTRIBUTE_KEY = - attributeKey(HttpClientTracingHandler.class.getName() + ".span"); + attributeKey(AttributeKeys.class.getName() + ".span"); public static final AttributeKey CLIENT_PARENT_ATTRIBUTE_KEY = - attributeKey(HttpClientTracingHandler.class.getName() + ".parent"); + attributeKey(AttributeKeys.class.getName() + ".parent"); /** * Generate an attribute key or reuse the one existing in the global app map. This implementation diff --git a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/ChannelFutureListenerInstrumentation.java b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/ChannelFutureListenerInstrumentation.java index 252e10bfee..ec7e679cb6 100644 --- a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/ChannelFutureListenerInstrumentation.java +++ b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/ChannelFutureListenerInstrumentation.java @@ -59,7 +59,7 @@ final class ChannelFutureListenerInstrumentation implements TypeInstrumentation return null; } Context parentContext = - future.channel().attr(AttributeKeys.PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY).getAndRemove(); + future.channel().attr(AttributeKeys.CONNECT_CONTEXT_ATTRIBUTE_KEY).getAndRemove(); if (parentContext == null) { return null; } diff --git a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/NettyChannelPipelineInstrumentation.java b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/NettyChannelPipelineInstrumentation.java index 7520b1375d..659be1162f 100644 --- a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/NettyChannelPipelineInstrumentation.java +++ b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/NettyChannelPipelineInstrumentation.java @@ -124,7 +124,7 @@ final class NettyChannelPipelineInstrumentation implements TypeInstrumentation { public static void addParentSpan(@Advice.This ChannelPipeline pipeline) { Context context = Java8BytecodeBridge.currentContext(); Attribute attribute = - pipeline.channel().attr(AttributeKeys.PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY); + pipeline.channel().attr(AttributeKeys.CONNECT_CONTEXT_ATTRIBUTE_KEY); attribute.compareAndSet(null, context); } } diff --git a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/HttpClientRequestTracingHandler.java b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/HttpClientRequestTracingHandler.java index 52cb415510..afe2ee189f 100644 --- a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/HttpClientRequestTracingHandler.java +++ b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/HttpClientRequestTracingHandler.java @@ -30,7 +30,7 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt // TODO pass Context into Tracer.startSpan() and then don't need this scoping Scope parentScope = null; Context parentContext = - ctx.channel().attr(AttributeKeys.PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY).getAndRemove(); + ctx.channel().attr(AttributeKeys.CONNECT_CONTEXT_ATTRIBUTE_KEY).getAndRemove(); if (parentContext != null) { parentScope = parentContext.makeCurrent(); } diff --git a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AttributeKeys.java b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AttributeKeys.java index cfed3085e6..168b54c49b 100644 --- a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AttributeKeys.java +++ b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/AttributeKeys.java @@ -9,8 +9,6 @@ import io.netty.util.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.javaagent.instrumentation.api.WeakMap; -import io.opentelemetry.javaagent.instrumentation.netty.v4_1.client.HttpClientTracingHandler; -import io.opentelemetry.javaagent.instrumentation.netty.v4_1.server.HttpServerTracingHandler; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -26,23 +24,18 @@ public class AttributeKeys { } }; - public static final AttributeKey PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY = - attributeKey("io.opentelemetry.javaagent.instrumentation.netty.v4_1.parent.connect.context"); + public static final AttributeKey CONNECT_CONTEXT_ATTRIBUTE_KEY = + attributeKey(AttributeKeys.class.getName() + "connect.context"); - /** - * This constant is copied over to - * io.opentelemetry.javaagent.instrumentation.ratpack.server.TracingHandler, so if this changes, - * that must also change. - */ + // this attribute key is also used by ratpack instrumentation public static final AttributeKey SERVER_ATTRIBUTE_KEY = - attributeKey(HttpServerTracingHandler.class.getName() + ".context"); + attributeKey(AttributeKeys.class.getName() + ".context"); - // TODO understand and change to context public static final AttributeKey CLIENT_ATTRIBUTE_KEY = - attributeKey(HttpClientTracingHandler.class.getName() + ".span"); + attributeKey(AttributeKeys.class.getName() + ".span"); public static final AttributeKey CLIENT_PARENT_ATTRIBUTE_KEY = - attributeKey(HttpClientTracingHandler.class.getName() + ".parent"); + attributeKey(AttributeKeys.class.getName() + ".parent"); /** * Generate an attribute key or reuse the one existing in the global app map. This implementation diff --git a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelFutureListenerInstrumentation.java b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelFutureListenerInstrumentation.java index ab0bd1e87a..4ca8240fe6 100644 --- a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelFutureListenerInstrumentation.java +++ b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelFutureListenerInstrumentation.java @@ -59,7 +59,7 @@ final class ChannelFutureListenerInstrumentation implements TypeInstrumentation return null; } Context parentContext = - future.channel().attr(AttributeKeys.PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY).getAndRemove(); + future.channel().attr(AttributeKeys.CONNECT_CONTEXT_ATTRIBUTE_KEY).getAndRemove(); if (parentContext == null) { return null; } diff --git a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java index 444009d3e9..9cde2075ca 100644 --- a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java +++ b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java @@ -132,7 +132,7 @@ final class NettyChannelPipelineInstrumentation implements TypeInstrumentation { @Advice.OnMethodEnter public static void addParentSpan(@Advice.This ChannelPipeline pipeline) { Attribute attribute = - pipeline.channel().attr(AttributeKeys.PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY); + pipeline.channel().attr(AttributeKeys.CONNECT_CONTEXT_ATTRIBUTE_KEY); attribute.compareAndSet(null, Java8BytecodeBridge.currentContext()); } } diff --git a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/HttpClientRequestTracingHandler.java b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/HttpClientRequestTracingHandler.java index e48df50706..e29dfc870e 100644 --- a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/HttpClientRequestTracingHandler.java +++ b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/HttpClientRequestTracingHandler.java @@ -30,7 +30,7 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt // TODO pass Context into Tracer.startSpan() and then don't need this scoping Scope parentScope = null; Context parentContext = - ctx.channel().attr(AttributeKeys.PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY).getAndRemove(); + ctx.channel().attr(AttributeKeys.CONNECT_CONTEXT_ATTRIBUTE_KEY).getAndRemove(); if (parentContext != null) { parentScope = parentContext.makeCurrent(); } diff --git a/instrumentation/ratpack-1.4/ratpack-1.4.gradle b/instrumentation/ratpack-1.4/ratpack-1.4.gradle index a909758e76..3b26033a5f 100644 --- a/instrumentation/ratpack-1.4/ratpack-1.4.gradle +++ b/instrumentation/ratpack-1.4/ratpack-1.4.gradle @@ -12,8 +12,9 @@ muzzle { dependencies { library group: 'io.ratpack', name: 'ratpack-core', version: '1.4.0' + implementation project(':instrumentation:netty:netty-4.1') + testLibrary group: 'io.ratpack', name: 'ratpack-groovy-test', version: '1.4.0' - testImplementation project(':instrumentation:netty:netty-4.1') if (JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_11)) { testImplementation group: 'com.sun.activation', name: 'jakarta.activation', version: '1.2.2' } diff --git a/instrumentation/ratpack-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackInstrumentationModule.java b/instrumentation/ratpack-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackInstrumentationModule.java index f4edfc52eb..c24a30bcd4 100644 --- a/instrumentation/ratpack-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackInstrumentationModule.java +++ b/instrumentation/ratpack-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackInstrumentationModule.java @@ -25,6 +25,8 @@ public class RatpackInstrumentationModule extends InstrumentationModule { packageName + ".BlockWrapper", packageName + ".RatpackTracer", packageName + ".TracingHandler", + "io.opentelemetry.javaagent.instrumentation.netty.v4_1.AttributeKeys", + "io.opentelemetry.javaagent.instrumentation.netty.v4_1.AttributeKeys$1" }; } diff --git a/instrumentation/ratpack-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/TracingHandler.java b/instrumentation/ratpack-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/TracingHandler.java index 548fcbf361..4c05fd9025 100644 --- a/instrumentation/ratpack-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/TracingHandler.java +++ b/instrumentation/ratpack-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/TracingHandler.java @@ -8,29 +8,20 @@ package io.opentelemetry.javaagent.instrumentation.ratpack; import static io.opentelemetry.javaagent.instrumentation.ratpack.RatpackTracer.tracer; import io.netty.util.Attribute; -import io.netty.util.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.Span.Kind; import io.opentelemetry.context.Scope; +import io.opentelemetry.javaagent.instrumentation.netty.v4_1.AttributeKeys; import ratpack.handling.Context; import ratpack.handling.Handler; public final class TracingHandler implements Handler { public static final Handler INSTANCE = new TracingHandler(); - /** - * This constant is copied over from - * io.opentelemetry.javaagent.instrumentation.netty.v4_1.AttributeKeys. The key string must be - * kept consistent. - */ - public static final AttributeKey SERVER_ATTRIBUTE_KEY = - AttributeKey.valueOf( - "io.opentelemetry.javaagent.instrumentation.netty.v4_1.server.HttpServerTracingHandler.context"); - @Override public void handle(Context ctx) { Attribute spanAttribute = - ctx.getDirectChannelAccess().getChannel().attr(SERVER_ATTRIBUTE_KEY); + ctx.getDirectChannelAccess().getChannel().attr(AttributeKeys.SERVER_ATTRIBUTE_KEY); io.opentelemetry.context.Context serverSpanContext = spanAttribute.get(); // Relying on executor instrumentation to assume the netty span is in context as the parent. diff --git a/instrumentation/reactor-netty-0.9/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/ReactorNettyInstrumentationModule.java b/instrumentation/reactor-netty-0.9/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/ReactorNettyInstrumentationModule.java index 325e44b58a..3cea9a56da 100644 --- a/instrumentation/reactor-netty-0.9/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/ReactorNettyInstrumentationModule.java +++ b/instrumentation/reactor-netty-0.9/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/ReactorNettyInstrumentationModule.java @@ -103,17 +103,20 @@ public final class ReactorNettyInstrumentationModule extends InstrumentationModu public static class MapConnect implements BiFunction, Bootstrap, Mono> { + + static final String CONTEXT_ATTRIBUTE = MapConnect.class.getName() + ".Context"; + @Override public Mono apply(Mono m, Bootstrap b) { - return m.subscriberContext(s -> s.put("otel_context", Context.current())); + return m.subscriberContext(s -> s.put(CONTEXT_ATTRIBUTE, Context.current())); } } public static class OnRequest implements BiConsumer { @Override public void accept(HttpClientRequest r, Connection c) { - Context context = r.currentContext().get("otel_context"); - c.channel().attr(AttributeKeys.PARENT_CONNECT_CONTEXT_ATTRIBUTE_KEY).set(context); + Context context = r.currentContext().get(MapConnect.CONTEXT_ATTRIBUTE); + c.channel().attr(AttributeKeys.CONNECT_CONTEXT_ATTRIBUTE_KEY).set(context); } } } diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/AdviceUtils.java b/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/AdviceUtils.java index 83c006a2fb..344f6b28af 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/AdviceUtils.java +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/AdviceUtils.java @@ -24,8 +24,7 @@ import reactor.util.context.Context; public class AdviceUtils { - public static final String CONTEXT_ATTRIBUTE = - "io.opentelemetry.javaagent.instrumentation.springwebflux.Context"; + public static final String CONTEXT_ATTRIBUTE = AdviceUtils.class.getName() + ".Context"; public static String parseOperationName(Object handler) { String className = tracer().spanNameForClass(handler.getClass());