From 88aa31b63edba7de4906dd87eaa0a4ba27d3e379 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 19 Feb 2020 14:53:18 -0800 Subject: [PATCH 1/2] Remove costly matchers from ignore list Removing these matchers caused the time spent matching ignores to go from greater than 1 second to less than 100 ms. --- .../datadog/trace/agent/tooling/AgentInstaller.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 0f466e1a49..9c2ad03e10 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 @@ -68,10 +68,11 @@ public class AgentInstaller { // https://github.com/raphw/byte-buddy/issues/558 // .with(AgentBuilder.LambdaInstrumentationStrategy.ENABLED) .ignore(any(), skipClassLoader()) - // Unlikely to ever need to instrument an annotation: - .or(ElementMatchers.isAnnotation()) - // Unlikely to ever need to instrument an enum: - .or(ElementMatchers.isEnum()) + /** + * 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. + */ .or( nameStartsWith("datadog.trace.") // FIXME: We should remove this once @@ -136,7 +137,6 @@ public class AgentInstaller { .or(nameContains(".asm.")) .or(nameContains("$__sisu")) .or(nameMatches("com\\.mchange\\.v2\\.c3p0\\..*Proxy")) - .or(isAnnotatedWith(named("javax.decorator.Decorator"))) .or(matchesConfiguredExcludes()); for (final AgentBuilder.Listener listener : listeners) { From 6e4b55304e34a32d357757fbd204b2b855b34b04 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 19 Feb 2020 16:21:41 -0800 Subject: [PATCH 2/2] Add special handling for ignoring `javax.decorator.Decorator` annotated classes. --- .../java/datadog/trace/agent/tooling/AgentInstaller.java | 2 -- .../main/java/datadog/trace/agent/tooling/Instrumenter.java | 6 ++++++ .../trace/agent/tooling/context/FieldBackedProvider.java | 6 +++++- .../cdi-1.2/src/test/groovy/CDIContainerTest.groovy | 1 - .../java/{datadog/trace/instrumentation => }/TestBean.java | 2 -- 5 files changed, 11 insertions(+), 6 deletions(-) rename dd-java-agent/instrumentation/cdi-1.2/src/test/java/{datadog/trace/instrumentation => }/TestBean.java (83%) 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 9c2ad03e10..eefa2a6b39 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 @@ -2,7 +2,6 @@ package datadog.trace.agent.tooling; import static datadog.trace.agent.tooling.ClassLoaderMatcher.skipClassLoader; import static net.bytebuddy.matcher.ElementMatchers.any; -import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; import static net.bytebuddy.matcher.ElementMatchers.nameContains; import static net.bytebuddy.matcher.ElementMatchers.nameMatches; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; @@ -24,7 +23,6 @@ import net.bytebuddy.agent.builder.ResettableClassFileTransformer; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.matcher.ElementMatcher; -import net.bytebuddy.matcher.ElementMatchers; import net.bytebuddy.utility.JavaModule; @Slf4j diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java index dca9fb6ccd..bb61b2cd83 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java @@ -2,6 +2,9 @@ package datadog.trace.agent.tooling; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.any; +import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; import datadog.trace.agent.tooling.context.FieldBackedProvider; import datadog.trace.agent.tooling.context.InstrumentationContextProvider; @@ -73,6 +76,9 @@ public interface Instrumenter { classLoaderMatcher(), "Instrumentation class loader matcher unexpected exception: " + getClass().getName())) + // Added here instead of AgentInstaller's ignores because it's relatively + // expensive. https://github.com/DataDog/dd-trace-java/pull/1045 + .and(not(isAnnotatedWith(named("javax.decorator.Decorator")))) .and(new MuzzleMatcher()) .and(new PostMatchHook()) .transform(DDTransformers.defaultTransformers()); diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/FieldBackedProvider.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/FieldBackedProvider.java index 3c77fb422e..e3dce626f2 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/FieldBackedProvider.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/FieldBackedProvider.java @@ -2,6 +2,7 @@ package datadog.trace.agent.tooling.context; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.BOOTSTRAP_CLASSLOADER; +import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -354,9 +355,12 @@ public class FieldBackedProvider implements InstrumentationContextProvider { builder = builder .type( - safeHasSuperType(named(entry.getKey())).and(not(isInterface())), + not(isInterface()).and(safeHasSuperType(named(entry.getKey()))), instrumenter.classLoaderMatcher()) .and(safeToInjectFieldsMatcher()) + // Added here instead of AgentInstaller's ignores because it's relatively + // expensive. https://github.com/DataDog/dd-trace-java/pull/1045 + .and(not(isAnnotatedWith(named("javax.decorator.Decorator")))) .transform(AgentBuilder.Transformer.NoOp.INSTANCE); /** diff --git a/dd-java-agent/instrumentation/cdi-1.2/src/test/groovy/CDIContainerTest.groovy b/dd-java-agent/instrumentation/cdi-1.2/src/test/groovy/CDIContainerTest.groovy index 6acb386213..f7878bd704 100644 --- a/dd-java-agent/instrumentation/cdi-1.2/src/test/groovy/CDIContainerTest.groovy +++ b/dd-java-agent/instrumentation/cdi-1.2/src/test/groovy/CDIContainerTest.groovy @@ -1,5 +1,4 @@ import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.instrumentation.TestBean import org.jboss.weld.environment.se.Weld import org.jboss.weld.environment.se.WeldContainer import org.jboss.weld.environment.se.threading.RunnableDecorator diff --git a/dd-java-agent/instrumentation/cdi-1.2/src/test/java/datadog/trace/instrumentation/TestBean.java b/dd-java-agent/instrumentation/cdi-1.2/src/test/java/TestBean.java similarity index 83% rename from dd-java-agent/instrumentation/cdi-1.2/src/test/java/datadog/trace/instrumentation/TestBean.java rename to dd-java-agent/instrumentation/cdi-1.2/src/test/java/TestBean.java index 3a7699aaa6..c68ad16a12 100644 --- a/dd-java-agent/instrumentation/cdi-1.2/src/test/java/datadog/trace/instrumentation/TestBean.java +++ b/dd-java-agent/instrumentation/cdi-1.2/src/test/java/TestBean.java @@ -1,5 +1,3 @@ -package datadog.trace.instrumentation; - public class TestBean { private String someField;