From 485d9cb6c26874663e93171c28bcb72ce182a56b Mon Sep 17 00:00:00 2001 From: Brian Devins-Suresh Date: Thu, 19 Mar 2020 16:49:38 -0400 Subject: [PATCH] Work so far --- .../play23/PlayInstrumentation.java | 2 +- .../instrumentation/play23/PlayAdvice.java | 5 ---- .../NettyServerTestInstrumentation.java | 5 ++-- .../test/groovy/server/PlayServerTest.groovy | 8 +++--- .../server/ControllerClosureAdapter.scala | 18 +++++++++++++ .../src/test/scala/server/SyncServer.scala | 25 +++++++++++++++---- .../agent/test/base/HttpServerTest.groovy | 9 ++----- .../trace/common/writer/ListWriter.java | 7 ++++++ 8 files changed, 56 insertions(+), 23 deletions(-) create mode 100644 dd-java-agent/instrumentation/play-2.3/src/test/scala/server/ControllerClosureAdapter.scala diff --git a/dd-java-agent/instrumentation/play-2.3/src/main/java/datadog/trace/instrumentation/play23/PlayInstrumentation.java b/dd-java-agent/instrumentation/play-2.3/src/main/java/datadog/trace/instrumentation/play23/PlayInstrumentation.java index a8ac6a72d5..1a7ca983e9 100644 --- a/dd-java-agent/instrumentation/play-2.3/src/main/java/datadog/trace/instrumentation/play23/PlayInstrumentation.java +++ b/dd-java-agent/instrumentation/play-2.3/src/main/java/datadog/trace/instrumentation/play23/PlayInstrumentation.java @@ -18,7 +18,7 @@ import net.bytebuddy.matcher.ElementMatcher; public final class PlayInstrumentation extends Instrumenter.Default { public PlayInstrumentation() { - super("play"); + super("play", "play-action"); } @Override diff --git a/dd-java-agent/instrumentation/play-2.3/src/main/java8/datadog/trace/instrumentation/play23/PlayAdvice.java b/dd-java-agent/instrumentation/play-2.3/src/main/java8/datadog/trace/instrumentation/play23/PlayAdvice.java index 924dbde45f..5c1de38c6b 100644 --- a/dd-java-agent/instrumentation/play-2.3/src/main/java8/datadog/trace/instrumentation/play23/PlayAdvice.java +++ b/dd-java-agent/instrumentation/play-2.3/src/main/java8/datadog/trace/instrumentation/play23/PlayAdvice.java @@ -64,9 +64,4 @@ public class PlayAdvice { // set the resource name on the upstream akka/netty span DECORATE.onRequest(rootSpan, req); } - - // Unused method for muzzle to allow only 2.4-2.5 - public static void muzzleCheck() { - play.libs.Akka.system(); - } } diff --git a/dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/NettyServerTestInstrumentation.java b/dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/NettyServerTestInstrumentation.java index 61cb150a54..b94ea96264 100644 --- a/dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/NettyServerTestInstrumentation.java +++ b/dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/NettyServerTestInstrumentation.java @@ -13,10 +13,11 @@ public class NettyServerTestInstrumentation implements Instrumenter { @Override public AgentBuilder instrument(final AgentBuilder agentBuilder) { return agentBuilder - .type(named("io.netty.handler.codec.ByteToMessageDecoder")) + .type(named("org.jboss.netty.handler.codec.http.HttpRequestDecoder")) .transform( new AgentBuilder.Transformer.ForAdvice() .advice( - named("channelRead"), HttpServerTestAdvice.ServerEntryAdvice.class.getName())); + named("createMessage"), + HttpServerTestAdvice.ServerEntryAdvice.class.getName())); } } diff --git a/dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/PlayServerTest.groovy index f6a733e58d..32aa2e83c9 100644 --- a/dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/PlayServerTest.groovy @@ -6,6 +6,7 @@ import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import datadog.trace.bootstrap.instrumentation.api.Tags +import datadog.trace.instrumentation.play23.PlayHttpServerDecorator import play.api.test.TestServer import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.* @@ -25,14 +26,15 @@ class PlayServerTest extends HttpServerTest { @Override String component() { - return "" + return "netty" } @Override String expectedOperationName() { - return "netty.request" + return "play.server" } + // We don't have instrumentation for this version of netty yet @Override boolean hasHandlerSpan() { true @@ -53,7 +55,7 @@ class PlayServerTest extends HttpServerTest { errored endpoint == ERROR || endpoint == EXCEPTION childOf(parent as DDSpan) tags { - "$Tags.COMPONENT" "" + "$Tags.COMPONENT" PlayHttpServerDecorator.DECORATE.component() "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER "$Tags.PEER_HOST_IPV4" { it == null || it == "127.0.0.1" } // Optional "$Tags.HTTP_URL" String diff --git a/dd-java-agent/instrumentation/play-2.3/src/test/scala/server/ControllerClosureAdapter.scala b/dd-java-agent/instrumentation/play-2.3/src/test/scala/server/ControllerClosureAdapter.scala new file mode 100644 index 0000000000..bf800b1c87 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.3/src/test/scala/server/ControllerClosureAdapter.scala @@ -0,0 +1,18 @@ +package server + +import groovy.lang.Closure +import play.api.mvc.Result + +import scala.concurrent.Future + +class ControllerClosureAdapter(response: Result) extends Closure[Result] { + override def call(): Result = response +} + +class BlockClosureAdapter(block: () => Result) extends Closure[Result] { + override def call(): Result = block() +} + +class AsyncControllerClosureAdapter(response: Future[Result]) extends Closure[Future[Result]] { + override def call(): Future[Result] = response +} diff --git a/dd-java-agent/instrumentation/play-2.3/src/test/scala/server/SyncServer.scala b/dd-java-agent/instrumentation/play-2.3/src/test/scala/server/SyncServer.scala index 699a900164..fad6181a13 100644 --- a/dd-java-agent/instrumentation/play-2.3/src/test/scala/server/SyncServer.scala +++ b/dd-java-agent/instrumentation/play-2.3/src/test/scala/server/SyncServer.scala @@ -1,16 +1,31 @@ package server +import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint._ import play.api.mvc.{Action, Handler, Results} import play.api.test.{FakeApplication, TestServer} object SyncServer { val routes: PartialFunction[(String, String), Handler] = { - case ("GET", "/success") => Action { request => Results.Status(SUCCESS.getStatus).apply(SUCCESS.getBody) } - case ("GET", "/redirect") => Action { request => Results.Redirect(REDIRECT.getBody, REDIRECT.getStatus) } - case ("GET", "/query") => Action { result => Results.Status(QUERY_PARAM.getStatus).apply(QUERY_PARAM.getBody) } - case ("GET", "/error-status") => Action { result => throw new RuntimeException(ERROR.getBody) } - case ("GET", "/exception") => Action { result => throw new RuntimeException(EXCEPTION.getBody) } + case ("GET", "/success") => Action { request => + HttpServerTest.controller(SUCCESS, new ControllerClosureAdapter(Results.Status(SUCCESS.getStatus).apply(SUCCESS.getBody))) + } + case ("GET", "/redirect") => Action { request => + HttpServerTest.controller(REDIRECT, new ControllerClosureAdapter(Results.Redirect(REDIRECT.getBody, REDIRECT.getStatus))) + } + case ("GET", "/query") => Action { request => + HttpServerTest.controller(QUERY_PARAM, new ControllerClosureAdapter(Results.Status(QUERY_PARAM.getStatus).apply(QUERY_PARAM.getBody))) + } + case ("GET", "/error-status") => Action { request => + HttpServerTest.controller(ERROR, new BlockClosureAdapter(() => { + throw new RuntimeException(ERROR.getBody) + })) + } + case ("GET", "/exception") => Action { request => + HttpServerTest.controller(EXCEPTION, new BlockClosureAdapter(() => { + throw new RuntimeException(EXCEPTION.getBody) + })) + } } def server(port: Int): TestServer = { 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 16990954b1..3968ce78f6 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 @@ -24,12 +24,7 @@ import spock.lang.Unroll import java.util.concurrent.atomic.AtomicBoolean import static datadog.trace.agent.test.asserts.TraceAssert.assertTrace -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.NOT_FOUND -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT -import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.* import static datadog.trace.agent.test.utils.ConfigUtils.withConfigOverride import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace @@ -533,7 +528,7 @@ abstract class HttpServerTest extends AgentTestRunner { tags { "$Tags.COMPONENT" component "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER - "$Tags.PEER_PORT" Integer + "$Tags.PEER_PORT" { it == null || it instanceof Integer } // Optional "$Tags.PEER_HOST_IPV4" { it == null || it == "127.0.0.1" } // Optional "$Tags.HTTP_URL" "${endpoint.resolve(address)}" "$Tags.HTTP_METHOD" method diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java index a92343350d..b1c8e06c68 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java @@ -39,6 +39,13 @@ public class ListWriter extends CopyOnWriteArrayList> implements Wr latches.add(latch); } if (!latch.await(20, TimeUnit.SECONDS)) { + System.out.println(this); + for (final List trace : this) { + System.out.println(trace.toString()); + for (final DDSpan span : trace) { + System.out.println(span.toString()); + } + } throw new TimeoutException( "Timeout waiting for " + number + " trace(s). ListWriter.size() == " + size()); }