From 9b3df4ff9e793962bccdd0eed6c8124fda31095d Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 13 Jun 2018 10:15:59 +1000 Subject: [PATCH 1/3] Netty HTTP client and server instrumentation first pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doesn’t target right versions. Needs lots of testing. --- .../trace/agent/tooling/AgentInstaller.java | 6 +- .../jetty8/HandlerInstrumentation.java | 8 +- .../instrumentation/netty/netty.gradle | 19 +++ .../NettyChannelPipelineInstrumentation.java | 125 ++++++++++++++++++ .../HttpClientRequestTracingHandler.java | 64 +++++++++ .../HttpClientResponseTracingHandler.java | 40 ++++++ .../client/HttpClientTracingHandler.java | 12 ++ .../client/NettyResponseInjectAdapter.java | 25 ++++ .../HttpServerRequestTracingHandler.java | 74 +++++++++++ .../HttpServerResponseTracingHandler.java | 41 ++++++ .../server/HttpServerTracingHandler.java | 12 ++ .../server/NettyRequestExtractAdapter.java | 25 ++++ .../src/test/groovy/NettyClientTest.groovy | 97 ++++++++++++++ .../src/test/groovy/NettyServerTest.groovy | 111 ++++++++++++++++ .../servlet3/HttpServlet3Instrumentation.java | 10 +- settings.gradle | 1 + 16 files changed, 660 insertions(+), 10 deletions(-) create mode 100644 dd-java-agent/instrumentation/netty/netty.gradle create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/NettyChannelPipelineInstrumentation.java create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientRequestTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientResponseTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/NettyResponseInjectAdapter.java create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerRequestTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerResponseTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/NettyRequestExtractAdapter.java create mode 100644 dd-java-agent/instrumentation/netty/src/test/groovy/NettyClientTest.groovy create mode 100644 dd-java-agent/instrumentation/netty/src/test/groovy/NettyServerTest.groovy diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index 26c0d1ce90..685d88e9ec 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -98,7 +98,11 @@ public class AgentInstaller { log.debug("--{}", mismatch); } } else { - log.debug("Failed to handle {} for transformation: {}", typeName, throwable.getMessage()); + log.debug( + "Failed to handle {} for transformation on classloader {}: {}", + typeName, + classLoader, + throwable.getMessage()); } } diff --git a/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java b/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java index 566e8e85a5..f62cdc467c 100644 --- a/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java @@ -124,7 +124,7 @@ public final class HandlerInstrumentation extends Instrumenter.Configurable { Tags.ERROR.set(span, Boolean.TRUE); span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); scope.close(); - scope.span().finish(); // Finish the span manually since finishSpanOnClose was false + span.finish(); // Finish the span manually since finishSpanOnClose was false } else if (req.isAsyncStarted()) { final AtomicBoolean activated = new AtomicBoolean(false); // what if async is already finished? This would not be called @@ -149,7 +149,7 @@ public final class HandlerInstrumentation extends Instrumenter.Configurable { @Override public void onComplete(final AsyncEvent event) throws IOException { if (activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { Tags.HTTP_STATUS.set( span, ((HttpServletResponse) event.getSuppliedResponse()).getStatus()); } @@ -159,7 +159,7 @@ public final class HandlerInstrumentation extends Instrumenter.Configurable { @Override public void onTimeout(final AsyncEvent event) throws IOException { if (activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { Tags.ERROR.set(span, Boolean.TRUE); span.setTag("timeout", event.getAsyncContext().getTimeout()); } @@ -169,7 +169,7 @@ public final class HandlerInstrumentation extends Instrumenter.Configurable { @Override public void onError(final AsyncEvent event) throws IOException { if (event.getThrowable() != null && activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { if (((HttpServletResponse) event.getSuppliedResponse()).getStatus() == HttpServletResponse.SC_OK) { // exception is thrown in filter chain, but status code is incorrect diff --git a/dd-java-agent/instrumentation/netty/netty.gradle b/dd-java-agent/instrumentation/netty/netty.gradle new file mode 100644 index 0000000000..53c8554931 --- /dev/null +++ b/dd-java-agent/instrumentation/netty/netty.gradle @@ -0,0 +1,19 @@ +apply from: "${rootDir}/gradle/java.gradle" + + +dependencies { + compileOnly group: 'io.netty', name: 'netty-all', version: '4.1.25.Final' + + compile project(':dd-java-agent:agent-tooling') + + compile deps.bytebuddy + compile deps.opentracing + annotationProcessor deps.autoservice + implementation deps.autoservice + + testCompile project(':dd-java-agent:testing') + + testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.4.11.v20180605' + testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' + testCompile group: 'org.asynchttpclient', name: 'async-http-client', version: '2.4.9' +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/NettyChannelPipelineInstrumentation.java new file mode 100644 index 0000000000..5958684ed3 --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/NettyChannelPipelineInstrumentation.java @@ -0,0 +1,125 @@ +package datadog.trace.instrumentation.netty; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.DDAdvice; +import datadog.trace.agent.tooling.DDTransformers; +import datadog.trace.agent.tooling.HelperInjector; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import datadog.trace.instrumentation.netty.client.HttpClientRequestTracingHandler; +import datadog.trace.instrumentation.netty.client.HttpClientResponseTracingHandler; +import datadog.trace.instrumentation.netty.client.HttpClientTracingHandler; +import datadog.trace.instrumentation.netty.server.HttpServerRequestTracingHandler; +import datadog.trace.instrumentation.netty.server.HttpServerResponseTracingHandler; +import datadog.trace.instrumentation.netty.server.HttpServerTracingHandler; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelPipeline; +import io.netty.handler.codec.http.HttpClientCodec; +import io.netty.handler.codec.http.HttpRequestDecoder; +import io.netty.handler.codec.http.HttpRequestEncoder; +import io.netty.handler.codec.http.HttpResponseDecoder; +import io.netty.handler.codec.http.HttpResponseEncoder; +import io.netty.handler.codec.http.HttpServerCodec; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public class NettyChannelPipelineInstrumentation extends Instrumenter.Configurable { + + public NettyChannelPipelineInstrumentation() { + super("netty", "netty-http-server"); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public AgentBuilder apply(final AgentBuilder agentBuilder) { + return agentBuilder + .type(not(isInterface()).and(hasSuperType(named("io.netty.channel.ChannelPipeline")))) + .transform( + new HelperInjector( + // client helpers + "datadog.trace.instrumentation.netty.client.NettyResponseInjectAdapter", + "datadog.trace.instrumentation.netty.client.HttpClientRequestTracingHandler", + "datadog.trace.instrumentation.netty.client.HttpClientResponseTracingHandler", + "datadog.trace.instrumentation.netty.client.HttpClientTracingHandler", + // server helpers + "datadog.trace.instrumentation.netty.server.NettyRequestExtractAdapter", + "datadog.trace.instrumentation.netty.server.HttpServerRequestTracingHandler", + "datadog.trace.instrumentation.netty.server.HttpServerResponseTracingHandler", + "datadog.trace.instrumentation.netty.server.HttpServerTracingHandler")) + .transform(DDTransformers.defaultTransformers()) + .transform( + DDAdvice.create() + .advice( + isMethod() + .and(nameStartsWith("add")) + .and(takesArgument(2, named("io.netty.channel.ChannelHandler"))), + ChannelPipelineAddAdvice.class.getName())) + .asDecorator(); + } + + /** + * When certain handlers are added to the pipeline, we want to add our corresponding tracing + * handlers. If those handlers are later removed, we may want to remove our handlers. That is not + * currently implemented. + */ + public static class ChannelPipelineAddAdvice { + @Advice.OnMethodEnter + public static int checkDepth() { + return CallDepthThreadLocalMap.incrementCallDepth(ChannelPipeline.class); + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void addHandler( + @Advice.Enter final int depth, + @Advice.This final ChannelPipeline pipeline, + @Advice.Argument(2) final ChannelHandler handler) { + if (depth > 0) return; + + try { + // Server pipeline handlers + if (handler instanceof HttpServerCodec) { + pipeline.addLast( + HttpServerTracingHandler.class.getName(), new HttpServerTracingHandler()); + } else if (handler instanceof HttpRequestDecoder) { + pipeline.addLast( + HttpServerRequestTracingHandler.class.getName(), + new HttpServerRequestTracingHandler()); + } else if (handler instanceof HttpResponseEncoder) { + pipeline.addLast( + HttpServerResponseTracingHandler.class.getName(), + new HttpServerResponseTracingHandler()); + } else + // Client pipeline handlers + if (handler instanceof HttpClientCodec) { + pipeline.addLast( + HttpClientTracingHandler.class.getName(), new HttpClientTracingHandler()); + } else if (handler instanceof HttpRequestEncoder) { + pipeline.addLast( + HttpClientRequestTracingHandler.class.getName(), + new HttpClientRequestTracingHandler()); + } else if (handler instanceof HttpResponseDecoder) { + pipeline.addLast( + HttpClientResponseTracingHandler.class.getName(), + new HttpClientResponseTracingHandler()); + } + } catch (final IllegalArgumentException e) { + // Prevented adding duplicate handlers. + } finally { + CallDepthThreadLocalMap.reset(ChannelPipeline.class); + } + } + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientRequestTracingHandler.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientRequestTracingHandler.java new file mode 100644 index 0000000000..64cac3fea5 --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientRequestTracingHandler.java @@ -0,0 +1,64 @@ +package datadog.trace.instrumentation.netty.client; + +import static io.opentracing.log.Fields.ERROR_OBJECT; + +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelOutboundHandlerAdapter; +import io.netty.channel.ChannelPromise; +import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpRequest; +import io.netty.util.AttributeKey; +import io.opentracing.Span; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import java.net.InetSocketAddress; +import java.util.Collections; + +public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapter { + + private static final AttributeKey attributeKey = AttributeKey.valueOf(Span.class.getName()); + + @Override + public void write(final ChannelHandlerContext ctx, final Object msg, final ChannelPromise prm) { + if (!(msg instanceof HttpRequest)) { + ctx.write(msg, prm); + return; + } + final HttpRequest request = (HttpRequest) msg; + final InetSocketAddress remoteAddress = (InetSocketAddress) ctx.channel().remoteAddress(); + + String url = request.uri(); + if (request.headers().contains(HttpHeaderNames.HOST)) { + url = "http://" + request.headers().get(HttpHeaderNames.HOST) + url; + } + final Span span = + GlobalTracer.get() + .buildSpan("netty.client.request") + .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) + .withTag(Tags.PEER_HOSTNAME.getKey(), remoteAddress.getHostName()) + .withTag(Tags.PEER_PORT.getKey(), remoteAddress.getPort()) + .withTag(Tags.HTTP_METHOD.getKey(), request.method().name()) + .withTag(Tags.HTTP_URL.getKey(), url) + .withTag(Tags.COMPONENT.getKey(), "netty-client") + .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) + .start(); + + GlobalTracer.get() + .inject( + span.context(), Format.Builtin.HTTP_HEADERS, new NettyResponseInjectAdapter(request)); + + ctx.channel().attr(attributeKey).set(span); + + try { + ctx.write(msg, prm); + } catch (final Throwable throwable) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientResponseTracingHandler.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientResponseTracingHandler.java new file mode 100644 index 0000000000..25f6aa792b --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientResponseTracingHandler.java @@ -0,0 +1,40 @@ +package datadog.trace.instrumentation.netty.client; + +import static io.opentracing.log.Fields.ERROR_OBJECT; + +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.util.AttributeKey; +import io.opentracing.Span; +import io.opentracing.tag.Tags; +import java.util.Collections; + +public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter { + + private static final AttributeKey attributeKey = AttributeKey.valueOf(Span.class.getName()); + + @Override + public void channelRead(final ChannelHandlerContext ctx, final Object msg) { + final Span span = ctx.channel().attr(attributeKey).get(); + if (span == null || !(msg instanceof HttpResponse)) { + ctx.fireChannelRead(msg); + return; + } + + final HttpResponse response = (HttpResponse) msg; + + try { + ctx.fireChannelRead(msg); + } catch (final Throwable throwable) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + Tags.HTTP_STATUS.set(span, 500); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } + + Tags.HTTP_STATUS.set(span, response.status().code()); + span.finish(); // Finish the span manually since finishSpanOnClose was false + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientTracingHandler.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientTracingHandler.java new file mode 100644 index 0000000000..fe70a2739f --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientTracingHandler.java @@ -0,0 +1,12 @@ +package datadog.trace.instrumentation.netty.client; + +import io.netty.channel.CombinedChannelDuplexHandler; + +public class HttpClientTracingHandler + extends CombinedChannelDuplexHandler< + HttpClientResponseTracingHandler, HttpClientRequestTracingHandler> { + + public HttpClientTracingHandler() { + super(new HttpClientResponseTracingHandler(), new HttpClientRequestTracingHandler()); + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/NettyResponseInjectAdapter.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/NettyResponseInjectAdapter.java new file mode 100644 index 0000000000..6636c05de6 --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/NettyResponseInjectAdapter.java @@ -0,0 +1,25 @@ +package datadog.trace.instrumentation.netty.client; + +import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpRequest; +import io.opentracing.propagation.TextMap; +import java.util.Iterator; +import java.util.Map; + +public class NettyResponseInjectAdapter implements TextMap { + private final HttpHeaders headers; + + NettyResponseInjectAdapter(final HttpRequest request) { + this.headers = request.headers(); + } + + @Override + public Iterator> iterator() { + throw new UnsupportedOperationException("This class should be used only with Tracer.inject()!"); + } + + @Override + public void put(final String key, final String value) { + headers.set(key, value); + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerRequestTracingHandler.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerRequestTracingHandler.java new file mode 100644 index 0000000000..200e39375a --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerRequestTracingHandler.java @@ -0,0 +1,74 @@ +package datadog.trace.instrumentation.netty.server; + +import static io.opentracing.log.Fields.ERROR_OBJECT; + +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import datadog.trace.context.TraceScope; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpRequest; +import io.netty.util.AttributeKey; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.SpanContext; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import java.net.InetSocketAddress; +import java.util.Collections; + +public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapter { + + private static final AttributeKey attributeKey = AttributeKey.valueOf(Span.class.getName()); + + @Override + public void channelRead(final ChannelHandlerContext ctx, final Object msg) { + if (!(msg instanceof HttpRequest)) { + ctx.fireChannelRead(msg); // superclass does not throw + return; + } + final HttpRequest request = (HttpRequest) msg; + final InetSocketAddress remoteAddress = (InetSocketAddress) ctx.channel().remoteAddress(); + + final SpanContext extractedContext = + GlobalTracer.get() + .extract(Format.Builtin.HTTP_HEADERS, new NettyRequestExtractAdapter(request)); + + String url = request.uri(); + if (request.headers().contains(HttpHeaderNames.HOST)) { + url = "http://" + request.headers().get(HttpHeaderNames.HOST) + url; + } + final Scope scope = + GlobalTracer.get() + .buildSpan("netty.request") + .asChildOf(extractedContext) + .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) + .withTag(Tags.PEER_HOSTNAME.getKey(), remoteAddress.getHostName()) + .withTag(Tags.PEER_PORT.getKey(), remoteAddress.getPort()) + .withTag(Tags.HTTP_METHOD.getKey(), request.method().name()) + .withTag(Tags.HTTP_URL.getKey(), url) + .withTag(Tags.COMPONENT.getKey(), "netty") + .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET) + .startActive(false); + + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } + + final Span span = scope.span(); + ctx.channel().attr(attributeKey).set(span); + + try { + ctx.fireChannelRead(msg); + } catch (final Throwable throwable) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } finally { + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerResponseTracingHandler.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerResponseTracingHandler.java new file mode 100644 index 0000000000..d50a05550a --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerResponseTracingHandler.java @@ -0,0 +1,41 @@ +package datadog.trace.instrumentation.netty.server; + +import static io.opentracing.log.Fields.ERROR_OBJECT; + +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelOutboundHandlerAdapter; +import io.netty.channel.ChannelPromise; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.util.AttributeKey; +import io.opentracing.Span; +import io.opentracing.tag.Tags; +import java.util.Collections; + +public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdapter { + + private static final AttributeKey attributeKey = AttributeKey.valueOf(Span.class.getName()); + + @Override + public void write(final ChannelHandlerContext ctx, final Object msg, final ChannelPromise prm) { + final Span span = ctx.channel().attr(attributeKey).get(); + if (span == null || !(msg instanceof HttpResponse)) { + ctx.write(msg, prm); + return; + } + + final HttpResponse response = (HttpResponse) msg; + + try { + ctx.write(msg, prm); + } catch (final Throwable throwable) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + Tags.HTTP_STATUS.set(span, 500); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } + + Tags.HTTP_STATUS.set(span, response.status().code()); + span.finish(); // Finish the span manually since finishSpanOnClose was false + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerTracingHandler.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerTracingHandler.java new file mode 100644 index 0000000000..dc15fecace --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerTracingHandler.java @@ -0,0 +1,12 @@ +package datadog.trace.instrumentation.netty.server; + +import io.netty.channel.CombinedChannelDuplexHandler; + +public class HttpServerTracingHandler + extends CombinedChannelDuplexHandler< + HttpServerRequestTracingHandler, HttpServerResponseTracingHandler> { + + public HttpServerTracingHandler() { + super(new HttpServerRequestTracingHandler(), new HttpServerResponseTracingHandler()); + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/NettyRequestExtractAdapter.java b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/NettyRequestExtractAdapter.java new file mode 100644 index 0000000000..35b856c5b1 --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/NettyRequestExtractAdapter.java @@ -0,0 +1,25 @@ +package datadog.trace.instrumentation.netty.server; + +import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpRequest; +import io.opentracing.propagation.TextMap; +import java.util.Iterator; +import java.util.Map; + +public class NettyRequestExtractAdapter implements TextMap { + private final HttpHeaders headers; + + NettyRequestExtractAdapter(final HttpRequest request) { + this.headers = request.headers(); + } + + @Override + public Iterator> iterator() { + return headers.iteratorAsString(); + } + + @Override + public void put(final String key, final String value) { + throw new UnsupportedOperationException("This class should be used only with Tracer.inject()!"); + } +} diff --git a/dd-java-agent/instrumentation/netty/src/test/groovy/NettyClientTest.groovy b/dd-java-agent/instrumentation/netty/src/test/groovy/NettyClientTest.groovy new file mode 100644 index 0000000000..c83fc82155 --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/test/groovy/NettyClientTest.groovy @@ -0,0 +1,97 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TestUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import io.opentracing.tag.Tags +import org.asynchttpclient.AsyncHttpClient +import org.eclipse.jetty.server.Handler +import org.eclipse.jetty.server.Request +import org.eclipse.jetty.server.Server +import org.eclipse.jetty.server.handler.AbstractHandler +import org.eclipse.jetty.util.MultiMap +import spock.lang.Shared + +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +import static datadog.trace.agent.test.ListWriterAssert.assertTraces +import static org.asynchttpclient.Dsl.asyncHttpClient + +class NettyClientTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.netty.enabled", "true") + } + + static final PORT = TestUtils.randomOpenPort() + + @Shared + Server server = new Server(PORT) + @Shared + AsyncHttpClient asyncHttpClient = asyncHttpClient() +// DefaultAsyncHttpClientConfig.Builder.newInstance().setRequestTimeout(Integer.MAX_VALUE).build()) + @Shared + def headers = new MultiMap() + + + def setupSpec() { + Handler handler = [ + handle: { String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response -> + request.getHeaderNames().each { + headers.add(it, request.getHeader(it)) + } + response.setContentType("text/plaincharset=utf-8") + response.setStatus(HttpServletResponse.SC_OK) + baseRequest.setHandled(true) + response.getWriter().println("Hello World") + } + ] as AbstractHandler + server.setHandler(handler) + server.start() + } + + def cleanupSpec() { + server.stop() + } + + def cleanup() { + headers.clear() + } + + def "test server request/response"() { + setup: + def responseFuture = asyncHttpClient.prepareGet("http://localhost:$PORT/").execute() + def response = responseFuture.get() + + expect: + response.statusCode == 200 + response.responseBody == "Hello World\n" + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "unnamed-java-app" + operationName "netty.client.request" + resourceName "GET /" + spanType DDSpanTypes.HTTP_CLIENT + errored false + tags { + "$Tags.COMPONENT.key" "netty-client" + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "http://localhost:$PORT/" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" Integer + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + defaultTags() + } + } + } + } + + and: + headers["x-datadog-trace-id"] == ["${TEST_WRITER.get(0).get(0).traceId}"] + headers["x-datadog-parent-id"] == ["${TEST_WRITER.get(0).get(0).spanId}"] + } +} diff --git a/dd-java-agent/instrumentation/netty/src/test/groovy/NettyServerTest.groovy b/dd-java-agent/instrumentation/netty/src/test/groovy/NettyServerTest.groovy new file mode 100644 index 0000000000..2d048474cc --- /dev/null +++ b/dd-java-agent/instrumentation/netty/src/test/groovy/NettyServerTest.groovy @@ -0,0 +1,111 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TestUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import io.netty.bootstrap.ServerBootstrap +import io.netty.buffer.ByteBuf +import io.netty.buffer.Unpooled +import io.netty.channel.ChannelInitializer +import io.netty.channel.EventLoopGroup +import io.netty.channel.SimpleChannelInboundHandler +import io.netty.channel.nio.NioEventLoopGroup +import io.netty.channel.socket.nio.NioServerSocketChannel +import io.netty.handler.codec.http.DefaultFullHttpResponse +import io.netty.handler.codec.http.FullHttpResponse +import io.netty.handler.codec.http.HttpHeaderNames +import io.netty.handler.codec.http.HttpResponseStatus +import io.netty.handler.codec.http.HttpServerCodec +import io.netty.handler.codec.http.HttpVersion +import io.netty.handler.codec.http.LastHttpContent +import io.netty.handler.logging.LogLevel +import io.netty.handler.logging.LoggingHandler +import io.netty.util.CharsetUtil +import io.opentracing.tag.Tags +import okhttp3.OkHttpClient +import okhttp3.Request +import spock.lang.Shared + +import static datadog.trace.agent.test.ListWriterAssert.assertTraces + +class NettyServerTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.netty.enabled", "true") + } + + static final PORT = TestUtils.randomOpenPort() + + @Shared + EventLoopGroup eventLoopGroup = new NioEventLoopGroup() + + OkHttpClient client = new OkHttpClient.Builder() + // Uncomment when debugging: +// .connectTimeout(1, TimeUnit.HOURS) +// .writeTimeout(1, TimeUnit.HOURS) +// .readTimeout(1, TimeUnit.HOURS) + .build() + + def setupSpec() { + ServerBootstrap bootstrap = new ServerBootstrap() + .group(eventLoopGroup) + .handler(new LoggingHandler(LogLevel.INFO)) + .childHandler([ + initChannel: { ch -> + def pipeline = ch.pipeline() + pipeline.addLast(new HttpServerCodec()) + pipeline.addLast([ + channelRead0 : { ctx, msg -> + if (msg instanceof LastHttpContent) { + ByteBuf content = Unpooled.copiedBuffer("Hello World", CharsetUtil.UTF_8) + FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK, content) + response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/plain") + response.headers().set(HttpHeaderNames.CONTENT_LENGTH, content.readableBytes()) + ctx.write(response) + } + }, + channelReadComplete: { it.flush() } + ] as SimpleChannelInboundHandler) + } + ] as ChannelInitializer) + + .channel(NioServerSocketChannel) + bootstrap.bind(PORT).sync() + } + + def cleanupSpec() { + eventLoopGroup.shutdownGracefully() + } + + def "test server request/response"() { + setup: + def request = new Request.Builder().url("http://localhost:$PORT/").get().build() + def response = client.newCall(request).execute() + + expect: + response.code() == 200 + response.body().string() == "Hello World" + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "unnamed-java-app" + operationName "netty.request" + resourceName "GET /" + spanType DDSpanTypes.WEB_SERVLET + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "http://localhost:$PORT/" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" Integer + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$DDTags.SPAN_TYPE" DDSpanTypes.WEB_SERVLET + defaultTags() + } + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java index 4aa78bdc04..907c798a74 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java @@ -115,7 +115,7 @@ public final class HttpServlet3Instrumentation extends Instrumenter.Configurable Tags.ERROR.set(span, Boolean.TRUE); span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); scope.close(); - scope.span().finish(); // Finish the span manually since finishSpanOnClose was false + span.finish(); // Finish the span manually since finishSpanOnClose was false } else if (req.isAsyncStarted()) { final AtomicBoolean activated = new AtomicBoolean(false); // what if async is already finished? This would not be called @@ -123,7 +123,7 @@ public final class HttpServlet3Instrumentation extends Instrumenter.Configurable } else { Tags.HTTP_STATUS.set(span, resp.getStatus()); scope.close(); - scope.span().finish(); // Finish the span manually since finishSpanOnClose was false + span.finish(); // Finish the span manually since finishSpanOnClose was false } } } @@ -140,7 +140,7 @@ public final class HttpServlet3Instrumentation extends Instrumenter.Configurable @Override public void onComplete(final AsyncEvent event) throws IOException { if (activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { Tags.HTTP_STATUS.set( span, ((HttpServletResponse) event.getSuppliedResponse()).getStatus()); } @@ -150,7 +150,7 @@ public final class HttpServlet3Instrumentation extends Instrumenter.Configurable @Override public void onTimeout(final AsyncEvent event) throws IOException { if (activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { Tags.ERROR.set(span, Boolean.TRUE); span.setTag("timeout", event.getAsyncContext().getTimeout()); } @@ -160,7 +160,7 @@ public final class HttpServlet3Instrumentation extends Instrumenter.Configurable @Override public void onError(final AsyncEvent event) throws IOException { if (event.getThrowable() != null && activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { if (((HttpServletResponse) event.getSuppliedResponse()).getStatus() == HttpServletResponse.SC_OK) { // exception is thrown in filter chain, but status code is incorrect diff --git a/settings.gradle b/settings.gradle index 73c07c1425..cc4361fe6a 100644 --- a/settings.gradle +++ b/settings.gradle @@ -36,6 +36,7 @@ include ':dd-java-agent:instrumentation:kafka-streams-0.11' include ':dd-java-agent:instrumentation:lettuce-5' include ':dd-java-agent:instrumentation:mongo-3.1' include ':dd-java-agent:instrumentation:mongo-async-3.3' +include ':dd-java-agent:instrumentation:netty' include ':dd-java-agent:instrumentation:okhttp-3' include ':dd-java-agent:instrumentation:osgi-classloading' include ':dd-java-agent:instrumentation:play-2.4' From 43e31eae80c7aab670913dbf31988b1fa4d0e3cb Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 15 Jun 2018 16:46:50 +1000 Subject: [PATCH 2/3] Separate out instrumentation for netty 4.0 from 4.1 --- .../netty-4.0/netty-4.0.gradle | 48 +++++ .../NettyChannelPipelineInstrumentation.java | 35 ++-- .../HttpClientRequestTracingHandler.java | 61 +++++++ .../HttpClientResponseTracingHandler.java | 37 ++++ .../client/HttpClientTracingHandler.java | 7 +- .../client/NettyResponseInjectAdapter.java | 2 +- .../HttpServerRequestTracingHandler.java | 71 ++++++++ .../HttpServerResponseTracingHandler.java | 38 ++++ .../server/HttpServerTracingHandler.java | 7 +- .../server/NettyRequestExtractAdapter.java | 25 +++ .../src/test/groovy/Netty40ClientTest.groovy} | 6 +- .../src/test/groovy/Netty40ServerTest.groovy | 164 ++++++++++++++++++ .../netty-4.1/netty-4.1.gradle | 49 ++++++ .../NettyChannelPipelineInstrumentation.java | 128 ++++++++++++++ .../HttpClientRequestTracingHandler.java | 13 +- .../HttpClientResponseTracingHandler.java | 7 +- .../client/HttpClientTracingHandler.java | 17 ++ .../client/NettyResponseInjectAdapter.java | 25 +++ .../HttpServerRequestTracingHandler.java | 13 +- .../HttpServerResponseTracingHandler.java | 7 +- .../server/HttpServerTracingHandler.java | 17 ++ .../server/NettyRequestExtractAdapter.java | 2 +- .../src/test/groovy/Netty41ClientTest.groovy | 97 +++++++++++ .../src/test/groovy/Netty41ServerTest.groovy} | 133 +++++++++----- .../instrumentation/netty/netty.gradle | 19 -- .../trace/agent/test/TagsAssert.groovy | 23 +-- gradle/java.gradle | 4 +- settings.gradle | 3 +- 28 files changed, 938 insertions(+), 120 deletions(-) create mode 100644 dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.0/src/main/java/datadog/trace/instrumentation/netty40}/NettyChannelPipelineInstrumentation.java (76%) create mode 100644 dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.0/src/main/java/datadog/trace/instrumentation/netty40}/client/HttpClientTracingHandler.java (59%) rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.0/src/main/java/datadog/trace/instrumentation/netty40}/client/NettyResponseInjectAdapter.java (92%) create mode 100644 dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerResponseTracingHandler.java rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.0/src/main/java/datadog/trace/instrumentation/netty40}/server/HttpServerTracingHandler.java (59%) create mode 100644 dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/NettyRequestExtractAdapter.java rename dd-java-agent/instrumentation/{netty/src/test/groovy/NettyClientTest.groovy => netty-4.0/src/test/groovy/Netty40ClientTest.groovy} (93%) create mode 100644 dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy create mode 100644 dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle create mode 100644 dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.1/src/main/java/datadog/trace/instrumentation/netty41}/client/HttpClientRequestTracingHandler.java (82%) rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.1/src/main/java/datadog/trace/instrumentation/netty41}/client/HttpClientResponseTracingHandler.java (81%) create mode 100644 dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientTracingHandler.java create mode 100644 dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/NettyResponseInjectAdapter.java rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.1/src/main/java/datadog/trace/instrumentation/netty41}/server/HttpServerRequestTracingHandler.java (84%) rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.1/src/main/java/datadog/trace/instrumentation/netty41}/server/HttpServerResponseTracingHandler.java (82%) create mode 100644 dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerTracingHandler.java rename dd-java-agent/instrumentation/{netty/src/main/java/datadog/trace/instrumentation/netty => netty-4.1/src/main/java/datadog/trace/instrumentation/netty41}/server/NettyRequestExtractAdapter.java (92%) create mode 100644 dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy rename dd-java-agent/instrumentation/{netty/src/test/groovy/NettyServerTest.groovy => netty-4.1/src/test/groovy/Netty41ServerTest.groovy} (54%) delete mode 100644 dd-java-agent/instrumentation/netty/netty.gradle diff --git a/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle b/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle new file mode 100644 index 0000000000..d5ecb06098 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle @@ -0,0 +1,48 @@ +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +dependencies { + compileOnly group: 'io.netty', name: 'netty-all', version: '4.0.0.Final' + + compile project(':dd-java-agent:agent-tooling') + + compile deps.bytebuddy + compile deps.opentracing + annotationProcessor deps.autoservice + implementation deps.autoservice + + testCompile project(':dd-java-agent:testing') + +// testCompile group: 'io.netty', name: 'netty-all', version: '4.0.0.Final' + testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.2.0.v20160908' + testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' + testCompile group: 'org.asynchttpclient', name: 'async-http-client', version: '2.0.0' +} + +// We need to force the dependency to the earliest supported version because other libraries declare newer versions. +configurations.testCompile { + resolutionStrategy { + eachDependency { DependencyResolveDetails details -> + //specifying a fixed version for all libraries with io.netty' group + if (details.requested.group == 'io.netty') { + details.useVersion "4.0.0.Final" + } + } + } +} + +configurations.latestDepTestCompile { + resolutionStrategy { + force group: 'io.netty', name: 'netty-all', version: '4.0.56.Final' + force group: 'org.asynchttpclient', name: 'async-http-client', version: '2.0.+' + } +} + +testJava8Only += '**/*Test.class' diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java similarity index 76% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/NettyChannelPipelineInstrumentation.java rename to dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java index 5958684ed3..078fc53157 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/NettyChannelPipelineInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java @@ -1,4 +1,4 @@ -package datadog.trace.instrumentation.netty; +package datadog.trace.instrumentation.netty40; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; @@ -14,12 +14,12 @@ import datadog.trace.agent.tooling.DDTransformers; import datadog.trace.agent.tooling.HelperInjector; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.bootstrap.CallDepthThreadLocalMap; -import datadog.trace.instrumentation.netty.client.HttpClientRequestTracingHandler; -import datadog.trace.instrumentation.netty.client.HttpClientResponseTracingHandler; -import datadog.trace.instrumentation.netty.client.HttpClientTracingHandler; -import datadog.trace.instrumentation.netty.server.HttpServerRequestTracingHandler; -import datadog.trace.instrumentation.netty.server.HttpServerResponseTracingHandler; -import datadog.trace.instrumentation.netty.server.HttpServerTracingHandler; +import datadog.trace.instrumentation.netty40.client.HttpClientRequestTracingHandler; +import datadog.trace.instrumentation.netty40.client.HttpClientResponseTracingHandler; +import datadog.trace.instrumentation.netty40.client.HttpClientTracingHandler; +import datadog.trace.instrumentation.netty40.server.HttpServerRequestTracingHandler; +import datadog.trace.instrumentation.netty40.server.HttpServerResponseTracingHandler; +import datadog.trace.instrumentation.netty40.server.HttpServerTracingHandler; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelPipeline; import io.netty.handler.codec.http.HttpClientCodec; @@ -34,8 +34,11 @@ import net.bytebuddy.asm.Advice; @AutoService(Instrumenter.class) public class NettyChannelPipelineInstrumentation extends Instrumenter.Configurable { + private static final String PACKAGE = + NettyChannelPipelineInstrumentation.class.getPackage().getName(); + public NettyChannelPipelineInstrumentation() { - super("netty", "netty-http-server"); + super("netty", "netty-4.1"); } @Override @@ -50,15 +53,15 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Configurab .transform( new HelperInjector( // client helpers - "datadog.trace.instrumentation.netty.client.NettyResponseInjectAdapter", - "datadog.trace.instrumentation.netty.client.HttpClientRequestTracingHandler", - "datadog.trace.instrumentation.netty.client.HttpClientResponseTracingHandler", - "datadog.trace.instrumentation.netty.client.HttpClientTracingHandler", + PACKAGE + ".client.NettyResponseInjectAdapter", + PACKAGE + ".client.HttpClientRequestTracingHandler", + PACKAGE + ".client.HttpClientResponseTracingHandler", + PACKAGE + ".client.HttpClientTracingHandler", // server helpers - "datadog.trace.instrumentation.netty.server.NettyRequestExtractAdapter", - "datadog.trace.instrumentation.netty.server.HttpServerRequestTracingHandler", - "datadog.trace.instrumentation.netty.server.HttpServerResponseTracingHandler", - "datadog.trace.instrumentation.netty.server.HttpServerTracingHandler")) + PACKAGE + ".server.NettyRequestExtractAdapter", + PACKAGE + ".server.HttpServerRequestTracingHandler", + PACKAGE + ".server.HttpServerResponseTracingHandler", + PACKAGE + ".server.HttpServerTracingHandler")) .transform(DDTransformers.defaultTransformers()) .transform( DDAdvice.create() diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java new file mode 100644 index 0000000000..2b5eed53eb --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java @@ -0,0 +1,61 @@ +package datadog.trace.instrumentation.netty40.client; + +import static io.netty.handler.codec.http.HttpHeaders.Names.HOST; +import static io.opentracing.log.Fields.ERROR_OBJECT; + +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelOutboundHandlerAdapter; +import io.netty.channel.ChannelPromise; +import io.netty.handler.codec.http.HttpRequest; +import io.opentracing.Span; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import java.net.InetSocketAddress; +import java.util.Collections; + +public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapter { + + @Override + public void write(final ChannelHandlerContext ctx, final Object msg, final ChannelPromise prm) { + if (!(msg instanceof HttpRequest)) { + ctx.write(msg, prm); + return; + } + final HttpRequest request = (HttpRequest) msg; + final InetSocketAddress remoteAddress = (InetSocketAddress) ctx.channel().remoteAddress(); + + String url = request.getUri(); + if (request.headers().contains(HOST)) { + url = "http://" + request.headers().get(HOST) + url; + } + final Span span = + GlobalTracer.get() + .buildSpan("netty.client.request") + .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) + .withTag(Tags.PEER_HOSTNAME.getKey(), remoteAddress.getHostName()) + .withTag(Tags.PEER_PORT.getKey(), remoteAddress.getPort()) + .withTag(Tags.HTTP_METHOD.getKey(), request.getMethod().name()) + .withTag(Tags.HTTP_URL.getKey(), url) + .withTag(Tags.COMPONENT.getKey(), "netty-client") + .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) + .start(); + + GlobalTracer.get() + .inject( + span.context(), Format.Builtin.HTTP_HEADERS, new NettyResponseInjectAdapter(request)); + + ctx.channel().attr(HttpClientTracingHandler.attributeKey).set(span); + + try { + ctx.write(msg, prm); + } catch (final Throwable throwable) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } + } +} diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java new file mode 100644 index 0000000000..8c3eef74ba --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientResponseTracingHandler.java @@ -0,0 +1,37 @@ +package datadog.trace.instrumentation.netty40.client; + +import static io.opentracing.log.Fields.ERROR_OBJECT; + +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.handler.codec.http.HttpResponse; +import io.opentracing.Span; +import io.opentracing.tag.Tags; +import java.util.Collections; + +public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter { + + @Override + public void channelRead(final ChannelHandlerContext ctx, final Object msg) { + final Span span = ctx.channel().attr(HttpClientTracingHandler.attributeKey).get(); + if (span == null || !(msg instanceof HttpResponse)) { + ctx.fireChannelRead(msg); + return; + } + + final HttpResponse response = (HttpResponse) msg; + + try { + ctx.fireChannelRead(msg); + } catch (final Throwable throwable) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + Tags.HTTP_STATUS.set(span, 500); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } + + Tags.HTTP_STATUS.set(span, response.getStatus().code()); + span.finish(); // Finish the span manually since finishSpanOnClose was false + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientTracingHandler.java similarity index 59% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientTracingHandler.java rename to dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientTracingHandler.java index fe70a2739f..5c48e47e41 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientTracingHandler.java @@ -1,11 +1,16 @@ -package datadog.trace.instrumentation.netty.client; +package datadog.trace.instrumentation.netty40.client; import io.netty.channel.CombinedChannelDuplexHandler; +import io.netty.util.AttributeKey; +import io.opentracing.Span; public class HttpClientTracingHandler extends CombinedChannelDuplexHandler< HttpClientResponseTracingHandler, HttpClientRequestTracingHandler> { + static final AttributeKey attributeKey = + new AttributeKey<>(HttpClientTracingHandler.class.getName()); + public HttpClientTracingHandler() { super(new HttpClientResponseTracingHandler(), new HttpClientRequestTracingHandler()); } diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/NettyResponseInjectAdapter.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/NettyResponseInjectAdapter.java similarity index 92% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/NettyResponseInjectAdapter.java rename to dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/NettyResponseInjectAdapter.java index 6636c05de6..4bd6fd8769 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/NettyResponseInjectAdapter.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/NettyResponseInjectAdapter.java @@ -1,4 +1,4 @@ -package datadog.trace.instrumentation.netty.client; +package datadog.trace.instrumentation.netty40.client; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpRequest; diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java new file mode 100644 index 0000000000..1b52170439 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.netty40.server; + +import static io.netty.handler.codec.http.HttpHeaders.Names.HOST; +import static io.opentracing.log.Fields.ERROR_OBJECT; + +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import datadog.trace.context.TraceScope; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.handler.codec.http.HttpRequest; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.SpanContext; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import java.net.InetSocketAddress; +import java.util.Collections; + +public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapter { + + @Override + public void channelRead(final ChannelHandlerContext ctx, final Object msg) { + if (!(msg instanceof HttpRequest)) { + ctx.fireChannelRead(msg); // superclass does not throw + return; + } + final HttpRequest request = (HttpRequest) msg; + final InetSocketAddress remoteAddress = (InetSocketAddress) ctx.channel().remoteAddress(); + + final SpanContext extractedContext = + GlobalTracer.get() + .extract(Format.Builtin.HTTP_HEADERS, new NettyRequestExtractAdapter(request)); + + String url = request.getUri(); + if (request.headers().contains(HOST)) { + url = "http://" + request.headers().get(HOST) + url; + } + final Scope scope = + GlobalTracer.get() + .buildSpan("netty.request") + .asChildOf(extractedContext) + .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) + .withTag(Tags.PEER_HOSTNAME.getKey(), remoteAddress.getHostName()) + .withTag(Tags.PEER_PORT.getKey(), remoteAddress.getPort()) + .withTag(Tags.HTTP_METHOD.getKey(), request.getMethod().name()) + .withTag(Tags.HTTP_URL.getKey(), url) + .withTag(Tags.COMPONENT.getKey(), "netty") + .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET) + .startActive(false); + + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } + + final Span span = scope.span(); + ctx.channel().attr(HttpServerTracingHandler.attributeKey).set(span); + + try { + ctx.fireChannelRead(msg); + } catch (final Throwable throwable) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } finally { + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerResponseTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerResponseTracingHandler.java new file mode 100644 index 0000000000..c9b4533ef3 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerResponseTracingHandler.java @@ -0,0 +1,38 @@ +package datadog.trace.instrumentation.netty40.server; + +import static io.opentracing.log.Fields.ERROR_OBJECT; + +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelOutboundHandlerAdapter; +import io.netty.channel.ChannelPromise; +import io.netty.handler.codec.http.HttpResponse; +import io.opentracing.Span; +import io.opentracing.tag.Tags; +import java.util.Collections; + +public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdapter { + + @Override + public void write(final ChannelHandlerContext ctx, final Object msg, final ChannelPromise prm) { + final Span span = ctx.channel().attr(HttpServerTracingHandler.attributeKey).get(); + if (span == null || !(msg instanceof HttpResponse)) { + ctx.write(msg, prm); + return; + } + + final HttpResponse response = (HttpResponse) msg; + + try { + ctx.write(msg, prm); + } catch (final Throwable throwable) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + Tags.HTTP_STATUS.set(span, 500); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } + + Tags.HTTP_STATUS.set(span, response.getStatus().code()); + span.finish(); // Finish the span manually since finishSpanOnClose was false + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerTracingHandler.java similarity index 59% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerTracingHandler.java rename to dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerTracingHandler.java index dc15fecace..aebfe59854 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerTracingHandler.java @@ -1,11 +1,16 @@ -package datadog.trace.instrumentation.netty.server; +package datadog.trace.instrumentation.netty40.server; import io.netty.channel.CombinedChannelDuplexHandler; +import io.netty.util.AttributeKey; +import io.opentracing.Span; public class HttpServerTracingHandler extends CombinedChannelDuplexHandler< HttpServerRequestTracingHandler, HttpServerResponseTracingHandler> { + static final AttributeKey attributeKey = + new AttributeKey<>(HttpServerTracingHandler.class.getName()); + public HttpServerTracingHandler() { super(new HttpServerRequestTracingHandler(), new HttpServerResponseTracingHandler()); } diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/NettyRequestExtractAdapter.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/NettyRequestExtractAdapter.java new file mode 100644 index 0000000000..877ebe554e --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/NettyRequestExtractAdapter.java @@ -0,0 +1,25 @@ +package datadog.trace.instrumentation.netty40.server; + +import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpRequest; +import io.opentracing.propagation.TextMap; +import java.util.Iterator; +import java.util.Map; + +public class NettyRequestExtractAdapter implements TextMap { + private final HttpHeaders headers; + + NettyRequestExtractAdapter(final HttpRequest request) { + this.headers = request.headers(); + } + + @Override + public Iterator> iterator() { + return headers.iterator(); + } + + @Override + public void put(final String key, final String value) { + throw new UnsupportedOperationException("This class should be used only with Tracer.inject()!"); + } +} diff --git a/dd-java-agent/instrumentation/netty/src/test/groovy/NettyClientTest.groovy b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy similarity index 93% rename from dd-java-agent/instrumentation/netty/src/test/groovy/NettyClientTest.groovy rename to dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy index c83fc82155..c178a016c5 100644 --- a/dd-java-agent/instrumentation/netty/src/test/groovy/NettyClientTest.groovy +++ b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy @@ -17,7 +17,7 @@ import javax.servlet.http.HttpServletResponse import static datadog.trace.agent.test.ListWriterAssert.assertTraces import static org.asynchttpclient.Dsl.asyncHttpClient -class NettyClientTest extends AgentTestRunner { +class Netty40ClientTest extends AgentTestRunner { static { System.setProperty("dd.integration.netty.enabled", "true") } @@ -91,7 +91,7 @@ class NettyClientTest extends AgentTestRunner { } and: - headers["x-datadog-trace-id"] == ["${TEST_WRITER.get(0).get(0).traceId}"] - headers["x-datadog-parent-id"] == ["${TEST_WRITER.get(0).get(0).spanId}"] + headers["x-datadog-trace-id"] == "${TEST_WRITER.get(0).get(0).traceId}" + headers["x-datadog-parent-id"] == "${TEST_WRITER.get(0).get(0).spanId}" } } diff --git a/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy new file mode 100644 index 0000000000..064cadca2d --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy @@ -0,0 +1,164 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TestUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import io.netty.bootstrap.ServerBootstrap +import io.netty.buffer.ByteBuf +import io.netty.buffer.Unpooled +import io.netty.channel.ChannelInitializer +import io.netty.channel.ChannelPipeline +import io.netty.channel.EventLoopGroup +import io.netty.channel.SimpleChannelInboundHandler +import io.netty.channel.nio.NioEventLoopGroup +import io.netty.channel.socket.nio.NioServerSocketChannel +import io.netty.handler.codec.http.DefaultFullHttpResponse +import io.netty.handler.codec.http.FullHttpResponse +import io.netty.handler.codec.http.HttpHeaders +import io.netty.handler.codec.http.HttpRequestDecoder +import io.netty.handler.codec.http.HttpResponseEncoder +import io.netty.handler.codec.http.HttpResponseStatus +import io.netty.handler.codec.http.HttpServerCodec +import io.netty.handler.codec.http.HttpVersion +import io.netty.handler.codec.http.LastHttpContent +import io.netty.handler.logging.LogLevel +import io.netty.handler.logging.LoggingHandler +import io.netty.util.CharsetUtil +import io.opentracing.tag.Tags +import okhttp3.OkHttpClient +import okhttp3.Request + +import static datadog.trace.agent.test.ListWriterAssert.assertTraces + +class Netty40ServerTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.netty.enabled", "true") + } + + OkHttpClient client = new OkHttpClient.Builder() + // Uncomment when debugging: +// .connectTimeout(1, TimeUnit.HOURS) +// .writeTimeout(1, TimeUnit.HOURS) +// .readTimeout(1, TimeUnit.HOURS) + .build() + + def "test server request/response"() { + setup: + EventLoopGroup eventLoopGroup = new NioEventLoopGroup() + int port = TestUtils.randomOpenPort() + initializeServer(eventLoopGroup, port, handlers, HttpResponseStatus.OK) + + def request = new Request.Builder().url("http://localhost:$port/").get().build() + def response = client.newCall(request).execute() + + expect: + response.code() == 200 + response.body().string() == "Hello World" + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "unnamed-java-app" + operationName "netty.request" + resourceName "GET /" + spanType DDSpanTypes.WEB_SERVLET + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "http://localhost:$port/" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" Integer + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$DDTags.SPAN_TYPE" DDSpanTypes.WEB_SERVLET + defaultTags() + } + } + } + } + + cleanup: + eventLoopGroup.shutdownGracefully() + + where: + handlers | _ + [new HttpServerCodec()] | _ + [new HttpRequestDecoder(), new HttpResponseEncoder()] | _ + } + + def "test #responseCode response handling"() { + setup: + EventLoopGroup eventLoopGroup = new NioEventLoopGroup() + int port = TestUtils.randomOpenPort() + initializeServer(eventLoopGroup, port, new HttpServerCodec(), responseCode) + + def request = new Request.Builder().url("http://localhost:$port/").get().build() + def response = client.newCall(request).execute() + + expect: + response.code() == responseCode.code() + response.body().string() == "Hello World" + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "unnamed-java-app" + operationName "netty.request" + resourceName name + spanType DDSpanTypes.WEB_SERVLET + errored error + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" responseCode.code() + "$Tags.HTTP_URL.key" "http://localhost:$port/" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" Integer + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$DDTags.SPAN_TYPE" DDSpanTypes.WEB_SERVLET + if (error) { + tag("error", true) + } + defaultTags() + } + } + } + } + + cleanup: + eventLoopGroup.shutdownGracefully() + + where: + responseCode | name | error + HttpResponseStatus.OK | "GET /" | false + HttpResponseStatus.NOT_FOUND | "404" | false + HttpResponseStatus.INTERNAL_SERVER_ERROR | "GET /" | true + } + + def initializeServer(eventLoopGroup, port, handlers, responseCode) { + ServerBootstrap bootstrap = new ServerBootstrap() + .group(eventLoopGroup) + .handler(new LoggingHandler(LogLevel.INFO)) + .childHandler([ + initChannel: { ch -> + ChannelPipeline pipeline = ch.pipeline() + handlers.each { pipeline.addLast(it) } + pipeline.addLast([ + channelRead0 : { ctx, msg -> + if (msg instanceof LastHttpContent) { + ByteBuf content = Unpooled.copiedBuffer("Hello World", CharsetUtil.UTF_8) + FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, responseCode, content) + response.headers().set(HttpHeaders.Names.CONTENT_TYPE, "text/plain") + response.headers().set(HttpHeaders.Names.CONTENT_LENGTH, content.readableBytes()) + ctx.write(response) + } + }, + channelReadComplete: { it.flush() } + ] as SimpleChannelInboundHandler) + } + ] as ChannelInitializer).channel(NioServerSocketChannel) + bootstrap.bind(port).sync() + } +} diff --git a/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle b/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle new file mode 100644 index 0000000000..99bc8af2f4 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle @@ -0,0 +1,49 @@ +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +dependencies { + compileOnly group: 'io.netty', name: 'netty-all', version: '4.1.0.Final' + + compile project(':dd-java-agent:agent-tooling') + + compile deps.bytebuddy + compile deps.opentracing + annotationProcessor deps.autoservice + implementation deps.autoservice + + testCompile project(':dd-java-agent:testing') + + testCompile group: 'io.netty', name: 'netty-all', version: '4.1.0.Final' + testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.2.0.v20160908' + testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' + testCompile group: 'org.asynchttpclient', name: 'async-http-client', version: '2.0.31' + // async-http-client:2.0.32+ would require netty:4.1.9.Final +} + +// We need to force the dependency to the earliest supported version because other libraries declare newer versions. +configurations.testCompile { + resolutionStrategy { + eachDependency { DependencyResolveDetails details -> + //specifying a fixed version for all libraries with io.netty' group + if (details.requested.group == 'io.netty') { + details.useVersion "4.1.0.Final" + } + } + } +} + +configurations.latestDepTestCompile { + resolutionStrategy { + force group: 'io.netty', name: 'netty-all', version: '+' + force group: 'org.asynchttpclient', name: 'async-http-client', version: '+' + } +} + +testJava8Only += '**/*Test.class' diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java new file mode 100644 index 0000000000..34f154ac7f --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java @@ -0,0 +1,128 @@ +package datadog.trace.instrumentation.netty41; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.DDAdvice; +import datadog.trace.agent.tooling.DDTransformers; +import datadog.trace.agent.tooling.HelperInjector; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import datadog.trace.instrumentation.netty41.client.HttpClientRequestTracingHandler; +import datadog.trace.instrumentation.netty41.client.HttpClientResponseTracingHandler; +import datadog.trace.instrumentation.netty41.client.HttpClientTracingHandler; +import datadog.trace.instrumentation.netty41.server.HttpServerRequestTracingHandler; +import datadog.trace.instrumentation.netty41.server.HttpServerResponseTracingHandler; +import datadog.trace.instrumentation.netty41.server.HttpServerTracingHandler; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelPipeline; +import io.netty.handler.codec.http.HttpClientCodec; +import io.netty.handler.codec.http.HttpRequestDecoder; +import io.netty.handler.codec.http.HttpRequestEncoder; +import io.netty.handler.codec.http.HttpResponseDecoder; +import io.netty.handler.codec.http.HttpResponseEncoder; +import io.netty.handler.codec.http.HttpServerCodec; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public class NettyChannelPipelineInstrumentation extends Instrumenter.Configurable { + + private static final String PACKAGE = + NettyChannelPipelineInstrumentation.class.getPackage().getName(); + + public NettyChannelPipelineInstrumentation() { + super("netty", "netty-4.1"); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public AgentBuilder apply(final AgentBuilder agentBuilder) { + return agentBuilder + .type(not(isInterface()).and(hasSuperType(named("io.netty.channel.ChannelPipeline")))) + .transform( + new HelperInjector( + // client helpers + PACKAGE + ".client.NettyResponseInjectAdapter", + PACKAGE + ".client.HttpClientRequestTracingHandler", + PACKAGE + ".client.HttpClientResponseTracingHandler", + PACKAGE + ".client.HttpClientTracingHandler", + // server helpers + PACKAGE + ".server.NettyRequestExtractAdapter", + PACKAGE + ".server.HttpServerRequestTracingHandler", + PACKAGE + ".server.HttpServerResponseTracingHandler", + PACKAGE + ".server.HttpServerTracingHandler")) + .transform(DDTransformers.defaultTransformers()) + .transform( + DDAdvice.create() + .advice( + isMethod() + .and(nameStartsWith("add")) + .and(takesArgument(2, named("io.netty.channel.ChannelHandler"))), + ChannelPipelineAddAdvice.class.getName())) + .asDecorator(); + } + + /** + * When certain handlers are added to the pipeline, we want to add our corresponding tracing + * handlers. If those handlers are later removed, we may want to remove our handlers. That is not + * currently implemented. + */ + public static class ChannelPipelineAddAdvice { + @Advice.OnMethodEnter + public static int checkDepth() { + return CallDepthThreadLocalMap.incrementCallDepth(ChannelPipeline.class); + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void addHandler( + @Advice.Enter final int depth, + @Advice.This final ChannelPipeline pipeline, + @Advice.Argument(2) final ChannelHandler handler) { + if (depth > 0) return; + + try { + // Server pipeline handlers + if (handler instanceof HttpServerCodec) { + pipeline.addLast( + HttpServerTracingHandler.class.getName(), new HttpServerTracingHandler()); + } else if (handler instanceof HttpRequestDecoder) { + pipeline.addLast( + HttpServerRequestTracingHandler.class.getName(), + new HttpServerRequestTracingHandler()); + } else if (handler instanceof HttpResponseEncoder) { + pipeline.addLast( + HttpServerResponseTracingHandler.class.getName(), + new HttpServerResponseTracingHandler()); + } else + // Client pipeline handlers + if (handler instanceof HttpClientCodec) { + pipeline.addLast( + HttpClientTracingHandler.class.getName(), new HttpClientTracingHandler()); + } else if (handler instanceof HttpRequestEncoder) { + pipeline.addLast( + HttpClientRequestTracingHandler.class.getName(), + new HttpClientRequestTracingHandler()); + } else if (handler instanceof HttpResponseDecoder) { + pipeline.addLast( + HttpClientResponseTracingHandler.class.getName(), + new HttpClientResponseTracingHandler()); + } + } catch (final IllegalArgumentException e) { + // Prevented adding duplicate handlers. + } finally { + CallDepthThreadLocalMap.reset(ChannelPipeline.class); + } + } + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientRequestTracingHandler.java similarity index 82% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientRequestTracingHandler.java rename to dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientRequestTracingHandler.java index 64cac3fea5..d8f5596b57 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientRequestTracingHandler.java @@ -1,5 +1,6 @@ -package datadog.trace.instrumentation.netty.client; +package datadog.trace.instrumentation.netty41.client; +import static io.netty.handler.codec.http.HttpHeaderNames.HOST; import static io.opentracing.log.Fields.ERROR_OBJECT; import datadog.trace.api.DDSpanTypes; @@ -7,9 +8,7 @@ import datadog.trace.api.DDTags; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOutboundHandlerAdapter; import io.netty.channel.ChannelPromise; -import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpRequest; -import io.netty.util.AttributeKey; import io.opentracing.Span; import io.opentracing.propagation.Format; import io.opentracing.tag.Tags; @@ -19,8 +18,6 @@ import java.util.Collections; public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapter { - private static final AttributeKey attributeKey = AttributeKey.valueOf(Span.class.getName()); - @Override public void write(final ChannelHandlerContext ctx, final Object msg, final ChannelPromise prm) { if (!(msg instanceof HttpRequest)) { @@ -31,8 +28,8 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt final InetSocketAddress remoteAddress = (InetSocketAddress) ctx.channel().remoteAddress(); String url = request.uri(); - if (request.headers().contains(HttpHeaderNames.HOST)) { - url = "http://" + request.headers().get(HttpHeaderNames.HOST) + url; + if (request.headers().contains(HOST)) { + url = "http://" + request.headers().get(HOST) + url; } final Span span = GlobalTracer.get() @@ -50,7 +47,7 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt .inject( span.context(), Format.Builtin.HTTP_HEADERS, new NettyResponseInjectAdapter(request)); - ctx.channel().attr(attributeKey).set(span); + ctx.channel().attr(HttpClientTracingHandler.attributeKey).set(span); try { ctx.write(msg, prm); diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientResponseTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientResponseTracingHandler.java similarity index 81% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientResponseTracingHandler.java rename to dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientResponseTracingHandler.java index 25f6aa792b..afbd70f595 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/client/HttpClientResponseTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientResponseTracingHandler.java @@ -1,22 +1,19 @@ -package datadog.trace.instrumentation.netty.client; +package datadog.trace.instrumentation.netty41.client; import static io.opentracing.log.Fields.ERROR_OBJECT; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.handler.codec.http.HttpResponse; -import io.netty.util.AttributeKey; import io.opentracing.Span; import io.opentracing.tag.Tags; import java.util.Collections; public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter { - private static final AttributeKey attributeKey = AttributeKey.valueOf(Span.class.getName()); - @Override public void channelRead(final ChannelHandlerContext ctx, final Object msg) { - final Span span = ctx.channel().attr(attributeKey).get(); + final Span span = ctx.channel().attr(HttpClientTracingHandler.attributeKey).get(); if (span == null || !(msg instanceof HttpResponse)) { ctx.fireChannelRead(msg); return; diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientTracingHandler.java new file mode 100644 index 0000000000..c1c4a57e9b --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientTracingHandler.java @@ -0,0 +1,17 @@ +package datadog.trace.instrumentation.netty41.client; + +import io.netty.channel.CombinedChannelDuplexHandler; +import io.netty.util.AttributeKey; +import io.opentracing.Span; + +public class HttpClientTracingHandler + extends CombinedChannelDuplexHandler< + HttpClientResponseTracingHandler, HttpClientRequestTracingHandler> { + + static final AttributeKey attributeKey = + AttributeKey.valueOf(HttpClientTracingHandler.class.getName()); + + public HttpClientTracingHandler() { + super(new HttpClientResponseTracingHandler(), new HttpClientRequestTracingHandler()); + } +} diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/NettyResponseInjectAdapter.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/NettyResponseInjectAdapter.java new file mode 100644 index 0000000000..3e98e2cd25 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/NettyResponseInjectAdapter.java @@ -0,0 +1,25 @@ +package datadog.trace.instrumentation.netty41.client; + +import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpRequest; +import io.opentracing.propagation.TextMap; +import java.util.Iterator; +import java.util.Map; + +public class NettyResponseInjectAdapter implements TextMap { + private final HttpHeaders headers; + + NettyResponseInjectAdapter(final HttpRequest request) { + this.headers = request.headers(); + } + + @Override + public Iterator> iterator() { + throw new UnsupportedOperationException("This class should be used only with Tracer.inject()!"); + } + + @Override + public void put(final String key, final String value) { + headers.set(key, value); + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java similarity index 84% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerRequestTracingHandler.java rename to dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java index 200e39375a..1d8d726d57 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java @@ -1,5 +1,6 @@ -package datadog.trace.instrumentation.netty.server; +package datadog.trace.instrumentation.netty41.server; +import static io.netty.handler.codec.http.HttpHeaderNames.HOST; import static io.opentracing.log.Fields.ERROR_OBJECT; import datadog.trace.api.DDSpanTypes; @@ -7,9 +8,7 @@ import datadog.trace.api.DDTags; import datadog.trace.context.TraceScope; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; -import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpRequest; -import io.netty.util.AttributeKey; import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.SpanContext; @@ -21,8 +20,6 @@ import java.util.Collections; public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapter { - private static final AttributeKey attributeKey = AttributeKey.valueOf(Span.class.getName()); - @Override public void channelRead(final ChannelHandlerContext ctx, final Object msg) { if (!(msg instanceof HttpRequest)) { @@ -37,8 +34,8 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte .extract(Format.Builtin.HTTP_HEADERS, new NettyRequestExtractAdapter(request)); String url = request.uri(); - if (request.headers().contains(HttpHeaderNames.HOST)) { - url = "http://" + request.headers().get(HttpHeaderNames.HOST) + url; + if (request.headers().contains(HOST)) { + url = "http://" + request.headers().get(HOST) + url; } final Scope scope = GlobalTracer.get() @@ -58,7 +55,7 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte } final Span span = scope.span(); - ctx.channel().attr(attributeKey).set(span); + ctx.channel().attr(HttpServerTracingHandler.attributeKey).set(span); try { ctx.fireChannelRead(msg); diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerResponseTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerResponseTracingHandler.java similarity index 82% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerResponseTracingHandler.java rename to dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerResponseTracingHandler.java index d50a05550a..65af08c5d6 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/HttpServerResponseTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerResponseTracingHandler.java @@ -1,4 +1,4 @@ -package datadog.trace.instrumentation.netty.server; +package datadog.trace.instrumentation.netty41.server; import static io.opentracing.log.Fields.ERROR_OBJECT; @@ -6,18 +6,15 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOutboundHandlerAdapter; import io.netty.channel.ChannelPromise; import io.netty.handler.codec.http.HttpResponse; -import io.netty.util.AttributeKey; import io.opentracing.Span; import io.opentracing.tag.Tags; import java.util.Collections; public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdapter { - private static final AttributeKey attributeKey = AttributeKey.valueOf(Span.class.getName()); - @Override public void write(final ChannelHandlerContext ctx, final Object msg, final ChannelPromise prm) { - final Span span = ctx.channel().attr(attributeKey).get(); + final Span span = ctx.channel().attr(HttpServerTracingHandler.attributeKey).get(); if (span == null || !(msg instanceof HttpResponse)) { ctx.write(msg, prm); return; diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerTracingHandler.java new file mode 100644 index 0000000000..942a78f5d2 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerTracingHandler.java @@ -0,0 +1,17 @@ +package datadog.trace.instrumentation.netty41.server; + +import io.netty.channel.CombinedChannelDuplexHandler; +import io.netty.util.AttributeKey; +import io.opentracing.Span; + +public class HttpServerTracingHandler + extends CombinedChannelDuplexHandler< + HttpServerRequestTracingHandler, HttpServerResponseTracingHandler> { + + static final AttributeKey attributeKey = + AttributeKey.valueOf(HttpServerTracingHandler.class.getName()); + + public HttpServerTracingHandler() { + super(new HttpServerRequestTracingHandler(), new HttpServerResponseTracingHandler()); + } +} diff --git a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/NettyRequestExtractAdapter.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyRequestExtractAdapter.java similarity index 92% rename from dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/NettyRequestExtractAdapter.java rename to dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyRequestExtractAdapter.java index 35b856c5b1..f633287784 100644 --- a/dd-java-agent/instrumentation/netty/src/main/java/datadog/trace/instrumentation/netty/server/NettyRequestExtractAdapter.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/NettyRequestExtractAdapter.java @@ -1,4 +1,4 @@ -package datadog.trace.instrumentation.netty.server; +package datadog.trace.instrumentation.netty41.server; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpRequest; diff --git a/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy b/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy new file mode 100644 index 0000000000..562e539271 --- /dev/null +++ b/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy @@ -0,0 +1,97 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TestUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import io.opentracing.tag.Tags +import org.asynchttpclient.AsyncHttpClient +import org.eclipse.jetty.server.Handler +import org.eclipse.jetty.server.Request +import org.eclipse.jetty.server.Server +import org.eclipse.jetty.server.handler.AbstractHandler +import org.eclipse.jetty.util.MultiMap +import spock.lang.Shared + +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +import static datadog.trace.agent.test.ListWriterAssert.assertTraces +import static org.asynchttpclient.Dsl.asyncHttpClient + +class Netty41ClientTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.netty.enabled", "true") + } + + static final PORT = TestUtils.randomOpenPort() + + @Shared + Server server = new Server(PORT) + @Shared + AsyncHttpClient asyncHttpClient = asyncHttpClient() +// DefaultAsyncHttpClientConfig.Builder.newInstance().setRequestTimeout(Integer.MAX_VALUE).build()) + @Shared + def headers = new MultiMap() + + + def setupSpec() { + Handler handler = [ + handle: { String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response -> + request.getHeaderNames().each { + headers.add(it, request.getHeader(it)) + } + response.setContentType("text/plaincharset=utf-8") + response.setStatus(HttpServletResponse.SC_OK) + baseRequest.setHandled(true) + response.getWriter().println("Hello World") + } + ] as AbstractHandler + server.setHandler(handler) + server.start() + } + + def cleanupSpec() { + server.stop() + } + + def cleanup() { + headers.clear() + } + + def "test server request/response"() { + setup: + def responseFuture = asyncHttpClient.prepareGet("http://localhost:$PORT/").execute() + def response = responseFuture.get() + + expect: + response.statusCode == 200 + response.responseBody == "Hello World\n" + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "unnamed-java-app" + operationName "netty.client.request" + resourceName "GET /" + spanType DDSpanTypes.HTTP_CLIENT + errored false + tags { + "$Tags.COMPONENT.key" "netty-client" + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "http://localhost:$PORT/" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" Integer + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + defaultTags() + } + } + } + } + + and: + headers["x-datadog-trace-id"] == "${TEST_WRITER.get(0).get(0).traceId}" + headers["x-datadog-parent-id"] == "${TEST_WRITER.get(0).get(0).spanId}" + } +} diff --git a/dd-java-agent/instrumentation/netty/src/test/groovy/NettyServerTest.groovy b/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ServerTest.groovy similarity index 54% rename from dd-java-agent/instrumentation/netty/src/test/groovy/NettyServerTest.groovy rename to dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ServerTest.groovy index 2d048474cc..836e706f3a 100644 --- a/dd-java-agent/instrumentation/netty/src/test/groovy/NettyServerTest.groovy +++ b/dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ServerTest.groovy @@ -6,6 +6,7 @@ import io.netty.bootstrap.ServerBootstrap import io.netty.buffer.ByteBuf import io.netty.buffer.Unpooled import io.netty.channel.ChannelInitializer +import io.netty.channel.ChannelPipeline import io.netty.channel.EventLoopGroup import io.netty.channel.SimpleChannelInboundHandler import io.netty.channel.nio.NioEventLoopGroup @@ -13,6 +14,8 @@ import io.netty.channel.socket.nio.NioServerSocketChannel import io.netty.handler.codec.http.DefaultFullHttpResponse import io.netty.handler.codec.http.FullHttpResponse import io.netty.handler.codec.http.HttpHeaderNames +import io.netty.handler.codec.http.HttpRequestDecoder +import io.netty.handler.codec.http.HttpResponseEncoder import io.netty.handler.codec.http.HttpResponseStatus import io.netty.handler.codec.http.HttpServerCodec import io.netty.handler.codec.http.HttpVersion @@ -23,20 +26,14 @@ import io.netty.util.CharsetUtil import io.opentracing.tag.Tags import okhttp3.OkHttpClient import okhttp3.Request -import spock.lang.Shared import static datadog.trace.agent.test.ListWriterAssert.assertTraces -class NettyServerTest extends AgentTestRunner { +class Netty41ServerTest extends AgentTestRunner { static { System.setProperty("dd.integration.netty.enabled", "true") } - static final PORT = TestUtils.randomOpenPort() - - @Shared - EventLoopGroup eventLoopGroup = new NioEventLoopGroup() - OkHttpClient client = new OkHttpClient.Builder() // Uncomment when debugging: // .connectTimeout(1, TimeUnit.HOURS) @@ -44,40 +41,13 @@ class NettyServerTest extends AgentTestRunner { // .readTimeout(1, TimeUnit.HOURS) .build() - def setupSpec() { - ServerBootstrap bootstrap = new ServerBootstrap() - .group(eventLoopGroup) - .handler(new LoggingHandler(LogLevel.INFO)) - .childHandler([ - initChannel: { ch -> - def pipeline = ch.pipeline() - pipeline.addLast(new HttpServerCodec()) - pipeline.addLast([ - channelRead0 : { ctx, msg -> - if (msg instanceof LastHttpContent) { - ByteBuf content = Unpooled.copiedBuffer("Hello World", CharsetUtil.UTF_8) - FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK, content) - response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/plain") - response.headers().set(HttpHeaderNames.CONTENT_LENGTH, content.readableBytes()) - ctx.write(response) - } - }, - channelReadComplete: { it.flush() } - ] as SimpleChannelInboundHandler) - } - ] as ChannelInitializer) - - .channel(NioServerSocketChannel) - bootstrap.bind(PORT).sync() - } - - def cleanupSpec() { - eventLoopGroup.shutdownGracefully() - } - def "test server request/response"() { setup: - def request = new Request.Builder().url("http://localhost:$PORT/").get().build() + EventLoopGroup eventLoopGroup = new NioEventLoopGroup() + int port = TestUtils.randomOpenPort() + initializeServer(eventLoopGroup, port, handlers, HttpResponseStatus.OK) + + def request = new Request.Builder().url("http://localhost:$port/").get().build() def response = client.newCall(request).execute() expect: @@ -97,7 +67,7 @@ class NettyServerTest extends AgentTestRunner { "$Tags.COMPONENT.key" "netty" "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "http://localhost:$PORT/" + "$Tags.HTTP_URL.key" "http://localhost:$port/" "$Tags.PEER_HOSTNAME.key" "localhost" "$Tags.PEER_PORT.key" Integer "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER @@ -107,5 +77,88 @@ class NettyServerTest extends AgentTestRunner { } } } + + cleanup: + eventLoopGroup.shutdownGracefully() + + where: + handlers | _ + [new HttpServerCodec()] | _ + [new HttpRequestDecoder(), new HttpResponseEncoder()] | _ + } + + def "test #responseCode response handling"() { + setup: + EventLoopGroup eventLoopGroup = new NioEventLoopGroup() + int port = TestUtils.randomOpenPort() + initializeServer(eventLoopGroup, port, new HttpServerCodec(), responseCode) + + def request = new Request.Builder().url("http://localhost:$port/").get().build() + def response = client.newCall(request).execute() + + expect: + response.code() == responseCode.code() + response.body().string() == "Hello World" + + and: + assertTraces(TEST_WRITER, 1) { + trace(0, 1) { + span(0) { + serviceName "unnamed-java-app" + operationName "netty.request" + resourceName name + spanType DDSpanTypes.WEB_SERVLET + errored error + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" responseCode.code() + "$Tags.HTTP_URL.key" "http://localhost:$port/" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" Integer + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$DDTags.SPAN_TYPE" DDSpanTypes.WEB_SERVLET + if (error) { + tag("error", true) + } + defaultTags() + } + } + } + } + + cleanup: + eventLoopGroup.shutdownGracefully() + + where: + responseCode | name | error + HttpResponseStatus.OK | "GET /" | false + HttpResponseStatus.NOT_FOUND | "404" | false + HttpResponseStatus.INTERNAL_SERVER_ERROR | "GET /" | true + } + + def initializeServer(eventLoopGroup, port, handlers, responseCode) { + ServerBootstrap bootstrap = new ServerBootstrap() + .group(eventLoopGroup) + .handler(new LoggingHandler(LogLevel.INFO)) + .childHandler([ + initChannel: { ch -> + ChannelPipeline pipeline = ch.pipeline() + handlers.each { pipeline.addLast(it) } + pipeline.addLast([ + channelRead0 : { ctx, msg -> + if (msg instanceof LastHttpContent) { + ByteBuf content = Unpooled.copiedBuffer("Hello World", CharsetUtil.UTF_8) + FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, responseCode, content) + response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/plain") + response.headers().set(HttpHeaderNames.CONTENT_LENGTH, content.readableBytes()) + ctx.write(response) + } + }, + channelReadComplete: { it.flush() } + ] as SimpleChannelInboundHandler) + } + ] as ChannelInitializer).channel(NioServerSocketChannel) + bootstrap.bind(port).sync() } } diff --git a/dd-java-agent/instrumentation/netty/netty.gradle b/dd-java-agent/instrumentation/netty/netty.gradle deleted file mode 100644 index 53c8554931..0000000000 --- a/dd-java-agent/instrumentation/netty/netty.gradle +++ /dev/null @@ -1,19 +0,0 @@ -apply from: "${rootDir}/gradle/java.gradle" - - -dependencies { - compileOnly group: 'io.netty', name: 'netty-all', version: '4.1.25.Final' - - compile project(':dd-java-agent:agent-tooling') - - compile deps.bytebuddy - compile deps.opentracing - annotationProcessor deps.autoservice - implementation deps.autoservice - - testCompile project(':dd-java-agent:testing') - - testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.4.11.v20180605' - testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' - testCompile group: 'org.asynchttpclient', name: 'async-http-client', version: '2.4.9' -} diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/TagsAssert.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/TagsAssert.groovy index d3f4dbf7b6..3e0b5e93a1 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/TagsAssert.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/TagsAssert.groovy @@ -43,19 +43,22 @@ class TagsAssert { } } + def tag(String name, value) { + assertedTags.add(name) + if (value instanceof Class) { + assert ((Class) value).isInstance(tags[name]) + } else if (value instanceof Closure) { + assert ((Closure) value).call(tags[name]) + } else { + assert tags[name] == value + } + } + def methodMissing(String name, args) { - if (args.length > 1) { + if (args.length != 1) { throw new IllegalArgumentException(args.toString()) } - assertedTags.add(name) - def arg = args[0] - if (arg instanceof Class) { - assert ((Class) arg).isInstance(tags[name]) - } else if (arg instanceof Closure) { - assert ((Closure) arg).call(tags[name]) - } else { - assert tags[name] == arg - } + tag(name, args[0]) } void assertTagsAllVerified() { diff --git a/gradle/java.gradle b/gradle/java.gradle index 6eaf4eaa1e..89c3c23f95 100644 --- a/gradle/java.gradle +++ b/gradle/java.gradle @@ -50,8 +50,10 @@ dependencies { testCompile deps.groovy testCompile deps.testLogging testCompile 'info.solidsoft.spock:spock-global-unroll:0.5.1' - testCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '1.4.6' testCompile group: 'com.github.stefanbirkner', name: 'system-rules', version: '1.17.1' + if (!project.name.contains("netty")) { // avoid screwing up the classpath... + testCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '1.4.6' + } } tasks.withType(Javadoc) { diff --git a/settings.gradle b/settings.gradle index cc4361fe6a..abb774e79b 100644 --- a/settings.gradle +++ b/settings.gradle @@ -36,7 +36,8 @@ include ':dd-java-agent:instrumentation:kafka-streams-0.11' include ':dd-java-agent:instrumentation:lettuce-5' include ':dd-java-agent:instrumentation:mongo-3.1' include ':dd-java-agent:instrumentation:mongo-async-3.3' -include ':dd-java-agent:instrumentation:netty' +include ':dd-java-agent:instrumentation:netty-4.0' +include ':dd-java-agent:instrumentation:netty-4.1' include ':dd-java-agent:instrumentation:okhttp-3' include ':dd-java-agent:instrumentation:osgi-classloading' include ':dd-java-agent:instrumentation:play-2.4' From c0da1f7093eb617c78e0a6c755a3e809cb74eb6f Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 18 Jun 2018 12:52:02 +1000 Subject: [PATCH 3/3] Add version scan. --- buildSrc/src/main/groovy/VersionScanPlugin.groovy | 1 + .../instrumentation/netty-4.0/netty-4.0.gradle | 12 ++++++++++++ .../netty40/NettyChannelPipelineInstrumentation.java | 5 ++++- .../instrumentation/netty-4.1/netty-4.1.gradle | 12 ++++++++++++ .../netty41/NettyChannelPipelineInstrumentation.java | 5 ++++- 5 files changed, 33 insertions(+), 2 deletions(-) diff --git a/buildSrc/src/main/groovy/VersionScanPlugin.groovy b/buildSrc/src/main/groovy/VersionScanPlugin.groovy index 31427e0898..3e00d76418 100644 --- a/buildSrc/src/main/groovy/VersionScanPlugin.groovy +++ b/buildSrc/src/main/groovy/VersionScanPlugin.groovy @@ -241,6 +241,7 @@ class VersionScanPlugin implements Plugin { list.removeIf { def version = it.toString().toLowerCase() return version.contains("rc") || + version.contains(".cr") || version.contains("alpha") || version.contains("beta") || version.contains("-b") || diff --git a/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle b/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle index d5ecb06098..8b58ad2623 100644 --- a/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle +++ b/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle @@ -1,3 +1,15 @@ +apply plugin: 'version-scan' + +versionScan { + group = "io.netty" + module = "netty-all" + legacyModule = "netty" + versions = "[4.0.0.Final,4.1.0.Final)" + verifyPresent = [ + "io.netty.channel.local.LocalEventLoop": null, + ] +} + apply from: "${rootDir}/gradle/java.gradle" apply plugin: 'org.unbroken-dome.test-sets' diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java index 078fc53157..07886b6212 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java @@ -1,5 +1,6 @@ package datadog.trace.instrumentation.netty40; +import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -49,7 +50,9 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Configurab @Override public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder - .type(not(isInterface()).and(hasSuperType(named("io.netty.channel.ChannelPipeline")))) + .type( + not(isInterface()).and(hasSuperType(named("io.netty.channel.ChannelPipeline"))), + classLoaderHasClasses("io.netty.channel.local.LocalEventLoop")) .transform( new HelperInjector( // client helpers diff --git a/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle b/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle index 99bc8af2f4..aa41e542d8 100644 --- a/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle +++ b/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle @@ -1,3 +1,15 @@ +apply plugin: 'version-scan' + +versionScan { + group = "io.netty" + module = "netty-all" + legacyModule = "netty" + versions = "[4.1.0.Final,)" + verifyPresent = [ + "io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder": null, + ] +} + apply from: "${rootDir}/gradle/java.gradle" apply plugin: 'org.unbroken-dome.test-sets' diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java index 34f154ac7f..bb3dfdacb6 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java @@ -1,5 +1,6 @@ package datadog.trace.instrumentation.netty41; +import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -49,7 +50,9 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Configurab @Override public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder - .type(not(isInterface()).and(hasSuperType(named("io.netty.channel.ChannelPipeline")))) + .type( + not(isInterface()).and(hasSuperType(named("io.netty.channel.ChannelPipeline"))), + classLoaderHasClasses("io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder")) .transform( new HelperInjector( // client helpers