Add test for ignore matchers for libraries
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
package datadog.trace.agent.tooling;
import static datadog.trace.agent.tooling.ClassLoaderMatcher.skipClassLoader;
-import static datadog.trace.agent.tooling.bytebuddy.GlobalIgnoresMatcher.globalIgnoresMatcher;
+import static datadog.trace.agent.tooling.bytebuddy.matcher.AdditionalLibraryIgnoresMatcher.additionalLibraryIgnoresMatcher;
+import static datadog.trace.agent.tooling.bytebuddy.matcher.GlobalIgnoresMatcher.globalIgnoresMatcher;
import static net.bytebuddy.matcher.ElementMatchers.any;
import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.none;
+import static net.bytebuddy.matcher.ElementMatchers.not;
import datadog.trace.agent.tooling.context.FieldBackedProvider;
import datadog.trace.api.Config;
@@ -36,7 +38,7 @@ public class AgentInstaller {
public static void installBytebuddyAgent(final Instrumentation inst) {
if (Config.get().isTraceEnabled()) {
- installBytebuddyAgent(inst, new AgentBuilder.Listener[0]);
+ installBytebuddyAgent(inst, false, new AgentBuilder.Listener[0]);
} else {
log.debug("Tracing is disabled, not installing instrumentations.");
}
@@ -49,14 +51,16 @@ public class AgentInstaller {
* @return the agent's class transformer
*/
public static ResettableClassFileTransformer installBytebuddyAgent(
- final Instrumentation inst, final AgentBuilder.Listener... listeners) {
+ final Instrumentation inst,
+ final boolean skipAdditionalLibraryMatcher,
+ final AgentBuilder.Listener... listeners) {
INSTRUMENTATION = inst;
addByteBuddyRawSetting();
FieldBackedProvider.resetContextMatchers();
- AgentBuilder agentBuilder =
+ AgentBuilder.Ignored ignoredAgentBuilder =
new AgentBuilder.Default()
.disableClassFormatChanges()
.with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
@@ -67,10 +71,17 @@ public class AgentInstaller {
// FIXME: we cannot enable it yet due to BB/JVM bug, see
// https://github.com/raphw/byte-buddy/issues/558
// .with(AgentBuilder.LambdaInstrumentationStrategy.ENABLED)
- .ignore(any(), skipClassLoader())
- .or(globalIgnoresMatcher())
- .or(matchesConfiguredExcludes());
+ .ignore(any(), skipClassLoader());
+ if (skipAdditionalLibraryMatcher) {
+ ignoredAgentBuilder =
+ ignoredAgentBuilder.or(
+ globalIgnoresMatcher().and(not(additionalLibraryIgnoresMatcher())));
+ } else {
+ ignoredAgentBuilder = ignoredAgentBuilder.or(globalIgnoresMatcher());
+ }
+ ignoredAgentBuilder = ignoredAgentBuilder.or(matchesConfiguredExcludes());
+ AgentBuilder agentBuilder = ignoredAgentBuilder;
if (log.isDebugEnabled()) {
agentBuilder =
agentBuilder

ADD
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/AdditionalLibraryIgnoresMatcher.java
+package datadog.trace.agent.tooling.bytebuddy.matcher;
+
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+/**
+ * Additional global matchers that are used to reduce number of classes we try to apply expensive
+ * matchers to.
+ *
+ * <p>This is separated from {@link GlobalIgnoresMatcher} to allow for better testing. The idea is
+ * that we should be able to remove this matcher from the agent and all tests should still pass.
+ * Moreover, no classes matched by this matcher should be modified during test run.
+ */
+public class AdditionalLibraryIgnoresMatcher<T extends TypeDescription>
+ extends ElementMatcher.Junction.AbstractBase<T> {
+
+ public static <T extends TypeDescription> Junction<T> additionalLibraryIgnoresMatcher() {
+ return new AdditionalLibraryIgnoresMatcher<>();
+ }
+
+ /**
+ * Be very careful about the types of matchers used in this section as they are called on every
+ * class load, so they must be fast. Generally speaking try to only use name matchers as they
+ * don't have to load additional info.
+ */
+ @Override
+ public boolean matches(final T target) {
+ final String name = target.getActualName();
+
+ if (name.startsWith("org.springframework.aop.")
+ || name.startsWith("org.springframework.beans.factory.annotation.")
+ || name.startsWith("org.springframework.beans.factory.config.")
+ || name.startsWith("org.springframework.beans.factory.parsing.")
+ || name.startsWith("org.springframework.beans.factory.xml.")
+ || name.startsWith("org.springframework.beans.propertyeditors.")
+ || name.startsWith("org.springframework.boot.autoconfigure.cache.")
+ || name.startsWith("org.springframework.boot.autoconfigure.condition.")
+ || name.startsWith("org.springframework.boot.autoconfigure.http.")
+ || name.startsWith("org.springframework.boot.autoconfigure.jackson.")
+ || name.startsWith("org.springframework.boot.autoconfigure.web.")
+ || name.startsWith("org.springframework.boot.context.")
+ || name.startsWith("org.springframework.boot.convert.")
+ || name.startsWith("org.springframework.boot.diagnostics.")
+ || name.startsWith("org.springframework.boot.web.server.")
+ || name.startsWith("org.springframework.boot.web.servlet.")
+ || name.startsWith("org.springframework.context.annotation.")
+ || name.startsWith("org.springframework.context.event.")
+ || name.startsWith("org.springframework.context.expression.")
+ || name.startsWith("org.springframework.core.annotation.")
+ || name.startsWith("org.springframework.core.convert.")
+ || name.startsWith("org.springframework.core.env.")
+ || name.startsWith("org.springframework.core.io.")
+ || name.startsWith("org.springframework.core.type.")
+ || name.startsWith("org.springframework.expression.")
+ || name.startsWith("org.springframework.format.")
+ || name.startsWith("org.springframework.ui.")
+ || name.startsWith("org.springframework.validation.")
+ || name.startsWith("org.springframework.web.context.")
+ || name.startsWith("org.springframework.web.filter.")
+ || name.startsWith("org.springframework.web.method.")
+ || name.startsWith("org.springframework.web.multipart.")
+ || name.startsWith("org.springframework.web.util.")) {
+ return true;
+ }
+
+ // Dynamic proxy classes we should not touch
+ if (name.startsWith("org.springframework.core.$Proxy")) {
+ return true;
+ }
+
+ if (name.startsWith("org.springframework.cglib.")) {
+ // This class contains nested Callable instance that we'd happily not touch, but unfortunately
+ // our field injection code is not flexible enough to realize that, so instead
+ // we instrument this Callable to make tests happy.
+ if (name.startsWith("org.springframework.cglib.core.internal.LoadingCache$")) {
+ return false;
+ }
+
+ return true;
+ }
+
+ if (name.startsWith("org.springframework.http.")) {
+ // There are some Mono implementation that get instrumented
+ if (name.startsWith("org.springframework.http.server.reactive.")) {
+ return false;
+ }
+
+ return true;
+ }
+
+ return false;
+ }
+
+ @Override
+ public String toString() {
+ return "additionalLibraryIgnoresMatcher()";
+ }
+
+ @Override
+ public boolean equals(final Object other) {
+ if (!super.equals(other)) {
+ return false;
+ } else if (this == other) {
+ return true;
+ } else if (other == null) {
+ return false;
+ } else {
+ return getClass() == other.getClass();
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ return 17;
+ }
+}

RENAME
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/GlobalIgnoresMatcher.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/GlobalIgnoresMatcher.java
-package datadog.trace.agent.tooling.bytebuddy;
+package datadog.trace.agent.tooling.bytebuddy.matcher;
import java.util.regex.Pattern;
-import net.bytebuddy.build.HashCodeAndEqualsPlugin;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
-@HashCodeAndEqualsPlugin.Enhance
+/**
+ * Global ignores matcher used by the agent.
+ *
+ * <p>This matcher services two main purposes:
+ * <li>
+ *
+ * <ul>
+ * Ignore classes that are unsafe or pointless to transform. 'System' level classes like jvm
+ * classes or groovy classes, other tracers, debuggers, etc.
+ * </ul>
+ *
+ * <ul>
+ * Uses {@link AdditionalLibraryIgnoresMatcher} to also ignore additional classes to minimize
+ * number of classes we apply expensive matchers to.
+ * </ul>
+ */
public class GlobalIgnoresMatcher<T extends TypeDescription>
extends ElementMatcher.Junction.AbstractBase<T> {
@@ -16,6 +30,9 @@ public class GlobalIgnoresMatcher<T extends TypeDescription>
return new GlobalIgnoresMatcher<>();
}
+ private final ElementMatcher<T> additionalLibraryIgnoreMatcher =
+ AdditionalLibraryIgnoresMatcher.additionalLibraryIgnoresMatcher();
+
/**
* Be very careful about the types of matchers used in this section as they are called on every
* class load, so they must be fast. Generally speaking try to only use name matchers as they
@@ -40,43 +57,7 @@ public class GlobalIgnoresMatcher<T extends TypeDescription>
|| name.startsWith("com.appdynamics.")
|| name.startsWith("com.singularity.")
|| name.startsWith("com.jinspired.")
- || name.startsWith("org.jinspired.")
- || name.startsWith("org.springframework.cglib.")
- || name.startsWith("org.springframework.aop.")
- || name.startsWith("org.springframework.beans.factory.annotation.")
- || name.startsWith("org.springframework.beans.factory.config.")
- || name.startsWith("org.springframework.beans.factory.parsing.")
- || name.startsWith("org.springframework.beans.factory.xml.")
- || name.startsWith("org.springframework.beans.propertyeditors.")
- || name.startsWith("org.springframework.boot.autoconfigure.cache.")
- || name.startsWith("org.springframework.boot.autoconfigure.condition.")
- || name.startsWith("org.springframework.boot.autoconfigure.http.")
- || name.startsWith("org.springframework.boot.autoconfigure.jackson.")
- || name.startsWith("org.springframework.boot.autoconfigure.web.")
- || name.startsWith("org.springframework.boot.context.")
- || name.startsWith("org.springframework.boot.convert.")
- || name.startsWith("org.springframework.boot.diagnostics.")
- || name.startsWith("org.springframework.boot.web.server.")
- || name.startsWith("org.springframework.boot.web.servlet.")
- || name.startsWith("org.springframework.context.annotation.")
- || name.startsWith("org.springframework.context.event.")
- || name.startsWith("org.springframework.context.expression.")
- || name.startsWith("org.springframework.core.annotation.")
- || name.startsWith("org.springframework.core.convert.")
- || name.startsWith("org.springframework.core.env.")
- || name.startsWith("org.springframework.core.io.")
- || name.startsWith("org.springframework.core.type.")
- || name.startsWith("org.springframework.expression.")
- || name.startsWith("org.springframework.format.")
- || name.startsWith("org.springframework.http.")
- || name.startsWith("org.springframework.ui.")
- || name.startsWith("org.springframework.validation.")
- || name.startsWith("org.springframework.web.context.")
- || name.startsWith("org.springframework.web.filter.")
- || name.startsWith("org.springframework.web.method.")
- || name.startsWith("org.springframework.web.multipart.")
- || name.startsWith("org.springframework.web.util.")) {
-
+ || name.startsWith("org.jinspired.")) {
return true;
}
@@ -157,6 +138,34 @@ public class GlobalIgnoresMatcher<T extends TypeDescription>
return true;
}
+ if (additionalLibraryIgnoreMatcher.matches(target)) {
+ return true;
+ }
+
+ return false;
+ }
+
+ @Override
+ public String toString() {
+ return "globalIgnoresMatcher(" + additionalLibraryIgnoreMatcher.toString() + ")";
+ }
+
+ @Override
+ public boolean equals(final Object other) {
+ if (this == other) {
+ return true;
+ } else if (other == null) {
return false;
+ } else if (getClass() != other.getClass()) {
+ return false;
+ } else {
+ return additionalLibraryIgnoreMatcher.equals(
+ ((GlobalIgnoresMatcher) other).additionalLibraryIgnoreMatcher);
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ return 17 * 31 + additionalLibraryIgnoreMatcher.hashCode();
}
}

MODIFY
dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableTest.groovy
dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableTest.groovy
@@ -58,7 +58,7 @@ class HystrixObservableTest extends AgentTestRunner {
}
expect:
- TRANSFORMED_CLASSES.contains("HystrixObservableTest\$1")
+ TRANSFORMED_CLASSES_NAMES.contains("HystrixObservableTest\$1")
result == "Hello!"
assertTraces(1) {
@@ -168,7 +168,7 @@ class HystrixObservableTest extends AgentTestRunner {
}
expect:
- TRANSFORMED_CLASSES.contains("HystrixObservableTest\$2")
+ TRANSFORMED_CLASSES_NAMES.contains("HystrixObservableTest\$2")
result == "Fallback!"
assertTraces(1) {
@@ -286,7 +286,7 @@ class HystrixObservableTest extends AgentTestRunner {
}
then:
- TRANSFORMED_CLASSES.contains("HystrixObservableTest\$3")
+ TRANSFORMED_CLASSES_NAMES.contains("HystrixObservableTest\$3")
def err = thrown HystrixRuntimeException
err.cause instanceof IllegalArgumentException

MODIFY
dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixTest.groovy
dd-java-agent/instrumentation/hystrix-1.4/src/test/groovy/HystrixTest.groovy
@@ -37,8 +37,8 @@ class HystrixTest extends AgentTestRunner {
operation(command)
}
expect:
- TRANSFORMED_CLASSES.contains("com.netflix.hystrix.strategy.concurrency.HystrixContextScheduler\$ThreadPoolWorker")
- TRANSFORMED_CLASSES.contains("HystrixTest\$1")
+ TRANSFORMED_CLASSES_NAMES.contains("com.netflix.hystrix.strategy.concurrency.HystrixContextScheduler\$ThreadPoolWorker")
+ TRANSFORMED_CLASSES_NAMES.contains("HystrixTest\$1")
result == "Hello!"
assertTraces(1) {
@@ -115,8 +115,8 @@ class HystrixTest extends AgentTestRunner {
operation(command)
}
expect:
- TRANSFORMED_CLASSES.contains("com.netflix.hystrix.strategy.concurrency.HystrixContextScheduler\$ThreadPoolWorker")
- TRANSFORMED_CLASSES.contains("HystrixTest\$2")
+ TRANSFORMED_CLASSES_NAMES.contains("com.netflix.hystrix.strategy.concurrency.HystrixContextScheduler\$ThreadPoolWorker")
+ TRANSFORMED_CLASSES_NAMES.contains("HystrixTest\$2")
result == "Fallback!"
assertTraces(1) {

MODIFY
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.java
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.java
@@ -10,6 +10,7 @@ import datadog.trace.agent.test.asserts.ListWriterAssert;
import datadog.trace.agent.test.utils.GlobalTracerUtils;
import datadog.trace.agent.tooling.AgentInstaller;
import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.agent.tooling.bytebuddy.matcher.AdditionalLibraryIgnoresMatcher;
import datadog.trace.api.GlobalTracer;
import datadog.trace.common.writer.ListWriter;
import datadog.trace.common.writer.Writer;
@@ -34,6 +35,7 @@ import net.bytebuddy.agent.ByteBuddyAgent;
import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.dynamic.DynamicType;
+import net.bytebuddy.matcher.ElementMatcher;
import net.bytebuddy.utility.JavaModule;
import org.junit.AfterClass;
import org.junit.Before;
@@ -74,8 +76,13 @@ public abstract class AgentTestRunner extends DDSpecification {
// so we declare tracer as an object and cast when needed.
protected static final Object TEST_TRACER;
- protected static final Set<String> TRANSFORMED_CLASSES = Sets.newConcurrentHashSet();
- private static final AtomicInteger INSTRUMENTATION_ERROR_COUNT = new AtomicInteger();
+ private static final ElementMatcher.Junction<TypeDescription> GLOBAL_LIBRARIES_IGNORES_MATCHER =
+ AdditionalLibraryIgnoresMatcher.additionalLibraryIgnoresMatcher();
+
+ protected static final Set<String> TRANSFORMED_CLASSES_NAMES = Sets.newConcurrentHashSet();
+ protected static final Set<TypeDescription> TRANSFORMED_CLASSES_TYPES =
+ Sets.newConcurrentHashSet();
+ private static final AtomicInteger INSTRUMENTATION_ERROR_COUNT = new AtomicInteger(0);
private static final TestRunnerListener TEST_LISTENER = new TestRunnerListener();
private static final Instrumentation INSTRUMENTATION;
@@ -147,9 +154,7 @@ public abstract class AgentTestRunner extends DDSpecification {
.iterator()
.hasNext()
: "No instrumentation found";
- activeTransformer = AgentInstaller.installBytebuddyAgent(INSTRUMENTATION, TEST_LISTENER);
-
- INSTRUMENTATION_ERROR_COUNT.set(0);
+ activeTransformer = AgentInstaller.installBytebuddyAgent(INSTRUMENTATION, true, TEST_LISTENER);
}
/**
@@ -185,6 +190,11 @@ public abstract class AgentTestRunner extends DDSpecification {
// Cleanup before assertion.
assert INSTRUMENTATION_ERROR_COUNT.get() == 0
: INSTRUMENTATION_ERROR_COUNT.get() + " Instrumentation errors during test";
+
+ for (final TypeDescription type : TRANSFORMED_CLASSES_TYPES) {
+ assert !GLOBAL_LIBRARIES_IGNORES_MATCHER.matches(type)
+ : "Transformed class matches global libraries ignore matcher: " + type;
+ }
}
public static void assertTraces(
@@ -246,7 +256,8 @@ public abstract class AgentTestRunner extends DDSpecification {
final JavaModule module,
final boolean loaded,
final DynamicType dynamicType) {
- TRANSFORMED_CLASSES.add(typeDescription.getActualName());
+ TRANSFORMED_CLASSES_NAMES.add(typeDescription.getActualName());
+ TRANSFORMED_CLASSES_TYPES.add(typeDescription);
}
@Override

MODIFY
dd-java-agent/testing/src/test/groovy/AgentTestRunnerTest.groovy
dd-java-agent/testing/src/test/groovy/AgentTestRunnerTest.groovy
@@ -92,7 +92,7 @@ class AgentTestRunnerTest extends AgentTestRunner {
}
then:
- !TRANSFORMED_CLASSES.contains(subject.class.name)
+ !TRANSFORMED_CLASSES_NAMES.contains(subject.class.name)
assertTraces(1) {
trace(0, 1) {
span(0) {

Add test for ignore matchers for libraries