Skip to content

Commit 393eaa1

Browse files
jpbempeljpbempel
committed
Add line probe exploration tests
1 parent 6b6cbf6 commit 393eaa1

File tree

12 files changed

+108
-39
lines changed

12 files changed

+108
-39
lines changed

‎.gitlab/exploration-tests.yml

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,32 +45,42 @@ build-exploration-tests-image:
4545
- "*_surefire-reports.tar.gz"
4646
- "*_debugger-dumps.tar.gz"
4747

48-
exploration-tests-jsoup:
48+
exploration-tests-method-jsoup:
4949
needs: [ build ]
5050
dependencies:
5151
- build
5252
<<: *common-exploration-tests
5353
variables:
5454
PROJECT: jsoup
5555
script:
56-
- ./run-exploration-tests.sh "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
56+
- ./run-exploration-tests.sh "method" "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
5757

58-
exploration-tests-jackson-core:
58+
exploration-tests-line-jsoup:
59+
needs: [ build ]
60+
dependencies:
61+
- build
62+
<<: *common-exploration-tests
63+
variables:
64+
PROJECT: jsoup
65+
script:
66+
- ./run-exploration-tests.sh "line" "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
67+
68+
exploration-tests-method-jackson-core:
5969
needs: [ build ]
6070
dependencies:
6171
- build
6272
<<: *common-exploration-tests
6373
variables:
6474
PROJECT: jackson-core
6575
script:
66-
- ./run-exploration-tests.sh "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
76+
- ./run-exploration-tests.sh "method" "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
6777

68-
exploration-tests-jackson-databind:
78+
exploration-tests-method-jackson-databind:
6979
needs: [ build ]
7080
dependencies:
7181
- build
7282
<<: *common-exploration-tests
7383
variables:
7484
PROJECT: jackson-databind
7585
script:
76-
- ./run-exploration-tests.sh "$PROJECT" "./mvnw verify" "exclude_$PROJECT.txt"
86+
- ./run-exploration-tests.sh "method" "$PROJECT" "./mvnw verify" "exclude_$PROJECT.txt"

‎dd-java-agent/agent-debugger/exploration-tests/run-exploration-tests.sh

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
#!/usr/bin/env bash
22
set -uo pipefail
3-
NAME=$1
4-
COMMAND=$2
5-
PROJECT_INCLUDE_FILE=${3:-}
6-
PROJECT_EXCLUDE_FILE=${4:-}
3+
ITW_TYPE=${1:-"method"}
4+
NAME=$2
5+
COMMAND=$3
6+
PROJECT_INCLUDE_FILE=${4:-}
7+
PROJECT_EXCLUDE_FILE=${5:-}
78
echo " === running debugger java exploration tests === "
8-
export JAVA_TOOL_OPTIONS="-javaagent:`pwd`/dd-java-agent.jar -Ddatadog.slf4j.simpleLogger.log.com.datadog.debugger=debug -Ddd.trace.enabled=false -Ddd.dynamic.instrumentation.enabled=true -Ddd.dynamic.instrumentation.instrument.the.world=true -Ddd.dynamic.instrumentation.classfile.dump.enabled=true -Ddd.dynamic.instrumentation.verify.bytecode=false -Ddd.dynamic.instrumentation.include.files=/exploration-tests/$PROJECT_INCLUDE_FILE -Ddd.dynamic.instrumentation.exclude.files=/exploration-tests/$PROJECT_EXCLUDE_FILE"
9+
export JAVA_TOOL_OPTIONS="-javaagent:`pwd`/dd-java-agent.jar -Ddatadog.slf4j.simpleLogger.log.com.datadog.debugger=debug -Ddd.trace.enabled=false -Ddd.dynamic.instrumentation.enabled=true -Ddd.dynamic.instrumentation.instrument.the.world=$ITW_TYPE -Ddd.dynamic.instrumentation.classfile.dump.enabled=true -Ddd.dynamic.instrumentation.verify.bytecode=false -Ddd.dynamic.instrumentation.include.files=/exploration-tests/$PROJECT_INCLUDE_FILE -Ddd.dynamic.instrumentation.exclude.files=/exploration-tests/$PROJECT_EXCLUDE_FILE"
910
echo "$JAVA_TOOL_OPTIONS"
1011
cd $NAME
1112
echo "Building repository $NAME..."

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public static synchronized void run(Instrumentation inst, SharedCommunicationObj
8686
}
8787
if (config.isDynamicInstrumentationEnabled()) {
8888
startDynamicInstrumentation();
89-
if (config.isDynamicInstrumentationInstrumentTheWorld()) {
89+
if (config.getDynamicInstrumentationInstrumentTheWorld() != null) {
9090
setupInstrumentTheWorldTransformer(config, instrumentation, sink);
9191
}
9292
}

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import java.util.Map;
5353
import java.util.Set;
5454
import java.util.concurrent.ConcurrentHashMap;
55+
import java.util.function.BiConsumer;
5556
import java.util.regex.Pattern;
5657
import net.bytebuddy.description.type.TypeDescription;
5758
import net.bytebuddy.pool.TypePool;
@@ -103,6 +104,7 @@ public class DebuggerTransformer implements ClassFileTransformer {
103104
private final Set<String> includeMethods;
104105
private final Trie includeTrie;
105106
private final Map<String, LogProbe> instrumentTheWorldProbes;
107+
private final BiConsumer<MethodInfo, List<ProbeDefinition>> probeCreator;
106108

107109
public interface InstrumentationListener {
108110
void instrumentationResult(ProbeDefinition definition, InstrumentationResult result);
@@ -119,7 +121,8 @@ public DebuggerTransformer(
119121
this.denyListHelper = new DenyListHelper(configuration.getDenyList());
120122
this.listener = listener;
121123
this.debuggerSink = debuggerSink;
122-
this.instrumentTheWorld = config.isDynamicInstrumentationInstrumentTheWorld();
124+
String itwType = config.getDynamicInstrumentationInstrumentTheWorld();
125+
this.instrumentTheWorld = itwType != null;
123126
if (this.instrumentTheWorld) {
124127
instrumentTheWorldProbes = new ConcurrentHashMap<>();
125128
excludeTrie = new Trie();
@@ -138,6 +141,17 @@ public DebuggerTransformer(
138141
includeTrie,
139142
includeClasses,
140143
includeMethods);
144+
if (itwType.equals("method")) {
145+
probeCreator = this::createMethodProbe;
146+
} else if (itwType.equals("line")) {
147+
probeCreator = this::createLineProbes;
148+
} else {
149+
log.warn(
150+
"Invalid value for 'dd.debugger.instrument-the-world' property: {}. "
151+
+ "Valid values are 'method' or 'line'.",
152+
itwType);
153+
probeCreator = null;
154+
}
141155
} else {
142156
instrumentTheWorldProbes = null;
143157
excludeTrie = null;
@@ -146,6 +160,7 @@ public DebuggerTransformer(
146160
includeTrie = null;
147161
includeClasses = null;
148162
includeMethods = null;
163+
probeCreator = null;
149164
}
150165
}
151166

@@ -211,8 +226,7 @@ public byte[] transform(
211226
ProtectionDomain protectionDomain,
212227
byte[] classfileBuffer) {
213228
if (instrumentTheWorld) {
214-
return transformTheWorld(
215-
loader, classFilePath, classBeingRedefined, protectionDomain, classfileBuffer);
229+
return transformTheWorld(loader, classFilePath, protectionDomain, classfileBuffer);
216230
}
217231
if (skipInstrumentation(loader, classFilePath)) {
218232
return null;
@@ -264,7 +278,6 @@ private boolean skipInstrumentation(ClassLoader loader, String classFilePath) {
264278
private byte[] transformTheWorld(
265279
ClassLoader loader,
266280
String classFilePath,
267-
Class<?> classBeingRedefined,
268281
ProtectionDomain protectionDomain,
269282
byte[] classfileBuffer) {
270283
try {
@@ -303,16 +316,11 @@ private byte[] transformTheWorld(
303316
}
304317
List<ProbeDefinition> probes = new ArrayList<>();
305318
Set<String> methodNames = new HashSet<>();
319+
ClassFileLines classFileLines = new ClassFileLines(classNode);
306320
for (MethodNode methodNode : classNode.methods) {
307321
if (isMethodIncludedForTransformation(methodNode, classNode, methodNames)) {
308-
LogProbe probe =
309-
LogProbe.builder()
310-
.probeId(RandomUtils.randomUUID().toString(), 0)
311-
.where(classNode.name, methodNode.name)
312-
.captureSnapshot(false)
313-
.build();
314-
probes.add(probe);
315-
instrumentTheWorldProbes.put(probe.getProbeId().getEncodedId(), probe);
322+
MethodInfo methodInfo = new MethodInfo(loader, classNode, methodNode, classFileLines);
323+
probeCreator.accept(methodInfo, probes);
316324
}
317325
}
318326
boolean transformed = performInstrumentation(loader, classFilePath, probes, classNode);
@@ -342,6 +350,42 @@ private boolean isMethodIncludedForTransformation(
342350
return methodNames.add(methodNode.name);
343351
}
344352

353+
private void createMethodProbe(MethodInfo methodInfo, List<ProbeDefinition> probes) {
354+
LogProbe probe =
355+
LogProbe.builder()
356+
.probeId(RandomUtils.randomUUID().toString(), 0)
357+
.where(methodInfo.getClassNode().name, methodInfo.getMethodNode().name)
358+
.captureSnapshot(false)
359+
.build();
360+
probes.add(probe);
361+
instrumentTheWorldProbes.put(probe.getProbeId().getEncodedId(), probe);
362+
}
363+
364+
private void createLineProbes(MethodInfo methodInfo, List<ProbeDefinition> probes) {
365+
log.debug("createLineProbes for {}", methodInfo.getMethodName());
366+
if (methodInfo.getMethodName().equals("<init>")) {
367+
// skip constructor for now to avoid dealing with code before super calls
368+
log.debug("createLineProbes skip for constructor");
369+
return;
370+
}
371+
if ((methodInfo.getMethodNode().access & Opcodes.ACC_SYNTHETIC) != 0) {
372+
// skip synthetic methods
373+
log.debug("createLineProbes skip for synthetic method");
374+
return;
375+
}
376+
List<Integer> lineNumbers = methodInfo.getLineNumbers();
377+
for (Integer lineNumber : lineNumbers) {
378+
LogProbe probe =
379+
LogProbe.builder()
380+
.probeId(RandomUtils.randomUUID().toString(), 0)
381+
.where(methodInfo.getSourceFileName(), lineNumber)
382+
.captureSnapshot(false)
383+
.build();
384+
probes.add(probe);
385+
instrumentTheWorldProbes.put(probe.getProbeId().getEncodedId(), probe);
386+
}
387+
}
388+
345389
private boolean isClassLoaderRelated(ClassNode classNode) {
346390
return classNode.superName.equals("java/lang/ClassLoader")
347391
|| classNode.superName.equals("java/net/URLClassLoader")

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ private void collectLocalVariables(AbstractInsnNode location, InsnList insnList)
910910
}
911911

912912
if (methodNode.localVariables == null || methodNode.localVariables.isEmpty()) {
913-
if (!Config.get().isDynamicInstrumentationInstrumentTheWorld()) {
913+
if (Config.get().getDynamicInstrumentationInstrumentTheWorld() == null) {
914914
reportWarning("Missing local variable debug info");
915915
}
916916
// no local variables info - bail out
@@ -1136,7 +1136,7 @@ private static List<FieldNode> extractStaticFields(
11361136
}
11371137
}
11381138
}
1139-
if (!Config.get().isDynamicInstrumentationInstrumentTheWorld()) {
1139+
if (Config.get().getDynamicInstrumentationInstrumentTheWorld() == null) {
11401140
// Collects inherited static fields only if the ITW mode is not enabled
11411141
// because it can lead to LinkageError: attempted duplicate class definition
11421142
// for example, when a probe is located in method overridden in enum element

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MethodInfo.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22

33
import com.datadog.debugger.util.ClassFileLines;
44
import datadog.trace.util.Strings;
5+
import java.util.ArrayList;
6+
import java.util.List;
7+
import org.objectweb.asm.tree.AbstractInsnNode;
58
import org.objectweb.asm.tree.ClassNode;
9+
import org.objectweb.asm.tree.LineNumberNode;
610
import org.objectweb.asm.tree.MethodNode;
711

812
/** Data class to store all information related to a method (class, classloader, lines) */
@@ -59,6 +63,19 @@ public String getTypeName() {
5963
return Strings.getClassName(classNode.name);
6064
}
6165

66+
public List<Integer> getLineNumbers() {
67+
List<Integer> lines = new ArrayList<>();
68+
AbstractInsnNode current = methodNode.instructions.getFirst();
69+
while (current != null) {
70+
if (current.getType() == AbstractInsnNode.LINE) {
71+
LineNumberNode lineNode = (LineNumberNode) current;
72+
lines.add(lineNode.line);
73+
}
74+
current = current.getNext();
75+
}
76+
return lines;
77+
}
78+
6279
@Override
6380
public String toString() {
6481
return String.format(

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/ProbeStatusSink.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public ProbeStatusSink(Config config, String diagnosticsEndpoint, boolean useMul
5858
this.interval = Duration.ofSeconds(config.getDynamicInstrumentationDiagnosticsInterval());
5959
this.batchSize = config.getDynamicInstrumentationUploadBatchSize();
6060
this.queue = new ArrayBlockingQueue<>(2 * this.batchSize);
61-
this.isInstrumentTheWorld = config.isDynamicInstrumentationInstrumentTheWorld();
61+
this.isInstrumentTheWorld = config.getDynamicInstrumentationInstrumentTheWorld() != null;
6262
}
6363

6464
public void stop() {

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/uploader/BatchUploader.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public BatchUploader(Config config, String endpoint, RetryPolicy retryPolicy) {
128128
RetryPolicy retryPolicy,
129129
String containerId,
130130
String entityId) {
131-
instrumentTheWorld = config.isDynamicInstrumentationInstrumentTheWorld();
131+
instrumentTheWorld = config.getDynamicInstrumentationInstrumentTheWorld() != null;
132132
if (endpoint == null || endpoint.length() == 0) {
133133
throw new IllegalArgumentException("Endpoint url is empty");
134134
}

‎dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2641,7 +2641,7 @@ private TestSnapshotListener setupInstrumentTheWorldTransformer(
26412641
Config config = mock(Config.class);
26422642
when(config.isDynamicInstrumentationEnabled()).thenReturn(true);
26432643
when(config.isDynamicInstrumentationClassFileDumpEnabled()).thenReturn(true);
2644-
when(config.isDynamicInstrumentationInstrumentTheWorld()).thenReturn(true);
2644+
when(config.getDynamicInstrumentationInstrumentTheWorld()).thenReturn("method");
26452645
when(config.getDynamicInstrumentationExcludeFiles()).thenReturn(excludeFileName);
26462646
when(config.getDynamicInstrumentationIncludeFiles()).thenReturn(includeFileName);
26472647
when(config.getFinalDebuggerSnapshotUrl())

‎dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ public final class ConfigDefaults {
193193
static final int DEFAULT_DYNAMIC_INSTRUMENTATION_UPLOAD_BATCH_SIZE = 100;
194194
static final int DEFAULT_DYNAMIC_INSTRUMENTATION_MAX_PAYLOAD_SIZE = 1024; // KiB
195195
static final boolean DEFAULT_DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE = true;
196-
static final boolean DEFAULT_DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD = false;
197196
static final int DEFAULT_DYNAMIC_INSTRUMENTATION_CAPTURE_TIMEOUT = 100; // milliseconds
198197
static final boolean DEFAULT_DYNAMIC_INSTRUMENTATION_HOIST_LOCALVARS_ENABLED = false;
199198
static final boolean DEFAULT_SYMBOL_DATABASE_ENABLED = true;

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ public static String getHostName() {
409409
private final int dynamicInstrumentationUploadBatchSize;
410410
private final long dynamicInstrumentationMaxPayloadSize;
411411
private final boolean dynamicInstrumentationVerifyByteCode;
412-
private final boolean dynamicInstrumentationInstrumentTheWorld;
412+
private final String dynamicInstrumentationInstrumentTheWorld;
413413
private final String dynamicInstrumentationExcludeFiles;
414414
private final String dynamicInstrumentationIncludeFiles;
415415
private final int dynamicInstrumentationCaptureTimeout;
@@ -1682,9 +1682,7 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment())
16821682
DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE,
16831683
DEFAULT_DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE);
16841684
dynamicInstrumentationInstrumentTheWorld =
1685-
configProvider.getBoolean(
1686-
DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD,
1687-
DEFAULT_DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD);
1685+
configProvider.getString(DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD);
16881686
dynamicInstrumentationExcludeFiles =
16891687
configProvider.getString(DYNAMIC_INSTRUMENTATION_EXCLUDE_FILES);
16901688
dynamicInstrumentationIncludeFiles =
@@ -3252,7 +3250,7 @@ public boolean isDynamicInstrumentationVerifyByteCode() {
32523250
return dynamicInstrumentationVerifyByteCode;
32533251
}
32543252

3255-
public boolean isDynamicInstrumentationInstrumentTheWorld() {
3253+
public String getDynamicInstrumentationInstrumentTheWorld() {
32563254
return dynamicInstrumentationInstrumentTheWorld;
32573255
}
32583256

‎internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ class ConfigTest extends DDSpecification {
260260
prop.setProperty(DYNAMIC_INSTRUMENTATION_POLL_INTERVAL, "10")
261261
prop.setProperty(DYNAMIC_INSTRUMENTATION_DIAGNOSTICS_INTERVAL, "60")
262262
prop.setProperty(DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE, "true")
263-
prop.setProperty(DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD, "true")
263+
prop.setProperty(DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD, "method")
264264
prop.setProperty(DYNAMIC_INSTRUMENTATION_EXCLUDE_FILES, "exclude file")
265265
prop.setProperty(EXCEPTION_REPLAY_ENABLED, "true")
266266
prop.setProperty(TRACE_X_DATADOG_TAGS_MAX_LENGTH, "128")
@@ -356,7 +356,7 @@ class ConfigTest extends DDSpecification {
356356
config.dynamicInstrumentationPollInterval == 10
357357
config.dynamicInstrumentationDiagnosticsInterval == 60
358358
config.dynamicInstrumentationVerifyByteCode == true
359-
config.dynamicInstrumentationInstrumentTheWorld == true
359+
config.dynamicInstrumentationInstrumentTheWorld == "method"
360360
config.dynamicInstrumentationExcludeFiles == "exclude file"
361361
config.debuggerExceptionEnabled == true
362362
config.jdkSocketEnabled == false
@@ -451,7 +451,7 @@ class ConfigTest extends DDSpecification {
451451
System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_POLL_INTERVAL, "10")
452452
System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_DIAGNOSTICS_INTERVAL, "60")
453453
System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE, "true")
454-
System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD, "true")
454+
System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD, "method")
455455
System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_EXCLUDE_FILES, "exclude file")
456456
System.setProperty(PREFIX + TRACE_X_DATADOG_TAGS_MAX_LENGTH, "128")
457457

@@ -543,7 +543,7 @@ class ConfigTest extends DDSpecification {
543543
config.dynamicInstrumentationPollInterval == 10
544544
config.dynamicInstrumentationDiagnosticsInterval == 60
545545
config.dynamicInstrumentationVerifyByteCode == true
546-
config.dynamicInstrumentationInstrumentTheWorld == true
546+
config.dynamicInstrumentationInstrumentTheWorld == "method"
547547
config.dynamicInstrumentationExcludeFiles == "exclude file"
548548

549549
config.xDatadogTagsMaxLength == 128

0 commit comments

Comments
 (0)