diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java index 151d18357db8..ae4cd9220b00 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java @@ -12,7 +12,6 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; @@ -70,8 +69,7 @@ public static Scope activateScope(@Advice.Argument(0) ChannelFuture future) { } Scope parentScope = parentContext.makeCurrent(); if (channelTraceContext.createConnectionSpan()) { - Context errorContext = tracer().startSpan("CONNECT", SpanKind.CLIENT); - tracer().endExceptionally(errorContext, cause); + tracer().connectionFailure(parentContext, future.getChannel(), cause); } return parentScope; } diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyHttpClientTracer.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyHttpClientTracer.java index 7ae7db761398..820daa9b4207 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyHttpClientTracer.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyHttpClientTracer.java @@ -7,6 +7,8 @@ import static io.opentelemetry.api.trace.SpanKind.CLIENT; import static io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.NettyResponseInjectAdapter.SETTER; +import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP; +import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP; import static org.jboss.netty.handler.codec.http.HttpHeaders.Names.HOST; import io.opentelemetry.api.trace.SpanBuilder; @@ -14,11 +16,14 @@ import io.opentelemetry.context.propagation.TextMapSetter; import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer; import io.opentelemetry.instrumentation.api.tracer.net.NetPeerAttributes; +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.net.InetSocketAddress; import java.net.URI; import java.net.URISyntaxException; import org.checkerframework.checker.nullness.qual.Nullable; +import org.jboss.netty.channel.Channel; import org.jboss.netty.channel.ChannelHandlerContext; +import org.jboss.netty.channel.socket.DatagramChannel; import org.jboss.netty.handler.codec.http.HttpHeaders; import org.jboss.netty.handler.codec.http.HttpRequest; import org.jboss.netty.handler.codec.http.HttpResponse; @@ -46,6 +51,17 @@ public Context startSpan(Context parentContext, ChannelHandlerContext ctx, HttpR return context; } + public void connectionFailure(Context parentContext, Channel channel, Throwable throwable) { + SpanBuilder spanBuilder = spanBuilder(parentContext, "CONNECT", CLIENT); + spanBuilder.setAttribute( + SemanticAttributes.NET_TRANSPORT, channel instanceof DatagramChannel ? IP_UDP : IP_TCP); + NetPeerAttributes.INSTANCE.setNetPeer( + spanBuilder, (InetSocketAddress) channel.getRemoteAddress()); + + Context context = withClientSpan(parentContext, spanBuilder.startSpan()); + tracer().endExceptionally(context, throwable); + } + @Override protected String method(HttpRequest httpRequest) { return httpRequest.getMethod().getName(); diff --git a/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ClientTest.groovy b/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ClientTest.groovy index 358e9ba1b078..ce9607fe6b9d 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ClientTest.groovy +++ b/instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ClientTest.groovy @@ -108,7 +108,7 @@ class Netty38ClientTest extends HttpClientTest implements AgentTestTrai } @Override - boolean hasClientSpanAttributes(URI uri) { + boolean hasClientSpanHttpAttributes(URI uri) { switch (uri.toString()) { case "http://localhost:61/": // unopened port case "http://www.google.com:81/": // dropped request diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/ChannelFutureListenerInstrumentation.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/ChannelFutureListenerInstrumentation.java index dce336639e22..aafe13ece1e9 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/ChannelFutureListenerInstrumentation.java +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/ChannelFutureListenerInstrumentation.java @@ -13,7 +13,6 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import io.netty.channel.ChannelFuture; -import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; @@ -62,8 +61,7 @@ public static Scope activateScope(@Advice.Argument(0) ChannelFuture future) { Scope parentScope = parentContext.makeCurrent(); if (tracer().shouldStartSpan(parentContext)) { - Context errorContext = tracer().startSpan("CONNECT", SpanKind.CLIENT); - tracer().endExceptionally(errorContext, cause); + tracer().connectionFailure(parentContext, future.channel(), cause); } return parentScope; } diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/NettyHttpClientTracer.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/NettyHttpClientTracer.java index a52264c03eee..872b45bb9e10 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/NettyHttpClientTracer.java +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/NettyHttpClientTracer.java @@ -8,8 +8,12 @@ import static io.netty.handler.codec.http.HttpHeaders.Names.HOST; import static io.opentelemetry.api.trace.SpanKind.CLIENT; import static io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyResponseInjectAdapter.SETTER; +import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP; +import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP; +import io.netty.channel.Channel; import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.socket.DatagramChannel; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponse; @@ -18,6 +22,7 @@ import io.opentelemetry.context.propagation.TextMapSetter; import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer; import io.opentelemetry.instrumentation.api.tracer.net.NetPeerAttributes; +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.net.InetSocketAddress; import java.net.URI; import java.net.URISyntaxException; @@ -46,6 +51,16 @@ public Context startSpan(Context parentContext, ChannelHandlerContext ctx, HttpR return context; } + public void connectionFailure(Context parentContext, Channel channel, Throwable throwable) { + SpanBuilder spanBuilder = spanBuilder(parentContext, "CONNECT", CLIENT); + spanBuilder.setAttribute( + SemanticAttributes.NET_TRANSPORT, channel instanceof DatagramChannel ? IP_UDP : IP_TCP); + NetPeerAttributes.INSTANCE.setNetPeer(spanBuilder, (InetSocketAddress) channel.remoteAddress()); + + Context context = withClientSpan(parentContext, spanBuilder.startSpan()); + tracer().endExceptionally(context, throwable); + } + @Override protected String method(HttpRequest httpRequest) { return httpRequest.getMethod().name(); diff --git a/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ClientTest.groovy b/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ClientTest.groovy index 35783e63966b..b603b4e813eb 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ClientTest.groovy +++ b/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ClientTest.groovy @@ -92,7 +92,7 @@ class Netty40ClientTest extends HttpClientTest implement } @Override - boolean hasClientSpanAttributes(URI uri) { + boolean hasClientSpanHttpAttributes(URI uri) { switch (uri.toString()) { case "http://localhost:61/": // unopened port case "http://www.google.com:81/": // dropped request diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelFutureListenerInstrumentation.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelFutureListenerInstrumentation.java index 44765bc57af7..f2451e152125 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelFutureListenerInstrumentation.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelFutureListenerInstrumentation.java @@ -63,8 +63,7 @@ public static Scope activateScope(@Advice.Argument(0) ChannelFuture future) { Scope parentScope = parentContext.makeCurrent(); if (tracer().shouldStartSpan(parentContext, SpanKind.CLIENT)) { - Context errorContext = tracer().startSpan("CONNECT", SpanKind.CLIENT); - tracer().endExceptionally(errorContext, cause); + tracer().connectionFailure(parentContext, future.channel(), cause); } return parentScope; } diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/NettyHttpClientTracer.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/NettyHttpClientTracer.java index 6e1fb768720c..34b2de388205 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/NettyHttpClientTracer.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/NettyHttpClientTracer.java @@ -8,8 +8,12 @@ import static io.netty.handler.codec.http.HttpHeaderNames.HOST; import static io.opentelemetry.api.trace.SpanKind.CLIENT; import static io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyResponseInjectAdapter.SETTER; +import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP; +import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP; +import io.netty.channel.Channel; import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.socket.DatagramChannel; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponse; @@ -18,6 +22,7 @@ import io.opentelemetry.context.propagation.TextMapSetter; import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer; import io.opentelemetry.instrumentation.api.tracer.net.NetPeerAttributes; +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.net.InetSocketAddress; import java.net.URI; import java.net.URISyntaxException; @@ -46,6 +51,16 @@ public Context startSpan(Context parentContext, ChannelHandlerContext ctx, HttpR return context; } + public void connectionFailure(Context parentContext, Channel channel, Throwable throwable) { + SpanBuilder spanBuilder = spanBuilder(parentContext, "CONNECT", CLIENT); + spanBuilder.setAttribute( + SemanticAttributes.NET_TRANSPORT, channel instanceof DatagramChannel ? IP_UDP : IP_TCP); + NetPeerAttributes.INSTANCE.setNetPeer(spanBuilder, (InetSocketAddress) channel.remoteAddress()); + + Context context = withClientSpan(parentContext, spanBuilder.startSpan()); + tracer().endExceptionally(context, throwable); + } + @Override protected String method(HttpRequest httpRequest) { return httpRequest.method().name(); diff --git a/instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ClientTest.groovy b/instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ClientTest.groovy index 766071e09183..a21f1b70eabd 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ClientTest.groovy +++ b/instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ClientTest.groovy @@ -110,7 +110,7 @@ class Netty41ClientTest extends HttpClientTest implement } @Override - boolean hasClientSpanAttributes(URI uri) { + boolean hasClientSpanHttpAttributes(URI uri) { switch (uri.toString()) { case "http://localhost:61/": // unopened port case "http://www.google.com:81/": // dropped request diff --git a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackHttpClientTest.groovy b/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackHttpClientTest.groovy index 494b8a7cdffe..c78af5ff9ff6 100644 --- a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackHttpClientTest.groovy +++ b/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackHttpClientTest.groovy @@ -99,7 +99,7 @@ class RatpackHttpClientTest extends HttpClientTest implements AgentTestTra } @Override - boolean hasClientSpanAttributes(URI uri) { + boolean hasClientSpanHttpAttributes(URI uri) { switch (uri.toString()) { case "http://localhost:61/": // unopened port case "http://www.google.com:81/": // dropped request diff --git a/instrumentation/reactor-netty/reactor-netty-0.9/javaagent/src/test/groovy/AbstractReactorNettyHttpClientTest.groovy b/instrumentation/reactor-netty/reactor-netty-0.9/javaagent/src/test/groovy/AbstractReactorNettyHttpClientTest.groovy index 2e0020e7b122..a09a27b66555 100644 --- a/instrumentation/reactor-netty/reactor-netty-0.9/javaagent/src/test/groovy/AbstractReactorNettyHttpClientTest.groovy +++ b/instrumentation/reactor-netty/reactor-netty-0.9/javaagent/src/test/groovy/AbstractReactorNettyHttpClientTest.groovy @@ -84,7 +84,7 @@ abstract class AbstractReactorNettyHttpClientTest extends HttpClientTest extends InstrumentationSpecification { void clientSpan(TraceAssert trace, int index, Object parentSpan, String method = "GET", URI uri = server.address.resolve("/success"), Integer responseCode = 200, Throwable exception = null, String httpFlavor = "1.1") { def userAgent = userAgent() def extraAttributes = extraAttributes() + def hasClientSpanHttpAttributes = hasClientSpanHttpAttributes(uri) trace.span(index) { if (parentSpan == null) { hasNoParent() @@ -835,25 +836,25 @@ abstract class HttpClientTest extends InstrumentationSpecification { } else if (responseCode >= 400) { status ERROR } - if (hasClientSpanAttributes(uri)) { - attributes { - "${SemanticAttributes.NET_TRANSPORT.key}" IP_TCP - if (uri.port == UNUSABLE_PORT || uri.host == "192.0.2.1" || (uri.host == "www.google.com" && uri.port == 81)) { - // TODO(anuraaga): For theses cases, there isn't actually a peer so we shouldn't be - // filling in peer information but some instrumentation does so based on the URL itself - // which is present in HTTP attributes. We should fix this. - "${SemanticAttributes.NET_PEER_NAME.key}" { it == null || it == uri.host } - "${SemanticAttributes.NET_PEER_PORT.key}" { it == null || it == uri.port || (uri.scheme == "https" && it == 443) } - } else { - "${SemanticAttributes.NET_PEER_NAME.key}" uri.host - "${SemanticAttributes.NET_PEER_PORT.key}" uri.port > 0 ? uri.port : { it == null || it == 443 } - } - if (uri.host == "www.google.com") { - // unpredictable IP address (or can be none if no connection is made, see comment above) - "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it instanceof String } - } else { - "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" || it == uri.host } // Optional - } + attributes { + "${SemanticAttributes.NET_TRANSPORT.key}" IP_TCP + if (uri.port == UNUSABLE_PORT || uri.host == "192.0.2.1" || (uri.host == "www.google.com" && uri.port == 81)) { + // TODO(anuraaga): For theses cases, there isn't actually a peer so we shouldn't be + // filling in peer information but some instrumentation does so based on the URL itself + // which is present in HTTP attributes. We should fix this. + "${SemanticAttributes.NET_PEER_NAME.key}" { it == null || it == uri.host } + "${SemanticAttributes.NET_PEER_PORT.key}" { it == null || it == uri.port || (uri.scheme == "https" && it == 443) } + } else { + "${SemanticAttributes.NET_PEER_NAME.key}" uri.host + "${SemanticAttributes.NET_PEER_PORT.key}" uri.port > 0 ? uri.port : { it == null || it == 443 } + } + if (uri.host == "www.google.com") { + // unpredictable IP address (or can be none if no connection is made, see comment above) + "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it instanceof String } + } else { + "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" || it == uri.host } // Optional + } + if (hasClientSpanHttpAttributes) { "${SemanticAttributes.HTTP_URL.key}" { it == "${uri}" || it == "${removeFragment(uri)}" } "${SemanticAttributes.HTTP_METHOD.key}" method if (uri.host == "www.google.com") { @@ -864,25 +865,25 @@ abstract class HttpClientTest extends InstrumentationSpecification { if (userAgent) { "${SemanticAttributes.HTTP_USER_AGENT.key}" { it.startsWith(userAgent) } } - if (responseCode) { - "${SemanticAttributes.HTTP_STATUS_CODE.key}" responseCode - } + } + if (responseCode) { + "${SemanticAttributes.HTTP_STATUS_CODE.key}" responseCode + } - if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) { - "${SemanticAttributes.HTTP_HOST}" { it == uri.host || it == "${uri.host}:${uri.port}" } - } - if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { - "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long - } - if (extraAttributes.contains(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH)) { - "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH}" Long - } - if (extraAttributes.contains(SemanticAttributes.HTTP_SCHEME)) { - "${SemanticAttributes.HTTP_SCHEME}" uri.scheme - } - if (extraAttributes.contains(SemanticAttributes.HTTP_TARGET)) { - "${SemanticAttributes.HTTP_TARGET}" uri.path + "${uri.query != null ? "?${uri.query}" : ""}" - } + if (extraAttributes.contains(SemanticAttributes.HTTP_HOST)) { + "${SemanticAttributes.HTTP_HOST}" { it == uri.host || it == "${uri.host}:${uri.port}" } + } + if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) { + "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long + } + if (extraAttributes.contains(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH)) { + "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH}" Long + } + if (extraAttributes.contains(SemanticAttributes.HTTP_SCHEME)) { + "${SemanticAttributes.HTTP_SCHEME}" uri.scheme + } + if (extraAttributes.contains(SemanticAttributes.HTTP_TARGET)) { + "${SemanticAttributes.HTTP_TARGET}" uri.path + "${uri.query != null ? "?${uri.query}" : ""}" } } } @@ -921,7 +922,7 @@ abstract class HttpClientTest extends InstrumentationSpecification { spanAssert.errorEvent(errorType, message) } - boolean hasClientSpanAttributes(URI uri) { + boolean hasClientSpanHttpAttributes(URI uri) { true }