From a4cded9b4f2681b7d128f4eed8e59bbbc12b1933 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Sat, 18 Aug 2018 13:38:06 -0400 Subject: [PATCH 1/2] Add some tests for distributed tracing in Tomcat and Jetty --- .../test/groovy/HttpUrlConnectionTest.groovy | 4 +-- .../src/test/groovy/JettyServlet3Test.groovy | 27 ++++++++++++++----- .../src/test/groovy/TomcatServlet3Test.groovy | 26 ++++++++++++------ 3 files changed, 40 insertions(+), 17 deletions(-) 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 01d953d784..f4a9f4de2f 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 @@ -31,7 +31,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { } } - def "trace request with propagation"() { + def "trace request with propagation (useCaches: #useCaches)"() { setup: runUnderTrace("someTrace") { HttpURLConnection connection = server.address.toURL().openConnection() @@ -107,7 +107,7 @@ class HttpUrlConnectionTest extends AgentTestRunner { useCaches << [false, true] } - def "trace request without propagation"() { + def "trace request without propagation (useCaches: #useCaches)"() { setup: runUnderTrace("someTrace") { HttpURLConnection connection = server.address.toURL().openConnection() 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 912eaf1b93..b9763bb638 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 @@ -60,11 +60,15 @@ class JettyServlet3Test extends AgentTestRunner { jettyServer.destroy() } - def "test #path servlet call"() { + def "test #path servlet call (auth: #auth, distributed tracing: #distributedTracing)"() { setup: def requestBuilder = new Request.Builder() .url("http://localhost:$port/$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")) } @@ -76,12 +80,17 @@ class JettyServlet3Test extends AgentTestRunner { assertTraces(TEST_WRITER, 1) { trace(0, 1) { span(0) { + if (distributedTracing) { + traceId "123" + parentId "456" + } else { + parent() + } serviceName "unnamed-java-app" operationName "servlet.request" resourceName "GET /$path" spanType DDSpanTypes.WEB_SERVLET errored false - parent() tags { "http.url" "http://localhost:$port/$path" "http.method" "GET" @@ -100,11 +109,15 @@ class JettyServlet3Test extends AgentTestRunner { } where: - path | expectedResponse | auth | origin - "async" | "Hello Async" | false | "Async" - "sync" | "Hello Sync" | false | "Sync" - "auth/async" | "Hello Async" | true | "Async" - "auth/sync" | "Hello Sync" | true | "Sync" + path | expectedResponse | auth | origin | distributedTracing + "async" | "Hello Async" | false | "Async" | false + "sync" | "Hello Sync" | false | "Sync" | false + "auth/async" | "Hello Async" | true | "Async" | false + "auth/sync" | "Hello Sync" | true | "Sync" | false + "async" | "Hello Async" | false | "Async" | true + "sync" | "Hello Sync" | false | "Sync" | true + "auth/async" | "Hello Async" | true | "Async" | true + "auth/sync" | "Hello Sync" | true | "Sync" | true } def "servlet instrumentation clears state after async request"() { diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy index ac200e2644..a6b9125f1b 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy @@ -59,13 +59,16 @@ class TomcatServlet3Test extends AgentTestRunner { tomcatServer.destroy() } - def "test #path servlet call"() { + def "test #path servlet call (distributed tracing: #distributedTracing)"() { setup: - def request = new Request.Builder() + def requestBuilder = new Request.Builder() .url("http://localhost:$port/my-context/$path") .get() - .build() - def response = client.newCall(request).execute() + if (distributedTracing) { + requestBuilder.header("x-datadog-trace-id", "123") + requestBuilder.header("x-datadog-parent-id", "456") + } + def response = client.newCall(requestBuilder.build()).execute() expect: response.body().string().trim() == expectedResponse @@ -73,12 +76,17 @@ class TomcatServlet3Test extends AgentTestRunner { assertTraces(TEST_WRITER, 1) { trace(0, 1) { span(0) { + if (distributedTracing) { + traceId "123" + parentId "456" + } else { + parent() + } serviceName "my-context" operationName "servlet.request" resourceName "GET /my-context/$path" spanType DDSpanTypes.WEB_SERVLET errored false - parent() tags { "http.url" "http://localhost:$port/my-context/$path" "http.method" "GET" @@ -95,9 +103,11 @@ class TomcatServlet3Test extends AgentTestRunner { } where: - path | expectedResponse - "async" | "Hello Async" - "sync" | "Hello Sync" + path | expectedResponse | distributedTracing + "async" | "Hello Async" | false + "sync" | "Hello Sync" | false + "async" | "Hello Async" | true + "sync" | "Hello Sync" | true } def "test #path error servlet call"() { From a0036a76cc1de156492ea6e47e0030e8fb817ecf Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Sat, 18 Aug 2018 23:41:50 -0400 Subject: [PATCH 2/2] Add `runFinalization` to `waitForGC` --- .../java/datadog/trace/agent/test/TestUtils.java | 1 + .../java/datadog/opentracing/PendingTrace.java | 14 -------------- .../datadog/opentracing/PendingTraceTest.groovy | 3 ++- .../scopemanager/ScopeManagerTest.groovy | 6 +++--- 4 files changed, 6 insertions(+), 18 deletions(-) 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 c4d4cd9356..70ba64bad0 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 @@ -212,6 +212,7 @@ public class TestUtils { obj = null; while (ref.get() != null) { System.gc(); + System.runFinalization(); } } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java b/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java index 11e7e13557..1ef8c7d770 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java @@ -213,20 +213,6 @@ public class PendingTrace extends ConcurrentLinkedDeque { return count > 0; } - /** - * This method ensures that garbage collection takes place, unlike {@link System#gc()} - * . Useful for testing. - */ - public static void awaitGC() { - System.gc(); // For good measure. - Object obj = new Object(); - final WeakReference ref = new WeakReference<>(obj); - obj = null; - while (ref.get() != null) { - System.gc(); - } - } - static void close() { SPAN_CLEANER.close(); } diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy index 77df8ffbb1..5e3de902a0 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy @@ -1,5 +1,6 @@ package datadog.opentracing +import datadog.trace.agent.test.TestUtils import datadog.trace.common.writer.ListWriter import spock.lang.Specification import spock.lang.Subject @@ -106,7 +107,7 @@ class PendingTraceTest extends Specification { when: child = null while (!trace.clean()) { - trace.awaitGC() + TestUtils.awaitGC() } then: diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy index ab3b13fad1..170aee79ce 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy @@ -3,7 +3,7 @@ package datadog.opentracing.scopemanager import datadog.opentracing.DDSpan import datadog.opentracing.DDSpanContext import datadog.opentracing.DDTracer -import datadog.opentracing.PendingTrace +import datadog.trace.agent.test.TestUtils import datadog.trace.common.writer.ListWriter import io.opentracing.Scope import io.opentracing.Span @@ -149,7 +149,7 @@ class ScopeManagerTest extends Specification { continuation.activate() if (forceGC) { continuation = null // Continuation references also hold up traces. - PendingTrace.awaitGC() + TestUtils.awaitGC() ((DDSpanContext) scope.span().context()).trace.clean() } if (autoClose) { @@ -196,7 +196,7 @@ class ScopeManagerTest extends Specification { if (forceGC) { continuation = null // Continuation references also hold up traces. while (!((DDSpanContext) span.context()).trace.clean()) { - PendingTrace.awaitGC() + TestUtils.awaitGC() } writer.waitForTraces(1) }