ClassLoaderHasNoResourceMatcher: put to cache outside critical section

This commit is contained in:
Lev Priima 2020-02-23 00:38:06 -08:00
parent 68e4bf8375
commit 885212ee36
4 changed files with 49 additions and 64 deletions

View File

@ -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<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;
}
v = hasNoResources(cl);
cache.put(cl, v);
return v;
}
}
}

View File

@ -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<ClassLoader> classLoaderMatcher() {
return not(classLoaderHasClasses("javax.ws.rs.container.AsyncResponse"));
return classLoaderHasNoResources("javax/ws/rs/container/AsyncResponse.class");
}
@Override

View File

@ -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<ClassLoader> classLoaderMatcher() {
return not(classLoaderHasClasses("redis.clients.jedis.commands.ProtocolCommand"));
return classLoaderHasNoResources("redis/clients/jedis/commands/ProtocolCommand.class");
}
@Override

View File

@ -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<ClassLoader> classLoaderMatcher() {
return not(classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener"));
return classLoaderHasNoResources(
"javax/servlet/AsyncEvent.class", "javax/servlet/AsyncListener.class");
}
@Override