From 99be242e22340810a16a4911480b8a363647c249 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 7 Jun 2021 19:50:19 +0200 Subject: [PATCH] Rename ComponentInstaller to AgentListener and add #order() method (#3182) * Rename ComponentInstaller to AgentListener and add #order() method * Code review comments * Update javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/SafeServiceLoader.java Co-authored-by: Nikita Salnikov-Tarnovski Co-authored-by: Nikita Salnikov-Tarnovski --- .../oshi/OshiMetricsInstaller.java | 10 +- .../RuntimeMetricsInstaller.java | 10 +- .../extension/{spi => }/AgentExtension.java | 12 +- .../javaagent/extension/AgentListener.java | 39 ++++++ .../javaagent/extension/Ordered.java | 16 +++ .../InstrumentationModule.java | 12 +- .../javaagent/spi/ComponentInstaller.java | 34 ------ .../javaagent/tooling/AgentInstaller.java | 115 ++++++++---------- .../tooling/OpenTelemetryInstaller.java | 8 +- .../javaagent/tooling}/SafeServiceLoader.java | 14 ++- .../tooling/config/ConfigInitializer.java | 4 +- .../InstrumentationLoader.java | 15 +-- .../muzzle/MuzzleBytecodeTransformTest.groovy | 3 +- .../testing/bytebuddy/TestAgentExtension.java | 2 +- 14 files changed, 143 insertions(+), 151 deletions(-) rename javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/{spi => }/AgentExtension.java (79%) create mode 100644 javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java create mode 100644 javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/Ordered.java delete mode 100644 javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/ComponentInstaller.java rename {javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api => javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling}/SafeServiceLoader.java (76%) diff --git a/instrumentation/oshi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java b/instrumentation/oshi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java index bb87a7c1f6ed..83c706abdde7 100644 --- a/instrumentation/oshi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java +++ b/instrumentation/oshi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java @@ -7,18 +7,18 @@ import com.google.auto.service.AutoService; import io.opentelemetry.instrumentation.api.config.Config; -import io.opentelemetry.javaagent.spi.ComponentInstaller; +import io.opentelemetry.javaagent.extension.AgentListener; import java.lang.reflect.Method; import java.util.Collections; /** - * {@link ComponentInstaller} to enable oshi metrics during agent startup if oshi is present on the + * An {@link AgentListener} that enables oshi metrics during agent startup if oshi is present on the * system classpath. */ -@AutoService(ComponentInstaller.class) -public class OshiMetricsInstaller implements ComponentInstaller { +@AutoService(AgentListener.class) +public class OshiMetricsInstaller implements AgentListener { @Override - public void afterByteBuddyAgent(Config config) { + public void afterAgent(Config config) { if (config.isInstrumentationEnabled( Collections.singleton("oshi"), /* defaultEnabled= */ true)) { try { diff --git a/instrumentation/runtime-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/RuntimeMetricsInstaller.java b/instrumentation/runtime-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/RuntimeMetricsInstaller.java index c0671fb6353b..df64f75075dd 100644 --- a/instrumentation/runtime-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/RuntimeMetricsInstaller.java +++ b/instrumentation/runtime-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/RuntimeMetricsInstaller.java @@ -9,14 +9,14 @@ import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.runtimemetrics.GarbageCollector; import io.opentelemetry.instrumentation.runtimemetrics.MemoryPools; -import io.opentelemetry.javaagent.spi.ComponentInstaller; +import io.opentelemetry.javaagent.extension.AgentListener; import java.util.Collections; -/** {@link ComponentInstaller} to enable runtime metrics during agent startup. */ -@AutoService(ComponentInstaller.class) -public class RuntimeMetricsInstaller implements ComponentInstaller { +/** An {@link AgentListener} that enables runtime metrics during agent startup. */ +@AutoService(AgentListener.class) +public class RuntimeMetricsInstaller implements AgentListener { @Override - public void afterByteBuddyAgent(Config config) { + public void afterAgent(Config config) { if (config.isInstrumentationEnabled( Collections.singleton("runtime-metrics"), /* defaultEnabled= */ true)) { GarbageCollector.registerObservers(); diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/spi/AgentExtension.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentExtension.java similarity index 79% rename from javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/spi/AgentExtension.java rename to javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentExtension.java index 946bfff2f27b..5dfcc46f884a 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/spi/AgentExtension.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentExtension.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.extension.spi; +package io.opentelemetry.javaagent.extension; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import net.bytebuddy.agent.builder.AgentBuilder; @@ -16,7 +16,7 @@ *

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 AgentExtension { +public interface AgentExtension extends Ordered { /** * Extend the passed {@code agentBuilder} with custom logic (e.g. instrumentation). @@ -31,12 +31,4 @@ public interface AgentExtension { * human-readable: javaagent uses the extension name in its logs. */ String extensionName(); - - /** - * Returns the order of adding extensions to the javaagent. Higher values are added later, for - * example: an extension with order=1 will run after an extension with order=0. - */ - default int order() { - return 0; - } } diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java new file mode 100644 index 000000000000..007e6de520f5 --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java @@ -0,0 +1,39 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.extension; + +import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.javaagent.instrumentation.api.InstrumentationContext; +import java.lang.instrument.Instrumentation; +import java.util.Map; +import net.bytebuddy.agent.builder.AgentBuilder; + +/** + * {@link AgentListener} can be used to execute code before/after Java agent installation, for + * example to install any implementation providers that are used by instrumentations. For instance, + * this project uses this SPI to install OpenTelemetry SDK. + * + *

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 AgentListener extends Ordered { + + /** + * Runs before the {@link AgentBuilder} construction, before any instrumentation is added. + * + *

Execute only a minimal code because any classes loaded before the agent installation will + * have to be retransformed, which takes extra time, and more importantly means that fields can't + * be added to those classes - which causes {@link InstrumentationContext} to fall back to the + * less performant {@link Map} implementation for those classes. + */ + default void beforeAgent(Config config) {} + + /** + * Runs after instrumentations are added to {@link AgentBuilder} and after the agent is installed + * on an {@link Instrumentation}. + */ + default void afterAgent(Config config) {} +} diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/Ordered.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/Ordered.java new file mode 100644 index 000000000000..76b7f0dd18e7 --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/Ordered.java @@ -0,0 +1,16 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.extension; + +public interface Ordered { + /** + * Returns the order of applying the SPI implementing this interface. Higher values are added + * later, for example: an SPI with order=1 will run after an SPI with order=0. + */ + default int order() { + return 0; + } +} diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/InstrumentationModule.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/InstrumentationModule.java index 85be375231ce..84eb884ff394 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/InstrumentationModule.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/InstrumentationModule.java @@ -9,6 +9,7 @@ import static net.bytebuddy.matcher.ElementMatchers.any; import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.javaagent.extension.Ordered; import io.opentelemetry.javaagent.extension.muzzle.ClassRef; import java.util.ArrayList; import java.util.Collections; @@ -30,7 +31,7 @@ * META-INF/services/} provider file is created for it to be picked up by the agent. See {@link * java.util.ServiceLoader} for more details. */ -public abstract class InstrumentationModule { +public abstract class InstrumentationModule implements Ordered { private static final boolean DEFAULT_ENABLED = Config.get().getBooleanProperty("otel.instrumentation.common.default-enabled", true); @@ -101,15 +102,6 @@ protected boolean defaultEnabled() { return DEFAULT_ENABLED; } - /** - * Returns the order of adding instrumentation modules to the javaagent. Higher values are added - * later, for example: an instrumentation module with order=1 will run after a module with - * order=0. - */ - public int order() { - return 0; - } - /** * Instrumentation modules can override this method to specify additional packages (or classes) * that should be treated as "library instrumentation" packages. Classes from those packages will diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/ComponentInstaller.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/ComponentInstaller.java deleted file mode 100644 index d1f8a66b28ad..000000000000 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/ComponentInstaller.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.spi; - -import io.opentelemetry.instrumentation.api.config.Config; - -/** - * {@link ComponentInstaller} can be used to install any implementation providers that are used by - * instrumentations. For instance Java agent uses this to install OpenTelemetry SDK. The - * instrumentation uses shaded OpenTelemetry API that lives in the bootstrap classloader and the - * implementation (SDK) is installed via service loader from agent's classloader. This way the - * application does not have a direct access to the OpenTelemetry SDK classes. The same approach can - * be done for other APIs used by custom instrumentations. - * - *

This is a service provider interface that requires implementations to be registered in {@code - * META-INF/services} folder. - */ -public interface ComponentInstaller { - - /** - * Runs before instrumentations are installed to ByteBuddy. Execute only a minimal code because - * any classes loaded before the instrumentations are installed will have to be retransformed, - * which takes extra time, and more importantly means that fields can't be added to those classes - * and InstrumentationContext falls back to the less performant Map implementation for those - * classes. - */ - default void beforeByteBuddyAgent(Config config) {} - - /** Runs after instrumentations are added to ByteBuddy. */ - default void afterByteBuddyAgent(Config config) {} -} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java index 6344c68c4b63..1f9b2cb642ce 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java +++ b/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.loadOrdered; import static io.opentelemetry.javaagent.tooling.Utils.getResourceName; import static io.opentelemetry.javaagent.tooling.matcher.GlobalIgnoresMatcher.globalIgnoresMatcher; import static net.bytebuddy.matcher.ElementMatchers.any; @@ -14,12 +15,11 @@ 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.instrumentation.InstrumentationModule; -import io.opentelemetry.javaagent.extension.spi.AgentExtension; -import io.opentelemetry.javaagent.instrumentation.api.SafeServiceLoader; import io.opentelemetry.javaagent.instrumentation.api.internal.BootstrapPackagePrefixesHolder; import io.opentelemetry.javaagent.spi.BootstrapPackagesProvider; -import io.opentelemetry.javaagent.spi.ComponentInstaller; import io.opentelemetry.javaagent.spi.IgnoreMatcherProvider; import io.opentelemetry.javaagent.tooling.config.ConfigInitializer; import io.opentelemetry.javaagent.tooling.context.FieldBackedProvider; @@ -28,15 +28,12 @@ import java.lang.reflect.Proxy; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.ServiceLoader; import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.StreamSupport; import net.bytebuddy.agent.builder.AgentBuilder; @@ -58,12 +55,12 @@ public class AgentInstaller { private static final String JAVAAGENT_ENABLED_CONFIG = "otel.javaagent.enabled"; private static final String EXCLUDED_CLASSES_CONFIG = "otel.javaagent.exclude-classes"; - // This property may be set to force synchronous ComponentInstaller#afterByteBuddyAgent() - // execution: the condition for delaying the ComponentInstaller initialization is pretty broad - // and in case it covers too much javaagent users can file a bug, force sync execution by setting - // this property to true and continue using the javaagent - private static final String FORCE_SYNCHRONOUS_COMPONENT_INSTALLER_CONFIG = - "otel.javaagent.experimental.force-synchronous-component-installers"; + // This property may be set to force synchronous AgentListener#afterAgent() execution: the + // condition for delaying the AgentListener initialization is pretty broad and in case it covers + // too much javaagent users can file a bug, force sync execution by setting this property to true + // and continue using the javaagent + private static final String FORCE_SYNCHRONOUS_AGENT_LISTENERS_CONFIG = + "otel.javaagent.experimental.force-synchronous-agent-listeners"; // We set this system property when running the agent with unit tests to allow verifying that we // don't ignore libraries that we actually attempt to instrument. It means either the list is @@ -99,8 +96,8 @@ public static void installBytebuddyAgent(Instrumentation inst) { logVersionInfo(); Config config = Config.get(); if (config.getBooleanProperty(JAVAAGENT_ENABLED_CONFIG, true)) { - Iterable componentInstallers = loadComponentProviders(); - installBytebuddyAgent(inst, componentInstallers); + List agentListeners = loadOrdered(AgentListener.class); + installBytebuddyAgent(inst, agentListeners); } else { log.debug("Tracing is disabled, not installing instrumentations."); } @@ -114,10 +111,10 @@ public static void installBytebuddyAgent(Instrumentation inst) { * @return the agent's class transformer */ public static ResettableClassFileTransformer installBytebuddyAgent( - Instrumentation inst, Iterable componentInstallers) { + Instrumentation inst, Iterable agentListeners) { Config config = Config.get(); - installComponentsBeforeByteBuddy(componentInstallers, config); + runBeforeAgentListeners(agentListeners, config); instrumentation = inst; @@ -160,7 +157,7 @@ public static ResettableClassFileTransformer installBytebuddyAgent( } int numberOfLoadedExtensions = 0; - for (AgentExtension agentExtension : loadAgentExtensions()) { + for (AgentExtension agentExtension : loadOrdered(AgentExtension.class)) { log.debug( "Loading extension {} [class {}]", agentExtension.extensionName(), @@ -179,60 +176,53 @@ public static ResettableClassFileTransformer installBytebuddyAgent( log.debug("Installed {} extension(s)", numberOfLoadedExtensions); ResettableClassFileTransformer resettableClassFileTransformer = agentBuilder.installOn(inst); - installComponentsAfterByteBuddy(componentInstallers, config); + runAfterAgentListeners(agentListeners, config); return resettableClassFileTransformer; } - private static void installComponentsBeforeByteBuddy( - Iterable componentInstallers, Config config) { - for (ComponentInstaller componentInstaller : componentInstallers) { - componentInstaller.beforeByteBuddyAgent(config); + private static void runBeforeAgentListeners( + Iterable agentListeners, Config config) { + for (AgentListener agentListener : agentListeners) { + agentListener.beforeAgent(config); } } - private static void installComponentsAfterByteBuddy( - Iterable componentInstallers, Config config) { + private static void runAfterAgentListeners( + Iterable agentListeners, Config config) { // java.util.logging.LogManager maintains a final static LogManager, which is created during - // class initialization. Some ComponentInstaller implementations may use JRE bootstrap classes + // class initialization. Some AgentListener implementations may use JRE bootstrap classes // which touch this class (e.g. JFR classes or some MBeans). // It is worth noting that starting from Java 9 (JEP 264) Java platform classes no longer use // JUL directly, but instead they use a new System.Logger interface, so the LogManager issue // applies mainly to Java 8. // This means applications which require a custom LogManager may not have a chance to set the - // global LogManager if one of those ComponentInstallers runs first: it will incorrectly + // global LogManager if one of those AgentListeners runs first: it will incorrectly // set the global LogManager to the default JVM one in cases where the instrumented application // sets the LogManager system property or when the custom LogManager class is not on the system // classpath. - // Our solution is to delay the initialization of ComponentInstallers when we detect a custom + // Our solution is to delay the initialization of AgentListeners when we detect a custom // log manager being used. - // Once we see the LogManager class loading, it's safe to run - // ComponentInstaller#afterByteBuddyAgent() because the application is already setting the - // global LogManager and ComponentInstaller won't be able to touch it due to classloader - // locking. - boolean shouldForceSynchronousComponentInstallerCalls = - Config.get().getBooleanProperty(FORCE_SYNCHRONOUS_COMPONENT_INSTALLER_CONFIG, false); - if (!shouldForceSynchronousComponentInstallerCalls + // Once we see the LogManager class loading, it's safe to run AgentListener#afterAgent() because + // the application is already setting the global LogManager and AgentListener won't be able + // to touch it due to classloader locking. + boolean shouldForceSynchronousAgentListenersCalls = + Config.get().getBooleanProperty(FORCE_SYNCHRONOUS_AGENT_LISTENERS_CONFIG, false); + if (!shouldForceSynchronousAgentListenersCalls && isJavaBefore9() && isAppUsingCustomLogManager()) { - log.debug( - "Custom JUL LogManager detected: delaying ComponentInstaller#afterByteBuddyAgent() calls"); + log.debug("Custom JUL LogManager detected: delaying AgentListener#afterAgent() calls"); registerClassLoadCallback( - "java.util.logging.LogManager", - new InstallComponentAfterByteBuddyCallback(config, componentInstallers)); + "java.util.logging.LogManager", new DelayedAfterAgentCallback(config, agentListeners)); } else { - for (ComponentInstaller componentInstaller : componentInstallers) { - componentInstaller.afterByteBuddyAgent(config); + for (AgentListener agentListener : agentListeners) { + agentListener.afterAgent(config); } } } - private static Iterable loadComponentProviders() { - return ServiceLoader.load(ComponentInstaller.class); - } - private static IgnoreMatcherProvider loadIgnoreMatcherProvider() { - ServiceLoader ignoreMatcherProviders = - ServiceLoader.load(IgnoreMatcherProvider.class); + Iterable ignoreMatcherProviders = + SafeServiceLoader.load(IgnoreMatcherProvider.class); Iterator iterator = ignoreMatcherProviders.iterator(); if (iterator.hasNext()) { @@ -241,12 +231,6 @@ private static IgnoreMatcherProvider loadIgnoreMatcherProvider() { return new NoopIgnoreMatcherProvider(); } - private static List loadAgentExtensions() { - return SafeServiceLoader.load(AgentExtension.class).stream() - .sorted(Comparator.comparingInt(AgentExtension::order)) - .collect(Collectors.toList()); - } - private static void addByteBuddyRawSetting() { String savedPropertyValue = System.getProperty(TypeDefinition.RAW_TYPES_PROPERTY); try { @@ -394,13 +378,12 @@ public static void registerClassLoadCallback(String className, Runnable callback } } - private static class InstallComponentAfterByteBuddyCallback implements Runnable { - private final Iterable componentInstallers; + private static class DelayedAfterAgentCallback implements Runnable { + private final Iterable agentListeners; private final Config config; - private InstallComponentAfterByteBuddyCallback( - Config config, Iterable componentInstallers) { - this.componentInstallers = componentInstallers; + private DelayedAfterAgentCallback(Config config, Iterable agentListeners) { + this.agentListeners = agentListeners; this.config = config; } @@ -411,18 +394,18 @@ public void run() { * to load classes being transformed. To avoid this we start a thread here that calls the callback. * This seems to resolve this problem. */ - Thread thread = new Thread(this::runComponentInstallers); - thread.setName("agent-component-installers"); + Thread thread = new Thread(this::runAgentListeners); + thread.setName("delayed-agent-listeners"); thread.setDaemon(true); thread.start(); } - private void runComponentInstallers() { - for (ComponentInstaller componentInstaller : componentInstallers) { + private void runAgentListeners() { + for (AgentListener agentListener : agentListeners) { try { - componentInstaller.afterByteBuddyAgent(config); + agentListener.afterAgent(config); } catch (RuntimeException e) { - log.error("Failed to execute {}", componentInstaller.getClass().getName(), e); + log.error("Failed to execute {}", agentListener.getClass().getName(), e); } } } @@ -506,7 +489,7 @@ private static boolean isAppUsingCustomLogManager() { // JBoss/Wildfly is known to set a custom log manager after startup. // Originally we were checking for the presence of a jboss class, // but it seems some non-jboss applications have jboss classes on the classpath. - // This would cause ComponentInstaller initialization to be delayed indefinitely. + // This would cause AgentListener#afterAgent() calls to be delayed indefinitely. // Checking for an environment variable required by jboss instead. return true; } @@ -519,12 +502,12 @@ private static boolean isAppUsingCustomLogManager() { boolean onSysClasspath = ClassLoader.getSystemResource(getResourceName(customLogManager)) != null; log.debug( - "Class {} is on system classpath: {}delaying ComponentInstaller#afterByteBuddyAgent()", + "Class {} is on system classpath: {}delaying AgentInstaller#afterAgent()", customLogManager, onSysClasspath ? "not " : ""); // Some applications set java.util.logging.manager but never actually initialize the logger. // Check to see if the configured manager is on the system classpath. - // If so, it should be safe to initialize ComponentInstaller which will setup the log manager: + // If so, it should be safe to initialize AgentInstaller which will setup the log manager: // LogManager tries to load the implementation first using system CL, then falls back to // current context CL return !onSysClasspath; diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java index df801b4ebf20..7411416d2604 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/OpenTelemetryInstaller.java @@ -8,22 +8,22 @@ import com.google.auto.service.AutoService; import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.config.ConfigBuilder; +import io.opentelemetry.javaagent.extension.AgentListener; import io.opentelemetry.javaagent.instrumentation.api.OpenTelemetrySdkAccess; -import io.opentelemetry.javaagent.spi.ComponentInstaller; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.OpenTelemetrySdkAutoConfiguration; import java.util.Properties; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@AutoService(ComponentInstaller.class) -public class OpenTelemetryInstaller implements ComponentInstaller { +@AutoService(AgentListener.class) +public class OpenTelemetryInstaller implements AgentListener { private static final Logger log = LoggerFactory.getLogger(OpenTelemetryInstaller.class); static final String JAVAAGENT_ENABLED_CONFIG = "otel.javaagent.enabled"; @Override - public void beforeByteBuddyAgent(Config config) { + public void beforeAgent(Config config) { installAgentTracer(config); } diff --git a/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/SafeServiceLoader.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/SafeServiceLoader.java similarity index 76% rename from javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/SafeServiceLoader.java rename to javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/SafeServiceLoader.java index f31b37dc25d7..3b1942071dab 100644 --- a/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/SafeServiceLoader.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/SafeServiceLoader.java @@ -3,9 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.instrumentation.api; +package io.opentelemetry.javaagent.tooling; +import io.opentelemetry.javaagent.extension.Ordered; import java.util.ArrayList; +import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.ServiceLoader; @@ -40,5 +42,15 @@ public static List load(Class serviceClass) { return result; } + /** + * Same as {@link #load(Class)}, but also orders the returned implementations by comparing their + * {@link Ordered#order()}. + */ + public static List loadOrdered(Class serviceClass) { + List result = load(serviceClass); + result.sort(Comparator.comparing(Ordered::order)); + return result; + } + private SafeServiceLoader() {} } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java index 8463611d358c..35abb217c2e0 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java @@ -8,6 +8,7 @@ import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.config.ConfigBuilder; import io.opentelemetry.javaagent.spi.config.PropertySource; +import io.opentelemetry.javaagent.tooling.SafeServiceLoader; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -15,7 +16,6 @@ import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.util.Properties; -import java.util.ServiceLoader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,7 +42,7 @@ static Config create(Properties spiConfiguration, Properties configurationFile) /** Retrieves all default configuration overloads using SPI and initializes Config. */ private static Properties loadSpiConfiguration() { Properties propertiesFromSpi = new Properties(); - for (PropertySource propertySource : ServiceLoader.load(PropertySource.class)) { + for (PropertySource propertySource : SafeServiceLoader.load(PropertySource.class)) { propertiesFromSpi.putAll(propertySource.getProperties()); } return propertiesFromSpi; diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationLoader.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationLoader.java index 520154eebeaa..1e9baa4d4175 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationLoader.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationLoader.java @@ -5,13 +5,11 @@ package io.opentelemetry.javaagent.tooling.instrumentation; +import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.loadOrdered; + import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.AgentExtension; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; -import io.opentelemetry.javaagent.extension.spi.AgentExtension; -import io.opentelemetry.javaagent.instrumentation.api.SafeServiceLoader; -import java.util.Comparator; -import java.util.List; -import java.util.stream.Collectors; import net.bytebuddy.agent.builder.AgentBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -25,13 +23,8 @@ public class InstrumentationLoader implements AgentExtension { @Override public AgentBuilder extend(AgentBuilder agentBuilder) { - List instrumentationModules = - SafeServiceLoader.load(InstrumentationModule.class).stream() - .sorted(Comparator.comparingInt(InstrumentationModule::order)) - .collect(Collectors.toList()); - int numberOfLoadedModules = 0; - for (InstrumentationModule instrumentationModule : instrumentationModules) { + for (InstrumentationModule instrumentationModule : loadOrdered(InstrumentationModule.class)) { log.debug( "Loading instrumentation {} [class {}]", instrumentationModule.instrumentationName(), diff --git a/javaagent/src/test/groovy/io/opentelemetry/javaagent/muzzle/MuzzleBytecodeTransformTest.groovy b/javaagent/src/test/groovy/io/opentelemetry/javaagent/muzzle/MuzzleBytecodeTransformTest.groovy index ad16ed90077b..032cb99411fd 100644 --- a/javaagent/src/test/groovy/io/opentelemetry/javaagent/muzzle/MuzzleBytecodeTransformTest.groovy +++ b/javaagent/src/test/groovy/io/opentelemetry/javaagent/muzzle/MuzzleBytecodeTransformTest.groovy @@ -6,7 +6,6 @@ package io.opentelemetry.javaagent.muzzle import io.opentelemetry.javaagent.IntegrationTestUtils -import io.opentelemetry.javaagent.instrumentation.api.SafeServiceLoader import java.lang.reflect.Field import java.lang.reflect.Method import spock.lang.Specification @@ -19,7 +18,7 @@ class MuzzleBytecodeTransformTest extends Specification { List nonLazyFields = [] List unInitFields = [] def instrumentationModuleClass = IntegrationTestUtils.getAgentClassLoader().loadClass("io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule") - for (Object instrumenter : SafeServiceLoader.load(instrumentationModuleClass)) { + for (Object instrumenter : ServiceLoader.load(instrumentationModuleClass)) { if (!instrumentationModuleClass.isAssignableFrom(instrumenter.getClass())) { // muzzle only applies to default instrumenters continue diff --git a/testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/bytebuddy/TestAgentExtension.java b/testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/bytebuddy/TestAgentExtension.java index a40d53af8fa0..8f2c0a590d05 100644 --- a/testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/bytebuddy/TestAgentExtension.java +++ b/testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/bytebuddy/TestAgentExtension.java @@ -6,7 +6,7 @@ package io.opentelemetry.javaagent.testing.bytebuddy; import com.google.auto.service.AutoService; -import io.opentelemetry.javaagent.extension.spi.AgentExtension; +import io.opentelemetry.javaagent.extension.AgentExtension; import net.bytebuddy.agent.builder.AgentBuilder; @AutoService(AgentExtension.class)