From 5f8a83486a31dc37eb26e026a69f3b15c82c9301 Mon Sep 17 00:00:00 2001 From: Andrew Kent Date: Mon, 2 Apr 2018 17:46:30 -0700 Subject: [PATCH] Play instrumentation cleanup Simplify error collecting. Use slf4j to log. randomOpenPort util. --- .../play/PlayInstrumentation.java | 22 +++---------------- .../play/src/test/groovy/PlayTest.groovy | 13 ++++++----- .../datadog/trace/agent/test/TestUtils.java | 15 +++++++++++++ 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/dd-java-agent/instrumentation/play/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java b/dd-java-agent/instrumentation/play/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java index 7d1f998988..0b7b85cad0 100644 --- a/dd-java-agent/instrumentation/play/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java +++ b/dd-java-agent/instrumentation/play/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java @@ -17,8 +17,6 @@ import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.io.PrintWriter; -import java.io.StringWriter; import java.util.*; import lombok.extern.slf4j.Slf4j; import net.bytebuddy.agent.builder.AgentBuilder; @@ -175,26 +173,12 @@ public final class PlayInstrumentation extends Instrumenter.Configurable { public static void onError(final Span span, final Throwable t) { Tags.ERROR.set(span, Boolean.TRUE); - span.log(logsForException(t)); + span.log(Collections.singletonMap("error.object", t)); Tags.HTTP_STATUS.set(span, 500); } - - public static Map logsForException(Throwable throwable) { - final Map errorLogs = new HashMap<>(4); - errorLogs.put("event", Tags.ERROR.getKey()); - errorLogs.put("error.kind", throwable.getClass().getName()); - errorLogs.put("error.object", throwable); - - errorLogs.put("message", throwable.getMessage()); - - final StringWriter sw = new StringWriter(); - throwable.printStackTrace(new PrintWriter(sw)); - errorLogs.put("stack", sw.toString()); - - return errorLogs; - } } + @Slf4j public static class RequestCallback implements Function1 { private final Span span; @@ -208,7 +192,7 @@ public final class PlayInstrumentation extends Instrumenter.Configurable { try { Tags.HTTP_STATUS.set(span, result.header().status()); } catch (Throwable t) { - LoggerFactory.getLogger(RequestCallback.class).debug("error in play instrumentation", t); + log.debug("error in play instrumentation", t); } span.finish(); return result; diff --git a/dd-java-agent/instrumentation/play/src/test/groovy/PlayTest.groovy b/dd-java-agent/instrumentation/play/src/test/groovy/PlayTest.groovy index fb89ffcdf3..c45c059608 100644 --- a/dd-java-agent/instrumentation/play/src/test/groovy/PlayTest.groovy +++ b/dd-java-agent/instrumentation/play/src/test/groovy/PlayTest.groovy @@ -1,5 +1,6 @@ import datadog.opentracing.DDSpan import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TestUtils import okhttp3.OkHttpClient import okhttp3.Request import play.api.test.TestServer @@ -12,11 +13,13 @@ class PlayTest extends AgentTestRunner { System.setProperty("dd.integration.play.enabled", "true") } + @Shared + int port = TestUtils.randomOpenPort() @Shared TestServer testServer def setupSpec() { - testServer = Helpers.testServer(9080, PlayTestUtils.buildTestApp()) + testServer = Helpers.testServer(port, PlayTestUtils.buildTestApp()) testServer.start() } @@ -33,7 +36,7 @@ class PlayTest extends AgentTestRunner { setup: OkHttpClient client = new OkHttpClient.Builder().build() def request = new Request.Builder() - .url("http://localhost:9080/helloplay/spock") + .url("http://localhost:$port/helloplay/spock") .header("x-datadog-trace-id", "123") .header("x-datadog-parent-id", "456") .get() @@ -69,7 +72,7 @@ class PlayTest extends AgentTestRunner { setup: OkHttpClient client = new OkHttpClient.Builder().build() def request = new Request.Builder() - .url("http://localhost:9080/make-error") + .url("http://localhost:$port/make-error") .get() .build() def response = client.newCall(request).execute() @@ -96,7 +99,7 @@ class PlayTest extends AgentTestRunner { setup: OkHttpClient client = new OkHttpClient.Builder().build() def request = new Request.Builder() - .url("http://localhost:9080/exception") + .url("http://localhost:$port/exception") .get() .build() def response = client.newCall(request).execute() @@ -126,7 +129,7 @@ class PlayTest extends AgentTestRunner { setup: OkHttpClient client = new OkHttpClient.Builder().build() def request = new Request.Builder() - .url("http://localhost:9080/nowhere") + .url("http://localhost:$port/nowhere") .get() .build() def response = client.newCall(request).execute() diff --git a/dd-java-agent/testing/src/main/java/datadog/trace/agent/test/TestUtils.java b/dd-java-agent/testing/src/main/java/datadog/trace/agent/test/TestUtils.java index a9b94ee13b..a4b5368659 100644 --- a/dd-java-agent/testing/src/main/java/datadog/trace/agent/test/TestUtils.java +++ b/dd-java-agent/testing/src/main/java/datadog/trace/agent/test/TestUtils.java @@ -6,6 +6,7 @@ import io.opentracing.Tracer; import io.opentracing.util.GlobalTracer; import java.io.*; import java.lang.reflect.Field; +import java.net.ServerSocket; import java.net.URL; import java.util.UUID; import java.util.concurrent.Callable; @@ -160,4 +161,18 @@ public class TestUtils { jarOutputStream.write(bytes, 0, bytes.length); jarOutputStream.closeEntry(); } + + /** Open up a random, reusable port. */ + public static int randomOpenPort() { + ServerSocket socket; + try { + socket = new ServerSocket(0); + socket.setReuseAddress(true); + socket.close(); + return socket.getLocalPort(); + } catch (IOException ioe) { + ioe.printStackTrace(); + return -1; + } + } }