Make all http client tests extend HttpClientTest

Add flexibility to handle inconsistencies between client integrations.

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java

@@ -1,6 +1,7 @@
 package datadog.trace.agent.decorator;
 
 import static io.opentracing.log.Fields.ERROR_OBJECT;
+import static java.util.Collections.singletonMap;
 
 import datadog.trace.api.Config;
 import datadog.trace.api.DDTags;
@@ -13,8 +14,8 @@ import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.TreeSet;
+import java.util.concurrent.ExecutionException;
 
 public abstract class BaseDecorator {
 
@@ -83,7 +84,10 @@ public abstract class BaseDecorator {
     assert span != null;
     if (throwable != null) {
       Tags.ERROR.set(span, true);
-      span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
+      span.log(
+          singletonMap(
+              ERROR_OBJECT,
+              throwable instanceof ExecutionException ? throwable.getCause() : throwable));
     }
     return span;
   }

dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy

@@ -1,152 +1,50 @@
 import akka.actor.ActorSystem
 import akka.http.javadsl.Http
+import akka.http.javadsl.model.HttpMethods
 import akka.http.javadsl.model.HttpRequest
-import akka.http.javadsl.model.HttpResponse
+import akka.http.javadsl.model.headers.RawHeader
-import akka.japi.Pair
 import akka.stream.ActorMaterializer
-import akka.stream.StreamTcpException
+import datadog.trace.agent.test.base.HttpClientTest
-import akka.stream.javadsl.Sink
-import akka.stream.javadsl.Source
-import datadog.trace.agent.test.AgentTestRunner
-import datadog.trace.api.Config
 import datadog.trace.api.DDSpanTypes
+import datadog.trace.instrumentation.akkahttp.AkkaHttpClientDecorator
 import io.opentracing.tag.Tags
-import scala.util.Try
-import spock.lang.AutoCleanup
 import spock.lang.Shared
 
-import java.util.concurrent.CompletionStage
+class AkkaHttpClientInstrumentationTest extends HttpClientTest<AkkaHttpClientDecorator> {
-import java.util.concurrent.ExecutionException
-
-import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
-import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
-import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
-
-class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
-
-  private static final String MESSAGE = "an\nmultiline\nhttp\nresponse"
   private static final long TIMEOUT = 10000L
 
-  @AutoCleanup
-  @Shared
-  def server = httpServer {
-    handlers {
-      prefix("success") {
-        handleDistributedRequest()
-
-        response.status(200).send(MESSAGE)
-      }
-
-      prefix("error") {
-        handleDistributedRequest()
-
-        throw new RuntimeException("error")
-      }
-    }
-  }
-
   @Shared
   ActorSystem system = ActorSystem.create()
   @Shared
   ActorMaterializer materializer = ActorMaterializer.create(system)
 
-  def pool = Http.get(system).superPool(materializer)
+//  String readMessage(HttpResponse response) {
+//    response.entity().toStrict(TIMEOUT, materializer).toCompletableFuture().get().getData().utf8String()
+//  }
 
-  def "#route request trace"() {
+  @Override
-    setup:
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    def url = server.address.resolve("/" + route).toURL()
+    def request = HttpRequest.create(uri.toString())
+      .withMethod(HttpMethods.lookup(method).get())
+      .addHeaders(headers.collect { RawHeader.create(it.key, it.value) })
 
-    HttpRequest request = HttpRequest.create(url.toString())
+    def response = Http.get(system).singleRequest(request, materializer).toCompletableFuture().get()
-
+    callback?.call()
-    when:
+    return response.status().intValue()
-    HttpResponse response = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
-      Http.get(system)
-        .singleRequest(request, materializer)
-        .toCompletableFuture().get()
-    }
-    String message = readMessage(response)
-
-    then:
-    response.status().intValue() == expectedStatus
-    if (expectedMessage != null) {
-      message == expectedMessage
-    }
-
-    assertTraces(2) {
-      server.distributedRequestTrace(it, 0, TEST_WRITER[1][0])
-      trace(1, 1) {
-        span(0) {
-          parent()
-          serviceName renameService ? "localhost" : "unnamed-java-app"
-          operationName "akka-http.request"
-          resourceName "GET /$route"
-          spanType DDSpanTypes.HTTP_CLIENT
-          errored expectedError
-          tags {
-            defaultTags()
-            "$Tags.HTTP_STATUS.key" expectedStatus
-            "$Tags.HTTP_URL.key" "${server.address}/$route"
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.PEER_HOSTNAME.key" server.address.host
-            "$Tags.PEER_PORT.key" server.address.port
-            "$Tags.COMPONENT.key" "akka-http-client"
-            if (expectedError) {
-              "$Tags.ERROR.key" true
-            }
-          }
-        }
-      }
-    }
-
-    where:
-    route     | expectedStatus | expectedError | expectedMessage | renameService
-    "success" | 200            | false         | MESSAGE         | true
-    "error"   | 500            | true          | null            | false
   }
 
-  def "error request trace"() {
+  @Override
-    setup:
+  AkkaHttpClientDecorator decorator() {
-    def url = new URL("http://localhost:$UNUSABLE_PORT/test")
+    return AkkaHttpClientDecorator.DECORATE
+  }
 
-    HttpRequest request = HttpRequest.create(url.toString())
+  @Override
-    CompletionStage<HttpResponse> responseFuture =
+  String expectedOperationName() {
-      withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
+    return "akka-http.request"
-        Http.get(system)
+  }
-          .singleRequest(request, materializer)
-      }
 
-    when:
+  boolean testRedirects() {
-    responseFuture.toCompletableFuture().get()
+    false
-
-    then:
-    thrown ExecutionException
-    assertTraces(1) {
-      trace(0, 1) {
-        span(0) {
-          parent()
-          serviceName renameService ? "localhost" : "unnamed-java-app"
-          operationName "akka-http.request"
-          resourceName "GET /test"
-          spanType DDSpanTypes.HTTP_CLIENT
-          errored true
-          tags {
-            defaultTags()
-            "$Tags.HTTP_URL.key" url.toString()
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.PEER_HOSTNAME.key" server.address.host
-            "$Tags.PEER_PORT.key" UNUSABLE_PORT
-            "$Tags.COMPONENT.key" "akka-http-client"
-            "$Tags.ERROR.key" true
-            errorTags(StreamTcpException, { it.contains("Tcp command") })
-          }
-        }
-      }
-    }
-
-    where:
-    renameService << [false, true]
   }
 
   def "singleRequest exception trace"() {
@@ -179,107 +77,4 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
     where:
     renameService << [false, true]
   }
-
-  def "#route pool request trace"() {
-    setup:
-    def url = server.address.resolve("/" + route).toURL()
-
-    when:
-    HttpResponse response = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
-      Source
-        .<Pair<HttpRequest, Integer>> single(new Pair(HttpRequest.create(url.toString()), 1))
-        .via(pool)
-        .runWith(Sink.<Pair<Try<HttpResponse>, Integer>> head(), materializer)
-        .toCompletableFuture().get().first().get()
-    }
-    String message = readMessage(response)
-
-    then:
-    response.status().intValue() == expectedStatus
-    if (expectedMessage != null) {
-      message == expectedMessage
-    }
-
-    assertTraces(2) {
-      server.distributedRequestTrace(it, 0, TEST_WRITER[1][0])
-      trace(1, 1) {
-        span(0) {
-          parent()
-          serviceName renameService ? "localhost" : "unnamed-java-app"
-          operationName "akka-http.request"
-          resourceName "GET /$route"
-          spanType DDSpanTypes.HTTP_CLIENT
-          errored expectedError
-          tags {
-            defaultTags()
-            "$Tags.HTTP_STATUS.key" expectedStatus
-            "$Tags.HTTP_URL.key" "${server.address}/$route"
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.PEER_HOSTNAME.key" server.address.host
-            "$Tags.PEER_PORT.key" server.address.port
-            "$Tags.COMPONENT.key" "akka-http-client"
-            if (expectedError) {
-              "$Tags.ERROR.key" true
-            }
-          }
-        }
-      }
-    }
-
-    where:
-    route     | expectedStatus | expectedError | expectedMessage | renameService
-    "success" | 200            | false         | MESSAGE         | true
-    "error"   | 500            | true          | null            | false
-  }
-
-  def "error request pool trace"() {
-    setup:
-    // Use port number that really should be closed
-    def url = new URL("http://localhost:$UNUSABLE_PORT/test")
-
-    def response = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
-      Source
-        .<Pair<HttpRequest, Integer>> single(new Pair(HttpRequest.create(url.toString()), 1))
-        .via(pool)
-        .runWith(Sink.<Pair<Try<HttpResponse>, Integer>> head(), materializer)
-        .toCompletableFuture().get().first()
-    }
-
-    when:
-    response.get()
-
-    then:
-    thrown StreamTcpException
-    assertTraces(1) {
-      trace(0, 1) {
-        span(0) {
-          parent()
-          serviceName renameService ? "localhost" : "unnamed-java-app"
-          operationName "akka-http.request"
-          resourceName "GET /test"
-          spanType DDSpanTypes.HTTP_CLIENT
-          errored true
-          tags {
-            defaultTags()
-            "$Tags.HTTP_URL.key" url.toString()
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.PEER_HOSTNAME.key" server.address.host
-            "$Tags.PEER_PORT.key" UNUSABLE_PORT
-            "$Tags.COMPONENT.key" "akka-http-client"
-            "$Tags.ERROR.key" true
-            errorTags(StreamTcpException, { it.contains("Tcp command") })
-          }
-        }
-      }
-    }
-
-    where:
-    renameService << [false, true]
-  }
-
-  String readMessage(HttpResponse response) {
-    response.entity().toStrict(TIMEOUT, materializer).toCompletableFuture().get().getData().utf8String()
-  }
 }

dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientPoolInstrumentationTest.groovy

@@ -0,0 +1,59 @@
+import akka.actor.ActorSystem
+import akka.http.javadsl.Http
+import akka.http.javadsl.model.HttpMethods
+import akka.http.javadsl.model.HttpRequest
+import akka.http.javadsl.model.HttpResponse
+import akka.http.javadsl.model.headers.RawHeader
+import akka.japi.Pair
+import akka.stream.ActorMaterializer
+import akka.stream.javadsl.Sink
+import akka.stream.javadsl.Source
+import datadog.trace.agent.test.base.HttpClientTest
+import datadog.trace.instrumentation.akkahttp.AkkaHttpClientDecorator
+import scala.util.Try
+import spock.lang.Shared
+
+class AkkaHttpClientPoolInstrumentationTest extends HttpClientTest<AkkaHttpClientDecorator> {
+  private static final long TIMEOUT = 10000L
+
+  @Shared
+  ActorSystem system = ActorSystem.create()
+  @Shared
+  ActorMaterializer materializer = ActorMaterializer.create(system)
+
+  def pool = Http.get(system).superPool(materializer)
+
+
+//  String readMessage(HttpResponse response) {
+//    response.entity().toStrict(TIMEOUT, materializer).toCompletableFuture().get().getData().utf8String()
+//  }
+
+  @Override
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
+    def request = HttpRequest.create(uri.toString())
+      .withMethod(HttpMethods.lookup(method).get())
+      .addHeaders(headers.collect { RawHeader.create(it.key, it.value) })
+
+    def response = Source
+      .<Pair<HttpRequest, Integer>> single(new Pair(request, 1))
+      .via(pool)
+      .runWith(Sink.<Pair<Try<HttpResponse>, Integer>> head(), materializer)
+      .toCompletableFuture().get().first().get()
+    callback?.call()
+    return response.status().intValue()
+  }
+
+  @Override
+  AkkaHttpClientDecorator decorator() {
+    return AkkaHttpClientDecorator.DECORATE
+  }
+
+  @Override
+  String expectedOperationName() {
+    return "akka-http.request"
+  }
+
+  boolean testRedirects() {
+    false
+  }
+}

dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientCallbackTest.groovy

@@ -2,7 +2,6 @@ import datadog.trace.agent.test.base.HttpClientTest
 import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator
 import io.opentracing.util.GlobalTracer
 import org.apache.http.HttpResponse
-import org.apache.http.client.methods.HttpGet
 import org.apache.http.concurrent.FutureCallback
 import org.apache.http.impl.nio.client.HttpAsyncClients
 import org.apache.http.message.BasicHeader
@@ -23,11 +22,9 @@ class ApacheHttpAsyncClientCallbackTest extends HttpClientTest<ApacheHttpAsyncCl
 
   @Override
   int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    assert method == "GET"
-
     def hasParent = GlobalTracer.get().activeSpan() != null
 
-    HttpGet request = new HttpGet(uri)
+    def request = new HttpUriRequest(method, uri)
     headers.entrySet().each {
       request.addHeader(new BasicHeader(it.key, it.value))
     }

dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientNullCallbackTest.groovy

@@ -1,6 +1,5 @@
 import datadog.trace.agent.test.base.HttpClientTest
 import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator
-import org.apache.http.client.methods.HttpGet
 import org.apache.http.impl.nio.client.HttpAsyncClients
 import org.apache.http.message.BasicHeader
 import spock.lang.AutoCleanup
@@ -20,9 +19,7 @@ class ApacheHttpAsyncClientNullCallbackTest extends HttpClientTest<ApacheHttpAsy
 
   @Override
   int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    assert method == "GET"
+    def request = new HttpUriRequest(method, uri)
-
-    HttpGet request = new HttpGet(uri)
     headers.entrySet().each {
       request.addHeader(new BasicHeader(it.key, it.value))
     }

dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientTest.groovy

@@ -1,7 +1,6 @@
 import datadog.trace.agent.test.base.HttpClientTest
 import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator
 import org.apache.http.HttpResponse
-import org.apache.http.client.methods.HttpGet
 import org.apache.http.concurrent.FutureCallback
 import org.apache.http.impl.nio.client.HttpAsyncClients
 import org.apache.http.message.BasicHeader
@@ -20,8 +19,7 @@ class ApacheHttpAsyncClientTest extends HttpClientTest<ApacheHttpAsyncClientDeco
 
   @Override
   int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    assert method == "GET"
+    def request = new HttpUriRequest(method, uri)
-    HttpGet request = new HttpGet(uri)
     headers.entrySet().each {
       request.addHeader(new BasicHeader(it.key, it.value))
     }
@@ -43,7 +41,7 @@ class ApacheHttpAsyncClientTest extends HttpClientTest<ApacheHttpAsyncClientDeco
     }
 
     def response = client.execute(request, handler).get()
-    response.entity.getContent().close() // Make sure the connection is closed.
+    response.entity?.content?.close() // Make sure the connection is closed.
     response.statusLine.statusCode
   }
 

dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/HttpUriRequest.groovy

@@ -0,0 +1,16 @@
+import org.apache.http.client.methods.HttpRequestBase
+
+class HttpUriRequest extends HttpRequestBase {
+
+  private final String methodName;
+
+  HttpUriRequest(final String methodName, final URI uri) {
+    this.methodName = methodName;
+    setURI(uri);
+  }
+
+  @Override
+  String getMethod() {
+    return methodName;
+  }
+}

dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientResponseHandlerTest.groovy

@@ -2,7 +2,6 @@ import datadog.trace.agent.test.base.HttpClientTest
 import datadog.trace.instrumentation.apachehttpclient.ApacheHttpClientDecorator
 import org.apache.http.HttpResponse
 import org.apache.http.client.ResponseHandler
-import org.apache.http.client.methods.HttpGet
 import org.apache.http.impl.client.DefaultHttpClient
 import org.apache.http.message.BasicHeader
 import spock.lang.Shared
@@ -22,8 +21,7 @@ class ApacheHttpClientResponseHandlerTest extends HttpClientTest<ApacheHttpClien
 
   @Override
   int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    assert method == "GET"
+    def request = new HttpUriRequest(method, uri)
-    HttpGet request = new HttpGet(uri)
     headers.entrySet().each {
       request.addHeader(new BasicHeader(it.key, it.value))
     }

dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy

@@ -1,6 +1,5 @@
 import datadog.trace.agent.test.base.HttpClientTest
 import datadog.trace.instrumentation.apachehttpclient.ApacheHttpClientDecorator
-import org.apache.http.client.methods.HttpGet
 import org.apache.http.impl.client.DefaultHttpClient
 import org.apache.http.message.BasicHeader
 import spock.lang.Shared
@@ -12,16 +11,16 @@ class ApacheHttpClientTest extends HttpClientTest<ApacheHttpClientDecorator> {
 
   @Override
   int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    assert method == "GET"
+    def request = new HttpUriRequest(method, uri)
-    HttpGet request = new HttpGet(uri)
     headers.entrySet().each {
       request.addHeader(new BasicHeader(it.key, it.value))
     }
 
     def response = client.execute(request)
     callback?.call()
-    response.entity.getContent().close() // Make sure the connection is closed.
+    response.entity?.content?.close() // Make sure the connection is closed.
-    response.statusLine.statusCode
+
+    return response.statusLine.statusCode
   }
 
   @Override

dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/HttpUriRequest.groovy

@@ -0,0 +1,16 @@
+import org.apache.http.client.methods.HttpRequestBase
+
+class HttpUriRequest extends HttpRequestBase {
+
+  private final String methodName;
+
+  HttpUriRequest(final String methodName, final URI uri) {
+    this.methodName = methodName;
+    setURI(uri);
+  }
+
+  @Override
+  String getMethod() {
+    return methodName;
+  }
+}

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

@@ -0,0 +1,29 @@
+import datadog.trace.agent.test.base.HttpClientTest
+import datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionDecorator
+
+class HttpUrlConnectionResponseCodeOnlyTest extends HttpClientTest<HttpUrlConnectionDecorator> {
+
+  @Override
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
+    HttpURLConnection connection = uri.toURL().openConnection()
+    try {
+      connection.setRequestMethod(method)
+      headers.each { connection.setRequestProperty(it.key, it.value) }
+      connection.setRequestProperty("Connection", "close")
+      return connection.getResponseCode()
+    } finally {
+      callback?.call()
+      connection.disconnect()
+    }
+  }
+
+  @Override
+  HttpUrlConnectionDecorator decorator() {
+    return HttpUrlConnectionDecorator.DECORATE
+  }
+
+  @Override
+  boolean testRedirects() {
+    false
+  }
+}

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

@@ -1,41 +1,59 @@
-import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.base.HttpClientTest
 import datadog.trace.api.Config
 import datadog.trace.api.DDSpanTypes
+import datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionDecorator
 import io.opentracing.tag.Tags
 import io.opentracing.util.GlobalTracer
-import org.springframework.web.client.RestTemplate
+import spock.lang.Ignore
-import spock.lang.AutoCleanup
 import spock.lang.Requires
-import spock.lang.Shared
 import sun.net.www.protocol.https.HttpsURLConnectionImpl
 
-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.OPERATION_NAME
 
-class HttpUrlConnectionTest extends AgentTestRunner {
+class HttpUrlConnectionTest extends HttpClientTest<HttpUrlConnectionDecorator> {
 
-  static final RESPONSE = "<html><body><h1>Hello test.</h1>"
+  static final RESPONSE = "Hello."
-  static final STATUS = 202
+  static final STATUS = 200
 
-  @AutoCleanup
+  @Override
-  @Shared
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-  def server = httpServer {
+    HttpURLConnection connection = uri.toURL().openConnection()
-    handlers {
+    try {
-      all {
+      connection.setRequestMethod(method)
-        handleDistributedRequest()
+      headers.each { connection.setRequestProperty(it.key, it.value) }
-
+      connection.setRequestProperty("Connection", "close")
-        response.status(STATUS).send(RESPONSE)
+      connection.useCaches = true
-      }
+      def parentSpan = GlobalTracer.get().scopeManager().active()
+      def stream = connection.inputStream
+      assert GlobalTracer.get().scopeManager().active() == parentSpan
+      stream.readLines()
+      stream.close()
+      callback?.call()
+      return connection.getResponseCode()
+    } finally {
+      connection.disconnect()
     }
   }
 
+  @Override
+  HttpUrlConnectionDecorator decorator() {
+    return HttpUrlConnectionDecorator.DECORATE
+  }
+
+  @Override
+  boolean testRedirects() {
+    false
+  }
+
+  @Ignore
   def "trace request with propagation (useCaches: #useCaches)"() {
     setup:
+    def url = server.address.resolve("/success").toURL()
     withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
       runUnderTrace("someTrace") {
-        HttpURLConnection connection = server.address.toURL().openConnection()
+        HttpURLConnection connection = url.openConnection()
         connection.useCaches = useCaches
         assert GlobalTracer.get().scopeManager().active() != null
         def stream = connection.inputStream
@@ -45,7 +63,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
         assert lines == [RESPONSE]
 
         // call again to ensure the cycling is ok
-        connection = server.getAddress().toURL().openConnection()
+        connection = url.openConnection()
         connection.useCaches = useCaches
         assert GlobalTracer.get().scopeManager().active() != null
         assert connection.getResponseCode() == STATUS // call before input stream to test alternate behavior
@@ -73,14 +91,14 @@ class HttpUrlConnectionTest extends AgentTestRunner {
         span(1) {
           serviceName renameService ? "localhost" : "unnamed-java-app"
           operationName OPERATION_NAME
-          resourceName "GET /"
+          resourceName "GET $url.path"
           spanType DDSpanTypes.HTTP_CLIENT
           childOf span(0)
           errored false
           tags {
             "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
+            "$Tags.HTTP_URL.key" "$url"
             "$Tags.HTTP_METHOD.key" "GET"
             "$Tags.HTTP_STATUS.key" STATUS
             "$Tags.PEER_HOSTNAME.key" "localhost"
@@ -91,14 +109,14 @@ class HttpUrlConnectionTest extends AgentTestRunner {
         span(2) {
           serviceName renameService ? "localhost" : "unnamed-java-app"
           operationName OPERATION_NAME
-          resourceName "GET /"
+          resourceName "GET $url.path"
           spanType DDSpanTypes.HTTP_CLIENT
           childOf span(0)
           errored false
           tags {
             "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
+            "$Tags.HTTP_URL.key" "$url"
             "$Tags.HTTP_METHOD.key" "GET"
             "$Tags.HTTP_STATUS.key" STATUS
             "$Tags.PEER_HOSTNAME.key" "localhost"
@@ -114,11 +132,13 @@ class HttpUrlConnectionTest extends AgentTestRunner {
     renameService << [true, false]
   }
 
+  @Ignore
   def "trace request without propagation (useCaches: #useCaches)"() {
     setup:
+    def url = server.address.resolve("/success").toURL()
     withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
       runUnderTrace("someTrace") {
-        HttpURLConnection connection = server.address.toURL().openConnection()
+        HttpURLConnection connection = url.openConnection()
         connection.useCaches = useCaches
         connection.addRequestProperty("is-dd-server", "false")
         assert GlobalTracer.get().scopeManager().active() != null
@@ -130,7 +150,7 @@ class HttpUrlConnectionTest extends AgentTestRunner {
         assert lines == [RESPONSE]
 
         // call again to ensure the cycling is ok
-        connection = server.getAddress().toURL().openConnection()
+        connection = url.openConnection()
         connection.useCaches = useCaches
         connection.addRequestProperty("is-dd-server", "false")
         assert GlobalTracer.get().scopeManager().active() != null
@@ -156,14 +176,14 @@ class HttpUrlConnectionTest extends AgentTestRunner {
         span(1) {
           serviceName renameService ? "localhost" : "unnamed-java-app"
           operationName OPERATION_NAME
-          resourceName "GET /"
+          resourceName "GET $url.path"
           spanType DDSpanTypes.HTTP_CLIENT
           childOf span(0)
           errored false
           tags {
             "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
+            "$Tags.HTTP_URL.key" "$url"
             "$Tags.HTTP_METHOD.key" "GET"
             "$Tags.HTTP_STATUS.key" STATUS
             "$Tags.PEER_HOSTNAME.key" "localhost"
@@ -174,14 +194,14 @@ class HttpUrlConnectionTest extends AgentTestRunner {
         span(2) {
           serviceName renameService ? "localhost" : "unnamed-java-app"
           operationName OPERATION_NAME
-          resourceName "GET /"
+          resourceName "GET $url.path"
           spanType DDSpanTypes.HTTP_CLIENT
           childOf span(0)
           errored false
           tags {
             "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
+            "$Tags.HTTP_URL.key" "$url"
             "$Tags.HTTP_METHOD.key" "GET"
             "$Tags.HTTP_STATUS.key" STATUS
             "$Tags.PEER_HOSTNAME.key" "localhost"
@@ -197,59 +217,13 @@ class HttpUrlConnectionTest extends AgentTestRunner {
     renameService << [false, true]
   }
 
-  def "test response code"() {
+  @Ignore
-    setup:
-    withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
-      runUnderTrace("someTrace") {
-        HttpURLConnection connection = server.address.toURL().openConnection()
-        connection.setRequestMethod("HEAD")
-        connection.addRequestProperty("is-dd-server", "false")
-        assert GlobalTracer.get().scopeManager().active() != null
-        assert connection.getResponseCode() == STATUS
-      }
-    }
-
-    expect:
-    assertTraces(1) {
-      trace(0, 2) {
-        span(0) {
-          operationName "someTrace"
-          parent()
-          errored false
-          tags {
-            defaultTags()
-          }
-        }
-        span(1) {
-          serviceName renameService ? "localhost" : "unnamed-java-app"
-          operationName OPERATION_NAME
-          resourceName "HEAD /"
-          spanType DDSpanTypes.HTTP_CLIENT
-          childOf span(0)
-          errored false
-          tags {
-            "$Tags.COMPONENT.key" "http-url-connection"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
-            "$Tags.HTTP_METHOD.key" "HEAD"
-            "$Tags.HTTP_STATUS.key" STATUS
-            "$Tags.PEER_HOSTNAME.key" "localhost"
-            "$Tags.PEER_PORT.key" server.address.port
-            defaultTags()
-          }
-        }
-      }
-    }
-
-    where:
-    renameService << [false, true]
-  }
-
   def "test broken API usage"() {
     setup:
+    def url = server.address.resolve("/success").toURL()
     HttpURLConnection conn = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
       runUnderTrace("someTrace") {
-        HttpURLConnection connection = server.address.toURL().openConnection()
+        HttpURLConnection connection = url.openConnection()
         connection.setRequestProperty("Connection", "close")
         connection.addRequestProperty("is-dd-server", "false")
         assert GlobalTracer.get().scopeManager().active() != null
@@ -272,14 +246,14 @@ class HttpUrlConnectionTest extends AgentTestRunner {
         span(1) {
           serviceName renameService ? "localhost" : "unnamed-java-app"
           operationName OPERATION_NAME
-          resourceName "GET /"
+          resourceName "GET $url.path"
           spanType DDSpanTypes.HTTP_CLIENT
           childOf span(0)
           errored false
           tags {
             "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
+            "$Tags.HTTP_URL.key" "$url"
             "$Tags.HTTP_METHOD.key" "GET"
             "$Tags.HTTP_STATUS.key" STATUS
             "$Tags.PEER_HOSTNAME.key" "localhost"
@@ -298,11 +272,13 @@ class HttpUrlConnectionTest extends AgentTestRunner {
     renameService = (iteration % 2 == 0) // alternate even/odd
   }
 
+  @Ignore
   def "test post request"() {
     setup:
+    def url = server.address.resolve("/success").toURL()
     withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
       runUnderTrace("someTrace") {
-        HttpURLConnection connection = server.address.toURL().openConnection()
+        HttpURLConnection connection = url.openConnection()
         connection.setRequestMethod("POST")
 
         String urlParameters = "q=ASDF&w=&e=&r=12345&t="
@@ -338,129 +314,14 @@ class HttpUrlConnectionTest extends AgentTestRunner {
         span(1) {
           serviceName renameService ? "localhost" : "unnamed-java-app"
           operationName OPERATION_NAME
-          resourceName "POST /"
+          resourceName "POST $url.path"
           spanType DDSpanTypes.HTTP_CLIENT
           childOf span(0)
           errored false
           tags {
             "$Tags.COMPONENT.key" "http-url-connection"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
+            "$Tags.HTTP_URL.key" "$url"
-            "$Tags.HTTP_METHOD.key" "POST"
-            "$Tags.HTTP_STATUS.key" STATUS
-            "$Tags.PEER_HOSTNAME.key" "localhost"
-            "$Tags.PEER_PORT.key" server.address.port
-            defaultTags()
-          }
-        }
-      }
-    }
-
-    where:
-    renameService << [false, true]
-  }
-
-  def "request that looks like a trace submission is ignored"() {
-    setup:
-    runUnderTrace("someTrace") {
-      HttpURLConnection connection = server.address.toURL().openConnection()
-      connection.addRequestProperty("Datadog-Meta-Lang", "false")
-      connection.addRequestProperty("is-dd-server", "false")
-      def stream = connection.inputStream
-      def lines = stream.readLines()
-      stream.close()
-      assert connection.getResponseCode() == STATUS
-      assert lines == [RESPONSE]
-    }
-
-    expect:
-    assertTraces(1) {
-      trace(0, 1) {
-        span(0) {
-          operationName "someTrace"
-          parent()
-          errored false
-          tags {
-            defaultTags()
-          }
-        }
-      }
-    }
-  }
-
-  def "top level httpurlconnection tracing disabled"() {
-    setup:
-    withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
-      HttpURLConnection connection = server.address.toURL().openConnection()
-      connection.addRequestProperty("is-dd-server", "false")
-      def stream = connection.inputStream
-      def lines = stream.readLines()
-      stream.close()
-      assert connection.getResponseCode() == STATUS
-      assert lines == [RESPONSE]
-    }
-
-    expect:
-    assertTraces(1) {
-      trace(0, 1) {
-        span(0) {
-          serviceName renameService ? "localhost" : "unnamed-java-app"
-          operationName OPERATION_NAME
-          resourceName "GET /"
-          spanType DDSpanTypes.HTTP_CLIENT
-          parent()
-          errored false
-          tags {
-            "$Tags.COMPONENT.key" "http-url-connection"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.HTTP_STATUS.key" STATUS
-            "$Tags.PEER_HOSTNAME.key" "localhost"
-            "$Tags.PEER_PORT.key" server.address.port
-            defaultTags()
-          }
-        }
-      }
-    }
-
-    where:
-    renameService << [false, true]
-  }
-
-  def "rest template"() {
-    setup:
-    withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
-      runUnderTrace("someTrace") {
-        RestTemplate restTemplate = new RestTemplate()
-        String res = restTemplate.postForObject(server.address.toString(), "Hello", String)
-        assert res == "$RESPONSE"
-      }
-    }
-
-    expect:
-    assertTraces(2) {
-      server.distributedRequestTrace(it, 0, TEST_WRITER[1][1])
-      trace(1, 2) {
-        span(0) {
-          operationName "someTrace"
-          parent()
-          errored false
-          tags {
-            defaultTags()
-          }
-        }
-        span(1) {
-          serviceName renameService ? "localhost" : "unnamed-java-app"
-          operationName OPERATION_NAME
-          resourceName "POST /"
-          spanType DDSpanTypes.HTTP_CLIENT
-          childOf span(0)
-          errored false
-          tags {
-            "$Tags.COMPONENT.key" "http-url-connection"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_URL.key" "$server.address/"
             "$Tags.HTTP_METHOD.key" "POST"
             "$Tags.HTTP_STATUS.key" STATUS
             "$Tags.PEER_HOSTNAME.key" "localhost"

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

@@ -0,0 +1,36 @@
+import datadog.trace.agent.test.base.HttpClientTest
+import datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionDecorator
+import io.opentracing.util.GlobalTracer
+
+class HttpUrlConnectionUseCachesFalseTest extends HttpClientTest<HttpUrlConnectionDecorator> {
+
+  @Override
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
+    HttpURLConnection connection = uri.toURL().openConnection()
+    try {
+      connection.setRequestMethod(method)
+      headers.each { connection.setRequestProperty(it.key, it.value) }
+      connection.setRequestProperty("Connection", "close")
+      connection.useCaches = false
+      def parentSpan = GlobalTracer.get().scopeManager().active()
+      def stream = connection.inputStream
+      assert GlobalTracer.get().scopeManager().active() == parentSpan
+      stream.readLines()
+      stream.close()
+      callback?.call()
+      return connection.getResponseCode()
+    } finally {
+      connection.disconnect()
+    }
+  }
+
+  @Override
+  HttpUrlConnectionDecorator decorator() {
+    return HttpUrlConnectionDecorator.DECORATE
+  }
+
+  @Override
+  boolean testRedirects() {
+    false
+  }
+}

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

@@ -0,0 +1,39 @@
+import datadog.trace.agent.test.base.HttpClientTest
+import datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionDecorator
+import org.springframework.http.HttpEntity
+import org.springframework.http.HttpHeaders
+import org.springframework.http.HttpMethod
+import org.springframework.http.ResponseEntity
+import org.springframework.web.client.RestTemplate
+import spock.lang.Shared
+
+class SpringRestTemplateTest extends HttpClientTest<HttpUrlConnectionDecorator> {
+
+  @Shared
+  RestTemplate restTemplate = new RestTemplate()
+
+  @Override
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
+    def httpHeaders = new HttpHeaders()
+    headers.each { httpHeaders.put(it.key, [it.value]) }
+    def request = new HttpEntity<String>(httpHeaders);
+    ResponseEntity<String> response = restTemplate.exchange(uri, HttpMethod.resolve(method), request, String)
+    callback?.call()
+    return response.statusCode.value()
+  }
+
+  @Override
+  HttpUrlConnectionDecorator decorator() {
+    return HttpUrlConnectionDecorator.DECORATE
+  }
+
+  @Override
+  boolean testRedirects() {
+    false
+  }
+
+  @Override
+  boolean testConnectionFailure() {
+    false
+  }
+}

dd-java-agent/instrumentation/jax-rs-client/jax-rs-client.gradle

@@ -35,6 +35,7 @@ dependencies {
   compile project(':dd-java-agent:agent-tooling')
 
   testCompile project(':dd-java-agent:testing')
+  testCompile project(':dd-java-agent:instrumentation:java-concurrent')
 
   testCompile project(':dd-java-agent:instrumentation:jax-rs-client:connection-error-handling-jersey')
   testCompile project(':dd-java-agent:instrumentation:jax-rs-client:connection-error-handling-resteasy')

dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientAsyncTest.groovy

@@ -0,0 +1,72 @@
+import datadog.trace.agent.test.base.HttpClientTest
+import datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator
+import javax.ws.rs.client.AsyncInvoker
+import javax.ws.rs.client.Client
+import javax.ws.rs.client.ClientBuilder
+import javax.ws.rs.client.WebTarget
+import javax.ws.rs.core.MediaType
+import javax.ws.rs.core.Response
+import org.apache.cxf.jaxrs.client.spec.ClientBuilderImpl
+import org.glassfish.jersey.client.JerseyClientBuilder
+import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder
+
+abstract class JaxRsClientAsyncTest extends HttpClientTest<JaxRsClientDecorator> {
+
+  @Override
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
+    Client client = builder().build()
+    WebTarget service = client.target(uri)
+    def builder = service.request(MediaType.TEXT_PLAIN)
+    headers.each { builder.header(it.key, it.value) }
+    AsyncInvoker request = builder.async()
+    
+    Response response = request.method(method).get()
+    callback?.call()
+
+    return response.status
+  }
+
+  @Override
+  JaxRsClientDecorator decorator() {
+    return JaxRsClientDecorator.DECORATE
+  }
+
+  @Override
+  String expectedOperationName() {
+    return "jax-rs.client.call"
+  }
+
+  boolean testRedirects() {
+    false
+  }
+
+  abstract ClientBuilder builder()
+}
+
+class JerseyClientAsyncTest extends JaxRsClientAsyncTest {
+
+  @Override
+  ClientBuilder builder() {
+    return new JerseyClientBuilder()
+  }
+}
+
+class ResteasyClientAsyncTest extends JaxRsClientAsyncTest {
+
+  @Override
+  ClientBuilder builder() {
+    return new ResteasyClientBuilder()
+  }
+}
+
+class CxfClientAsyncTest extends JaxRsClientAsyncTest {
+
+  @Override
+  ClientBuilder builder() {
+    return new ClientBuilderImpl()
+  }
+
+  boolean testConnectionFailure() {
+    false
+  }
+}

dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientTest.groovy

@@ -1,131 +1,71 @@
-import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.base.HttpClientTest
-import io.opentracing.tag.Tags
+import datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator
-import org.apache.cxf.jaxrs.client.spec.ClientBuilderImpl
-import org.glassfish.jersey.client.JerseyClientBuilder
-import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder
-import spock.lang.AutoCleanup
-import spock.lang.Shared
-
-import javax.ws.rs.ProcessingException
-import javax.ws.rs.client.AsyncInvoker
 import javax.ws.rs.client.Client
+import javax.ws.rs.client.ClientBuilder
 import javax.ws.rs.client.Invocation
 import javax.ws.rs.client.WebTarget
 import javax.ws.rs.core.MediaType
 import javax.ws.rs.core.Response
-import java.util.concurrent.ExecutionException
+import org.apache.cxf.jaxrs.client.spec.ClientBuilderImpl
+import org.glassfish.jersey.client.JerseyClientBuilder
+import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder
 
-import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+abstract class JaxRsClientTest extends HttpClientTest<JaxRsClientDecorator> {
-import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
 
-class JaxRsClientTest extends AgentTestRunner {
+  @Override
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
 
-  @AutoCleanup
+    Client client = builder().build()
-  @Shared
+    WebTarget service = client.target(uri)
-  def server = httpServer {
+    Invocation.Builder request = service.request(MediaType.TEXT_PLAIN)
-    handlers {
+    headers.each { request.header(it.key, it.value) }
-      all {
+    Response response = request.method(method)
-        response.status(200).send("pong")
+    callback?.call()
-      }
+
-    }
+    return response.status
   }
 
-  def "#lib request creates spans and sends headers"() {
+  @Override
-    setup:
+  JaxRsClientDecorator decorator() {
-    Client client = builder.build()
+    return JaxRsClientDecorator.DECORATE
-    WebTarget service = client.target("$server.address/ping")
-    Response response
-    if (async) {
-      AsyncInvoker request = service.request(MediaType.TEXT_PLAIN).async()
-      response = request.get().get()
-    } else {
-      Invocation.Builder request = service.request(MediaType.TEXT_PLAIN)
-      response = request.get()
-    }
-
-    expect:
-    response.readEntity(String) == "pong"
-
-    assertTraces(1) {
-      trace(0, 1) {
-        span(0) {
-          serviceName "unnamed-java-app"
-          resourceName "GET /ping"
-          operationName "jax-rs.client.call"
-          spanType "http"
-          parent()
-          errored false
-          tags {
-            "$Tags.COMPONENT.key" "jax-rs.client"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.HTTP_STATUS.key" 200
-            "$Tags.HTTP_URL.key" "$server.address/ping"
-            "$Tags.PEER_HOSTNAME.key" "localhost"
-            "$Tags.PEER_PORT.key" server.address.port
-            defaultTags()
-          }
-        }
-      }
-    }
-
-    server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[0][0].traceId
-    server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[0][0].spanId
-
-    where:
-    builder                     | async | lib
-    new JerseyClientBuilder()   | false | "jersey"
-    new ClientBuilderImpl()     | false | "cxf"
-    new ResteasyClientBuilder() | false | "resteasy"
-    new JerseyClientBuilder()   | true  | "jersey async"
-    new ClientBuilderImpl()     | true  | "cxf async"
-    new ResteasyClientBuilder() | true  | "resteasy async"
   }
 
-  def "#lib connection failure creates errored span"() {
+  @Override
-    when:
+  String expectedOperationName() {
-    Client client = builder.build()
+    return "jax-rs.client.call"
-    WebTarget service = client.target("http://localhost:$UNUSABLE_PORT/ping")
+  }
-    if (async) {
-      AsyncInvoker request = service.request(MediaType.TEXT_PLAIN).async()
-      request.get().get()
-    } else {
-      Invocation.Builder request = service.request(MediaType.TEXT_PLAIN)
-      request.get()
-    }
 
-    then:
+  boolean testRedirects() {
-    thrown async ? ExecutionException : ProcessingException
+    false
+  }
 
-    assertTraces(1) {
+  abstract ClientBuilder builder()
-      trace(0, 1) {
+}
-        span(0) {
-          serviceName "unnamed-java-app"
-          resourceName "GET /ping"
-          operationName "jax-rs.client.call"
-          spanType "http"
-          parent()
-          errored true
-          tags {
-            "$Tags.COMPONENT.key" "jax-rs.client"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.HTTP_URL.key" "http://localhost:$UNUSABLE_PORT/ping"
-            "$Tags.PEER_HOSTNAME.key" "localhost"
-            "$Tags.PEER_PORT.key" UNUSABLE_PORT
-            errorTags ProcessingException, String
-            defaultTags()
-          }
-        }
-      }
-    }
 
-    where:
+class JerseyClientTest extends JaxRsClientTest {
-    builder                     | async | lib
+
-    new JerseyClientBuilder()   | false | "jersey"
+  @Override
-    new ResteasyClientBuilder() | false | "resteasy"
+  ClientBuilder builder() {
-    new JerseyClientBuilder()   | true  | "jersey async"
+    return new JerseyClientBuilder()
-    new ResteasyClientBuilder() | true  | "resteasy async"
+  }
-    // Unfortunately there's not a good way to instrument this for CXF.
+}
+
+class ResteasyClientTest extends JaxRsClientTest {
+
+  @Override
+  ClientBuilder builder() {
+    return new ResteasyClientBuilder()
+  }
+}
+
+class CxfClientTest extends JaxRsClientTest {
+
+  @Override
+  ClientBuilder builder() {
+    return new ClientBuilderImpl()
+  }
+
+  boolean testConnectionFailure() {
+    false
   }
 }

dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/NettyHttpClientDecorator.java

@@ -39,13 +39,32 @@ public class NettyHttpClientDecorator extends HttpClientDecorator<HttpRequest, H
   }
 
   @Override
-  protected String hostname(final HttpRequest httpRequest) {
+  protected String hostname(final HttpRequest request) {
-    return null;
+    try {
+      final URI uri = new URI(request.getUri());
+      if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) {
+        return request.headers().get(HOST).split(":")[0];
+      } else {
+        return uri.getHost();
+      }
+    } catch (final Exception e) {
+      return null;
+    }
   }
 
   @Override
-  protected Integer port(final HttpRequest httpRequest) {
+  protected Integer port(final HttpRequest request) {
-    return null;
+    try {
+      final URI uri = new URI(request.getUri());
+      if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) {
+        final String[] hostPort = request.headers().get(HOST).split(":");
+        return hostPort.length == 2 ? Integer.parseInt(hostPort[1]) : null;
+      } else {
+        return uri.getPort();
+      }
+    } catch (final Exception e) {
+      return null;
+    }
   }
 
   @Override

dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ClientTest.groovy

@@ -1,102 +1,72 @@
-import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.base.HttpClientTest
-import datadog.trace.agent.test.utils.PortUtils
+import datadog.trace.instrumentation.netty40.client.NettyHttpClientDecorator
-import datadog.trace.api.DDSpanTypes
 import io.opentracing.tag.Tags
 import org.asynchttpclient.AsyncHttpClient
 import org.asynchttpclient.DefaultAsyncHttpClientConfig
-import spock.lang.AutoCleanup
 import spock.lang.Shared
 
 import java.util.concurrent.ExecutionException
 import java.util.concurrent.TimeUnit
 
-import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static org.asynchttpclient.Dsl.asyncHttpClient
 
-class Netty40ClientTest extends AgentTestRunner {
+class Netty40ClientTest extends HttpClientTest<NettyHttpClientDecorator> {
 
-  @AutoCleanup
-  @Shared
-  def server = httpServer {
-    handlers {
-      all {
-        response.send("Hello World")
-      }
-    }
-  }
   @Shared
   def clientConfig = DefaultAsyncHttpClientConfig.Builder.newInstance().setRequestTimeout(TimeUnit.SECONDS.toMillis(10).toInteger())
   @Shared
   AsyncHttpClient asyncHttpClient = asyncHttpClient(clientConfig)
 
-  def "test server request/response"() {
+  @Override
-    setup:
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    def responseFuture = runUnderTrace("parent") {
+    def methodName = "prepare" + method.toLowerCase().capitalize()
-      asyncHttpClient.prepareGet("$server.address").execute()
+    def requestBuilder = asyncHttpClient."$methodName"(uri.toString())
-    }
+    headers.each { requestBuilder.setHeader(it.key, it.value) }
-    def response = responseFuture.get()
+    def response = requestBuilder.execute().get()
-
+    callback?.call()
-    expect:
+    return response.statusCode
-    response.statusCode == 200
-    response.responseBody == "Hello World"
-
-    and:
-    assertTraces(1) {
-      trace(0, 2) {
-        span(0) {
-          serviceName "unnamed-java-app"
-          operationName "netty.client.request"
-          resourceName "GET /"
-          spanType DDSpanTypes.HTTP_CLIENT
-          childOf span(1)
-          errored false
-          tags {
-            "$Tags.COMPONENT.key" "netty-client"
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.HTTP_STATUS.key" 200
-            "$Tags.HTTP_URL.key" "$server.address/"
-            "$Tags.PEER_HOSTNAME.key" "localhost"
-            "$Tags.PEER_HOST_IPV4.key" "127.0.0.1"
-            "$Tags.PEER_PORT.key" server.address.port
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            defaultTags()
-          }
-        }
-        span(1) {
-          operationName "parent"
-          parent()
-        }
-      }
-    }
-
-    and:
-    server.lastRequest.headers.get("x-datadog-trace-id") == "${TEST_WRITER.get(0).get(0).traceId}"
-    server.lastRequest.headers.get("x-datadog-parent-id") == "${TEST_WRITER.get(0).get(0).spanId}"
   }
 
-  def "test connection failure"() {
+  @Override
-    setup:
+  NettyHttpClientDecorator decorator() {
-    def invalidPort = PortUtils.randomOpenPort()
+    return NettyHttpClientDecorator.DECORATE
+  }
 
-    def responseFuture = runUnderTrace("parent") {
+  @Override
-      asyncHttpClient.prepareGet("http://localhost:$invalidPort/").execute()
+  String expectedOperationName() {
-    }
+    return "netty.client.request"
+  }
+
+  @Override
+  boolean testRedirects() {
+    false
+  }
+
+  @Override
+  boolean testConnectionFailure() {
+    false
+  }
+
+  def "connection error (unopened port)"() {
+    given:
+    def uri = new URI("http://localhost:$UNUSABLE_PORT/")
 
     when:
-    responseFuture.get()
+    runUnderTrace("parent") {
+      doRequest(method, uri)
+    }
 
     then:
-    def throwable = thrown(ExecutionException)
+    def ex = thrown(Exception)
-    throwable.cause instanceof ConnectException
+    def thrownException = ex instanceof ExecutionException ? ex.cause : ex
 
     and:
     assertTraces(1) {
       trace(0, 2) {
-        span(0) {
+        parentSpan(it, 0, thrownException)
-          operationName "parent"
+
-          parent()
-        }
         span(1) {
           operationName "netty.connect"
           resourceName "netty.connect"
@@ -110,11 +80,14 @@ class Netty40ClientTest extends AgentTestRunner {
             } catch (ClassNotFoundException e) {
               // Older versions use 'java.net.ConnectException' and do not have 'io.netty.channel.AbstractChannel$AnnotatedConnectException'
             }
-            errorTags errorClass, "Connection refused: localhost/127.0.0.1:$invalidPort"
+            errorTags errorClass, "Connection refused: localhost/127.0.0.1:$UNUSABLE_PORT"
             defaultTags()
           }
         }
       }
     }
+
+    where:
+    method = "GET"
   }
 }

dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/NettyHttpClientDecorator.java

@@ -39,13 +39,32 @@ public class NettyHttpClientDecorator extends HttpClientDecorator<HttpRequest, H
   }
 
   @Override
-  protected String hostname(final HttpRequest httpRequest) {
+  protected String hostname(final HttpRequest request) {
-    return null;
+    try {
+      final URI uri = new URI(request.uri());
+      if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) {
+        return request.headers().get(HOST).split(":")[0];
+      } else {
+        return uri.getHost();
+      }
+    } catch (final Exception e) {
+      return null;
+    }
   }
 
   @Override
-  protected Integer port(final HttpRequest httpRequest) {
+  protected Integer port(final HttpRequest request) {
-    return null;
+    try {
+      final URI uri = new URI(request.uri());
+      if ((uri.getHost() == null || uri.getHost().equals("")) && request.headers().contains(HOST)) {
+        final String[] hostPort = request.headers().get(HOST).split(":");
+        return hostPort.length == 2 ? Integer.parseInt(hostPort[1]) : null;
+      } else {
+        return uri.getPort();
+      }
+    } catch (final Exception e) {
+      return null;
+    }
   }
 
   @Override

dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy

@@ -1,103 +1,74 @@
-import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.base.HttpClientTest
-import datadog.trace.agent.test.utils.PortUtils
+import datadog.trace.instrumentation.netty41.client.NettyHttpClientDecorator
-import datadog.trace.api.DDSpanTypes
 import io.netty.channel.AbstractChannel
 import io.opentracing.tag.Tags
 import org.asynchttpclient.AsyncHttpClient
 import org.asynchttpclient.DefaultAsyncHttpClientConfig
-import spock.lang.AutoCleanup
 import spock.lang.Shared
 
 import java.util.concurrent.ExecutionException
 import java.util.concurrent.TimeUnit
 
-import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static org.asynchttpclient.Dsl.asyncHttpClient
 
-class Netty41ClientTest extends AgentTestRunner {
+class Netty41ClientTest extends HttpClientTest<NettyHttpClientDecorator> {
 
-  @AutoCleanup
-  @Shared
-  def server = httpServer {
-    handlers {
-      all {
-        response.send("Hello World")
-      }
-    }
-  }
   @Shared
   def clientConfig = DefaultAsyncHttpClientConfig.Builder.newInstance().setRequestTimeout(TimeUnit.SECONDS.toMillis(10).toInteger())
   @Shared
   AsyncHttpClient asyncHttpClient = asyncHttpClient(clientConfig)
 
-  def "test server request/response"() {
+  @Override
-    setup:
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    def responseFuture = runUnderTrace("parent") {
+    def methodName = "prepare" + method.toLowerCase().capitalize()
-      asyncHttpClient.prepareGet("$server.address").execute()
+    def requestBuilder = asyncHttpClient."$methodName"(uri.toString())
-    }
+    headers.each { requestBuilder.setHeader(it.key, it.value) }
-    def response = responseFuture.get()
+    def response = requestBuilder.execute().get()
-
+    callback?.call()
-    expect:
+    return response.statusCode
-    response.statusCode == 200
-    response.responseBody == "Hello World"
-
-    and:
-    assertTraces(1) {
-      trace(0, 2) {
-        span(0) {
-          serviceName "unnamed-java-app"
-          operationName "netty.client.request"
-          resourceName "GET /"
-          spanType DDSpanTypes.HTTP_CLIENT
-          childOf span(1)
-          errored false
-          tags {
-            "$Tags.COMPONENT.key" "netty-client"
-            "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.HTTP_STATUS.key" 200
-            "$Tags.HTTP_URL.key" "$server.address/"
-            "$Tags.PEER_HOSTNAME.key" "localhost"
-            "$Tags.PEER_HOST_IPV4.key" "127.0.0.1"
-            "$Tags.PEER_PORT.key" server.address.port
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            defaultTags()
-          }
-        }
-        span(1) {
-          operationName "parent"
-          parent()
-        }
-      }
-    }
-
-    and:
-    server.lastRequest.headers.get("x-datadog-trace-id") == "${TEST_WRITER.get(0).get(0).traceId}"
-    server.lastRequest.headers.get("x-datadog-parent-id") == "${TEST_WRITER.get(0).get(0).spanId}"
   }
 
-  def "test connection failure"() {
+  @Override
-    setup:
+  NettyHttpClientDecorator decorator() {
-    def invalidPort = PortUtils.randomOpenPort()
+    return NettyHttpClientDecorator.DECORATE
+  }
 
-    def responseFuture = runUnderTrace("parent") {
+  @Override
-      asyncHttpClient.prepareGet("http://localhost:$invalidPort/").execute()
+  String expectedOperationName() {
-    }
+    return "netty.client.request"
+  }
+
+  @Override
+  boolean testRedirects() {
+    false
+  }
+
+  @Override
+  boolean testConnectionFailure() {
+    false
+  }
+
+  def "connection error (unopened port)"() {
+    given:
+    def uri = new URI("http://localhost:$UNUSABLE_PORT/")
 
     when:
-    responseFuture.get()
+    runUnderTrace("parent") {
+      doRequest(method, uri)
+    }
 
     then:
-    def throwable = thrown(ExecutionException)
+    def ex = thrown(Exception)
-    throwable.cause instanceof ConnectException
+    ex.cause instanceof ConnectException
+    def thrownException = ex instanceof ExecutionException ? ex.cause : ex
 
     and:
     assertTraces(1) {
       trace(0, 2) {
-        span(0) {
+        parentSpan(it, 0, thrownException)
-          operationName "parent"
+
-          parent()
-        }
         span(1) {
           operationName "netty.connect"
           resourceName "netty.connect"
@@ -105,11 +76,14 @@ class Netty41ClientTest extends AgentTestRunner {
           errored true
           tags {
             "$Tags.COMPONENT.key" "netty"
-            errorTags AbstractChannel.AnnotatedConnectException, "Connection refused: localhost/127.0.0.1:$invalidPort"
+            errorTags AbstractChannel.AnnotatedConnectException, "Connection refused: localhost/127.0.0.1:$UNUSABLE_PORT"
             defaultTags()
           }
         }
       }
     }
+
+    where:
+    method = "GET"
   }
 }

dd-java-agent/instrumentation/okhttp-3/okhttp-3.gradle

@@ -30,6 +30,7 @@ dependencies {
   testCompile(project(':dd-java-agent:testing')) {
     exclude module: 'okhttp'
   }
+  testCompile project(':dd-java-agent:instrumentation:java-concurrent')
   testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'
 
   // 4.x.x-alpha has been released and it looks like there are lots of incompatible changes

dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3AsyncTest.groovy

@@ -0,0 +1,48 @@
+import okhttp3.Call
+import okhttp3.Callback
+import okhttp3.Headers
+import okhttp3.MediaType
+import okhttp3.Request
+import okhttp3.RequestBody
+import okhttp3.Response
+import okhttp3.internal.http.HttpMethod
+
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.atomic.AtomicReference
+
+import static java.util.concurrent.TimeUnit.SECONDS
+
+class OkHttp3AsyncTest extends OkHttp3Test {
+
+  @Override
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
+    def body = HttpMethod.requiresRequestBody(method) ? RequestBody.create(MediaType.parse("text/plain"), "") : null
+    def request = new Request.Builder()
+      .url(uri.toURL())
+      .method(method, body)
+      .headers(Headers.of(headers))
+      .build()
+
+    AtomicReference<Response> responseRef = new AtomicReference()
+    AtomicReference<Exception> exRef = new AtomicReference()
+    def latch = new CountDownLatch(1)
+
+    client.newCall(request).enqueue(new Callback() {
+      void onResponse(Call call, Response response) {
+        responseRef.set(response)
+        callback?.call()
+        latch.countDown()
+      }
+
+      void onFailure(Call call, IOException e) {
+        exRef.set(e)
+        latch.countDown()
+      }
+    })
+    latch.await(10, SECONDS)
+    if (exRef.get() != null) {
+      throw exRef.get()
+    }
+    return responseRef.get().code()
+  }
+}

dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy

@@ -1,160 +1,124 @@
-import datadog.trace.agent.test.AgentTestRunner
+import datadog.opentracing.DDSpan
-import datadog.trace.agent.test.utils.PortUtils
+import datadog.trace.agent.test.asserts.TraceAssert
-import datadog.trace.api.Config
+import datadog.trace.agent.test.base.HttpClientTest
 import datadog.trace.api.DDSpanTypes
+import datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator
 import io.opentracing.tag.Tags
-import okhttp3.Call
+import okhttp3.Headers
-import okhttp3.Callback
+import okhttp3.MediaType
 import okhttp3.OkHttpClient
 import okhttp3.Request
-import okhttp3.Response
+import okhttp3.RequestBody
-import spock.lang.AutoCleanup
+import okhttp3.internal.http.HttpMethod
-import spock.lang.Shared
 
-import java.util.concurrent.CountDownLatch
+import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.NETWORK_DECORATE
-import java.util.concurrent.atomic.AtomicReference
 
-import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+class OkHttp3Test extends HttpClientTest<OkHttpClientDecorator> {
-import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
-import static java.util.concurrent.TimeUnit.SECONDS
-
-class OkHttp3Test extends AgentTestRunner {
-  @AutoCleanup
-  @Shared
-  def server = httpServer {
-    handlers {
-      all {
-        response.status(200).send("pong")
-      }
-    }
-  }
 
   def client = new OkHttpClient()
 
-  def "sending a request creates spans and sends headers"() {
+  @Override
-    setup:
+  int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
-    def requestBulder = new Request.Builder()
+    def body = HttpMethod.requiresRequestBody(method) ? RequestBody.create(MediaType.parse("text/plain"), "") : null
-      .url("http://localhost:$server.address.port/ping")
+    def request = new Request.Builder()
-    if (agentRequest) {
+      .url(uri.toURL())
-      requestBulder.addHeader("Datadog-Meta-Lang", "java")
+      .method(method, body)
-    }
+      .headers(Headers.of(headers)).build()
-    def request = requestBulder.build()
+    def response = client.newCall(request).execute()
-
+    callback?.call()
-    Response response = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
+    return response.code()
-      if (!async) {
-        return client.newCall(request).execute()
-      }
-
-      AtomicReference<Response> responseRef = new AtomicReference()
-      def latch = new CountDownLatch(1)
-
-      client.newCall(request).enqueue(new Callback() {
-        void onResponse(Call call, Response response) {
-          responseRef.set(response)
-          latch.countDown()
-        }
-
-        void onFailure(Call call, IOException e) {
-          latch.countDown()
-        }
-      })
-      latch.await(10, SECONDS)
-      return responseRef.get()
-    }
-
-    expect:
-    response.body.string() == "pong"
-    if (agentRequest) {
-      assert TEST_WRITER.size() == 0
-      assert server.lastRequest.headers.get("x-datadog-trace-id") == null
-      assert server.lastRequest.headers.get("x-datadog-parent-id") == null
-    } else {
-      assertTraces(1) {
-        trace(0, 2) {
-          span(0) {
-            operationName "okhttp.http"
-            serviceName "okhttp"
-            resourceName "okhttp.http"
-            spanType DDSpanTypes.HTTP_CLIENT
-            errored false
-            parent()
-            tags {
-              "$Tags.COMPONENT.key" "okhttp"
-              "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-              defaultTags()
-            }
-          }
-          span(1) {
-            operationName "okhttp.http"
-            serviceName renameService ? "localhost" : "okhttp"
-            resourceName "GET /ping"
-            spanType DDSpanTypes.HTTP_CLIENT
-            errored false
-            childOf(span(0))
-            tags {
-              defaultTags()
-              "$Tags.COMPONENT.key" "okhttp-network"
-              "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-              "$Tags.HTTP_METHOD.key" "GET"
-              "$Tags.HTTP_STATUS.key" 200
-              "$Tags.HTTP_URL.key" "http://localhost:$server.address.port/ping"
-              "$Tags.PEER_HOSTNAME.key" "localhost"
-              "$Tags.PEER_PORT.key" server.address.port
-              "$Tags.PEER_HOST_IPV4.key" "127.0.0.1"
-            }
-          }
-        }
-      }
-
-      assert server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[0][1].traceId
-      assert server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[0][1].spanId
-    }
-
-    where:
-    renameService | async | agentRequest
-    false         | false | false
-    true          | false | false
-    false         | true  | false
-    true          | true  | false
-    false         | false | true
-    false         | false | true
   }
 
-  def "sending an invalid request creates an error span"() {
+  @Override
-    setup:
+  OkHttpClientDecorator decorator() {
-    def unusablePort = PortUtils.UNUSABLE_PORT
+    return OkHttpClientDecorator.DECORATE
-    def request = new Request.Builder()
+  }
-      .url("http://localhost:$unusablePort/ping")
-      .build()
 
-    when:
+  @Override
-    withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
+  // parent span must be cast otherwise it breaks debugging classloading (junit loads it early)
-      client.newCall(request).execute()
+  void clientSpan(TraceAssert trace, int index, Object parentSpan, String method = "GET", boolean renameService = false, boolean tagQueryString = false, URI uri = server.address.resolve("/success"), Integer status = 200, Throwable exception = null) {
+    trace.span(index) {
+      if (parentSpan == null) {
+        parent()
+      } else {
+        childOf((DDSpan) parentSpan)
+      }
+      serviceName decorator().service()
+      operationName "okhttp.http"
+      resourceName "okhttp.http"
+//      resourceName "GET $uri.path"
+      spanType DDSpanTypes.HTTP_CLIENT
+      errored exception != null
+      tags {
+        defaultTags()
+        if (exception) {
+          errorTags(exception.class, exception.message)
+        }
+        "$Tags.COMPONENT.key" decorator.component()
+        "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+      }
     }
+    if (!exception) {
+      trace.span(index + 1) {
+        serviceName renameService ? "localhost" : decorator().service()
+        operationName "okhttp.http"
+        resourceName "$method $uri.path"
+        childOf trace.span(index)
+        spanType DDSpanTypes.HTTP_CLIENT
+        errored exception != null
+        tags {
+          defaultTags()
+          if (exception) {
+            errorTags(exception.class, exception.message)
+          }
+          "$Tags.COMPONENT.key" NETWORK_DECORATE.component()
+          if (status) {
+            "$Tags.HTTP_STATUS.key" status
+          }
+          "$Tags.HTTP_URL.key" "${uri.resolve(uri.path)}"
+          if (tagQueryString) {
+            "http.query.string" uri.query
+            "http.fragment.string" uri.fragment
+          }
+          "$Tags.PEER_HOSTNAME.key" "localhost"
+          "$Tags.PEER_PORT.key" server.address.port
+          "$Tags.PEER_HOST_IPV4.key" "127.0.0.1"
+          "$Tags.HTTP_METHOD.key" method
+          "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+        }
+      }
+    }
+  }
+
+  @Override
+  int size(int size) {
+    return size + 1
+  }
+
+  boolean testRedirects() {
+    false
+  }
+
+  def "request to agent not traced"() {
+    when:
+    def status = doRequest(method, url, ["Datadog-Meta-Lang": "java"])
 
     then:
-    thrown(ConnectException)
+    status == 200
-
     assertTraces(1) {
-      trace(0, 1) {
+      server.distributedRequestTrace(it, 0)
-        span(0) {
-          operationName "okhttp.http"
-          serviceName "okhttp"
-          resourceName "okhttp.http"
-          spanType DDSpanTypes.HTTP_CLIENT
-          errored true
-          parent()
-          tags {
-            "$Tags.COMPONENT.key" "okhttp"
-            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-            errorTags(ConnectException, ~/Failed to connect to localhost\/[\d:\.]+:61/)
-            defaultTags()
-          }
-        }
-      }
     }
 
     where:
-    renameService << [false, true]
+    path                                | tagQueryString
+    "/success"                          | false
+    "/success"                          | true
+    "/success?with=params"              | false
+    "/success?with=params"              | true
+    "/success#with+fragment"            | true
+    "/success?with=params#and=fragment" | true
+
+    method = "GET"
+    url = server.address.resolve(path)
   }
 }

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy

@@ -9,14 +9,18 @@ import datadog.trace.api.DDSpanTypes
 import io.opentracing.tag.Tags
 import spock.lang.AutoCleanup
 import spock.lang.Shared
+import spock.lang.Unroll
 
 import java.util.concurrent.ExecutionException
 
 import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
+import static org.junit.Assume.assumeTrue
 
 abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRunner {
+  protected static final BODY_METHODS = ["POST", "PUT"]
 
   @AutoCleanup
   @Shared
@@ -63,24 +67,34 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     return null
   }
 
-  def "basic #method request"() {
+  @Unroll
+  def "basic #method request #url"() {
     when:
     def status = doRequest(method, server.address.resolve(url))
 
     then:
     status == 200
     assertTraces(2) {
-      server.distributedRequestTrace(it, 0, trace(1).get(0))
+      server.distributedRequestTrace(it, 0, trace(1).last())
-      trace(1, 1) {
+      trace(1, size(1)) {
-        clientSpan(it, 0, null, false)
+        clientSpan(it, 0, null, method, false, tagQueryString, url)
       }
     }
 
     where:
+    path                                | tagQueryString
+    "/success"                          | false
+    "/success"                          | true
+    "/success?with=params"              | false
+    "/success?with=params"              | true
+    "/success#with+fragment"            | true
+    "/success?with=params#and=fragment" | true
+
     method = "GET"
-    url << ["/success", "/success?with=params"]
+    url = server.address.resolve(path)
   }
 
+  @Unroll
   def "basic #method request with parent"() {
     when:
     def status = runUnderTrace("parent") {
@@ -90,17 +104,18 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     then:
     status == 200
     assertTraces(2) {
-      server.distributedRequestTrace(it, 0, trace(1).get(1))
+      server.distributedRequestTrace(it, 0, trace(1).last())
-      trace(1, 2) {
+      trace(1, size(2)) {
         parentSpan(it, 0)
-        clientSpan(it, 1, span(0), false)
+        clientSpan(it, 1, span(0), method, false)
       }
     }
 
     where:
-    method = "GET"
+    method << BODY_METHODS
   }
 
+  @Unroll
   def "basic #method request with split-by-domain"() {
     when:
     def status = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true") {
@@ -110,14 +125,14 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     then:
     status == 200
     assertTraces(2) {
-      server.distributedRequestTrace(it, 0, trace(1).get(0))
+      server.distributedRequestTrace(it, 0, trace(1).last())
-      trace(1, 1) {
+      trace(1, size(1)) {
-        clientSpan(it, 0, null, true)
+        clientSpan(it, 0, null, method, true)
       }
     }
 
     where:
-    method = "GET"
+    method = "HEAD"
   }
 
   def "trace request without propagation"() {
@@ -132,9 +147,9 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     status == 200
     // only one trace (client).
     assertTraces(1) {
-      trace(0, 2) {
+      trace(0, size(2)) {
         parentSpan(it, 0)
-        clientSpan(it, 1, span(0), renameService)
+        clientSpan(it, 1, span(0), method, renameService)
       }
     }
 
@@ -155,13 +170,13 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     status == 200
     // only one trace (client).
     assertTraces(1) {
-      trace(0, 3) {
+      trace(0, size(3)) {
         parentSpan(it, 0)
         span(1) {
           operationName "child"
           childOf span(0)
         }
-        clientSpan(it, 2, span(0), false)
+        clientSpan(it, 2, span(0), method, false)
       }
     }
 
@@ -182,8 +197,8 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     status == 200
     // only one trace (client).
     assertTraces(2) {
-      trace(0, 1) {
+      trace(0, size(1)) {
-        clientSpan(it, 0, null, false)
+        clientSpan(it, 0, null, method, false)
       }
       trace(1, 1) {
         span(0) {
@@ -197,8 +212,10 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     method = "GET"
   }
 
+  @Unroll
   def "basic #method request with 1 redirect"() {
-    setup:
+    given:
+    assumeTrue(testRedirects())
     def uri = server.address.resolve("/redirect")
 
     when:
@@ -207,10 +224,10 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     then:
     status == 200
     assertTraces(3) {
-      server.distributedRequestTrace(it, 0, trace(2).get(0))
+      server.distributedRequestTrace(it, 0, trace(2).last())
-      server.distributedRequestTrace(it, 1, trace(2).get(0))
+      server.distributedRequestTrace(it, 1, trace(2).last())
-      trace(2, 1) {
+      trace(2, size(1)) {
-        clientSpan(it, 0, null, false, uri)
+        clientSpan(it, 0, null, method, false, false, uri)
       }
     }
 
@@ -218,8 +235,10 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     method = "GET"
   }
 
+  @Unroll
   def "basic #method request with 2 redirects"() {
-    setup:
+    given:
+    assumeTrue(testRedirects())
     def uri = server.address.resolve("/another-redirect")
 
     when:
@@ -228,11 +247,11 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     then:
     status == 200
     assertTraces(4) {
-      server.distributedRequestTrace(it, 0, trace(3).get(0))
+      server.distributedRequestTrace(it, 0, trace(3).last())
-      server.distributedRequestTrace(it, 1, trace(3).get(0))
+      server.distributedRequestTrace(it, 1, trace(3).last())
-      server.distributedRequestTrace(it, 2, trace(3).get(0))
+      server.distributedRequestTrace(it, 2, trace(3).last())
-      trace(3, 1) {
+      trace(3, size(1)) {
-        clientSpan(it, 0, null, false, uri)
+        clientSpan(it, 0, null, method, false, false, uri)
       }
     }
 
@@ -240,8 +259,10 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
     method = "GET"
   }
 
+  @Unroll
   def "basic #method request with circular redirects"() {
-    setup:
+    given:
+    assumeTrue(testRedirects())
     def uri = server.address.resolve("/circular-redirect")
 
     when:
@@ -253,10 +274,36 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
 
     and:
     assertTraces(3) {
-      server.distributedRequestTrace(it, 0, trace(2).get(0))
+      server.distributedRequestTrace(it, 0, trace(2).last())
-      server.distributedRequestTrace(it, 1, trace(2).get(0))
+      server.distributedRequestTrace(it, 1, trace(2).last())
-      trace(2, 1) {
+      trace(2, size(1)) {
-        clientSpan(it, 0, null, false, uri, statusOnRedirectError(), thrownException)
+        clientSpan(it, 0, null, method, false, false, uri, statusOnRedirectError(), thrownException)
+      }
+    }
+
+    where:
+    method = "GET"
+  }
+
+  def "connection error (unopened port)"() {
+    given:
+    assumeTrue(testConnectionFailure())
+    def uri = new URI("http://localhost:$UNUSABLE_PORT/")
+
+    when:
+    runUnderTrace("parent") {
+      doRequest(method, uri)
+    }
+
+    then:
+    def ex = thrown(Exception)
+    def thrownException = ex instanceof ExecutionException ? ex.cause : ex
+
+    and:
+    assertTraces(1) {
+      trace(0, 2) {
+        parentSpan(it, 0, thrownException)
+        clientSpan(it, 1, span(0), method, false, false, uri, null, thrownException)
       }
     }
 
@@ -274,14 +321,14 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
       tags {
         defaultTags()
         if (exception) {
-          errorTags(exception.class)
+          errorTags(exception.class, exception.message)
         }
       }
     }
   }
 
   // parent span must be cast otherwise it breaks debugging classloading (junit loads it early)
-  void clientSpan(TraceAssert trace, int index, Object parentSpan, boolean renameService, URI uri = server.address.resolve("/success"), Integer status = 200, Throwable exception = null) {
+  void clientSpan(TraceAssert trace, int index, Object parentSpan, String method = "GET", boolean renameService = false, boolean tagQueryString = false, URI uri = server.address.resolve("/success"), Integer status = 200, Throwable exception = null) {
     trace.span(index) {
       if (parentSpan == null) {
         parent()
@@ -289,8 +336,8 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
         childOf((DDSpan) parentSpan)
       }
       serviceName renameService ? "localhost" : "unnamed-java-app"
-      operationName "http.request"
+      operationName expectedOperationName()
-      resourceName "GET $uri.path"
+      resourceName "$method $uri.path"
       spanType DDSpanTypes.HTTP_CLIENT
       errored exception != null
       tags {
@@ -302,12 +349,29 @@ abstract class HttpClientTest<T extends HttpClientDecorator> extends AgentTestRu
         if (status) {
           "$Tags.HTTP_STATUS.key" status
         }
-        "$Tags.HTTP_URL.key" "$uri"
+        "$Tags.HTTP_URL.key" "${uri.resolve(uri.path)}"
         "$Tags.PEER_HOSTNAME.key" "localhost"
-        "$Tags.PEER_PORT.key" server.address.port
+        "$Tags.PEER_PORT.key" uri.port
-        "$Tags.HTTP_METHOD.key" "GET"
+        "$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional
+        "$Tags.HTTP_METHOD.key" method
         "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
       }
     }
   }
+
+  String expectedOperationName() {
+    return "http.request"
+  }
+
+  int size(int size) {
+    size
+  }
+
+  boolean testRedirects() {
+    true
+  }
+
+  boolean testConnectionFailure() {
+    true
+  }
 }

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy

@@ -1,10 +1,9 @@
 package datadog.trace.agent.test.utils
 
+import datadog.trace.agent.decorator.BaseDecorator
 import datadog.trace.api.Config
 import datadog.trace.context.TraceScope
 import io.opentracing.Scope
-import io.opentracing.Span
-import io.opentracing.tag.Tags
 import io.opentracing.util.GlobalTracer
 import lombok.SneakyThrows
 
@@ -12,24 +11,35 @@ import java.lang.reflect.Field
 import java.lang.reflect.Modifier
 import java.util.concurrent.Callable
 
-import static io.opentracing.log.Fields.ERROR_OBJECT
-
 class TraceUtils {
+  private static final BaseDecorator DECORATOR = new BaseDecorator() {
+    protected String[] instrumentationNames() {
+      return new String[0]
+    }
+
+    protected String spanType() {
+      return null
+    }
+
+    protected String component() {
+//      return "runUnderTrace"
+      return null
+    }
+  }
 
   @SneakyThrows
   static <T extends Object> Object runUnderTrace(final String rootOperationName, final Callable<T> r) {
     final Scope scope = GlobalTracer.get().buildSpan(rootOperationName).startActive(true)
+    DECORATOR.afterStart(scope)
     ((TraceScope) scope).setAsyncPropagation(true)
 
     try {
       return r.call()
     } catch (final Exception e) {
-      final Span span = scope.span()
+      DECORATOR.onError(scope, e)
-      Tags.ERROR.set(span, true)
-      span.log(Collections.singletonMap(ERROR_OBJECT, e))
-
       throw e
     } finally {
+      DECORATOR.beforeFinish(scope)
       scope.close()
     }
   }