ClassLoaderHasNoResourceMatcher: put to cache outside critical section
MODIFY
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java
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<ClassLoader> classLoaderHasClasses(
- final String... names) {
- return new ClassLoaderHasClassMatcher(names);
+ public static ElementMatcher.Junction.AbstractBase<ClassLoader> classLoaderHasNoResources(
+ final String... resources) {
+ return new ClassLoaderHasNoResourceMatcher(resources);
}
private static final class SkipClassLoaderMatcher
extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
public static final SkipClassLoaderMatcher INSTANCE = new SkipClassLoaderMatcher();
/* Cache of classloader-instance -> (true|false). True = skip instrumentation. False = safe to instrument. */
- private static final LoadingCache<ClassLoader, Boolean> skipCache =
- CacheBuilder.newBuilder()
- .weakKeys()
- .build(
- new CacheLoader<ClassLoader, Boolean>() {
- @Override
- public Boolean load(ClassLoader loader) {
- return !delegatesToBootstrap(loader);
- }
- });
+ private static final Cache<ClassLoader, Boolean> 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<ClassLoader> {
+ private final Cache<ClassLoader, Boolean> cache =
+ CacheBuilder.newBuilder().weakKeys().concurrencyLevel(CACHE_CONCURRENCY).build();
- private final LoadingCache<ClassLoader, Boolean> cache =
- CacheBuilder.newBuilder()
- .weakKeys()
- .build(
- new CacheLoader<ClassLoader, Boolean>() {
- @Override
- public Boolean load(ClassLoader cl) {
- for (final String name : names) {
- if (cl.getResource(Utils.getResourceName(name)) == null) {
- return false;
- }
+ private final String[] resources;
+
+ private ClassLoaderHasNoResourceMatcher(final String... resources) {
+ this.resources = resources;
}
+
+ private boolean hasNoResources(ClassLoader cl) {
+ for (final String resource : resources) {
+ if (cl.getResource(resource) == null) {
return true;
}
- });
-
- private final String[] names;
-
- private ClassLoaderHasClassMatcher(final String... names) {
- this.names = names;
+ }
+ return false;
}
@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);
+ public boolean matches(final ClassLoader cl) {
+ if (cl == null) {
+ return false;
}
+ Boolean v = cache.getIfPresent(cl);
+ if (v != null) {
+ return v;
}
- return false;
+ v = hasNoResources(cl);
+ cache.put(cl, v);
+ return v;
}
}
}

MODIFY
dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java
dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java
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<ClassLoader> classLoaderMatcher() {
- return not(classLoaderHasClasses("javax.ws.rs.container.AsyncResponse"));
+ return classLoaderHasNoResources("javax/ws/rs/container/AsyncResponse.class");
}
@Override

MODIFY
dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java
dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java
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<ClassLoader> classLoaderMatcher() {
- return not(classLoaderHasClasses("redis.clients.jedis.commands.ProtocolCommand"));
+ return classLoaderHasNoResources("redis/clients/jedis/commands/ProtocolCommand.class");
}
@Override

MODIFY
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java
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<ClassLoader> classLoaderMatcher() {
- return not(classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener"));
+ return classLoaderHasNoResources(
+ "javax/servlet/AsyncEvent.class", "javax/servlet/AsyncListener.class");
}
@Override

ClassLoaderHasNoResourceMatcher: put to cache outside critical section