From 6414de82d91d90848fc60048cfc5d22e4f9a1c23 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 25 Apr 2019 10:24:13 -0700 Subject: [PATCH] Add support for HystrixObservableCommand This change is slightly breaking for existing hystrixCommand code since the resource name changes from run->execute and getFallback->fallback. The fallback span is also now a child of the execute span. --- .../trace/agent/tooling/Constants.java | 10 +- .../hystrix-1.4/hystrix-1.4.gradle | 1 + .../HystrixCommandInstrumentation.java | 70 ------ .../hystrix/HystrixDecorator.java | 6 +- .../hystrix/HystrixInstrumentation.java | 206 ++++++++++++++++++ .../src/main/java/rx/DDTracingUtil.java | 10 + .../groovy/HystrixObservableChainTest.groovy | 125 +++++++++++ .../test/groovy/HystrixObservableTest.groovy | 182 ++++++++++++++++ .../src/test/groovy/HystrixTest.groovy | 44 ++-- ...syncPropagatingDisableInstrumentation.java | 102 +++++++++ .../test/groovy/JBossClassloadingTest.groovy | 2 +- .../test/groovy/OSGIClassloadingTest.groovy | 2 +- .../datadog/trace/agent/test/SpockRunner.java | 8 +- 13 files changed, 663 insertions(+), 105 deletions(-) delete mode 100644 dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java create mode 100644 dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixInstrumentation.java create mode 100644 dd-java-agent/instrumentation/hystrix-1.4/src/main/java/rx/DDTracingUtil.java create mode 100644 dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableChainTest.groovy create mode 100644 dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableTest.groovy create mode 100644 dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AsyncPropagatingDisableInstrumentation.java diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Constants.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Constants.java index b1081c90f1..d92b11ad72 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Constants.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Constants.java @@ -13,11 +13,11 @@ public final class Constants { *

Updates should be mirrored in TestUtils#BOOTSTRAP_PACKAGE_PREFIXES_COPY */ public static final String[] BOOTSTRAP_PACKAGE_PREFIXES = { - "io.opentracing", "datadog.slf4j", - "datadog.trace.bootstrap", "datadog.trace.api", - "datadog.trace.context" + "datadog.trace.bootstrap", + "datadog.trace.context", + "io.opentracing", }; // This is used in IntegrationTestUtils.java @@ -46,7 +46,9 @@ public final class Constants { "okio", "jnr", "org.objectweb.asm", - "com.kenai" + "com.kenai", + // Custom RxJava Utility + "rx.DDTracingUtil", }; private Constants() {} diff --git a/dd-java-agent/instrumentation/hystrix-1.4/hystrix-1.4.gradle b/dd-java-agent/instrumentation/hystrix-1.4/hystrix-1.4.gradle index dacf809375..faaca0f737 100644 --- a/dd-java-agent/instrumentation/hystrix-1.4/hystrix-1.4.gradle +++ b/dd-java-agent/instrumentation/hystrix-1.4/hystrix-1.4.gradle @@ -18,6 +18,7 @@ testSets { dependencies { compileOnly group: 'com.netflix.hystrix', name: 'hystrix-core', version: '1.4.0' + compileOnly group: 'io.reactivex', name: 'rxjava', version: '1.0.7' compile project(':dd-trace-ot') compile project(':dd-java-agent:agent-tooling') diff --git a/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java deleted file mode 100644 index d56c7af59c..0000000000 --- a/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java +++ /dev/null @@ -1,70 +0,0 @@ -package datadog.trace.instrumentation.hystrix; - -import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static datadog.trace.instrumentation.hystrix.HystrixDecorator.DECORATE; -import static java.util.Collections.singletonMap; -import static net.bytebuddy.matcher.ElementMatchers.isInterface; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; - -import com.google.auto.service.AutoService; -import com.netflix.hystrix.HystrixCommand; -import datadog.trace.agent.tooling.Instrumenter; -import io.opentracing.Scope; -import io.opentracing.util.GlobalTracer; -import java.util.Map; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -@AutoService(Instrumenter.class) -public class HystrixCommandInstrumentation extends Instrumenter.Default { - - private static final String OPERATION_NAME = "hystrix.cmd"; - - public HystrixCommandInstrumentation() { - super("hystrix"); - } - - @Override - public ElementMatcher typeMatcher() { - return not(isInterface()).and(safeHasSuperType(named("com.netflix.hystrix.HystrixCommand"))); - } - - @Override - public String[] helperClassNames() { - return new String[] { - "datadog.trace.agent.decorator.BaseDecorator", packageName + ".HystrixDecorator", - }; - } - - @Override - public Map, String> transformers() { - return singletonMap( - isMethod().and(named("run").or(named("getFallback"))), TraceAdvice.class.getName()); - } - - public static class TraceAdvice { - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope startSpan( - @Advice.This final HystrixCommand command, - @Advice.Origin("#m") final String methodName) { - - final Scope scope = GlobalTracer.get().buildSpan(OPERATION_NAME).startActive(true); - DECORATE.afterStart(scope); - DECORATE.onCommand(scope, command, methodName); - return scope; - } - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void stopSpan( - @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { - DECORATE.onError(scope, throwable); - DECORATE.beforeFinish(scope); - scope.close(); - } - } -} diff --git a/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixDecorator.java b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixDecorator.java index d6203d512f..7ba2498d1d 100644 --- a/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixDecorator.java +++ b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixDecorator.java @@ -1,9 +1,8 @@ package datadog.trace.instrumentation.hystrix; -import com.netflix.hystrix.HystrixCommand; +import com.netflix.hystrix.HystrixInvokableInfo; import datadog.trace.agent.decorator.BaseDecorator; import datadog.trace.api.DDTags; -import io.opentracing.Scope; import io.opentracing.Span; public class HystrixDecorator extends BaseDecorator { @@ -25,7 +24,7 @@ public class HystrixDecorator extends BaseDecorator { } public void onCommand( - final Scope scope, final HystrixCommand command, final String methodName) { + final Span span, final HystrixInvokableInfo command, final String methodName) { if (command != null) { final String commandName = command.getCommandKey().name(); final String groupName = command.getCommandGroup().name(); @@ -33,7 +32,6 @@ public class HystrixDecorator extends BaseDecorator { final String resourceName = groupName + "." + commandName + "." + methodName; - final Span span = scope.span(); span.setTag(DDTags.RESOURCE_NAME, resourceName); span.setTag("hystrix.command", commandName); span.setTag("hystrix.group", groupName); diff --git a/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixInstrumentation.java b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixInstrumentation.java new file mode 100644 index 0000000000..51ef00d7e0 --- /dev/null +++ b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixInstrumentation.java @@ -0,0 +1,206 @@ +package datadog.trace.instrumentation.hystrix; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.hystrix.HystrixDecorator.DECORATE; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; + +import com.google.auto.service.AutoService; +import com.netflix.hystrix.HystrixInvokableInfo; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.ScopeManager; +import io.opentracing.Span; +import io.opentracing.Tracer; +import io.opentracing.util.GlobalTracer; +import java.util.HashMap; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import rx.DDTracingUtil; +import rx.Observable; +import rx.Subscriber; + +@AutoService(Instrumenter.class) +public class HystrixInstrumentation extends Instrumenter.Default { + + private static final String OPERATION_NAME = "hystrix.cmd"; + + public HystrixInstrumentation() { + super("hystrix"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType( + named("com.netflix.hystrix.HystrixCommand") + .or(named("com.netflix.hystrix.HystrixObservableCommand"))); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "rx.DDTracingUtil", + "datadog.trace.agent.decorator.BaseDecorator", + packageName + ".HystrixDecorator", + packageName + ".HystrixInstrumentation$TracedSubscriber", + packageName + ".HystrixInstrumentation$TracedOnSubscribe", + }; + } + + @Override + public Map, String> transformers() { + final Map, String> transformers = new HashMap<>(); + transformers.put( + named("getExecutionObservable").and(returns(named("rx.Observable"))), + ExecuteAdvice.class.getName()); + transformers.put( + named("getFallbackObservable").and(returns(named("rx.Observable"))), + FallbackAdvice.class.getName()); + return transformers; + } + + public static class ExecuteAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.This final HystrixInvokableInfo command, + @Advice.Return(readOnly = false) Observable result, + @Advice.Thrown final Throwable throwable) { + final Observable.OnSubscribe onSubscribe = DDTracingUtil.extractOnSubscribe(result); + result = + Observable.create( + new TracedOnSubscribe( + onSubscribe, command, "execute", GlobalTracer.get().scopeManager().active())); + } + } + + public static class FallbackAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.This final HystrixInvokableInfo command, + @Advice.Return(readOnly = false) Observable result, + @Advice.Thrown final Throwable throwable) { + final Observable.OnSubscribe onSubscribe = DDTracingUtil.extractOnSubscribe(result); + result = + Observable.create( + new TracedOnSubscribe( + onSubscribe, command, "fallback", GlobalTracer.get().scopeManager().active())); + } + } + + public static class TracedOnSubscribe implements Observable.OnSubscribe { + + private final Observable.OnSubscribe delegate; + private final HystrixInvokableInfo command; + private final String methodName; + private final TraceScope.Continuation continuation; + + public TracedOnSubscribe( + final Observable.OnSubscribe delegate, + final HystrixInvokableInfo command, + final String methodName, + final Scope parentScope) { + this.delegate = delegate; + this.command = command; + this.methodName = methodName; + continuation = + parentScope instanceof TraceScope ? ((TraceScope) parentScope).capture() : null; + } + + @Override + public void call(final Subscriber subscriber) { + final Tracer tracer = GlobalTracer.get(); + final Span span; // span finished by TracedSubscriber + if (continuation != null) { + try (final TraceScope scope = continuation.activate()) { + span = tracer.buildSpan(OPERATION_NAME).start(); + } + } else { + span = tracer.buildSpan(OPERATION_NAME).start(); + } + DECORATE.afterStart(span); + DECORATE.onCommand(span, command, methodName); + + try (final Scope scope = tracer.scopeManager().activate(span, false)) { + delegate.call(new TracedSubscriber(span, subscriber)); + } + } + } + + public static class TracedSubscriber extends Subscriber { + + private final ScopeManager scopeManager = GlobalTracer.get().scopeManager(); + private final Span span; + private final Subscriber delegate; + + public TracedSubscriber(final Span span, final Subscriber delegate) { + this.span = span; + this.delegate = delegate; + } + + @Override + public void onStart() { + try (final Scope scope = scopeManager.activate(span, false)) { + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } + delegate.onStart(); + } + } + + @Override + public void onNext(final T value) { + try (final Scope scope = scopeManager.activate(span, false)) { + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } + delegate.onNext(value); + } catch (final Throwable e) { + onError(e); + } + } + + @Override + public void onCompleted() { + boolean errored = false; + try (final Scope scope = scopeManager.activate(span, false)) { + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } + delegate.onCompleted(); + } catch (final Throwable e) { + onError(e); + errored = true; + } finally { + // finish called by onError, so don't finish again. + if (!errored) { + DECORATE.beforeFinish(span); + span.finish(); + } + } + } + + @Override + public void onError(final Throwable e) { + try (final Scope scope = scopeManager.activate(span, false)) { + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } + DECORATE.onError(span, e); + delegate.onError(e); + } catch (final Throwable e2) { + DECORATE.onError(span, e2); + // This recursive call might be dangerous... not sure what the best response is. + onError(e2); + } finally { + DECORATE.beforeFinish(span); + span.finish(); + } + } + } +} diff --git a/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/rx/DDTracingUtil.java b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/rx/DDTracingUtil.java new file mode 100644 index 0000000000..a877d1385a --- /dev/null +++ b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/rx/DDTracingUtil.java @@ -0,0 +1,10 @@ +package rx; + +/** + * This class must be in the rx package in order to access the package accessible onSubscribe field. + */ +public class DDTracingUtil { + public static Observable.OnSubscribe extractOnSubscribe(final Observable observable) { + return observable.onSubscribe; + } +} diff --git a/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableChainTest.groovy b/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableChainTest.groovy new file mode 100644 index 0000000000..6d83b404b3 --- /dev/null +++ b/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableChainTest.groovy @@ -0,0 +1,125 @@ +import com.netflix.hystrix.HystrixObservableCommand +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.Trace +import io.opentracing.tag.Tags +import rx.Observable + +import static com.netflix.hystrix.HystrixCommandGroupKey.Factory.asKey +import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace + +class HystrixObservableChainTest extends AgentTestRunner { + // Uncomment for debugging: + // static { + // System.setProperty("hystrix.command.default.execution.timeout.enabled", "false") + // } + + def "test command #action"() { + setup: + def command = new HystrixObservableCommand(asKey("ExampleGroup")) { + @Trace + private String tracedMethod() { + return "Hello" + } + + @Override + protected Observable construct() { + Observable.defer { + Observable.just(tracedMethod()) + } + } + } + + def result = runUnderTrace("parent") { + command.toObservable().map { + it.toUpperCase() + }.flatMap { str -> + new HystrixObservableCommand(asKey("OtherGroup")) { + @Trace + private String tracedMethod() { + return "$str!" + } + + @Override + protected Observable construct() { + Observable.defer { + Observable.just(tracedMethod()) + } + } + }.toObservable() + }.toBlocking().first() + } + + expect: + result == "HELLO!" + + assertTraces(1) { + trace(0, 5) { + span(0) { + serviceName "unnamed-java-app" + operationName "parent" + resourceName "parent" + spanType null + parent() + errored false + tags { + defaultTags() + } + } + span(1) { + serviceName "unnamed-java-app" + operationName "hystrix.cmd" + resourceName "ExampleGroup.HystrixObservableChainTest\$1.execute" + spanType null + childOf span(0) + errored false + tags { + "hystrix.command" "HystrixObservableChainTest\$1" + "hystrix.group" "ExampleGroup" + "hystrix.circuit-open" false + "$Tags.COMPONENT.key" "hystrix" + defaultTags() + } + } + span(2) { + serviceName "unnamed-java-app" + operationName "hystrix.cmd" + resourceName "OtherGroup.HystrixObservableChainTest\$2.execute" + spanType null + childOf span(1) + errored false + tags { + "hystrix.command" "HystrixObservableChainTest\$2" + "hystrix.group" "OtherGroup" + "hystrix.circuit-open" false + "$Tags.COMPONENT.key" "hystrix" + defaultTags() + } + } + span(3) { + serviceName "unnamed-java-app" + operationName "HystrixObservableChainTest\$2.tracedMethod" + resourceName "HystrixObservableChainTest\$2.tracedMethod" + spanType null + childOf span(2) + errored false + tags { + "$Tags.COMPONENT.key" "trace" + defaultTags() + } + } + span(4) { + serviceName "unnamed-java-app" + operationName "HystrixObservableChainTest\$1.tracedMethod" + resourceName "HystrixObservableChainTest\$1.tracedMethod" + spanType null + childOf span(1) + errored false + tags { + "$Tags.COMPONENT.key" "trace" + defaultTags() + } + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableTest.groovy b/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableTest.groovy new file mode 100644 index 0000000000..0d74ba1a1d --- /dev/null +++ b/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableTest.groovy @@ -0,0 +1,182 @@ +import com.netflix.hystrix.HystrixObservable +import com.netflix.hystrix.HystrixObservableCommand +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.Trace +import io.opentracing.tag.Tags +import rx.Observable + +import java.util.concurrent.BlockingQueue +import java.util.concurrent.LinkedBlockingQueue + +import static com.netflix.hystrix.HystrixCommandGroupKey.Factory.asKey +import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace + +class HystrixObservableTest extends AgentTestRunner { + // Uncomment for debugging: + // static { + // System.setProperty("hystrix.command.default.execution.timeout.enabled", "false") + // } + + def "test command #action"() { + setup: + def command = new HystrixObservableCommand(asKey("ExampleGroup")) { + @Trace + private String tracedMethod() { + return "Hello!" + } + + @Override + protected Observable construct() { + Observable.defer { + Observable.just(tracedMethod()) + } + } + } + + def result = runUnderTrace("parent") { + operation(command) + } + + expect: + TRANSFORMED_CLASSES.contains("HystrixObservableTest\$1") + result == "Hello!" + + assertTraces(1) { + trace(0, 3) { + span(0) { + serviceName "unnamed-java-app" + operationName "parent" + resourceName "parent" + spanType null + parent() + errored false + tags { + defaultTags() + } + } + span(1) { + serviceName "unnamed-java-app" + operationName "hystrix.cmd" + resourceName "ExampleGroup.HystrixObservableTest\$1.execute" + spanType null + childOf span(0) + errored false + tags { + "hystrix.command" "HystrixObservableTest\$1" + "hystrix.group" "ExampleGroup" + "hystrix.circuit-open" false + "$Tags.COMPONENT.key" "hystrix" + defaultTags() + } + } + span(2) { + serviceName "unnamed-java-app" + operationName "HystrixObservableTest\$1.tracedMethod" + resourceName "HystrixObservableTest\$1.tracedMethod" + spanType null + childOf span(1) + errored false + tags { + "$Tags.COMPONENT.key" "trace" + defaultTags() + } + } + } + } + + where: + action | operation + "toObservable" | { HystrixObservable cmd -> cmd.toObservable().toBlocking().first() } + "observe" | { HystrixObservable cmd -> cmd.observe().toBlocking().first() } + "observe block" | { HystrixObservable cmd -> + BlockingQueue queue = new LinkedBlockingQueue() + cmd.observe().subscribe { next -> + queue.put(next) + } + queue.take() + } + } + + def "test command #action fallback"() { + setup: + def command = new HystrixObservableCommand(asKey("ExampleGroup")) { + @Override + protected Observable construct() { + Observable.defer { + Observable.error(new IllegalArgumentException()) + } + } + + protected Observable resumeWithFallback() { + return Observable.just("Fallback!") + } + } + + def result = runUnderTrace("parent") { + operation(command) + } + + expect: + TRANSFORMED_CLASSES.contains("HystrixObservableTest\$2") + result == "Fallback!" + + assertTraces(1) { + trace(0, 3) { + span(0) { + serviceName "unnamed-java-app" + operationName "parent" + resourceName "parent" + spanType null + parent() + errored false + tags { + defaultTags() + } + } + span(1) { + serviceName "unnamed-java-app" + operationName "hystrix.cmd" + resourceName "ExampleGroup.HystrixObservableTest\$2.execute" + spanType null + childOf span(0) + errored true + tags { + "hystrix.command" "HystrixObservableTest\$2" + "hystrix.group" "ExampleGroup" + "hystrix.circuit-open" false + "$Tags.COMPONENT.key" "hystrix" + errorTags(IllegalArgumentException) + defaultTags() + } + } + span(2) { + serviceName "unnamed-java-app" + operationName "hystrix.cmd" + resourceName "ExampleGroup.HystrixObservableTest\$2.fallback" + spanType null + childOf span(1) + errored false + tags { + "hystrix.command" "HystrixObservableTest\$2" + "hystrix.group" "ExampleGroup" + "hystrix.circuit-open" false + "$Tags.COMPONENT.key" "hystrix" + defaultTags() + } + } + } + } + + where: + action | operation + "toObservable" | { HystrixObservable cmd -> cmd.toObservable().toBlocking().first() } + "observe" | { HystrixObservable cmd -> cmd.observe().toBlocking().first() } + "observe block" | { HystrixObservable cmd -> + BlockingQueue queue = new LinkedBlockingQueue() + cmd.observe().subscribe { next -> + queue.put(next) + } + queue.take() + } + } +} diff --git a/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixTest.groovy b/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixTest.groovy index 70c978da46..0154435319 100644 --- a/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixTest.groovy +++ b/dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixTest.groovy @@ -17,9 +17,9 @@ class HystrixTest extends AgentTestRunner { def "test command #action"() { setup: - def command = new HystrixCommand(asKey("ExampleGroup")) { + def command = new HystrixCommand(asKey("ExampleGroup")) { @Override - protected Object run() throws Exception { + protected String run() throws Exception { return tracedMethod() } @@ -52,7 +52,7 @@ class HystrixTest extends AgentTestRunner { span(1) { serviceName "unnamed-java-app" operationName "hystrix.cmd" - resourceName "ExampleGroup.HystrixTest\$1.run" + resourceName "ExampleGroup.HystrixTest\$1.execute" spanType null childOf span(0) errored false @@ -83,6 +83,7 @@ class HystrixTest extends AgentTestRunner { action | operation "execute" | { HystrixCommand cmd -> cmd.execute() } "queue" | { HystrixCommand cmd -> cmd.queue().get() } + "toObservable" | { HystrixCommand cmd -> cmd.toObservable().toBlocking().first() } "observe" | { HystrixCommand cmd -> cmd.observe().toBlocking().first() } "observe block" | { HystrixCommand cmd -> BlockingQueue queue = new LinkedBlockingQueue() @@ -95,9 +96,9 @@ class HystrixTest extends AgentTestRunner { def "test command #action fallback"() { setup: - def command = new HystrixCommand(asKey("ExampleGroup")) { + def command = new HystrixCommand(asKey("ExampleGroup")) { @Override - protected Object run() throws Exception { + protected String run() throws Exception { throw new IllegalArgumentException() } @@ -129,22 +130,7 @@ class HystrixTest extends AgentTestRunner { span(1) { serviceName "unnamed-java-app" operationName "hystrix.cmd" - resourceName "ExampleGroup.HystrixTest\$2.getFallback" - spanType null - childOf span(0) - errored false - tags { - "hystrix.command" "HystrixTest\$2" - "hystrix.group" "ExampleGroup" - "hystrix.circuit-open" false - "$Tags.COMPONENT.key" "hystrix" - defaultTags() - } - } - span(2) { - serviceName "unnamed-java-app" - operationName "hystrix.cmd" - resourceName "ExampleGroup.HystrixTest\$2.run" + resourceName "ExampleGroup.HystrixTest\$2.execute" spanType null childOf span(0) errored true @@ -157,6 +143,21 @@ class HystrixTest extends AgentTestRunner { defaultTags() } } + span(2) { + serviceName "unnamed-java-app" + operationName "hystrix.cmd" + resourceName "ExampleGroup.HystrixTest\$2.fallback" + spanType null + childOf span(1) + errored false + tags { + "hystrix.command" "HystrixTest\$2" + "hystrix.group" "ExampleGroup" + "hystrix.circuit-open" false + "$Tags.COMPONENT.key" "hystrix" + defaultTags() + } + } } } @@ -164,6 +165,7 @@ class HystrixTest extends AgentTestRunner { action | operation "execute" | { HystrixCommand cmd -> cmd.execute() } "queue" | { HystrixCommand cmd -> cmd.queue().get() } + "toObservable" | { HystrixCommand cmd -> cmd.toObservable().toBlocking().first() } "observe" | { HystrixCommand cmd -> cmd.observe().toBlocking().first() } "observe block" | { HystrixCommand cmd -> BlockingQueue queue = new LinkedBlockingQueue() diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AsyncPropagatingDisableInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AsyncPropagatingDisableInstrumentation.java new file mode 100644 index 0000000000..d446c34f4d --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AsyncPropagatingDisableInstrumentation.java @@ -0,0 +1,102 @@ +package datadog.trace.instrumentation.java.concurrent; + +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableMap; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.ScopeManager; +import io.opentracing.noop.NoopSpan; +import io.opentracing.util.GlobalTracer; +import java.util.Map; +import lombok.extern.slf4j.Slf4j; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import net.bytebuddy.matcher.ElementMatchers; + +/** + * Sometimes classes do lazy initialization for scheduling of tasks. If this is done during a trace + * it can cause the trace to never be reported. Add matchers below to disable async propagation + * during this period. + */ +@Slf4j +@AutoService(Instrumenter.class) +public final class AsyncPropagatingDisableInstrumentation implements Instrumenter { + + private static final Map< + ElementMatcher, ElementMatcher> + CLASS_AND_METHODS = + new ImmutableMap.Builder< + ElementMatcher, ElementMatcher>() + .put(named("rx.internal.util.ObjectPool"), isConstructor()) + .build(); + + @Override + public AgentBuilder instrument(AgentBuilder agentBuilder) { + + for (final Map.Entry, ElementMatcher> + entry : CLASS_AND_METHODS.entrySet()) { + agentBuilder = + new DisableAsyncInstrumentation(entry.getKey(), entry.getValue()) + .instrument(agentBuilder); + } + return agentBuilder; + } + + // Not Using AutoService to hook up this instrumentation + public static class DisableAsyncInstrumentation extends Default { + + private final ElementMatcher typeMatcher; + private final ElementMatcher methodMatcher; + + /** No-arg constructor only used by muzzle and tests. */ + public DisableAsyncInstrumentation() { + this(ElementMatchers.none(), ElementMatchers.none()); + } + + public DisableAsyncInstrumentation( + final ElementMatcher typeMatcher, + final ElementMatcher methodMatcher) { + super(AbstractExecutorInstrumentation.EXEC_NAME); + this.typeMatcher = typeMatcher; + this.methodMatcher = methodMatcher; + } + + @Override + public ElementMatcher typeMatcher() { + return typeMatcher; + } + + @Override + public Map, String> transformers() { + return singletonMap(methodMatcher, DisableAsyncAdvice.class.getName()); + } + } + + public static class DisableAsyncAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope enter() { + final ScopeManager scopeManager = GlobalTracer.get().scopeManager(); + final Scope scope = scopeManager.active(); + if (scope instanceof TraceScope && ((TraceScope) scope).isAsyncPropagating()) { + return scopeManager.activate(NoopSpan.INSTANCE, false); + } + return null; + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void exit(@Advice.Enter final Scope scope) { + if (scope != null) { + scope.close(); + } + } + } +} diff --git a/dd-java-agent/instrumentation/jboss-classloading/src/test/groovy/JBossClassloadingTest.groovy b/dd-java-agent/instrumentation/jboss-classloading/src/test/groovy/JBossClassloadingTest.groovy index 48960e4602..98db8b6420 100644 --- a/dd-java-agent/instrumentation/jboss-classloading/src/test/groovy/JBossClassloadingTest.groovy +++ b/dd-java-agent/instrumentation/jboss-classloading/src/test/groovy/JBossClassloadingTest.groovy @@ -6,6 +6,6 @@ class JBossClassloadingTest extends AgentTestRunner { org.jboss.modules.Module.getName() expect: - System.getProperty("jboss.modules.system.pkgs") == "io.opentracing,datadog.slf4j,datadog.trace.bootstrap,datadog.trace.api,datadog.trace.context" + System.getProperty("jboss.modules.system.pkgs") == "datadog.slf4j,datadog.trace.api,datadog.trace.bootstrap,datadog.trace.context,io.opentracing" } } diff --git a/dd-java-agent/instrumentation/osgi-classloading/src/test/groovy/OSGIClassloadingTest.groovy b/dd-java-agent/instrumentation/osgi-classloading/src/test/groovy/OSGIClassloadingTest.groovy index 37cf7a4ec9..6ca92d074f 100644 --- a/dd-java-agent/instrumentation/osgi-classloading/src/test/groovy/OSGIClassloadingTest.groovy +++ b/dd-java-agent/instrumentation/osgi-classloading/src/test/groovy/OSGIClassloadingTest.groovy @@ -15,7 +15,7 @@ class OSGIClassloadingTest extends AgentTestRunner { org.osgi.framework.Bundle.getName() then: - System.getProperty("org.osgi.framework.bootdelegation") == "io.opentracing.*,io.opentracing,datadog.slf4j.*,datadog.slf4j,datadog.trace.bootstrap.*,datadog.trace.bootstrap,datadog.trace.api.*,datadog.trace.api,datadog.trace.context.*,datadog.trace.context" + System.getProperty("org.osgi.framework.bootdelegation") == "datadog.slf4j.*,datadog.slf4j,datadog.trace.api.*,datadog.trace.api,datadog.trace.bootstrap.*,datadog.trace.bootstrap,datadog.trace.context.*,datadog.trace.context,io.opentracing.*,io.opentracing" } def "test Eclipse OSGi framework factory"() { diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java index 149eb386c9..f803323c85 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java @@ -25,17 +25,17 @@ import org.spockframework.runtime.Sputnik; */ public class SpockRunner extends Sputnik { /** - * An exact copy of Utils#BOOSTRAP_PACKAGE_PREFIXES. + * An exact copy of {@link datadog.trace.agent.tooling.Constants#BOOTSTRAP_PACKAGE_PREFIXES}. * *

This list is needed to initialize the bootstrap classpath because Utils' static initializer * references bootstrap classes (e.g. DatadogClassLoader). */ public static final String[] BOOTSTRAP_PACKAGE_PREFIXES_COPY = { - "io.opentracing", "datadog.slf4j", - "datadog.trace.bootstrap", "datadog.trace.api", - "datadog.trace.context" + "datadog.trace.bootstrap", + "datadog.trace.context", + "io.opentracing", }; private static final String[] TEST_BOOTSTRAP_PREFIXES;