Fix AgentInstaller ignores and naming
DELETE
dd-java-agent-ittests/src/test/java/datadog/trace/agent/instrumentation/annotation/TraceAnnotationsTest.java
-package datadog.trace.agent.instrumentation.annotation;
-
-import static org.assertj.core.api.Assertions.assertThat;
-
-import datadog.opentracing.DDSpan;
-import datadog.opentracing.DDTracer;
-import datadog.opentracing.decorators.ErrorFlag;
-import datadog.trace.agent.test.IntegrationTestUtils;
-import datadog.trace.agent.test.SayTracedHello;
-import datadog.trace.common.writer.ListWriter;
-import io.opentracing.util.GlobalTracer;
-import java.io.PrintWriter;
-import java.io.StringWriter;
-import org.junit.Before;
-import org.junit.Test;
-
-public class TraceAnnotationsTest {
- private final ListWriter writer = new ListWriter();
- private final DDTracer tracer = new DDTracer(writer);
-
- @Before
- public void beforeTest() throws Exception {
- IntegrationTestUtils.registerOrReplaceGlobalTracer(tracer);
-
- writer.start();
- assertThat(GlobalTracer.isRegistered()).isTrue();
- }
-
- @Test
- public void testSimpleCaseAnnotations() {
- // Test single span in new trace
- SayTracedHello.sayHello();
-
- assertThat(writer.firstTrace().size()).isEqualTo(1);
- assertThat(writer.firstTrace().get(0).getOperationName()).isEqualTo("SayTracedHello.sayHello");
- assertThat(writer.firstTrace().get(0).getServiceName()).isEqualTo("test");
- }
-
- @Test
- public void testComplexCaseAnnotations() {
-
- // Test new trace with 2 children spans
- SayTracedHello.sayHELLOsayHA();
- assertThat(writer.firstTrace().size()).isEqualTo(3);
- final long parentId = writer.firstTrace().get(0).context().getSpanId();
-
- assertThat(writer.firstTrace().get(0).getOperationName()).isEqualTo("NEW_TRACE");
- assertThat(writer.firstTrace().get(0).getParentId()).isEqualTo(0); // ROOT / no parent
- assertThat(writer.firstTrace().get(0).context().getParentId()).isEqualTo(0);
- assertThat(writer.firstTrace().get(0).getServiceName()).isEqualTo("test2");
-
- 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("SayTracedHello.sayHello");
- assertThat(writer.firstTrace().get(2).getServiceName()).isEqualTo("test");
- assertThat(writer.firstTrace().get(2).getParentId()).isEqualTo(parentId);
- }
-
- @Test
- public void testExceptionExit() {
-
- tracer.addDecorator(new ErrorFlag());
-
- Throwable error = null;
- try {
- SayTracedHello.sayERROR();
- } catch (final Throwable ex) {
- error = ex;
- }
-
- final StringWriter errorString = new StringWriter();
- error.printStackTrace(new PrintWriter(errorString));
-
- final DDSpan span = writer.firstTrace().get(0);
- assertThat(span.getOperationName()).isEqualTo("ERROR");
- assertThat(span.getTags().get("error")).isEqualTo(true);
- assertThat(span.getTags().get("error.msg")).isEqualTo(error.getMessage());
- assertThat(span.getTags().get("error.type")).isEqualTo(error.getClass().getName());
- assertThat(span.getTags().get("error.stack")).isEqualTo(errorString.toString());
- assertThat(span.getError()).isEqualTo(1);
- }
-}

MODIFY
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java
@@ -5,6 +5,7 @@ import static net.bytebuddy.matcher.ElementMatchers.any;
import static net.bytebuddy.matcher.ElementMatchers.nameContains;
import static net.bytebuddy.matcher.ElementMatchers.nameMatches;
import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
+import static net.bytebuddy.matcher.ElementMatchers.not;
import java.lang.instrument.Instrumentation;
import java.util.ServiceLoader;
@@ -42,11 +43,12 @@ public class AgentInstaller {
.with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
.with(new LoggingListener())
.with(new DDLocationStrategy())
- .ignore(nameStartsWith("datadog.trace."))
+ .ignore(any(), skipClassLoader())
+ .or(nameStartsWith("datadog.trace."))
.or(nameStartsWith("datadog.opentracing."))
.or(nameStartsWith("datadog.slf4j."))
- .or(nameStartsWith("java."))
- .or(nameStartsWith("com.sun."))
+ .or(nameStartsWith("java.").and(not(nameStartsWith("java.util.concurrent."))))
+ .or(nameStartsWith("com.sun.").and(not(nameStartsWith("com.sun.proxy."))))
.or(nameStartsWith("sun."))
.or(nameStartsWith("jdk."))
.or(nameStartsWith("org.aspectj."))
@@ -55,8 +57,7 @@ public class AgentInstaller {
.or(nameStartsWith("org.slf4j."))
.or(nameContains("javassist"))
.or(nameContains(".asm."))
- .or(nameMatches("com\\.mchange\\.v2\\.c3p0\\..*Proxy"))
- .ignore(any(), skipClassLoader());
+ .or(nameMatches("com\\.mchange\\.v2\\.c3p0\\..*Proxy"));
for (final AgentBuilder.Listener listener : listeners) {
agentBuilder = agentBuilder.with(listener);
}

MODIFY
dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationInstrumentation.java
dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationInstrumentation.java
@@ -44,7 +44,18 @@ public final class TraceAnnotationInstrumentation extends Instrumenter.Configura
final Trace trace = method.getAnnotation(Trace.class);
String operationName = trace == null ? null : trace.operationName();
if (operationName == null || operationName.isEmpty()) {
- operationName = method.getDeclaringClass().getSimpleName() + "." + method.getName();
+ final Class<?> declaringClass = method.getDeclaringClass();
+ String className = declaringClass.getSimpleName();
+ if (className.isEmpty()) {
+ className = declaringClass.getName();
+ if (declaringClass.getPackage() != null) {
+ final String pkgName = declaringClass.getPackage().getName();
+ if (!pkgName.isEmpty()) {
+ className = declaringClass.getName().replace(pkgName, "").substring(1);
+ }
+ }
+ }
+ operationName = className + "." + method.getName();
}
return GlobalTracer.get().buildSpan(operationName).startActive(true);

ADD
dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy
+import datadog.opentracing.DDSpan
+import datadog.opentracing.decorators.ErrorFlag
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.Trace
+import dd.test.trace.annotation.SayTracedHello
+import org.assertj.core.api.Assertions
+
+import java.util.concurrent.Callable
+
+class TraceAnnotationsTest extends AgentTestRunner {
+
+ def "test simple case annotations"() {
+ setup:
+ // Test single span in new trace
+ SayTracedHello.sayHello()
+
+ Assertions.assertThat(TEST_WRITER.firstTrace().size()).isEqualTo(1)
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(0).getOperationName())
+ .isEqualTo("SayTracedHello.sayHello")
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(0).getServiceName()).isEqualTo("test")
+ }
+
+ def "test complex case annotations"() {
+ setup:
+
+ // Test new trace with 2 children spans
+ SayTracedHello.sayHELLOsayHA()
+ Assertions.assertThat(TEST_WRITER.firstTrace().size()).isEqualTo(3)
+ final long parentId = TEST_WRITER.firstTrace().get(0).context().getSpanId()
+
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(0).getOperationName()).isEqualTo("NEW_TRACE")
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(0).getParentId())
+ .isEqualTo(0) // ROOT / no parent
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(0).context().getParentId()).isEqualTo(0)
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(0).getServiceName()).isEqualTo("test2")
+
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(1).getOperationName()).isEqualTo("SAY_HA")
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(1).getParentId()).isEqualTo(parentId)
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(1).context().getSpanType()).isEqualTo("DB")
+
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(2).getOperationName())
+ .isEqualTo("SayTracedHello.sayHello")
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(2).getServiceName()).isEqualTo("test")
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(2).getParentId()).isEqualTo(parentId)
+ }
+
+ def "test exception exit"() {
+ setup:
+
+ TEST_TRACER.addDecorator(new ErrorFlag())
+
+ Throwable error = null
+ try {
+ SayTracedHello.sayERROR()
+ } catch (final Throwable ex) {
+ error = ex
+ }
+
+ final StringWriter errorString = new StringWriter()
+ error.printStackTrace(new PrintWriter(errorString))
+
+ final DDSpan span = TEST_WRITER.firstTrace().get(0)
+ Assertions.assertThat(span.getOperationName()).isEqualTo("ERROR")
+ Assertions.assertThat(span.getTags().get("error")).isEqualTo(true)
+ Assertions.assertThat(span.getTags().get("error.msg")).isEqualTo(error.getMessage())
+ Assertions.assertThat(span.getTags().get("error.type")).isEqualTo(error.getClass().getName())
+ Assertions.assertThat(span.getTags().get("error.stack")).isEqualTo(errorString.toString())
+ Assertions.assertThat(span.getError()).isEqualTo(1)
+ }
+
+ def "test annonymous class annotations"() {
+ setup:
+ // Test anonymous classes with package.
+ SayTracedHello.fromCallable()
+
+ Assertions.assertThat(TEST_WRITER.size()).isEqualTo(1)
+ Assertions.assertThat(TEST_WRITER.firstTrace().size()).isEqualTo(1)
+ Assertions.assertThat(TEST_WRITER.firstTrace().get(0).getOperationName())
+ .isEqualTo("SayTracedHello\$1.call")
+
+ when:
+ // Test anonymous classes with no package.
+ new Callable<String>() {
+ @Trace
+ @Override
+ String call() throws Exception {
+ return "Howdy!"
+ }
+ }.call()
+ TEST_WRITER.waitForTraces(2)
+
+ then:
+ Assertions.assertThat(TEST_WRITER.size()).isEqualTo(2)
+ Assertions.assertThat(TEST_WRITER.get(1).size()).isEqualTo(1)
+ Assertions.assertThat(TEST_WRITER.get(1).get(0).getOperationName())
+ .isEqualTo("TraceAnnotationsTest\$1.call")
+ }
+}

RENAME
dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/SayTracedHello.java
dd-java-agent/instrumentation/trace-annotation/src/test/java/dd/test/trace/annotation/SayTracedHello.java
-package datadog.trace.agent.test;
+package dd.test.trace.annotation;
import datadog.trace.api.DDTags;
import datadog.trace.api.Trace;
import io.opentracing.tag.StringTag;
import io.opentracing.util.GlobalTracer;
+import java.util.concurrent.Callable;
public class SayTracedHello {
@@ -33,4 +34,14 @@ public class SayTracedHello {
public static String sayERROR() {
throw new RuntimeException();
}
+
+ public static String fromCallable() throws Exception {
+ return new Callable<String>() {
+ @Trace
+ @Override
+ public String call() throws Exception {
+ return "Howdy!";
+ }
+ }.call();
+ }
}

MODIFY
dd-java-agent/instrumentation/trace-annotation/trace-annotation.gradle
dd-java-agent/instrumentation/trace-annotation/trace-annotation.gradle
@@ -7,4 +7,6 @@ dependencies {
compile deps.bytebuddy
compile deps.opentracing
compile deps.autoservice
+
+ testCompile project(':dd-java-agent:testing')
}

MODIFY
dd-java-agent/testing/src/main/java/datadog/trace/agent/test/AgentTestRunner.java
dd-java-agent/testing/src/main/java/datadog/trace/agent/test/AgentTestRunner.java
@@ -11,7 +11,7 @@ import datadog.trace.common.writer.Writer;
import io.opentracing.Tracer;
import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.Instrumentation;
-import java.util.*;
+import java.util.List;
import java.util.concurrent.Phaser;
import java.util.concurrent.atomic.AtomicInteger;
import net.bytebuddy.agent.ByteBuddyAgent;
@@ -56,7 +56,7 @@ public abstract class AgentTestRunner extends Specification {
// having a reference to io.opentracing.Tracer in test field
// loads opentracing before bootstrap classpath is setup
// so we declare tracer as an object and cast when needed.
- private static final Object TEST_TRACER;
+ protected static final Object TEST_TRACER;
private static final AtomicInteger INSTRUMENTATION_ERROR_COUNT = new AtomicInteger();
private static final Instrumentation instrumentation;

Fix AgentInstaller ignores and @Trace naming