Skip to content

Commit e02c852

Browse files
mccullsmcculls
authored
Configure byte-buddy to use simpler method-graph (DataDog#6579)
1 parent 315ea6f commit e02c852

7 files changed

Lines changed: 74 additions & 11 deletions

File tree

‎dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232
import net.bytebuddy.description.type.TypeDefinition;
3333
import net.bytebuddy.description.type.TypeDescription;
3434
import net.bytebuddy.dynamic.DynamicType;
35+
import net.bytebuddy.dynamic.VisibilityBridgeStrategy;
36+
import net.bytebuddy.dynamic.scaffold.InstrumentedType;
37+
import net.bytebuddy.dynamic.scaffold.MethodGraph;
3538
import net.bytebuddy.matcher.LatentMatcher;
3639
import net.bytebuddy.utility.JavaModule;
3740
import org.slf4j.Logger;
@@ -114,8 +117,25 @@ public static ClassFileTransformer installBytebuddyAgent(
114117
// but we need to instrument some synthetic methods in Scala, so change the ignore matcher
115118
ByteBuddy byteBuddy =
116119
new ByteBuddy().ignore(new LatentMatcher.Resolved<>(isDefaultFinalizer()));
117-
AgentBuilder agentBuilder =
118-
new AgentBuilder.Default(byteBuddy)
120+
121+
boolean simpleMethodGraph = InstrumenterConfig.get().isResolverSimpleMethodGraph();
122+
if (simpleMethodGraph) {
123+
// faster compiler that just considers visibility of locally declared methods
124+
byteBuddy =
125+
byteBuddy
126+
.with(MethodGraph.Compiler.ForDeclaredMethods.INSTANCE)
127+
.with(VisibilityBridgeStrategy.Default.NEVER)
128+
.with(InstrumentedType.Factory.Default.FROZEN);
129+
}
130+
131+
AgentBuilder agentBuilder = new AgentBuilder.Default(byteBuddy);
132+
if (simpleMethodGraph) {
133+
// faster strategy that assumes transformations use @Advice or AsmVisitorWrapper
134+
agentBuilder = agentBuilder.with(AgentBuilder.TypeStrategy.Default.DECORATE);
135+
}
136+
137+
agentBuilder =
138+
agentBuilder
119139
.disableClassFormatChanges()
120140
.assureReadEdgeTo(inst, FieldBackedContextAccessor.class)
121141
.with(AgentStrategies.transformerDecorator())

‎dd-java-agent/testing/src/test/groovy/locator/ClassInjectingForkedTest.groovy‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ package locator
22

33
import datadog.trace.agent.test.AgentTestRunner
44
import net.bytebuddy.agent.builder.AgentBuilder
5+
import net.bytebuddy.description.type.TypeDescription
6+
import net.bytebuddy.dynamic.DynamicType
7+
import net.bytebuddy.utility.JavaModule
8+
import spock.lang.Shared
59

610
import java.lang.instrument.ClassFileTransformer
711

@@ -33,11 +37,20 @@ class ClassInjectingForkedTest extends AgentTestRunner {
3337
super.cleanupAfterAgent()
3438
}
3539

40+
@Override
41+
void onTransformation(TypeDescription typeDescription, ClassLoader classLoader, JavaModule module, boolean loaded, DynamicType dynamicType) {
42+
transformed += typeDescription.name
43+
}
44+
45+
@Shared
46+
def transformed = []
47+
3648
def "should find classes injected via defineClass"() {
3749
setup:
3850
def instrumented = new ClassInjectingTestInstrumentation.ToBeInstrumented("test")
3951

4052
expect:
53+
transformed.contains('locator.ClassInjectingTestInstrumentation$ToBeInstrumented')
4154
instrumented.message == "test:instrumented:${ClassInjectingTransformer.NAME}"
4255
}
4356
}

‎dd-java-agent/testing/src/test/groovy/locator/ClassInjectingLoadClassDisabledForkedTest.groovy‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package locator
22

33
import datadog.trace.agent.test.AgentTestRunner
44
import net.bytebuddy.agent.builder.AgentBuilder
5+
import net.bytebuddy.description.type.TypeDescription
6+
import net.bytebuddy.dynamic.DynamicType
57
import net.bytebuddy.utility.JavaModule
68
import spock.lang.Shared
79

@@ -41,23 +43,19 @@ class ClassInjectingLoadClassDisabledForkedTest extends AgentTestRunner {
4143
}
4244

4345
@Override
44-
void onError(String typeName, ClassLoader classLoader, JavaModule module, boolean loaded, Throwable throwable) {
45-
if (typeName == "${ClassInjectingTestInstrumentation.name}\$ToBeInstrumented") {
46-
instrumentationFailure = true
47-
} else {
48-
super.onError(typeName, classLoader, module, loaded, throwable)
49-
}
46+
void onTransformation(TypeDescription typeDescription, ClassLoader classLoader, JavaModule module, boolean loaded, DynamicType dynamicType) {
47+
transformed += typeDescription.name
5048
}
5149

5250
@Shared
53-
volatile boolean instrumentationFailure = false
51+
def transformed = []
5452

5553
def "should not find classes injected via defineClass"() {
5654
setup:
5755
def instrumented = new ClassInjectingTestInstrumentation.ToBeInstrumented("test")
5856

5957
expect:
60-
instrumentationFailure
58+
!transformed.contains('locator.ClassInjectingTestInstrumentation$ToBeInstrumented')
6159
instrumented.message == "test:${ClassInjectingTransformer.NAME}"
6260
}
6361
}

‎dd-java-agent/testing/src/test/java/locator/ClassInjectingTestInstrumentation.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,34 @@
11
package locator;
22

3+
import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.declaresAnnotation;
4+
import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.hasInterface;
5+
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
36
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
47

58
import com.google.auto.service.AutoService;
69
import datadog.trace.agent.test.base.TestInstrumentation;
710
import datadog.trace.agent.tooling.Instrumenter;
11+
import java.lang.annotation.Retention;
12+
import java.lang.annotation.RetentionPolicy;
813
import net.bytebuddy.asm.Advice;
14+
import net.bytebuddy.description.type.TypeDescription;
15+
import net.bytebuddy.matcher.ElementMatcher;
916

1017
@AutoService(Instrumenter.class)
11-
public class ClassInjectingTestInstrumentation extends TestInstrumentation {
18+
public class ClassInjectingTestInstrumentation extends TestInstrumentation
19+
implements Instrumenter.WithTypeStructure {
20+
1221
@Override
1322
public String instrumentedType() {
1423
return getClass().getName() + "$ToBeInstrumented";
1524
}
1625

26+
@Override
27+
public ElementMatcher<TypeDescription> structureMatcher() {
28+
// additional constraint which requires loading the InjectedInterface to match
29+
return hasInterface(declaresAnnotation(named(getClass().getName() + "$ToBeMatched")));
30+
}
31+
1732
@Override
1833
public void methodAdvice(MethodTransformer transformer) {
1934
transformer.applyAdvice(isConstructor(), getClass().getName() + "$ConstructorAdvice");
@@ -27,6 +42,9 @@ public static void appendToMessage(
2742
}
2843
}
2944

45+
@Retention(RetentionPolicy.RUNTIME)
46+
public @interface ToBeMatched {}
47+
3048
public static final class ToBeInstrumented {
3149
private final String message;
3250

‎dd-java-agent/testing/src/test/java/locator/ClassInjectingTransformer.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ public static void injectInterfaceNamed(String binaryName, ClassLoader classLoad
5959
null,
6060
"java/lang/Object",
6161
null);
62+
String markerName = ClassInjectingTestInstrumentation.class.getName() + "$ToBeMatched";
63+
cw.visitAnnotation("L" + markerName.replace(".", "/") + ";", true).visitEnd();
6264
byte[] bytes = cw.toByteArray();
6365
defineMethod.invoke(classLoader, binaryName.replace("/", "."), bytes, 0, bytes.length, null);
6466
} catch (Throwable e) {

‎dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ public final class TraceInstrumentationConfig {
120120

121121
public static final String RESOLVER_CACHE_CONFIG = "resolver.cache.config";
122122
public static final String RESOLVER_CACHE_DIR = "resolver.cache.dir";
123+
public static final String RESOLVER_SIMPLE_METHOD_GRAPH = "resolver.simple.method.graph";
123124
public static final String RESOLVER_USE_LOADCLASS = "resolver.use.loadclass";
124125
public static final String RESOLVER_USE_URL_CACHES = "resolver.use.url.caches";
125126
public static final String RESOLVER_RESET_INTERVAL = "resolver.reset.interval";

‎internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_CACHE_DIR;
3838
import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_NAMES_ARE_UNIQUE;
3939
import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_RESET_INTERVAL;
40+
import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_SIMPLE_METHOD_GRAPH;
4041
import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_USE_LOADCLASS;
4142
import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_USE_URL_CACHES;
4243
import static datadog.trace.api.config.TraceInstrumentationConfig.RUNTIME_CONTEXT_FIELD_INJECTION;
@@ -116,6 +117,7 @@ public class InstrumenterConfig {
116117
private final ResolverCacheConfig resolverCacheConfig;
117118
private final String resolverCacheDir;
118119
private final boolean resolverNamesAreUnique;
120+
private final boolean resolverSimpleMethodGraph;
119121
private final boolean resolverUseLoadClass;
120122
private final Boolean resolverUseUrlCaches;
121123
private final int resolverResetInterval;
@@ -196,6 +198,9 @@ private InstrumenterConfig() {
196198
RESOLVER_CACHE_CONFIG, ResolverCacheConfig.class, ResolverCacheConfig.MEMOS);
197199
resolverCacheDir = configProvider.getString(RESOLVER_CACHE_DIR);
198200
resolverNamesAreUnique = configProvider.getBoolean(RESOLVER_NAMES_ARE_UNIQUE, false);
201+
resolverSimpleMethodGraph =
202+
// use simpler approach everywhere except GraalVM, where it affects reachability analysis
203+
configProvider.getBoolean(RESOLVER_SIMPLE_METHOD_GRAPH, !Platform.isNativeImageBuilder());
199204
resolverUseLoadClass = configProvider.getBoolean(RESOLVER_USE_LOADCLASS, true);
200205
resolverUseUrlCaches = configProvider.getBoolean(RESOLVER_USE_URL_CACHES);
201206
resolverResetInterval =
@@ -353,6 +358,10 @@ public boolean isResolverNamesAreUnique() {
353358
return resolverNamesAreUnique;
354359
}
355360

361+
public boolean isResolverSimpleMethodGraph() {
362+
return resolverSimpleMethodGraph;
363+
}
364+
356365
public boolean isResolverUseLoadClass() {
357366
return resolverUseLoadClass;
358367
}
@@ -479,6 +488,8 @@ public String toString() {
479488
+ resolverCacheDir
480489
+ ", resolverNamesAreUnique="
481490
+ resolverNamesAreUnique
491+
+ ", resolverSimpleMethodGraph="
492+
+ resolverSimpleMethodGraph
482493
+ ", resolverUseLoadClass="
483494
+ resolverUseLoadClass
484495
+ ", resolverUseUrlCaches="

0 commit comments

Comments
 (0)