Add line probe exploration tests

.gitlab/exploration-tests.yml

@@ -45,32 +45,62 @@ build-exploration-tests-image:
       - "*_surefire-reports.tar.gz"
       - "*_debugger-dumps.tar.gz"
 
-exploration-tests-jsoup:
+exploration-tests-method-jsoup:
   needs: [ build ]
   dependencies:
     - build
   <<: *common-exploration-tests
   variables:
     PROJECT: jsoup
   script:
-    - ./run-exploration-tests.sh "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
+    - ./run-exploration-tests.sh "method" "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
 
-exploration-tests-jackson-core:
+exploration-tests-line-jsoup:
+  needs: [ build ]
+  dependencies:
+    - build
+  <<: *common-exploration-tests
+  variables:
+    PROJECT: jsoup
+  script:
+    - ./run-exploration-tests.sh "line" "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
+
+exploration-tests-method-jackson-core:
+  needs: [ build ]
+  dependencies:
+    - build
+  <<: *common-exploration-tests
+  variables:
+    PROJECT: jackson-core
+  script:
+    - ./run-exploration-tests.sh "method" "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
+
+exploration-tests-line-jackson-core:
   needs: [ build ]
   dependencies:
     - build
   <<: *common-exploration-tests
   variables:
     PROJECT: jackson-core
   script:
-    - ./run-exploration-tests.sh "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
+    - ./run-exploration-tests.sh "line" "$PROJECT" "mvn verify" "include_${PROJECT}.txt" "exclude_${PROJECT}.txt"
+
+exploration-tests-method-jackson-databind:
+  needs: [ build ]
+  dependencies:
+    - build
+  <<: *common-exploration-tests
+  variables:
+    PROJECT: jackson-databind
+  script:
+    - ./run-exploration-tests.sh "method" "$PROJECT" "./mvnw verify" "include_${PROJECT}.txt" "exclude_$PROJECT.txt"
 
-exploration-tests-jackson-databind:
+exploration-tests-line-jackson-databind:
   needs: [ build ]
   dependencies:
     - build
   <<: *common-exploration-tests
   variables:
     PROJECT: jackson-databind
   script:
-    - ./run-exploration-tests.sh "$PROJECT" "./mvnw verify" "exclude_$PROJECT.txt"
+    - ./run-exploration-tests.sh "line" "$PROJECT" "./mvnw verify" "include_${PROJECT}.txt" "exclude_line_$PROJECT.txt"

dd-java-agent/agent-debugger/exploration-tests/exclude_line_jackson-databind.txt

@@ -0,0 +1,3 @@
+com/fasterxml/jackson/databind/BaseTest
+com/fasterxml/jackson/databind/BaseMapTest*
+com/fasterxml/jackson/databind/type/TypeFactory

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

@@ -1,11 +1,12 @@
 #!/usr/bin/env bash
 set -uo pipefail
-NAME=$1
-COMMAND=$2
-PROJECT_INCLUDE_FILE=${3:-}
-PROJECT_EXCLUDE_FILE=${4:-}
+ITW_TYPE=${1:-"method"}
+NAME=$2
+COMMAND=$3
+PROJECT_INCLUDE_FILE=${4:-}
+PROJECT_EXCLUDE_FILE=${5:-}
 echo " === running debugger java exploration tests === "
-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"
+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"
 echo "$JAVA_TOOL_OPTIONS"
 cd $NAME
 echo "Building repository $NAME..."

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

@@ -86,7 +86,7 @@ public static synchronized void run(Instrumentation inst, SharedCommunicationObj
     }
     if (config.isDynamicInstrumentationEnabled()) {
       startDynamicInstrumentation();
-      if (config.isDynamicInstrumentationInstrumentTheWorld()) {
+      if (config.getDynamicInstrumentationInstrumentTheWorld() != null) {
         setupInstrumentTheWorldTransformer(config, instrumentation, sink);
       }
     }

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

@@ -52,6 +52,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiConsumer;
 import java.util.regex.Pattern;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.pool.TypePool;
@@ -103,6 +104,7 @@ public class DebuggerTransformer implements ClassFileTransformer {
   private final Set<String> includeMethods;
   private final Trie includeTrie;
   private final Map<String, LogProbe> instrumentTheWorldProbes;
+  private final BiConsumer<MethodInfo, List<ProbeDefinition>> probeCreator;
 
   public interface InstrumentationListener {
     void instrumentationResult(ProbeDefinition definition, InstrumentationResult result);
@@ -119,7 +121,8 @@ public DebuggerTransformer(
     this.denyListHelper = new DenyListHelper(configuration.getDenyList());
     this.listener = listener;
     this.debuggerSink = debuggerSink;
-    this.instrumentTheWorld = config.isDynamicInstrumentationInstrumentTheWorld();
+    String itwType = config.getDynamicInstrumentationInstrumentTheWorld();
+    this.instrumentTheWorld = itwType != null;
     if (this.instrumentTheWorld) {
       instrumentTheWorldProbes = new ConcurrentHashMap<>();
       excludeTrie = new Trie();
@@ -138,6 +141,17 @@ public DebuggerTransformer(
           includeTrie,
           includeClasses,
           includeMethods);
+      if (itwType.equals("method")) {
+        probeCreator = this::createMethodProbe;
+      } else if (itwType.equals("line")) {
+        probeCreator = this::createLineProbes;
+      } else {
+        log.warn(
+            "Invalid value for 'dd.debugger.instrument-the-world' property: {}. "
+                + "Valid values are 'method' or 'line'.",
+            itwType);
+        probeCreator = null;
+      }
     } else {
       instrumentTheWorldProbes = null;
       excludeTrie = null;
@@ -146,6 +160,7 @@ public DebuggerTransformer(
       includeTrie = null;
       includeClasses = null;
       includeMethods = null;
+      probeCreator = null;
     }
   }
 
@@ -211,8 +226,7 @@ public byte[] transform(
       ProtectionDomain protectionDomain,
       byte[] classfileBuffer) {
     if (instrumentTheWorld) {
-      return transformTheWorld(
-          loader, classFilePath, classBeingRedefined, protectionDomain, classfileBuffer);
+      return transformTheWorld(loader, classFilePath, protectionDomain, classfileBuffer);
     }
     if (skipInstrumentation(loader, classFilePath)) {
       return null;
@@ -264,7 +278,6 @@ private boolean skipInstrumentation(ClassLoader loader, String classFilePath) {
   private byte[] transformTheWorld(
       ClassLoader loader,
       String classFilePath,
-      Class<?> classBeingRedefined,
       ProtectionDomain protectionDomain,
       byte[] classfileBuffer) {
     try {
@@ -303,16 +316,11 @@ private byte[] transformTheWorld(
       }
       List<ProbeDefinition> probes = new ArrayList<>();
       Set<String> methodNames = new HashSet<>();
+      ClassFileLines classFileLines = new ClassFileLines(classNode);
       for (MethodNode methodNode : classNode.methods) {
         if (isMethodIncludedForTransformation(methodNode, classNode, methodNames)) {
-          LogProbe probe =
-              LogProbe.builder()
-                  .probeId(RandomUtils.randomUUID().toString(), 0)
-                  .where(classNode.name, methodNode.name)
-                  .captureSnapshot(false)
-                  .build();
-          probes.add(probe);
-          instrumentTheWorldProbes.put(probe.getProbeId().getEncodedId(), probe);
+          MethodInfo methodInfo = new MethodInfo(loader, classNode, methodNode, classFileLines);
+          probeCreator.accept(methodInfo, probes);
         }
       }
       boolean transformed = performInstrumentation(loader, classFilePath, probes, classNode);
@@ -342,6 +350,39 @@ private boolean isMethodIncludedForTransformation(
     return methodNames.add(methodNode.name);
   }
 
+  private void createMethodProbe(MethodInfo methodInfo, List<ProbeDefinition> probes) {
+    LogProbe probe =
+        LogProbe.builder()
+            .probeId(RandomUtils.randomUUID().toString(), 0)
+            .where(methodInfo.getClassNode().name, methodInfo.getMethodNode().name)
+            .captureSnapshot(false)
+            .build();
+    probes.add(probe);
+    instrumentTheWorldProbes.put(probe.getProbeId().getEncodedId(), probe);
+  }
+
+  private void createLineProbes(MethodInfo methodInfo, List<ProbeDefinition> probes) {
+    if (methodInfo.getMethodName().equals("<init>")) {
+      // skip constructor for now to avoid dealing with code before super calls
+      return;
+    }
+    if ((methodInfo.getMethodNode().access & Opcodes.ACC_SYNTHETIC) != 0) {
+      // skip synthetic methods
+      return;
+    }
+    List<Integer> lineNumbers = methodInfo.getLineNumbers();
+    for (Integer lineNumber : lineNumbers) {
+      LogProbe probe =
+          LogProbe.builder()
+              .probeId(RandomUtils.randomUUID().toString(), 0)
+              .where(methodInfo.getSourceFileName(), lineNumber)
+              .captureSnapshot(false)
+              .build();
+      probes.add(probe);
+      instrumentTheWorldProbes.put(probe.getProbeId().getEncodedId(), probe);
+    }
+  }
+
   private boolean isClassLoaderRelated(ClassNode classNode) {
     return classNode.superName.equals("java/lang/ClassLoader")
         || classNode.superName.equals("java/net/URLClassLoader")

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

@@ -910,7 +910,7 @@ private void collectLocalVariables(AbstractInsnNode location, InsnList insnList)
     }
 
     if (methodNode.localVariables == null || methodNode.localVariables.isEmpty()) {
-      if (!Config.get().isDynamicInstrumentationInstrumentTheWorld()) {
+      if (Config.get().getDynamicInstrumentationInstrumentTheWorld() == null) {
         reportWarning("Missing local variable debug info");
       }
       // no local variables info - bail out
@@ -1136,7 +1136,7 @@ private static List<FieldNode> extractStaticFields(
         }
       }
     }
-    if (!Config.get().isDynamicInstrumentationInstrumentTheWorld()) {
+    if (Config.get().getDynamicInstrumentationInstrumentTheWorld() == null) {
       // Collects inherited static fields only if the ITW mode is not enabled
       // because it can lead to LinkageError: attempted duplicate class definition
       // 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

@@ -2,7 +2,11 @@
 
 import com.datadog.debugger.util.ClassFileLines;
 import datadog.trace.util.Strings;
+import java.util.ArrayList;
+import java.util.List;
+import org.objectweb.asm.tree.AbstractInsnNode;
 import org.objectweb.asm.tree.ClassNode;
+import org.objectweb.asm.tree.LineNumberNode;
 import org.objectweb.asm.tree.MethodNode;
 
 /** Data class to store all information related to a method (class, classloader, lines) */
@@ -59,6 +63,19 @@ public String getTypeName() {
     return Strings.getClassName(classNode.name);
   }
 
+  public List<Integer> getLineNumbers() {
+    List<Integer> lines = new ArrayList<>();
+    AbstractInsnNode current = methodNode.instructions.getFirst();
+    while (current != null) {
+      if (current.getType() == AbstractInsnNode.LINE) {
+        LineNumberNode lineNode = (LineNumberNode) current;
+        lines.add(lineNode.line);
+      }
+      current = current.getNext();
+    }
+    return lines;
+  }
+
   @Override
   public String toString() {
     return String.format(

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

@@ -58,7 +58,7 @@ public ProbeStatusSink(Config config, String diagnosticsEndpoint, boolean useMul
     this.interval = Duration.ofSeconds(config.getDynamicInstrumentationDiagnosticsInterval());
     this.batchSize = config.getDynamicInstrumentationUploadBatchSize();
     this.queue = new ArrayBlockingQueue<>(2 * this.batchSize);
-    this.isInstrumentTheWorld = config.isDynamicInstrumentationInstrumentTheWorld();
+    this.isInstrumentTheWorld = config.getDynamicInstrumentationInstrumentTheWorld() != null;
   }
 
   public void stop() {

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

@@ -128,7 +128,7 @@ public BatchUploader(Config config, String endpoint, RetryPolicy retryPolicy) {
       RetryPolicy retryPolicy,
       String containerId,
       String entityId) {
-    instrumentTheWorld = config.isDynamicInstrumentationInstrumentTheWorld();
+    instrumentTheWorld = config.getDynamicInstrumentationInstrumentTheWorld() != null;
     if (endpoint == null || endpoint.length() == 0) {
       throw new IllegalArgumentException("Endpoint url is empty");
     }

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

@@ -2641,7 +2641,7 @@ private TestSnapshotListener setupInstrumentTheWorldTransformer(
     Config config = mock(Config.class);
     when(config.isDynamicInstrumentationEnabled()).thenReturn(true);
     when(config.isDynamicInstrumentationClassFileDumpEnabled()).thenReturn(true);
-    when(config.isDynamicInstrumentationInstrumentTheWorld()).thenReturn(true);
+    when(config.getDynamicInstrumentationInstrumentTheWorld()).thenReturn("method");
     when(config.getDynamicInstrumentationExcludeFiles()).thenReturn(excludeFileName);
     when(config.getDynamicInstrumentationIncludeFiles()).thenReturn(includeFileName);
     when(config.getFinalDebuggerSnapshotUrl())

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

@@ -193,7 +193,6 @@ public final class ConfigDefaults {
   static final int DEFAULT_DYNAMIC_INSTRUMENTATION_UPLOAD_BATCH_SIZE = 100;
   static final int DEFAULT_DYNAMIC_INSTRUMENTATION_MAX_PAYLOAD_SIZE = 1024; // KiB
   static final boolean DEFAULT_DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE = true;
-  static final boolean DEFAULT_DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD = false;
   static final int DEFAULT_DYNAMIC_INSTRUMENTATION_CAPTURE_TIMEOUT = 100; // milliseconds
   static final boolean DEFAULT_DYNAMIC_INSTRUMENTATION_HOIST_LOCALVARS_ENABLED = false;
   static final boolean DEFAULT_SYMBOL_DATABASE_ENABLED = true;

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

@@ -409,7 +409,7 @@ public static String getHostName() {
   private final int dynamicInstrumentationUploadBatchSize;
   private final long dynamicInstrumentationMaxPayloadSize;
   private final boolean dynamicInstrumentationVerifyByteCode;
-  private final boolean dynamicInstrumentationInstrumentTheWorld;
+  private final String dynamicInstrumentationInstrumentTheWorld;
   private final String dynamicInstrumentationExcludeFiles;
   private final String dynamicInstrumentationIncludeFiles;
   private final int dynamicInstrumentationCaptureTimeout;
@@ -1682,9 +1682,7 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment())
             DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE,
             DEFAULT_DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE);
     dynamicInstrumentationInstrumentTheWorld =
-        configProvider.getBoolean(
-            DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD,
-            DEFAULT_DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD);
+        configProvider.getString(DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD);
     dynamicInstrumentationExcludeFiles =
         configProvider.getString(DYNAMIC_INSTRUMENTATION_EXCLUDE_FILES);
     dynamicInstrumentationIncludeFiles =
@@ -3252,7 +3250,7 @@ public boolean isDynamicInstrumentationVerifyByteCode() {
     return dynamicInstrumentationVerifyByteCode;
   }
 
-  public boolean isDynamicInstrumentationInstrumentTheWorld() {
+  public String getDynamicInstrumentationInstrumentTheWorld() {
     return dynamicInstrumentationInstrumentTheWorld;
   }
 

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

@@ -260,7 +260,7 @@ class ConfigTest extends DDSpecification {
     prop.setProperty(DYNAMIC_INSTRUMENTATION_POLL_INTERVAL, "10")
     prop.setProperty(DYNAMIC_INSTRUMENTATION_DIAGNOSTICS_INTERVAL, "60")
     prop.setProperty(DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE, "true")
-    prop.setProperty(DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD, "true")
+    prop.setProperty(DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD, "method")
     prop.setProperty(DYNAMIC_INSTRUMENTATION_EXCLUDE_FILES, "exclude file")
     prop.setProperty(EXCEPTION_REPLAY_ENABLED, "true")
     prop.setProperty(TRACE_X_DATADOG_TAGS_MAX_LENGTH, "128")
@@ -356,7 +356,7 @@ class ConfigTest extends DDSpecification {
     config.dynamicInstrumentationPollInterval == 10
     config.dynamicInstrumentationDiagnosticsInterval == 60
     config.dynamicInstrumentationVerifyByteCode == true
-    config.dynamicInstrumentationInstrumentTheWorld == true
+    config.dynamicInstrumentationInstrumentTheWorld == "method"
     config.dynamicInstrumentationExcludeFiles == "exclude file"
     config.debuggerExceptionEnabled == true
     config.jdkSocketEnabled == false
@@ -451,7 +451,7 @@ class ConfigTest extends DDSpecification {
     System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_POLL_INTERVAL, "10")
     System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_DIAGNOSTICS_INTERVAL, "60")
     System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_VERIFY_BYTECODE, "true")
-    System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD, "true")
+    System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_INSTRUMENT_THE_WORLD, "method")
     System.setProperty(PREFIX + DYNAMIC_INSTRUMENTATION_EXCLUDE_FILES, "exclude file")
     System.setProperty(PREFIX + TRACE_X_DATADOG_TAGS_MAX_LENGTH, "128")
 
@@ -543,7 +543,7 @@ class ConfigTest extends DDSpecification {
     config.dynamicInstrumentationPollInterval == 10
     config.dynamicInstrumentationDiagnosticsInterval == 60
     config.dynamicInstrumentationVerifyByteCode == true
-    config.dynamicInstrumentationInstrumentTheWorld == true
+    config.dynamicInstrumentationInstrumentTheWorld == "method"
     config.dynamicInstrumentationExcludeFiles == "exclude file"
 
     config.xDatadogTagsMaxLength == 128