Skip to content
Open
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
5 changes: 5 additions & 0 deletions dd-java-agent/agent-bootstrap/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,17 @@ plugins {
id 'idea'
}

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

// The shadowJar of this project will be injected into the JVM's bootstrap classloader

tasks.named("compileJava", JavaCompile) {
configureCompiler(it, 8, JavaVersion.VERSION_1_8, "Need access to sun.* packages")
dependsOn 'generateClassNameTries'
}

tasks.named("sourcesJar") { dependsOn 'generateClassNameTries' }

// FIXME: Improve test coverage.
minimumBranchCoverage = 0.0
minimumInstructionCoverage = 0.0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package datadog.trace.bootstrap.instrumentation.java.concurrent;

import datadog.trace.bootstrap.FieldBackedContextAccessor;
import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType;

/**
* This is used to wrap lambda runnables so we can apply field-injection. RunnableWrapper can be
* transformed to add the necessary context-store fields, while lambdas currently cannot until the
* issue reported in https://github.com/raphw/byte-buddy/issues/558 is addressed.
* Wraps anonymous Runnable classes that were not field-injected.
*
* <p>We also make this class final to stop instrumentations from extending it in their injected
* helper classes, because if this class is loaded during helper injection then we can miss the
Expand All @@ -25,9 +24,11 @@ public void run() {
}

public static Runnable wrapIfNeeded(final Runnable task) {
if (!(task instanceof RunnableWrapper) && !ExcludeFilter.exclude(ExcludeType.RUNNABLE, task)) {
// We wrap only lambdas' anonymous classes and if given object has not already been wrapped.
// Anonymous classes have '/' in class name which is not allowed in 'normal' classes.
// Field-injected tasks are already instrumented and must retain their identity.
if (!(task instanceof RunnableWrapper)
&& !(task instanceof FieldBackedContextAccessor)
&& !ExcludeFilter.exclude(ExcludeType.RUNNABLE, task)) {
Comment on lines +28 to +30

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Retain per-submission context for reused lambdas

When the same lambda instance is submitted more than once before its first execution consumes the stored continuation—for example, a static or non-capturing Runnable queued concurrently by two requests—skipping the wrapper makes every submission share one State slot. The second capture is rejected, and overlapping executions can either lose a parent or both resume the first submission's continuation, producing missing or incorrectly parented traces; this path needs per-submission state rather than opting every field-backed lambda out of wrapping.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I explcitly already put in the PR description for reviewers:

Previously, wrapped lambdas had per-submission state. This limitation is most relevant to reusable singleton or stateless lambdas submitted concurrently; lambdas created per operation are unlikely to encounter it.

This is a known limitation and already existing for all the other field injected classes that are going throught threadpool. It's not introduced by this change and can be considered a trade-off we can perhaps live together for now.

// Hidden lambda class names contain '/'.
final String className = task.getClass().getName();
if (className.indexOf('/', className.lastIndexOf('.')) > 0) {
return new RunnableWrapper(task);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import datadog.trace.api.InstrumenterConfig;
import datadog.trace.api.Platform;
import datadog.trace.bootstrap.ContextStore;
import datadog.trace.bootstrap.FieldBackedContextAccessor;
import java.util.Set;
import java.util.concurrent.ThreadPoolExecutor;

Expand All @@ -22,18 +23,19 @@ public final class TPEHelper {
// If legacy is enabled, we will try to propagate via wrapping, if not we will try to propagate
// via storing the state in the existing field in the Runnable
private static final boolean useWrapping;
// A ThreadPoolExecutor with one of these types will newer be propagated/wrapped
// A ThreadPoolExecutor with one of these types will never be propagated/wrapped
private static final Set<String> excludedClasses;
// A ThreadLocal to store the Scope between beforeExecute and afterExecute if wrapping is not used
private static final ThreadLocal<ContextScope> threadLocalScope;

private static final ClassValue<Boolean> WRAP =
GenericClassValue.of(
input -> {
if (FieldBackedContextAccessor.class.isAssignableFrom(input)) {
return false;
}
String className = input.getName();
// We should always wrap anonymous lambda classes since we can't inject fields into
// them, and they can never be anything more than a _pure_ Runnable. They have '/' in
// their class name which is not allowed in 'normal' classes.
// Wrap anonymous lambda classes that were not field-injected.
return className.indexOf('/', className.lastIndexOf('.')) > 0;
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package datadog.trace.bootstrap.instrumentation.java.lang.invoke;

/** Transforms a generated lambda class before it is defined. */
public interface LambdaTransformer {
/**
* @param slashClassName internal (slash-separated) name of the generated lambda class
* @param targetClass the class declaring the lambda
* @param classBytes the freshly generated lambda class bytes
* @return the transformed bytes, or {@code null}/the original bytes if unchanged
*/
byte[] transform(String slashClassName, Class<?> targetClass, byte[] classBytes);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package datadog.trace.bootstrap.instrumentation.java.lang.invoke;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** Transforms eligible lambda bytes before definition, falling back to the original on failure. */
public final class LambdaTransformerHelper {
private static final Logger log = LoggerFactory.getLogger(LambdaTransformerHelper.class);

// Agent transformation may itself create lambdas.
private static final ThreadLocal<Boolean> TRANSFORMING = new ThreadLocal<>();

private LambdaTransformerHelper() {}

/**
* @param classBytes the generated lambda class bytes
* @param lambdaClassName internal (slash-separated) name of the generated lambda class
* @param targetClass the class declaring the lambda
* @param interfaceClass the functional interface implemented by the lambda
* @return possibly transformed bytes; the original bytes on any failure
*/
public static byte[] transform(
byte[] classBytes, String lambdaClassName, Class<?> targetClass, Class<?> interfaceClass) {
try {
// Only exact allowlisted interfaces enter the transformer.
if (interfaceClass == null || LambdaInterfaceNameTrie.apply(interfaceClass.getName()) != 1) {
return classBytes;
}
LambdaTransformer transformer = LambdaTransformerHolder.get();
if (transformer == null) {
log.debug("Lambda {} skipped: no transformer registered", lambdaClassName);
return classBytes;
}
if (targetClass == null) {
log.debug("Lambda {} skipped: no target class", lambdaClassName);
return classBytes;
}
// Skip lambdas declared by the agent itself to avoid self-instrumentation and recursion.
String targetName = targetClass.getName();
if (targetName.startsWith("datadog.") || targetName.startsWith("net.bytebuddy.")) {
log.debug("Lambda {} skipped: declared by the agent", lambdaClassName);
return classBytes;
}
if (Boolean.TRUE.equals(TRANSFORMING.get())) {
log.debug("Lambda {} skipped: re-entrant transform", lambdaClassName);
return classBytes;
}
TRANSFORMING.set(Boolean.TRUE);
try {
byte[] result = transformer.transform(lambdaClassName, targetClass, classBytes);
if (result == null) {
log.debug("Lambda {} not transformed", lambdaClassName);
return classBytes;
}
return result;
} finally {
TRANSFORMING.set(Boolean.FALSE);
}
} catch (Throwable e) {
log.debug("Lambda {} skipped: {}", lambdaClassName, e.toString());
return classBytes;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package datadog.trace.bootstrap.instrumentation.java.lang.invoke;

/**
* Holds the {@link LambdaTransformer} registered by the agent installer. Lives on the bootstrap
* class path so it is reachable from instrumented {@code java.lang.invoke} code.
*/
public final class LambdaTransformerHolder {
private static volatile LambdaTransformer transformer;

private LambdaTransformerHolder() {}

public static void set(LambdaTransformer transformer) {
LambdaTransformerHolder.transformer = transformer;
}

public static LambdaTransformer get() {
return transformer;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Generates 'LambdaInterfaceNameTrie.java'

# Exact functional interfaces whose generated lambda classes should be sent through the agent's
# matching and transformation pipeline. Keep this list narrow: the lookup runs for every lambda
# linkage in the application.

1 java.lang.Runnable
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static datadog.trace.agent.tooling.bytebuddy.matcher.GlobalIgnoresMatcher.globalIgnoresMatcher;
import static net.bytebuddy.matcher.ElementMatchers.isDefaultFinalizer;

import datadog.environment.JavaVirtualMachine;
import datadog.environment.SystemProperties;
import datadog.trace.agent.tooling.bytebuddy.SharedTypePools;
import datadog.trace.agent.tooling.bytebuddy.iast.TaintableRedefinitionStrategyListener;
Expand All @@ -19,6 +20,9 @@
import datadog.trace.api.telemetry.IntegrationsCollector;
import datadog.trace.bootstrap.FieldBackedContextAccessor;
import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
import datadog.trace.bootstrap.instrumentation.java.lang.invoke.LambdaTransformer;
import datadog.trace.bootstrap.instrumentation.java.lang.invoke.LambdaTransformerHelper;
import datadog.trace.bootstrap.instrumentation.java.lang.invoke.LambdaTransformerHolder;
import datadog.trace.bootstrap.instrumentation.java.module.JpmsHelper;
import datadog.trace.util.AgentTaskScheduler;
import de.thetaphi.forbiddenapis.SuppressForbidden;
Expand All @@ -35,6 +39,7 @@
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;
import java.util.function.BooleanSupplier;
import java.util.function.Function;
import net.bytebuddy.ByteBuddy;
import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.description.type.TypeDescription;
Expand Down Expand Up @@ -147,7 +152,7 @@ public static ClassFileTransformer installBytebuddyAgent(
agentBuilder =
agentBuilder
.disableClassFormatChanges()
.assureReadEdgeTo(inst, FieldBackedContextAccessor.class)
.assureReadEdgeTo(inst, FieldBackedContextAccessor.class, LambdaTransformerHelper.class)
.with(AgentStrategies.transformerDecorator())
.with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
.with(AgentStrategies.rediscoveryStrategy())
Expand Down Expand Up @@ -253,12 +258,65 @@ public void applied(Iterable<String> instrumentationNames) {

InstrumenterState.resetDefaultState();
try {
return transformerBuilder.installOn(inst);
ClassFileTransformer classFileTransformer = transformerBuilder.installOn(inst);
registerLambdaTransformer(classFileTransformer);
return classFileTransformer;
} finally {
SharedTypePools.endInstall();
}
}

/** Registers the installed class-file transformer for generated lambdas. */
private static void registerLambdaTransformer(final ClassFileTransformer classFileTransformer) {
LambdaTransformer lambdaTransformer = newLambdaTransformer(classFileTransformer);
if (null != lambdaTransformer) {
LambdaTransformerHolder.set(lambdaTransformer);
}
}

/**
* Java 9+ requires the module-aware transformer for injected read edges. Failure must disable
* lambda transformation rather than fall back to the module-less overload.
*/
@SuppressWarnings("unchecked")
private static LambdaTransformer newLambdaTransformer(
final ClassFileTransformer classFileTransformer) {
if (JavaVirtualMachine.isJavaVersionAtLeast(9)) {
try {
Function<ClassFileTransformer, LambdaTransformer> factory =
(Function<ClassFileTransformer, LambdaTransformer>)
Instrumenter.class
.getClassLoader()
.loadClass("datadog.trace.agent.tooling.bytebuddy.DDJava9LambdaTransformer")
.getField("FACTORY")
.get(null);
return factory.apply(classFileTransformer);
} catch (Throwable e) {
log.debug("Problem loading Java 9 lambda transformer, disabling lambda field-injection", e);
return null;
}
}
// Avoid invoking the instrumented metafactory while installing its transformer.
return new LambdaTransformer() {
@Override
public byte[] transform(String slashClassName, Class<?> targetClass, byte[] classBytes) {
TypePoolFacade.beginLambdaTransform();
try {
return classFileTransformer.transform(
targetClass.getClassLoader(),
slashClassName,
null,
targetClass.getProtectionDomain(),
classBytes);
} catch (Throwable ignored) {
return null;
} finally {
TypePoolFacade.endLambdaTransform();
}
}
};
}

/** Returns an iterable that combines the original sequence with any discovered extensions. */
private static Iterable<InstrumenterModule> withExtensions(Iterable<InstrumenterModule> initial) {
String extensionsPath = InstrumenterConfig.get().getTraceExtensionsPath();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package datadog.trace.agent.tooling.bytebuddy;

import datadog.trace.agent.tooling.bytebuddy.outline.TypePoolFacade;
import datadog.trace.bootstrap.instrumentation.java.lang.invoke.LambdaTransformer;
import java.lang.instrument.ClassFileTransformer;
import java.util.function.Function;

/** Routes generated lambdas through the module-aware Java 9+ transformer overload. */
public final class DDJava9LambdaTransformer implements LambdaTransformer {

/** Read reflectively by the agent installer, which cannot name {@link Module} itself. */
public static final Function<ClassFileTransformer, LambdaTransformer> FACTORY =
new Function<ClassFileTransformer, LambdaTransformer>() {
@Override
public LambdaTransformer apply(ClassFileTransformer classFileTransformer) {
return new DDJava9LambdaTransformer(classFileTransformer);
}
};

private final ClassFileTransformer classFileTransformer;

public DDJava9LambdaTransformer(ClassFileTransformer classFileTransformer) {
this.classFileTransformer = classFileTransformer;
}

@Override
public byte[] transform(String slashClassName, Class<?> targetClass, byte[] classBytes) {
TypePoolFacade.beginLambdaTransform();
try {
return classFileTransformer.transform(
targetClass.getModule(),
targetClass.getClassLoader(),
slashClassName,
null,
targetClass.getProtectionDomain(),
classBytes);
} catch (Throwable ignored) {
return null;
} finally {
TypePoolFacade.endLambdaTransform();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ static final class MemoizingMatcher
@Override
protected boolean doMatch(TypeDescription target) {
String targetName = target.getName();
// Same-owner hidden lambdas share a symbolic name. Bypass these caches before supporting
// lambda interfaces with different matcher results.
if (noMatchFilter.contains(targetName)
|| "java.lang.Object".equals(targetName)
|| target.isPrimitive()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ final class TypeFactory {

boolean createOutlines = OUTLINING_ENABLED;

boolean transformingLambda;

ClassLoader originalClassLoader;

ClassLoader currentClassLoader;
Expand Down Expand Up @@ -159,6 +161,14 @@ void beginTransform(String name, byte[] bytecode) {
}
}

void beginLambdaTransform() {
transformingLambda = true;
}

void endLambdaTransform() {
transformingLambda = false;
}

/** Once matching is complete we need full descriptions for the actual transformation. */
void enableFullDescriptions() {
createOutlines = false;
Expand Down Expand Up @@ -258,8 +268,9 @@ private TypeDescription lookupType(
boolean isOutline = typeParser == outlineTypeParser;
long fromTick = InstrumenterMetrics.tick();

// existing type description from same classloader?
SharedTypeInfo<TypeDescription> sharedType = types.find(name);
// Same-owner lambdas share a symbolic name, so build their target from the supplied bytes.
SharedTypeInfo<TypeDescription> sharedType =
transformingLambda && name.equals(targetName) ? null : types.find(name);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Do not cache hidden lambdas by symbolic name

The agent can apply wrong instrumentation to the normal class.

Assertion details
  • Input: On JDK 21 or later, transform a hidden Runnable lambda for Foo, then define Foo.$Lambda in the same class loader.
  • Expected: The transformer must parse the normal class from its own bytecode.
  • Actual: The normal transformation reuses the hidden lambda type description from the shared name cache.

Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

if (null != sharedType
&& (name.startsWith("java.") || sharedType.sameClassLoader(classLoaderId))) {
InstrumenterMetrics.reuseTypeDescription(fromTick, isOutline);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ public static void beginTransform(String name, byte[] bytecode) {
typeFactory.get().beginTransform(name, bytecode);
}

public static void beginLambdaTransform() {
typeFactory.get().beginLambdaTransform();
}

public static void endLambdaTransform() {
typeFactory.get().endLambdaTransform();
}

/** Switch to full descriptions, needed for the actual class transformation. */
public static void enableFullDescriptions() {
typeFactory.get().enableFullDescriptions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
0 java.lang.Runtime
# allow context tracking for VirtualThread
0 java.lang.VirtualThread
# allow instrumenting the lambda metafactory to field-inject generated lambda classes
0 java.lang.invoke.InnerClassLambdaMetafactory
0 java.net.http.*
0 java.net.HttpURLConnection
0 java.net.InetAddress
Expand Down
Loading
Loading