Add attributes to netty connection failure span (#3115)

This commit is contained in:
Lauri Tulmin 2021-05-29 01:01:10 +03:00 committed by GitHub
parent ed88cca533
commit e16cf3001f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 94 additions and 52 deletions

View File

@ -12,7 +12,6 @@ import static net.bytebuddy.matcher.ElementMatchers.isMethod;
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 class ChannelFutureListenerInstrumentation implements TypeInstrumentation
}
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;
}

View File

@ -7,6 +7,8 @@ package io.opentelemetry.javaagent.instrumentation.netty.v3_8.client;
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.Context;
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 class NettyHttpClientTracer
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();

View File

@ -108,7 +108,7 @@ class Netty38ClientTest extends HttpClientTest<Request> 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

View File

@ -13,7 +13,6 @@ import static net.bytebuddy.matcher.ElementMatchers.named;
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 class ChannelFutureListenerInstrumentation implements TypeInstrumentation
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;
}

View File

@ -8,8 +8,12 @@ package io.opentelemetry.javaagent.instrumentation.netty.v4_0.client;
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.Context;
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 class NettyHttpClientTracer
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();

View File

@ -92,7 +92,7 @@ class Netty40ClientTest extends HttpClientTest<DefaultFullHttpRequest> 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

View File

@ -63,8 +63,7 @@ public class ChannelFutureListenerInstrumentation implements TypeInstrumentation
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;
}

View File

@ -8,8 +8,12 @@ package io.opentelemetry.javaagent.instrumentation.netty.v4_1.client;
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.Context;
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 class NettyHttpClientTracer
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();

View File

@ -110,7 +110,7 @@ class Netty41ClientTest extends HttpClientTest<DefaultFullHttpRequest> 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

View File

@ -99,7 +99,7 @@ class RatpackHttpClientTest extends HttpClientTest<Void> 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

View File

@ -84,7 +84,7 @@ abstract class AbstractReactorNettyHttpClientTest extends HttpClientTest<HttpCli
}
@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

View File

@ -84,7 +84,7 @@ abstract class AbstractReactorNettyHttpClientTest extends HttpClientTest<HttpCli
}
@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

View File

@ -821,6 +821,7 @@ abstract class HttpClientTest<REQUEST> 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<REQUEST> 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<REQUEST> 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<REQUEST> extends InstrumentationSpecification {
spanAssert.errorEvent(errorType, message)
}
boolean hasClientSpanAttributes(URI uri) {
boolean hasClientSpanHttpAttributes(URI uri) {
true
}