Use IgnoredTypesConfigurer to ignore classloaders (#3323)

benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java

@@ -6,7 +6,6 @@
 package io.opentelemetry.benchmark;
 
 import io.opentelemetry.instrumentation.api.config.Config;
-import io.opentelemetry.javaagent.tooling.AgentInstaller;
 import io.opentelemetry.javaagent.tooling.ignore.AdditionalLibraryIgnoredTypesConfigurer;
 import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesBuilderImpl;
 import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesMatcher;
@@ -38,9 +37,7 @@ public class IgnoredTypesMatcherBenchmark {
   static {
     IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl();
     new AdditionalLibraryIgnoredTypesConfigurer().configure(Config.get(), builder);
-    ignoredTypesMatcher =
+    ignoredTypesMatcher = new IgnoredTypesMatcher(builder.buildIgnoredTypesTrie());
-        new IgnoredTypesMatcher(
-            new AgentInstaller.NoopIgnoreMatcherProvider(), builder.buildIgnoredTypesTrie());
   }
 
   @Benchmark

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/IgnoreMatcherProvider.java

@@ -1,53 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.spi;
-
-import net.bytebuddy.description.type.TypeDescription;
-
-/**
- * {@link IgnoreMatcherProvider} can be used to ignore (or allow) types (e.g. classes or
- * classloaders) from being instrumented. OpenTelemetry agent by default ignores specific set of
- * classes (e.g. {@code org.gradle.*}) and classloaders. This is mainly done to improve startup
- * time, but also to explicitly disable instrumentation of a specific types (e.g. other agents). An
- * implementation of this class can be used to override this behaviour.
- *
- * <p>This is a service provider interface that requires implementations to be registered in {@code
- * META-INF/services} folder. Only a single implementation of this SPI can be provided.
- *
- * @deprecated Please use {@link io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer}
- *     instead.
- */
-@Deprecated
-public interface IgnoreMatcherProvider {
-
-  /**
-   * Whether to ignore (or allow) type. This method is called for every class, therefore the
-   * implementation has to be as efficient as possible.
-   *
-   * @param target a class.
-   * @return the result of the ignore evaluation.
-   */
-  Result type(TypeDescription target);
-
-  /**
-   * Whether to ignore (or allow) classloader. This method is called for every classloader,
-   * therefore the implementation has to be as efficient as possible.
-   *
-   * @param classLoader a classloader.
-   * @return the result of the ignore evaluation.
-   */
-  Result classloader(ClassLoader classLoader);
-
-  /** Result of the ignore evaluation. */
-  enum Result {
-    /** Default - delegate the evaluation to global ignore matchers from javaagent-tooling. */
-    DEFAULT,
-    /** Ignore instrumentation for a type. */
-    IGNORE,
-    /** Allow instrumentation for a type. */
-    ALLOW
-  }
-}

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java

@@ -18,18 +18,16 @@ import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.instrumentation.api.internal.BootstrapPackagePrefixesHolder;
 import io.opentelemetry.javaagent.spi.BootstrapPackagesProvider;
-import io.opentelemetry.javaagent.spi.IgnoreMatcherProvider;
 import io.opentelemetry.javaagent.tooling.config.ConfigInitializer;
 import io.opentelemetry.javaagent.tooling.context.FieldBackedProvider;
+import io.opentelemetry.javaagent.tooling.ignore.IgnoredClassLoadersMatcher;
 import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesBuilderImpl;
 import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesMatcher;
-import io.opentelemetry.javaagent.tooling.matcher.GlobalClassloaderIgnoresMatcher;
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.Proxy;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ForkJoinPool;
@@ -175,20 +173,14 @@ public class AgentInstaller {
   }
 
   private static AgentBuilder configureIgnoredTypes(Config config, AgentBuilder agentBuilder) {
-    IgnoreMatcherProvider ignoreMatcherProvider = loadIgnoreMatcherProvider();
+    IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl();
-    log.debug(
-        "Ignore matcher provider {} will be used", ignoreMatcherProvider.getClass().getName());
-
-    IgnoredTypesBuilderImpl ignoredTypesBuilder = new IgnoredTypesBuilderImpl();
     for (IgnoredTypesConfigurer configurer : loadOrdered(IgnoredTypesConfigurer.class)) {
-      configurer.configure(config, ignoredTypesBuilder);
+      configurer.configure(config, builder);
     }
 
     return agentBuilder
-        .ignore(any(), GlobalClassloaderIgnoresMatcher.skipClassLoader(ignoreMatcherProvider))
+        .ignore(any(), new IgnoredClassLoadersMatcher(builder.buildIgnoredClassLoadersTrie()))
-        .or(
+        .or(new IgnoredTypesMatcher(builder.buildIgnoredTypesTrie()));
-            new IgnoredTypesMatcher(
-                ignoreMatcherProvider, ignoredTypesBuilder.buildIgnoredTypesTrie()));
   }
 
   private static void runAfterAgentListeners(
@@ -224,17 +216,6 @@ public class AgentInstaller {
     }
   }
 
-  private static IgnoreMatcherProvider loadIgnoreMatcherProvider() {
-    Iterable<IgnoreMatcherProvider> ignoreMatcherProviders =
-        SafeServiceLoader.load(IgnoreMatcherProvider.class);
-
-    Iterator<IgnoreMatcherProvider> iterator = ignoreMatcherProviders.iterator();
-    if (iterator.hasNext()) {
-      return iterator.next();
-    }
-    return new NoopIgnoreMatcherProvider();
-  }
-
   private static void addByteBuddyRawSetting() {
     String savedPropertyValue = System.getProperty(TypeDefinition.RAW_TYPES_PROPERTY);
     try {
@@ -500,19 +481,5 @@ public class AgentInstaller {
         "{} loaded on {}", AgentInstaller.class.getName(), AgentInstaller.class.getClassLoader());
   }
 
-  /** This class will be removed together with {@link IgnoreMatcherProvider}. */
-  @Deprecated
-  public static final class NoopIgnoreMatcherProvider implements IgnoreMatcherProvider {
-    @Override
-    public Result classloader(ClassLoader classLoader) {
-      return Result.DEFAULT;
-    }
-
-    @Override
-    public Result type(TypeDescription target) {
-      return Result.DEFAULT;
-    }
-  }
-
   private AgentInstaller() {}
 }

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/GlobalIgnoredTypesConfigurer.java

@@ -7,14 +7,22 @@ package io.opentelemetry.javaagent.tooling.ignore;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.instrumentation.api.config.Config;
+import io.opentelemetry.javaagent.bootstrap.AgentClassLoader;
 import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesBuilder;
 import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer;
+import io.opentelemetry.javaagent.tooling.ExporterClassLoader;
+import io.opentelemetry.javaagent.tooling.ExtensionClassLoader;
 
 @AutoService(IgnoredTypesConfigurer.class)
 public class GlobalIgnoredTypesConfigurer implements IgnoredTypesConfigurer {
 
   @Override
   public void configure(Config config, IgnoredTypesBuilder builder) {
+    configureIgnoredTypes(builder);
+    configureIgnoredClassLoaders(builder);
+  }
+
+  private static void configureIgnoredTypes(IgnoredTypesBuilder builder) {
     builder
         .ignoreClass("org.gradle.")
         .ignoreClass("net.bytebuddy.")
@@ -94,4 +102,25 @@ public class GlobalIgnoredTypesConfigurer implements IgnoredTypesConfigurer {
         // proxy, and as there is no reason why it should be instrumented anyway, exclude it.
         .ignoreClass("$HttpServletRequest_");
   }
+
+  private static void configureIgnoredClassLoaders(IgnoredTypesBuilder builder) {
+    builder
+        .ignoreClassLoader("org.codehaus.groovy.runtime.callsite.CallSiteClassLoader")
+        .ignoreClassLoader("sun.reflect.DelegatingClassLoader")
+        .ignoreClassLoader("jdk.internal.reflect.DelegatingClassLoader")
+        .ignoreClassLoader("clojure.lang.DynamicClassLoader")
+        .ignoreClassLoader("org.apache.cxf.common.util.ASMHelper$TypeHelperClassLoader")
+        .ignoreClassLoader("sun.misc.Launcher$ExtClassLoader")
+        .ignoreClassLoader(AgentClassLoader.class.getName())
+        .ignoreClassLoader(ExporterClassLoader.class.getName())
+        .ignoreClassLoader(ExtensionClassLoader.class.getName());
+
+    builder
+        .ignoreClassLoader("datadog.")
+        .ignoreClassLoader("com.dynatrace.")
+        .ignoreClassLoader("com.appdynamics.")
+        .ignoreClassLoader("com.newrelic.agent.")
+        .ignoreClassLoader("com.newrelic.api.agent.")
+        .ignoreClassLoader("com.nr.agent.");
+  }
 }

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredClassLoadersMatcher.java

@@ -3,57 +3,45 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.tooling.matcher;
+package io.opentelemetry.javaagent.tooling.ignore;
 
 import io.opentelemetry.instrumentation.api.caching.Cache;
 import io.opentelemetry.javaagent.bootstrap.PatchLogger;
-import io.opentelemetry.javaagent.spi.IgnoreMatcherProvider;
+import io.opentelemetry.javaagent.tooling.ignore.trie.Trie;
 import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
 import net.bytebuddy.matcher.ElementMatcher;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class GlobalClassloaderIgnoresMatcher
+public class IgnoredClassLoadersMatcher extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
-    extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
+  private static final Logger log = LoggerFactory.getLogger(IgnoredClassLoadersMatcher.class);
-  private static final Logger log = LoggerFactory.getLogger(GlobalClassloaderIgnoresMatcher.class);
 
   /* Cache of classloader-instance -> (true|false). True = skip instrumentation. False = safe to instrument. */
-  private static final String AGENT_CLASSLOADER_NAME =
-      "io.opentelemetry.javaagent.bootstrap.AgentClassLoader";
-  private static final String EXPORTER_CLASSLOADER_NAME =
-      "io.opentelemetry.javaagent.tooling.ExporterClassLoader";
   private static final Cache<ClassLoader, Boolean> skipCache =
       Cache.newBuilder().setWeakKeys().build();
 
-  public static ElementMatcher.Junction.AbstractBase<ClassLoader> skipClassLoader(
+  private final Trie<IgnoreAllow> ignoredClassLoaders;
-      IgnoreMatcherProvider ignoreMatcherProvider) {
-    return new GlobalClassloaderIgnoresMatcher(ignoreMatcherProvider);
-  }
 
-  private final IgnoreMatcherProvider ignoreMatcherProviders;
+  public IgnoredClassLoadersMatcher(Trie<IgnoreAllow> ignoredClassLoaders) {
-
+    this.ignoredClassLoaders = ignoredClassLoaders;
-  private GlobalClassloaderIgnoresMatcher(IgnoreMatcherProvider ignoreMatcherProviders) {
-    this.ignoreMatcherProviders = ignoreMatcherProviders;
   }
 
   @Override
   public boolean matches(ClassLoader cl) {
-    IgnoreMatcherProvider.Result ignoreResult = ignoreMatcherProviders.classloader(cl);
-    switch (ignoreResult) {
-      case IGNORE:
-        return true;
-      case ALLOW:
-        return false;
-      case DEFAULT:
-    }
-
     if (cl == ClassLoadingStrategy.BOOTSTRAP_LOADER) {
       // Don't skip bootstrap loader
       return false;
     }
-    if (canSkipClassLoaderByName(cl)) {
+
+    String name = cl.getClass().getName();
+
+    IgnoreAllow ignored = ignoredClassLoaders.getOrNull(name);
+    if (ignored == IgnoreAllow.ALLOW) {
+      return false;
+    } else if (ignored == IgnoreAllow.IGNORE) {
       return true;
     }
+
     return skipCache.computeIfAbsent(
         cl,
         c -> {
@@ -74,34 +62,6 @@ public class GlobalClassloaderIgnoresMatcher
         });
   }
 
-  private static boolean canSkipClassLoaderByName(ClassLoader loader) {
-    String name = loader.getClass().getName();
-    // check by FQCN
-    switch (name) {
-      case "org.codehaus.groovy.runtime.callsite.CallSiteClassLoader":
-      case "sun.reflect.DelegatingClassLoader":
-      case "jdk.internal.reflect.DelegatingClassLoader":
-      case "clojure.lang.DynamicClassLoader":
-      case "org.apache.cxf.common.util.ASMHelper$TypeHelperClassLoader":
-      case "sun.misc.Launcher$ExtClassLoader":
-      case AGENT_CLASSLOADER_NAME:
-      case EXPORTER_CLASSLOADER_NAME:
-        return true;
-      default:
-        // noop
-    }
-    // check by package prefix
-    if (name.startsWith("datadog.")
-        || name.startsWith("com.dynatrace.")
-        || name.startsWith("com.appdynamics.")
-        || name.startsWith("com.newrelic.agent.")
-        || name.startsWith("com.newrelic.api.agent.")
-        || name.startsWith("com.nr.agent.")) {
-      return true;
-    }
-    return false;
-  }
-
   /**
    * TODO: this turns out to be useless with OSGi: {@code
    * org.eclipse.osgi.internal.loader.BundleLoader#isRequestFromVM} returns {@code true} when class

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java

@@ -9,30 +9,31 @@ import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesBuilder;
 import io.opentelemetry.javaagent.tooling.ignore.trie.Trie;
 
 public class IgnoredTypesBuilderImpl implements IgnoredTypesBuilder {
-  private final Trie.Builder<IgnoreAllow> ignoreMatcherTrie = Trie.newBuilder();
+  private final Trie.Builder<IgnoreAllow> ignoredTypesTrie = Trie.newBuilder();
+  private final Trie.Builder<IgnoreAllow> ignoredClassLoadersTrie = Trie.newBuilder();
 
   @Override
-  public IgnoredTypesBuilder ignoreClass(String className) {
+  public IgnoredTypesBuilder ignoreClass(String classNameOrPrefix) {
-    ignoreMatcherTrie.put(className, IgnoreAllow.IGNORE);
+    ignoredTypesTrie.put(classNameOrPrefix, IgnoreAllow.IGNORE);
     return this;
   }
 
   @Override
-  public IgnoredTypesBuilder allowClass(String className) {
+  public IgnoredTypesBuilder allowClass(String classNameOrPrefix) {
-    ignoreMatcherTrie.put(className, IgnoreAllow.ALLOW);
+    ignoredTypesTrie.put(classNameOrPrefix, IgnoreAllow.ALLOW);
     return this;
   }
 
   @Override
   public IgnoredTypesBuilder ignoreClassLoader(String classNameOrPrefix) {
-    // TODO: collect classloader classes into a separate trie
+    ignoredClassLoadersTrie.put(classNameOrPrefix, IgnoreAllow.IGNORE);
-    throw new UnsupportedOperationException("not implemented yet");
+    return this;
   }
 
   @Override
   public IgnoredTypesBuilder allowClassLoader(String classNameOrPrefix) {
-    // TODO: collect classloader classes into a separate trie
+    ignoredClassLoadersTrie.put(classNameOrPrefix, IgnoreAllow.ALLOW);
-    throw new UnsupportedOperationException("not implemented yet");
+    return this;
   }
 
   @Override
@@ -48,6 +49,10 @@ public class IgnoredTypesBuilderImpl implements IgnoredTypesBuilder {
   }
 
   public Trie<IgnoreAllow> buildIgnoredTypesTrie() {
-    return ignoreMatcherTrie.build();
+    return ignoredTypesTrie.build();
+  }
+
+  public Trie<IgnoreAllow> buildIgnoredClassLoadersTrie() {
+    return ignoredClassLoadersTrie.build();
   }
 }

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesMatcher.java

@@ -5,7 +5,6 @@
 
 package io.opentelemetry.javaagent.tooling.ignore;
 
-import io.opentelemetry.javaagent.spi.IgnoreMatcherProvider;
 import io.opentelemetry.javaagent.tooling.ignore.trie.Trie;
 import java.util.regex.Pattern;
 import net.bytebuddy.description.type.TypeDescription;
@@ -16,12 +15,9 @@ public class IgnoredTypesMatcher extends ElementMatcher.Junction.AbstractBase<Ty
   private static final Pattern COM_MCHANGE_PROXY =
       Pattern.compile("com\\.mchange\\.v2\\.c3p0\\..*Proxy");
 
-  private final IgnoreMatcherProvider ignoreMatcherProvider;
   private final Trie<IgnoreAllow> ignoredTypes;
 
-  public IgnoredTypesMatcher(
+  public IgnoredTypesMatcher(Trie<IgnoreAllow> ignoredTypes) {
-      IgnoreMatcherProvider ignoreMatcherProvider, Trie<IgnoreAllow> ignoredTypes) {
-    this.ignoreMatcherProvider = ignoreMatcherProvider;
     this.ignoredTypes = ignoredTypes;
   }
 
@@ -29,14 +25,6 @@ public class IgnoredTypesMatcher extends ElementMatcher.Junction.AbstractBase<Ty
   public boolean matches(TypeDescription target) {
     String name = target.getActualName();
 
-    // TODO: will be removed together with IgnoreMatcherProvider
-    IgnoreMatcherProvider.Result ignoreResult = ignoreMatcherProvider.type(target);
-    if (ignoreResult == IgnoreMatcherProvider.Result.ALLOW) {
-      return false;
-    } else if (ignoreResult == IgnoreMatcherProvider.Result.IGNORE) {
-      return true;
-    }
-
     IgnoreAllow ignored = ignoredTypes.getOrNull(name);
     if (ignored == IgnoreAllow.ALLOW) {
       return false;

javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/matcher/ClassLoaderMatcherTest.groovy

@@ -1,60 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.tooling.matcher
-
-import io.opentelemetry.javaagent.bootstrap.AgentClassLoader
-import io.opentelemetry.javaagent.spi.IgnoreMatcherProvider
-import io.opentelemetry.javaagent.tooling.ExporterClassLoader
-import spock.lang.Specification
-
-class ClassLoaderMatcherTest extends Specification {
-
-  private final IgnoreMatcherProvider matcherProvider = [classloader: { cl -> IgnoreMatcherProvider.Result.DEFAULT }] as IgnoreMatcherProvider
-
-  def "skips agent classloader"() {
-    setup:
-    URL url = AgentClassLoader.getProtectionDomain().getCodeSource().getLocation()
-    URLClassLoader agentLoader = new AgentClassLoader(new File(url.toURI()), "", null)
-    expect:
-    GlobalClassloaderIgnoresMatcher.skipClassLoader(matcherProvider).matches(agentLoader)
-  }
-
-  def "skips exporter classloader"() {
-    setup:
-    URL url = new URL("file://")
-    URLClassLoader exporterLoader = new ExporterClassLoader(url, null)
-    expect:
-    GlobalClassloaderIgnoresMatcher.skipClassLoader(matcherProvider).matches(exporterLoader)
-  }
-
-  def "does not skip empty classloader"() {
-    setup:
-    ClassLoader emptyLoader = new ClassLoader() {}
-    expect:
-    !GlobalClassloaderIgnoresMatcher.skipClassLoader(matcherProvider).matches(emptyLoader)
-  }
-
-  def "does not skip bootstrap classloader"() {
-    expect:
-    !GlobalClassloaderIgnoresMatcher.skipClassLoader(matcherProvider).matches(null)
-  }
-
-  def "skip bootstrap classloader"() {
-    IgnoreMatcherProvider skipBootstrapClMatcherProvider = [classloader: { cl -> cl == null ? IgnoreMatcherProvider.Result.IGNORE : IgnoreMatcherProvider.Result.DEFAULT }] as IgnoreMatcherProvider
-    expect:
-    GlobalClassloaderIgnoresMatcher.skipClassLoader(skipBootstrapClMatcherProvider).matches(null)
-  }
-
-  def "AgentClassLoader class name is hardcoded in ClassLoaderMatcher"() {
-    expect:
-    AgentClassLoader.name == "io.opentelemetry.javaagent.bootstrap.AgentClassLoader"
-  }
-
-  def "ExporterClassLoader class name is hardcoded in ClassLoaderMatcher"() {
-    expect:
-    ExporterClassLoader.name == "io.opentelemetry.javaagent.tooling.ExporterClassLoader"
-  }
-}