Refactor/rename BootstrapPackagesProvider and PropertySource (#3435)

* Refactor/rename BootstrapPackagesConfigurer and PropertySource

* Update comment

instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentation.java

@@ -28,6 +28,7 @@ import java.util.List;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
+import org.slf4j.LoggerFactory;
 
 /*
  * Some class loaders do not delegate to their parent, so classes in those class loaders
@@ -90,6 +91,8 @@ public class ClassLoaderInstrumentation implements TypeInstrumentation {
         //noinspection unchecked
         return (List<String>) methodHandle.invokeExact();
       } catch (Throwable e) {
+        LoggerFactory.getLogger(Holder.class)
+            .warn("Unable to load bootstrap package prefixes from the bootstrap CL", e);
         return Constants.BOOTSTRAP_PACKAGE_PREFIXES;
       }
     }

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/bootstrap/BootstrapPackagesBuilder.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.extension.bootstrap;
+
+import java.util.Collection;
+
+/**
+ * This interface exposes a way to define which packages/classes are present in the bootstrap
+ * classloader.
+ *
+ * <p>This interface should not be implemented by the javaagent extension developer - the javaagent
+ * will provide the implementation.
+ */
+public interface BootstrapPackagesBuilder {
+
+  /**
+   * Mark {@code classNameOrPrefix} as one that belongs to the bootstrap classloader.
+   *
+   * @return {@code this}
+   */
+  BootstrapPackagesBuilder add(String classNameOrPrefix);
+
+  /**
+   * Mark all elements of {@code classNamesOrPrefixes} as ones that belongs to the bootstrap
+   * classloader.
+   *
+   * @return {@code this}
+   */
+  BootstrapPackagesBuilder addAll(Collection<String> classNamesOrPrefixes);
+}

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/bootstrap/BootstrapPackagesConfigurer.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.extension.bootstrap;
+
+import io.opentelemetry.instrumentation.api.config.Config;
+
+/**
+ * This SPI can be used to define which packages/classes belong to the bootstrap class loader: all
+ * packages configured here will always be loaded by the bootstrap class loader, even if class
+ * loader that initiated loading of the class does not normally delegate to bootstrap class loader.
+ *
+ * <p><b>IMPORTANT</b>: This SPI cannot add new packages to the bootstrap CL, it only defines those
+ * that are already there - the purpose is to make sure they're loaded by the correct class loader.
+ *
+ * <p>This is a service provider interface that requires implementations to be registered in a
+ * provider-configuration file stored in the {@code META-INF/services} resource directory.
+ */
+public interface BootstrapPackagesConfigurer {
+
+  /**
+   * Configure the passed {@code builder} and define which classes should always be loaded by the
+   * bootstrap class loader.
+   */
+  void configure(Config config, BootstrapPackagesBuilder builder);
+}

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/config/ConfigPropertySource.java

@@ -3,8 +3,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.spi.config;
+package io.opentelemetry.javaagent.extension.config;
 
+import io.opentelemetry.javaagent.extension.Ordered;
 import java.util.Map;
 
 /**
@@ -12,8 +13,11 @@ import java.util.Map;
  * by implementations of this interface will be used after the following methods fail to find a
  * non-empty property value: system properties, environment variables, properties configuration
  * file.
+ *
+ * <p>This is a service provider interface that requires implementations to be registered in a
+ * provider-configuration file stored in the {@code META-INF/services} resource directory.
  */
-public interface PropertySource {
+public interface ConfigPropertySource extends Ordered {
   /**
    * Returns all properties whose default values are overridden by this property source. Key of the
    * map is the propertyName (same as system property name, e.g. {@code otel.traces.exporter}),

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

@@ -1,24 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.spi;
-
-import java.util.List;
-
-/**
- * A service provider to allow adding classes from specified package prefixes to the bootstrap
- * classloader. The classes in the bootstrap classloader are available to all instrumentations. This
- * is useful if large number of custom instrumentations are using functionality from common
- * packages.
- */
-public interface BootstrapPackagesProvider {
-
-  /**
-   * Classes from returned package prefixes will be available in the bootstrap classloader.
-   *
-   * @return package prefixes.
-   */
-  List<String> getPackagePrefixes();
-}

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

@@ -6,6 +6,7 @@
 package io.opentelemetry.javaagent.tooling;
 
 import static io.opentelemetry.javaagent.bootstrap.AgentInitializer.isJavaBefore9;
+import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.load;
 import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.loadOrdered;
 import static io.opentelemetry.javaagent.tooling.Utils.getResourceName;
 import static net.bytebuddy.matcher.ElementMatchers.any;
@@ -14,12 +15,13 @@ import io.opentelemetry.instrumentation.api.config.Config;
 import io.opentelemetry.javaagent.bootstrap.AgentClassLoader;
 import io.opentelemetry.javaagent.extension.AgentExtension;
 import io.opentelemetry.javaagent.extension.AgentListener;
+import io.opentelemetry.javaagent.extension.bootstrap.BootstrapPackagesConfigurer;
 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.instrumentation.api.internal.InstrumentedTaskClasses;
-import io.opentelemetry.javaagent.spi.BootstrapPackagesProvider;
 import io.opentelemetry.javaagent.tooling.asyncannotationsupport.WeakRefAsyncOperationEndStrategies;
+import io.opentelemetry.javaagent.tooling.bootstrap.BootstrapPackagesBuilderImpl;
 import io.opentelemetry.javaagent.tooling.config.ConfigInitializer;
 import io.opentelemetry.javaagent.tooling.context.FieldBackedProvider;
 import io.opentelemetry.javaagent.tooling.ignore.IgnoredClassLoadersMatcher;
@@ -70,9 +72,9 @@ public class AgentInstaller {
     log = LoggerFactory.getLogger(AgentInstaller.class);
 
     addByteBuddyRawSetting();
-    BootstrapPackagePrefixesHolder.setBoostrapPackagePrefixes(loadBootstrapPackagePrefixes());
     // this needs to be done as early as possible - before the first Config.get() call
     ConfigInitializer.initialize();
+
     // ensure java.lang.reflect.Proxy is loaded, as transformation code uses it internally
     // loading java.lang.reflect.Proxy after the bytebuddy transformer is set up causes
     // the internal-proxy instrumentation module to transform it, and then the bytebuddy
@@ -115,6 +117,9 @@ public class AgentInstaller {
     WeakRefAsyncOperationEndStrategies.initialize();
 
     Config config = Config.get();
+
+    setBootstrapPackages(config);
+
     runBeforeAgentListeners(agentListeners, config);
 
     instrumentation = inst;
@@ -169,6 +174,14 @@ public class AgentInstaller {
     return resettableClassFileTransformer;
   }
 
+  private static void setBootstrapPackages(Config config) {
+    BootstrapPackagesBuilderImpl builder = new BootstrapPackagesBuilderImpl();
+    for (BootstrapPackagesConfigurer configurer : load(BootstrapPackagesConfigurer.class)) {
+      configurer.configure(config, builder);
+    }
+    BootstrapPackagePrefixesHolder.setBoostrapPackagePrefixes(builder.build());
+  }
+
   private static void runBeforeAgentListeners(
       Iterable<AgentListener> agentListeners, Config config) {
     for (AgentListener agentListener : agentListeners) {
@@ -239,21 +252,6 @@ public class AgentInstaller {
     }
   }
 
-  private static List<String> loadBootstrapPackagePrefixes() {
-    List<String> bootstrapPackages = new ArrayList<>(Constants.BOOTSTRAP_PACKAGE_PREFIXES);
-    Iterable<BootstrapPackagesProvider> bootstrapPackagesProviders =
-        SafeServiceLoader.load(BootstrapPackagesProvider.class);
-    for (BootstrapPackagesProvider provider : bootstrapPackagesProviders) {
-      List<String> packagePrefixes = provider.getPackagePrefixes();
-      log.debug(
-          "Loaded bootstrap package prefixes from {}: {}",
-          provider.getClass().getName(),
-          packagePrefixes);
-      bootstrapPackages.addAll(packagePrefixes);
-    }
-    return bootstrapPackages;
-  }
-
   static class RedefinitionLoggingListener implements AgentBuilder.RedefinitionStrategy.Listener {
 
     private static final Logger log = LoggerFactory.getLogger(RedefinitionLoggingListener.class);

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/bootstrap/BootstrapPackagesBuilderImpl.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.tooling.bootstrap;
+
+import io.opentelemetry.javaagent.extension.bootstrap.BootstrapPackagesBuilder;
+import io.opentelemetry.javaagent.tooling.Constants;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+public class BootstrapPackagesBuilderImpl implements BootstrapPackagesBuilder {
+
+  // TODO: consider using a Trie
+  private final List<String> packages = new ArrayList<>(Constants.BOOTSTRAP_PACKAGE_PREFIXES);
+
+  @Override
+  public BootstrapPackagesBuilder add(String classNameOrPrefix) {
+    packages.add(classNameOrPrefix);
+    return this;
+  }
+
+  @Override
+  public BootstrapPackagesBuilder addAll(Collection<String> classNamesOrPrefixes) {
+    packages.addAll(classNamesOrPrefixes);
+    return this;
+  }
+
+  public List<String> build() {
+    return packages;
+  }
+}

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java

@@ -5,9 +5,10 @@
 
 package io.opentelemetry.javaagent.tooling.config;
 
+import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.loadOrdered;
+
 import io.opentelemetry.instrumentation.api.config.Config;
-import io.opentelemetry.javaagent.spi.config.PropertySource;
-import io.opentelemetry.javaagent.tooling.SafeServiceLoader;
+import io.opentelemetry.javaagent.extension.config.ConfigPropertySource;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -41,7 +42,7 @@ public final class ConfigInitializer {
   /** Retrieves all default configuration overloads using SPI and initializes Config. */
   private static Properties loadSpiConfiguration() {
     Properties propertiesFromSpi = new Properties();
-    for (PropertySource propertySource : SafeServiceLoader.load(PropertySource.class)) {
+    for (ConfigPropertySource propertySource : loadOrdered(ConfigPropertySource.class)) {
       propertiesFromSpi.putAll(propertySource.getProperties());
     }
     return propertiesFromSpi;

testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/exporter/AgentTestingExporterPropertySource.java

@@ -6,12 +6,12 @@
 package io.opentelemetry.javaagent.testing.exporter;
 
 import com.google.auto.service.AutoService;
-import io.opentelemetry.javaagent.spi.config.PropertySource;
+import io.opentelemetry.javaagent.extension.config.ConfigPropertySource;
 import java.util.HashMap;
 import java.util.Map;
 
-@AutoService(PropertySource.class)
-public class AgentTestingExporterPropertySource implements PropertySource {
+@AutoService(ConfigPropertySource.class)
+public class AgentTestingExporterPropertySource implements ConfigPropertySource {
   @Override
   public Map<String, String> getProperties() {
     Map<String, String> properties = new HashMap<>();