Don't call Thread#setContextClassLoader() (#7391)

Related to #7220

Unfortunately it doesn't fix the aforementioned issue; while the CL used
is no longer the agent classloader, gauge collection still throws that
error.
Still, I think this is a good change that removes one source of agent's
CL leaking into application runtime.
This commit is contained in:
Mateusz Rzeszutek 2022-12-13 19:24:40 +01:00 committed by GitHub
parent 9e5d9623c3
commit c03bfc255b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 50 additions and 53 deletions

View File

@ -19,7 +19,6 @@ import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties; import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties;
import io.opentelemetry.javaagent.bootstrap.AgentClassLoader; import io.opentelemetry.javaagent.bootstrap.AgentClassLoader;
import io.opentelemetry.javaagent.bootstrap.AgentInitializer;
import io.opentelemetry.javaagent.bootstrap.BootstrapPackagePrefixesHolder; import io.opentelemetry.javaagent.bootstrap.BootstrapPackagePrefixesHolder;
import io.opentelemetry.javaagent.bootstrap.ClassFileTransformerHolder; import io.opentelemetry.javaagent.bootstrap.ClassFileTransformerHolder;
import io.opentelemetry.javaagent.bootstrap.DefineClassHelper; import io.opentelemetry.javaagent.bootstrap.DefineClassHelper;
@ -78,7 +77,7 @@ public class AgentInstaller {
private static final Map<String, List<Runnable>> CLASS_LOAD_CALLBACKS = new HashMap<>(); private static final Map<String, List<Runnable>> CLASS_LOAD_CALLBACKS = new HashMap<>();
public static void installBytebuddyAgent(Instrumentation inst) { public static void installBytebuddyAgent(Instrumentation inst, ClassLoader extensionClassLoader) {
addByteBuddyRawSetting(); addByteBuddyRawSetting();
Integer strictContextStressorMillis = Integer.getInteger(STRICT_CONTEXT_STRESSOR_MILLIS); Integer strictContextStressorMillis = Integer.getInteger(STRICT_CONTEXT_STRESSOR_MILLIS);
@ -90,34 +89,37 @@ public class AgentInstaller {
logVersionInfo(); logVersionInfo();
if (ConfigPropertiesUtil.getBoolean(JAVAAGENT_ENABLED_CONFIG, true)) { if (ConfigPropertiesUtil.getBoolean(JAVAAGENT_ENABLED_CONFIG, true)) {
setupUnsafe(inst); setupUnsafe(inst);
List<AgentListener> agentListeners = loadOrdered(AgentListener.class); List<AgentListener> agentListeners = loadOrdered(AgentListener.class, extensionClassLoader);
installBytebuddyAgent(inst, agentListeners); installBytebuddyAgent(inst, extensionClassLoader, agentListeners);
} else { } else {
logger.fine("Tracing is disabled, not installing instrumentations."); logger.fine("Tracing is disabled, not installing instrumentations.");
} }
} }
private static void installBytebuddyAgent( private static void installBytebuddyAgent(
Instrumentation inst, Iterable<AgentListener> agentListeners) { Instrumentation inst,
ClassLoader extensionClassLoader,
Iterable<AgentListener> agentListeners) {
WeakRefAsyncOperationEndStrategies.initialize(); WeakRefAsyncOperationEndStrategies.initialize();
EmbeddedInstrumentationProperties.setPropertiesLoader( EmbeddedInstrumentationProperties.setPropertiesLoader(extensionClassLoader);
AgentInitializer.getExtensionsClassLoader());
setDefineClassHandler(); setDefineClassHandler();
// If noop OpenTelemetry is enabled, autoConfiguredSdk will be null and AgentListeners are not // If noop OpenTelemetry is enabled, autoConfiguredSdk will be null and AgentListeners are not
// called // called
AutoConfiguredOpenTelemetrySdk autoConfiguredSdk = installOpenTelemetrySdk(); AutoConfiguredOpenTelemetrySdk autoConfiguredSdk =
installOpenTelemetrySdk(extensionClassLoader);
ConfigProperties sdkConfig = autoConfiguredSdk.getConfig(); ConfigProperties sdkConfig = autoConfiguredSdk.getConfig();
InstrumentationConfig.internalInitializeConfig(new ConfigPropertiesBridge(sdkConfig)); InstrumentationConfig.internalInitializeConfig(new ConfigPropertiesBridge(sdkConfig));
copyNecessaryConfigToSystemProperties(sdkConfig); copyNecessaryConfigToSystemProperties(sdkConfig);
setBootstrapPackages(sdkConfig); setBootstrapPackages(sdkConfig, extensionClassLoader);
for (BeforeAgentListener agentListener : loadOrdered(BeforeAgentListener.class)) { for (BeforeAgentListener agentListener :
loadOrdered(BeforeAgentListener.class, extensionClassLoader)) {
agentListener.beforeAgent(autoConfiguredSdk); agentListener.beforeAgent(autoConfiguredSdk);
} }
@ -140,7 +142,7 @@ public class AgentInstaller {
agentBuilder = agentBuilder.with(new ExposeAgentBootstrapListener(inst)); agentBuilder = agentBuilder.with(new ExposeAgentBootstrapListener(inst));
} }
agentBuilder = configureIgnoredTypes(sdkConfig, agentBuilder); agentBuilder = configureIgnoredTypes(sdkConfig, extensionClassLoader, agentBuilder);
if (AgentConfig.isDebugModeEnabled(sdkConfig)) { if (AgentConfig.isDebugModeEnabled(sdkConfig)) {
agentBuilder = agentBuilder =
@ -152,7 +154,7 @@ public class AgentInstaller {
} }
int numberOfLoadedExtensions = 0; int numberOfLoadedExtensions = 0;
for (AgentExtension agentExtension : loadOrdered(AgentExtension.class)) { for (AgentExtension agentExtension : loadOrdered(AgentExtension.class, extensionClassLoader)) {
if (logger.isLoggable(FINE)) { if (logger.isLoggable(FINE)) {
logger.log( logger.log(
FINE, FINE,
@ -196,9 +198,11 @@ public class AgentInstaller {
} }
} }
private static void setBootstrapPackages(ConfigProperties config) { private static void setBootstrapPackages(
ConfigProperties config, ClassLoader extensionClassLoader) {
BootstrapPackagesBuilderImpl builder = new BootstrapPackagesBuilderImpl(); BootstrapPackagesBuilderImpl builder = new BootstrapPackagesBuilderImpl();
for (BootstrapPackagesConfigurer configurer : load(BootstrapPackagesConfigurer.class)) { for (BootstrapPackagesConfigurer configurer :
load(BootstrapPackagesConfigurer.class, extensionClassLoader)) {
configurer.configure(builder, config); configurer.configure(builder, config);
} }
BootstrapPackagePrefixesHolder.setBoostrapPackagePrefixes(builder.build()); BootstrapPackagePrefixesHolder.setBoostrapPackagePrefixes(builder.build());
@ -209,9 +213,10 @@ public class AgentInstaller {
} }
private static AgentBuilder configureIgnoredTypes( private static AgentBuilder configureIgnoredTypes(
ConfigProperties config, AgentBuilder agentBuilder) { ConfigProperties config, ClassLoader extensionClassLoader, AgentBuilder agentBuilder) {
IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl(); IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl();
for (IgnoredTypesConfigurer configurer : loadOrdered(IgnoredTypesConfigurer.class)) { for (IgnoredTypesConfigurer configurer :
loadOrdered(IgnoredTypesConfigurer.class, extensionClassLoader)) {
configurer.configure(builder, config); configurer.configure(builder, config);
} }

View File

@ -62,18 +62,9 @@ public class AgentStarterImpl implements AgentStarter {
@Override @Override
public void start() { public void start() {
extensionClassLoader = createExtensionClassLoader(getClass().getClassLoader(), javaagentFile); extensionClassLoader = createExtensionClassLoader(getClass().getClassLoader(), javaagentFile);
ClassLoader savedContextClassLoader = Thread.currentThread().getContextClassLoader();
try {
Thread.currentThread().setContextClassLoader(extensionClassLoader);
internalStart();
} finally {
Thread.currentThread().setContextClassLoader(savedContextClassLoader);
}
}
private void internalStart() {
Iterator<LoggingCustomizer> loggingCustomizers = Iterator<LoggingCustomizer> loggingCustomizers =
ServiceLoader.load(LoggingCustomizer.class).iterator(); ServiceLoader.load(LoggingCustomizer.class, extensionClassLoader).iterator();
LoggingCustomizer loggingCustomizer; LoggingCustomizer loggingCustomizer;
if (loggingCustomizers.hasNext()) { if (loggingCustomizers.hasNext()) {
loggingCustomizer = loggingCustomizers.next(); loggingCustomizer = loggingCustomizers.next();
@ -84,7 +75,7 @@ public class AgentStarterImpl implements AgentStarter {
Throwable startupError = null; Throwable startupError = null;
try { try {
loggingCustomizer.init(); loggingCustomizer.init();
AgentInstaller.installBytebuddyAgent(instrumentation); AgentInstaller.installBytebuddyAgent(instrumentation, extensionClassLoader);
WeakConcurrentMapCleaner.start(); WeakConcurrentMapCleaner.start();
} catch (Throwable t) { } catch (Throwable t) {
// this is logged below and not rethrown to avoid logging it twice // this is logged below and not rethrown to avoid logging it twice

View File

@ -5,11 +5,9 @@
package io.opentelemetry.javaagent.tooling; package io.opentelemetry.javaagent.tooling;
import io.opentelemetry.javaagent.bootstrap.AgentInitializer;
import io.opentelemetry.javaagent.bootstrap.OpenTelemetrySdkAccess; import io.opentelemetry.javaagent.bootstrap.OpenTelemetrySdkAccess;
import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder;
import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.CompletableResultCode;
import java.util.Arrays; import java.util.Arrays;
@ -21,17 +19,14 @@ public final class OpenTelemetryInstaller {
* *
* @return the {@link AutoConfiguredOpenTelemetrySdk} * @return the {@link AutoConfiguredOpenTelemetrySdk}
*/ */
public static AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk() { public static AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk(
AutoConfiguredOpenTelemetrySdkBuilder builder = ClassLoader extensionClassLoader) {
AutoConfiguredOpenTelemetrySdk.builder().setResultAsGlobal(true);
ClassLoader classLoader = AgentInitializer.getExtensionsClassLoader(); AutoConfiguredOpenTelemetrySdk autoConfiguredSdk =
if (classLoader != null) { AutoConfiguredOpenTelemetrySdk.builder()
// May be null in unit tests. .setResultAsGlobal(true)
builder.setServiceClassLoader(classLoader); .setServiceClassLoader(extensionClassLoader)
} .build();
AutoConfiguredOpenTelemetrySdk autoConfiguredSdk = builder.build();
OpenTelemetrySdk sdk = autoConfiguredSdk.getOpenTelemetrySdk(); OpenTelemetrySdk sdk = autoConfiguredSdk.getOpenTelemetrySdk();
OpenTelemetrySdkAccess.internalSetForceFlush( OpenTelemetrySdkAccess.internalSetForceFlush(

View File

@ -30,9 +30,9 @@ public final class SafeServiceLoader {
*/ */
// Because we want to catch exception per iteration // Because we want to catch exception per iteration
@SuppressWarnings("ForEachIterable") @SuppressWarnings("ForEachIterable")
public static <T> List<T> load(Class<T> serviceClass) { public static <T> List<T> load(Class<T> serviceClass, ClassLoader classLoader) {
List<T> result = new ArrayList<>(); List<T> result = new ArrayList<>();
ServiceLoader<T> services = ServiceLoader.load(serviceClass); ServiceLoader<T> services = ServiceLoader.load(serviceClass, classLoader);
for (Iterator<T> iterator = new SafeIterator<>(services.iterator()); iterator.hasNext(); ) { for (Iterator<T> iterator = new SafeIterator<>(services.iterator()); iterator.hasNext(); ) {
T service = iterator.next(); T service = iterator.next();
if (service != null) { if (service != null) {
@ -43,11 +43,12 @@ public final class SafeServiceLoader {
} }
/** /**
* Same as {@link #load(Class)}, but also orders the returned implementations by comparing their * Same as {@link #load(Class, ClassLoader)}, but also orders the returned implementations by
* {@link Ordered#order()}. * comparing their {@link Ordered#order()}.
*/ */
public static <T extends Ordered> List<T> loadOrdered(Class<T> serviceClass) { public static <T extends Ordered> List<T> loadOrdered(
List<T> result = load(serviceClass); Class<T> serviceClass, ClassLoader classLoader) {
List<T> result = load(serviceClass, classLoader);
result.sort(Comparator.comparing(Ordered::order)); result.sort(Comparator.comparing(Ordered::order));
return result; return result;
} }

View File

@ -13,6 +13,7 @@ import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder; import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.tooling.AgentExtension; import io.opentelemetry.javaagent.tooling.AgentExtension;
import io.opentelemetry.javaagent.tooling.Utils;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.util.logging.Logger; import java.util.logging.Logger;
import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.agent.builder.AgentBuilder;
@ -27,7 +28,8 @@ public class InstrumentationLoader implements AgentExtension {
@Override @Override
public AgentBuilder extend(AgentBuilder agentBuilder, ConfigProperties config) { public AgentBuilder extend(AgentBuilder agentBuilder, ConfigProperties config) {
int numberOfLoadedModules = 0; int numberOfLoadedModules = 0;
for (InstrumentationModule instrumentationModule : loadOrdered(InstrumentationModule.class)) { for (InstrumentationModule instrumentationModule :
loadOrdered(InstrumentationModule.class, Utils.getExtensionsClassLoader())) {
if (logger.isLoggable(FINE)) { if (logger.isLoggable(FINE)) {
logger.log( logger.log(
FINE, FINE,

View File

@ -61,7 +61,7 @@ class HelperInjectionTest extends Specification {
def "helpers injected on bootstrap classloader"() { def "helpers injected on bootstrap classloader"() {
setup: setup:
ByteBuddyAgent.install() ByteBuddyAgent.install()
AgentInstaller.installBytebuddyAgent(ByteBuddyAgent.getInstrumentation()) AgentInstaller.installBytebuddyAgent(ByteBuddyAgent.getInstrumentation(), this.class.classLoader)
String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass' String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass'
HelperInjector injector = new HelperInjector("test", [helperClassName], [], this.class.classLoader, ByteBuddyAgent.getInstrumentation()) HelperInjector injector = new HelperInjector("test", [helperClassName], [], this.class.classLoader, ByteBuddyAgent.getInstrumentation())
URLClassLoader bootstrapChild = new URLClassLoader(new URL[0], (ClassLoader) null) URLClassLoader bootstrapChild = new URLClassLoader(new URL[0], (ClassLoader) null)

View File

@ -24,7 +24,7 @@ class OpenTelemetryInstallerTest extends Specification {
def "should initialize GlobalOpenTelemetry"() { def "should initialize GlobalOpenTelemetry"() {
when: when:
def otelInstaller = OpenTelemetryInstaller.installOpenTelemetrySdk() def otelInstaller = OpenTelemetryInstaller.installOpenTelemetrySdk(OpenTelemetryInstaller.classLoader)
then: then:
otelInstaller != null otelInstaller != null

View File

@ -46,7 +46,7 @@ class ConfigurationFileLoaderTest {
// when // when
AutoConfiguredOpenTelemetrySdk autoConfiguredSdk = AutoConfiguredOpenTelemetrySdk autoConfiguredSdk =
OpenTelemetryInstaller.installOpenTelemetrySdk(); OpenTelemetryInstaller.installOpenTelemetrySdk(this.getClass().getClassLoader());
// then // then
assertThat(autoConfiguredSdk.getConfig().getString("custom.key")).isEqualTo("42"); assertThat(autoConfiguredSdk.getConfig().getString("custom.key")).isEqualTo("42");

View File

@ -32,7 +32,8 @@ public final class AgentTooling {
private static ClassLoader getBootstrapProxy() { private static ClassLoader getBootstrapProxy() {
Iterator<BootstrapProxyProvider> iterator = Iterator<BootstrapProxyProvider> iterator =
ServiceLoader.load(BootstrapProxyProvider.class).iterator(); ServiceLoader.load(BootstrapProxyProvider.class, AgentTooling.class.getClassLoader())
.iterator();
if (iterator.hasNext()) { if (iterator.hasNext()) {
BootstrapProxyProvider bootstrapProxyProvider = iterator.next(); BootstrapProxyProvider bootstrapProxyProvider = iterator.next();
return bootstrapProxyProvider.getBootstrapProxy(); return bootstrapProxyProvider.getBootstrapProxy();

View File

@ -33,7 +33,7 @@ public class ClassLoaderMatcher {
public static Map<String, List<Mismatch>> matchesAll( public static Map<String, List<Mismatch>> matchesAll(
ClassLoader classLoader, boolean injectHelpers, Set<String> excludedInstrumentationNames) { ClassLoader classLoader, boolean injectHelpers, Set<String> excludedInstrumentationNames) {
Map<String, List<Mismatch>> result = new HashMap<>(); Map<String, List<Mismatch>> result = new HashMap<>();
ServiceLoader.load(InstrumentationModule.class) ServiceLoader.load(InstrumentationModule.class, ClassLoaderMatcher.class.getClassLoader())
.forEach( .forEach(
module -> { module -> {
if (module.instrumentationNames().stream() if (module.instrumentationNames().stream()

View File

@ -26,7 +26,7 @@ public final class ReferencesPrinter {
*/ */
public static void printMuzzleReferences() { public static void printMuzzleReferences() {
for (InstrumentationModule instrumentationModule : for (InstrumentationModule instrumentationModule :
ServiceLoader.load(InstrumentationModule.class)) { ServiceLoader.load(InstrumentationModule.class, ReferencesPrinter.class.getClassLoader())) {
try { try {
System.out.println(instrumentationModule.getClass().getName()); System.out.println(instrumentationModule.getClass().getName());
for (ClassRef ref : for (ClassRef ref :

View File

@ -10,6 +10,7 @@ import static java.util.logging.Level.SEVERE;
import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer; import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer;
import io.opentelemetry.javaagent.tooling.EmptyConfigProperties; import io.opentelemetry.javaagent.tooling.EmptyConfigProperties;
import io.opentelemetry.javaagent.tooling.SafeServiceLoader; import io.opentelemetry.javaagent.tooling.SafeServiceLoader;
import io.opentelemetry.javaagent.tooling.Utils;
import io.opentelemetry.javaagent.tooling.ignore.AdditionalLibraryIgnoredTypesConfigurer; import io.opentelemetry.javaagent.tooling.ignore.AdditionalLibraryIgnoredTypesConfigurer;
import io.opentelemetry.javaagent.tooling.ignore.GlobalIgnoredTypesConfigurer; import io.opentelemetry.javaagent.tooling.ignore.GlobalIgnoredTypesConfigurer;
import io.opentelemetry.javaagent.tooling.ignore.IgnoreAllow; import io.opentelemetry.javaagent.tooling.ignore.IgnoreAllow;
@ -51,7 +52,8 @@ public class TestAgentListener implements AgentBuilder.Listener {
private static Trie<IgnoreAllow> buildOtherConfiguredIgnores() { private static Trie<IgnoreAllow> buildOtherConfiguredIgnores() {
IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl(); IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl();
for (IgnoredTypesConfigurer configurer : for (IgnoredTypesConfigurer configurer :
SafeServiceLoader.loadOrdered(IgnoredTypesConfigurer.class)) { SafeServiceLoader.loadOrdered(
IgnoredTypesConfigurer.class, Utils.getExtensionsClassLoader())) {
// skip built-in agent ignores // skip built-in agent ignores
if (configurer instanceof AdditionalLibraryIgnoredTypesConfigurer if (configurer instanceof AdditionalLibraryIgnoredTypesConfigurer
|| configurer instanceof GlobalIgnoredTypesConfigurer) { || configurer instanceof GlobalIgnoredTypesConfigurer) {