diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java b/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java index e01fdef841..50d4eaff8b 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java @@ -104,6 +104,7 @@ public final class AkkaHttpServerInstrumentation extends Instrumenter.Default { final Scope scope = GlobalTracer.get() .buildSpan("akka-http.request") + .ignoreActiveSpan() .asChildOf(extractedContext) .startActive(false); diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy index b64939e69a..5c158ef414 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy @@ -1,188 +1,91 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.utils.OkHttpUtils +import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.api.DDSpanTypes +import datadog.trace.instrumentation.akkahttp.AkkaHttpServerDecorator import io.opentracing.tag.Tags -import okhttp3.Request -import spock.lang.Shared -class AkkaHttpServerInstrumentationTest extends AgentTestRunner { +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS - @Shared - int asyncPort - @Shared - int syncPort +abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest { - @Shared - def client = OkHttpUtils.client() - - def setupSpec() { - AkkaHttpTestAsyncWebServer.start() - asyncPort = AkkaHttpTestAsyncWebServer.port() - AkkaHttpTestSyncWebServer.start() - syncPort = AkkaHttpTestSyncWebServer.port() + @Override + AkkaHttpServerDecorator decorator() { + return AkkaHttpServerDecorator.DECORATE } - def cleanupSpec() { - AkkaHttpTestAsyncWebServer.stop() - AkkaHttpTestSyncWebServer.stop() + @Override + String expectedOperationName() { + return "akka-http.request" } - def "#server 200 request trace"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/test") - .header("x-datadog-trace-id", "123") - .header("x-datadog-parent-id", "456") - .get() - .build() - def response = client.newCall(request).execute() + @Override + boolean testExceptionBody() { + false + } - expect: - response.code() == 200 +// FIXME: This doesn't work because we don't support bindAndHandle. +// @Override +// void startServer(int port) { +// AkkaHttpTestWebServer.start(port) +// } +// +// @Override +// void stopServer() { +// AkkaHttpTestWebServer.stop() +// } - assertTraces(1) { - trace(0, 2) { - span(0) { - traceId "123" - parentId "456" - serviceName "unnamed-java-app" - operationName "akka-http.request" - resourceName "GET /test" - spanType DDSpanTypes.HTTP_SERVER - errored false - tags { - defaultTags(true) - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "http://localhost:$port/test" - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.COMPONENT.key" "akka-http-server" - } - } - span(1) { - childOf span(0) - assert span(1).operationName.endsWith('.tracedMethod') + void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName expectedOperationName() + resourceName endpoint.status == 404 ? "404" : "$method ${endpoint.resolve(address).path}" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint.errored + if (parentID != null) { + traceId traceID + parentId parentID + } else { + parent() + } + tags { + defaultTags(true) + "$Tags.COMPONENT.key" serverDecorator.component() + if (endpoint.errored) { + "$Tags.ERROR.key" endpoint.errored + "error.msg" { it == null || it == EXCEPTION.body } + "error.type" { it == null || it == Exception.name } + "error.stack" { it == null || it instanceof String } } + "$Tags.HTTP_STATUS.key" endpoint.status + "$Tags.HTTP_URL.key" "${endpoint.resolve(address)}" + "$Tags.HTTP_METHOD.key" method + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER } } - - where: - server | port - "async" | asyncPort - "sync" | syncPort - } - - def "#server exceptions trace for #endpoint"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/$endpoint") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - response.code() == 500 - - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "unnamed-java-app" - operationName "akka-http.request" - resourceName "GET /$endpoint" - spanType DDSpanTypes.HTTP_SERVER - errored true - tags { - defaultTags() - "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "http://localhost:$port/$endpoint" - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.COMPONENT.key" "akka-http-server" - errorTags RuntimeException, errorMessage - } - } - } - } - - where: - server | port | endpoint | errorMessage - "async" | asyncPort | "throw-handler" | "Oh no handler" - "async" | asyncPort | "throw-callback" | "Oh no callback" - "sync" | syncPort | "throw-handler" | "Oh no handler" - } - - def "#server 5xx trace"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/server-error") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - response.code() == 500 - - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "unnamed-java-app" - operationName "akka-http.request" - resourceName "GET /server-error" - spanType DDSpanTypes.HTTP_SERVER - errored true - tags { - defaultTags() - "$Tags.HTTP_STATUS.key" 500 - "$Tags.HTTP_URL.key" "http://localhost:$port/server-error" - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.COMPONENT.key" "akka-http-server" - "$Tags.ERROR.key" true - } - } - } - } - - where: - server | port - "async" | asyncPort - "sync" | syncPort - } - - def "#server 4xx trace"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/not-found") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - response.code() == 404 - - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "unnamed-java-app" - operationName "akka-http.request" - resourceName "404" - spanType DDSpanTypes.HTTP_SERVER - errored false - tags { - defaultTags() - "$Tags.HTTP_STATUS.key" 404 - "$Tags.HTTP_URL.key" "http://localhost:$port/not-found" - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.COMPONENT.key" "akka-http-server" - } - } - } - } - - where: - server | port - "async" | asyncPort - "sync" | syncPort + } +} + +class AkkaHttpServerInstrumentationTestSync extends AkkaHttpServerInstrumentationTest { + @Override + void startServer(int port) { + AkkaHttpTestSyncWebServer.start(port) + } + + @Override + void stopServer() { + AkkaHttpTestSyncWebServer.stop() + } +} + +class AkkaHttpServerInstrumentationTestAsync extends AkkaHttpServerInstrumentationTest { + @Override + void startServer(int port) { + AkkaHttpTestAsyncWebServer.start(port) + } + + @Override + void stopServer() { + AkkaHttpTestAsyncWebServer.stop() } } diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpTestInstrumentation.java b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpTestInstrumentation.java new file mode 100644 index 0000000000..cd10a5c4ca --- /dev/null +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpTestInstrumentation.java @@ -0,0 +1,19 @@ +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.test.base.HttpServerTestAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import net.bytebuddy.agent.builder.AgentBuilder; + +@AutoService(Instrumenter.class) +public class AkkaHttpTestInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(named("akka.http.impl.engine.server.HttpServerBluePrint$PrepareRequests$$anon$1")) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice(named("onPush"), HttpServerTestAdvice.ServerEntryAdvice.class.getName())); + } +} diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestAsyncWebServer.scala b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestAsyncWebServer.scala new file mode 100644 index 0000000000..1b5c47b19c --- /dev/null +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestAsyncWebServer.scala @@ -0,0 +1,54 @@ +import akka.actor.ActorSystem +import akka.http.scaladsl.Http +import akka.http.scaladsl.Http.ServerBinding +import akka.http.scaladsl.model.HttpMethods.GET +import akka.http.scaladsl.model._ +import akka.stream.ActorMaterializer +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint._ +import groovy.lang.Closure + +import scala.concurrent.{Await, Future} + +object AkkaHttpTestAsyncWebServer { + implicit val system = ActorSystem("my-system") + implicit val materializer = ActorMaterializer() + // needed for the future flatMap/onComplete in the end + implicit val executionContext = system.dispatcher + val asyncHandler: HttpRequest => Future[HttpResponse] = { + case HttpRequest(GET, uri: Uri, _, _, _) => { + Future { + val endpoint = HttpServerTest.ServerEndpoint.forPath(uri.path.toString()) + HttpServerTest.controller(endpoint, new Closure[HttpResponse]() { + def doCall(): HttpResponse = { + val resp = HttpResponse(status = endpoint.getStatus) //.withHeaders(headers.Type)resp.contentType = "text/plain" + endpoint match { + case SUCCESS => resp.withEntity(endpoint.getBody) + case REDIRECT => resp.withHeaders(headers.Location(endpoint.getBody)) + case ERROR => resp.withEntity(endpoint.getBody) + case EXCEPTION => throw new Exception(endpoint.getBody) + case _ => HttpResponse(status = NOT_FOUND.getStatus).withEntity(NOT_FOUND.getBody) + } + } + }) + } + } + } + + private var binding: ServerBinding = null + + def start(port: Int): Unit = synchronized { + if (null == binding) { + import scala.concurrent.duration._ + binding = Await.result(Http().bindAndHandleAsync(asyncHandler, "localhost", port), 10 seconds) + } + } + + def stop(): Unit = synchronized { + if (null != binding) { + binding.unbind() + system.terminate() + binding = null + } + } +} diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestSyncWebServer.scala b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestSyncWebServer.scala new file mode 100644 index 0000000000..99bbdf63ff --- /dev/null +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestSyncWebServer.scala @@ -0,0 +1,52 @@ +import akka.actor.ActorSystem +import akka.http.scaladsl.Http +import akka.http.scaladsl.Http.ServerBinding +import akka.http.scaladsl.model.HttpMethods.GET +import akka.http.scaladsl.model._ +import akka.stream.ActorMaterializer +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint._ +import groovy.lang.Closure + +import scala.concurrent.Await + +object AkkaHttpTestSyncWebServer { + implicit val system = ActorSystem("my-system") + implicit val materializer = ActorMaterializer() + // needed for the future flatMap/onComplete in the end + implicit val executionContext = system.dispatcher + val syncHandler: HttpRequest => HttpResponse = { + case HttpRequest(GET, uri: Uri, _, _, _) => { + val endpoint = HttpServerTest.ServerEndpoint.forPath(uri.path.toString()) + HttpServerTest.controller(endpoint, new Closure[HttpResponse]() { + def doCall(): HttpResponse = { + val resp = HttpResponse(status = endpoint.getStatus) + endpoint match { + case SUCCESS => resp.withEntity(endpoint.getBody) + case REDIRECT => resp.withHeaders(headers.Location(endpoint.getBody)) + case ERROR => resp.withEntity(endpoint.getBody) + case EXCEPTION => throw new Exception(endpoint.getBody) + case _ => HttpResponse(status = NOT_FOUND.getStatus).withEntity(NOT_FOUND.getBody) + } + } + }) + } + } + + private var binding: ServerBinding = null + + def start(port: Int): Unit = synchronized { + if (null == binding) { + import scala.concurrent.duration._ + binding = Await.result(Http().bindAndHandleSync(syncHandler, "localhost", port), 10 seconds) + } + } + + def stop(): Unit = synchronized { + if (null != binding) { + binding.unbind() + system.terminate() + binding = null + } + } +} diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestWebServer.scala b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestWebServer.scala index 83682c5dce..caeb2379cc 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestWebServer.scala +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/scala/AkkaHttpTestWebServer.scala @@ -1,48 +1,43 @@ import akka.actor.ActorSystem import akka.http.scaladsl.Http import akka.http.scaladsl.Http.ServerBinding -import akka.http.scaladsl.model.HttpMethods.GET import akka.http.scaladsl.model._ +import akka.http.scaladsl.server.Directives._ +import akka.http.scaladsl.server.ExceptionHandler import akka.stream.ActorMaterializer -import datadog.trace.agent.test.utils.PortUtils -import datadog.trace.api.Trace +import datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint._ -import scala.concurrent.{Await, Future} +import scala.concurrent.Await -object AkkaHttpTestAsyncWebServer { - val port = PortUtils.randomOpenPort() +// FIXME: This doesn't work because we don't support bindAndHandle. +object AkkaHttpTestWebServer { implicit val system = ActorSystem("my-system") implicit val materializer = ActorMaterializer() // needed for the future flatMap/onComplete in the end implicit val executionContext = system.dispatcher - val asyncHandler: HttpRequest => Future[HttpResponse] = { - case HttpRequest(GET, Uri.Path("/test"), _, _, _) => - Future { - tracedMethod() - HttpResponse(entity = "Hello unit test.") - } - case HttpRequest(GET, Uri.Path("/throw-handler"), _, _, _) => - sys.error("Oh no handler") - case HttpRequest(GET, Uri.Path("/throw-callback"), _, _, _) => - Future { - sys.error("Oh no callback") - } - case HttpRequest(GET, Uri.Path("/server-error"), _, _, _) => - Future { - HttpResponse(entity = "Error unit test.", status = StatusCodes.InternalServerError) - } - case _ => - Future { - HttpResponse(entity = "Not found unit test.", status = StatusCodes.NotFound) - } + + val exceptionHandler = ExceptionHandler { + case ex: Exception => complete(HttpResponse(status = EXCEPTION.getStatus).withEntity(ex.getMessage)) + } + + val route = { //handleExceptions(exceptionHandler) { + path(SUCCESS.rawPath) { + complete(HttpResponse(status = SUCCESS.getStatus).withEntity(SUCCESS.getBody)) + } ~ path(REDIRECT.rawPath) { + redirect(Uri(REDIRECT.getBody), StatusCodes.Found) + } ~ path(ERROR.rawPath) { + complete(HttpResponse(status = ERROR.getStatus).withEntity(ERROR.getBody)) + } ~ path(EXCEPTION.rawPath) { + failWith(new Exception(EXCEPTION.getBody)) + } } private var binding: ServerBinding = null - def start(): Unit = synchronized { + def start(port: Int): Unit = synchronized { if (null == binding) { import scala.concurrent.duration._ - binding = Await.result(Http().bindAndHandleAsync(asyncHandler, "localhost", port), 10 seconds) + binding = Await.result(Http().bindAndHandle(route, "localhost", port), 10 seconds) } } @@ -53,48 +48,4 @@ object AkkaHttpTestAsyncWebServer { binding = null } } - - @Trace - def tracedMethod(): Unit = { - } -} - -object AkkaHttpTestSyncWebServer { - val port = PortUtils.randomOpenPort() - implicit val system = ActorSystem("my-system") - implicit val materializer = ActorMaterializer() - // needed for the future flatMap/onComplete in the end - implicit val executionContext = system.dispatcher - val syncHandler: HttpRequest => HttpResponse = { - case HttpRequest(GET, Uri.Path("/test"), _, _, _) => - tracedMethod() - HttpResponse(entity = "Hello unit test.") - case HttpRequest(GET, Uri.Path("/throw-handler"), _, _, _) => - sys.error("Oh no handler") - case HttpRequest(GET, Uri.Path("/server-error"), _, _, _) => - HttpResponse(entity = "Error unit test.", status = StatusCodes.InternalServerError) - case _ => - HttpResponse(entity = "Not found unit test.", status = StatusCodes.NotFound) - } - - private var binding: ServerBinding = null - - def start(): Unit = synchronized { - if (null == binding) { - import scala.concurrent.duration._ - binding = Await.result(Http().bindAndHandleSync(syncHandler, "localhost", port), 10 seconds) - } - } - - def stop(): Unit = synchronized { - if (null != binding) { - binding.unbind() - system.terminate() - binding = null - } - } - - @Trace - def tracedMethod(): Unit = { - } } diff --git a/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle b/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle index 0e7ab43bf2..bd997261d5 100644 --- a/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle +++ b/dd-java-agent/instrumentation/netty-4.0/netty-4.0.gradle @@ -19,6 +19,12 @@ muzzle { versions = "[4.0.0.Final,4.1.0.Final)" assertInverse = true } + pass { + group = "io.vertx" + module = "vertx-core" + versions = "[2.0.0,3.3.0)" + assertInverse = true + } } apply plugin: 'org.unbroken-dome.test-sets' diff --git a/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle b/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle index f05a4326f1..d823a6af28 100644 --- a/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle +++ b/dd-java-agent/instrumentation/netty-4.1/netty-4.1.gradle @@ -18,6 +18,12 @@ muzzle { versions = "[4.1.0.Final,)" assertInverse = true } + pass { + group = "io.vertx" + module = "vertx-core" + versions = "[3.3.0,)" + assertInverse = true + } } apply plugin: 'org.unbroken-dome.test-sets' diff --git a/dd-java-agent/instrumentation/servlet-2/servlet-2.gradle b/dd-java-agent/instrumentation/servlet-2/servlet-2.gradle index 1910352f6c..fa45b539cf 100644 --- a/dd-java-agent/instrumentation/servlet-2/servlet-2.gradle +++ b/dd-java-agent/instrumentation/servlet-2/servlet-2.gradle @@ -11,6 +11,14 @@ muzzle { apply from: "${rootDir}/gradle/java.gradle" +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + dependencies { compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3' @@ -26,4 +34,7 @@ dependencies { } testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '7.0.0.v20091005' testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '7.0.0.v20091005' + + latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '7.+' + latestDepTestCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '7.+' } diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java index cb2ed723a1..e1ece26995 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java @@ -24,9 +24,10 @@ public abstract class AbstractServlet2Instrumentation extends Instrumenter.Defau "datadog.trace.agent.decorator.BaseDecorator", "datadog.trace.agent.decorator.ServerDecorator", "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".Servlet2Decorator", packageName + ".HttpServletRequestExtractAdapter", packageName + ".HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", - packageName + ".Servlet2Decorator", + packageName + ".StatusSavingHttpServletResponseWrapper", }; } } diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java index f116523fd9..9b02f13edf 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java @@ -8,23 +8,35 @@ import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.security.Principal; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; public class Servlet2Advice { + public static final String SERVLET_SPAN = "datadog.servlet.span"; @Advice.OnMethodEnter(suppress = Throwable.class) public static Scope startSpan( - @Advice.This final Object servlet, @Advice.Argument(0) final ServletRequest req) { - if (GlobalTracer.get().activeSpan() != null || !(req instanceof HttpServletRequest)) { + @Advice.This final Object servlet, + @Advice.Argument(0) final ServletRequest req, + @Advice.Argument(value = 1, readOnly = false, typing = Assigner.Typing.DYNAMIC) + ServletResponse resp) { + final Object spanAttr = req.getAttribute(SERVLET_SPAN); + if (!(req instanceof HttpServletRequest) || spanAttr != null) { // Tracing might already be applied by the FilterChain. If so ignore this. return null; } + if (resp instanceof HttpServletResponse) { + resp = new StatusSavingHttpServletResponseWrapper((HttpServletResponse) resp); + } + final HttpServletRequest httpServletRequest = (HttpServletRequest) req; final SpanContext extractedContext = GlobalTracer.get() @@ -35,6 +47,7 @@ public class Servlet2Advice { final Scope scope = GlobalTracer.get() .buildSpan("servlet.request") + .ignoreActiveSpan() .asChildOf(extractedContext) .withTag("span.origin.type", servlet.getClass().getName()) .startActive(true); @@ -47,6 +60,8 @@ public class Servlet2Advice { if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true); } + + req.setAttribute(SERVLET_SPAN, span); return scope; } @@ -68,9 +83,18 @@ public class Servlet2Advice { } if (scope != null) { - DECORATE.onResponse(scope.span(), response); - DECORATE.onError(scope.span(), throwable); - DECORATE.beforeFinish(scope.span()); + final Span span = scope.span(); + DECORATE.onResponse(span, response); + if (throwable != null) { + if (response instanceof StatusSavingHttpServletResponseWrapper + && ((StatusSavingHttpServletResponseWrapper) response).status + == HttpServletResponse.SC_OK) { + // exception was thrown but status code wasn't set + Tags.HTTP_STATUS.set(span, 500); + } + DECORATE.onError(span, throwable); + } + DECORATE.beforeFinish(span); if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(false); diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java index 597ac0bcfe..50c6998339 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java @@ -43,13 +43,18 @@ public class Servlet2Decorator @Override protected Integer peerPort(final HttpServletRequest httpServletRequest) { + // HttpServletResponse doesn't have accessor for remote port. return null; } @Override protected Integer status(final ServletResponse httpServletResponse) { - // HttpServletResponse doesn't have accessor for status code. - return null; + if (httpServletResponse instanceof StatusSavingHttpServletResponseWrapper) { + return ((StatusSavingHttpServletResponseWrapper) httpServletResponse).status; + } else { + // HttpServletResponse doesn't have accessor for status code. + return null; + } } @Override diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java new file mode 100644 index 0000000000..28e50fb4cd --- /dev/null +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java @@ -0,0 +1,37 @@ +package datadog.trace.instrumentation.servlet2; + +import java.io.IOException; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; + +public class StatusSavingHttpServletResponseWrapper extends HttpServletResponseWrapper { + public int status = 200; + + public StatusSavingHttpServletResponseWrapper(final HttpServletResponse response) { + super(response); + } + + @Override + public void sendError(final int status) throws IOException { + this.status = status; + super.sendError(status); + } + + @Override + public void sendError(final int status, final String message) throws IOException { + this.status = status; + super.sendError(status, message); + } + + @Override + public void setStatus(final int status) { + this.status = status; + super.setStatus(status); + } + + @Override + public void setStatus(final int status, final String message) { + this.status = status; + super.setStatus(status, message); + } +} diff --git a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy index a2904fc103..023f2ea1d6 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy +++ b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy @@ -1,197 +1,116 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.agent.test.utils.PortUtils +import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.api.DDSpanTypes -import datadog.trace.api.DDTags -import okhttp3.Credentials -import okhttp3.Interceptor -import okhttp3.OkHttpClient -import okhttp3.Request -import okhttp3.Response -import org.eclipse.jetty.http.HttpHeaders -import org.eclipse.jetty.http.security.Constraint -import org.eclipse.jetty.security.ConstraintMapping -import org.eclipse.jetty.security.ConstraintSecurityHandler -import org.eclipse.jetty.security.HashLoginService -import org.eclipse.jetty.security.LoginService -import org.eclipse.jetty.security.authentication.BasicAuthenticator +import datadog.trace.instrumentation.servlet2.Servlet2Decorator +import io.opentracing.tag.Tags +import javax.servlet.http.HttpServletRequest import org.eclipse.jetty.server.Server +import org.eclipse.jetty.server.handler.ErrorHandler import org.eclipse.jetty.servlet.ServletContextHandler -class JettyServlet2Test extends AgentTestRunner { +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS - OkHttpClient client = OkHttpUtils.clientBuilder().addNetworkInterceptor(new Interceptor() { - @Override - Response intercept(Interceptor.Chain chain) throws IOException { - def response = chain.proceed(chain.request()) - TEST_WRITER.waitForTraces(1) - return response - } - }) - .build() +class JettyServlet2Test extends HttpServerTest { - int port + private static final CONTEXT = "ctx" private Server jettyServer - private ServletContextHandler servletContext - def setup() { - port = PortUtils.randomOpenPort() + @Override + void startServer(int port) { jettyServer = new Server(port) - servletContext = new ServletContextHandler() - servletContext.contextPath = "/ctx" + jettyServer.connectors.each { it.resolveNames = true } // get localhost instead of 127.0.0.1 + ServletContextHandler servletContext = new ServletContextHandler(null, "/$CONTEXT") + servletContext.errorHandler = new ErrorHandler() { + protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message) throws IOException { + Throwable th = (Throwable) request.getAttribute("javax.servlet.error.exception") + writer.write(th ? th.message : message) + } + } - ConstraintSecurityHandler security = setupAuthentication(jettyServer) + // FIXME: Add tests for security/authentication. +// ConstraintSecurityHandler security = setupAuthentication(jettyServer) +// servletContext.setSecurityHandler(security) - servletContext.setSecurityHandler(security) - servletContext.addServlet(TestServlet2.Sync, "/sync") - servletContext.addServlet(TestServlet2.Sync, "/auth/sync") + servletContext.addServlet(TestServlet2.Sync, SUCCESS.path) + servletContext.addServlet(TestServlet2.Sync, ERROR.path) + servletContext.addServlet(TestServlet2.Sync, EXCEPTION.path) + servletContext.addServlet(TestServlet2.Sync, REDIRECT.path) + servletContext.addServlet(TestServlet2.Sync, AUTH_REQUIRED.path) jettyServer.setHandler(servletContext) jettyServer.start() } - def cleanup() { + @Override + void stopServer() { jettyServer.stop() jettyServer.destroy() } - def "test #path servlet call (auth: #auth, distributed tracing: #distributedTracing)"() { - setup: - def requestBuilder = new Request.Builder() - .url("http://localhost:$port/ctx/$path") - .get() - if (distributedTracing) { - requestBuilder.header("x-datadog-trace-id", "123") - requestBuilder.header("x-datadog-parent-id", "456") - } - if (auth) { - requestBuilder.header(HttpHeaders.AUTHORIZATION, Credentials.basic("user", "password")) - } - def response = client.newCall(requestBuilder.build()).execute() - - expect: - response.body().string().trim() == expectedResponse - - assertTraces(1) { - trace(0, 1) { - span(0) { - if (distributedTracing) { - traceId "123" - parentId "456" - } else { - parent() - } - serviceName "ctx" - operationName "servlet.request" - resourceName "GET /ctx/$path" - spanType DDSpanTypes.HTTP_SERVER - errored false - tags { - "http.url" "http://localhost:$port/ctx/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "peer.hostname" "127.0.0.1" - "peer.ipv4" "127.0.0.1" - "span.origin.type" "TestServlet2\$Sync" - "servlet.context" "/ctx" - if (auth) { - "$DDTags.USER_NAME" "user" - } - defaultTags(distributedTracing) - } - } - } - } - - where: - path | expectedResponse | auth | distributedTracing - "sync" | "Hello Sync" | false | false - "auth/sync" | "Hello Sync" | true | false - "sync" | "Hello Sync" | false | true - "auth/sync" | "Hello Sync" | true | true + @Override + URI buildAddress() { + return new URI("http://localhost:$port/$CONTEXT/") } - def "test #path error servlet call"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/ctx/$path?error=true") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - response.body().string().trim() != expectedResponse - - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "ctx" - operationName "servlet.request" - resourceName "GET /ctx/$path" - spanType DDSpanTypes.HTTP_SERVER - errored true - parent() - tags { - "http.url" "http://localhost:$port/ctx/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "peer.hostname" "127.0.0.1" - "peer.ipv4" "127.0.0.1" - "span.origin.type" "TestServlet2\$Sync" - "servlet.context" "/ctx" - errorTags(RuntimeException, "some $path error") - defaultTags() - } - } - } - } - - where: - path | expectedResponse - "sync" | "Hello Sync" + @Override + Servlet2Decorator decorator() { + return Servlet2Decorator.DECORATE } - def "test #path non-throwing-error servlet call"() { - // This doesn't actually detect the error because we can't get the status code via the old servlet API. - setup: - def request = new Request.Builder() - .url("http://localhost:$port/ctx/$path?non-throwing-error=true") - .get() - .build() - def response = client.newCall(request).execute() + @Override + String expectedServiceName() { + CONTEXT + } - expect: - response.body().string().trim() != expectedResponse + @Override + String expectedOperationName() { + return "servlet.request" + } - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "ctx" - operationName "servlet.request" - resourceName "GET /ctx/$path" - spanType DDSpanTypes.HTTP_SERVER - errored false - parent() - tags { - "http.url" "http://localhost:$port/ctx/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "peer.hostname" "127.0.0.1" - "peer.ipv4" "127.0.0.1" - "span.origin.type" "TestServlet2\$Sync" - "servlet.context" "/ctx" - defaultTags() - } + @Override + boolean testNotFound() { + false + } + + // parent span must be cast otherwise it breaks debugging classloading (junit loads it early) + void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName expectedOperationName() + resourceName endpoint.status == 404 ? "404" : "$method ${endpoint.resolve(address).path}" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint.errored + if (parentID != null) { + traceId traceID + parentId parentID + } else { + parent() + } + tags { + "servlet.context" "/$CONTEXT" + "span.origin.type" TestServlet2.Sync.name + + defaultTags(true) + "$Tags.COMPONENT.key" serverDecorator.component() + if (endpoint.errored) { + "$Tags.ERROR.key" endpoint.errored + "error.msg" { it == null || it == EXCEPTION.body } + "error.type" { it == null || it == Exception.name } + "error.stack" { it == null || it instanceof String } } + "$Tags.HTTP_STATUS.key" endpoint.status + "$Tags.HTTP_URL.key" "${endpoint.resolve(address)}" + "$Tags.PEER_HOSTNAME.key" "localhost" + // No peer port + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.HTTP_METHOD.key" method + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER } } - - where: - path | expectedResponse - "sync" | "Hello Sync" } /** @@ -204,26 +123,26 @@ class JettyServlet2Test extends AgentTestRunner { * @param jettyServer server to attach login service * @return SecurityHandler that can be assigned to servlet */ - private ConstraintSecurityHandler setupAuthentication(Server jettyServer) { - ConstraintSecurityHandler security = new ConstraintSecurityHandler() - - Constraint constraint = new Constraint() - constraint.setName("auth") - constraint.setAuthenticate(true) - constraint.setRoles("role") - - ConstraintMapping mapping = new ConstraintMapping() - mapping.setPathSpec("/auth/*") - mapping.setConstraint(constraint) - - security.setConstraintMappings(mapping) - security.setAuthenticator(new BasicAuthenticator()) - - LoginService loginService = new HashLoginService("TestRealm", - "src/test/resources/realm.properties") - security.setLoginService(loginService) - jettyServer.addBean(loginService) - - security - } +// private ConstraintSecurityHandler setupAuthentication(Server jettyServer) { +// ConstraintSecurityHandler security = new ConstraintSecurityHandler() +// +// Constraint constraint = new Constraint() +// constraint.setName("auth") +// constraint.setAuthenticate(true) +// constraint.setRoles("role") +// +// ConstraintMapping mapping = new ConstraintMapping() +// mapping.setPathSpec("/auth/*") +// mapping.setConstraint(constraint) +// +// security.setConstraintMappings(mapping) +// security.setAuthenticator(new BasicAuthenticator()) +// +// LoginService loginService = new HashLoginService("TestRealm", +// "src/test/resources/realm.properties") +// security.setLoginService(loginService) +// jettyServer.addBean(loginService) +// +// security +// } } diff --git a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/ServletTestInstrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/ServletTestInstrumentation.java new file mode 100644 index 0000000000..05b6073c54 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/ServletTestInstrumentation.java @@ -0,0 +1,28 @@ +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.test.base.HttpServerTestAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import net.bytebuddy.agent.builder.AgentBuilder; + +@AutoService(Instrumenter.class) +public class ServletTestInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + // Jetty 7.0 + .type(named("org.eclipse.jetty.server.HttpConnection")) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + named("handleRequest"), HttpServerTestAdvice.ServerEntryAdvice.class.getName())) + // Jetty 7.latest + .type(named("org.eclipse.jetty.server.AbstractHttpConnection")) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + named("headerComplete"), + HttpServerTestAdvice.ServerEntryAdvice.class.getName())); + } +} diff --git a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/TestServlet2.groovy b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/TestServlet2.groovy index 9ddcc8d92b..45691654cc 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/TestServlet2.groovy +++ b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/TestServlet2.groovy @@ -1,21 +1,37 @@ +import datadog.trace.agent.test.base.HttpServerTest import groovy.servlet.AbstractHttpServlet - import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + class TestServlet2 { static class Sync extends AbstractHttpServlet { @Override - void doGet(HttpServletRequest req, HttpServletResponse resp) { - if (req.getParameter("error") != null) { - throw new RuntimeException("some sync error") + protected void service(HttpServletRequest req, HttpServletResponse resp) { + req.getRequestDispatcher() + HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath) + HttpServerTest.controller(endpoint) { + resp.contentType = "text/plain" + switch (endpoint) { + case SUCCESS: + resp.status = endpoint.status + resp.writer.print(endpoint.body) + break + case REDIRECT: + resp.sendRedirect(endpoint.body) + break + case ERROR: + resp.sendError(endpoint.status, endpoint.body) + break + case EXCEPTION: + throw new Exception(endpoint.body) + } } - if (req.getParameter("non-throwing-error") != null) { - resp.sendError(500, "some sync error") - return - } - resp.writer.print("Hello Sync") } } } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy index 92150f16bf..7908912522 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy @@ -43,7 +43,7 @@ abstract class JettyServlet3Test extends AbstractServlet3Test { + + @Shared + Vertx vertx = Vertx.vertx(new VertxOptions()) + @Shared + WebClient client = WebClient.create(vertx) + @Shared + CircuitBreaker breaker = CircuitBreaker.create("my-circuit-breaker", vertx, + new CircuitBreakerOptions() + .setTimeout(-1) // Disable the timeout otherwise it makes each test take this long. + ) + + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + def request = client.request(HttpMethod.valueOf(method), uri.port, uri.host, "$uri") + headers.each { request.putHeader(it.key, it.value) } + Future result = breaker.execute { command -> + request.rxSend().doOnSuccess { + command.complete(it) + }.doOnError { + command.fail(it) + }.subscribe() + } + + def future = new CompletableFuture() + result.setHandler { + callback?.call() + if (it.succeeded()) { + future.complete(it.result().statusCode()) + } else { + future.completeExceptionally(it.cause()) + } + } + return future.get() + } + + @Override + NettyHttpClientDecorator decorator() { + return NettyHttpClientDecorator.DECORATE + } + + @Override + String expectedOperationName() { + return "netty.client.request" + } + + @Override + boolean testRedirects() { + false + } + + @Override + boolean testConnectionFailure() { + false + } +} diff --git a/dd-java-agent/instrumentation/vertx/src/test/groovy/VertxRxWebClientTest.groovy b/dd-java-agent/instrumentation/vertx/src/test/groovy/client/VertxRxWebClientTest.groovy similarity index 98% rename from dd-java-agent/instrumentation/vertx/src/test/groovy/VertxRxWebClientTest.groovy rename to dd-java-agent/instrumentation/vertx/src/test/groovy/client/VertxRxWebClientTest.groovy index fc556865c4..c39456a82b 100644 --- a/dd-java-agent/instrumentation/vertx/src/test/groovy/VertxRxWebClientTest.groovy +++ b/dd-java-agent/instrumentation/vertx/src/test/groovy/client/VertxRxWebClientTest.groovy @@ -1,3 +1,5 @@ +package client + import datadog.trace.agent.test.base.HttpClientTest import datadog.trace.instrumentation.netty41.client.NettyHttpClientDecorator import io.vertx.core.VertxOptions diff --git a/dd-java-agent/instrumentation/vertx/src/test/groovy/server/NettyServerTestInstrumentation.java b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/NettyServerTestInstrumentation.java new file mode 100644 index 0000000000..61cb150a54 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/NettyServerTestInstrumentation.java @@ -0,0 +1,22 @@ +package server; + +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.test.base.HttpServerTestAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import net.bytebuddy.agent.builder.AgentBuilder; + +@AutoService(Instrumenter.class) +public class NettyServerTestInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(named("io.netty.handler.codec.ByteToMessageDecoder")) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + named("channelRead"), HttpServerTestAdvice.ServerEntryAdvice.class.getName())); + } +} diff --git a/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxHttpServerTest.groovy b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxHttpServerTest.groovy new file mode 100644 index 0000000000..f4f753093b --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxHttpServerTest.groovy @@ -0,0 +1,128 @@ +package server + +import datadog.trace.agent.test.asserts.ListWriterAssert +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator +import groovy.transform.stc.ClosureParams +import groovy.transform.stc.SimpleType +import io.vertx.core.AbstractVerticle +import io.vertx.core.DeploymentOptions +import io.vertx.core.Future +import io.vertx.core.Vertx +import io.vertx.core.VertxOptions +import io.vertx.core.json.JsonObject +import io.vertx.ext.web.Router +import spock.lang.Shared + +import java.util.concurrent.CompletableFuture + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class VertxHttpServerTest extends HttpServerTest { + public static final String CONFIG_HTTP_SERVER_PORT = "http.server.port" + + @Shared + Vertx server + + @Override + void startServer(int port) { + server = Vertx.vertx(new VertxOptions() + // Useful for debugging: + // .setBlockedThreadCheckInterval(Integer.MAX_VALUE) + .setClusterPort(port)) + final CompletableFuture future = new CompletableFuture<>() + server.deployVerticle(verticle().name, + new DeploymentOptions() + .setConfig(new JsonObject().put(CONFIG_HTTP_SERVER_PORT, port)) + .setInstances(3)) { res -> + if (!res.succeeded()) { + throw new RuntimeException("Cannot deploy server Verticle", res.cause()) + } + future.complete(null) + } + + future.get() + } + + protected Class verticle() { + return VertxWebTestServer + } + + @Override + void stopServer() { + server.close() + } + + @Override + NettyHttpServerDecorator decorator() { + return NettyHttpServerDecorator.DECORATE + } + + @Override + String expectedOperationName() { + "netty.request" + } + + @Override + boolean testExceptionBody() { + false + } + + static class VertxWebTestServer extends AbstractVerticle { + + @Override + void start(final Future startFuture) { + final int port = config().getInteger(CONFIG_HTTP_SERVER_PORT) + final Router router = Router.router(vertx) + + router.route(SUCCESS.path).handler { ctx -> + controller(SUCCESS) { + ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) + } + } + router.route(REDIRECT.path).handler { ctx -> + controller(REDIRECT) { + ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body) + } + } + router.route(ERROR.path).handler { ctx -> + controller(ERROR) { + ctx.response().setStatusCode(ERROR.status).end(ERROR.body) + } + } + router.route(EXCEPTION.path).handler { ctx -> + controller(EXCEPTION) { + throw new Exception(EXCEPTION.body) + } + } + + vertx.createHttpServer() + .requestHandler { router.accept(it) } + .listen(port) { startFuture.complete() } + } + } + + void cleanAndAssertTraces( + final int size, + @ClosureParams(value = SimpleType, options = "datadog.trace.agent.test.asserts.ListWriterAssert") + @DelegatesTo(value = ListWriterAssert, strategy = Closure.DELEGATE_FIRST) + final Closure spec) { + // If this is failing, make sure HttpServerTestAdvice is applied correctly. + TEST_WRITER.waitForTraces(size * 2) + + // Netty closes the parent span before the controller returns, so we need to manually reorder it. + TEST_WRITER.each { + def controllerSpan = it.find { + it.operationName == "controller" + } + if (controllerSpan) { + it.remove(controllerSpan) + it.add(controllerSpan) + } + } + super.cleanAndAssertTraces(size, spec) + } +} diff --git a/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy new file mode 100644 index 0000000000..56bbbbce2a --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy @@ -0,0 +1,98 @@ +package server + +import datadog.trace.agent.test.base.HttpServerTest +import io.vertx.circuitbreaker.CircuitBreakerOptions +import io.vertx.reactivex.circuitbreaker.CircuitBreaker +import io.vertx.reactivex.core.AbstractVerticle +import io.vertx.reactivex.ext.web.Router + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class VertxRxCircuitBreakerHttpServerTest extends VertxHttpServerTest { + + @Override + protected Class verticle() { + return VertxRxCircuitBreakerWebTestServer + } + + static class VertxRxCircuitBreakerWebTestServer extends AbstractVerticle { + + @Override + void start(final io.vertx.core.Future startFuture) { + final int port = config().getInteger(CONFIG_HTTP_SERVER_PORT) + final Router router = Router.router(super.@vertx) + final CircuitBreaker breaker = + CircuitBreaker.create( + "my-circuit-breaker", + super.@vertx, + new CircuitBreakerOptions() + .setTimeout(-1) // Disable the timeout otherwise it makes each test take this long. + ) + + router.route(SUCCESS.path).handler { ctx -> + def result = breaker.execute { future -> + future.complete(SUCCESS) + } + result.setHandler { + if (it.failed()) { + throw it.cause() + } + HttpServerTest.ServerEndpoint endpoint = it.result() + controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) + } + } + } + router.route(REDIRECT.path).handler { ctx -> + def result = breaker.execute { future -> + future.complete(REDIRECT) + } + result.setHandler { + if (it.failed()) { + throw it.cause() + } + HttpServerTest.ServerEndpoint endpoint = it.result() + controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body) + } + } + } + router.route(ERROR.path).handler { ctx -> + def result = breaker.execute { future -> + future.complete(ERROR) + } + result.setHandler { + if (it.failed()) { + throw it.cause() + } + HttpServerTest.ServerEndpoint endpoint = it.result() + controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) + } + } + } + router.route(EXCEPTION.path).handler { ctx -> + def result = breaker.execute { future -> + future.fail(new Exception(EXCEPTION.body)) + } + result.setHandler { + try { + def cause = it.cause() + controller(EXCEPTION) { + throw cause + } + } catch (Exception ex) { + ctx.response().setStatusCode(EXCEPTION.status).end(ex.message) + } + } + } + + super.@vertx.createHttpServer() + .requestHandler { router.accept(it) } + .listen(port) { startFuture.complete() } + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxHttpServerTest.groovy b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxHttpServerTest.groovy new file mode 100644 index 0000000000..a271ae0e93 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxHttpServerTest.groovy @@ -0,0 +1,53 @@ +package server + + +import io.vertx.core.Future +import io.vertx.reactivex.core.AbstractVerticle +import io.vertx.reactivex.ext.web.Router + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class VertxRxHttpServerTest extends VertxHttpServerTest { + + @Override + protected Class verticle() { + return VertxRxWebTestServer + } + + static class VertxRxWebTestServer extends AbstractVerticle { + + @Override + void start(final Future startFuture) { + final int port = config().getInteger(CONFIG_HTTP_SERVER_PORT) + final Router router = Router.router(super.@vertx) + + router.route(SUCCESS.path).handler { ctx -> + controller(SUCCESS) { + ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) + } + } + router.route(REDIRECT.path).handler { ctx -> + controller(REDIRECT) { + ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body) + } + } + router.route(ERROR.path).handler { ctx -> + controller(ERROR) { + ctx.response().setStatusCode(ERROR.status).end(ERROR.body) + } + } + router.route(EXCEPTION.path).handler { ctx -> + controller(EXCEPTION) { + throw new Exception(EXCEPTION.body) + } + } + + super.@vertx.createHttpServer() + .requestHandler { router.accept(it) } + .listen(port) { startFuture.complete() } + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/src/test/java/VertxRxWebTestServer.java b/dd-java-agent/instrumentation/vertx/src/test/java/VertxRxWebTestServer.java deleted file mode 100644 index 521253c268..0000000000 --- a/dd-java-agent/instrumentation/vertx/src/test/java/VertxRxWebTestServer.java +++ /dev/null @@ -1,116 +0,0 @@ -import datadog.trace.api.Trace; -import io.vertx.circuitbreaker.CircuitBreakerOptions; -import io.vertx.core.DeploymentOptions; -import io.vertx.core.Future; -import io.vertx.core.Vertx; -import io.vertx.core.VertxOptions; -import io.vertx.core.json.JsonObject; -import io.vertx.reactivex.circuitbreaker.CircuitBreaker; -import io.vertx.reactivex.core.AbstractVerticle; -import io.vertx.reactivex.core.buffer.Buffer; -import io.vertx.reactivex.ext.web.Router; -import io.vertx.reactivex.ext.web.RoutingContext; -import io.vertx.reactivex.ext.web.client.WebClient; -import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; - -public class VertxRxWebTestServer extends AbstractVerticle { - public static final String CONFIG_HTTP_SERVER_PORT = "http.server.port"; - - public static Vertx start(final int port) throws ExecutionException, InterruptedException { - /* This is highly against Vertx ideas, but our tests are synchronous - so we have to make sure server is up and running */ - final CompletableFuture future = new CompletableFuture<>(); - - final Vertx vertx = Vertx.vertx(new VertxOptions().setClusterPort(port)); - - vertx.deployVerticle( - VertxRxWebTestServer.class.getName(), - new DeploymentOptions() - .setConfig(new JsonObject().put(CONFIG_HTTP_SERVER_PORT, port)) - .setInstances(3), - res -> { - if (!res.succeeded()) { - throw new RuntimeException("Cannot deploy server Verticle", res.cause()); - } - future.complete(null); - }); - - future.get(); - - return vertx; - } - - @Override - public void start(final Future startFuture) { - // final io.vertx.reactivex.core.Vertx vertx = new io.vertx.reactivex.core.Vertx(this.vertx); - final WebClient client = WebClient.create(vertx); - - final int port = config().getInteger(CONFIG_HTTP_SERVER_PORT); - - final Router router = Router.router(vertx); - final CircuitBreaker breaker = - CircuitBreaker.create( - "my-circuit-breaker", - vertx, - new CircuitBreakerOptions() - .setMaxFailures(5) // number of failure before opening the circuit - .setTimeout(2000) // consider a failure if the operation does not succeed in time - // .setFallbackOnFailure(true) // do we call the fallback on failure - .setResetTimeout(10000) // time spent in open state before attempting to re-try - ); - - router - .route("/") - .handler( - routingContext -> { - routingContext.response().putHeader("content-type", "text/html").end("Hello World"); - }); - router - .route("/error") - .handler( - routingContext -> { - routingContext.response().setStatusCode(500).end(); - }); - router - .route("/proxy") - .handler( - routingContext -> { - breaker.execute( - ctx -> { - client - .get(port, "localhost", "/test") - .rxSendBuffer( - Optional.ofNullable(routingContext.getBody()).orElse(Buffer.buffer())) - .subscribe( - response -> { - routingContext - .response() - .setStatusCode(response.statusCode()) - .end(response.body()); - }); - }); - }); - router - .route("/test") - .handler( - routingContext -> { - tracedMethod(); - routingContext.next(); - }) - .blockingHandler(RoutingContext::next) - .handler( - routingContext -> { - routingContext.response().putHeader("content-type", "text/html").end("Hello World"); - }); - - vertx - .createHttpServer() - .requestHandler(router::accept) - .listen(port, h -> startFuture.complete()); - } - - @Trace - private void tracedMethod() {} -} diff --git a/dd-java-agent/instrumentation/vertx/src/test/java/VertxWebTestServer.java b/dd-java-agent/instrumentation/vertx/src/test/java/VertxWebTestServer.java deleted file mode 100644 index 571487c9d5..0000000000 --- a/dd-java-agent/instrumentation/vertx/src/test/java/VertxWebTestServer.java +++ /dev/null @@ -1,107 +0,0 @@ -import static datadog.trace.agent.test.AgentTestRunner.blockUntilChildSpansFinished; - -import datadog.trace.api.Trace; -import io.vertx.core.AbstractVerticle; -import io.vertx.core.DeploymentOptions; -import io.vertx.core.Future; -import io.vertx.core.Vertx; -import io.vertx.core.VertxOptions; -import io.vertx.core.buffer.Buffer; -import io.vertx.core.http.HttpClient; -import io.vertx.core.json.JsonObject; -import io.vertx.ext.web.Router; -import io.vertx.ext.web.RoutingContext; -import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; - -public class VertxWebTestServer extends AbstractVerticle { - public static final String CONFIG_HTTP_SERVER_PORT = "http.server.port"; - - public static Vertx start(final int port) throws ExecutionException, InterruptedException { - /* This is highly against Vertx ideas, but our tests are synchronous - so we have to make sure server is up and running */ - final CompletableFuture future = new CompletableFuture<>(); - - final Vertx vertx = Vertx.vertx(new VertxOptions().setClusterPort(port)); - - vertx.deployVerticle( - VertxWebTestServer.class.getName(), - new DeploymentOptions() - .setConfig(new JsonObject().put(CONFIG_HTTP_SERVER_PORT, port)) - .setInstances(3), - res -> { - if (!res.succeeded()) { - throw new RuntimeException("Cannot deploy server Verticle", res.cause()); - } - future.complete(null); - }); - - future.get(); - - return vertx; - } - - @Override - public void start(final Future startFuture) { - final HttpClient client = vertx.createHttpClient(); - - final int port = config().getInteger(CONFIG_HTTP_SERVER_PORT); - - final Router router = Router.router(vertx); - - router - .route("/") - .handler( - routingContext -> { - routingContext.response().putHeader("content-type", "text/html").end("Hello World"); - }); - router - .route("/error") - .handler( - routingContext -> { - routingContext.response().setStatusCode(500).end(); - }); - router - .route("/proxy") - .handler( - routingContext -> { - client - .get( - port, - "localhost", - "/test", - response -> { - response.bodyHandler( - buffer -> { - routingContext - .response() - .setStatusCode(response.statusCode()) - .end(buffer); - }); - blockUntilChildSpansFinished(1); - }) - .end(Optional.ofNullable(routingContext.getBody()).orElse(Buffer.buffer())); - }); - router - .route("/test") - .handler( - routingContext -> { - tracedMethod(); - routingContext.next(); - }) - .blockingHandler(RoutingContext::next) - .handler( - routingContext -> { - routingContext.response().putHeader("content-type", "text/html").end("Hello World"); - }); - - vertx - .createHttpServer() - .requestHandler(router::accept) - .listen(port, h -> startFuture.complete()); - } - - @Trace - private void tracedMethod() {} -} diff --git a/dd-java-agent/instrumentation/vertx/vertx.gradle b/dd-java-agent/instrumentation/vertx/vertx.gradle index 95a0d298c8..9c616c7821 100644 --- a/dd-java-agent/instrumentation/vertx/vertx.gradle +++ b/dd-java-agent/instrumentation/vertx/vertx.gradle @@ -5,21 +5,6 @@ ext { apply from: "${rootDir}/gradle/java.gradle" -muzzle { - pass { - group = "io.vertx" - module = "vertx-web" - versions = "[4.1.0.Final,)" - assertInverse = true - } - pass { - group = "io.netty" - module = "netty" - versions = "[4.1.0.Final,)" - assertInverse = true - } -} - apply plugin: 'org.unbroken-dome.test-sets' testSets { diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index f55b959467..ba4ae11c3d 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -48,12 +48,14 @@ abstract class HttpServerTest extends Age def setupSpec() { startServer(port) + println "Http server started at: http://localhost:$port/" } abstract void startServer(int port) def cleanupSpec() { stopServer() + println "Http server stopped at: http://localhost:$port/" } abstract void stopServer() @@ -70,11 +72,15 @@ abstract class HttpServerTest extends Age true } + boolean testExceptionBody() { + true + } + enum ServerEndpoint { SUCCESS("success", 200, "success"), + REDIRECT("redirect", 302, null), ERROR("error", 500, "controller error"), EXCEPTION("exception", 500, "controller exception"), - REDIRECT("redirect", 302, null), NOT_FOUND("notFound", 404, "not found"), AUTH_REQUIRED("authRequired", 200, null), @@ -94,6 +100,10 @@ abstract class HttpServerTest extends Age return "/$path" } + String rawPath() { + return path + } + URI resolve(URI address) { return address.resolve(path) } @@ -113,10 +123,9 @@ abstract class HttpServerTest extends Age static T controller(ServerEndpoint endpoint, Closure closure) { if (endpoint == NOT_FOUND) { - closure() - } else { - runUnderTrace("controller", closure) + return closure() } + return runUnderTrace("controller", closure) } def "test success with #count requests"() { @@ -204,7 +213,9 @@ abstract class HttpServerTest extends Age expect: response.code() == EXCEPTION.status - response.body().string() == EXCEPTION.body + if (testExceptionBody()) { + assert response.body().string() == EXCEPTION.body + } and: cleanAndAssertTraces(1) { diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTestAdvice.java b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTestAdvice.java index d41ed92c65..d6d2c22b75 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTestAdvice.java +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTestAdvice.java @@ -1,6 +1,7 @@ package datadog.trace.agent.test.base; import datadog.trace.api.DDTags; +import datadog.trace.context.TraceScope; import io.opentracing.Scope; import io.opentracing.Tracer; import io.opentracing.noop.NoopScopeManager; @@ -24,10 +25,13 @@ public abstract class HttpServerTestAdvice { if (tracer.activeSpan() != null) { return NoopScopeManager.NoopScope.INSTANCE; } else { - return tracer - .buildSpan("TEST_SPAN") - .withTag(DDTags.RESOURCE_NAME, "ServerEntry") - .startActive(true); + final Scope scope = + tracer + .buildSpan("TEST_SPAN") + .withTag(DDTags.RESOURCE_NAME, "ServerEntry") + .startActive(true); + ((TraceScope) scope).setAsyncPropagation(true); + return scope; } }