Allow configuration of additional trace annotations
List can be set to empty if needed.
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
package datadog.trace.instrumentation.trace_annotation;
+import static datadog.trace.instrumentation.trace_annotation.TraceConfigInstrumentation.PACKAGE_CLASS_NAME_REGEX;
import static net.bytebuddy.matcher.ElementMatchers.declaresMethod;
import static net.bytebuddy.matcher.ElementMatchers.failSafe;
import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
@@ -8,19 +9,32 @@ import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith;
import static net.bytebuddy.matcher.ElementMatchers.named;
import com.google.auto.service.AutoService;
+import com.google.common.collect.Sets;
import datadog.trace.agent.tooling.DDAdvice;
import datadog.trace.agent.tooling.DDTransformers;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.Trace;
+import java.util.Collections;
+import java.util.Set;
+import lombok.extern.slf4j.Slf4j;
import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.description.NamedElement;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
+@Slf4j
@AutoService(Instrumenter.class)
public final class TraceAnnotationInstrumentation extends Instrumenter.Configurable {
+ private static final String CONFIG_NAME = "dd.trace.annotations";
- private static final String[] ADDITIONAL_ANNOTATIONS =
+ static final String CONFIG_FORMAT =
+ "(?:\\s*"
+ + PACKAGE_CLASS_NAME_REGEX
+ + "\\s*;)*\\s*"
+ + PACKAGE_CLASS_NAME_REGEX
+ + "\\s*;?\\s*";
+
+ private static final String[] DEFAULT_ANNOTATIONS =
new String[] {
"com.newrelic.api.agent.Trace",
"kamon.annotation.Trace",
@@ -29,15 +43,39 @@ public final class TraceAnnotationInstrumentation extends Instrumenter.Configura
"org.springframework.cloud.sleuth.annotation.NewSpan"
};
+ private final Set<String> additionalTraceAnnotations;
+
public TraceAnnotationInstrumentation() {
super("trace", "trace-annotation");
+
+ final String configString = getPropOrEnv(CONFIG_NAME);
+ if (configString == null) {
+ additionalTraceAnnotations =
+ Collections.unmodifiableSet(Sets.<String>newHashSet(DEFAULT_ANNOTATIONS));
+ } else if (configString.trim().isEmpty()) {
+ additionalTraceAnnotations = Collections.emptySet();
+ } else if (!configString.matches(CONFIG_FORMAT)) {
+ log.warn(
+ "Invalid trace annotations config '{}'. Must match 'package.Annotation$Name;*'.",
+ configString);
+ additionalTraceAnnotations = Collections.emptySet();
+ } else {
+ final Set<String> annotations = Sets.newHashSet();
+ final String[] annotationClasses = configString.split(";", -1);
+ for (final String annotationClass : annotationClasses) {
+ if (!annotationClass.trim().isEmpty()) {
+ annotations.add(annotationClass.trim());
+ }
+ }
+ additionalTraceAnnotations = Collections.unmodifiableSet(annotations);
+ }
}
@Override
public AgentBuilder apply(final AgentBuilder agentBuilder) {
ElementMatcher.Junction<NamedElement> methodTraceMatcher =
is(new TypeDescription.ForLoadedType(Trace.class));
- for (final String annotationName : ADDITIONAL_ANNOTATIONS) {
+ for (final String annotationName : additionalTraceAnnotations) {
methodTraceMatcher = methodTraceMatcher.or(named(annotationName));
}
return agentBuilder
@@ -48,4 +86,12 @@ public final class TraceAnnotationInstrumentation extends Instrumenter.Configura
.advice(isAnnotatedWith(methodTraceMatcher), TraceAdvice.class.getName()))
.asDecorator();
}
+
+ private String getPropOrEnv(final String name) {
+ return System.getProperty(name, System.getenv(propToEnvName(name)));
+ }
+
+ static String propToEnvName(final String name) {
+ return name.toUpperCase().replace(".", "_");
+ }
}

MODIFY
dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceConfigInstrumentation.java
dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceConfigInstrumentation.java
@@ -21,6 +21,19 @@ import net.bytebuddy.matcher.ElementMatcher;
public class TraceConfigInstrumentation extends Instrumenter.Configurable {
private static final String CONFIG_NAME = "dd.trace.methods";
+ static final String PACKAGE_CLASS_NAME_REGEX = "[\\w.\\$]+";
+ private static final String METHOD_LIST_REGEX = "\\s*(?:\\w+\\s*,)*\\s*(?:\\w+\\s*,?)\\s*";
+ private static final String CONFIG_FORMAT =
+ "(?:\\s*"
+ + PACKAGE_CLASS_NAME_REGEX
+ + "\\["
+ + METHOD_LIST_REGEX
+ + "\\]\\s*;)*\\s*"
+ + PACKAGE_CLASS_NAME_REGEX
+ + "\\["
+ + METHOD_LIST_REGEX
+ + "\\]\\s*;?\\s*";
+
private final Map<String, Set<String>> classMethodsToTrace;
public TraceConfigInstrumentation() {
@@ -30,10 +43,10 @@ public class TraceConfigInstrumentation extends Instrumenter.Configurable {
if (configString == null || configString.trim().isEmpty()) {
classMethodsToTrace = Collections.emptyMap();
- } else if (!configString.matches(
- "(?:([\\w.\\$]+)\\[((?:\\w+,)*(?:\\w+,?))\\];)*([\\w.\\$]+)\\[((?:\\w+,)*(?:\\w+,?))\\];?")) {
+ } else if (!configString.matches(CONFIG_FORMAT)) {
log.warn(
- "Invalid config '{}'. Must match 'package.Class$Name[method1,method2];*'.", configString);
+ "Invalid trace method config '{}'. Must match 'package.Class$Name[method1,method2];*'.",
+ configString);
classMethodsToTrace = Collections.emptyMap();
} else {
@@ -42,10 +55,20 @@ public class TraceConfigInstrumentation extends Instrumenter.Configurable {
for (final String classMethod : classMethods) {
final String[] splitClassMethod = classMethod.split("\\[");
final String className = splitClassMethod[0];
- final String methodNames =
- splitClassMethod[1].substring(0, splitClassMethod[1].length() - 1);
+ final String method = splitClassMethod[1].trim();
+ final String methodNames = method.substring(0, method.length() - 1);
final String[] splitMethodNames = methodNames.split(",");
- toTrace.put(className, Sets.newHashSet(splitMethodNames));
+ final Set<String> trimmedMethodNames =
+ Sets.newHashSetWithExpectedSize(splitMethodNames.length);
+ for (final String methodName : splitMethodNames) {
+ final String trimmedMethodName = methodName.trim();
+ if (!trimmedMethodName.isEmpty()) {
+ trimmedMethodNames.add(trimmedMethodName);
+ }
+ }
+ if (!trimmedMethodNames.isEmpty()) {
+ toTrace.put(className.trim(), trimmedMethodNames);
+ }
}
classMethodsToTrace = Collections.unmodifiableMap(toTrace);
}

ADD
dd-java-agent/instrumentation/trace-annotation/src/test/groovy/ConfiguredTraceAnnotationsTest.groovy
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.instrumentation.trace_annotation.TraceAnnotationInstrumentation
+import dd.test.trace.annotation.SayTracedHello
+import spock.lang.Unroll
+
+import java.util.concurrent.Callable
+
+import static datadog.trace.agent.test.ListWriterAssert.assertTraces
+import static datadog.trace.agent.test.TestUtils.withSystemProperty
+import static datadog.trace.instrumentation.trace_annotation.TraceAnnotationInstrumentation.DEFAULT_ANNOTATIONS
+
+class ConfiguredTraceAnnotationsTest extends AgentTestRunner {
+
+ static {
+ // nr annotation not included here, so should be disabled.
+ System.setProperty("dd.trace.annotations", "package.Class\$Name;${OuterClass.InterestingMethod.name}")
+ }
+
+ def specCleanup() {
+ System.clearProperty("dd.trace.annotations")
+ }
+
+ def "test disabled nr annotation"() {
+ setup:
+ SayTracedHello.fromCallable()
+
+ expect:
+ TEST_WRITER == []
+ }
+
+ def "test custom annotation based trace"() {
+ expect:
+ new AnnotationTracedCallable().call() == "Hello!"
+
+ when:
+ TEST_WRITER.waitForTraces(1)
+
+ then:
+ assertTraces(TEST_WRITER, 1) {
+ trace(0, 1) {
+ span(0) {
+ resourceName "AnnotationTracedCallable.call"
+ operationName "AnnotationTracedCallable.call"
+ }
+ }
+ }
+ }
+
+ @Unroll
+ def "test configuration #value"() {
+ setup:
+ def config = null
+ withSystemProperty("dd.trace.annotations", value) {
+ def instrumentation = new TraceAnnotationInstrumentation()
+ config = instrumentation.additionalTraceAnnotations
+ }
+ expect:
+ config == expected.toSet()
+
+ where:
+ value | expected
+ null | DEFAULT_ANNOTATIONS.toList()
+ " " | []
+ "some.Invalid[]" | []
+ "some.package.ClassName " | ["some.package.ClassName"]
+ " some.package.Class\$Name" | ["some.package.Class\$Name"]
+ " ClassName " | ["ClassName"]
+ "ClassName" | ["ClassName"]
+ "Class\$1;Class\$2;" | ["Class\$1", "Class\$2"]
+ "Duplicate ;Duplicate ;Duplicate; " | ["Duplicate"]
+ }
+
+ class AnnotationTracedCallable implements Callable<String> {
+ @OuterClass.InterestingMethod
+ @Override
+ String call() throws Exception {
+ return "Hello!"
+ }
+ }
+}

MODIFY
dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy
dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy
@@ -9,6 +9,10 @@ import static datadog.trace.agent.test.ListWriterAssert.assertTraces
class TraceAnnotationsTest extends AgentTestRunner {
+ static {
+ System.clearProperty("dd.trace.annotations")
+ }
+
def "test simple case annotations"() {
setup:
// Test single span in new trace
@@ -143,33 +147,4 @@ class TraceAnnotationsTest extends AgentTestRunner {
}
}
}
-
- def "test configuration based trace"() {
- expect:
- new ConfigTracedCallable().call() == "Hello!"
-
- when:
- TEST_WRITER.waitForTraces(1)
-
- then:
- assertTraces(TEST_WRITER, 1) {
- trace(0, 1) {
- span(0) {
- resourceName "ConfigTracedCallable.call"
- operationName "ConfigTracedCallable.call"
- }
- }
- }
- }
-
- static {
- System.setProperty("dd.trace.methods", "package.ClassName[method1,method2];${ConfigTracedCallable.name}[call]")
- }
-
- class ConfigTracedCallable implements Callable<String> {
- @Override
- String call() throws Exception {
- return "Hello!"
- }
- }
}

ADD
dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.instrumentation.trace_annotation.TraceConfigInstrumentation
+import spock.lang.Unroll
+
+import java.util.concurrent.Callable
+
+import static datadog.trace.agent.test.ListWriterAssert.assertTraces
+import static datadog.trace.agent.test.TestUtils.withSystemProperty
+
+class TraceConfigTest extends AgentTestRunner {
+
+ static {
+ System.setProperty("dd.trace.methods", "package.ClassName[method1,method2];${ConfigTracedCallable.name}[call]")
+ }
+
+ def specCleanup() {
+ System.clearProperty("dd.trace.methods")
+ }
+
+ class ConfigTracedCallable implements Callable<String> {
+ @Override
+ String call() throws Exception {
+ return "Hello!"
+ }
+ }
+
+ def "test configuration based trace"() {
+ expect:
+ new ConfigTracedCallable().call() == "Hello!"
+
+ when:
+ TEST_WRITER.waitForTraces(1)
+
+ then:
+ assertTraces(TEST_WRITER, 1) {
+ trace(0, 1) {
+ span(0) {
+ resourceName "ConfigTracedCallable.call"
+ operationName "ConfigTracedCallable.call"
+ }
+ }
+ }
+ }
+
+ @Unroll
+ def "test configuration #value"() {
+ setup:
+ def config = null
+ withSystemProperty("dd.trace.methods", value) {
+ def instrumentation = new TraceConfigInstrumentation()
+ config = instrumentation.classMethodsToTrace
+ }
+ expect:
+
+ config == expected
+
+ where:
+ value | expected
+ null | [:]
+ " " | [:]
+ "some.package.ClassName" | [:]
+ "some.package.ClassName[ , ]" | [:]
+ "some.package.Class\$Name[ method , ]" | ["some.package.Class\$Name": ["method"].toSet()]
+ "ClassName[ method1,]" | ["ClassName": ["method1"].toSet()]
+ "ClassName[method1 , method2]" | ["ClassName": ["method1", "method2"].toSet()]
+ "Class\$1[method1 ] ; Class\$2[ method2];" | ["Class\$1": ["method1"].toSet(), "Class\$2": ["method2"].toSet()]
+ "Duplicate[method1] ; Duplicate[method2] ;Duplicate[method3];" | ["Duplicate": ["method3"].toSet()]
+ }
+}

ADD
dd-java-agent/instrumentation/trace-annotation/src/test/java/OuterClass.java
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+public class OuterClass {
+
+ @Retention(RUNTIME)
+ @Target(METHOD)
+ public @interface InterestingMethod {}
+}

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
@@ -127,7 +127,7 @@ public abstract class AgentTestRunner extends Specification {
}
@AfterClass
- public static synchronized void agentClenup() {
+ public static synchronized void agentCleanup() {
if (null != activeTransformer) {
instrumentation.removeTransformer(activeTransformer);
activeTransformer = null;

MODIFY
dd-java-agent/testing/src/main/java/datadog/trace/agent/test/TestUtils.java
dd-java-agent/testing/src/main/java/datadog/trace/agent/test/TestUtils.java
@@ -4,7 +4,11 @@ import datadog.trace.agent.tooling.Utils;
import io.opentracing.Scope;
import io.opentracing.Tracer;
import io.opentracing.util.GlobalTracer;
-import java.io.*;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
import java.lang.reflect.Field;
import java.net.ServerSocket;
import java.net.URL;
@@ -56,6 +60,22 @@ public class TestUtils {
}
}
+ public static <T extends Object> Object withSystemProperty(
+ final String name, final String value, final Callable<T> r) {
+ if (value == null) {
+ System.clearProperty(name);
+ } else {
+ System.setProperty(name, value);
+ }
+ try {
+ return r.call();
+ } catch (final Exception e) {
+ throw new IllegalStateException(e);
+ } finally {
+ System.clearProperty(name);
+ }
+ }
+
public static <T extends Object> Object runUnderTrace(
final String rootOperationName, final Callable<T> r) {
final Scope scope = GlobalTracer.get().buildSpan(rootOperationName).startActive(true);
@@ -68,10 +88,10 @@ public class TestUtils {
}
}
- public static byte[] convertToByteArray(InputStream resource) throws IOException {
- ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+ public static byte[] convertToByteArray(final InputStream resource) throws IOException {
+ final ByteArrayOutputStream buffer = new ByteArrayOutputStream();
int bytesRead;
- byte[] data = new byte[1024];
+ final byte[] data = new byte[1024];
while ((bytesRead = resource.read(data, 0, data.length)) != -1) {
buffer.write(data, 0, bytesRead);
}
@@ -79,7 +99,7 @@ public class TestUtils {
return buffer.toByteArray();
}
- public static byte[] convertToByteArray(Class<?> clazz) throws IOException {
+ public static byte[] convertToByteArray(final Class<?> clazz) throws IOException {
InputStream inputStream = null;
try {
inputStream =
@@ -164,13 +184,13 @@ public class TestUtils {
/** Open up a random, reusable port. */
public static int randomOpenPort() {
- ServerSocket socket;
+ final ServerSocket socket;
try {
socket = new ServerSocket(0);
socket.setReuseAddress(true);
socket.close();
return socket.getLocalPort();
- } catch (IOException ioe) {
+ } catch (final IOException ioe) {
ioe.printStackTrace();
return -1;
}

Allow configuration of additional trace annotations

List can be set to empty if needed.