From e260b1d044cb3f467f84b4a9ef1638455ba2ec4d Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 28 May 2019 18:24:43 -0700 Subject: [PATCH] Make all http client tests extend HttpClientTest Add flexibility to handle inconsistencies between client integrations. --- .../trace/agent/decorator/BaseDecorator.java | 8 +- .../AkkaHttpClientInstrumentationTest.groovy | 257 ++---------------- ...kaHttpClientPoolInstrumentationTest.groovy | 59 ++++ .../ApacheHttpAsyncClientCallbackTest.groovy | 5 +- ...acheHttpAsyncClientNullCallbackTest.groovy | 5 +- .../groovy/ApacheHttpAsyncClientTest.groovy | 6 +- .../src/test/groovy/HttpUriRequest.groovy | 16 ++ ...ApacheHttpClientResponseHandlerTest.groovy | 4 +- .../test/groovy/ApacheHttpClientTest.groovy | 9 +- .../src/test/groovy/HttpUriRequest.groovy | 16 ++ ...tpUrlConnectionResponseCodeOnlyTest.groovy | 29 ++ .../test/groovy/HttpUrlConnectionTest.groovy | 257 ++++-------------- ...HttpUrlConnectionUseCachesFalseTest.groovy | 36 +++ .../test/groovy/SpringRestTemplateTest.groovy | 39 +++ .../jax-rs-client/jax-rs-client.gradle | 1 + .../test/groovy/JaxRsClientAsyncTest.groovy | 72 +++++ .../src/test/groovy/JaxRsClientTest.groovy | 168 ++++-------- .../client/NettyHttpClientDecorator.java | 27 +- .../src/test/groovy/Netty40ClientTest.groovy | 117 +++----- .../client/NettyHttpClientDecorator.java | 27 +- .../src/test/groovy/Netty41ClientTest.groovy | 118 ++++---- .../instrumentation/okhttp-3/okhttp-3.gradle | 1 + .../src/test/groovy/OkHttp3AsyncTest.groovy | 48 ++++ .../src/test/groovy/OkHttp3Test.groovy | 242 +++++++---------- .../agent/test/base/HttpClientTest.groovy | 148 +++++++--- .../trace/agent/test/utils/TraceUtils.groovy | 26 +- 26 files changed, 835 insertions(+), 906 deletions(-) create mode 100644 dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientPoolInstrumentationTest.groovy create mode 100644 dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/HttpUriRequest.groovy create mode 100644 dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/HttpUriRequest.groovy create mode 100644 dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionResponseCodeOnlyTest.groovy create mode 100644 dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionUseCachesFalseTest.groovy create mode 100644 dd-java-agent/instrumentation/http-url-connection/src/test/groovy/SpringRestTemplateTest.groovy create mode 100644 dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientAsyncTest.groovy create mode 100644 dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3AsyncTest.groovy diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java index 3be22c7500..0f24cf4d05 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java +++ b/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; } diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy index 4e6ac1d1a1..6135719cb9 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy +++ b/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.japi.Pair +import akka.http.javadsl.model.headers.RawHeader import akka.stream.ActorMaterializer -import akka.stream.StreamTcpException -import akka.stream.javadsl.Sink -import akka.stream.javadsl.Source -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.api.Config +import datadog.trace.agent.test.base.HttpClientTest 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 -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" +class AkkaHttpClientInstrumentationTest extends HttpClientTest { 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"() { - setup: - def url = server.address.resolve("/" + route).toURL() + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + 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()) - - when: - 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 response = Http.get(system).singleRequest(request, materializer).toCompletableFuture().get() + callback?.call() + return response.status().intValue() } - def "error request trace"() { - setup: - def url = new URL("http://localhost:$UNUSABLE_PORT/test") + @Override + AkkaHttpClientDecorator decorator() { + return AkkaHttpClientDecorator.DECORATE + } - HttpRequest request = HttpRequest.create(url.toString()) - CompletionStage responseFuture = - withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") { - Http.get(system) - .singleRequest(request, materializer) - } + @Override + String expectedOperationName() { + return "akka-http.request" + } - when: - responseFuture.toCompletableFuture().get() - - 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] + boolean testRedirects() { + false } 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 - .> single(new Pair(HttpRequest.create(url.toString()), 1)) - .via(pool) - .runWith(Sink., 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 - .> single(new Pair(HttpRequest.create(url.toString()), 1)) - .via(pool) - .runWith(Sink., 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() - } } diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientPoolInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientPoolInstrumentationTest.groovy new file mode 100644 index 0000000000..122843e543 --- /dev/null +++ b/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 { + 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 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 + .> single(new Pair(request, 1)) + .via(pool) + .runWith(Sink., 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 + } +} diff --git a/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientCallbackTest.groovy b/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientCallbackTest.groovy index ff5adcd29e..fde0e1bfad 100644 --- a/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientCallbackTest.groovy +++ b/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 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)) } diff --git a/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientNullCallbackTest.groovy b/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientNullCallbackTest.groovy index ea3ecff48b..0ce26b025a 100644 --- a/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientNullCallbackTest.groovy +++ b/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 headers, Closure callback) { - assert method == "GET" - - HttpGet request = new HttpGet(uri) + def request = new HttpUriRequest(method, uri) headers.entrySet().each { request.addHeader(new BasicHeader(it.key, it.value)) } diff --git a/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientTest.groovy b/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientTest.groovy index 1254e86316..c7a1c1cc1d 100644 --- a/dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientTest.groovy +++ b/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 headers, Closure callback) { - assert method == "GET" - HttpGet request = new HttpGet(uri) + def request = new HttpUriRequest(method, uri) headers.entrySet().each { request.addHeader(new BasicHeader(it.key, it.value)) } @@ -43,7 +41,7 @@ class ApacheHttpAsyncClientTest extends HttpClientTest headers, Closure callback) { - assert method == "GET" - HttpGet request = new HttpGet(uri) + def request = new HttpUriRequest(method, uri) headers.entrySet().each { request.addHeader(new BasicHeader(it.key, it.value)) } diff --git a/dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy b/dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy index 1865b41ef8..de242ccb24 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy +++ b/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 { @Override int doRequest(String method, URI uri, Map headers, Closure callback) { - assert method == "GET" - HttpGet request = new HttpGet(uri) + def request = new HttpUriRequest(method, 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.statusLine.statusCode + response.entity?.content?.close() // Make sure the connection is closed. + + return response.statusLine.statusCode } @Override diff --git a/dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/HttpUriRequest.groovy b/dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/HttpUriRequest.groovy new file mode 100644 index 0000000000..adec36dbd3 --- /dev/null +++ b/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; + } +} diff --git a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionResponseCodeOnlyTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionResponseCodeOnlyTest.groovy new file mode 100644 index 0000000000..7826334d64 --- /dev/null +++ b/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 { + + @Override + int doRequest(String method, URI uri, Map 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 + } +} diff --git a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy index 25262a7524..8422f8a461 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionTest.groovy +++ b/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.AutoCleanup +import spock.lang.Ignore 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 { - static final RESPONSE = "

Hello test.

" - static final STATUS = 202 + static final RESPONSE = "Hello." + static final STATUS = 200 - @AutoCleanup - @Shared - def server = httpServer { - handlers { - all { - handleDistributedRequest() - - response.status(STATUS).send(RESPONSE) - } + @Override + int doRequest(String method, URI uri, Map 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 = 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"() { - 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] - } - + @Ignore 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_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_URL.key" "$url" "$Tags.HTTP_METHOD.key" "POST" "$Tags.HTTP_STATUS.key" STATUS "$Tags.PEER_HOSTNAME.key" "localhost" diff --git a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionUseCachesFalseTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/HttpUrlConnectionUseCachesFalseTest.groovy new file mode 100644 index 0000000000..954e6960c5 --- /dev/null +++ b/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 { + + @Override + int doRequest(String method, URI uri, Map 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 + } +} diff --git a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/SpringRestTemplateTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/SpringRestTemplateTest.groovy new file mode 100644 index 0000000000..781456fea1 --- /dev/null +++ b/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 { + + @Shared + RestTemplate restTemplate = new RestTemplate() + + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + def httpHeaders = new HttpHeaders() + headers.each { httpHeaders.put(it.key, [it.value]) } + def request = new HttpEntity(httpHeaders); + ResponseEntity 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 + } +} diff --git a/dd-java-agent/instrumentation/jax-rs-client/jax-rs-client.gradle b/dd-java-agent/instrumentation/jax-rs-client/jax-rs-client.gradle index 511e986137..17017e084b 100644 --- a/dd-java-agent/instrumentation/jax-rs-client/jax-rs-client.gradle +++ b/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') diff --git a/dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientAsyncTest.groovy b/dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientAsyncTest.groovy new file mode 100644 index 0000000000..70bc570fcc --- /dev/null +++ b/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 { + + @Override + int doRequest(String method, URI uri, Map 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 + } +} diff --git a/dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientTest.groovy b/dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientTest.groovy index a5ae27a2df..e6303ba00b 100644 --- a/dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientTest.groovy +++ b/dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientTest.groovy @@ -1,131 +1,71 @@ -import datadog.trace.agent.test.AgentTestRunner -import io.opentracing.tag.Tags -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 datadog.trace.agent.test.base.HttpClientTest +import datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator 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 -import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT +abstract class JaxRsClientTest extends HttpClientTest { -class JaxRsClientTest extends AgentTestRunner { + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { - @AutoCleanup - @Shared - def server = httpServer { - handlers { - all { - response.status(200).send("pong") - } - } + Client client = builder().build() + WebTarget service = client.target(uri) + Invocation.Builder request = service.request(MediaType.TEXT_PLAIN) + headers.each { request.header(it.key, it.value) } + Response response = request.method(method) + callback?.call() + + return response.status } - def "#lib request creates spans and sends headers"() { - setup: - Client client = builder.build() - 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" + @Override + JaxRsClientDecorator decorator() { + return JaxRsClientDecorator.DECORATE } - def "#lib connection failure creates errored span"() { - when: - Client client = builder.build() - 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() - } + @Override + String expectedOperationName() { + return "jax-rs.client.call" + } - then: - thrown async ? ExecutionException : ProcessingException + boolean testRedirects() { + false + } - assertTraces(1) { - 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() - } - } - } - } + abstract ClientBuilder builder() +} - where: - builder | async | lib - new JerseyClientBuilder() | false | "jersey" - new ResteasyClientBuilder() | false | "resteasy" - new JerseyClientBuilder() | true | "jersey async" - new ResteasyClientBuilder() | true | "resteasy async" - // Unfortunately there's not a good way to instrument this for CXF. +class JerseyClientTest extends JaxRsClientTest { + + @Override + ClientBuilder builder() { + return new JerseyClientBuilder() + } +} + +class ResteasyClientTest extends JaxRsClientTest { + + @Override + ClientBuilder builder() { + return new ResteasyClientBuilder() + } +} + +class CxfClientTest extends JaxRsClientTest { + + @Override + ClientBuilder builder() { + return new ClientBuilderImpl() + } + + boolean testConnectionFailure() { + false } } diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/NettyHttpClientDecorator.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/NettyHttpClientDecorator.java index 0cb74b1e4c..fec0b41cd0 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/NettyHttpClientDecorator.java +++ b/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 { - @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"() { - setup: - def responseFuture = runUnderTrace("parent") { - asyncHttpClient.prepareGet("$server.address").execute() - } - def response = responseFuture.get() - - expect: - 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}" + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + def methodName = "prepare" + method.toLowerCase().capitalize() + def requestBuilder = asyncHttpClient."$methodName"(uri.toString()) + headers.each { requestBuilder.setHeader(it.key, it.value) } + def response = requestBuilder.execute().get() + callback?.call() + return response.statusCode } - def "test connection failure"() { - setup: - def invalidPort = PortUtils.randomOpenPort() + @Override + NettyHttpClientDecorator decorator() { + return NettyHttpClientDecorator.DECORATE + } - def responseFuture = runUnderTrace("parent") { - asyncHttpClient.prepareGet("http://localhost:$invalidPort/").execute() - } + @Override + 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) - throwable.cause instanceof ConnectException + def ex = thrown(Exception) + def thrownException = ex instanceof ExecutionException ? ex.cause : ex and: assertTraces(1) { trace(0, 2) { - span(0) { - operationName "parent" - parent() - } + parentSpan(it, 0, thrownException) + 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" } } diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/NettyHttpClientDecorator.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/NettyHttpClientDecorator.java index be116e2d78..5f33f5ce5d 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/NettyHttpClientDecorator.java +++ b/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 { - @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"() { - setup: - def responseFuture = runUnderTrace("parent") { - asyncHttpClient.prepareGet("$server.address").execute() - } - def response = responseFuture.get() - - expect: - 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}" + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + def methodName = "prepare" + method.toLowerCase().capitalize() + def requestBuilder = asyncHttpClient."$methodName"(uri.toString()) + headers.each { requestBuilder.setHeader(it.key, it.value) } + def response = requestBuilder.execute().get() + callback?.call() + return response.statusCode } - def "test connection failure"() { - setup: - def invalidPort = PortUtils.randomOpenPort() + @Override + NettyHttpClientDecorator decorator() { + return NettyHttpClientDecorator.DECORATE + } - def responseFuture = runUnderTrace("parent") { - asyncHttpClient.prepareGet("http://localhost:$invalidPort/").execute() - } + @Override + 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) - throwable.cause instanceof ConnectException + def ex = thrown(Exception) + ex.cause instanceof ConnectException + def thrownException = ex instanceof ExecutionException ? ex.cause : ex and: assertTraces(1) { trace(0, 2) { - span(0) { - operationName "parent" - parent() - } + parentSpan(it, 0, thrownException) + 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" } } diff --git a/dd-java-agent/instrumentation/okhttp-3/okhttp-3.gradle b/dd-java-agent/instrumentation/okhttp-3/okhttp-3.gradle index d518271037..9689977a12 100644 --- a/dd-java-agent/instrumentation/okhttp-3/okhttp-3.gradle +++ b/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 diff --git a/dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3AsyncTest.groovy b/dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3AsyncTest.groovy new file mode 100644 index 0000000000..fc949d76b9 --- /dev/null +++ b/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 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 responseRef = new AtomicReference() + AtomicReference 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() + } +} diff --git a/dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy b/dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy index b64cd4d8a3..0317287375 100644 --- a/dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy +++ b/dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy @@ -1,160 +1,124 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.utils.PortUtils -import datadog.trace.api.Config +import datadog.opentracing.DDSpan +import datadog.trace.agent.test.asserts.TraceAssert +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.Callback +import okhttp3.Headers +import okhttp3.MediaType import okhttp3.OkHttpClient import okhttp3.Request -import okhttp3.Response -import spock.lang.AutoCleanup -import spock.lang.Shared +import okhttp3.RequestBody +import okhttp3.internal.http.HttpMethod -import java.util.concurrent.CountDownLatch -import java.util.concurrent.atomic.AtomicReference +import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.NETWORK_DECORATE -import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer -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") - } - } - } +class OkHttp3Test extends HttpClientTest { def client = new OkHttpClient() - def "sending a request creates spans and sends headers"() { - setup: - def requestBulder = new Request.Builder() - .url("http://localhost:$server.address.port/ping") - if (agentRequest) { - requestBulder.addHeader("Datadog-Meta-Lang", "java") - } - def request = requestBulder.build() - - Response response = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") { - if (!async) { - return client.newCall(request).execute() - } - - AtomicReference 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 + @Override + int doRequest(String method, URI uri, Map 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() + def response = client.newCall(request).execute() + callback?.call() + return response.code() } - def "sending an invalid request creates an error span"() { - setup: - def unusablePort = PortUtils.UNUSABLE_PORT - def request = new Request.Builder() - .url("http://localhost:$unusablePort/ping") - .build() + @Override + OkHttpClientDecorator decorator() { + return OkHttpClientDecorator.DECORATE + } - when: - withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") { - client.newCall(request).execute() + @Override + // parent span must be cast otherwise it breaks debugging classloading (junit loads it early) + 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) { - 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() - } - } - } + server.distributedRequestTrace(it, 0) } 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) } } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy index db52bade30..77729217fb 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy +++ b/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 extends AgentTestRunner { + protected static final BODY_METHODS = ["POST", "PUT"] @AutoCleanup @Shared @@ -63,24 +67,34 @@ abstract class HttpClientTest 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)) - trace(1, 1) { - clientSpan(it, 0, null, false) + server.distributedRequestTrace(it, 0, trace(1).last()) + trace(1, size(1)) { + 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 extends AgentTestRu then: status == 200 assertTraces(2) { - server.distributedRequestTrace(it, 0, trace(1).get(1)) - trace(1, 2) { + server.distributedRequestTrace(it, 0, trace(1).last()) + 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 extends AgentTestRu then: status == 200 assertTraces(2) { - server.distributedRequestTrace(it, 0, trace(1).get(0)) - trace(1, 1) { - clientSpan(it, 0, null, true) + server.distributedRequestTrace(it, 0, trace(1).last()) + trace(1, size(1)) { + clientSpan(it, 0, null, method, true) } } where: - method = "GET" + method = "HEAD" } def "trace request without propagation"() { @@ -132,9 +147,9 @@ abstract class HttpClientTest 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 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 extends AgentTestRu status == 200 // only one trace (client). assertTraces(2) { - trace(0, 1) { - clientSpan(it, 0, null, false) + trace(0, size(1)) { + clientSpan(it, 0, null, method, false) } trace(1, 1) { span(0) { @@ -197,8 +212,10 @@ abstract class HttpClientTest 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 extends AgentTestRu then: status == 200 assertTraces(3) { - server.distributedRequestTrace(it, 0, trace(2).get(0)) - server.distributedRequestTrace(it, 1, trace(2).get(0)) - trace(2, 1) { - clientSpan(it, 0, null, false, uri) + server.distributedRequestTrace(it, 0, trace(2).last()) + server.distributedRequestTrace(it, 1, trace(2).last()) + trace(2, size(1)) { + clientSpan(it, 0, null, method, false, false, uri) } } @@ -218,8 +235,10 @@ abstract class HttpClientTest 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 extends AgentTestRu then: status == 200 assertTraces(4) { - server.distributedRequestTrace(it, 0, trace(3).get(0)) - server.distributedRequestTrace(it, 1, trace(3).get(0)) - server.distributedRequestTrace(it, 2, trace(3).get(0)) - trace(3, 1) { - clientSpan(it, 0, null, false, uri) + server.distributedRequestTrace(it, 0, trace(3).last()) + server.distributedRequestTrace(it, 1, trace(3).last()) + server.distributedRequestTrace(it, 2, trace(3).last()) + trace(3, size(1)) { + clientSpan(it, 0, null, method, false, false, uri) } } @@ -240,8 +259,10 @@ abstract class HttpClientTest 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 extends AgentTestRu and: assertTraces(3) { - server.distributedRequestTrace(it, 0, trace(2).get(0)) - server.distributedRequestTrace(it, 1, trace(2).get(0)) - trace(2, 1) { - clientSpan(it, 0, null, false, uri, statusOnRedirectError(), thrownException) + server.distributedRequestTrace(it, 0, trace(2).last()) + server.distributedRequestTrace(it, 1, trace(2).last()) + trace(2, size(1)) { + 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 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 extends AgentTestRu childOf((DDSpan) parentSpan) } serviceName renameService ? "localhost" : "unnamed-java-app" - operationName "http.request" - resourceName "GET $uri.path" + operationName expectedOperationName() + resourceName "$method $uri.path" spanType DDSpanTypes.HTTP_CLIENT errored exception != null tags { @@ -302,12 +349,29 @@ abstract class HttpClientTest 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.HTTP_METHOD.key" "GET" + "$Tags.PEER_PORT.key" uri.port + "$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 + } } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy index 50319015b3..65eb3618fc 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/TraceUtils.groovy +++ b/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 Object runUnderTrace(final String rootOperationName, final Callable 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() - Tags.ERROR.set(span, true) - span.log(Collections.singletonMap(ERROR_OBJECT, e)) - + DECORATOR.onError(scope, e) throw e } finally { + DECORATOR.beforeFinish(scope) scope.close() } }