Make HelperInjector's dependency on Instrumentation instance more visible (#3491)

* Make HelperInjector's dependency on Instrumentation instance more visible

* Polish

* Polish

* Fix docs
This commit is contained in:
Nikita Salnikov-Tarnovski 2021-07-06 07:44:26 +03:00 committed by GitHub
parent 3fc5d71771
commit 901bae57b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 131 additions and 67 deletions

View File

@ -16,7 +16,6 @@ dependencies {
compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-extension-api:${versions.opentelemetryJavaagentAlpha}")
compileOnly deps.bytebuddy
compileOnly deps.bytebuddyagent
annotationProcessor deps.autoservice
compileOnly deps.autoservice

View File

@ -6,6 +6,7 @@
package io.opentelemetry.javaagent;
import io.opentelemetry.javaagent.bootstrap.AgentInitializer;
import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder;
import java.io.File;
import java.io.IOException;
import java.lang.instrument.Instrumentation;
@ -46,6 +47,7 @@ public final class OpenTelemetryAgent {
public static void agentmain(String agentArgs, Instrumentation inst) {
try {
File javaagentFile = installBootstrapJar(inst);
InstrumentationHolder.setInstrumentation(inst);
AgentInitializer.initialize(inst, javaagentFile);
} catch (Throwable ex) {
// Don't rethrow. We don't have a log manager here, so just print.

View File

@ -0,0 +1,21 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.bootstrap;
import java.lang.instrument.Instrumentation;
/** This class serves as a "everywhere accessible" source of {@link Instrumentation} instance. */
public class InstrumentationHolder {
private static volatile Instrumentation instrumentation;
public static Instrumentation getInstrumentation() {
return instrumentation;
}
public static void setInstrumentation(Instrumentation instrumentation) {
InstrumentationHolder.instrumentation = instrumentation;
}
}

View File

@ -61,11 +61,6 @@ public class AgentInstaller {
"otel.javaagent.experimental.force-synchronous-agent-listeners";
private static final Map<String, List<Runnable>> CLASS_LOAD_CALLBACKS = new HashMap<>();
private static volatile Instrumentation instrumentation;
public static Instrumentation getInstrumentation() {
return instrumentation;
}
static {
LoggingConfigurer.configureLogger();
@ -122,8 +117,6 @@ public class AgentInstaller {
runBeforeAgentListeners(agentListeners, config);
instrumentation = inst;
FieldBackedProvider.resetContextMatchers();
AgentBuilder agentBuilder =

View File

@ -11,6 +11,7 @@ import io.opentelemetry.instrumentation.api.caching.Cache;
import io.opentelemetry.javaagent.bootstrap.HelperResources;
import java.io.File;
import java.io.IOException;
import java.lang.instrument.Instrumentation;
import java.lang.ref.WeakReference;
import java.net.URL;
import java.nio.file.Files;
@ -34,7 +35,18 @@ import org.checkerframework.checker.nullness.qual.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Injects instrumentation helper classes into the user's classloader. */
/**
* Injects instrumentation helper classes into the user's classloader.
*
* <p>Care must be taken when using this class. It is used both by the javaagent during its runtime
* and by gradle muzzle verification plugin during build time. And some code paths in this class
* require the usage of {@link Instrumentation}, which is available for the former, but not for the
* latter. Unfortunately, these two "modes of operations" and not easily discernible just by reading
* source code. Be careful.
*
* <p>In a nutshell, an instance of {@link Instrumentation} is needed for class injection into
* bootstrap classloader. This should NOT happen during build-time muzzle verification phase.
*/
public class HelperInjector implements Transformer {
private static final Logger log = LoggerFactory.getLogger(HelperInjector.class);
@ -56,6 +68,7 @@ public class HelperInjector implements Transformer {
private final Set<String> helperClassNames;
private final Set<String> helperResourceNames;
@Nullable private final ClassLoader helpersSource;
@Nullable private final Instrumentation instrumentation;
private final Map<String, byte[]> dynamicTypeMap = new LinkedHashMap<>();
private final Cache<ClassLoader, Boolean> injectedClassLoaders =
@ -78,15 +91,23 @@ public class HelperInjector implements Transformer {
String requestingName,
List<String> helperClassNames,
List<String> helperResourceNames,
ClassLoader helpersSource) {
ClassLoader helpersSource,
Instrumentation instrumentation) {
this.requestingName = requestingName;
this.helperClassNames = new LinkedHashSet<>(helperClassNames);
this.helperResourceNames = new LinkedHashSet<>(helperResourceNames);
this.helpersSource = helpersSource;
this.instrumentation = instrumentation;
}
/** Must be used ONLY by gradle muzzle verification plugin. */
public HelperInjector(String requestingName, Map<String, byte[]> helperMap) {
this(requestingName, helperMap, null);
}
private HelperInjector(
String requestingName, Map<String, byte[]> helperMap, Instrumentation instrumentation) {
this.requestingName = requestingName;
this.helperClassNames = helperMap.keySet();
@ -94,15 +115,18 @@ public class HelperInjector implements Transformer {
this.helperResourceNames = Collections.emptySet();
this.helpersSource = null;
this.instrumentation = instrumentation;
}
public static HelperInjector forDynamicTypes(
String requestingName, Collection<DynamicType.Unloaded<?>> helpers) {
String requestingName,
Collection<DynamicType.Unloaded<?>> helpers,
Instrumentation instrumentation) {
Map<String, byte[]> bytes = new HashMap<>(helpers.size());
for (DynamicType.Unloaded<?> helper : helpers) {
bytes.put(helper.getTypeDescription().getName(), helper.getBytes());
}
return new HelperInjector(requestingName, bytes);
return new HelperInjector(requestingName, bytes, instrumentation);
}
private Map<String, byte[]> getHelperMap() throws IOException {
@ -129,51 +153,10 @@ public class HelperInjector implements Transformer {
ClassLoader classLoader,
JavaModule module) {
if (!helperClassNames.isEmpty()) {
if (classLoader == BOOTSTRAP_CLASSLOADER) {
classLoader = BOOTSTRAP_CLASSLOADER_PLACEHOLDER;
}
injectedClassLoaders.computeIfAbsent(
classLoader,
cl -> {
try {
log.debug("Injecting classes onto classloader {} -> {}", cl, helperClassNames);
Map<String, byte[]> classnameToBytes = getHelperMap();
Map<String, Class<?>> classes;
if (cl == BOOTSTRAP_CLASSLOADER_PLACEHOLDER) {
classes = injectBootstrapClassLoader(classnameToBytes);
} else {
classes = injectClassLoader(cl, classnameToBytes);
}
classes.values().forEach(c -> injectedClasses.put(c, Boolean.TRUE));
// All agent helper classes are in the unnamed module
// And there's exactly one unnamed module per classloader
// Use the module of the first class for convenience
if (JavaModule.isSupported()) {
JavaModule javaModule = JavaModule.ofType(classes.values().iterator().next());
helperModules.add(new WeakReference<>(javaModule.unwrap()));
}
} catch (Exception e) {
if (log.isErrorEnabled()) {
log.error(
"Error preparing helpers while processing {} for {}. Failed to inject helper classes into instance {}",
typeDescription,
requestingName,
cl,
e);
}
throw new IllegalStateException(e);
}
return true;
});
ensureModuleCanReadHelperModules(module);
classLoader = injectHelperClasses(typeDescription, classLoader, module);
}
if (!helperResourceNames.isEmpty()) {
if (helpersSource != null && !helperResourceNames.isEmpty()) {
for (String resourceName : helperResourceNames) {
URL resource = helpersSource.getResource(resourceName);
if (resource == null) {
@ -189,8 +172,60 @@ public class HelperInjector implements Transformer {
return builder;
}
private static Map<String, Class<?>> injectBootstrapClassLoader(
Map<String, byte[]> classnameToBytes) throws IOException {
private ClassLoader injectHelperClasses(
TypeDescription typeDescription, ClassLoader classLoader, JavaModule module) {
if (classLoader == BOOTSTRAP_CLASSLOADER) {
classLoader = BOOTSTRAP_CLASSLOADER_PLACEHOLDER;
}
if (classLoader == BOOTSTRAP_CLASSLOADER_PLACEHOLDER && instrumentation == null) {
log.error(
"Cannot inject helpers into bootstrap classloader without an instance of Instrumentation. Programmer error!");
return classLoader;
}
injectedClassLoaders.computeIfAbsent(
classLoader,
cl -> {
try {
log.debug("Injecting classes onto classloader {} -> {}", cl, helperClassNames);
Map<String, byte[]> classnameToBytes = getHelperMap();
Map<String, Class<?>> classes;
if (cl == BOOTSTRAP_CLASSLOADER_PLACEHOLDER) {
classes = injectBootstrapClassLoader(classnameToBytes);
} else {
classes = injectClassLoader(cl, classnameToBytes);
}
classes.values().forEach(c -> injectedClasses.put(c, Boolean.TRUE));
// All agent helper classes are in the unnamed module
// And there's exactly one unnamed module per classloader
// Use the module of the first class for convenience
if (JavaModule.isSupported()) {
JavaModule javaModule = JavaModule.ofType(classes.values().iterator().next());
helperModules.add(new WeakReference<>(javaModule.unwrap()));
}
} catch (Exception e) {
if (log.isErrorEnabled()) {
log.error(
"Error preparing helpers while processing {} for {}. Failed to inject helper classes into instance {}",
typeDescription,
requestingName,
cl,
e);
}
throw new IllegalStateException(e);
}
return true;
});
ensureModuleCanReadHelperModules(module);
return classLoader;
}
private Map<String, Class<?>> injectBootstrapClassLoader(Map<String, byte[]> classnameToBytes)
throws IOException {
// Mar 2020: Since we're proactively cleaning up tempDirs, we cannot share dirs per thread.
// If this proves expensive, we could do a per-process tempDir with
// a reference count -- but for now, starting simple.
@ -199,9 +234,7 @@ public class HelperInjector implements Transformer {
File tempDir = createTempDir();
try {
return ClassInjector.UsingInstrumentation.of(
tempDir,
ClassInjector.UsingInstrumentation.Target.BOOTSTRAP,
AgentInstaller.getInstrumentation())
tempDir, ClassInjector.UsingInstrumentation.Target.BOOTSTRAP, instrumentation)
.injectRaw(classnameToBytes);
} finally {
// Delete fails silently
@ -226,7 +259,8 @@ public class HelperInjector implements Transformer {
if (!target.canRead(helperModule)) {
log.debug("Adding module read from {} to {}", target, helperModule);
ClassInjector.UsingInstrumentation.redefineModule(
AgentInstaller.getInstrumentation(),
// TODO can we guarantee that this is always present?
instrumentation,
target,
Collections.singleton(helperModule),
Collections.emptyMap(),

View File

@ -14,12 +14,14 @@ import static net.bytebuddy.matcher.ElementMatchers.not;
import io.opentelemetry.instrumentation.api.caching.Cache;
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.javaagent.bootstrap.FieldBackedContextStoreAppliedMarker;
import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder;
import io.opentelemetry.javaagent.instrumentation.api.ContextStore;
import io.opentelemetry.javaagent.instrumentation.api.InstrumentationContext;
import io.opentelemetry.javaagent.tooling.HelperInjector;
import io.opentelemetry.javaagent.tooling.TransformSafeLogger;
import io.opentelemetry.javaagent.tooling.Utils;
import io.opentelemetry.javaagent.tooling.instrumentation.InstrumentationModuleInstaller;
import java.lang.instrument.Instrumentation;
import java.lang.reflect.Method;
import java.security.ProtectionDomain;
import java.util.Arrays;
@ -71,7 +73,7 @@ import net.bytebuddy.utility.JavaModule;
* <p>Example:<br>
* <em>InstrumentationContext.get(Runnable.class, RunnableState.class)")</em><br>
* is rewritten to:<br>
* <em>FieldBackedProvider$ContextStore$Runnable$RunnableState12345.getContextStore(runnableRunnable.class,
* <em>FieldBackedProvider$ContextStore$Runnable$RunnableState12345.getContextStore(Runnable.class,
* RunnableState.class)</em>
*/
public class FieldBackedProvider implements InstrumentationContextProvider {
@ -122,9 +124,14 @@ public class FieldBackedProvider implements InstrumentationContextProvider {
private final AgentBuilder.Transformer contextStoreImplementationsInjector;
private final Instrumentation instrumentation;
public FieldBackedProvider(Class<?> instrumenterClass, Map<String, String> contextStore) {
this.instrumenterClass = instrumenterClass;
this.contextStore = contextStore;
// This class is used only when running with javaagent, thus this calls is safe
this.instrumentation = InstrumentationHolder.getInstrumentation();
byteBuddy = new ByteBuddy();
fieldAccessorInterfaces = generateFieldAccessorInterfaces();
fieldAccessorInterfacesInjector = bootstrapHelperInjector(fieldAccessorInterfaces.values());
@ -316,7 +323,7 @@ public class FieldBackedProvider implements InstrumentationContextProvider {
// TODO: Better to pass through the context of the Instrumenter
return new AgentBuilder.Transformer() {
final HelperInjector injector =
HelperInjector.forDynamicTypes(getClass().getSimpleName(), helpers);
HelperInjector.forDynamicTypes(getClass().getSimpleName(), helpers, instrumentation);
@Override
public DynamicType.Builder<?> transform(

View File

@ -8,6 +8,7 @@ package io.opentelemetry.javaagent.tooling.instrumentation;
import static io.opentelemetry.javaagent.tooling.SafeServiceLoader.loadOrdered;
import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder;
import io.opentelemetry.javaagent.extension.AgentExtension;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import net.bytebuddy.agent.builder.AgentBuilder;
@ -19,7 +20,7 @@ public class InstrumentationLoader implements AgentExtension {
private static final Logger log = LoggerFactory.getLogger(InstrumentationLoader.class);
private final InstrumentationModuleInstaller instrumentationModuleInstaller =
new InstrumentationModuleInstaller();
new InstrumentationModuleInstaller(InstrumentationHolder.getInstrumentation());
@Override
public AgentBuilder extend(AgentBuilder agentBuilder) {

View File

@ -20,6 +20,7 @@ import io.opentelemetry.javaagent.tooling.context.InstrumentationContextProvider
import io.opentelemetry.javaagent.tooling.context.NoopContextProvider;
import io.opentelemetry.javaagent.tooling.muzzle.matcher.Mismatch;
import io.opentelemetry.javaagent.tooling.muzzle.matcher.ReferenceMatcher;
import java.lang.instrument.Instrumentation;
import java.security.ProtectionDomain;
import java.util.List;
import java.util.Map;
@ -36,12 +37,17 @@ public final class InstrumentationModuleInstaller {
private static final TransformSafeLogger log =
TransformSafeLogger.getLogger(InstrumentationModule.class);
private static final Logger muzzleLog = LoggerFactory.getLogger("muzzleMatcher");
private final Instrumentation instrumentation;
// Added here instead of AgentInstaller's ignores because it's relatively
// expensive. https://github.com/DataDog/dd-trace-java/pull/1045
public static final ElementMatcher.Junction<AnnotationSource> NOT_DECORATOR_MATCHER =
not(isAnnotatedWith(named("javax.decorator.Decorator")));
public InstrumentationModuleInstaller(Instrumentation instrumentation) {
this.instrumentation = instrumentation;
}
AgentBuilder install(
InstrumentationModule instrumentationModule, AgentBuilder parentAgentBuilder) {
if (!instrumentationModule.isEnabled()) {
@ -69,7 +75,8 @@ public final class InstrumentationModuleInstaller {
instrumentationModule.instrumentationName(),
helperClassNames,
helperResourceNames,
Utils.getExtensionsClassLoader());
Utils.getExtensionsClassLoader(),
instrumentation);
InstrumentationContextProvider contextProvider =
createInstrumentationContextProvider(instrumentationModule);

View File

@ -29,7 +29,7 @@ class HelperInjectionTest extends Specification {
ClassLoader helpersSourceLoader = new URLClassLoader(helpersSourceUrls)
String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass'
HelperInjector injector = new HelperInjector("test", [helperClassName], [], helpersSourceLoader)
HelperInjector injector = new HelperInjector("test", [helperClassName], [], helpersSourceLoader, null)
AtomicReference<URLClassLoader> emptyLoader = new AtomicReference<>(new URLClassLoader(new URL[0], (ClassLoader) null))
when:
@ -61,7 +61,7 @@ class HelperInjectionTest extends Specification {
ByteBuddyAgent.install()
AgentInstaller.installBytebuddyAgent(ByteBuddyAgent.getInstrumentation())
String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass'
HelperInjector injector = new HelperInjector("test", [helperClassName], [], this.class.classLoader)
HelperInjector injector = new HelperInjector("test", [helperClassName], [], this.class.classLoader, ByteBuddyAgent.getInstrumentation())
URLClassLoader bootstrapChild = new URLClassLoader(new URL[0], (ClassLoader) null)
when: