Migrate HttpURLConnection instrumentation to Decorator

dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionDecorator.java

@@ -0,0 +1,51 @@
+package datadog.trace.instrumentation.http_url_connection;
+
+import datadog.trace.agent.decorator.HttpClientDecorator;
+import java.net.HttpURLConnection;
+import javax.net.ssl.HttpsURLConnection;
+
+public class HttpUrlConnectionDecorator extends HttpClientDecorator<HttpURLConnection, Integer> {
+  public static final HttpUrlConnectionDecorator DECORATE = new HttpUrlConnectionDecorator();
+
+  @Override
+  protected String[] instrumentationNames() {
+    return new String[] {"httpurlconnection"};
+  }
+
+  @Override
+  protected String component() {
+    return "http-url-connection";
+  }
+
+  @Override
+  protected String method(final HttpURLConnection connection) {
+    return connection.getRequestMethod();
+  }
+
+  @Override
+  protected String url(final HttpURLConnection connection) {
+    return connection.getURL().toString();
+  }
+
+  @Override
+  protected String hostname(final HttpURLConnection connection) {
+    return connection.getURL().getHost();
+  }
+
+  @Override
+  protected Integer port(final HttpURLConnection connection) {
+    final int port = connection.getURL().getPort();
+    if (port > 0) {
+      return port;
+    } else if (connection instanceof HttpsURLConnection) {
+      return 443;
+    } else {
+      return 80;
+    }
+  }
+
+  @Override
+  protected Integer status(final Integer status) {
+    return status;
+  }
+}

dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java

@@ -1,7 +1,7 @@
 package datadog.trace.instrumentation.http_url_connection;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static io.opentracing.log.Fields.ERROR_OBJECT;
+import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionDecorator.DECORATE;
 import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;
@@ -10,9 +10,6 @@ import static net.bytebuddy.matcher.ElementMatchers.not;
 
 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 datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
@@ -20,13 +17,10 @@ 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.net.HttpURLConnection;
-import java.net.URL;
 import java.util.Iterator;
 import java.util.Map;
-import javax.net.ssl.HttpsURLConnection;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
@@ -49,9 +43,13 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
   @Override
   public String[] helperClassNames() {
     return new String[] {
+      "datadog.trace.agent.decorator.BaseDecorator",
+      "datadog.trace.agent.decorator.ClientDecorator",
+      "datadog.trace.agent.decorator.HttpClientDecorator",
+      packageName + ".HttpUrlConnectionDecorator",
       HttpUrlConnectionInstrumentation.class.getName() + "$HeadersInjectAdapter",
       HttpUrlConnectionInstrumentation.class.getName() + "$HttpUrlState",
-      HttpUrlConnectionInstrumentation.class.getName() + "$HttpUrlState$1"
+      HttpUrlConnectionInstrumentation.class.getName() + "$HttpUrlState$1",
     };
   }
 
@@ -165,7 +163,6 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
   public static class HttpUrlState {
 
     public static final String OPERATION_NAME = "http.request";
-    public static final String COMPONENT_NAME = "http-url-connection";
 
     public static final ContextStore.Factory<HttpUrlState> FACTORY =
         new ContextStore.Factory<HttpUrlState>() {
@@ -179,28 +176,10 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
     private volatile boolean finished = false;
 
     public Span startSpan(final HttpURLConnection connection) {
-      final Tracer.SpanBuilder builder =
+      final Tracer.SpanBuilder builder = GlobalTracer.get().buildSpan(OPERATION_NAME);
-          GlobalTracer.get()
-              .buildSpan(OPERATION_NAME)
-              .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
-              .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT);
       span = builder.start();
-      final URL url = connection.getURL();
+      DECORATE.afterStart(span);
-      Tags.COMPONENT.set(span, COMPONENT_NAME);
+      DECORATE.onRequest(span, connection);
-      Tags.HTTP_URL.set(span, url.toString());
-      Tags.PEER_HOSTNAME.set(span, url.getHost());
-      if (Config.get().isHttpClientSplitByDomain()) {
-        span.setTag(DDTags.SERVICE_NAME, url.getHost());
-      }
-
-      if (url.getPort() > 0) {
-        Tags.PEER_PORT.set(span, url.getPort());
-      } else if (connection instanceof HttpsURLConnection) {
-        Tags.PEER_PORT.set(span, 443);
-      } else {
-        Tags.PEER_PORT.set(span, 80);
-      }
-      Tags.HTTP_METHOD.set(span, connection.getRequestMethod());
       return span;
     }
 
@@ -217,8 +196,8 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
     }
 
     public void finishSpan(final Throwable throwable) {
-      Tags.ERROR.set(span, true);
+      DECORATE.onError(span, throwable);
-      span.log(singletonMap(ERROR_OBJECT, throwable));
+      DECORATE.beforeFinish(span);
       span.finish();
       span = null;
       finished = true;
@@ -231,7 +210,8 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
        * (e.g. breaks getOutputStream).
        */
       if (responseCode > 0) {
-        Tags.HTTP_STATUS.set(span, responseCode);
+        DECORATE.onResponse(span, responseCode);
+        DECORATE.beforeFinish(span);
         span.finish();
         span = null;
         finished = true;

dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy

@@ -11,7 +11,6 @@ import spock.lang.Shared
 import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
-import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.COMPONENT_NAME
 import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.OPERATION_NAME
 
 class HttpUrlConnectionTest extends AgentTestRunner {
@@ -76,7 +75,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"
@@ -93,7 +92,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"
@@ -157,7 +156,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"
@@ -174,7 +173,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"
@@ -222,7 +221,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"
@@ -270,7 +269,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"
@@ -335,7 +334,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"
@@ -402,7 +401,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           parent()
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"
@@ -448,7 +447,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored false
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$server.address"

dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy

@@ -8,7 +8,6 @@ import io.opentracing.util.GlobalTracer
 
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
-import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.COMPONENT_NAME
 import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.OPERATION_NAME
 
 class UrlConnectionTest extends AgentTestRunner {
@@ -48,7 +47,7 @@ class UrlConnectionTest extends AgentTestRunner {
           childOf span(0)
           errored true
           tags {
-            "$Tags.COMPONENT.key" COMPONENT_NAME
+            "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.HTTP_URL.key" "$url"