Merge tag 'v0.42.0'

agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/InternalJarURLHandler.java

@@ -1,7 +1,6 @@
 package io.opentelemetry.auto.bootstrap;
 
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -24,14 +23,16 @@ public class InternalJarURLHandler extends URLStreamHandler {
   private JarFile bootstrapJarFile;
 
   InternalJarURLHandler(final String internalJarFileName, final URL bootstrapJarLocation) {
+    final String filePrefix = internalJarFileName + "/";
+
     try {
       if (bootstrapJarLocation != null) {
-        bootstrapJarFile = new JarFile(new File(bootstrapJarLocation.toURI()));
+        bootstrapJarFile = new JarFile(new File(bootstrapJarLocation.toURI()), false);
         final Enumeration<JarEntry> entries = bootstrapJarFile.entries();
         while (entries.hasMoreElements()) {
           final JarEntry entry = entries.nextElement();
 
-          if (!entry.isDirectory() && entry.getName().startsWith(internalJarFileName + "/")) {
+          if (!entry.isDirectory() && entry.getName().startsWith(filePrefix)) {
             filenameToEntry.put(entry.getName().substring(internalJarFileName.length()), entry);
           }
         }
@@ -41,62 +42,33 @@ public class InternalJarURLHandler extends URLStreamHandler {
     }
 
     if (filenameToEntry.isEmpty()) {
-      log.warn("Internal jar entries found");
+      log.warn("No internal jar entries found");
     }
   }
 
   @Override
   protected URLConnection openConnection(final URL url) throws IOException {
-
-    final byte[] bytes;
-
     final String filename = url.getFile().replaceAll("\\.class$", ".classdata");
     if ("/".equals(filename)) {
       // "/" is used as the default url of the jar
       // This is called by the SecureClassLoader trying to obtain permissions
-      bytes = new byte[0];
+
+      // nullInputStream() is not available until Java 11
+      return new InternalJarURLConnection(url, new ByteArrayInputStream(new byte[0]));
     } else if (filenameToEntry.containsKey(filename)) {
       final JarEntry entry = filenameToEntry.get(filename);
-      bytes = getBytes(bootstrapJarFile.getInputStream(entry));
+      return new InternalJarURLConnection(url, bootstrapJarFile.getInputStream(entry));
     } else {
       throw new NoSuchFileException(url.getFile(), null, url.getFile() + " not in internal jar");
     }
-
-    return new InternalJarURLConnection(url, bytes);
-  }
-
-  /**
-   * Standard "copy InputStream to byte[]" implementation using a ByteArrayOutputStream
-   *
-   * <p>IOUtils.toByteArray() or Java 9's InputStream.readAllBytes() could be replacements if they
-   * were available
-   *
-   * <p>This can be optimized using the JarEntry's size(), but its not always available
-   *
-   * @param inputStream stream to read
-   * @return a byte[] from the inputstream
-   */
-  private static byte[] getBytes(final InputStream inputStream) throws IOException {
-    final byte[] buffer = new byte[4096];
-
-    int bytesRead = inputStream.read(buffer, 0, buffer.length);
-    try (final ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
-      while (bytesRead != -1) {
-        outputStream.write(buffer, 0, bytesRead);
-
-        bytesRead = inputStream.read(buffer, 0, buffer.length);
-      }
-
-      return outputStream.toByteArray();
-    }
   }
 
   private static class InternalJarURLConnection extends URLConnection {
-    private final byte[] bytes;
+    private final InputStream inputStream;
 
-    private InternalJarURLConnection(final URL url, final byte[] bytes) {
+    private InternalJarURLConnection(final URL url, final InputStream inputStream) {
       super(url);
-      this.bytes = bytes;
+      this.inputStream = inputStream;
     }
 
     @Override
@@ -106,7 +78,7 @@ public class InternalJarURLHandler extends URLStreamHandler {
 
     @Override
     public InputStream getInputStream() throws IOException {
-      return new ByteArrayInputStream(bytes);
+      return inputStream;
     }
 
     @Override

agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/WeakMap.java

@@ -18,7 +18,7 @@ public interface WeakMap<K, V> {
 
   void putIfAbsent(K key, V value);
 
-  V getOrCreate(K key, ValueSupplier<V> supplier);
+  V computeIfAbsent(K key, ValueSupplier<? super K, ? extends V> supplier);
 
   @Slf4j
   class Provider {
@@ -62,8 +62,8 @@ public interface WeakMap<K, V> {
    * Supplies the value to be stored and it is called only when a value does not exists yet in the
    * registry.
    */
-  interface ValueSupplier<V> {
+  interface ValueSupplier<K, V> {
-    V get();
+    V get(K key);
   }
 
   class MapAdapter<K, V> implements WeakMap<K, V> {
@@ -107,16 +107,22 @@ public interface WeakMap<K, V> {
     }
 
     @Override
-    public V getOrCreate(final K key, final ValueSupplier<V> supplier) {
+    public V computeIfAbsent(final K key, final ValueSupplier<? super K, ? extends V> supplier) {
-      if (!map.containsKey(key)) {
+      // We can't use computeIfAbsent since it was added in 1.8.
-        synchronized (this) {
+      if (map.containsKey(key)) {
-          if (!map.containsKey(key)) {
+        return map.get(key);
-            map.put(key, supplier.get());
-          }
-        }
       }
 
-      return map.get(key);
+      synchronized (this) {
+        if (map.containsKey(key)) {
+          return map.get(key);
+        } else {
+          final V value = supplier.get(key);
+
+          map.put(key, value);
+          return value;
+        }
+      }
     }
 
     @Override

agent-bootstrap/src/main/java/io/opentelemetry/auto/config/Config.java

@@ -52,6 +52,7 @@ public class Config {
   public static final String HTTP_CLIENT_TAG_QUERY_STRING = "http.client.tag.query-string";
   public static final String HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN = "trace.http.client.split-by-domain";
   public static final String DB_CLIENT_HOST_SPLIT_BY_INSTANCE = "trace.db.client.split-by-instance";
+  public static final String SCOPE_DEPTH_LIMIT = "trace.scope.depth.limit";
   public static final String RUNTIME_CONTEXT_FIELD_INJECTION =
       "trace.runtime.context.field.injection";
 
@@ -70,6 +71,7 @@ public class Config {
   private static final boolean DEFAULT_HTTP_CLIENT_TAG_QUERY_STRING = false;
   private static final boolean DEFAULT_HTTP_CLIENT_SPLIT_BY_DOMAIN = false;
   private static final boolean DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE = false;
+  private static final int DEFAULT_SCOPE_DEPTH_LIMIT = 100;
 
   public static final boolean DEFAULT_LOGS_INJECTION_ENABLED = false;
 
@@ -91,6 +93,7 @@ public class Config {
   @Getter private final boolean httpClientTagQueryString;
   @Getter private final boolean httpClientSplitByDomain;
   @Getter private final boolean dbClientSplitByInstance;
+  @Getter private final Integer scopeDepthLimit;
   @Getter private final boolean runtimeContextFieldInjection;
 
   @Getter private final boolean logsInjectionEnabled;
@@ -142,6 +145,9 @@ public class Config {
         getBooleanSettingFromEnvironment(
             DB_CLIENT_HOST_SPLIT_BY_INSTANCE, DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE);
 
+    scopeDepthLimit =
+        getIntegerSettingFromEnvironment(SCOPE_DEPTH_LIMIT, DEFAULT_SCOPE_DEPTH_LIMIT);
+
     runtimeContextFieldInjection =
         getBooleanSettingFromEnvironment(
             RUNTIME_CONTEXT_FIELD_INJECTION, DEFAULT_RUNTIME_CONTEXT_FIELD_INJECTION);
@@ -197,6 +203,9 @@ public class Config {
         getPropertyBooleanValue(
             properties, DB_CLIENT_HOST_SPLIT_BY_INSTANCE, parent.dbClientSplitByInstance);
 
+    scopeDepthLimit =
+        getPropertyIntegerValue(properties, SCOPE_DEPTH_LIMIT, parent.scopeDepthLimit);
+
     runtimeContextFieldInjection =
         getPropertyBooleanValue(
             properties, RUNTIME_CONTEXT_FIELD_INJECTION, parent.runtimeContextFieldInjection);

agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/WeakMapTest.groovy

@@ -10,7 +10,7 @@ class WeakMapTest extends Specification {
 
   def "getOrCreate a value"() {
     when:
-    def count = sut.getOrCreate('key', supplier)
+    def count = sut.computeIfAbsent('key', supplier)
 
     then:
     count == 1
@@ -19,8 +19,8 @@ class WeakMapTest extends Specification {
 
   def "getOrCreate a value multiple times same class loader same key"() {
     when:
-    def count1 = sut.getOrCreate('key', supplier)
+    def count1 = sut.computeIfAbsent('key', supplier)
-    def count2 = sut.getOrCreate('key', supplier)
+    def count2 = sut.computeIfAbsent('key', supplier)
 
     then:
     count1 == 1
@@ -30,8 +30,8 @@ class WeakMapTest extends Specification {
 
   def "getOrCreate a value multiple times same class loader different keys"() {
     when:
-    def count1 = sut.getOrCreate('key1', supplier)
+    def count1 = sut.computeIfAbsent('key1', supplier)
-    def count2 = sut.getOrCreate('key2', supplier)
+    def count2 = sut.computeIfAbsent('key2', supplier)
 
     then:
     count1 == 1
@@ -39,12 +39,12 @@ class WeakMapTest extends Specification {
     supplier.counter == 2
   }
 
-  class CounterSupplier implements WeakMap.ValueSupplier<Integer> {
+  class CounterSupplier implements WeakMap.ValueSupplier<String, Integer> {
 
     def counter = 0
 
     @Override
-    Integer get() {
+    Integer get(String ignored) {
       counter = counter + 1
       return counter
     }

agent-tooling/src/main/java/io/opentelemetry/auto/tooling/AgentCachingPoolStrategy.java

@@ -1,142 +1,245 @@
 package io.opentelemetry.auto.tooling;
 
-import static io.opentelemetry.auto.tooling.ClassLoaderMatcher.BOOTSTRAP_CLASSLOADER;
-import static io.opentelemetry.auto.tooling.ClassLoaderMatcher.skipClassLoader;
 import static net.bytebuddy.agent.builder.AgentBuilder.PoolStrategy;
 
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
-import io.opentelemetry.auto.bootstrap.WeakMap;
+import java.lang.ref.WeakReference;
-import java.util.concurrent.Callable;
+import lombok.extern.slf4j.Slf4j;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.ClassFileLocator;
 import net.bytebuddy.pool.TypePool;
 
 /**
- * Custom Pool strategy.
+ * NEW (Jan 2020) Custom Pool strategy.
  *
- * <p>Here we are using WeakMap.Provider as the backing ClassLoader -> CacheProvider lookup.
+ * <ul>
+ *   Uses a Guava Cache directly...
+ *   <li>better control over locking than WeakMap.Provider
+ *   <li>provides direct control over concurrency level
+ *   <li>initial and maximum capacity
+ * </ul>
  *
- * <p>We also use our bootstrap proxy when matching against the bootstrap loader.
+ * <ul>
+ *   There two core parts to the cache...
+ *   <li>a cache of ClassLoader to WeakReference&lt;ClassLoader&gt;
+ *   <li>a single cache of TypeResolutions for all ClassLoaders - keyed by a custom composite key of
+ *       ClassLoader & class name
+ * </ul>
  *
- * <p>The CacheProvider is a custom implementation that uses guava's cache to expire and limit size.
+ * <p>This design was chosen to create a single limited size cache that can be adjusted for the
+ * entire application -- without having to create a large number of WeakReference objects.
  *
- * <p>By evicting from the cache we are able to reduce the memory overhead of the agent for apps
+ * <p>Eviction is handled almost entirely through a size restriction; however, softValues are still
- * that have many classes.
+ * used as a further safeguard.
- *
- * <p>See eviction policy below.
  */
+@Slf4j
 public class AgentCachingPoolStrategy implements PoolStrategy {
-  private final WeakMap<ClassLoader, TypePool.CacheProvider> typePoolCache =
+  // Many things are package visible for testing purposes --
-      WeakMap.Provider.newWeakMap();
+  // others to avoid creation of synthetic accessors
-  private final Cleaner cleaner;
 
-  public AgentCachingPoolStrategy(final Cleaner cleaner) {
+  static final int CONCURRENCY_LEVEL = 8;
-    this.cleaner = cleaner;
+  static final int LOADER_CAPACITY = 64;
-  }
+  static final int TYPE_CAPACITY = 64;
+
+  static final int BOOTSTRAP_HASH = 0;
+
+  /**
+   * Cache of recent ClassLoader WeakReferences; used to...
+   *
+   * <ul>
+   *   <li>Reduced number of WeakReferences created
+   *   <li>Allow for quick fast path equivalence check of composite keys
+   * </ul>
+   */
+  final Cache<ClassLoader, WeakReference<ClassLoader>> loaderRefCache =
+      CacheBuilder.newBuilder()
+          .weakKeys()
+          .concurrencyLevel(CONCURRENCY_LEVEL)
+          .initialCapacity(LOADER_CAPACITY / 2)
+          .maximumSize(LOADER_CAPACITY)
+          .build();
+
+  /**
+   * Single shared Type.Resolution cache -- uses a composite key -- conceptually of loader & name
+   */
+  final Cache<TypeCacheKey, TypePool.Resolution> sharedResolutionCache =
+      CacheBuilder.newBuilder()
+          .softValues()
+          .concurrencyLevel(CONCURRENCY_LEVEL)
+          .initialCapacity(TYPE_CAPACITY)
+          .maximumSize(TYPE_CAPACITY)
+          .build();
+
+  /** Fast path for bootstrap */
+  final SharedResolutionCacheAdapter bootstrapCacheProvider =
+      new SharedResolutionCacheAdapter(BOOTSTRAP_HASH, null, sharedResolutionCache);
 
   @Override
-  public TypePool typePool(final ClassFileLocator classFileLocator, final ClassLoader classLoader) {
+  public final TypePool typePool(
-    final ClassLoader key =
+      final ClassFileLocator classFileLocator, final ClassLoader classLoader) {
-        BOOTSTRAP_CLASSLOADER == classLoader ? Utils.getBootstrapProxy() : classLoader;
+    if (classLoader == null) {
-    TypePool.CacheProvider cache = typePoolCache.get(key);
+      return createCachingTypePool(bootstrapCacheProvider, classFileLocator);
-    if (null == cache) {
-      synchronized (key) {
-        cache = typePoolCache.get(key);
-        if (null == cache) {
-          if (skipClassLoader().matches(classLoader)) {
-            // Don't bother creating a cache for a classloader that won't match.
-            // (avoiding a lot of DelegatingClassLoader instances)
-            // This is primarily an optimization.
-            cache = TypePool.CacheProvider.NoOp.INSTANCE;
-          } else {
-            cache = EvictingCacheProvider.withObjectType(cleaner, 1, TimeUnit.MINUTES);
-          }
-          typePoolCache.put(key, cache);
-        }
-      }
     }
-    return new TypePool.Default.WithLazyResolution(
+
-        cache, classFileLocator, TypePool.Default.ReaderMode.FAST);
+    WeakReference<ClassLoader> loaderRef = loaderRefCache.getIfPresent(classLoader);
+
+    if (loaderRef == null) {
+      loaderRef = new WeakReference<>(classLoader);
+      loaderRefCache.put(classLoader, loaderRef);
+    }
+
+    final int loaderHash = classLoader.hashCode();
+    return createCachingTypePool(loaderHash, loaderRef, classFileLocator);
   }
 
-  private static class EvictingCacheProvider implements TypePool.CacheProvider {
+  private TypePool.CacheProvider createCacheProvider(
+      final int loaderHash, final WeakReference<ClassLoader> loaderRef) {
+    return new SharedResolutionCacheAdapter(loaderHash, loaderRef, sharedResolutionCache);
+  }
 
-    /** A map containing all cached resolutions by their names. */
+  private TypePool createCachingTypePool(
-    private final Cache<String, TypePool.Resolution> cache;
+      final int loaderHash,
+      final WeakReference<ClassLoader> loaderRef,
+      final ClassFileLocator classFileLocator) {
+    return new TypePool.Default.WithLazyResolution(
+        createCacheProvider(loaderHash, loaderRef),
+        classFileLocator,
+        TypePool.Default.ReaderMode.FAST);
+  }
 
-    /** Creates a new simple cache. */
+  private TypePool createCachingTypePool(
-    private EvictingCacheProvider(
+      final TypePool.CacheProvider cacheProvider, final ClassFileLocator classFileLocator) {
-        final Cleaner cleaner, final long expireDuration, final TimeUnit unit) {
+    return new TypePool.Default.WithLazyResolution(
-      cache =
+        cacheProvider, classFileLocator, TypePool.Default.ReaderMode.FAST);
-          CacheBuilder.newBuilder()
+  }
-              .initialCapacity(100) // Per classloader, so we want a small default.
-              .maximumSize(5000)
-              .softValues()
-              .expireAfterAccess(expireDuration, unit)
-              .build();
 
-      /*
+  final long approximateSize() {
-       * The cache only does cleanup on occasional reads and writes.
+    return sharedResolutionCache.size();
-       * We want to ensure this happens more regularly, so we schedule a thread to do run cleanup manually.
+  }
-       */
-      cleaner.scheduleCleaning(cache, CacheCleaner.CLEANER, expireDuration, unit);
-    }
 
-    private static EvictingCacheProvider withObjectType(
+  /**
-        final Cleaner cleaner, final long expireDuration, final TimeUnit unit) {
+   * TypeCacheKey is key for the sharedResolutionCache. Conceptually, it is a mix of ClassLoader &
-      final EvictingCacheProvider cacheProvider =
+   * class name.
-          new EvictingCacheProvider(cleaner, expireDuration, unit);
+   *
-      cacheProvider.register(
+   * <p>For efficiency & GC purposes, it is actually composed of loaderHash &
-          Object.class.getName(), new TypePool.Resolution.Simple(TypeDescription.OBJECT));
+   * WeakReference&lt;ClassLoader&gt;
-      return cacheProvider;
+   *
+   * <p>The loaderHash exists to avoid calling get & strengthening the Reference.
+   */
+  static final class TypeCacheKey {
+    private final int loaderHash;
+    private final WeakReference<ClassLoader> loaderRef;
+    private final String className;
+
+    private final int hashCode;
+
+    TypeCacheKey(
+        final int loaderHash, final WeakReference<ClassLoader> loaderRef, final String className) {
+      this.loaderHash = loaderHash;
+      this.loaderRef = loaderRef;
+      this.className = className;
+
+      hashCode = (int) (31 * this.loaderHash) ^ className.hashCode();
     }
 
     @Override
-    public TypePool.Resolution find(final String name) {
+    public final int hashCode() {
-      return cache.getIfPresent(name);
+      return hashCode;
     }
 
     @Override
-    public TypePool.Resolution register(final String name, final TypePool.Resolution resolution) {
+    public boolean equals(final Object obj) {
-      try {
+      if (!(obj instanceof TypeCacheKey)) {
-        return cache.get(name, new ResolutionProvider(resolution));
+        return false;
-      } catch (final ExecutionException e) {
+      }
+
+      final TypeCacheKey that = (TypeCacheKey) obj;
+
+      if (loaderHash != that.loaderHash) {
+        return false;
+      }
+
+      // Fastpath loaderRef equivalence -- works because of WeakReference cache used
+      // Also covers the bootstrap null loaderRef case
+      if (loaderRef == that.loaderRef) {
+        // still need to check name
+        return className.equals(that.className);
+      } else if (className.equals(that.className)) {
+        // need to perform a deeper loader check -- requires calling Reference.get
+        // which can strengthen the Reference, so deliberately done last
+
+        // If either reference has gone null, they aren't considered equivalent
+        // Technically, this is a bit of violation of equals semantics, since
+        // two equivalent references can become not equivalent.
+
+        // In this case, it is fine because that means the ClassLoader is no
+        // longer live, so the entries will never match anyway and will fall
+        // out of the cache.
+        final ClassLoader thisLoader = loaderRef.get();
+        if (thisLoader == null) {
+          return false;
+        }
+
+        final ClassLoader thatLoader = that.loaderRef.get();
+        if (thatLoader == null) {
+          return false;
+        }
+
+        return (thisLoader == thatLoader);
+      } else {
+        return false;
+      }
+    }
+  }
+
+  static final class SharedResolutionCacheAdapter implements TypePool.CacheProvider {
+    private static final String OBJECT_NAME = "java.lang.Object";
+    private static final TypePool.Resolution OBJECT_RESOLUTION =
+        new TypePool.Resolution.Simple(TypeDescription.OBJECT);
+
+    private final int loaderHash;
+    private final WeakReference<ClassLoader> loaderRef;
+    private final Cache<TypeCacheKey, TypePool.Resolution> sharedResolutionCache;
+
+    SharedResolutionCacheAdapter(
+        final int loaderHash,
+        final WeakReference<ClassLoader> loaderRef,
+        final Cache<TypeCacheKey, TypePool.Resolution> sharedResolutionCache) {
+      this.loaderHash = loaderHash;
+      this.loaderRef = loaderRef;
+      this.sharedResolutionCache = sharedResolutionCache;
+    }
+
+    @Override
+    public TypePool.Resolution find(final String className) {
+      final TypePool.Resolution existingResolution =
+          sharedResolutionCache.getIfPresent(new TypeCacheKey(loaderHash, loaderRef, className));
+      if (existingResolution != null) {
+        return existingResolution;
+      }
+
+      if (OBJECT_NAME.equals(className)) {
+        return OBJECT_RESOLUTION;
+      }
+
+      return null;
+    }
+
+    @Override
+    public TypePool.Resolution register(
+        final String className, final TypePool.Resolution resolution) {
+      if (OBJECT_NAME.equals(className)) {
         return resolution;
       }
+
+      sharedResolutionCache.put(new TypeCacheKey(loaderHash, loaderRef, className), resolution);
+      return resolution;
     }
 
     @Override
     public void clear() {
-      cache.invalidateAll();
+      // Allowing the high-level eviction policy make the clearing decisions
-    }
-
-    public long size() {
-      return cache.size();
-    }
-
-    private static class CacheCleaner implements Cleaner.Adapter<Cache> {
-      private static final CacheCleaner CLEANER = new CacheCleaner();
-
-      @Override
-      public void clean(final Cache target) {
-        target.cleanUp();
-      }
-    }
-
-    private static class ResolutionProvider implements Callable<TypePool.Resolution> {
-      private final TypePool.Resolution value;
-
-      private ResolutionProvider(final TypePool.Resolution value) {
-        this.value = value;
-      }
-
-      @Override
-      public TypePool.Resolution call() {
-        return value;
-      }
     }
   }
 }

agent-tooling/src/main/java/io/opentelemetry/auto/tooling/AgentTooling.java

@@ -16,8 +16,7 @@ public class AgentTooling {
   }
 
   private static final AgentLocationStrategy LOCATION_STRATEGY = new AgentLocationStrategy();
-  private static final AgentCachingPoolStrategy POOL_STRATEGY =
+  private static final AgentCachingPoolStrategy POOL_STRATEGY = new AgentCachingPoolStrategy();
-      new AgentCachingPoolStrategy(CLEANER);
 
   public static void init() {
     // Only need to trigger static initializers for now.

agent-tooling/src/main/java/io/opentelemetry/auto/tooling/ClassLoaderMatcher.java

@@ -29,18 +29,9 @@ public class ClassLoaderMatcher {
     return new ClassLoaderHasClassMatcher(names);
   }
 
-  public static ElementMatcher.Junction.AbstractBase<ClassLoader> classLoaderHasClassWithField(
-      final String className, final String fieldName) {
-    return new ClassLoaderHasClassWithFieldMatcher(className, fieldName);
-  }
-
-  public static ElementMatcher.Junction.AbstractBase<ClassLoader> classLoaderHasClassWithMethod(
-      final String className, final String methodName, final String... methodArgs) {
-    return new ClassLoaderHasClassWithMethodMatcher(className, methodName, methodArgs);
-  }
-
   private static class SkipClassLoaderMatcher
-      extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
+      extends ElementMatcher.Junction.AbstractBase<ClassLoader>
+      implements WeakMap.ValueSupplier<ClassLoader, Boolean> {
     public static final SkipClassLoaderMatcher INSTANCE = new SkipClassLoaderMatcher();
     /* Cache of classloader-instance -> (true|false). True = skip instrumentation. False = safe to instrument. */
     private static final WeakMap<ClassLoader, Boolean> SKIP_CACHE = newWeakMap();
@@ -73,23 +64,18 @@ public class ClassLoaderMatcher {
     }
 
     private boolean shouldSkipInstance(final ClassLoader loader) {
-      Boolean cached = SKIP_CACHE.get(loader);
+      return SKIP_CACHE.computeIfAbsent(loader, this);
-      if (null != cached) {
+    }
-        return cached.booleanValue();
+
-      }
+    @Override
-      synchronized (this) {
+    public Boolean get(final ClassLoader loader) {
-        cached = SKIP_CACHE.get(loader);
+      final boolean skip = !delegatesToBootstrap(loader);
-        if (null != cached) {
+      if (skip) {
-          return cached.booleanValue();
+        log.debug(
-        }
+            "skipping classloader instance {} of type {}", loader, loader.getClass().getName());
-        final boolean skip = !delegatesToBootstrap(loader);
-        if (skip) {
-          log.debug(
-              "skipping classloader instance {} of type {}", loader, loader.getClass().getName());
-        }
-        SKIP_CACHE.put(loader, skip);
-        return skip;
       }
+
+      return skip;
     }
 
     /**
@@ -116,23 +102,9 @@ public class ClassLoaderMatcher {
     }
   }
 
-  private static class ClassLoaderNameMatcher
-      extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
-
-    private final String name;
-
-    private ClassLoaderNameMatcher(final String name) {
-      this.name = name;
-    }
-
-    @Override
-    public boolean matches(final ClassLoader target) {
-      return target != null && name.equals(target.getClass().getName());
-    }
-  }
-
   public static class ClassLoaderHasClassMatcher
-      extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
+      extends ElementMatcher.Junction.AbstractBase<ClassLoader>
+      implements WeakMap.ValueSupplier<ClassLoader, Boolean> {
 
     private final WeakMap<ClassLoader, Boolean> cache = newWeakMap();
 
@@ -145,123 +117,21 @@ public class ClassLoaderMatcher {
     @Override
     public boolean matches(final ClassLoader target) {
       if (target != null) {
-        Boolean result = cache.get(target);
+        return cache.computeIfAbsent(target, this);
-        if (result != null) {
-          return result;
-        }
-        synchronized (target) {
-          result = cache.get(target);
-          if (result != null) {
-            return result;
-          }
-          for (final String name : names) {
-            if (target.getResource(Utils.getResourceName(name)) == null) {
-              cache.put(target, false);
-              return false;
-            }
-          }
-          cache.put(target, true);
-          return true;
-        }
       }
+
       return false;
     }
-  }
-
-  public static class ClassLoaderHasClassWithFieldMatcher
-      extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
-
-    private final WeakMap<ClassLoader, Boolean> cache = newWeakMap();
-
-    private final String className;
-    private final String fieldName;
-
-    private ClassLoaderHasClassWithFieldMatcher(final String className, final String fieldName) {
-      this.className = className;
-      this.fieldName = fieldName;
-    }
 
     @Override
-    public boolean matches(final ClassLoader target) {
+    public Boolean get(final ClassLoader target) {
-      if (target != null) {
+      for (final String name : names) {
-        Boolean result = cache.get(target);
+        if (target.getResource(Utils.getResourceName(name)) == null) {
-        if (result != null) {
+          return false;
-          return result;
-        }
-        synchronized (target) {
-          result = cache.get(target);
-          if (result != null) {
-            return result;
-          }
-          try {
-            final Class<?> aClass = Class.forName(className, false, target);
-            aClass.getDeclaredField(fieldName);
-            cache.put(target, true);
-            return true;
-          } catch (final ClassNotFoundException e) {
-            cache.put(target, false);
-            return false;
-          } catch (final NoSuchFieldException e) {
-            cache.put(target, false);
-            return false;
-          }
         }
       }
-      return false;
-    }
-  }
 
-  public static class ClassLoaderHasClassWithMethodMatcher
+      return true;
-      extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
-
-    private final WeakMap<ClassLoader, Boolean> cache = newWeakMap();
-
-    private final String className;
-    private final String methodName;
-    private final String[] methodArgs;
-
-    private ClassLoaderHasClassWithMethodMatcher(
-        final String className, final String methodName, final String... methodArgs) {
-      this.className = className;
-      this.methodName = methodName;
-      this.methodArgs = methodArgs;
-    }
-
-    @Override
-    public boolean matches(final ClassLoader target) {
-      if (target != null) {
-        Boolean result = cache.get(target);
-        if (result != null) {
-          return result;
-        }
-        synchronized (target) {
-          result = cache.get(target);
-          if (result != null) {
-            return result;
-          }
-          try {
-            final Class<?> aClass = Class.forName(className, false, target);
-            final Class[] methodArgsClasses = new Class[methodArgs.length];
-            for (int i = 0; i < methodArgs.length; ++i) {
-              methodArgsClasses[i] = target.loadClass(methodArgs[i]);
-            }
-            if (aClass.isInterface()) {
-              aClass.getMethod(methodName, methodArgsClasses);
-            } else {
-              aClass.getDeclaredMethod(methodName, methodArgsClasses);
-            }
-            cache.put(target, true);
-            return true;
-          } catch (final ClassNotFoundException e) {
-            cache.put(target, false);
-            return false;
-          } catch (final NoSuchMethodException e) {
-            cache.put(target, false);
-            return false;
-          }
-        }
-      }
-      return false;
     }
   }
 }

agent-tooling/src/main/java/io/opentelemetry/auto/tooling/Cleaner.java

@@ -20,6 +20,7 @@ class Cleaner {
           final Thread thread = new Thread(r, "agent-cleaner");
           thread.setDaemon(true);
           thread.setPriority(Thread.MIN_PRIORITY);
+          thread.setContextClassLoader(null);
           return thread;
         }
       };

agent-tooling/src/main/java/io/opentelemetry/auto/tooling/WeakMapSuppliers.java

@@ -41,7 +41,7 @@ class WeakMapSuppliers {
     public <K, V> WeakMap<K, V> get() {
       final WeakConcurrentMap<K, V> map = new WeakConcurrentMap<>(false);
       cleaner.scheduleCleaning(map, MapCleaner.CLEANER, CLEAN_FREQUENCY_SECONDS, TimeUnit.SECONDS);
-      return new Adapter(map);
+      return new Adapter<>(map);
     }
 
     private static class MapCleaner implements Cleaner.Adapter<WeakConcurrentMap> {
@@ -86,16 +86,21 @@ class WeakMapSuppliers {
       }
 
       @Override
-      public V getOrCreate(final K key, final ValueSupplier<V> supplier) {
+      public V computeIfAbsent(final K key, final ValueSupplier<? super K, ? extends V> supplier) {
-        if (!map.containsKey(key)) {
+        if (map.containsKey(key)) {
-          synchronized (this) {
+          return map.get(key);
-            if (!map.containsKey(key)) {
-              map.put(key, supplier.get());
-            }
-          }
         }
 
-        return map.get(key);
+        synchronized (this) {
+          if (map.containsKey(key)) {
+            return map.get(key);
+          } else {
+            final V value = supplier.get(key);
+
+            map.put(key, value);
+            return value;
+          }
+        }
       }
     }
 
@@ -103,7 +108,7 @@ class WeakMapSuppliers {
 
       @Override
       public <K, V> WeakMap<K, V> get() {
-        return new Adapter(new WeakConcurrentMap.WithInlinedExpunction<K, V>());
+        return new Adapter<>(new WeakConcurrentMap.WithInlinedExpunction<K, V>());
       }
     }
   }

agent-tooling/src/main/java/io/opentelemetry/auto/tooling/muzzle/ReferenceMatcher.java

@@ -22,7 +22,8 @@ import net.bytebuddy.pool.TypePool;
 
 /** Matches a set of references against a classloader. */
 @Slf4j
-public class ReferenceMatcher {
+public class ReferenceMatcher
+    implements WeakMap.ValueSupplier<ClassLoader, List<Reference.Mismatch>> {
   private final WeakMap<ClassLoader, List<Reference.Mismatch>> mismatchCache = newWeakMap();
   private final Reference[] references;
   private final Set<String> helperClassNames;
@@ -45,7 +46,7 @@ public class ReferenceMatcher {
    * @return true if all references match the classpath of loader
    */
   public boolean matches(final ClassLoader loader) {
-    return getMismatchedReferenceSources(loader).size() == 0;
+    return getMismatchedReferenceSources(loader).isEmpty();
   }
 
   /**
@@ -56,23 +57,22 @@ public class ReferenceMatcher {
     if (loader == BOOTSTRAP_LOADER) {
       loader = Utils.getBootstrapProxy();
     }
-    List<Reference.Mismatch> mismatches = mismatchCache.get(loader);
+
-    if (null == mismatches) {
+    return mismatchCache.computeIfAbsent(loader, this);
-      synchronized (loader) {
+  }
-        mismatches = mismatchCache.get(loader);
+
-        if (null == mismatches) {
+  @Override
-          mismatches = new ArrayList<>(0);
+  public List<Mismatch> get(final ClassLoader loader) {
-          for (final Reference reference : references) {
+    final List<Mismatch> mismatches = new ArrayList<>(0);
-            // Don't reference-check helper classes.
+
-            // They will be injected by the instrumentation's HelperInjector.
+    for (final Reference reference : references) {
-            if (!helperClassNames.contains(reference.getClassName())) {
+      // Don't reference-check helper classes.
-              mismatches.addAll(checkMatch(reference, loader));
+      // They will be injected by the instrumentation's HelperInjector.
-            }
+      if (!helperClassNames.contains(reference.getClassName())) {
-          }
+        mismatches.addAll(checkMatch(reference, loader));
-          mismatchCache.put(loader, mismatches);
-        }
       }
     }
+
     return mismatches;
   }
 

agent-tooling/src/test/groovy/io/opentelemetry/auto/tooling/CacheProviderTest.groovy

@@ -0,0 +1,221 @@
+package io.opentelemetry.auto.tooling
+
+import io.opentelemetry.auto.util.test.AgentSpecification
+import net.bytebuddy.description.type.TypeDescription
+import net.bytebuddy.dynamic.ClassFileLocator
+import net.bytebuddy.pool.TypePool
+import spock.lang.Timeout
+
+import java.lang.ref.WeakReference
+
+@Timeout(5)
+class CacheProviderTest extends AgentSpecification {
+  def "key bootstrap equivalence"() {
+    // def loader = null
+    def loaderHash = AgentCachingPoolStrategy.BOOTSTRAP_HASH
+    def loaderRef = null
+
+    def key1 = new AgentCachingPoolStrategy.TypeCacheKey(loaderHash, loaderRef, "foo")
+    def key2 = new AgentCachingPoolStrategy.TypeCacheKey(loaderHash, loaderRef, "foo")
+
+    expect:
+    key1.hashCode() == key2.hashCode()
+    key1.equals(key2)
+  }
+
+  def "key same ref equivalence"() {
+    setup:
+    def loader = newClassLoader()
+    def loaderHash = loader.hashCode()
+    def loaderRef = new WeakReference<ClassLoader>(loader)
+
+    def key1 = new AgentCachingPoolStrategy.TypeCacheKey(loaderHash, loaderRef, "foo")
+    def key2 = new AgentCachingPoolStrategy.TypeCacheKey(loaderHash, loaderRef, "foo")
+
+    expect:
+    key1.hashCode() == key2.hashCode()
+    key1.equals(key2)
+  }
+
+  def "key different ref equivalence"() {
+    setup:
+    def loader = newClassLoader()
+    def loaderHash = loader.hashCode()
+    def loaderRef1 = new WeakReference<ClassLoader>(loader)
+    def loaderRef2 = new WeakReference<ClassLoader>(loader)
+
+    def key1 = new AgentCachingPoolStrategy.TypeCacheKey(loaderHash, loaderRef1, "foo")
+    def key2 = new AgentCachingPoolStrategy.TypeCacheKey(loaderHash, loaderRef2, "foo")
+
+    expect:
+    loaderRef1 != loaderRef2
+
+    key1.hashCode() == key2.hashCode()
+    key1.equals(key2)
+  }
+
+  def "key mismatch -- same loader - diff name"() {
+    setup:
+    def loader = newClassLoader()
+    def loaderHash = loader.hashCode()
+    def loaderRef = new WeakReference<ClassLoader>(loader)
+    def fooKey = new AgentCachingPoolStrategy.TypeCacheKey(loaderHash, loaderRef, "foo")
+    def barKey = new AgentCachingPoolStrategy.TypeCacheKey(loaderHash, loaderRef, "bar")
+
+    expect:
+    // not strictly guaranteed -- but important for performance
+    fooKey.hashCode() != barKey.hashCode()
+    !fooKey.equals(barKey)
+  }
+
+  def "key mismatch -- same name - diff loader"() {
+    setup:
+    def loader1 = newClassLoader()
+    def loader1Hash = loader1.hashCode()
+    def loaderRef1 = new WeakReference<ClassLoader>(loader1)
+
+    def loader2 = newClassLoader()
+    def loader2Hash = loader2.hashCode()
+    def loaderRef2 = new WeakReference<ClassLoader>(loader2)
+
+    def fooKey1 = new AgentCachingPoolStrategy.TypeCacheKey(loader1Hash, loaderRef1, "foo")
+    def fooKey2 = new AgentCachingPoolStrategy.TypeCacheKey(loader2Hash, loaderRef2, "foo")
+
+    expect:
+    // not strictly guaranteed -- but important for performance
+    fooKey1.hashCode() != fooKey2.hashCode()
+    !fooKey1.equals(fooKey2)
+  }
+
+  def "test basic caching"() {
+    setup:
+    def poolStrat = new AgentCachingPoolStrategy()
+
+    def loader = newClassLoader()
+    def loaderHash = loader.hashCode()
+    def loaderRef = new WeakReference<ClassLoader>(loader)
+
+    def cacheProvider = poolStrat.createCacheProvider(loaderHash, loaderRef)
+
+    when:
+    cacheProvider.register("foo", new TypePool.Resolution.Simple(TypeDescription.VOID))
+
+    then:
+    // not strictly guaranteed, but fine for this test
+    cacheProvider.find("foo") != null
+    poolStrat.approximateSize() == 1
+  }
+
+  def "test loader equivalence"() {
+    setup:
+    def poolStrat = new AgentCachingPoolStrategy()
+
+    def loader1 = newClassLoader()
+    def loaderHash1 = loader1.hashCode()
+    def loaderRef1A = new WeakReference<ClassLoader>(loader1)
+    def loaderRef1B = new WeakReference<ClassLoader>(loader1)
+
+    def cacheProvider1A = poolStrat.createCacheProvider(loaderHash1, loaderRef1A)
+    def cacheProvider1B = poolStrat.createCacheProvider(loaderHash1, loaderRef1B)
+
+    when:
+    cacheProvider1A.register("foo", newVoid())
+
+    then:
+    // not strictly guaranteed, but fine for this test
+    cacheProvider1A.find("foo") != null
+    cacheProvider1B.find("foo") != null
+
+    cacheProvider1A.find("foo").is(cacheProvider1B.find("foo"))
+    poolStrat.approximateSize() == 1
+  }
+
+  def "test loader separation"() {
+    setup:
+    def poolStrat = new AgentCachingPoolStrategy()
+
+    def loader1 = newClassLoader()
+    def loaderHash1 = loader1.hashCode()
+    def loaderRef1 = new WeakReference<ClassLoader>(loader1)
+
+    def loader2 = newClassLoader()
+    def loaderHash2 = loader2.hashCode()
+    def loaderRef2 = new WeakReference<ClassLoader>(loader2)
+
+    def cacheProvider1 = poolStrat.createCacheProvider(loaderHash1, loaderRef1)
+    def cacheProvider2 = poolStrat.createCacheProvider(loaderHash2, loaderRef2)
+
+    when:
+    cacheProvider1.register("foo", newVoid())
+    cacheProvider2.register("foo", newVoid())
+
+    then:
+    // not strictly guaranteed, but fine for this test
+    cacheProvider1.find("foo") != null
+    cacheProvider2.find("foo") != null
+
+    !cacheProvider1.find("foo").is(cacheProvider2.find("foo"))
+    poolStrat.approximateSize() == 2
+  }
+
+  def "test capacity"() {
+    setup:
+    def poolStrat = new AgentCachingPoolStrategy()
+    def capacity = AgentCachingPoolStrategy.TYPE_CAPACITY
+
+    def loader1 = newClassLoader()
+    def loaderHash1 = loader1.hashCode()
+    def loaderRef1 = new WeakReference<ClassLoader>(loader1)
+
+    def loader2 = newClassLoader()
+    def loaderHash2 = loader2.hashCode()
+    def loaderRef2 = new WeakReference<ClassLoader>(loader2)
+
+    def cacheProvider1 = poolStrat.createCacheProvider(loaderHash1, loaderRef1)
+    def cacheProvider2 = poolStrat.createCacheProvider(loaderHash2, loaderRef2)
+
+    def id = 0
+
+    when:
+    (capacity / 2).times {
+      id += 1
+      cacheProvider1.register("foo${id}", newVoid())
+      cacheProvider2.register("foo${id}", newVoid())
+    }
+
+    then:
+    // cache will start to proactively free slots & size calc is approximate
+    poolStrat.approximateSize() > 0.8 * capacity
+
+    when:
+    10.times {
+      id += 1
+      cacheProvider1.register("foo${id}", newVoid())
+      cacheProvider2.register("foo${id}", newVoid())
+    }
+
+    then:
+    // cache will start to proactively free slots & size calc is approximate
+    poolStrat.approximateSize() > 0.8 * capacity
+  }
+
+  static newVoid() {
+    return new TypePool.Resolution.Simple(TypeDescription.VOID)
+  }
+
+  static newClassLoader() {
+    return new URLClassLoader([] as URL[], (ClassLoader) null)
+  }
+
+  static newLocator() {
+    return new ClassFileLocator() {
+      @Override
+      ClassFileLocator.Resolution locate(String name) throws IOException {
+        return null
+      }
+
+      @Override
+      void close() throws IOException {}
+    }
+  }
+}

agent-tooling/src/test/groovy/io/opentelemetry/auto/tooling/EvictingCacheProviderTest.groovy

@@ -1,102 +0,0 @@
-package io.opentelemetry.auto.tooling
-
-import io.opentelemetry.auto.util.gc.GCUtils
-import io.opentelemetry.auto.util.test.AgentSpecification
-import net.bytebuddy.description.type.TypeDescription
-import net.bytebuddy.pool.TypePool
-import spock.lang.Timeout
-
-import java.lang.ref.WeakReference
-import java.util.concurrent.TimeUnit
-import java.util.concurrent.atomic.AtomicReference
-
-import static io.opentelemetry.auto.tooling.AgentTooling.CLEANER
-
-@Timeout(5)
-class EvictingCacheProviderTest extends AgentSpecification {
-
-  def "test provider"() {
-    setup:
-    def provider = new AgentCachingPoolStrategy.EvictingCacheProvider(CLEANER, 2, TimeUnit.MINUTES)
-
-    expect:
-    provider.size() == 0
-    provider.find(className) == null
-
-    when:
-    provider.register(className, new TypePool.Resolution.Simple(TypeDescription.VOID))
-
-    then:
-    provider.size() == 1
-    provider.find(className) == new TypePool.Resolution.Simple(TypeDescription.VOID)
-
-    when:
-    provider.clear()
-
-    then:
-    provider.size() == 0
-    provider.find(className) == null
-
-    where:
-    className = "SomeClass"
-  }
-
-  def "test timeout eviction"() {
-    setup:
-    def provider = new AgentCachingPoolStrategy.EvictingCacheProvider(CLEANER, timeout, TimeUnit.MILLISECONDS)
-    def resolutionRef = new AtomicReference<TypePool.Resolution>(new TypePool.Resolution.Simple(TypeDescription.VOID))
-    def weakRef = new WeakReference(resolutionRef.get())
-
-    when:
-    def lastAccess = System.nanoTime()
-    provider.register(className, resolutionRef.get())
-
-    then:
-    // Ensure continued access prevents expiration.
-    for (int i = 0; i < timeout + 10; i++) {
-      assert TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - lastAccess) < timeout: "test took too long on " + i
-      assert provider.find(className) != null
-      assert provider.size() == 1
-      lastAccess = System.nanoTime()
-      Thread.sleep(1)
-    }
-
-    when:
-    Thread.sleep(timeout)
-
-    then:
-    provider.find(className) == null
-
-    when:
-    provider.register(className, resolutionRef.get())
-    resolutionRef.set(null)
-    GCUtils.awaitGC(weakRef)
-
-    then:
-    // Verify properly GC'd
-    provider.find(className) == null
-    weakRef.get() == null
-
-    where:
-    className = "SomeClass"
-    timeout = 500 // Takes about 50 ms locally, adding an order of magnitude for CI.
-  }
-
-  def "test size limit"() {
-    setup:
-    def provider = new AgentCachingPoolStrategy.EvictingCacheProvider(CLEANER, 2, TimeUnit.MINUTES)
-    def typeDef = new TypePool.Resolution.Simple(TypeDescription.VOID)
-    for (int i = 0; i < 10000; i++) {
-      provider.register("ClassName$i", typeDef)
-    }
-
-    expect:
-    provider.size() == 5000
-
-    when:
-    provider.clear()
-
-    then:
-    provider.size() == 0
-  }
-}

buildSrc/src/main/groovy/MuzzlePlugin.groovy

@@ -291,7 +291,10 @@ class MuzzlePlugin implements Plugin<Project> {
       doLast {
         final ClassLoader instrumentationCL = createInstrumentationClassloader(instrumentationProject, toolingProject)
         def ccl = Thread.currentThread().contextClassLoader
-        def bogusLoader = new SecureClassLoader()
+        def bogusLoader = new SecureClassLoader() {
+          @Override
+          String toString() { return "bogus" }
+        }
         Thread.currentThread().contextClassLoader = bogusLoader
         final ClassLoader userCL = createClassLoaderForTask(instrumentationProject, bootstrapProject, taskName)
         try {

gradle/dependencies.gradle

@@ -12,9 +12,9 @@ ext {
     groovy       : groovyVer,
     logback      : "1.2.3",
     lombok       : "1.18.10",
-    bytebuddy    : "1.10.4",
+    bytebuddy    : "1.10.6",
     scala        : "2.11.12",  // Last version to support Java 7 (2.12+ require Java 8+)
-    kotlin       : "1.3.50",
+    kotlin       : "1.3.61",
     coroutines   : "1.3.0"
   ]
 

gradle/java.gradle

@@ -332,8 +332,8 @@ for (def env : System.getenv().entrySet()) {
 }
 
 tasks.withType(Test).configureEach {
-  // All tests must complete within 2 minutes.
+  // All tests must complete within 3 minutes.
-  timeout = Duration.ofMinutes(2)
+  timeout = Duration.ofMinutes(3)
 
   // Disable all tests if skipTests property was specified
   onlyIf { !project.rootProject.hasProperty("skipTests") }

gradle/test-with-kotlin.gradle

@@ -2,6 +2,7 @@
 apply plugin: 'kotlin'
 
 compileTestGroovy {
-  classpath = classpath.plus(files(compileTestKotlin.destinationDir))
+  //Note: look like it should be `classpath += files(sourceSets.test.kotlin.classesDirectory)`
-  dependsOn compileTestKotlin
+  //instead, but kotlin plugin doesn't support it (yet?)
+  classpath += files(compileTestKotlin.destinationDir)
 }

gradle/test-with-scala.gradle

@@ -13,6 +13,5 @@ configurations {
 }
 
 compileTestGroovy {
-  classpath = classpath.plus(files(compileTestScala.destinationDir))
+  classpath += files(sourceSets.test.scala.classesDirectory)
-  dependsOn compileTestScala
 }

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-6.0.1-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-6.1-all.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

instrumentation/hibernate/src/main/java/io/opentelemetry/auto/instrumentation/hibernate/HibernateDecorator.java

@@ -45,6 +45,9 @@ public class HibernateDecorator extends OrmClientDecorator {
 
   @Override
   public String entityName(final Object entity) {
+    if (entity == null) {
+      return null;
+    }
     String name = null;
     final Set<String> annotations = new HashSet<>();
     for (final Annotation annotation : entity.getClass().getDeclaredAnnotations()) {

instrumentation/java-concurrent/java-concurrent.gradle

@@ -7,7 +7,6 @@
 
 apply from: "${rootDir}/gradle/java.gradle"
 apply from: "${rootDir}/gradle/test-with-scala.gradle"
-apply from: "${rootDir}/gradle/test-with-kotlin.gradle"
 
 apply plugin: 'org.unbroken-dome.test-sets'
 
@@ -16,8 +15,7 @@ testSets {
 }
 
 compileSlickTestGroovy {
-  classpath = classpath.plus(files(compileSlickTestScala.destinationDir))
+  classpath += files(sourceSets.slickTest.scala.classesDirectory)
-  dependsOn compileSlickTestScala
 }
 
 dependencies {

instrumentation/netty-4.0/src/main/java/io/opentelemetry/auto/instrumentation/netty40/AttributeKeys.java

@@ -13,13 +13,14 @@ public class AttributeKeys {
   private static final WeakMap<ClassLoader, Map<String, AttributeKey<?>>> map =
       WeakMap.Implementation.DEFAULT.get();
 
-  private static final WeakMap.ValueSupplier<Map<String, AttributeKey<?>>> mapSupplier =
+  private static final WeakMap.ValueSupplier<ClassLoader, Map<String, AttributeKey<?>>>
-      new WeakMap.ValueSupplier<Map<String, AttributeKey<?>>>() {
+      mapSupplier =
-        @Override
+          new WeakMap.ValueSupplier<ClassLoader, Map<String, AttributeKey<?>>>() {
-        public Map<String, AttributeKey<?>> get() {
+            @Override
-          return new ConcurrentHashMap<>();
+            public Map<String, AttributeKey<?>> get(final ClassLoader ignored) {
-        }
+              return new ConcurrentHashMap<>();
-      };
+            }
+          };
 
   public static final AttributeKey<Span> PARENT_CONNECT_SPAN_ATTRIBUTE_KEY =
       attributeKey("io.opentelemetry.auto.instrumentation.netty40.parent.connect.span");
@@ -42,7 +43,7 @@ public class AttributeKeys {
    */
   private static <T> AttributeKey<T> attributeKey(final String key) {
     final Map<String, AttributeKey<?>> classLoaderMap =
-        map.getOrCreate(AttributeKey.class.getClassLoader(), mapSupplier);
+        map.computeIfAbsent(AttributeKey.class.getClassLoader(), mapSupplier);
     if (classLoaderMap.containsKey(key)) {
       return (AttributeKey<T>) classLoaderMap.get(key);
     }

instrumentation/netty-4.1/src/main/java/io/opentelemetry/auto/instrumentation/netty41/AttributeKeys.java

@@ -13,13 +13,14 @@ public class AttributeKeys {
   private static final WeakMap<ClassLoader, Map<String, AttributeKey<?>>> map =
       WeakMap.Implementation.DEFAULT.get();
 
-  private static final WeakMap.ValueSupplier<Map<String, AttributeKey<?>>> mapSupplier =
+  private static final WeakMap.ValueSupplier<ClassLoader, Map<String, AttributeKey<?>>>
-      new WeakMap.ValueSupplier<Map<String, AttributeKey<?>>>() {
+      mapSupplier =
-        @Override
+          new WeakMap.ValueSupplier<ClassLoader, Map<String, AttributeKey<?>>>() {
-        public Map<String, AttributeKey<?>> get() {
+            @Override
-          return new ConcurrentHashMap<>();
+            public Map<String, AttributeKey<?>> get(final ClassLoader ignored) {
-        }
+              return new ConcurrentHashMap<>();
-      };
+            }
+          };
 
   public static final AttributeKey<Span> PARENT_CONNECT_SPAN_ATTRIBUTE_KEY =
       attributeKey("io.opentelemetry.auto.instrumentation.netty41.parent.connect.span");
@@ -47,7 +48,7 @@ public class AttributeKeys {
    */
   private static <T> AttributeKey<T> attributeKey(final String key) {
     final Map<String, AttributeKey<?>> classLoaderMap =
-        map.getOrCreate(AttributeKey.class.getClassLoader(), mapSupplier);
+        map.computeIfAbsent(AttributeKey.class.getClassLoader(), mapSupplier);
     if (classLoaderMap.containsKey(key)) {
       return (AttributeKey<T>) classLoaderMap.get(key);
     }

instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java

@@ -68,6 +68,7 @@ public final class RequestDispatcherInstrumentation extends Instrumenter.Default
     public static SpanScopePair start(
         @Advice.Origin("#m") final String method,
         @Advice.This final RequestDispatcher dispatcher,
+        @Advice.Local("_requestSpan") Object requestSpan,
         @Advice.Argument(0) final ServletRequest request) {
       if (!TRACER.getCurrentSpan().getContext().isValid()) {
         // Don't want to generate a new top-level span
@@ -84,8 +85,9 @@ public final class RequestDispatcherInstrumentation extends Instrumenter.Default
       // In case we lose context, inject trace into to the request.
       TRACER.getHttpTextFormat().inject(span.getContext(), request, SETTER);
 
-      // temporarily remove from request to avoid spring resource name bubbling up:
+      // temporarily replace from request to avoid spring resource name bubbling up:
-      request.removeAttribute(SPAN_ATTRIBUTE);
+      requestSpan = request.getAttribute(SPAN_ATTRIBUTE);
+      request.setAttribute(SPAN_ATTRIBUTE, span);
 
       return new SpanScopePair(span, TRACER.withSpan(span));
     }
@@ -93,16 +95,19 @@ public final class RequestDispatcherInstrumentation extends Instrumenter.Default
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stop(
         @Advice.Enter final SpanScopePair scope,
+        @Advice.Local("_requestSpan") final Object requestSpan,
         @Advice.Argument(0) final ServletRequest request,
         @Advice.Thrown final Throwable throwable) {
       if (scope == null) {
         return;
       }
 
-      // now add it back...
+      if (requestSpan != null) {
-      final Span span = scope.getSpan();
+        // now add it back...
-      request.setAttribute(SPAN_ATTRIBUTE, span);
+        request.setAttribute(SPAN_ATTRIBUTE, requestSpan);
+      }
 
+      final Span span = scope.getSpan();
       DECORATE.onError(span, throwable);
       DECORATE.beforeFinish(span);
 

instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy

@@ -1,17 +1,22 @@
 import io.opentelemetry.auto.instrumentation.api.MoreTags
 import io.opentelemetry.auto.instrumentation.api.Tags
 import io.opentelemetry.auto.test.AgentTestRunner
+import io.opentelemetry.trace.Span
 
 import javax.servlet.ServletException
 import javax.servlet.http.HttpServletRequest
 import javax.servlet.http.HttpServletResponse
 
+import static io.opentelemetry.auto.decorator.HttpServerDecorator.SPAN_ATTRIBUTE
 import static io.opentelemetry.auto.test.utils.TraceUtils.basicSpan
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace
 
 class RequestDispatcherTest extends AgentTestRunner {
 
-  def dispatcher = new RequestDispatcherUtils(Mock(HttpServletRequest), Mock(HttpServletResponse))
+  def request = Mock(HttpServletRequest)
+  def response = Mock(HttpServletResponse)
+  def mockSpan = Mock(Span)
+  def dispatcher = new RequestDispatcherUtils(request, response)
 
   def "test dispatch no-parent"() {
     when:
@@ -19,7 +24,17 @@ class RequestDispatcherTest extends AgentTestRunner {
     dispatcher.include("")
 
     then:
-    assertTraces(0) {}
+    assertTraces(2) {
+      trace(0, 1) {
+        basicSpan(it, 0, "forward-child")
+      }
+      trace(1, 1) {
+        basicSpan(it, 0, "include-child")
+      }
+    }
+
+    and:
+    0 * _
   }
 
   def "test dispatcher #method with parent"() {
@@ -30,7 +45,7 @@ class RequestDispatcherTest extends AgentTestRunner {
 
     then:
     assertTraces(1) {
-      trace(0, 2) {
+      trace(0, 3) {
         basicSpan(it, 0, "parent")
         span(1) {
           operationName "servlet.$operation"
@@ -40,9 +55,20 @@ class RequestDispatcherTest extends AgentTestRunner {
             "$Tags.COMPONENT" "java-web-servlet-dispatcher"
           }
         }
+        basicSpan(it, 2, "$operation-child", null, span(1))
       }
     }
 
+    then:
+    1 * request.setAttribute("traceparent", _)
+    then:
+    1 * request.getAttribute(SPAN_ATTRIBUTE) >> mockSpan
+    then:
+    1 * request.setAttribute(SPAN_ATTRIBUTE, { it.name == "servlet.$operation" })
+    then:
+    1 * request.setAttribute(SPAN_ATTRIBUTE, mockSpan)
+    0 * _
+
     where:
     operation | method
     "forward" | "forward"
@@ -56,7 +82,7 @@ class RequestDispatcherTest extends AgentTestRunner {
   def "test dispatcher #method exception"() {
     setup:
     def ex = new ServletException("some error")
-    def dispatcher = new RequestDispatcherUtils(Mock(HttpServletRequest), Mock(HttpServletResponse), ex)
+    def dispatcher = new RequestDispatcherUtils(request, response, ex)
 
     when:
     runUnderTrace("parent") {
@@ -68,7 +94,7 @@ class RequestDispatcherTest extends AgentTestRunner {
     th == ex
 
     assertTraces(1) {
-      trace(0, 2) {
+      trace(0, 3) {
         basicSpan(it, 0, "parent", null, null, ex)
         span(1) {
           operationName "servlet.$operation"
@@ -80,9 +106,20 @@ class RequestDispatcherTest extends AgentTestRunner {
             errorTags(ex.class, ex.message)
           }
         }
+        basicSpan(it, 2, "$operation-child", null, span(1))
       }
     }
 
+    then:
+    1 * request.setAttribute("traceparent", _)
+    then:
+    1 * request.getAttribute(SPAN_ATTRIBUTE) >> mockSpan
+    then:
+    1 * request.setAttribute(SPAN_ATTRIBUTE, { it.name == "servlet.$operation" })
+    then:
+    1 * request.setAttribute(SPAN_ATTRIBUTE, mockSpan)
+    0 * _
+
     where:
     operation | method
     "forward" | "forward"

instrumentation/servlet/src/test/groovy/RequestDispatcherUtils.java

@@ -1,9 +1,12 @@
+import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Enumeration;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.Servlet;
 import javax.servlet.ServletContext;
@@ -164,7 +167,15 @@ public class RequestDispatcherUtils {
   class TestDispatcher implements RequestDispatcher {
     @Override
     public void forward(final ServletRequest servletRequest, final ServletResponse servletResponse)
-        throws ServletException, IOException {
+        throws ServletException {
+      runUnderTrace(
+          "forward-child",
+          new Callable<Object>() {
+            @Override
+            public Object call() throws Exception {
+              return null;
+            }
+          });
       if (toThrow != null) {
         throw toThrow;
       }
@@ -172,7 +183,15 @@ public class RequestDispatcherUtils {
 
     @Override
     public void include(final ServletRequest servletRequest, final ServletResponse servletResponse)
-        throws ServletException, IOException {
+        throws ServletException {
+      runUnderTrace(
+          "include-child",
+          new Callable<Object>() {
+            @Override
+            public Object call() throws Exception {
+              return null;
+            }
+          });
       if (toThrow != null) {
         throw toThrow;
       }

instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/trace_annotation/TraceConfigInstrumentation.java

@@ -42,16 +42,25 @@ public class TraceConfigInstrumentation implements Instrumenter {
           + PACKAGE_CLASS_NAME_REGEX
           + "\\["
           + METHOD_LIST_REGEX
-          + "\\]\\s*;?\\s*";
+          + "\\]";
 
   private final Map<String, Set<String>> classMethodsToTrace;
 
+  private boolean validateConfigString(String configString) {
+    for (String segment : configString.split(";")) {
+      if (!segment.trim().matches(CONFIG_FORMAT)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   public TraceConfigInstrumentation() {
     final String configString = Config.get().getTraceMethods();
     if (configString == null || configString.trim().isEmpty()) {
       classMethodsToTrace = Collections.emptyMap();
 
-    } else if (!configString.matches(CONFIG_FORMAT)) {
+    } else if (!validateConfigString(configString)) {
       log.warn(
           "Invalid trace method config '{}'. Must match 'package.Class$Name[method1,method2];*'.",
           configString);

java-agent/src/test/groovy/io/opentelemetry/auto/AgentLoadedIntoBootstrapTest.groovy

@@ -3,7 +3,9 @@ package io.opentelemetry.auto
 import io.opentelemetry.auto.test.IntegrationTestUtils
 import jvmbootstraptest.AgentLoadedChecker
 import spock.lang.Specification
+import spock.lang.Timeout
 
+@Timeout(30)
 class AgentLoadedIntoBootstrapTest extends Specification {
 
   def "Agent loads in when separate jvm is launched"() {

java-agent/src/test/groovy/io/opentelemetry/auto/LogLevelTest.groovy

@@ -3,7 +3,9 @@ package io.opentelemetry.auto
 import io.opentelemetry.auto.test.IntegrationTestUtils
 import jvmbootstraptest.LogLevelChecker
 import spock.lang.Specification
+import spock.lang.Timeout
 
+@Timeout(30)
 class LogLevelTest extends Specification {
 
 

java-agent/src/test/groovy/io/opentelemetry/auto/integration/classloading/ClassLoadingTest.groovy

@@ -77,7 +77,7 @@ class ClassLoadingTest extends Specification {
     loader.count == countAfterFirstLoad
   }
 
-  def "make sure that ByteBuddy doesn't resue cached type descriptions between different classloaders"() {
+  def "make sure that ByteBuddy doesn't reuse cached type descriptions between different classloaders"() {
     setup:
     CountingClassLoader loader1 = new CountingClassLoader(classpath)
     CountingClassLoader loader2 = new CountingClassLoader(classpath)

trace-java.gradle

@@ -9,7 +9,7 @@ plugins {
   // Not applying google java format by default because it gets confused by stray java build
   // files in 'workspace' build directory in CI
   id 'com.github.sherter.google-java-format' version '0.8' apply false
-  id 'com.dorongold.task-tree' version '1.4'
+  id 'com.dorongold.task-tree' version '1.5'
 
   id "com.github.johnrengelman.shadow" version "5.2.0"
 }