From 13e708ec42a0dea701a0c4ca93dfc5639544b99d Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 20 Feb 2019 09:47:35 -0800 Subject: [PATCH] Akka and Apache HttpClient migrate to decorators --- .../src/lagomTest/groovy/LagomTest.groovy | 4 ++ .../akkahttp/AkkaHttpClientDecorator.java | 49 +++++++++++++++ .../AkkaHttpClientInstrumentation.java | 51 +++++----------- .../AkkaHttpClientTransformFlow.scala | 28 ++------- .../akkahttp/AkkaHttpServerDecorator.java | 44 ++++++++++++++ .../AkkaHttpServerInstrumentation.java | 25 ++++---- .../AkkaHttpServerInstrumentationTest.groovy | 8 +++ .../ApacheHttpClientDecorator.java | 60 +++++++++++++++++++ .../ApacheHttpClientInstrumentation.java | 57 +++++++----------- .../latestDepTest/groovy/Play26Test.groovy | 8 +++ 10 files changed, 227 insertions(+), 107 deletions(-) create mode 100644 dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientDecorator.java create mode 100644 dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java create mode 100644 dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientDecorator.java diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy index 997d8bad03..397631b558 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy @@ -69,6 +69,8 @@ class LagomTest extends AgentTestRunner { "$Tags.HTTP_STATUS.key" 101 "$Tags.HTTP_URL.key" "ws://localhost:${server.port()}/echo" "$Tags.HTTP_METHOD.key" "GET" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.port() "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER "$Tags.COMPONENT.key" "akka-http-server" @@ -107,6 +109,8 @@ class LagomTest extends AgentTestRunner { "$Tags.HTTP_STATUS.key" 500 "$Tags.HTTP_URL.key" "ws://localhost:${server.port()}/error" "$Tags.HTTP_METHOD.key" "GET" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.port() "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER "$Tags.COMPONENT.key" "akka-http-server" diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientDecorator.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientDecorator.java new file mode 100644 index 0000000000..71c08e548f --- /dev/null +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientDecorator.java @@ -0,0 +1,49 @@ +package datadog.trace.instrumentation.akkahttp; + +import akka.http.scaladsl.model.HttpRequest; +import akka.http.scaladsl.model.HttpResponse; +import datadog.trace.agent.decorator.HttpClientDecorator; + +public class AkkaHttpClientDecorator extends HttpClientDecorator { + public static final AkkaHttpClientDecorator INSTANCE = new AkkaHttpClientDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"akka-http", "akka-http-client"}; + } + + @Override + protected String component() { + return "akka-http-client"; + } + + @Override + protected String service() { + return null; + } + + @Override + protected String method(final HttpRequest httpRequest) { + return httpRequest.method().value(); + } + + @Override + protected String url(final HttpRequest httpRequest) { + return httpRequest.uri().toString(); + } + + @Override + protected String hostname(final HttpRequest httpRequest) { + return httpRequest.getUri().host().address(); + } + + @Override + protected Integer port(final HttpRequest httpRequest) { + return httpRequest.getUri().port(); + } + + @Override + protected Integer status(final HttpResponse httpResponse) { + return httpResponse.status().intValue(); + } +} diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientInstrumentation.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientInstrumentation.java index 703ac48910..866aa2ceeb 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientInstrumentation.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.akkahttp; -import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -13,18 +12,12 @@ import akka.http.scaladsl.model.HttpResponse; import akka.stream.scaladsl.Flow; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.Config; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import io.opentracing.Scope; import io.opentracing.Span; -import io.opentracing.Tracer; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -55,12 +48,14 @@ public final class AkkaHttpClientInstrumentation extends Instrumenter.Default { return new String[] { AkkaHttpClientInstrumentation.class.getName() + "$OnCompleteHandler", AkkaHttpClientInstrumentation.class.getName() + "$AkkaHttpHeaders", - AkkaHttpClientInstrumentation.class.getPackage().getName() + ".AkkaHttpClientTransformFlow", - AkkaHttpClientInstrumentation.class.getPackage().getName() + ".AkkaHttpClientTransformFlow$", - AkkaHttpClientInstrumentation.class.getPackage().getName() - + ".AkkaHttpClientTransformFlow$$anonfun$transform$1", - AkkaHttpClientInstrumentation.class.getPackage().getName() - + ".AkkaHttpClientTransformFlow$$anonfun$transform$2", + packageName + ".AkkaHttpClientTransformFlow", + packageName + ".AkkaHttpClientTransformFlow$", + packageName + ".AkkaHttpClientTransformFlow$$anonfun$transform$1", + packageName + ".AkkaHttpClientTransformFlow$$anonfun$transform$2", + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.HttpClientDecorator", + packageName + ".AkkaHttpClientDecorator", }; } @@ -102,23 +97,9 @@ public final class AkkaHttpClientInstrumentation extends Instrumenter.Default { return null; } - final Tracer.SpanBuilder builder = - GlobalTracer.get() - .buildSpan("akka-http.request") - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) - .withTag(Tags.COMPONENT.getKey(), "akka-http-client"); - if (request != null) { - builder - .withTag(Tags.HTTP_METHOD.getKey(), request.method().value()) - .withTag(Tags.HTTP_URL.getKey(), request.getUri().toString()) - .withTag(Tags.PEER_PORT.getKey(), request.getUri().port()) - .withTag(Tags.PEER_HOSTNAME.getKey(), request.getUri().host().address()); - if (Config.get().isHttpClientSplitByDomain()) { - builder.withTag(DDTags.SERVICE_NAME, request.getUri().host().address()); - } - } - final Scope scope = builder.startActive(false); + final Scope scope = GlobalTracer.get().buildSpan("akka-http.request").startActive(false); + AkkaHttpClientDecorator.INSTANCE.afterStart(scope.span()); + AkkaHttpClientDecorator.INSTANCE.onRequest(scope.span(), request); if (request != null) { final AkkaHttpHeaders headers = new AkkaHttpHeaders(request); @@ -147,8 +128,8 @@ public final class AkkaHttpClientInstrumentation extends Instrumenter.Default { if (throwable == null) { responseFuture.onComplete(new OnCompleteHandler(span), thiz.system().dispatcher()); } else { - Tags.ERROR.set(span, Boolean.TRUE); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + AkkaHttpClientDecorator.INSTANCE.onError(span, throwable); + AkkaHttpClientDecorator.INSTANCE.beforeFinish(span); span.finish(); } scope.close(); @@ -193,11 +174,11 @@ public final class AkkaHttpClientInstrumentation extends Instrumenter.Default { @Override public Void apply(final Try result) { if (result.isSuccess()) { - Tags.HTTP_STATUS.set(span, result.get().status().intValue()); + AkkaHttpClientDecorator.INSTANCE.onResponse(span, result.get()); } else { - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, result.failed().get())); + AkkaHttpClientDecorator.INSTANCE.onError(span, result.failed().get()); } + AkkaHttpClientDecorator.INSTANCE.beforeFinish(span); span.finish(); return null; } diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientTransformFlow.scala b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientTransformFlow.scala index b0a06b3d4e..237d23f1e4 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientTransformFlow.scala +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpClientTransformFlow.scala @@ -1,15 +1,10 @@ package datadog.trace.instrumentation.akkahttp -import java.util.Collections - import akka.NotUsed import akka.http.scaladsl.model.{HttpRequest, HttpResponse} import akka.stream.scaladsl.Flow -import datadog.trace.api.{Config, DDSpanTypes, DDTags} import io.opentracing.Span -import io.opentracing.log.Fields.ERROR_OBJECT import io.opentracing.propagation.Format -import io.opentracing.tag.Tags import io.opentracing.util.GlobalTracer import scala.util.{Failure, Success, Try} @@ -20,29 +15,18 @@ object AkkaHttpClientTransformFlow { Flow.fromFunction((input: (HttpRequest, T)) => { val (request, data) = input - span = GlobalTracer.get - .buildSpan("akka-http.request") - .withTag(Tags.SPAN_KIND.getKey, Tags.SPAN_KIND_CLIENT) - .withTag(Tags.HTTP_METHOD.getKey, request.method.value) - .withTag(Tags.PEER_HOSTNAME.getKey, request.getUri().host().address()) - .withTag(Tags.PEER_PORT.getKey, request.getUri().port()) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) - .withTag(Tags.COMPONENT.getKey, "akka-http-client") - .withTag(Tags.HTTP_URL.getKey, request.getUri.toString) - .start() - if (Config.get.isHttpClientSplitByDomain) { - span.setTag(DDTags.SERVICE_NAME, request.getUri.host.address) - } + span = GlobalTracer.get.buildSpan("akka-http.request").start() + AkkaHttpClientDecorator.INSTANCE.afterStart(span) + AkkaHttpClientDecorator.INSTANCE.onRequest(span, request) val headers = new AkkaHttpClientInstrumentation.AkkaHttpHeaders(request) GlobalTracer.get.inject(span.context, Format.Builtin.HTTP_HEADERS, headers) (headers.getRequest, data) }).via(flow).map(output => { output._1 match { - case Success(response) => Tags.HTTP_STATUS.set(span, response.status.intValue) - case Failure(e) => - Tags.ERROR.set(span, true) - span.log(Collections.singletonMap(ERROR_OBJECT, e)) + case Success(response) => AkkaHttpClientDecorator.INSTANCE.onResponse(span, response) + case Failure(e) => AkkaHttpClientDecorator.INSTANCE.onError(span, e) } + AkkaHttpClientDecorator.INSTANCE.beforeFinish(span) span.finish() output }) diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java new file mode 100644 index 0000000000..37e159581b --- /dev/null +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerDecorator.java @@ -0,0 +1,44 @@ +package datadog.trace.instrumentation.akkahttp; + +import akka.http.scaladsl.model.HttpRequest; +import akka.http.scaladsl.model.HttpResponse; +import datadog.trace.agent.decorator.HttpServerDecorator; + +public class AkkaHttpServerDecorator extends HttpServerDecorator { + public static final AkkaHttpServerDecorator INSTANCE = new AkkaHttpServerDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"akka-http", "akka-http-server"}; + } + + @Override + protected String component() { + return "akka-http-server"; + } + + @Override + protected String method(final HttpRequest httpRequest) { + return httpRequest.method().value(); + } + + @Override + protected String url(final HttpRequest httpRequest) { + return httpRequest.uri().toString(); + } + + @Override + protected String hostname(final HttpRequest httpRequest) { + return httpRequest.getUri().host().address(); + } + + @Override + protected Integer port(final HttpRequest httpRequest) { + return httpRequest.getUri().port(); + } + + @Override + protected Integer status(final HttpResponse httpResponse) { + return httpResponse.status().intValue(); + } +} diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java index ad375eda61..6e09237079 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/scala/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.akkahttp; -import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -10,8 +9,6 @@ import akka.http.scaladsl.model.HttpResponse; import akka.stream.Materializer; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; import datadog.trace.context.TraceScope; import io.opentracing.Scope; import io.opentracing.Span; @@ -20,7 +17,6 @@ import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -54,7 +50,11 @@ public final class AkkaHttpServerInstrumentation extends Instrumenter.Default { AkkaHttpServerInstrumentation.class.getName() + "$DatadogAsyncWrapper", AkkaHttpServerInstrumentation.class.getName() + "$DatadogAsyncWrapper$1", AkkaHttpServerInstrumentation.class.getName() + "$DatadogAsyncWrapper$2", - AkkaHttpServerInstrumentation.class.getName() + "$AkkaHttpServerHeaders" + AkkaHttpServerInstrumentation.class.getName() + "$AkkaHttpServerHeaders", + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".AkkaHttpServerDecorator", }; } @@ -104,13 +104,11 @@ public final class AkkaHttpServerInstrumentation extends Instrumenter.Default { GlobalTracer.get() .buildSpan("akka-http.request") .asChildOf(extractedContext) - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) - .withTag(Tags.HTTP_METHOD.getKey(), request.method().value()) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_SERVER) - .withTag(Tags.COMPONENT.getKey(), "akka-http-server") - .withTag(Tags.HTTP_URL.getKey(), request.getUri().toString()) .startActive(false); + AkkaHttpServerDecorator.INSTANCE.afterStart(scope.span()); + AkkaHttpServerDecorator.INSTANCE.onRequest(scope.span(), request); + if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true); } @@ -118,7 +116,8 @@ public final class AkkaHttpServerInstrumentation extends Instrumenter.Default { } public static void finishSpan(final Span span, final HttpResponse response) { - Tags.HTTP_STATUS.set(span, response.status().intValue()); + AkkaHttpServerDecorator.INSTANCE.onResponse(span, response); + AkkaHttpServerDecorator.INSTANCE.beforeFinish(span); if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); @@ -127,9 +126,9 @@ public final class AkkaHttpServerInstrumentation extends Instrumenter.Default { } public static void finishSpan(final Span span, final Throwable t) { - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, t)); + AkkaHttpServerDecorator.INSTANCE.onError(span, t); Tags.HTTP_STATUS.set(span, 500); + AkkaHttpServerDecorator.INSTANCE.beforeFinish(span); if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy index b7ffbc6dc1..5bc6caf8d6 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy @@ -56,6 +56,8 @@ class AkkaHttpServerInstrumentationTest extends AgentTestRunner { "$Tags.HTTP_STATUS.key" 200 "$Tags.HTTP_URL.key" "http://localhost:$port/test" "$Tags.HTTP_METHOD.key" "GET" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" port "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER "$Tags.COMPONENT.key" "akka-http-server" @@ -98,6 +100,8 @@ class AkkaHttpServerInstrumentationTest extends AgentTestRunner { "$Tags.HTTP_STATUS.key" 500 "$Tags.HTTP_URL.key" "http://localhost:$port/$endpoint" "$Tags.HTTP_METHOD.key" "GET" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" port "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER "$Tags.COMPONENT.key" "akka-http-server" @@ -138,6 +142,8 @@ class AkkaHttpServerInstrumentationTest extends AgentTestRunner { "$Tags.HTTP_STATUS.key" 500 "$Tags.HTTP_URL.key" "http://localhost:$port/server-error" "$Tags.HTTP_METHOD.key" "GET" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" port "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER "$Tags.COMPONENT.key" "akka-http-server" @@ -177,6 +183,8 @@ class AkkaHttpServerInstrumentationTest extends AgentTestRunner { "$Tags.HTTP_STATUS.key" 404 "$Tags.HTTP_URL.key" "http://localhost:$port/not-found" "$Tags.HTTP_METHOD.key" "GET" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" port "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER "$Tags.COMPONENT.key" "akka-http-server" diff --git a/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientDecorator.java b/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientDecorator.java new file mode 100644 index 0000000000..3a7be238c0 --- /dev/null +++ b/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientDecorator.java @@ -0,0 +1,60 @@ +package datadog.trace.instrumentation.apachehttpclient; + +import datadog.trace.agent.decorator.HttpClientDecorator; +import java.net.URI; +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpUriRequest; + +public class ApacheHttpClientDecorator extends HttpClientDecorator { + public static final ApacheHttpClientDecorator INSTANCE = new ApacheHttpClientDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"httpclient", "apache-httpclient", "apache-http-client"}; + } + + @Override + protected String component() { + return "apache-httpclient"; + } + + @Override + protected String service() { + return null; + } + + @Override + protected String method(final HttpUriRequest httpRequest) { + return httpRequest.getRequestLine().getMethod(); + } + + @Override + protected String url(final HttpUriRequest httpRequest) { + return httpRequest.getRequestLine().getUri(); + } + + @Override + protected String hostname(final HttpUriRequest httpRequest) { + final URI uri = httpRequest.getURI(); + if (uri != null) { + return uri.getHost(); + } else { + return null; + } + } + + @Override + protected Integer port(final HttpUriRequest httpRequest) { + final URI uri = httpRequest.getURI(); + if (uri != null) { + return uri.getPort(); + } else { + return null; + } + } + + @Override + protected Integer status(final HttpResponse httpResponse) { + return httpResponse.getStatusLine().getStatusCode(); + } +} diff --git a/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java b/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java index 3b7829092e..93fb6168d7 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.apachehttpclient; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static io.opentracing.log.Fields.ERROR_OBJECT; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -11,19 +10,14 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.Config; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.Tracer; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.io.IOException; -import java.net.URI; import java.util.Iterator; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -42,7 +36,7 @@ import org.apache.http.client.methods.HttpUriRequest; public class ApacheHttpClientInstrumentation extends Instrumenter.Default { public ApacheHttpClientInstrumentation() { - super("httpclient"); + super("httpclient", "apache-httpclient", "apache-http-client"); } @Override @@ -55,6 +49,10 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { return new String[] { getClass().getName() + "$HttpHeadersInjectAdapter", getClass().getName() + "$WrappingStatusSettingResponseHandler", + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.HttpClientDecorator", + packageName + ".ApacheHttpClientDecorator", }; } @@ -91,18 +89,12 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { return null; } final Tracer tracer = GlobalTracer.get(); - final Scope scope = - tracer - .buildSpan("http.request") - .withTag(Tags.COMPONENT.getKey(), "apache-httpclient") - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) - .withTag(Tags.HTTP_METHOD.getKey(), request.getRequestLine().getMethod()) - .withTag(Tags.HTTP_URL.getKey(), request.getRequestLine().getUri()) - .startActive(true); - + final Scope scope = tracer.buildSpan("http.request").startActive(true); final Span span = scope.span(); + ApacheHttpClientDecorator.INSTANCE.afterStart(span); + ApacheHttpClientDecorator.INSTANCE.onRequest(span, request); + // Wrap the handler so we capture the status code if (handler1 instanceof ResponseHandler) { handler1 = new WrappingStatusSettingResponseHandler(span, (ResponseHandler) handler1); @@ -116,15 +108,6 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { tracer.inject( span.context(), Format.Builtin.HTTP_HEADERS, new HttpHeadersInjectAdapter(request)); } - final URI uri = request.getURI(); - // zuul users have encountered cases where getURI returns null - if (null != uri) { - Tags.PEER_PORT.set(span, uri.getPort() == -1 ? 80 : uri.getPort()); - Tags.PEER_HOSTNAME.set(span, uri.getHost()); - if (Config.get().isHttpClientSplitByDomain()) { - span.setTag(DDTags.SERVICE_NAME, uri.getHost()); - } - } return scope; } @@ -134,19 +117,19 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { @Advice.Return final Object result, @Advice.Thrown final Throwable throwable) { if (scope != null) { - final Span span = scope.span(); + try { + final Span span = scope.span(); - if (result instanceof HttpResponse) { - Tags.HTTP_STATUS.set(span, ((HttpResponse) result).getStatusLine().getStatusCode()); - } // else they probably provided a ResponseHandler. + if (result instanceof HttpResponse) { + ApacheHttpClientDecorator.INSTANCE.onResponse(span, (HttpResponse) result); + } // else they probably provided a ResponseHandler. - if (throwable != null) { - Tags.ERROR.set(span, Boolean.TRUE); - span.log(singletonMap(ERROR_OBJECT, throwable)); - span.finish(); + ApacheHttpClientDecorator.INSTANCE.onError(span, throwable); + ApacheHttpClientDecorator.INSTANCE.beforeFinish(span); + } finally { + scope.close(); + CallDepthThreadLocalMap.reset(HttpClient.class); } - scope.close(); - CallDepthThreadLocalMap.reset(HttpClient.class); } } } @@ -164,7 +147,7 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { public Object handleResponse(final HttpResponse response) throws ClientProtocolException, IOException { if (null != span) { - Tags.HTTP_STATUS.set(span, response.getStatusLine().getStatusCode()); + ApacheHttpClientDecorator.INSTANCE.onResponse(span, response); } return handler.handleResponse(response); } diff --git a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy b/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy index 96a591d114..f4c523e524 100644 --- a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy +++ b/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy @@ -58,6 +58,8 @@ class Play26Test extends AgentTestRunner { "http.status_code" 200 "http.url" "http://localhost:$port/helloplay/spock" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" DDSpanTypes.HTTP_SERVER "component" "akka-http-server" @@ -109,6 +111,8 @@ class Play26Test extends AgentTestRunner { "http.status_code" 500 "http.url" "http://localhost:$port/make-error" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" DDSpanTypes.HTTP_SERVER "component" "akka-http-server" @@ -160,6 +164,8 @@ class Play26Test extends AgentTestRunner { "http.status_code" 500 "http.url" "http://localhost:$port/exception" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" DDSpanTypes.HTTP_SERVER "component" "akka-http-server" @@ -214,6 +220,8 @@ class Play26Test extends AgentTestRunner { "http.status_code" 404 "http.url" "http://localhost:$port/nowhere" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" DDSpanTypes.HTTP_SERVER "component" "akka-http-server"