Skip to content

Commit

Permalink
Add BundleWiring instrumentation to improve tracer behaviour on OSGi:
Browse files Browse the repository at this point in the history
* Rework our class-file location strategy to make it easier to mark agent requests
* Record the type of agent class-loader requests to help with OSGi instrumentation
* Widen class/resource lookup on OSGi for agent requests (and support lightweight probing)
* Enable checks for common OSGi issues in the OSGi smoke test
  • Loading branch information
mcculls committed Jan 25, 2021
1 parent 994804c commit 0bcc153
Show file tree
Hide file tree
Showing 13 changed files with 502 additions and 23 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package datadog.trace.bootstrap;

/**
* Helps distinguish agent class-loading requests from application requests.
*
* <p>Should be used in a short try..finally block around the class-loader call.
*/
public enum AgentClassLoading {
/** Lightweight class-loader probing to select instrumentations. */
PROBING_CLASSLOADER,
/** Locating class resources for Byte-Buddy. */
LOCATING_CLASS,
/** Injecting helper classes into class-loaders. */
INJECTING_HELPERS;

private static final ThreadLocal<AgentClassLoading> REQUEST = new ThreadLocal<>();

/**
* Gets the current agent class-loading request type; {@code null} if there's no agent request.
*/
public static AgentClassLoading type() {
return REQUEST.get();
}

/** Records that this agent class-loading request has begun. */
public final void begin() {
REQUEST.set(this);
}

/** Records that this agent class-loading request has ended. */
public final void end() {
REQUEST.remove();
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package datadog.trace.agent.tooling;

import static datadog.trace.bootstrap.AgentClassLoading.PROBING_CLASSLOADER;

import datadog.trace.api.Tracer;
import datadog.trace.bootstrap.PatchLogger;
import datadog.trace.bootstrap.WeakCache;
Expand Down Expand Up @@ -140,12 +142,17 @@ private WeakCache<ClassLoader, Boolean> getCache() {
}

private boolean hasResources(final ClassLoader cl) {
for (final String resource : resources) {
if (cl.getResource(resource) == null) {
return false;
PROBING_CLASSLOADER.begin();
try {
for (final String resource : resources) {
if (cl.getResource(resource) == null) {
return false;
}
}
return true;
} finally {
PROBING_CLASSLOADER.end();
}
return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.agent.tooling;

import static datadog.trace.agent.tooling.ClassLoaderMatcher.BOOTSTRAP_CLASSLOADER;
import static datadog.trace.bootstrap.AgentClassLoading.INJECTING_HELPERS;

import datadog.trace.api.Config;
import java.io.File;
Expand Down Expand Up @@ -165,21 +166,28 @@ private Map<String, Class<?>> injectBootstrapClassLoader(

// Failures to create a tempDir are propagated as IOException and handled by transform
final File tempDir = createTempDir();
INJECTING_HELPERS.begin();
try {
return ClassInjector.UsingInstrumentation.of(
tempDir,
ClassInjector.UsingInstrumentation.Target.BOOTSTRAP,
AgentInstaller.getInstrumentation())
.injectRaw(classnameToBytes);
} finally {
INJECTING_HELPERS.end();
// Delete fails silently
deleteTempDir(tempDir);
}
}

private Map<String, Class<?>> injectClassLoader(
final ClassLoader classLoader, final Map<String, byte[]> classnameToBytes) {
return new ClassInjector.UsingReflection(classLoader).injectRaw(classnameToBytes);
INJECTING_HELPERS.begin();
try {
return new ClassInjector.UsingReflection(classLoader).injectRaw(classnameToBytes);
} finally {
INJECTING_HELPERS.end();
}
}

private void ensureModuleCanReadHelperModules(final JavaModule target) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.agent.tooling.bytebuddy;

import static datadog.trace.bootstrap.AgentClassLoading.LOCATING_CLASS;
import static net.bytebuddy.agent.builder.AgentBuilder.PoolStrategy;

import com.googlecode.concurrentlinkedhashmap.ConcurrentLinkedHashMap;
Expand Down Expand Up @@ -295,6 +296,7 @@ public TypeDescription resolve() {
if (!isResolved) {
Class<?> klass = null;
ClassLoader classLoader = null;
LOCATING_CLASS.begin();
try {
// Please note that by doing a loadClass, the type we are resolving will bypass
// transformation since we are in the middle of a transformation. This should
Expand All @@ -310,6 +312,8 @@ public TypeDescription resolve() {
klass = Class.forName(className, false, null);
}
} catch (Throwable ignored) {
} finally {
LOCATING_CLASS.end();
}
if (klass != null) {
// We managed to load the class
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package datadog.trace.agent.tooling.bytebuddy;

import static datadog.trace.bootstrap.AgentClassLoading.LOCATING_CLASS;

import datadog.trace.agent.tooling.Utils;
import java.io.IOException;
import java.io.InputStream;
import java.lang.ref.WeakReference;
import net.bytebuddy.dynamic.ClassFileLocator;
import net.bytebuddy.utility.StreamDrainer;

/**
* Locate resources with the loading classloader. Because of a quirk with the way classes appended
* to the bootstrap classpath work, we first check our bootstrap proxy. If the loading classloader
* cannot find the desired resource, check up the classloader hierarchy until a resource is found.
*/
public final class DDClassFileLocator extends WeakReference<ClassLoader>
implements ClassFileLocator {

public DDClassFileLocator(final ClassLoader classLoader) {
super(classLoader);
}

@Override
public Resolution locate(final String className) throws IOException {
String resourceName = className.replace('.', '/') + ".class";

// try bootstrap first
Resolution resolution = loadClassResource(Utils.getBootstrapProxy(), resourceName);
ClassLoader cl = get();

// now go up the classloader hierarchy
if (null == resolution && null != cl) {
LOCATING_CLASS.begin();
try {
do {
resolution = loadClassResource(cl, resourceName);
cl = cl.getParent();
} while (null == resolution && null != cl);
} finally {
LOCATING_CLASS.end();
}
}

return resolution != null ? resolution : new Resolution.Illegal(className);
}

@Override
public void close() {
// nothing to close
}

private static Resolution loadClassResource(
final ClassLoader classLoader, final String resourceName) throws IOException {
try (InputStream in = classLoader.getResourceAsStream(resourceName)) {
if (null != in) {
return new Resolution.Explicit(StreamDrainer.DEFAULT.drain(in));
}
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,18 @@
package datadog.trace.agent.tooling.bytebuddy;

import datadog.trace.agent.tooling.Utils;
import java.util.ArrayList;
import java.util.List;
import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.dynamic.ClassFileLocator;
import net.bytebuddy.utility.JavaModule;

/**
* Locate resources with the loading classloader. Because of a quirk with the way classes appended
* to the bootstrap classpath work, we first check our bootstrap proxy. If the loading classloader
* cannot find the desired resource, check up the classloader hierarchy until a resource is found.
*/
public class DDLocationStrategy implements AgentBuilder.LocationStrategy {
/** Strategy that uses {@link DDClassFileLocator} to locate class files. */
public final class DDLocationStrategy implements AgentBuilder.LocationStrategy {
public ClassFileLocator classFileLocator(final ClassLoader classLoader) {
return classFileLocator(classLoader, null);
}

@Override
public ClassFileLocator classFileLocator(ClassLoader classLoader, final JavaModule javaModule) {
final List<ClassFileLocator> locators = new ArrayList<>();
locators.add(ClassFileLocator.ForClassLoader.of(Utils.getBootstrapProxy()));
while (classLoader != null) {
locators.add(ClassFileLocator.ForClassLoader.WeaklyReferenced.of(classLoader));
classLoader = classLoader.getParent();
}
return new ClassFileLocator.Compound(locators.toArray(new ClassFileLocator[0]));
public ClassFileLocator classFileLocator(
final ClassLoader classLoader, final JavaModule javaModule) {
return new DDClassFileLocator(classLoader);
}
}
21 changes: 21 additions & 0 deletions dd-java-agent/instrumentation/osgi-4.3/osgi-4.3.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
muzzle {
// old coordinates
pass {
group = 'org.osgi'
module = 'org.osgi.core'
versions = '[4.3,]'
}

// new coordinates
pass {
group = 'org.osgi'
module = 'osgi.core'
versions = '[4.3.1,]'
}
}

apply from: "$rootDir/gradle/java.gradle"

dependencies {
compileOnly group: 'org.osgi', name: 'org.osgi.core', version: '4.3.0'
}
Loading

0 comments on commit 0bcc153

Please sign in to comment.