Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BundleWiring instrumentation to improve tracer behaviour on OSGi #2321

Merged
merged 1 commit into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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