diff --git a/.gitignore b/.gitignore index e71792ea50c..9604997d2b9 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ Thumbs.db *.iml *.ipr *.iws +out/ # Visual Studio Code # ###################### diff --git a/dd-java-agent-ittests/dd-java-agent-ittests.gradle b/dd-java-agent-ittests/dd-java-agent-ittests.gradle index dcc11adeb9a..916542b1eb1 100644 --- a/dd-java-agent-ittests/dd-java-agent-ittests.gradle +++ b/dd-java-agent-ittests/dd-java-agent-ittests.gradle @@ -1,4 +1,6 @@ apply from: "${rootDir}/gradle/java.gradle" +// TODO: Test scala in separate project +apply plugin: 'scala' description = 'dd-java-agent-ittests' @@ -17,8 +19,12 @@ if (JavaVersion.current() != JavaVersion.VERSION_1_8) { } dependencies { - testCompile project(':dd-trace-api') - testCompile project(':dd-trace-ot') + compile project(':dd-trace-api') + compile project(':dd-trace-ot') + + // calling scala classes in spock requires an explicit dependency, + // hence the compile instead of testCompile + compile group: 'org.scala-lang', name: 'scala-library', version: '2.11.12' testCompile deps.opentracingMock testCompile deps.testLogging @@ -45,6 +51,7 @@ test { doFirst { // Defining here to allow jacoco to be first on the command line. jvmArgs "-javaagent:${project(':dd-java-agent').tasks.shadowJar.archivePath}" + jvmArgs "-Ddd.integration.java_concurrent.enabled=true" } testLogging { diff --git a/dd-java-agent-ittests/src/main/scala/datadog/trace/agent/integration/executors/ScalaConcurrentTests.scala b/dd-java-agent-ittests/src/main/scala/datadog/trace/agent/integration/executors/ScalaConcurrentTests.scala new file mode 100644 index 00000000000..827f2989c39 --- /dev/null +++ b/dd-java-agent-ittests/src/main/scala/datadog/trace/agent/integration/executors/ScalaConcurrentTests.scala @@ -0,0 +1,131 @@ +package datadog.trace.agent.integration.executors + +import datadog.trace.api.Trace +import io.opentracing.util.GlobalTracer + +import scala.concurrent.duration._ +import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.{Await, Future, Promise} + +class ScalaConcurrentTests { + + /** + * @return Number of expected spans in the trace + */ + @Trace + def traceWithFutureAndCallbacks() : Integer = { + val goodFuture: Future[Integer] = Future { + tracedChild("goodFuture") + 1 + } + goodFuture onSuccess { + case _ => tracedChild("successCallback") + } + val badFuture: Future[Integer] = Future { + tracedChild("badFuture") + throw new RuntimeException("Uh-oh") + } + badFuture onFailure { + case t: Throwable => tracedChild("failureCallback") + } + + return 5 + } + + @Trace + def tracedAcrossThreadsWithNoTrace() :Integer = { + val goodFuture: Future[Integer] = Future { + 1 + } + goodFuture onSuccess { + case _ => Future { + 2 + } onSuccess { + case _ => tracedChild("callback") + } + } + + return 2 + } + + /** + * @return Number of expected spans in the trace + */ + @Trace + def traceWithPromises() : Integer = { + val keptPromise = Promise[Boolean]() + val brokenPromise = Promise[Boolean]() + val afterPromise = keptPromise.future + val afterPromise2 = keptPromise.future + + val failedAfterPromise = brokenPromise.future + + Future { + tracedChild("future1") + keptPromise success true + brokenPromise failure new IllegalStateException() + } + + afterPromise onSuccess { + case b => tracedChild("keptPromise") + } + afterPromise2 onSuccess { + case b => tracedChild("keptPromise2") + } + + failedAfterPromise onFailure { + case t => tracedChild("brokenPromise") + } + + return 5 + } + + /** + * @return Number of expected spans in the trace + */ + @Trace + def tracedWithFutureFirstCompletions() :Integer = { + val completedVal = Future.firstCompletedOf( + List( + Future { + tracedChild("timeout1") + false + }, + Future { + tracedChild("timeout2") + false + }, + Future { + tracedChild("timeout3") + true + })) + Await.result(completedVal, 30 seconds) + return 4 + } + + /** + * @return Number of expected spans in the trace + */ + @Trace + def tracedTimeout(): Integer = { + val f: Future[String] = Future { + tracedChild("timeoutChild") + while(true) { + // never actually finish + } + "done" + } + + try { + Await.result(f, 1 milliseconds) + } catch { + case e: Exception => {} + } + return 2 + } + + @Trace + def tracedChild(opName: String): Unit = { + GlobalTracer.get().activeSpan().setOperationName(opName) + } +} diff --git a/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/DDInfoTest.groovy b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/DDInfoTest.groovy new file mode 100644 index 00000000000..3a571d3bc7f --- /dev/null +++ b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/DDInfoTest.groovy @@ -0,0 +1,31 @@ +package datadog.trace.agent + +import datadog.opentracing.DDTraceOTInfo +import datadog.trace.api.DDTraceApiInfo + +class DDInfoTest { + def "info accessible from api"() { + expect: + DDTraceApiInfo.VERSION == DDTraceOTInfo.VERSION + + DDTraceApiInfo.VERSION != null + DDTraceApiInfo.VERSION != "" + DDTraceApiInfo.VERSION != "unknown" + DDTraceOTInfo.VERSION != null + DDTraceOTInfo.VERSION != "" + DDTraceOTInfo.VERSION != "unknown" + } + + def "info accessible from agent"() { + setup: + def clazz = Class.forName("datadog.trace.agent.tooling.DDJavaAgentInfo") + def versionField = clazz.getDeclaredField("VERSION") + def version = versionField.get(null) + + expect: + version != null + version != "" + version != "unknown" + version == DDTraceApiInfo.VERSION + } +} diff --git a/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/executors/ExecutorInstrumentationTest.groovy b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/executors/ExecutorInstrumentationTest.groovy new file mode 100644 index 00000000000..f6acad1041d --- /dev/null +++ b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/executors/ExecutorInstrumentationTest.groovy @@ -0,0 +1,197 @@ +package datadog.trace.agent.integration.executors + +import datadog.opentracing.DDSpan +import datadog.opentracing.DDTracer +import datadog.trace.agent.test.IntegrationTestUtils +import datadog.trace.api.Trace +import datadog.trace.common.writer.ListWriter +import spock.lang.Shared +import spock.lang.Specification +import spock.lang.Unroll + +import java.lang.reflect.Method +import java.util.concurrent.ArrayBlockingQueue +import java.util.concurrent.Callable +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executor +import java.util.concurrent.ForkJoinPool +import java.util.concurrent.Future +import java.util.concurrent.RejectedExecutionException +import java.util.concurrent.ScheduledThreadPoolExecutor +import java.util.concurrent.ThreadPoolExecutor +import java.util.concurrent.TimeUnit + +class ExecutorInstrumentationTest extends Specification { + @Shared + ListWriter testWriter = new ListWriter() + @Shared + DDTracer tracer = new DDTracer(testWriter) + @Shared + Method submitMethod + @Shared + Method executeMethod + + def setupSpec() { + IntegrationTestUtils.registerOrReplaceGlobalTracer(tracer) + testWriter.start() + + executeMethod = Executor.getMethod("execute", Runnable) + submitMethod = ExecutorService.getMethod("submit", Callable) + } + + def setup() { + getTestWriter().close() + } + + @Unroll + // more useful name breaks java9 javac + // def "#poolImpl.getClass().getSimpleName() #method.getName() propagates"() + def "#poolImpl #method propagates"() { + setup: + def pool = poolImpl + def m = method + + new Runnable(){ + @Override + @Trace(operationName = "parent") + void run() { + // this child will have a span + m.invoke(pool, new AsyncChild()) + // this child won't + m.invoke(pool, new AsyncChild(false, false)) + } + }.run() + + testWriter.waitForTraces(1) + List trace = testWriter.get(0) + + expect: + testWriter.size() == 1 + trace.size() == 2 + trace.get(0).operationName == "parent" + trace.get(1).operationName == "asyncChild" + trace.get(1).parentId == trace.get(0).spanId + + cleanup: + pool?.shutdown() + + // Unfortunately, there's no simple way to test the cross product of methods/pools. + where: + poolImpl | method + new ForkJoinPool() | submitMethod + new ForkJoinPool() | executeMethod + new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) | submitMethod + new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) | executeMethod + new ScheduledThreadPoolExecutor(1) | submitMethod + new ScheduledThreadPoolExecutor(1) | executeMethod + } + + @Unroll + // more useful name breaks java9 javac + // def "#poolImpl.getClass().getSimpleName() #method.getName() propagates"() + def "#poolImpl reports after canceled jobs" () { + setup: + def pool = poolImpl + final AsyncChild child = new AsyncChild(true, true) + List jobFutures = new ArrayList() + + new Runnable(){ + @Override + @Trace(operationName = "parent") + void run() { + try { + for (int i = 0; i < 20; ++ i) { + Future f = pool.submit((Callable)child) + jobFutures.add(f) + } + } catch (RejectedExecutionException e) { + } + + for (Future f : jobFutures) { + f.cancel(false) + } + child.unblock() + } + }.run() + + testWriter.waitForTraces(1) + + expect: + testWriter.size() == 1 + + where: + poolImpl | _ + new ForkJoinPool() | _ + new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) | _ + new ScheduledThreadPoolExecutor(1) | _ + } + + def "scala futures and callbacks"() { + setup: + ScalaConcurrentTests scalaTest = new ScalaConcurrentTests() + int expectedNumberOfSpans = scalaTest.traceWithFutureAndCallbacks() + testWriter.waitForTraces(1) + List trace = testWriter.get(0) + + expect: + trace.size() == expectedNumberOfSpans + trace[0].operationName == "ScalaConcurrentTests.traceWithFutureAndCallbacks" + findSpan(trace, "goodFuture").context().getParentId() == trace[0].context().getSpanId() + findSpan(trace, "badFuture").context().getParentId() == trace[0].context().getSpanId() + findSpan(trace, "successCallback").context().getParentId() == trace[0].context().getSpanId() + findSpan(trace, "failureCallback").context().getParentId() == trace[0].context().getSpanId() + } + + def "scala propagates across futures with no traces"() { + setup: + ScalaConcurrentTests scalaTest = new ScalaConcurrentTests() + int expectedNumberOfSpans = scalaTest.tracedAcrossThreadsWithNoTrace() + testWriter.waitForTraces(1) + List trace = testWriter.get(0) + + expect: + trace.size() == expectedNumberOfSpans + trace[0].operationName == "ScalaConcurrentTests.tracedAcrossThreadsWithNoTrace" + findSpan(trace, "callback").context().getParentId() == trace[0].context().getSpanId() + } + + def "scala either promise completion"() { + setup: + ScalaConcurrentTests scalaTest = new ScalaConcurrentTests() + int expectedNumberOfSpans = scalaTest.traceWithPromises() + testWriter.waitForTraces(1) + List trace = testWriter.get(0) + + expect: + testWriter.size() == 1 + trace.size() == expectedNumberOfSpans + trace[0].operationName == "ScalaConcurrentTests.traceWithPromises" + findSpan(trace, "keptPromise").context().getParentId() == trace[0].context().getSpanId() + findSpan(trace, "keptPromise2").context().getParentId() == trace[0].context().getSpanId() + findSpan(trace, "brokenPromise").context().getParentId() == trace[0].context().getSpanId() + } + + def "scala first completed future"() { + setup: + ScalaConcurrentTests scalaTest = new ScalaConcurrentTests() + int expectedNumberOfSpans = scalaTest.tracedWithFutureFirstCompletions() + testWriter.waitForTraces(1) + List trace = testWriter.get(0) + + expect: + testWriter.size() == 1 + trace.size() == expectedNumberOfSpans + findSpan(trace, "timeout1").context().getParentId() == trace[0].context().getSpanId() + findSpan(trace, "timeout2").context().getParentId() == trace[0].context().getSpanId() + findSpan(trace, "timeout3").context().getParentId() == trace[0].context().getSpanId() + } + + private DDSpan findSpan(List trace, String opName) { + for (DDSpan span : trace) { + if (span.getOperationName() == opName) { + return span + } + } + return null + } +} diff --git a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/instrumentation/annotation/TraceAnnotationsTest.java b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/instrumentation/annotation/TraceAnnotationsTest.java index 81f14ea56d5..87097047b32 100644 --- a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/instrumentation/annotation/TraceAnnotationsTest.java +++ b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/instrumentation/annotation/TraceAnnotationsTest.java @@ -49,13 +49,13 @@ public void testComplexCaseAnnotations() { assertThat(writer.firstTrace().get(0).context().getParentId()).isEqualTo(0); assertThat(writer.firstTrace().get(0).getServiceName()).isEqualTo("test2"); - assertThat(writer.firstTrace().get(1).getOperationName()).isEqualTo("SayTracedHello.sayHello"); - assertThat(writer.firstTrace().get(1).getServiceName()).isEqualTo("test"); + assertThat(writer.firstTrace().get(1).getOperationName()).isEqualTo("SAY_HA"); assertThat(writer.firstTrace().get(1).getParentId()).isEqualTo(parentId); + assertThat(writer.firstTrace().get(1).context().getSpanType()).isEqualTo("DB"); - assertThat(writer.firstTrace().get(2).getOperationName()).isEqualTo("SAY_HA"); + assertThat(writer.firstTrace().get(2).getOperationName()).isEqualTo("SayTracedHello.sayHello"); + assertThat(writer.firstTrace().get(2).getServiceName()).isEqualTo("test"); assertThat(writer.firstTrace().get(2).getParentId()).isEqualTo(parentId); - assertThat(writer.firstTrace().get(2).context().getSpanType()).isEqualTo("DB"); } @Test diff --git a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/executors/AsyncChild.java b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/executors/AsyncChild.java new file mode 100644 index 00000000000..8cb2a6d0d1b --- /dev/null +++ b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/integration/executors/AsyncChild.java @@ -0,0 +1,53 @@ +package datadog.trace.agent.integration.executors; + +import datadog.trace.api.Trace; +import java.util.concurrent.Callable; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +public class AsyncChild implements Runnable, Callable { + private final AtomicBoolean blockThread; + private final boolean doTraceableWork; + private final AtomicInteger numberOfWorkers = new AtomicInteger(0); + + public AsyncChild() { + this(true, false); + } + + public AsyncChild(boolean doTraceableWork, boolean blockThread) { + this.doTraceableWork = doTraceableWork; + this.blockThread = new AtomicBoolean(blockThread); + } + + public void unblock() { + blockThread.set(false); + } + + @Override + public void run() { + runImpl(); + } + + @Override + public Object call() throws Exception { + runImpl(); + return null; + } + + private void runImpl() { + if (doTraceableWork) { + asyncChild(); + } + numberOfWorkers.getAndIncrement(); + try { + while (blockThread.get()) { + // busy-wait to block thread + } + } finally { + numberOfWorkers.getAndDecrement(); + } + } + + @Trace(operationName = "asyncChild") + private void asyncChild() {} +} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index 94b54746acc..d54baf22c41 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -17,6 +17,11 @@ @Slf4j public class AgentInstaller { + private static volatile Instrumentation INSTRUMENTATION; + + public static Instrumentation getInstrumentation() { + return INSTRUMENTATION; + } public static ResettableClassFileTransformer installBytebuddyAgent(final Instrumentation inst) { return installBytebuddyAgent(inst, new AgentBuilder.Listener[0]); @@ -30,6 +35,7 @@ public static ResettableClassFileTransformer installBytebuddyAgent(final Instrum */ public static ResettableClassFileTransformer installBytebuddyAgent( final Instrumentation inst, final AgentBuilder.Listener... listeners) { + INSTRUMENTATION = inst; AgentBuilder agentBuilder = new AgentBuilder.Default() .disableClassFormatChanges() @@ -55,7 +61,7 @@ public static ResettableClassFileTransformer installBytebuddyAgent( } int numInstrumenters = 0; for (final Instrumenter instrumenter : ServiceLoader.load(Instrumenter.class)) { - log.debug("Loading instrumentation {}", instrumenter); + log.debug("Loading instrumentation {}", instrumenter.getClass().getName()); agentBuilder = instrumenter.instrument(agentBuilder); numInstrumenters++; } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java index 66743269ef6..b6e8d81e20b 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java @@ -13,6 +13,7 @@ public class ClassLoaderMatcher { public static final String[] BOOTSTRAP_PACKAGE_PREFIXES = { "io.opentracing", "datadog.slf4j", "datadog.trace" }; + public static final ClassLoader BOOTSTRAP_CLASSLOADER = null; /** A private constructor that must not be invoked. */ private ClassLoaderMatcher() { @@ -53,9 +54,9 @@ private SkipClassLoaderMatcher() {} @Override public boolean matches(ClassLoader target) { - if (null == target) { - // bootstrap instrumentation not supported yet. - return true; + if (target == BOOTSTRAP_CLASSLOADER) { + // Don't skip bootstrap loader + return false; } return shouldSkipClass(target) || shouldSkipInstance(target); } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java index 800816d2868..f8c77f6d91b 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java @@ -1,5 +1,8 @@ package datadog.trace.agent.tooling; +import static datadog.trace.agent.tooling.ClassLoaderMatcher.BOOTSTRAP_CLASSLOADER; + +import java.io.File; import java.io.IOException; import java.util.Arrays; import java.util.Collections; @@ -57,14 +60,14 @@ public DynamicType.Builder transform( final TypeDescription typeDescription, final ClassLoader classLoader, final JavaModule module) { - if (helperClassNames.size() > 0 && classLoader != null) { + if (helperClassNames.size() > 0) { synchronized (this) { if (!injectedClassLoaders.contains(classLoader)) { try { final Map helperMap = getHelperMap(); final Set existingClasses = new HashSet<>(); final ClassLoader systemCL = ClassLoader.getSystemClassLoader(); - if (!classLoader.equals(systemCL)) { + if (classLoader != BOOTSTRAP_CLASSLOADER && !classLoader.equals(systemCL)) { // Build a list of existing helper classes. for (final TypeDescription def : helperMap.keySet()) { final String name = def.getName(); @@ -73,8 +76,16 @@ public DynamicType.Builder transform( } } } - new ClassInjector.UsingReflection(classLoader).inject(helperMap); - if (!classLoader.equals(systemCL)) { + if (classLoader == BOOTSTRAP_CLASSLOADER) { + ClassInjector.UsingInstrumentation.of( + new File(System.getProperty("java.io.tmpdir")), + ClassInjector.UsingInstrumentation.Target.BOOTSTRAP, + AgentInstaller.getInstrumentation()) + .inject(helperMap); + } else { + new ClassInjector.UsingReflection(classLoader).inject(helperMap); + } + if (classLoader != BOOTSTRAP_CLASSLOADER && !classLoader.equals(systemCL)) { for (final TypeDescription def : helperMap.keySet()) { // Ensure we didn't add any helper classes to the system CL. final String name = def.getName(); @@ -91,7 +102,9 @@ public DynamicType.Builder transform( + ". Failed to inject helper classes into instance " + classLoader + " of type " - + classLoader.getClass().getName(), + + (classLoader == BOOTSTRAP_CLASSLOADER + ? "" + : classLoader.getClass().getName()), e); throw new RuntimeException(e); } diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatcherTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatcherTest.groovy index 7168f3cb474..4307106e80c 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatcherTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatcherTest.groovy @@ -29,6 +29,11 @@ class ClassLoaderMatcherTest extends Specification { !ClassLoaderMatcher.skipClassLoader().matches(emptyLoader) } + def "does not skip bootstrap classloader"() { + expect: + !ClassLoaderMatcher.skipClassLoader().matches(null) + } + /* * A URLClassloader which only delegates java.* classes */ diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/HelperInjectionTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/HelperInjectionTest.groovy index 8ee475779a0..7b333e70d42 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/HelperInjectionTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/HelperInjectionTest.groovy @@ -1,7 +1,12 @@ package datadog.trace.agent.test +import datadog.trace.agent.tooling.AgentInstaller + +import static datadog.trace.agent.tooling.ClassLoaderMatcher.BOOTSTRAP_CLASSLOADER + import datadog.trace.agent.tooling.HelperInjector import datadog.trace.agent.tooling.Utils +import net.bytebuddy.agent.ByteBuddyAgent import spock.lang.Specification import java.lang.reflect.Method @@ -31,6 +36,26 @@ class HelperInjectionTest extends Specification { emptyLoader?.close() } + def "helpers injected on bootstrap classloader"() { + setup: + ByteBuddyAgent.install() + AgentInstaller.installBytebuddyAgent(ByteBuddyAgent.getInstrumentation()) + String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass' + HelperInjector injector = new HelperInjector(helperClassName) + URLClassLoader bootstrapChild = new URLClassLoader(new URL[0], (ClassLoader) null) + + when: + bootstrapChild.loadClass(helperClassName) + then: + thrown ClassNotFoundException + + when: + injector.transform(null, null, BOOTSTRAP_CLASSLOADER, null) + Class helperClass = bootstrapChild.loadClass(helperClassName) + then: + helperClass.getClassLoader() == BOOTSTRAP_CLASSLOADER + } + private static boolean isClassLoaded(String className, ClassLoader classLoader) { final Method findLoadedClassMethod = ClassLoader.getDeclaredMethod("findLoadedClass", String) try { diff --git a/dd-java-agent/dd-java-agent.gradle b/dd-java-agent/dd-java-agent.gradle index bc7fbae06ac..e96a1f3ce83 100644 --- a/dd-java-agent/dd-java-agent.gradle +++ b/dd-java-agent/dd-java-agent.gradle @@ -48,12 +48,6 @@ def includeShadowJar(subproject, jarname) { relocate 'java.util.logging.Logger', 'datadog.trace.agent.bootstrap.PatchLogger' if (!project.hasProperty("disableShadowRelocate") || !disableShadowRelocate) { - - // These need to be relocated to prevent conflicts in case the regular dd-trace is already on the classpath. - relocate('datadog.trace.api', 'datadog.trace.agent.api') { - // We want to ensure to not miss if a user is using the annotation. - exclude 'datadog.trace.api.Trace' - } relocate 'datadog.trace.common', 'datadog.trace.agent.common' relocate 'datadog.opentracing', 'datadog.trace.agent.ot' diff --git a/dd-java-agent/instrumentation/aws-sdk/src/test/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-sdk/src/test/groovy/AWSClientTest.groovy index 12ac772118f..6bc8f17ee1b 100644 --- a/dd-java-agent/instrumentation/aws-sdk/src/test/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-sdk/src/test/groovy/AWSClientTest.groovy @@ -17,7 +17,7 @@ import java.util.concurrent.atomic.AtomicReference import static ratpack.groovy.test.embed.GroovyEmbeddedApp.ratpack -@Timeout(5) +@Timeout(20) class AWSClientTest extends AgentTestRunner { def "request handler is hooked up with builder"() { @@ -61,7 +61,6 @@ class AWSClientTest extends AgentTestRunner { false | 1 } - @Timeout(10) def "send request with mocked back end"() { setup: def receivedHeaders = new AtomicReference() @@ -111,7 +110,7 @@ class AWSClientTest extends AgentTestRunner { tags1["component"] == "apache-httpclient" tags1["thread.name"] != null tags1["thread.id"] != null - tags1.size() == 4 + tags1.size() == 3 and: // span 1 - from aws instrumentation def span2 = trace[1] diff --git a/dd-java-agent/instrumentation/classloaders/classloaders.gradle b/dd-java-agent/instrumentation/classloaders/classloaders.gradle new file mode 100644 index 00000000000..15326191b5c --- /dev/null +++ b/dd-java-agent/instrumentation/classloaders/classloaders.gradle @@ -0,0 +1,12 @@ +apply from: "${rootDir}/gradle/java.gradle" + +dependencies { + compile project(':dd-trace-ot') + compile project(':dd-java-agent:agent-tooling') + + compile deps.bytebuddy + compile deps.opentracing + compile deps.autoservice + + testCompile project(':dd-java-agent:testing') +} diff --git a/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/CallDepthThreadLocalMap.java b/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/CallDepthThreadLocalMap.java new file mode 100644 index 00000000000..6eadaaac785 --- /dev/null +++ b/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/CallDepthThreadLocalMap.java @@ -0,0 +1,22 @@ +package datadog.trace.instrumentation.classloaders; + +import java.util.concurrent.atomic.AtomicInteger; + +public class CallDepthThreadLocalMap { + private static final ThreadLocal tls = new ThreadLocal<>(); + + public static int incrementCallDepth() { + AtomicInteger depth = tls.get(); + if (depth == null) { + depth = new AtomicInteger(0); + tls.set(depth); + return 0; + } else { + return depth.incrementAndGet(); + } + } + + public static void reset() { + tls.remove(); + } +} diff --git a/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java b/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java new file mode 100644 index 00000000000..fa2bcc39371 --- /dev/null +++ b/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java @@ -0,0 +1,73 @@ +package datadog.trace.instrumentation.classloaders; + +import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; + +import com.google.auto.service.AutoService; +import datadog.opentracing.DDTracer; +import datadog.trace.agent.tooling.DDAdvice; +import datadog.trace.agent.tooling.HelperInjector; +import datadog.trace.agent.tooling.Instrumenter; +import io.opentracing.util.GlobalTracer; +import java.lang.reflect.Field; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public final class ClassLoaderInstrumentation extends Instrumenter.Configurable { + + public ClassLoaderInstrumentation() { + super("classloader"); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public AgentBuilder apply(final AgentBuilder agentBuilder) { + return agentBuilder + .type( + failSafe(isSubTypeOf(ClassLoader.class)), + classLoaderHasClasses("io.opentracing.util.GlobalTracer")) + .transform( + new HelperInjector( + "datadog.trace.instrumentation.classloaders.CallDepthThreadLocalMap")) + .transform(DDAdvice.create().advice(isConstructor(), ClassloaderAdvice.class.getName())) + .asDecorator(); + } + + public static class ClassloaderAdvice { + + @Advice.OnMethodEnter + public static int constructorEnter() { + // We use this to make sure we only apply the exit instrumentation + // after the constructors are done calling their super constructors. + return CallDepthThreadLocalMap.incrementCallDepth(); + } + + // Not sure why, but adding suppress causes a verify error. + @Advice.OnMethodExit // (suppress = Throwable.class) + public static void constructorExit( + @Advice.This final ClassLoader cl, @Advice.Enter final int depth) { + if (depth == 0) { + CallDepthThreadLocalMap.reset(); + + try { + final Field field = GlobalTracer.class.getDeclaredField("tracer"); + field.setAccessible(true); + + final Object o = field.get(null); + if (o instanceof DDTracer) { + final DDTracer tracer = (DDTracer) o; + tracer.registerClassLoader(cl); + } + } catch (final Throwable e) { + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/classloaders/src/test/groovy/ClassLoaderInstrumentationTest.groovy b/dd-java-agent/instrumentation/classloaders/src/test/groovy/ClassLoaderInstrumentationTest.groovy new file mode 100644 index 00000000000..f43845c1ffc --- /dev/null +++ b/dd-java-agent/instrumentation/classloaders/src/test/groovy/ClassLoaderInstrumentationTest.groovy @@ -0,0 +1,53 @@ +import datadog.opentracing.DDTracer +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TestUtils + +import java.security.SecureClassLoader + +class ClassLoaderInstrumentationTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.classloader.enabled", "true") + } + + DDTracer tracer = Mock() + + def setup() { + TestUtils.registerOrReplaceGlobalTracer(tracer) + } + + def "creating classloader calls register on tracer"() { + when: + new EmptyNonDelegatingLoader() + + then: + 1 * tracer.registerClassLoader(_ as EmptyNonDelegatingLoader) + 0 * _ + } + + def "creating anonymous classloader calls register on tracer"() { + when: + new EmptyNonDelegatingLoader() {} + + then: + 1 * tracer.registerClassLoader(_ as EmptyNonDelegatingLoader) + 0 * _ + } + + def "bootstrap classloaders aren't instrumented"() { + // (Because they don't have access to GlobalTracer) + when: + new SecureClassLoader() + new SecureClassLoader(null) {} + new URLClassLoader(new URL[0], ClassLoader.systemClassLoader) + + then: + 0 * tracer.registerClassLoader(_) + 0 * _ + } + + class EmptyNonDelegatingLoader extends SecureClassLoader { + EmptyNonDelegatingLoader() { + super(null) + } + } +} diff --git a/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle b/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle new file mode 100644 index 00000000000..cc3bf64f640 --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle @@ -0,0 +1,10 @@ +apply from: "${rootDir}/gradle/java.gradle" + +dependencies { + compile project(':dd-trace-api') + compile project(':dd-java-agent:agent-tooling') + + compile deps.bytebuddy + compile deps.opentracing + compile deps.autoservice +} diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/ExecutorInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/ExecutorInstrumentation.java new file mode 100644 index 00000000000..8bc4ee7c41c --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/ExecutorInstrumentation.java @@ -0,0 +1,317 @@ +package datadog.trace.instrumentation.java.concurrent; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.nameMatches; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.DDAdvice; +import datadog.trace.agent.tooling.HelperInjector; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.util.GlobalTracer; +import java.lang.reflect.Field; +import java.util.*; +import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import lombok.extern.slf4j.Slf4j; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@Slf4j +@AutoService(Instrumenter.class) +public final class ExecutorInstrumentation extends Instrumenter.Configurable { + public static final String EXEC_NAME = "java_concurrent"; + public static final HelperInjector EXEC_HELPER_INJECTOR = + new HelperInjector( + ExecutorInstrumentation.class.getName() + "$ConcurrentUtils", + ExecutorInstrumentation.class.getName() + "$DatadogWrapper", + ExecutorInstrumentation.class.getName() + "$CallableWrapper", + ExecutorInstrumentation.class.getName() + "$RunnableWrapper"); + + /** + * Only apply executor instrumentation to whitelisted executors. In the future, this restriction + * may be lifted to include all executors. + */ + private static final Collection WHITELISTED_EXECUTORS; + + static { + final String[] whitelist = { + "java.util.concurrent.AbstractExecutorService", + "java.util.concurrent.ThreadPoolExecutor", + "java.util.concurrent.ScheduledThreadPoolExecutor", + "java.util.concurrent.ForkJoinPool", + "java.util.concurrent.Executors$FinalizableDelegatedExecutorService", + "java.util.concurrent.Executors$DelegatedExecutorService", + "javax.management.NotificationBroadcasterSupport$1", + "scala.concurrent.Future$InternalCallbackExecutor$", + "scala.concurrent.impl.ExecutionContextImpl", + "scala.concurrent.impl.ExecutionContextImpl$$anon$1", + "scala.concurrent.forkjoin.ForkJoinPool", + "scala.concurrent.impl.ExecutionContextImpl$$anon$3", + "akka.dispatch.MessageDispatcher", + "akka.dispatch.Dispatcher", + "akka.dispatch.Dispatcher$LazyExecutorServiceDelegate", + "akka.actor.ActorSystemImpl$$anon$1", + "akka.dispatch.ForkJoinExecutorConfigurator$AkkaForkJoinPool", + "akka.dispatch.forkjoin.ForkJoinPool", + "akka.dispatch.MessageDispatcher", + "akka.dispatch.Dispatcher", + "akka.dispatch.Dispatcher$LazyExecutorServiceDelegate", + "akka.actor.ActorSystemImpl$$anon$1", + "akka.dispatch.ForkJoinExecutorConfigurator$AkkaForkJoinPool", + "akka.dispatch.forkjoin.ForkJoinPool", + "akka.dispatch.BalancingDispatcher", + "akka.dispatch.ThreadPoolConfig$ThreadPoolExecutorServiceFactory$$anon$1", + "akka.dispatch.PinnedDispatcher", + "akka.dispatch.ExecutionContexts$sameThreadExecutionContext$", + "akka.dispatch.ExecutionContexts$sameThreadExecutionContext$", + "play.api.libs.streams.Execution$trampoline$" + }; + WHITELISTED_EXECUTORS = + Collections.unmodifiableSet(new HashSet(Arrays.asList(whitelist))); + } + + public ExecutorInstrumentation() { + super(EXEC_NAME); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public AgentBuilder apply(final AgentBuilder agentBuilder) { + return agentBuilder + .type(not(isInterface()).and(hasSuperType(named(Executor.class.getName())))) + .and( + new ElementMatcher() { + @Override + public boolean matches(TypeDescription target) { + final boolean whitelisted = WHITELISTED_EXECUTORS.contains(target.getName()); + if (!whitelisted) { + log.debug("Skipping executor instrumentation for {}", target.getName()); + } + return whitelisted; + } + }) + .transform(EXEC_HELPER_INJECTOR) + .transform( + DDAdvice.create() + .advice( + named("execute").and(takesArgument(0, Runnable.class)), + WrapRunnableAdvice.class.getName())) + .asDecorator() + .type(not(isInterface()).and(hasSuperType(named(ExecutorService.class.getName())))) + .transform(EXEC_HELPER_INJECTOR) + .transform( + DDAdvice.create() + .advice( + named("submit").and(takesArgument(0, Runnable.class)), + WrapRunnableAdvice.class.getName())) + .transform( + DDAdvice.create() + .advice( + named("submit").and(takesArgument(0, Callable.class)), + WrapCallableAdvice.class.getName())) + .transform( + DDAdvice.create() + .advice( + nameMatches("invoke(Any|All)$").and(takesArgument(0, Callable.class)), + WrapCallableCollectionAdvice.class.getName())) + .asDecorator(); + } + + public static class WrapRunnableAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static DatadogWrapper wrapJob( + @Advice.Argument(value = 0, readOnly = false) Runnable task) { + final Scope scope = GlobalTracer.get().scopeManager().active(); + if (scope instanceof TraceScope && task != null && !(task instanceof DatadogWrapper)) { + task = new RunnableWrapper(task, (TraceScope) scope); + return (DatadogWrapper) task; + } + return null; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void checkCancel( + @Advice.Enter final DatadogWrapper wrapper, @Advice.Thrown final Throwable throwable) { + if (null != wrapper && null != throwable) { + wrapper.cancel(); + } + } + } + + public static class WrapCallableAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static DatadogWrapper wrapJob( + @Advice.Argument(value = 0, readOnly = false) Callable task) { + final Scope scope = GlobalTracer.get().scopeManager().active(); + if (scope instanceof TraceScope && task != null && !(task instanceof DatadogWrapper)) { + task = new CallableWrapper(task, (TraceScope) scope); + return (DatadogWrapper) task; + } + return null; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void checkCancel( + @Advice.Enter final DatadogWrapper wrapper, @Advice.Thrown final Throwable throwable) { + if (null != wrapper && null != throwable) { + wrapper.cancel(); + } + } + } + + public static class WrapCallableCollectionAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Collection wrapJob( + @Advice.Argument(value = 0, readOnly = false) Collection> tasks) { + final Scope scope = GlobalTracer.get().scopeManager().active(); + if (scope instanceof TraceScope) { + Collection> wrappedTasks = new ArrayList<>(tasks.size()); + for (Callable task : tasks) { + if (task != null) { + if (!(task instanceof CallableWrapper)) { + task = new CallableWrapper(task, (TraceScope) scope); + } + wrappedTasks.add(task); + } + } + tasks = wrappedTasks; + return tasks; + } + return null; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void checkCancel( + @Advice.Enter final Collection wrappedJobs, @Advice.Thrown final Throwable throwable) { + if (null != wrappedJobs && null != throwable) { + for (Object wrapper : wrappedJobs) { + if (wrapper instanceof DatadogWrapper) { + ((DatadogWrapper) wrapper).cancel(); + } + } + } + } + } + + /** Marker interface for tasks which are wrapped to propagate the trace context. */ + @Slf4j + public abstract static class DatadogWrapper { + protected final TraceScope.Continuation continuation; + + public DatadogWrapper(TraceScope scope) { + continuation = scope.capture(true); + log.debug("created continuation {} from scope {}", continuation, scope); + } + + public void cancel() { + if (null != continuation) { + continuation.activate().close(); + log.debug("canceled continuation {}", continuation); + } + } + } + + @Slf4j + public static class RunnableWrapper extends DatadogWrapper implements Runnable { + private final Runnable delegatee; + + public RunnableWrapper(Runnable toWrap, TraceScope scope) { + super(scope); + delegatee = toWrap; + } + + @Override + public void run() { + final TraceScope context = continuation.activate(); + try { + delegatee.run(); + } finally { + context.close(); + } + } + } + + @Slf4j + public static class CallableWrapper extends DatadogWrapper implements Callable { + private final Callable delegatee; + + public CallableWrapper(Callable toWrap, TraceScope scope) { + super(scope); + delegatee = toWrap; + } + + @Override + public T call() throws Exception { + final TraceScope context = continuation.activate(); + try { + return delegatee.call(); + } finally { + context.close(); + } + } + } + + /** Utils for pulling DatadogWrapper out of Future instances. */ + public static class ConcurrentUtils { + private static Map, Field> fieldCache = new ConcurrentHashMap<>(); + private static String[] wrapperFields = {"runnable", "callable"}; + + public static boolean safeToWrap(Future f) { + return null != getDatadogWrapper(f); + } + + public static DatadogWrapper getDatadogWrapper(Future f) { + final Field field; + if (fieldCache.containsKey(f.getClass())) { + field = fieldCache.get(f.getClass()); + } else { + field = getWrapperField(f.getClass()); + fieldCache.put(f.getClass(), field); + } + + if (field != null) { + try { + field.setAccessible(true); + Object o = field.get(f); + if (o instanceof DatadogWrapper) { + return (DatadogWrapper) o; + } + } catch (IllegalArgumentException | IllegalAccessException e) { + } finally { + field.setAccessible(false); + } + } + return null; + } + + private static Field getWrapperField(Class clazz) { + Field field = null; + while (field == null && clazz != null) { + for (int i = 0; i < wrapperFields.length; ++i) { + try { + field = clazz.getDeclaredField(wrapperFields[i]); + break; + } catch (Exception e) { + } + } + clazz = clazz.getSuperclass(); + } + return field; + } + } +} diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java new file mode 100644 index 00000000000..ba99f08f6b3 --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java @@ -0,0 +1,116 @@ +package datadog.trace.instrumentation.java.concurrent; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.returns; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.DDAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.instrumentation.java.concurrent.ExecutorInstrumentation.ConcurrentUtils; +import datadog.trace.instrumentation.java.concurrent.ExecutorInstrumentation.DatadogWrapper; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.concurrent.Future; +import lombok.extern.slf4j.Slf4j; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@Slf4j +@AutoService(Instrumenter.class) +public final class FutureInstrumentation extends Instrumenter.Configurable { + + /** + * Only apply executor instrumentation to whitelisted executors. In the future, this restriction + * may be lifted to include all executors. + */ + private static final Collection WHITELISTED_FUTURES; + + static { + final String[] whitelist = { + "java.util.concurrent.ForkJoinTask", + "java.util.concurrent.CountedCompleter", + "java.util.concurrent.ForkJoinTask$AdaptedCallable", + "java.util.concurrent.ForkJoinTask$RunnableExecuteAction", + "java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask", + "java.util.concurrent.FutureTask", + "java.util.concurrent.ExecutorCompletionService$QueueingFuture", + "java.util.concurrent.RecursiveAction", + "scala.concurrent.forkjoin.ForkJoinTask", + "scala.concurrent.impl.ExecutionContextImpl$AdaptedForkJoinTask", + "scala.concurrent.forkjoin.ForkJoinTask$AdaptedRunnableAction", + "scala.concurrent.forkjoin.ForkJoinTask$AdaptedCallable", + "scala.concurrent.forkjoin.ForkJoinTask$AdaptedRunnable", + "scala.collection.parallel.AdaptiveWorkStealingForkJoinTasks$WrappedTask", + /* + "akka.dispatch.Mailbox", + "akka.dispatch.forkjoin.ForkJoinTask", + "akka.dispatch.Mailboxes$$anon$1", + "akka.dispatch.forkjoin.ForkJoinTask$AdaptedRunnableAction", + "akka.dispatch.forkjoin.ForkJoinTask$AdaptedCallable", + "akka.dispatch.forkjoin.ForkJoinTask$AdaptedRunnable", + "akka.dispatch.Dispatcher$$anon$1", + "akka.dispatch.ForkJoinExecutorConfigurator$AkkaForkJoinTask", + */ + "com.google.common.util.concurrent.SettableFuture", + "com.google.common.util.concurrent.AbstractFuture$TrustedFuture", + "com.google.common.util.concurrent.AbstractFuture" + }; + WHITELISTED_FUTURES = + Collections.unmodifiableSet(new HashSet(Arrays.asList(whitelist))); + } + + public FutureInstrumentation() { + super(ExecutorInstrumentation.EXEC_NAME); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public AgentBuilder apply(final AgentBuilder agentBuilder) { + return agentBuilder + .type(not(isInterface()).and(hasSuperType(named(Future.class.getName())))) + .and( + new ElementMatcher() { + @Override + public boolean matches(TypeDescription target) { + final boolean whitelisted = WHITELISTED_FUTURES.contains(target.getName()); + if (!whitelisted) { + log.debug("Skipping future instrumentation for {}", target.getName()); + } + return whitelisted; + } + }) + .transform(ExecutorInstrumentation.EXEC_HELPER_INJECTOR) + .transform( + DDAdvice.create() + .advice( + named("cancel").and(returns(boolean.class)), + CanceledFutureAdvice.class.getName())) + .asDecorator(); + } + + public static class CanceledFutureAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static DatadogWrapper findWrapper(@Advice.This Future future) { + return ConcurrentUtils.getDatadogWrapper(future); + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void abortTracing( + @Advice.Enter final DatadogWrapper wrapper, @Advice.Return boolean canceled) { + if (canceled && null != wrapper) { + wrapper.cancel(); + } + } + } +} diff --git a/dd-java-agent/instrumentation/jax-rs/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsInstrumentation.java b/dd-java-agent/instrumentation/jax-rs/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsInstrumentation.java index 2a6905b72dd..a4b7831c49b 100644 --- a/dd-java-agent/instrumentation/jax-rs/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsInstrumentation.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.jaxrs; import static net.bytebuddy.matcher.ElementMatchers.declaresMethod; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -38,7 +39,10 @@ protected AgentBuilder apply(final AgentBuilder agentBuilder) { .type( hasSuperType( isAnnotatedWith(named("javax.ws.rs.Path")) - .or(hasSuperType(declaresMethod(isAnnotatedWith(named("javax.ws.rs.Path"))))))) + .or( + failSafe( + hasSuperType( + declaresMethod(isAnnotatedWith(named("javax.ws.rs.Path")))))))) .transform( DDAdvice.create() .advice( diff --git a/dd-java-agent/instrumentation/jax-rs/src/test/groovy/JerseyTest.groovy b/dd-java-agent/instrumentation/jax-rs/src/test/groovy/JerseyTest.groovy index e765697fba3..83609913410 100644 --- a/dd-java-agent/instrumentation/jax-rs/src/test/groovy/JerseyTest.groovy +++ b/dd-java-agent/instrumentation/jax-rs/src/test/groovy/JerseyTest.groovy @@ -4,7 +4,7 @@ import org.junit.ClassRule import spock.lang.Shared import spock.lang.Timeout -@Timeout(1) +@Timeout(5) class JerseyTest extends AgentTestRunner { static { diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java index 4ca6122982f..74209cf2b80 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java @@ -1,10 +1,10 @@ package datadog.trace.instrumentation.jdbc; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.isConstructor; import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; -import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -28,7 +28,7 @@ public ConnectionInstrumentation() { @Override public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder - .type(not(isInterface()).and(hasSuperType(named(Connection.class.getName())))) + .type(not(isInterface()).and(failSafe(isSubTypeOf(Connection.class)))) .transform( DDAdvice.create() .advice( diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java index a01acece506..ff5ff360fee 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -1,11 +1,11 @@ package datadog.trace.instrumentation.jdbc; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; -import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -35,7 +35,7 @@ public PreparedStatementInstrumentation() { @Override public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder - .type(not(isInterface()).and(hasSuperType(named(PreparedStatement.class.getName())))) + .type(not(isInterface()).and(failSafe(isSubTypeOf(PreparedStatement.class)))) .transform( DDAdvice.create() .advice( diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java index 341076f6eec..4fcfbf7699c 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java @@ -1,11 +1,11 @@ package datadog.trace.instrumentation.jdbc; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; -import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -35,7 +35,7 @@ public StatementInstrumentation() { @Override public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder - .type(not(isInterface()).and(hasSuperType(named(Statement.class.getName())))) + .type(not(isInterface()).and(failSafe(isSubTypeOf(Statement.class)))) .transform( DDAdvice.create() .advice( diff --git a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java index 8705e277f11..6a758a4d163 100644 --- a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java @@ -3,6 +3,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -45,7 +46,7 @@ public JMS1MessageConsumerInstrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.jms.MessageConsumer"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageConsumer")))), not(classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener"))) .transform(JMS1_HELPER_INJECTOR) .transform( diff --git a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java index 68be43aa0af..b62f9505fdc 100644 --- a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java @@ -3,6 +3,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -39,7 +40,7 @@ public JMS1MessageListenerInstrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.jms.MessageListener"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageListener")))), not(classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener"))) .transform(JMS1MessageConsumerInstrumentation.JMS1_HELPER_INJECTOR) .transform( diff --git a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java index f7e82288abe..52e4d847aa1 100644 --- a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java @@ -3,6 +3,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -40,7 +41,7 @@ public JMS1MessageProducerInstrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.jms.MessageProducer"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageProducer")))), not(classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener"))) .transform(JMS1MessageConsumerInstrumentation.JMS1_HELPER_INJECTOR) .transform( diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java index 44f23328c92..9514b852fe7 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java @@ -3,6 +3,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -45,7 +46,7 @@ public JMS2MessageConsumerInstrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.jms.MessageConsumer"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageConsumer")))), classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener")) .transform(JMS2_HELPER_INJECTOR) .transform( diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java index c596bfae671..0d716e1f325 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java @@ -3,6 +3,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -39,7 +40,7 @@ public JMS2MessageListenerInstrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.jms.MessageListener"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageListener")))), classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener")) .transform(JMS2MessageConsumerInstrumentation.JMS2_HELPER_INJECTOR) .transform( diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java index 7a2e3725eda..46fab79d557 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java @@ -3,6 +3,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -40,7 +41,7 @@ public JMS2MessageProducerInstrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.jms.MessageProducer"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageProducer")))), classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener")) .transform(JMS2MessageConsumerInstrumentation.JMS2_HELPER_INJECTOR) .transform( diff --git a/dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy b/dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy index 69c0e245913..ecf21c732ae 100644 --- a/dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy +++ b/dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy @@ -25,7 +25,7 @@ import spock.lang.Timeout import java.util.concurrent.LinkedBlockingQueue import java.util.concurrent.TimeUnit -@Timeout(5) +@Timeout(15) class KafkaStreamsTest extends AgentTestRunner { static final STREAM_PENDING = "test.pending" static final STREAM_PROCESSED = "test.processed" @@ -135,41 +135,41 @@ class KafkaStreamsTest extends AgentTestRunner { and: // STREAMING span 0 def t2span1 = t2[0] - t2span1.context().operationName == "kafka.consume" + t2span1.context().operationName == "kafka.produce" t2span1.serviceName == "kafka" - t2span1.resourceName == "Consume Topic $STREAM_PENDING" + t2span1.resourceName == "Produce Topic $STREAM_PROCESSED" t2span1.type == "queue" !t2span1.context().getErrorFlag() - t2span1.context().parentId == t1span1.context().spanId def t2tags1 = t2span1.context().tags t2tags1["component"] == "java-kafka" - t2tags1["span.kind"] == "consumer" - t1tags1["span.type"] == "queue" - t2tags1["partition"] >= 0 - t2tags1["offset"] == 0 + t2tags1["span.kind"] == "producer" + t2tags1["span.type"] == "queue" t2tags1["thread.name"] != null t2tags1["thread.id"] != null - t2tags1["asdf"] == "testing" - t2tags1.size() == 8 + t2tags1.size() == 5 and: // STREAMING span 1 def t2span2 = t2[1] + t2span1.context().parentId == t2span2.context().spanId - t2span2.context().operationName == "kafka.produce" + t2span2.context().operationName == "kafka.consume" t2span2.serviceName == "kafka" - t2span2.resourceName == "Produce Topic $STREAM_PROCESSED" + t2span2.resourceName == "Consume Topic $STREAM_PENDING" t2span2.type == "queue" !t2span2.context().getErrorFlag() - t2span2.context().parentId == t2span1.context().spanId + t2span2.context().parentId == t1span1.context().spanId def t2tags2 = t2span2.context().tags t2tags2["component"] == "java-kafka" - t2tags2["span.kind"] == "producer" - t2tags2["span.type"] == "queue" + t2tags2["span.kind"] == "consumer" + t1tags1["span.type"] == "queue" + t2tags2["partition"] >= 0 + t2tags2["offset"] == 0 t2tags2["thread.name"] != null t2tags2["thread.id"] != null - t2tags2.size() == 5 + t2tags2["asdf"] == "testing" + t2tags2.size() == 8 and: // CONSUMER span 0 def t3span1 = t3[0] @@ -179,7 +179,7 @@ class KafkaStreamsTest extends AgentTestRunner { t3span1.resourceName == "Consume Topic $STREAM_PROCESSED" t3span1.type == "queue" !t3span1.context().getErrorFlag() - t3span1.context().parentId == t2span2.context().spanId + t3span1.context().parentId == t2span1.context().spanId def t3tags1 = t3span1.context().tags t3tags1["component"] == "java-kafka" @@ -194,8 +194,8 @@ class KafkaStreamsTest extends AgentTestRunner { def headers = received.headers() headers.iterator().hasNext() - new String(headers.headers("x-datadog-trace-id").iterator().next().value()) == "$t2span2.traceId" - new String(headers.headers("x-datadog-parent-id").iterator().next().value()) == "$t2span2.spanId" + new String(headers.headers("x-datadog-trace-id").iterator().next().value()) == "$t2span1.traceId" + new String(headers.headers("x-datadog-parent-id").iterator().next().value()) == "$t2span1.spanId" cleanup: diff --git a/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/DDTracingCommandListener.java b/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/DDTracingCommandListener.java index 8502f9852b2..f6ed65a1313 100644 --- a/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/DDTracingCommandListener.java +++ b/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/DDTracingCommandListener.java @@ -73,7 +73,7 @@ private Span buildSpan(final CommandStartedEvent event) { final Tracer.SpanBuilder spanBuilder = tracer.buildSpan(MONGO_OPERATION).withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT); - final Span span = spanBuilder.startManual(); + final Span span = spanBuilder.start(); try { decorate(span, event); } catch (final Throwable e) { diff --git a/dd-java-agent/instrumentation/mongo-3.1/src/test/java/datadog/trace/instrumentation/mongo/MongoClientInstrumentationTest.java b/dd-java-agent/instrumentation/mongo-3.1/src/test/java/datadog/trace/instrumentation/mongo/MongoClientInstrumentationTest.java index 3e3acefb9e1..175e0845e99 100644 --- a/dd-java-agent/instrumentation/mongo-3.1/src/test/java/datadog/trace/instrumentation/mongo/MongoClientInstrumentationTest.java +++ b/dd-java-agent/instrumentation/mongo-3.1/src/test/java/datadog/trace/instrumentation/mongo/MongoClientInstrumentationTest.java @@ -28,7 +28,7 @@ public void mongoSpan() { final CommandStartedEvent cmd = new CommandStartedEvent(1, makeConnection(), "databasename", "query", new BsonDocument()); - final DDSpan span = new DDTracer().buildSpan("foo").startManual(); + final DDSpan span = new DDTracer().buildSpan("foo").start(); DDTracingCommandListener.decorate(span, cmd); assertThat(span.context().getSpanType()).isEqualTo("mongodb"); @@ -50,7 +50,7 @@ public void queryScrubbing() { final CommandStartedEvent cmd = new CommandStartedEvent(1, makeConnection(), "databasename", "query", query); - final DDSpan span = new DDTracer().buildSpan("foo").startManual(); + final DDSpan span = new DDTracer().buildSpan("foo").start(); DDTracingCommandListener.decorate(span, cmd); assertThat(span.getTags().get(Tags.DB_STATEMENT.getKey())) 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 82ac953ff73..42aa6bda4af 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 @@ -53,7 +53,7 @@ class OkHttp3Test extends AgentTestRunner { tags1["component"] == "okhttp" tags1["thread.name"] != null tags1["thread.id"] != null - tags1.size() == 4 + tags1.size() == 3 and: // span 1 def span2 = trace[1] diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java index 4db67e831ed..842c473757e 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java @@ -2,6 +2,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -42,7 +43,7 @@ public FilterChain2Instrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.servlet.FilterChain"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.FilterChain")))), not(classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener")) .and( classLoaderHasClasses( diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java index 4b7a8e3e80a..61f829493bf 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java @@ -2,6 +2,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isProtected; @@ -42,7 +43,7 @@ public HttpServlet2Instrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.servlet.http.HttpServlet"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.http.HttpServlet")))), not(classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener")) .and( classLoaderHasClasses( diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java index bdab63fa476..94ef0c947f3 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java @@ -2,6 +2,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -47,7 +48,7 @@ public FilterChain3Instrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.servlet.FilterChain"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.FilterChain")))), classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener")) .transform( new HelperInjector( diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java index c89961473c6..9952838349c 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java @@ -2,6 +2,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isProtected; @@ -45,7 +46,7 @@ public HttpServlet3Instrumentation() { public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.servlet.http.HttpServlet"))), + not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.http.HttpServlet")))), classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener")) .transform( new HelperInjector( diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java index c01ebb06f20..5809f2375c2 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java @@ -2,6 +2,7 @@ import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithField; import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -39,7 +40,9 @@ public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( not(isInterface()) - .and(hasSuperType(named("org.springframework.web.servlet.HandlerAdapter"))), + .and( + failSafe( + hasSuperType(named("org.springframework.web.servlet.HandlerAdapter")))), classLoaderHasClassWithField( "org.springframework.web.servlet.HandlerMapping", "BEST_MATCHING_PATTERN_ATTRIBUTE")) diff --git a/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationInstrumentation.java b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationInstrumentation.java index 0e32eadbcbf..842a5ed23c6 100644 --- a/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationInstrumentation.java +++ b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationInstrumentation.java @@ -2,6 +2,7 @@ import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.declaresMethod; +import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; @@ -28,7 +29,7 @@ public TraceAnnotationInstrumentation() { @Override public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder - .type(hasSuperType(declaresMethod(isAnnotatedWith(Trace.class)))) + .type(failSafe(hasSuperType(declaresMethod(isAnnotatedWith(Trace.class))))) .transform( DDAdvice.create().advice(isAnnotatedWith(Trace.class), TraceAdvice.class.getName())) .asDecorator(); diff --git a/dd-trace-api/src/main/java/datadog/trace/api/interceptor/MutableSpan.java b/dd-trace-api/src/main/java/datadog/trace/api/interceptor/MutableSpan.java new file mode 100644 index 00000000000..6c15eb54b16 --- /dev/null +++ b/dd-trace-api/src/main/java/datadog/trace/api/interceptor/MutableSpan.java @@ -0,0 +1,37 @@ +package datadog.trace.api.interceptor; + +import java.util.Map; + +public interface MutableSpan { + String getOperationName(); + + MutableSpan setOperationName(final String serviceName); + + String getServiceName(); + + MutableSpan setServiceName(final String serviceName); + + String getResourceName(); + + MutableSpan setResourceName(final String resourceName); + + Integer getSamplingPriority(); + + MutableSpan setSamplingPriority(final int newPriority); + + String getSpanType(); + + MutableSpan setSpanType(final String type); + + Map getTags(); + + MutableSpan setTag(final String tag, final String value); + + MutableSpan setTag(final String tag, final boolean value); + + MutableSpan setTag(final String tag, final Number value); + + Boolean isError(); + + MutableSpan setError(boolean value); +} diff --git a/dd-trace-api/src/main/java/datadog/trace/api/interceptor/TraceInterceptor.java b/dd-trace-api/src/main/java/datadog/trace/api/interceptor/TraceInterceptor.java new file mode 100644 index 00000000000..822e0b9fd68 --- /dev/null +++ b/dd-trace-api/src/main/java/datadog/trace/api/interceptor/TraceInterceptor.java @@ -0,0 +1,22 @@ +package datadog.trace.api.interceptor; + +import java.util.Collection; + +public interface TraceInterceptor { + + /** + * After a trace is "complete" but before it is written, it is provided to the interceptors to + * modify. The result following all interceptors is sampled then sent to the trace writer. + * + * @param trace - The collection of spans that represent a trace. Can be modified in place. Order + * of spans should not be relied upon. + * @return A potentially modified or replaced collection of spans. Must not be null. + */ + Collection onTraceComplete(Collection trace); + + /** + * @return A unique priority for sorting relative to other TraceInterceptors. Unique because + * interceptors are stored in a sorted set, so duplicates will not be added. + */ + int priority(); +} diff --git a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java new file mode 100644 index 00000000000..1886796edc3 --- /dev/null +++ b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java @@ -0,0 +1,25 @@ +package datadog.trace.context; + +/** An object when can propagate a datadog trace across multiple threads. */ +public interface TraceScope { + /** + * Prevent the trace attached to this TraceScope from reporting until the returned Continuation + * finishes. + * + *

Should be called on the parent thread. + */ + Continuation capture(boolean finishOnClose); + + /** Close the activated context and allow any underlying spans to finish. */ + void close(); + + /** Used to pass async context between workers. */ + interface Continuation { + /** + * Activate the continuation. + * + *

Should be called on the child thread. + */ + TraceScope activate(); + } +} diff --git a/dd-trace-ot/docs/opentracing-api.md b/dd-trace-ot/docs/opentracing-api.md index 95fd45c6c82..be7a6ac098a 100644 --- a/dd-trace-ot/docs/opentracing-api.md +++ b/dd-trace-ot/docs/opentracing-api.md @@ -62,7 +62,7 @@ Sometimes you need to create a span without promoting it as the active. If you w ```java // Create a span, but do not promoting it as the active span - Span anotherSpan = tracer.buildSpan("componentTracking").startManual(); + Span anotherSpan = tracer.buildSpan("componentTracking").start(); ``` Typically, span creations are made in the beginning of the methods you want to trace. diff --git a/dd-trace-ot/src/jmh/java/datadog/trace/DDTraceBenchmark.java b/dd-trace-ot/src/jmh/java/datadog/trace/DDTraceBenchmark.java index f175c11114d..a7dc5b118a5 100644 --- a/dd-trace-ot/src/jmh/java/datadog/trace/DDTraceBenchmark.java +++ b/dd-trace-ot/src/jmh/java/datadog/trace/DDTraceBenchmark.java @@ -25,12 +25,12 @@ public Object testBuildSpan(final TraceState state) { @Benchmark public Object testBuildStartSpan(final TraceState state) { - return state.tracer.buildSpan(SPAN_NAME).startManual(); + return state.tracer.buildSpan(SPAN_NAME).start(); } @Benchmark public Object testFullSpan(final TraceState state) { - final Span span = state.tracer.buildSpan(SPAN_NAME).startManual(); + final Span span = state.tracer.buildSpan(SPAN_NAME).start(); span.finish(); return span; } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java index ebadd77606b..0baca187c71 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java @@ -7,15 +7,17 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; import datadog.trace.api.DDTags; +import datadog.trace.api.interceptor.MutableSpan; import datadog.trace.common.sampling.PrioritySampling; import datadog.trace.common.util.Clock; import io.opentracing.Span; import java.io.PrintWriter; import java.io.StringWriter; +import java.lang.ref.WeakReference; import java.util.HashMap; import java.util.Map; -import java.util.Queue; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import lombok.extern.slf4j.Slf4j; /** @@ -25,7 +27,7 @@ * according to the DD agent. */ @Slf4j -public class DDSpan implements Span { +public class DDSpan implements Span, MutableSpan { /** The context attached to the span */ private final DDSpanContext context; @@ -40,7 +42,10 @@ public class DDSpan implements Span { * The duration in nanoseconds computed using the startTimeMicro or startTimeNano. Span is * considered finished when this is set. */ - private volatile long durationNano; + private final AtomicLong durationNano = new AtomicLong(); + + /** Implementation detail. Stores the weak reference to this span. Used by TraceCollection. */ + volatile WeakReference ref; /** * Spans should be constructed using the builder, not by calling the constructor directly. @@ -61,20 +66,24 @@ public class DDSpan implements Span { // timestamp might have come from an external clock, so don't bother with nanotime. this.startTimeNano = 0; } + this.context.getTrace().registerSpan(this); + } - // track each span of the trace - this.context.getTrace().add(this); + @JsonIgnore + public boolean isFinished() { + return durationNano.get() != 0; } @Override public final void finish() { if (startTimeNano != 0) { - if (durationNano != 0) { - log.debug("Span already finished: {}", this); + // no external clock was used, so we can rely on nanotime, but still ensure a min duration of 1. + if (this.durationNano.compareAndSet( + 0, Math.max(1, Clock.currentNanoTicks() - startTimeNano))) { + context.getTrace().addSpan(this); + } else { + log.debug("{} - already finished!", this); } - // no external clock was used, so we can rely on nanotime. - this.durationNano = Math.max(1, Clock.currentNanoTicks() - startTimeNano); - afterFinish(); } else { finish(Clock.currentMicroTime()); } @@ -82,35 +91,12 @@ public final void finish() { @Override public final void finish(final long stoptimeMicros) { - if (durationNano != 0) { - log.debug("Span already finished: {}", this); - } // Ensure that duration is at least 1. Less than 1 is possible due to our use of system clock instead of nano time. - this.durationNano = - Math.max(1, TimeUnit.MICROSECONDS.toNanos(stoptimeMicros - this.startTimeMicro)); - afterFinish(); - } - - /** - * Close the span. If the current span is the parent, check if each child has also been closed If - * not, warned it - */ - protected final void afterFinish() { - log.debug("{} - Closing the span.", this); - - // warn if one of the parent's children is not finished - if (this.isRootSpan()) { - final Queue spans = this.context().getTrace(); - - for (final DDSpan span : spans) { - if (span.getDurationNano() == 0L) { - log.warn( - "{} - The parent span is marked as finished but this span isn't. You have to close each children.", - this); - } - } - this.context.getTracer().write(this.context.getTrace()); - log.debug("{} - Write the trace", this); + if (this.durationNano.compareAndSet( + 0, Math.max(1, TimeUnit.MICROSECONDS.toNanos(stoptimeMicros - this.startTimeMicro)))) { + context.getTrace().addSpan(this); + } else { + log.debug("{} - already finished!", this); } } @@ -121,18 +107,17 @@ protected final void afterFinish() { */ @JsonIgnore public final boolean isRootSpan() { + return context.getParentId() == 0; + } - if (context().getTrace().isEmpty()) { - return false; - } - // First item of the array AND tracer set - final DDSpan first = context().getTrace().peek(); - return first.context().getSpanId() == this.context().getSpanId() - && this.context.getTracer() != null; + @Override + public DDSpan setError(final boolean error) { + context.setErrorFlag(true); + return this; } public void setErrorMeta(final Throwable error) { - context.setErrorFlag(true); + setError(true); setTag(DDTags.ERROR_MSG, error.getMessage()); setTag(DDTags.ERROR_TYPE, error.getClass().getName()); @@ -155,7 +140,7 @@ private boolean extractError(final Map map) { * @see io.opentracing.BaseSpan#setTag(java.lang.String, java.lang.String) */ @Override - public final Span setTag(final String tag, final String value) { + public final DDSpan setTag(final String tag, final String value) { this.context().setTag(tag, (Object) value); return this; } @@ -164,7 +149,7 @@ public final Span setTag(final String tag, final String value) { * @see io.opentracing.BaseSpan#setTag(java.lang.String, boolean) */ @Override - public final Span setTag(final String tag, final boolean value) { + public final DDSpan setTag(final String tag, final boolean value) { this.context().setTag(tag, (Object) value); return this; } @@ -173,7 +158,7 @@ public final Span setTag(final String tag, final boolean value) { * @see io.opentracing.BaseSpan#setTag(java.lang.String, java.lang.Number) */ @Override - public final Span setTag(final String tag, final Number value) { + public final DDSpan setTag(final String tag, final Number value) { this.context().setTag(tag, (Object) value); return this; } @@ -252,11 +237,13 @@ public final DDSpan log(final long l, final String s) { return this; } + @Override public final DDSpan setServiceName(final String serviceName) { this.context().setServiceName(serviceName); return this; } + @Override public final DDSpan setResourceName(final String resourceName) { this.context().setResourceName(resourceName); return this; @@ -267,11 +254,13 @@ public final DDSpan setResourceName(final String resourceName) { * *

Has no effect if the span priority has been propagated (injected or extracted). */ + @Override public final DDSpan setSamplingPriority(final int newPriority) { this.context().setSamplingPriority(newPriority); return this; } + @Override public final DDSpan setSpanType(final String type) { this.context().setSpanType(type); return this; @@ -303,9 +292,10 @@ public long getStartTime() { @JsonGetter("duration") public long getDurationNano() { - return durationNano; + return durationNano.get(); } + @Override @JsonGetter("service") public String getServiceName() { return context.getServiceName(); @@ -326,16 +316,19 @@ public long getParentId() { return context.getParentId(); } + @Override @JsonGetter("resource") public String getResourceName() { return context.getResourceName(); } + @Override @JsonGetter("name") public String getOperationName() { return context.getOperationName(); } + @Override @JsonGetter("sampling_priority") @JsonInclude(Include.NON_NULL) public Integer getSamplingPriority() { @@ -347,6 +340,13 @@ public Integer getSamplingPriority() { } } + @Override + @JsonIgnore + public String getSpanType() { + return context.getSpanType(); + } + + @Override @JsonIgnore public Map getTags() { return this.context().getTags(); @@ -357,6 +357,12 @@ public String getType() { return context.getSpanType(); } + @Override + @JsonIgnore + public Boolean isError() { + return context.getErrorFlag(); + } + @JsonGetter public int getError() { return context.getErrorFlag() ? 1 : 0; diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java index eb37968aa1e..284ff01ee4f 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java @@ -1,18 +1,15 @@ package datadog.opentracing; import com.fasterxml.jackson.annotation.JsonIgnore; -import com.google.common.collect.Maps; import datadog.opentracing.decorators.AbstractDecorator; import datadog.trace.api.DDTags; import datadog.trace.common.sampling.PrioritySampling; import io.opentracing.tag.Tags; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Queue; import java.util.TreeMap; -import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ConcurrentHashMap; import lombok.extern.slf4j.Slf4j; /** @@ -26,37 +23,42 @@ @Slf4j public class DDSpanContext implements io.opentracing.SpanContext { - // Opentracing attributes + // Shared with other span contexts + /** For technical reasons, the ref to the original tracer */ + private final DDTracer tracer; + + /** The collection of all span related to this one */ + private final PendingTrace trace; + + /** Baggage is associated with the whole trace and shared with other spans */ + private final Map baggageItems; + + // Not Shared with other span contexts private final long traceId; private final long spanId; private final long parentId; - private final String threadName = Thread.currentThread().getName(); - private final long threadId = Thread.currentThread().getId(); - /** The collection of all span related to this one */ - private final Queue trace; - // DD attributes - /** For technical reasons, the ref to the original tracer */ - private final DDTracer tracer; + /** Tags are associated to the current span, they will not propagate to the children span */ + private final Map tags = new ConcurrentHashMap<>(); - private Map baggageItems; /** The service name is required, otherwise the span are dropped by the agent */ - private String serviceName; + private volatile String serviceName; /** The resource associated to the service (server_web, database, etc.) */ - private String resourceName; - /** True indicates that the span reports an error */ - private boolean errorFlag; - /** The type of the span. If null, the Datadog Agent will report as a custom */ - private String spanType; + private volatile String resourceName; /** Each span have an operation name describing the current span */ - private String operationName; + private volatile String operationName; + /** The type of the span. If null, the Datadog Agent will report as a custom */ + private volatile String spanType; + /** True indicates that the span reports an error */ + private volatile boolean errorFlag; /** The sampling priority of the trace */ private volatile int samplingPriority = PrioritySampling.UNSET; /** When true, the samplingPriority cannot be changed. */ private volatile boolean samplingPriorityLocked = false; - // Others attributes - /** Tags are associated to the current span, they will not propagate to the children span */ - private Map tags; + + // Additional Metadata + private final String threadName = Thread.currentThread().getName(); + private final long threadId = Thread.currentThread().getId(); public DDSpanContext( final long traceId, @@ -70,36 +72,34 @@ public DDSpanContext( final boolean errorFlag, final String spanType, final Map tags, - final Queue trace, + final PendingTrace trace, final DDTracer tracer) { + assert tracer != null; + assert trace != null; + this.tracer = tracer; + this.trace = trace; + this.traceId = traceId; this.spanId = spanId; this.parentId = parentId; if (baggageItems == null) { - this.baggageItems = Collections.emptyMap(); + this.baggageItems = new ConcurrentHashMap<>(0); } else { this.baggageItems = baggageItems; } + if (tags != null) { + this.tags.putAll(tags); + } + this.serviceName = serviceName; this.operationName = operationName; this.resourceName = resourceName; this.samplingPriority = samplingPriority; this.errorFlag = errorFlag; this.spanType = spanType; - - this.tags = tags; - - if (trace == null) { - // TODO: figure out better concurrency model. - this.trace = new ConcurrentLinkedQueue<>(); - } else { - this.trace = trace; - } - - this.tracer = tracer; } public long getTraceId() { @@ -132,6 +132,14 @@ public void setResourceName(final String resourceName) { this.resourceName = resourceName; } + public String getOperationName() { + return operationName; + } + + public void setOperationName(final String operationName) { + this.operationName = operationName; + } + public boolean getErrorFlag() { return errorFlag; } @@ -189,9 +197,6 @@ public boolean lockSamplingPriority() { } public void setBaggageItem(final String key, final String value) { - if (this.baggageItems.isEmpty()) { - this.baggageItems = new HashMap<>(); - } this.baggageItems.put(key, value); } @@ -212,7 +217,7 @@ public Iterable> baggageItems() { } @JsonIgnore - public Queue getTrace() { + public PendingTrace getTrace() { return this.trace; } @@ -244,9 +249,6 @@ public synchronized void setTag(final String tag, final Object value) { return; } - if (this.tags.isEmpty()) { - this.tags = new HashMap<>(); - } this.tags.put(tag, value); // Call decorators @@ -271,12 +273,12 @@ public synchronized void setTag(final String tag, final Object value) { } public synchronized Map getTags() { - if (tags.isEmpty()) { - tags = Maps.newHashMapWithExpectedSize(3); - } tags.put(DDTags.THREAD_NAME, threadName); tags.put(DDTags.THREAD_ID, threadId); - tags.put(DDTags.SPAN_TYPE, getSpanType()); + final String spanType = getSpanType(); + if (spanType != null) { + tags.put(DDTags.SPAN_TYPE, spanType); + } return Collections.unmodifiableMap(tags); } @@ -307,12 +309,4 @@ public String toString() { } return s.toString(); } - - public String getOperationName() { - return operationName; - } - - public void setOperationName(final String operationName) { - this.operationName = operationName; - } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index 4c5292e2eed..935e9e6d26f 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -1,12 +1,18 @@ package datadog.opentracing; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import datadog.opentracing.decorators.AbstractDecorator; import datadog.opentracing.decorators.DDDecoratorsFactory; import datadog.opentracing.propagation.Codec; +import datadog.opentracing.propagation.ExtractedContext; import datadog.opentracing.propagation.HTTPCodec; +import datadog.opentracing.scopemanager.ContextualScopeManager; +import datadog.opentracing.scopemanager.ScopeContext; import datadog.trace.api.DDTags; +import datadog.trace.api.interceptor.MutableSpan; +import datadog.trace.api.interceptor.TraceInterceptor; import datadog.trace.common.DDTraceConfig; import datadog.trace.common.Service; import datadog.trace.common.sampling.AllSampler; @@ -21,20 +27,25 @@ import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.propagation.Format; -import io.opentracing.util.ThreadLocalScopeManager; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; -import java.util.Queue; +import java.util.ServiceConfigurationError; +import java.util.ServiceLoader; +import java.util.SortedSet; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentSkipListSet; import java.util.concurrent.ThreadLocalRandom; import lombok.extern.slf4j.Slf4j; /** DDTracer makes it easy to send traces and span to DD using the OpenTracing API. */ @Slf4j -public class DDTracer extends ThreadLocalScopeManager implements io.opentracing.Tracer { +public class DDTracer implements io.opentracing.Tracer { public static final String UNASSIGNED_DEFAULT_SERVICE_NAME = "unnamed-java-app"; @@ -44,13 +55,24 @@ public class DDTracer extends ThreadLocalScopeManager implements io.opentracing. final Writer writer; /** Sampler defines the sampling policy in order to reduce the number of traces for instance */ final Sampler sampler; + /** Scope manager is in charge of managing the scopes from which spans are created */ + final ContextualScopeManager scopeManager = new ContextualScopeManager(); /** A set of tags that are added to every span */ private final Map spanTags; /** Span context decorators */ - private final Map> spanContextDecorators = new HashMap<>(); - + private final Map> spanContextDecorators = + new ConcurrentHashMap<>(); + + private final SortedSet interceptors = + new ConcurrentSkipListSet<>( + new Comparator() { + @Override + public int compare(final TraceInterceptor o1, final TraceInterceptor o2) { + return Integer.compare(o1.priority(), o2.priority()); + } + }); private final CodecRegistry registry; private final Map services = new HashMap<>(); @@ -101,6 +123,9 @@ public DDTracer( final DDApi api = ((DDAgentWriter) this.writer).getApi(); api.addResponseListener((DDApi.ResponseListener) this.sampler); } + + registerClassLoader(ClassLoader.getSystemClassLoader()); + log.info("New instance: {}", this); } @@ -137,20 +162,52 @@ public void addDecorator(final AbstractDecorator decorator) { spanContextDecorators.put(decorator.getMatchingTag(), list); } + /** + * Add a new interceptor to the tracer. Interceptors with duplicate priority to existing ones are + * ignored. + * + * @param interceptor + * @return false if an interceptor with same priority exists. + */ + public boolean addInterceptor(final TraceInterceptor interceptor) { + return interceptors.add(interceptor); + } + + public void addScopeContext(final ScopeContext context) { + scopeManager.addScopeContext(context); + } + + /** + * If an application is using a non-system classloader, that classloader should be registered + * here. Due to the way Spring Boot structures its' executable jar, this might log some warnings. + * + * @param classLoader to register. + */ + public void registerClassLoader(final ClassLoader classLoader) { + try { + for (final TraceInterceptor interceptor : + ServiceLoader.load(TraceInterceptor.class, classLoader)) { + addInterceptor(interceptor); + } + } catch (final ServiceConfigurationError e) { + log.warn("Problem loading TraceInterceptor for classLoader: " + classLoader, e); + } + } + @Override - public ScopeManager scopeManager() { - return this; + public ContextualScopeManager scopeManager() { + return scopeManager; } @Override public Span activeSpan() { - final Scope active = active(); + final Scope active = scopeManager.active(); return active == null ? null : active.span(); } @Override public DDSpanBuilder buildSpan(final String operationName) { - return new DDSpanBuilder(operationName, this); + return new DDSpanBuilder(operationName, scopeManager); } @Override @@ -181,12 +238,27 @@ public SpanContext extract(final Format format, final T carrier) { * * @param trace a list of the spans related to the same trace */ - public void write(final Queue trace) { + void write(final PendingTrace trace) { if (trace.isEmpty()) { return; } - if (this.sampler.sample(trace.peek())) { - this.writer.write(new ArrayList<>(trace)); + final ArrayList writtenTrace; + if (interceptors.isEmpty()) { + writtenTrace = Lists.newArrayList(trace); + } else { + Collection interceptedTrace = Lists.newArrayList(trace); + for (final TraceInterceptor interceptor : interceptors) { + interceptedTrace = interceptor.onTraceComplete(interceptedTrace); + } + writtenTrace = Lists.newArrayListWithExpectedSize(interceptedTrace.size()); + for (final MutableSpan span : interceptedTrace) { + if (span instanceof DDSpan) { + writtenTrace.add((DDSpan) span); + } + } + } + if (!writtenTrace.isEmpty() && this.sampler.sample(writtenTrace.get(0))) { + this.writer.write(writtenTrace); } } @@ -405,17 +477,18 @@ private DDSpanContext buildSpanContext() { final long spanId = generateNewId(); final long parentSpanId; final Map baggage; - final Queue parentTrace; + final PendingTrace parentTrace; final int samplingPriority; final DDSpanContext context; SpanContext parentContext = this.parent; if (parentContext == null && !ignoreScope) { // use the Scope as parent unless overridden or ignored. - final Scope scope = active(); + final Scope scope = scopeManager.active(); if (scope != null) parentContext = scope.span().context(); } + // Propagate internal trace if (parentContext instanceof DDSpanContext) { final DDSpanContext ddsc = (DDSpanContext) parentContext; traceId = ddsc.getTraceId(); @@ -423,14 +496,24 @@ private DDSpanContext buildSpanContext() { baggage = ddsc.getBaggageItems(); parentTrace = ddsc.getTrace(); samplingPriority = ddsc.getSamplingPriority(); - if (this.serviceName == null) this.serviceName = ddsc.getServiceName(); if (this.spanType == null) this.spanType = ddsc.getSpanType(); + + // Propagate external trace + } else if (parentContext instanceof ExtractedContext) { + final ExtractedContext ddsc = (ExtractedContext) parentContext; + traceId = ddsc.getTraceId(); + parentSpanId = ddsc.getSpanId(); + baggage = ddsc.getBaggage(); + parentTrace = new PendingTrace(DDTracer.this, traceId); + samplingPriority = ddsc.getSamplingPriority(); + + // Start a new trace } else { traceId = generateNewId(); parentSpanId = 0L; baggage = null; - parentTrace = null; + parentTrace = new PendingTrace(DDTracer.this, traceId); samplingPriority = PrioritySampling.UNSET; } @@ -441,8 +524,6 @@ private DDSpanContext buildSpanContext() { final String operationName = this.operationName != null ? this.operationName : this.resourceName; - // this.operationName, this.tags, - // some attributes are inherited from the parent context = new DDSpanContext( diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java b/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java new file mode 100644 index 00000000000..2bdfbdcf263 --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java @@ -0,0 +1,193 @@ +package datadog.opentracing; + +import com.google.common.collect.Sets; +import com.google.common.util.concurrent.ThreadFactoryBuilder; +import datadog.opentracing.scopemanager.ContinuableScope; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.util.Set; +import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class PendingTrace extends ConcurrentLinkedDeque { + static { + SpanCleaner.start(); + } + + private final DDTracer tracer; + private final long traceId; + + private final ReferenceQueue referenceQueue = new ReferenceQueue(); + private final Set> weakReferences = Sets.newConcurrentHashSet(); + + private final AtomicInteger pendingReferenceCount = new AtomicInteger(0); + + /** Ensure a trace is never written multiple times */ + private final AtomicBoolean isWritten = new AtomicBoolean(false); + + PendingTrace(final DDTracer tracer, final long traceId) { + this.tracer = tracer; + this.traceId = traceId; + SpanCleaner.pendingTraces.add(this); + } + + public void registerSpan(final DDSpan span) { + if (span.context().getTraceId() != traceId) { + log.warn("{} - span registered for wrong trace ({})", span, traceId); + return; + } + synchronized (span) { + if (null == span.ref) { + span.ref = new WeakReference(span, referenceQueue); + weakReferences.add(span.ref); + final int count = pendingReferenceCount.incrementAndGet(); + log.debug("traceId: {} -- registered span {}. count = {}", traceId, span, count); + } else { + log.debug("span {} already registered in trace {}", span, traceId); + } + } + } + + private void expireSpan(final DDSpan span) { + if (span.context().getTraceId() != traceId) { + log.warn("{} - span expired for wrong trace ({})", span, traceId); + return; + } + synchronized (span) { + if (null == span.ref) { + log.debug("span {} not registered in trace {}", span, traceId); + } else { + weakReferences.remove(span.ref); + span.ref.clear(); + span.ref = null; + expireReference(); + } + } + } + + public void addSpan(final DDSpan span) { + if (span.getDurationNano() == 0) { + log.warn("{} - added to trace, but not complete.", span); + return; + } + if (traceId != span.getTraceId()) { + log.warn("{} - added to a mismatched trace.", span); + return; + } + + if (!isWritten.get()) { + addFirst(span); + expireSpan(span); + } else { + log.warn("{} - finished after trace reported.", span); + } + } + + /** + * When using continuations, it's possible one may be used after all existing spans are otherwise + * completed, so we need to wait till continuations are de-referenced before reporting. + */ + public void registerContinuation(final ContinuableScope.Continuation continuation) { + synchronized (continuation) { + if (continuation.ref == null) { + continuation.ref = + new WeakReference(continuation, referenceQueue); + weakReferences.add(continuation.ref); + final int count = pendingReferenceCount.incrementAndGet(); + log.debug( + "traceId: {} -- registered continuation {}. count = {}", traceId, continuation, count); + } else { + log.debug("continuation {} already registered in trace {}", continuation, traceId); + } + } + } + + public void cancelContinuation(final ContinuableScope.Continuation continuation) { + synchronized (continuation) { + if (continuation.ref == null) { + log.debug("continuation {} not registered in trace {}", continuation, traceId); + } else { + weakReferences.remove(continuation.ref); + continuation.ref.clear(); + continuation.ref = null; + expireReference(); + } + } + } + + private void expireReference() { + final int count = pendingReferenceCount.decrementAndGet(); + if (count == 0) { + write(); + } + log.debug("traceId: {} -- Expired reference. count = {}", traceId, count); + } + + private void write() { + if (isWritten.compareAndSet(false, true)) { + SpanCleaner.pendingTraces.remove(this); + if (!isEmpty()) { + log.debug("Writing {} spans to {}.", this.size(), tracer.writer); + tracer.write(this); + } + } + } + + public synchronized boolean clean() { + Reference ref; + int count = 0; + while ((ref = referenceQueue.poll()) != null) { + weakReferences.remove(ref); + count++; + expireReference(); + } + if (count > 0) { + log.debug("{} unfinished spans garbage collected!", count); + } + 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(); + } + } + + private static class SpanCleaner implements Runnable { + private static final long CLEAN_FREQUENCY = 1; + private static final ThreadFactory FACTORY = + new ThreadFactoryBuilder().setNameFormat("dd-span-cleaner-%d").setDaemon(true).build(); + + private static final ScheduledExecutorService EXECUTOR_SERVICE = + Executors.newScheduledThreadPool(1, FACTORY); + + static final Set pendingTraces = Sets.newConcurrentHashSet(); + + static void start() { + EXECUTOR_SERVICE.scheduleAtFixedRate(new SpanCleaner(), 0, CLEAN_FREQUENCY, TimeUnit.SECONDS); + } + + @Override + public void run() { + for (final PendingTrace trace : pendingTraces) { + trace.clean(); + } + } + } +} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/Codec.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/Codec.java index a89d45cc38d..f6f097d3220 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/Codec.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/Codec.java @@ -41,5 +41,5 @@ public interface Codec { * @param carrier * @return the span context */ - DDSpanContext extract(T carrier); + ExtractedContext extract(T carrier); } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/ExtractedContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/ExtractedContext.java new file mode 100644 index 00000000000..f388f6b5c2f --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/ExtractedContext.java @@ -0,0 +1,53 @@ +package datadog.opentracing.propagation; + +import io.opentracing.SpanContext; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; + +public class ExtractedContext implements SpanContext { + private final Long traceId; + private final Long spanId; + private final int samplingPriority; + private final Map baggage; + private final AtomicBoolean samplingPriorityLocked = new AtomicBoolean(false); + + public ExtractedContext( + final Long traceId, + final Long spanId, + final int samplingPriority, + final Map baggage) { + this.traceId = traceId; + this.spanId = spanId; + this.samplingPriority = samplingPriority; + this.baggage = baggage; + } + + @Override + public Iterable> baggageItems() { + return baggage.entrySet(); + } + + public void lockSamplingPriority() { + samplingPriorityLocked.set(true); + } + + public Long getTraceId() { + return traceId; + } + + public Long getSpanId() { + return spanId; + } + + public int getSamplingPriority() { + return samplingPriority; + } + + public Map getBaggage() { + return baggage; + } + + public boolean getSamplingPriorityLocked() { + return samplingPriorityLocked.get(); + } +} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java index a7a76e92d40..942448fff35 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java @@ -34,7 +34,7 @@ public void inject(final DDSpanContext context, final TextMap carrier) { } @Override - public DDSpanContext extract(final TextMap carrier) { + public ExtractedContext extract(final TextMap carrier) { Map baggage = Collections.emptyMap(); Long traceId = 0L; @@ -56,23 +56,9 @@ public DDSpanContext extract(final TextMap carrier) { samplingPriority = Integer.parseInt(entry.getValue()); } } - DDSpanContext context = null; + ExtractedContext context = null; if (traceId != 0L) { - context = - new DDSpanContext( - traceId, - spanId, - 0L, - null, - null, - null, - samplingPriority, - baggage, - false, - null, - null, - null, - null); + context = new ExtractedContext(traceId, spanId, samplingPriority, baggage); context.lockSamplingPriority(); log.debug("{} - Parent context extracted", context); diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContextualScopeManager.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContextualScopeManager.java new file mode 100644 index 00000000000..094e9f869ec --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContextualScopeManager.java @@ -0,0 +1,36 @@ +package datadog.opentracing.scopemanager; + +import io.opentracing.Scope; +import io.opentracing.ScopeManager; +import io.opentracing.Span; +import java.util.Deque; +import java.util.concurrent.ConcurrentLinkedDeque; + +public class ContextualScopeManager implements ScopeManager { + final ThreadLocal tlsScope = new ThreadLocal<>(); + final Deque scopeContexts = new ConcurrentLinkedDeque<>(); + + @Override + public Scope activate(final Span span, final boolean finishOnClose) { + for (final ScopeContext context : scopeContexts) { + if (context.inContext()) { + return context.activate(span, finishOnClose); + } + } + return new ContinuableScope(this, span, finishOnClose); + } + + @Override + public Scope active() { + for (final ScopeContext csm : scopeContexts) { + if (csm.inContext()) { + return csm.active(); + } + } + return tlsScope.get(); + } + + public void addScopeContext(final ScopeContext context) { + scopeContexts.addFirst(context); + } +} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java new file mode 100644 index 00000000000..3250894705f --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java @@ -0,0 +1,144 @@ +package datadog.opentracing.scopemanager; + +import datadog.opentracing.DDSpanContext; +import datadog.opentracing.PendingTrace; +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.noop.NoopScopeManager; +import java.io.Closeable; +import java.lang.ref.WeakReference; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class ContinuableScope implements Scope, TraceScope { + final ContextualScopeManager scopeManager; + final AtomicInteger refCount; + private final Span wrapped; + private final boolean finishOnClose; + private final Scope toRestore; + + ContinuableScope( + final ContextualScopeManager scopeManager, final Span wrapped, final boolean finishOnClose) { + this(scopeManager, new AtomicInteger(1), wrapped, finishOnClose); + } + + private ContinuableScope( + final ContextualScopeManager scopeManager, + final AtomicInteger refCount, + final Span wrapped, + final boolean finishOnClose) { + + this.scopeManager = scopeManager; + this.refCount = refCount; + this.wrapped = wrapped; + this.finishOnClose = finishOnClose; + this.toRestore = scopeManager.tlsScope.get(); + scopeManager.tlsScope.set(this); + } + + @Override + public void close() { + if (scopeManager.tlsScope.get() != this) { + return; + } + + if (refCount.decrementAndGet() == 0 && finishOnClose) { + wrapped.finish(); + } + + scopeManager.tlsScope.set(toRestore); + } + + @Override + public Span span() { + return wrapped; + } + + /** + * The continuation returned should be closed after the associa + * + * @param finishOnClose + * @return + */ + public Continuation capture(final boolean finishOnClose) { + return new Continuation(this.finishOnClose && finishOnClose); + } + + public class Continuation implements Closeable, TraceScope.Continuation { + public WeakReference ref; + + private final AtomicBoolean used = new AtomicBoolean(false); + private final PendingTrace trace; + private final boolean finishSpanOnClose; + + private Continuation(final boolean finishOnClose) { + this.finishSpanOnClose = finishOnClose; + refCount.incrementAndGet(); + if (wrapped.context() instanceof DDSpanContext) { + final DDSpanContext context = (DDSpanContext) wrapped.context(); + trace = context.getTrace(); + trace.registerContinuation(this); + } else { + trace = null; + } + } + + public ClosingScope activate() { + if (used.compareAndSet(false, true)) { + for (final ScopeContext context : scopeManager.scopeContexts) { + if (context.inContext()) { + return new ClosingScope(context.activate(wrapped, finishSpanOnClose)); + } + } + return new ClosingScope( + new ContinuableScope(scopeManager, refCount, wrapped, finishSpanOnClose)); + } else { + log.debug("Reusing a continuation not allowed. Returning no-op scope."); + return new ClosingScope(NoopScopeManager.NoopScope.INSTANCE); + } + } + + @Override + public void close() { + used.getAndSet(true); + if (trace != null) { + trace.cancelContinuation(this); + } + } + + private class ClosingScope implements Scope, TraceScope { + private final Scope wrappedScope; + + private ClosingScope(final Scope wrappedScope) { + this.wrappedScope = wrappedScope; + } + + @Override + public Continuation capture(boolean finishOnClose) { + if (wrappedScope instanceof TraceScope) { + return ((TraceScope) wrappedScope).capture(finishOnClose); + } else { + log.debug( + "{} Failed to capture. ClosingScope does not wrap a TraceScope: {}.", + this, + wrappedScope); + return null; + } + } + + @Override + public void close() { + wrappedScope.close(); + ContinuableScope.Continuation.this.close(); + } + + @Override + public Span span() { + return wrappedScope.span(); + } + } + } +} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ScopeContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ScopeContext.java new file mode 100644 index 00000000000..37786b0b8d0 --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ScopeContext.java @@ -0,0 +1,14 @@ +package datadog.opentracing.scopemanager; + +import io.opentracing.ScopeManager; + +/** Represents a ScopeManager that is only valid in certain cases such as on a specific thread. */ +public interface ScopeContext extends ScopeManager { + + /** + * When multiple ScopeContexts are active, the first one to respond true will have control. + * + * @return true if this ScopeContext should be active + */ + boolean inContext(); +} diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy index 0c354be7bd1..a796b5a1c47 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy @@ -48,9 +48,10 @@ class DDSpanBuilderTest extends Specification { span = tracer.buildSpan(expectedName).withServiceName("foo").start() then: - span.getTags() == [(DDTags.THREAD_NAME): Thread.currentThread().getName(), - (DDTags.THREAD_ID) : Thread.currentThread().getId(), - (DDTags.SPAN_TYPE) : null] + span.getTags() == [ + (DDTags.THREAD_NAME): Thread.currentThread().getName(), + (DDTags.THREAD_ID) : Thread.currentThread().getId(), + ] when: // with all custom fields provided @@ -118,6 +119,7 @@ class DDSpanBuilderTest extends Specification { when(mockedContext.getSpanId()).thenReturn(spanId) when(mockedContext.getServiceName()).thenReturn("foo") + when(mockedContext.getTrace()).thenReturn(new PendingTrace(tracer, 1L)) final String expectedName = "fakeName" @@ -204,13 +206,15 @@ class DDSpanBuilderTest extends Specification { final long tickEnd = System.currentTimeMillis() for (int i = 1; i <= 10; i++) { - spans.add(tracer + def span = tracer .buildSpan("fake_" + i) .withServiceName("foo") .asChildOf(spans.get(i - 1)) - .start()) + .start() + spans.add(span) + span.finish() } - spans.get(1).finish(tickEnd) + root.finish(tickEnd) expect: root.context().getTrace().size() == nbSamples + 1 @@ -223,13 +227,17 @@ class DDSpanBuilderTest extends Specification { System.setProperty("dd.trace.span.tags", tagString) tracer = new DDTracer(writer) def span = tracer.buildSpan("op name").withServiceName("foo").start() - tags.putAll([(DDTags.THREAD_NAME): Thread.currentThread().getName(), - (DDTags.THREAD_ID) : Thread.currentThread().getId(), - (DDTags.SPAN_TYPE) : null]) + tags.putAll([ + (DDTags.THREAD_NAME): Thread.currentThread().getName(), + (DDTags.THREAD_ID) : Thread.currentThread().getId(), + ]) expect: span.tags == tags + cleanup: + System.clearProperty("dd.trace.span.tags") + where: tagString | tags "" | [:] diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanSerializationTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanSerializationTest.groovy index 38be3314acb..1c35d641bce 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanSerializationTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanSerializationTest.groovy @@ -4,11 +4,12 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.google.common.collect.Maps import datadog.trace.api.DDTags import datadog.trace.common.sampling.PrioritySampling +import datadog.trace.common.writer.ListWriter import spock.lang.Specification import spock.lang.Timeout import spock.lang.Unroll -@Timeout(2) +@Timeout(5) class DDSpanSerializationTest extends Specification { @Unroll @@ -35,6 +36,8 @@ class DDSpanSerializationTest extends Specification { expected.put("parent_id", 0l) expected.put("trace_id", 1l) + def writer = new ListWriter() + def tracer = new DDTracer(writer) final DDSpanContext context = new DDSpanContext( 1L, @@ -48,8 +51,8 @@ class DDSpanSerializationTest extends Specification { false, "type", tags, - null, - null) + new PendingTrace(tracer, 1L), + tracer) baggage.put(DDTags.THREAD_NAME, Thread.currentThread().getName()) baggage.put(DDTags.THREAD_ID, String.valueOf(Thread.currentThread().getId())) diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy index 7b86ed026a7..fa10e49531a 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy @@ -27,8 +27,8 @@ class DDSpanTest extends Specification { false, "fakeType", null, - null, - null) + new PendingTrace(tracer, 1L), + tracer) final DDSpan span = new DDSpan(1L, context) @@ -125,7 +125,7 @@ class DDSpanTest extends Specification { expect: span.durationNano >= TimeUnit.MILLISECONDS.toNanos(betweenDur) span.durationNano <= TimeUnit.MILLISECONDS.toNanos(total) - span.durationNano % mod == 0 + span.durationNano % mod == 0 || span.durationNano == 1 } def "stopping with a timestamp disables nanotime"() { diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy new file mode 100644 index 00000000000..2f653e1018e --- /dev/null +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy @@ -0,0 +1,145 @@ +package datadog.opentracing + +import datadog.trace.common.writer.ListWriter +import spock.lang.Specification +import spock.lang.Subject + + +class PendingTraceTest extends Specification { + def writer = new ListWriter() + def tracer = new DDTracer(writer) + + def traceId = System.identityHashCode(this) + + @Subject + PendingTrace trace = new PendingTrace(tracer, traceId) + + DDSpan rootSpan = SpanFactory.newSpanOf(trace) + + def setup() { + assert trace.size() == 0 + assert trace.pendingReferenceCount.get() == 1 + assert trace.weakReferences.size() == 1 + assert trace.isWritten.get() == false + } + + def "single span gets added to trace and written when finished"() { + setup: + rootSpan.finish() + + expect: + trace.asList() == [rootSpan] + writer == [[rootSpan]] + } + + def "child finishes before parent"() { + when: + def child = tracer.buildSpan("child").asChildOf(rootSpan).start() + + then: + trace.pendingReferenceCount.get() == 2 + trace.weakReferences.size() == 2 + + when: + child.finish() + + then: + trace.pendingReferenceCount.get() == 1 + trace.weakReferences.size() == 1 + trace.asList() == [child] + writer == [] + + when: + rootSpan.finish() + + then: + trace.pendingReferenceCount.get() == 0 + trace.weakReferences.size() == 0 + trace.asList() == [rootSpan, child] + writer == [[rootSpan, child]] + } + + def "parent finishes before child which holds up trace"() { + when: + def child = tracer.buildSpan("child").asChildOf(rootSpan).start() + + then: + trace.pendingReferenceCount.get() == 2 + trace.weakReferences.size() == 2 + + when: + rootSpan.finish() + + then: + trace.pendingReferenceCount.get() == 1 + trace.weakReferences.size() == 1 + trace.asList() == [rootSpan] + writer == [] + + when: + child.finish() + + then: + trace.pendingReferenceCount.get() == 0 + trace.weakReferences.size() == 0 + trace.asList() == [child, rootSpan] + writer == [[child, rootSpan]] + } + + def "trace reported when unfinished child discarded"() { + when: + def child = tracer.buildSpan("child").asChildOf(rootSpan).start() + rootSpan.finish() + + then: + trace.pendingReferenceCount.get() == 1 + trace.weakReferences.size() == 1 + trace.asList() == [rootSpan] + writer == [] + + when: + child = null + while (!trace.clean()) { + trace.awaitGC() + } + + then: + trace.pendingReferenceCount.get() == 0 + trace.weakReferences.size() == 0 + trace.asList() == [rootSpan] + writer == [[rootSpan]] + } + + def "add unfinished span to trace fails"() { + setup: + trace.addSpan(rootSpan) + + expect: + trace.pendingReferenceCount.get() == 1 + trace.weakReferences.size() == 1 + trace.asList() == [] + } + + def "register span to wrong trace fails"() { + setup: + def otherTrace = new PendingTrace(tracer, traceId - 10) + otherTrace.registerSpan(new DDSpan(0, rootSpan.context())) + + expect: + otherTrace.pendingReferenceCount.get() == 0 + otherTrace.weakReferences.size() == 0 + otherTrace.asList() == [] + } + + def "add span to wrong trace fails"() { + setup: + def otherTrace = new PendingTrace(tracer, traceId - 10) + rootSpan.finish() + otherTrace.addSpan(rootSpan) + + expect: + otherTrace.pendingReferenceCount.get() == 0 + otherTrace.weakReferences.size() == 0 + otherTrace.asList() == [] + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/SpanFactory.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/SpanFactory.groovy new file mode 100644 index 00000000000..f33d93bdf2a --- /dev/null +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/SpanFactory.groovy @@ -0,0 +1,83 @@ +package datadog.opentracing + +import datadog.trace.common.sampling.PrioritySampling +import datadog.trace.common.writer.ListWriter + +class SpanFactory { + static newSpanOf(long timestampMicro) { + def writer = new ListWriter() + def tracer = new DDTracer(writer) + def context = new DDSpanContext( + 1L, + 1L, + 0L, + "fakeService", + "fakeOperation", + "fakeResource", + PrioritySampling.UNSET, + Collections.emptyMap(), + false, + "fakeType", + Collections.emptyMap(), + new PendingTrace(tracer, 1L), + tracer) + return new DDSpan(timestampMicro, context) + } + + static newSpanOf(DDTracer tracer) { + def context = new DDSpanContext( + 1L, + 1L, + 0L, + "fakeService", + "fakeOperation", + "fakeResource", + PrioritySampling.UNSET, + Collections.emptyMap(), + false, + "fakeType", + Collections.emptyMap(), + new PendingTrace(tracer, 1L), + tracer) + return new DDSpan(1, context) + } + + static newSpanOf(PendingTrace trace) { + def context = new DDSpanContext( + trace.traceId, + 1L, + 0L, + "fakeService", + "fakeOperation", + "fakeResource", + PrioritySampling.UNSET, + Collections.emptyMap(), + false, + "fakeType", + Collections.emptyMap(), + trace, + trace.tracer) + return new DDSpan(1, context) + } + + static DDSpan newSpanOf(String serviceName, String envName) { + def writer = new ListWriter() + def tracer = new DDTracer(writer) + def context = new DDSpanContext( + 1L, + 1L, + 0L, + serviceName, + "fakeOperation", + "fakeResource", + PrioritySampling.UNSET, + Collections.emptyMap(), + false, + "fakeType", + Collections.emptyMap(), + new PendingTrace(tracer, 1L), + tracer) + context.setTag("env", envName) + return new DDSpan(0l, context) + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/TraceInterceptorTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/TraceInterceptorTest.groovy new file mode 100644 index 00000000000..77579f412b9 --- /dev/null +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/TraceInterceptorTest.groovy @@ -0,0 +1,149 @@ +package datadog.opentracing + +import datadog.trace.api.interceptor.MutableSpan +import datadog.trace.api.interceptor.TraceInterceptor +import datadog.trace.common.writer.ListWriter +import spock.lang.Specification +import spock.lang.Unroll + +import java.util.concurrent.atomic.AtomicBoolean + +class TraceInterceptorTest extends Specification { + def writer = new ListWriter() + def tracer = new DDTracer(writer) + + def "interceptor is registered as a service"() { + expect: + tracer.interceptors.iterator().hasNext() + tracer.interceptors.iterator().next() instanceof TestInterceptor + } + + def "interceptors with the same priority replaced"() { + setup: + int priority = 999 + ((TestInterceptor) tracer.interceptors.iterator().next()).priority = priority + tracer.interceptors.add(new TraceInterceptor() { + @Override + Collection onTraceComplete(Collection trace) { + return null + } + + @Override + int priority() { + return priority + } + }) + + expect: + tracer.interceptors.size() == 1 + tracer.interceptors.iterator().next() instanceof TestInterceptor + } + + def "interceptors with different priority sorted"() { + setup: + def priority = score + def existingInterceptor = tracer.interceptors.iterator().next() + def newInterceptor = new TraceInterceptor() { + @Override + Collection onTraceComplete(Collection trace) { + return null + } + + @Override + int priority() { + return priority + } + } + tracer.interceptors.add(newInterceptor) + + expect: + tracer.interceptors == reverse ? [newInterceptor, existingInterceptor] : [existingInterceptor, newInterceptor] + + where: + score | reverse + -1 | false + 1 | true + } + + @Unroll + def "interceptor can discard a trace (p=#score)"() { + setup: + def called = new AtomicBoolean(false) + def priority = score + tracer.interceptors.add(new TraceInterceptor() { + @Override + Collection onTraceComplete(Collection trace) { + called.set(true) + return Collections.emptyList() + } + + @Override + int priority() { + return priority + } + }) + tracer.buildSpan("test").start().finish() + + expect: + tracer.interceptors.size() == Math.abs(score) + 1 + (called.get()) == (score != 0) + (writer == []) == (score != 0) + + where: + score | _ + -1 | _ + 0 | _ + 1 | _ + } + + @Unroll + def "interceptor can modify a span"() { + setup: + tracer.interceptors.add(new TraceInterceptor() { + @Override + Collection onTraceComplete(Collection trace) { + for (MutableSpan span : trace) { + span + .setOperationName("modifiedON-" + span.getOperationName()) + .setServiceName("modifiedSN-" + span.getServiceName()) + .setResourceName("modifiedRN-" + span.getResourceName()) + .setSpanType("modifiedST-" + span.getSpanType()) + .setTag("boolean-tag", true) + .setTag("number-tag", 5.0) + .setTag("string-tag", "howdy") + .setError(true) + } + return trace + } + + @Override + int priority() { + return 1 + } + }) + tracer.buildSpan("test").start().finish() + + expect: + def trace = writer.firstTrace() + trace.size() == 1 + + def span = trace[0] + + span.context().operationName == "modifiedON-test" + span.serviceName == "modifiedSN-unnamed-java-app" + span.resourceName == "modifiedRN-modifiedON-test" + span.type == "modifiedST-null" + span.context().getErrorFlag() + + def tags = span.context().tags + + tags["boolean-tag"] == true + tags["number-tag"] == 5.0 + tags["string-tag"] == "howdy" + + tags["span.type"] == "modifiedST-null" + tags["thread.name"] != null + tags["thread.id"] != null + tags.size() == 6 + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy index 7469ebd584e..ef08682f454 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy @@ -2,7 +2,7 @@ package datadog.opentracing.decorators import datadog.opentracing.DDSpanContext import datadog.opentracing.DDTracer -import datadog.trace.SpanFactory +import datadog.opentracing.SpanFactory import datadog.trace.common.writer.LoggingWriter import io.opentracing.tag.StringTag import io.opentracing.tag.Tags diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy index 0307d8daecd..aa273fb1b43 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy @@ -1,7 +1,10 @@ package datadog.opentracing.decorators import datadog.opentracing.DDSpanContext +import datadog.opentracing.DDTracer +import datadog.opentracing.PendingTrace import datadog.trace.common.sampling.PrioritySampling +import datadog.trace.common.writer.ListWriter import io.opentracing.tag.Tags import spock.lang.Specification import spock.lang.Subject @@ -9,6 +12,8 @@ import spock.lang.Timeout @Timeout(1) class URLAsResourceNameTest extends Specification { + def writer = new ListWriter() + def tracer = new DDTracer(writer) @Subject def decorator = new URLAsResourceName() @@ -95,8 +100,8 @@ class URLAsResourceNameTest extends Specification { false, "fakeType", tags, - null, - null) + new PendingTrace(tracer, 1L), + tracer) then: decorator.afterSetTag(context, Tags.HTTP_URL.getKey(), value) diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HTTPCodecTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HTTPCodecTest.groovy index 35fffa230cb..e2a2491da16 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HTTPCodecTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HTTPCodecTest.groovy @@ -1,7 +1,10 @@ package datadog.opentracing.propagation import datadog.opentracing.DDSpanContext +import datadog.opentracing.DDTracer +import datadog.opentracing.PendingTrace import datadog.trace.common.sampling.PrioritySampling +import datadog.trace.common.writer.ListWriter import io.opentracing.propagation.TextMapExtractAdapter import io.opentracing.propagation.TextMapInjectAdapter import spock.lang.Shared @@ -23,26 +26,28 @@ class HTTPCodecTest extends Specification { @Unroll def "inject http headers"() { setup: + def writer = new ListWriter() + def tracer = new DDTracer(writer) final DDSpanContext mockedContext = - new DDSpanContext( - 1L, - 2L, - 0L, - "fakeService", - "fakeOperation", - "fakeResource", - samplingPriority, - new HashMap() { - { - put("k1", "v1") - put("k2", "v2") - } - }, - false, - "fakeType", - null, - null, - null) + new DDSpanContext( + 1L, + 2L, + 0L, + "fakeService", + "fakeOperation", + "fakeResource", + samplingPriority, + new HashMap() { + { + put("k1", "v1") + put("k2", "v2") + } + }, + false, + "fakeType", + null, + new PendingTrace(tracer, 1L), + tracer) final Map carrier = new HashMap<>() @@ -57,41 +62,38 @@ class HTTPCodecTest extends Specification { carrier.get(OT_BAGGAGE_PREFIX + "k2") == "v2" where: - samplingPriority | _ - PrioritySampling.UNSET | _ - PrioritySampling.SAMPLER_KEEP | _ + samplingPriority | _ + PrioritySampling.UNSET | _ + PrioritySampling.SAMPLER_KEEP | _ } @Unroll def "extract http headers"() { setup: - final Map actual = - new HashMap() { - { - put(TRACE_ID_KEY.toUpperCase(), "1") - put(SPAN_ID_KEY.toUpperCase(), "2") - put(OT_BAGGAGE_PREFIX.toUpperCase() + "k1", "v1") - put(OT_BAGGAGE_PREFIX.toUpperCase() + "k2", "v2") - } - } + final Map actual = [ + (TRACE_ID_KEY.toUpperCase()) : "1", + (SPAN_ID_KEY.toUpperCase()) : "2", + (OT_BAGGAGE_PREFIX.toUpperCase() + "k1"): "v1", + (OT_BAGGAGE_PREFIX.toUpperCase() + "k2"): "v2", + ] if (samplingPriority != PrioritySampling.UNSET) { actual.put(SAMPLING_PRIORITY_KEY, String.valueOf(samplingPriority)) } final HTTPCodec codec = new HTTPCodec() - final DDSpanContext context = codec.extract(new TextMapExtractAdapter(actual)) + final ExtractedContext context = codec.extract(new TextMapExtractAdapter(actual)) expect: context.getTraceId() == 1l context.getSpanId() == 2l - context.getBaggageItem("k1") == "v1" - context.getBaggageItem("k2") == "v2" + context.getBaggage().get("k1") == "v1" + context.getBaggage().get("k2") == "v2" context.getSamplingPriority() == samplingPriority where: - samplingPriority | _ - PrioritySampling.UNSET | _ - PrioritySampling.SAMPLER_KEEP | _ + samplingPriority | _ + PrioritySampling.UNSET | _ + PrioritySampling.SAMPLER_KEEP | _ } } 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 new file mode 100644 index 00000000000..6c0924e8be4 --- /dev/null +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy @@ -0,0 +1,433 @@ +package datadog.opentracing.scopemanager + +import datadog.opentracing.DDSpan +import datadog.opentracing.DDSpanContext +import datadog.opentracing.DDTracer +import datadog.opentracing.PendingTrace +import datadog.trace.common.writer.ListWriter +import io.opentracing.Scope +import io.opentracing.Span +import spock.lang.Specification +import spock.lang.Subject +import spock.lang.Timeout +import spock.lang.Unroll + +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference + +@Timeout(1) +class ScopeManagerTest extends Specification { + def writer = new ListWriter() + def tracer = new DDTracer(writer) + + @Subject + def scopeManager = tracer.scopeManager() + + def cleanup() { + scopeManager.tlsScope.remove() + } + + def "threadlocal is empty"() { + setup: + def builder = tracer.buildSpan("test") + builder.start() + + expect: + scopeManager.active() == null + writer.empty + } + + def "threadlocal is active"() { + when: + def builder = tracer.buildSpan("test") + def scope = builder.startActive(finishSpan) + + then: + !spanFinished(scope.span()) + scopeManager.active() == scope + scope instanceof ContinuableScope + writer.empty + + when: + scope.close() + + then: + spanFinished(scope.span()) == finishSpan + writer == [[scope.span()]] || !finishSpan + scopeManager.active() == null + + where: + finishSpan << [true, false] + } + + def "sets parent as current upon close"() { + setup: + def parentScope = tracer.buildSpan("parent").startActive(finishSpan) + def childScope = tracer.buildSpan("parent").startActive(finishSpan) + + expect: + scopeManager.active() == childScope + childScope.span().context().parentId == parentScope.span().context().spanId + childScope.span().context().trace == parentScope.span().context().trace + + when: + childScope.close() + + then: + scopeManager.active() == parentScope + spanFinished(childScope.span()) == finishSpan + !spanFinished(parentScope.span()) + writer == [] + + when: + parentScope.close() + + then: + spanFinished(childScope.span()) == finishSpan + spanFinished(parentScope.span()) == finishSpan + writer == [[parentScope.span(), childScope.span()]] || !finishSpan + scopeManager.active() == null + + where: + finishSpan << [true, false] + } + + def "ref counting scope doesn't close if non-zero"() { + setup: + def builder = tracer.buildSpan("test") + def scope = (ContinuableScope) builder.startActive(true) + def continuation = scope.capture(true) + + expect: + !spanFinished(scope.span()) + scopeManager.active() == scope + scope instanceof ContinuableScope + writer.empty + + when: + scope.close() + + then: + !spanFinished(scope.span()) + scopeManager.active() == null + writer.empty + + when: + continuation.activate() + if (forceGC) { + continuation = null // Continuation references also hold up traces. + PendingTrace.awaitGC() + ((DDSpanContext) scope.span().context()).trace.clean() + } + if (autoClose) { + if (continuation != null) { + continuation.close() + } + } + + then: + scopeManager.active() != null + + when: + scopeManager.active().close() + writer.waitForTraces(1) + + then: + scopeManager.active() == null + spanFinished(scope.span()) + writer == [[scope.span()]] + + where: + autoClose | forceGC + true | true + true | false + false | true + } + + def "hard reference on continuation prevents trace from reporting"() { + setup: + def builder = tracer.buildSpan("test") + def scope = (ContinuableScope) builder.startActive(false) + def span = scope.span() + def continuation = scope.capture(true) + scope.close() + span.finish() + + expect: + scopeManager.active() == null + spanFinished(span) + writer == [] + + when: + if (forceGC) { + continuation = null // Continuation references also hold up traces. + PendingTrace.awaitGC() + ((DDSpanContext) span.context()).trace.clean() + writer.waitForTraces(1) + } + if (autoClose) { + if (continuation != null) { + continuation.close() + } + } + + then: + writer == [[span]] + + where: + autoClose | forceGC + true | true + true | false + false | true + } + + def "continuation restores trace"() { + setup: + def parentScope = tracer.buildSpan("parent").startActive(true) + def parentSpan = parentScope.span() + ContinuableScope childScope = (ContinuableScope) tracer.buildSpan("parent").startActive(true) + def childSpan = childScope.span() + + def continuation = childScope.capture(true) + childScope.close() + + expect: + parentSpan.context().trace == childSpan.context().trace + scopeManager.active() == parentScope + !spanFinished(childSpan) + !spanFinished(parentSpan) + + when: + parentScope.close() + // parent span is finished, but trace is not reported + + then: + scopeManager.active() == null + !spanFinished(childSpan) + spanFinished(parentSpan) + writer == [] + + when: + def newScope = continuation.activate() + + then: + scopeManager.active() == newScope.wrappedScope + newScope != childScope && newScope != parentScope + newScope.span() == childSpan + !spanFinished(childSpan) + spanFinished(parentSpan) + writer == [] + + when: + newScope.close() + + then: + scopeManager.active() == null + spanFinished(childSpan) + spanFinished(parentSpan) + writer == [[childSpan, parentSpan]] + } + + def "continuation allows adding spans even after other spans were completed"() { + setup: + def builder = tracer.buildSpan("test") + def scope = (ContinuableScope) builder.startActive(false) + def span = scope.span() + def continuation = scope.capture(false) + scope.close() + span.finish() + + def newScope = continuation.activate() + + expect: + newScope != scope + scopeManager.active() == newScope.wrappedScope + spanFinished(span) + writer == [] + + when: + def childScope = tracer.buildSpan("child").startActive(true) + def childSpan = childScope.span() + childScope.close() + scopeManager.active().close() + + then: + scopeManager.active() == null + spanFinished(childSpan) + childSpan.context().parentId == span.context().spanId + writer == [] + + when: + if (closeScope) { + newScope.close() + } + if (closeContinuation) { + continuation.close() + } + + then: + writer == [[childSpan, span]] + + where: + closeScope | closeContinuation + true | false + false | true + true | true + } + + @Unroll + def "context takes control (#active)"() { + setup: + contexts.each { + scopeManager.addScopeContext(it) + } + def builder = tracer.buildSpan("test") + def scope = (AtomicReferenceScope) builder.startActive(true) + + expect: + scopeManager.tlsScope.get() == null + scopeManager.active() == scope + contexts[active].get() == scope.get() + writer.empty + + where: + active | contexts + 0 | [new AtomicReferenceScope(true)] + 1 | [new AtomicReferenceScope(true), new AtomicReferenceScope(true)] + 3 | [new AtomicReferenceScope(false), new AtomicReferenceScope(true), new AtomicReferenceScope(false), new AtomicReferenceScope(true)] + } + + @Unroll + def "disabled context is ignored (#contexts.size)"() { + setup: + contexts.each { + scopeManager.addScopeContext(it) + } + def builder = tracer.buildSpan("test") + def scope = builder.startActive(true) + + expect: + contexts.findAll { + it.get() != null + } == [] + + scopeManager.tlsScope.get() == scope + scopeManager.active() == scope + writer.empty + + where: + contexts | _ + [] | _ + [new AtomicReferenceScope(false)] | _ + [new AtomicReferenceScope(false), new AtomicReferenceScope(false)] | _ + [new AtomicReferenceScope(false), new AtomicReferenceScope(false), new AtomicReferenceScope(false)] | _ + } + + @Unroll + def "threadlocal to context with capture (#active)"() { + setup: + contexts.each { + scopeManager.addScopeContext(it) + } + ContinuableScope scope = (ContinuableScope) tracer.buildSpan("parent").startActive(true) + + expect: + scopeManager.tlsScope.get() == scope + + when: + def cont = scope.capture(true) + scope.close() + + then: + scopeManager.tlsScope.get() == null + + when: + active.each { + ((AtomicBoolean) contexts[it].enabled).set(true) + } + cont.activate() + + then: + scopeManager.tlsScope.get() == null + + where: + active | contexts + [0] | [new AtomicReferenceScope(false)] + [0] | [new AtomicReferenceScope(false), new AtomicReferenceScope(false)] + [1] | [new AtomicReferenceScope(false), new AtomicReferenceScope(false), new AtomicReferenceScope(false)] + [2] | [new AtomicReferenceScope(false), new AtomicReferenceScope(false), new AtomicReferenceScope(false)] + [0, 2] | [new AtomicReferenceScope(false), new AtomicReferenceScope(false), new AtomicReferenceScope(false)] + } + + @Unroll + def "context to threadlocal (#contexts.size)"() { + setup: + contexts.each { + scopeManager.addScopeContext(it) + } + def scope = tracer.buildSpan("parent").startActive(false) + def span = scope.span() + + expect: + scope instanceof AtomicReferenceScope + scopeManager.tlsScope.get() == null + + when: + scope.close() + contexts.each { + ((AtomicBoolean) it.enabled).set(false) + } + scope = scopeManager.activate(span, true) + + then: + scope instanceof ContinuableScope + scopeManager.tlsScope.get() == scope + + where: + contexts | _ + [new AtomicReferenceScope(true)] | _ + [new AtomicReferenceScope(true), new AtomicReferenceScope(true)] | _ + } + + boolean spanFinished(Span span) { + return ((DDSpan) span).isFinished() + } + + class AtomicReferenceScope extends AtomicReference implements ScopeContext, Scope { + final AtomicBoolean enabled + + AtomicReferenceScope(boolean enabled) { + this.enabled = new AtomicBoolean(enabled) + } + + @Override + boolean inContext() { + return enabled.get() + } + + @Override + void close() { + getAndSet(null).finish() + } + + @Override + Span span() { + return get() + } + + @Override + Scope activate(Span span, boolean finishSpanOnClose) { + set(span) + return this + } + + @Override + Scope active() { + return get() == null ? null : this + } + + String toString() { + return "Ref: " + super.toString() + } + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/DDSpanContextTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/DDSpanContextTest.groovy index 8db8fcd393f..cb7e1cb7792 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/DDSpanContextTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/DDSpanContextTest.groovy @@ -1,5 +1,6 @@ package datadog.trace +import datadog.opentracing.SpanFactory import datadog.trace.api.DDTags import spock.lang.Specification import spock.lang.Timeout diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy index 222f7c674a8..6975ae995c8 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy @@ -72,6 +72,7 @@ class DDTraceConfigTest extends Specification { tracer.writer instanceof LoggingWriter } + @Timeout(5) def "sys props override env vars"() { when: environmentVariables.set(propToEnvName(PREFIX + SERVICE_NAME), "still something else") @@ -101,6 +102,7 @@ class DDTraceConfigTest extends Specification { tracer.spanContextDecorators.size() == 6 } + @Timeout(5) @Unroll def "verify single override on #source for #key"() { when: diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/SpanFactory.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/SpanFactory.groovy deleted file mode 100644 index 9f901536f7b..00000000000 --- a/dd-trace-ot/src/test/groovy/datadog/trace/SpanFactory.groovy +++ /dev/null @@ -1,44 +0,0 @@ -package datadog.trace - -import datadog.opentracing.DDSpan -import datadog.opentracing.DDSpanContext -import datadog.opentracing.DDTracer -import datadog.trace.common.sampling.PrioritySampling - -class SpanFactory { - static newSpanOf(long timestampMicro) { - def context = new DDSpanContext( - 1L, - 1L, - 0L, - "fakeService", - "fakeOperation", - "fakeResource", - PrioritySampling.UNSET, - Collections.emptyMap(), - false, - "fakeType", - Collections.emptyMap(), - null, - new DDTracer()) - return new DDSpan(timestampMicro, context) - } - - static newSpanOf(DDTracer tracer) { - def context = new DDSpanContext( - 1L, - 1L, - 0L, - "fakeService", - "fakeOperation", - "fakeResource", - PrioritySampling.UNSET, - Collections.emptyMap(), - false, - "fakeType", - Collections.emptyMap(), - null, - tracer) - return new DDSpan(1, context) - } -} diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/sampling/RateByServiceSamplerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/sampling/RateByServiceSamplerTest.groovy index ef4480d52c6..93afde98597 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/sampling/RateByServiceSamplerTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/sampling/RateByServiceSamplerTest.groovy @@ -2,8 +2,7 @@ package datadog.trace.api.sampling import com.fasterxml.jackson.databind.ObjectMapper import datadog.opentracing.DDSpan -import datadog.opentracing.DDSpanContext -import datadog.opentracing.DDTracer +import datadog.opentracing.SpanFactory import datadog.trace.common.sampling.PrioritySampling import datadog.trace.common.sampling.RateByServiceSampler import spock.lang.Specification @@ -20,20 +19,20 @@ class RateByServiceSamplerTest extends Specification { when: String response = '{"rate_by_service": {"service:,env:":1.0, "service:spock,env:test":0.000001}}' serviceSampler.onResponse("traces", serializer.readTree(response)) - DDSpan span1 = makeTrace("foo", "bar") + DDSpan span1 = SpanFactory.newSpanOf("foo", "bar") serviceSampler.initializeSamplingPriority(span1) then: span1.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP serviceSampler.sample(span1) - // !serviceSampler.sample(makeTrace("spock", "test")) + // !serviceSampler.sample(SpanFactory.newSpanOf("spock", "test")) when: response = '{"rate_by_service": {"service:,env:":0.000001, "service:spock,env:test":1.0}}' serviceSampler.onResponse("traces", serializer.readTree(response)) - DDSpan span2 = makeTrace("spock", "test") + DDSpan span2 = SpanFactory.newSpanOf("spock", "test") serviceSampler.initializeSamplingPriority(span2) then: - // !serviceSampler.sample(makeTrace("foo", "bar")) + // !serviceSampler.sample(SpanFactory.newSpanOf("foo", "bar")) span2.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP serviceSampler.sample(span2) } @@ -45,29 +44,10 @@ class RateByServiceSamplerTest extends Specification { String response = '{"rate_by_service": {"service:,env:":1.0}}' serviceSampler.onResponse("traces", serializer.readTree(response)) - DDSpan span = makeTrace("foo", "bar") + DDSpan span = SpanFactory.newSpanOf("foo", "bar") serviceSampler.initializeSamplingPriority(span) expect: // sets correctly on root span span.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP } - - private DDSpan makeTrace(String serviceName, String envName) { - def context = new DDSpanContext( - 1L, - 1L, - 0L, - serviceName, - "fakeOperation", - "fakeResource", - PrioritySampling.UNSET, - Collections.emptyMap(), - false, - "fakeType", - Collections.emptyMap(), - null, - new DDTracer()) - context.setTag("env", envName) - return new DDSpan(0l, context) - } } diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy index 8119f408254..72679c681df 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy @@ -7,7 +7,7 @@ import datadog.trace.common.writer.WriterQueue import spock.lang.Specification import spock.lang.Timeout -import static datadog.trace.SpanFactory.newSpanOf +import static datadog.opentracing.SpanFactory.newSpanOf import static org.mockito.Mockito.mock import static org.mockito.Mockito.verifyNoMoreInteractions diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy index 19cae1be62e..a548d5988e0 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy @@ -3,7 +3,7 @@ package datadog.trace.api.writer import com.fasterxml.jackson.core.type.TypeReference import com.fasterxml.jackson.databind.JsonNode import com.fasterxml.jackson.databind.ObjectMapper -import datadog.trace.SpanFactory +import datadog.opentracing.SpanFactory import datadog.trace.common.Service import datadog.trace.common.writer.DDApi import datadog.trace.common.writer.DDApi.ResponseListener diff --git a/dd-trace-ot/src/test/java/datadog/opentracing/TestInterceptor.java b/dd-trace-ot/src/test/java/datadog/opentracing/TestInterceptor.java new file mode 100644 index 00000000000..6084008cf38 --- /dev/null +++ b/dd-trace-ot/src/test/java/datadog/opentracing/TestInterceptor.java @@ -0,0 +1,22 @@ +package datadog.opentracing; + +import com.google.auto.service.AutoService; +import datadog.trace.api.interceptor.MutableSpan; +import datadog.trace.api.interceptor.TraceInterceptor; +import java.util.Collection; + +@AutoService(TraceInterceptor.class) +public class TestInterceptor implements TraceInterceptor { + public volatile int priority = 0; + + @Override + public Collection onTraceComplete( + final Collection trace) { + return trace; + } + + @Override + public int priority() { + return priority; + } +} diff --git a/settings.gradle b/settings.gradle index 89ae5570b68..3605f150160 100644 --- a/settings.gradle +++ b/settings.gradle @@ -12,8 +12,10 @@ include ':dd-trace-api' // instrumentation: include ':dd-java-agent:instrumentation:apache-httpclient-4.3' include ':dd-java-agent:instrumentation:aws-sdk' +include ':dd-java-agent:instrumentation:classloaders' include ':dd-java-agent:instrumentation:datastax-cassandra-3.2' include ':dd-java-agent:instrumentation:jax-rs' +include ':dd-java-agent:instrumentation:java-concurrent' include ':dd-java-agent:instrumentation:jboss-classloading' include ':dd-java-agent:instrumentation:jdbc' include ':dd-java-agent:instrumentation:jedis-1.4'