diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java index 7137f3835d..d19aad4117 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java @@ -1,17 +1,17 @@ package datadog.trace.agent.tooling; +import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; import datadog.trace.bootstrap.PatchLogger; import io.opentracing.util.GlobalTracer; -import java.util.concurrent.ExecutionException; import lombok.extern.slf4j.Slf4j; import net.bytebuddy.matcher.ElementMatcher; @Slf4j -public class ClassLoaderMatcher { +public final class ClassLoaderMatcher { public static final ClassLoader BOOTSTRAP_CLASSLOADER = null; + public static final int CACHE_CONCURRENCY = + Math.max(8, Runtime.getRuntime().availableProcessors()); /** A private constructor that must not be invoked. */ private ClassLoaderMatcher() { @@ -22,37 +22,35 @@ public class ClassLoaderMatcher { return SkipClassLoaderMatcher.INSTANCE; } - public static ElementMatcher.Junction.AbstractBase classLoaderHasClasses( - final String... names) { - return new ClassLoaderHasClassMatcher(names); + public static ElementMatcher.Junction.AbstractBase classLoaderHasNoResources( + final String... resources) { + return new ClassLoaderHasNoResourceMatcher(resources); } private static final class SkipClassLoaderMatcher extends ElementMatcher.Junction.AbstractBase { public static final SkipClassLoaderMatcher INSTANCE = new SkipClassLoaderMatcher(); /* Cache of classloader-instance -> (true|false). True = skip instrumentation. False = safe to instrument. */ - private static final LoadingCache skipCache = - CacheBuilder.newBuilder() - .weakKeys() - .build( - new CacheLoader() { - @Override - public Boolean load(ClassLoader loader) { - return !delegatesToBootstrap(loader); - } - }); + private static final Cache skipCache = + CacheBuilder.newBuilder().weakKeys().concurrencyLevel(CACHE_CONCURRENCY).build(); private static final String DATADOG_CLASSLOADER_NAME = "datadog.trace.bootstrap.DatadogClassLoader"; private SkipClassLoaderMatcher() {} @Override - public boolean matches(final ClassLoader target) { - if (target == BOOTSTRAP_CLASSLOADER) { + public boolean matches(final ClassLoader cl) { + if (cl == BOOTSTRAP_CLASSLOADER) { // Don't skip bootstrap loader return false; } - return shouldSkipClass(target) || shouldSkipInstance(target); + Boolean v = skipCache.getIfPresent(cl); + if (v != null) { + return v; + } + v = shouldSkipClass(cl) || !delegatesToBootstrap(cl); + skipCache.put(cl, v); + return v; } private static boolean shouldSkipClass(final ClassLoader loader) { @@ -69,15 +67,6 @@ public class ClassLoaderMatcher { return false; } - private static boolean shouldSkipInstance(final ClassLoader loader) { - try { - return skipCache.get(loader); - } catch (ExecutionException e) { - log.warn("Exception while getting from cache", e); - } - return false; - } - /** * TODO: this turns out to be useless with OSGi: {@code * org.eclipse.osgi.internal.loader.BundleLoader#isRequestFromVM} returns {@code true} when @@ -107,41 +96,38 @@ public class ClassLoaderMatcher { } } - public static class ClassLoaderHasClassMatcher + private static class ClassLoaderHasNoResourceMatcher extends ElementMatcher.Junction.AbstractBase { + private final Cache cache = + CacheBuilder.newBuilder().weakKeys().concurrencyLevel(CACHE_CONCURRENCY).build(); - private final LoadingCache cache = - CacheBuilder.newBuilder() - .weakKeys() - .build( - new CacheLoader() { - @Override - public Boolean load(ClassLoader cl) { - for (final String name : names) { - if (cl.getResource(Utils.getResourceName(name)) == null) { - return false; - } - } - return true; - } - }); + private final String[] resources; - private final String[] names; - - private ClassLoaderHasClassMatcher(final String... names) { - this.names = names; + private ClassLoaderHasNoResourceMatcher(final String... resources) { + this.resources = resources; } - @Override - public boolean matches(final ClassLoader target) { - if (target != null) { - try { - return cache.get(target); - } catch (ExecutionException e) { - log.warn("Can't get from cache", e); + private boolean hasNoResources(ClassLoader cl) { + for (final String resource : resources) { + if (cl.getResource(resource) == null) { + return true; } } return false; } + + @Override + public boolean matches(final ClassLoader cl) { + if (cl == null) { + return false; + } + Boolean v = cache.getIfPresent(cl); + if (v != null) { + return v; + } + v = hasNoResources(cl); + cache.put(cl, v); + return v; + } } } diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java index 6250c78dbe..1ddc8b2f27 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.jaxrs1; -import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; +import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasNoResources; import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.hasSuperMethod; import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.safeHasSuperType; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; @@ -12,7 +12,6 @@ import static net.bytebuddy.matcher.ElementMatchers.declaresMethod; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; 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 datadog.trace.agent.tooling.Instrumenter; @@ -37,7 +36,7 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default // this is required to make sure instrumentation won't apply to jax-rs 2 @Override public ElementMatcher classLoaderMatcher() { - return not(classLoaderHasClasses("javax.ws.rs.container.AsyncResponse")); + return classLoaderHasNoResources("javax/ws/rs/container/AsyncResponse.class"); } @Override diff --git a/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java b/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java index 9c34b523b4..a521f0af22 100644 --- a/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java +++ b/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.jedis; -import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; +import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasNoResources; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.instrumentation.jedis.JedisClientDecorator.DECORATE; @@ -8,7 +8,6 @@ import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; @@ -34,7 +33,7 @@ public final class JedisInstrumentation extends Instrumenter.Default { @Override public ElementMatcher classLoaderMatcher() { - return not(classLoaderHasClasses("redis.clients.jedis.commands.ProtocolCommand")); + return classLoaderHasNoResources("redis/clients/jedis/commands/ProtocolCommand.class"); } @Override diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java index 4dabc22522..8a7e1a40dc 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.servlet2; -import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; +import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasNoResources; import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.safeHasSuperType; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -25,7 +25,8 @@ public final class Servlet2Instrumentation extends Instrumenter.Default { // this is required to make sure servlet 2 instrumentation won't apply to servlet 3 @Override public ElementMatcher classLoaderMatcher() { - return not(classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener")); + return classLoaderHasNoResources( + "javax/servlet/AsyncEvent.class", "javax/servlet/AsyncListener.class"); } @Override