Fix exploration tests

update the registry for pushing the image install jdk 21 use the jsoup library and patch it for running test with instrument-the-world mode add include file to specify which packages/classes we want to include into the instrumentation instead of instrumenting everything (like build tools) do not collect static inherited fields to avoid duplicate class definition for enums.
DataDog · Oct 3, 2024 · 38a78ce · 38a78ce
1 parent 5a939c0
commit 38a78ce
Show file tree

Hide file tree

Showing 14 changed files with 175 additions and 61 deletions.
diff --git a/.gitlab/exploration-tests.yml b/.gitlab/exploration-tests.yml
@@ -1,5 +1,5 @@
 variables:
-  EXPLORATION_TESTS_IMAGE: $REGISTRY/ci/dd-trace-java:exploration-tests
+  EXPLORATION_TESTS_IMAGE: registry.ddbuild.io/ci/dd-trace-java:exploration-tests
 
 build-exploration-tests-image:
   stage: exploration-tests
@@ -18,24 +18,32 @@ build-exploration-tests-image:
     - ./gradlew :dd-java-agent:shadowJar --no-scan
     - cp workspace/dd-java-agent/build/libs/*.jar /exploration-tests/dd-java-agent.jar
     - cp dd-java-agent/agent-debugger/exploration-tests/run-exploration-tests.sh /exploration-tests
-    - cp dd-java-agent/agent-debugger/exploration-tests/exclude*.txt /exploration-tests
+    - cp dd-java-agent/agent-debugger/exploration-tests/exclude_*.txt /exploration-tests
+    - cp dd-java-agent/agent-debugger/exploration-tests/include_*.txt /exploration-tests
     - cd /exploration-tests
   after_script:
     - cd $CI_PROJECT_DIR
-    - cp /exploration-tests/$PROJECT/agent.log agent.log
-    - gzip agent.log
-    - tar czf surefire-reports.tar.gz /exploration-tests/$PROJECT/target/surefire-reports
-    - tar czf debugger-dumps.tar.gz /tmp/debugger
+    - cp /exploration-tests/$PROJECT/agent.log $PROJECT_agent.log
+    - gzip $PROJECT_agent.log
+    - tar czf $PROJECT_surefire-reports.tar.gz /exploration-tests/$PROJECT/target/surefire-reports
+    - tar czf $PROJECT_debugger-dumps.tar.gz /tmp/debugger
   stage: exploration-tests
   when: manual
   tags: [ "runner:main"]
   needs: []
   image: $EXPLORATION_TESTS_IMAGE
   artifacts:
     paths:
-      - agent.log.gz
-      - surefire-reports.tar.gz
-      - debugger-dumps.tar.gz
+      - $PROJECT_agent.log.gz
+      - $PROJECT_surefire-reports.tar.gz
+      - $PROJECT_debugger-dumps.tar.gz
+
+exploration-tests-jsoup:
+  variables:
+    PROJECT: jsoup
+  <<: *common-exploration-tests
+  script:
+    - ./run-exploration-tests.sh "$PROJECT" "mvn verify" "include_jsoup.txt" "exclude_jsoup.txt"
 
 
 exploration-tests-jackson-core:

diff --git a/dd-java-agent/agent-debugger/exploration-tests/Dockerfile.exploration-tests b/dd-java-agent/agent-debugger/exploration-tests/Dockerfile.exploration-tests
@@ -3,6 +3,7 @@ FROM debian:bookworm-slim
 ARG JAVA_8_VERSION="8.0.372-tem"
 ARG JAVA_11_VERSION="11.0.19-tem"
 ARG JAVA_17_VERSION="17.0.7-tem"
+ARG JAVA_21_VERSION="21.0.4-tem"
 ARG MAVEN_VERSION=3.8.4
 
 RUN apt-get update && \
@@ -16,25 +17,36 @@ RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && \
     yes | sdk install java $JAVA_8_VERSION && \
     yes | sdk install java $JAVA_11_VERSION && \
     yes | sdk install java $JAVA_17_VERSION && \
+    yes | sdk install java $JAVA_21_VERSION && \
     yes | sdk install maven $MAVEN_VERSION && \
     rm -rf $HOME/.sdkman/archives/* && \
     rm -rf $HOME/.sdkman/tmp/*"
 ENV JAVA_8_HOME=/root/.sdkman/candidates/java/$JAVA_8_VERSION
 ENV JAVA_11_HOME=/root/.sdkman/candidates/java/$JAVA_11_VERSION
 ENV JAVA_17_HOME=/root/.sdkman/candidates/java/$JAVA_17_VERSION
+ENV JAVA_21_HOME=/root/.sdkman/candidates/java/$JAVA_21_VERSION
 
 RUN mkdir exploration-tests
 WORKDIR /exploration-tests
+# jsoup
+RUN git clone -b jsoup-1.18.1 https://github.com/jhy/jsoup.git
+COPY jsoup_exploration-tests.patch .
+# fix tests that are failing because checking time to execute
+RUN cd jsoup && git apply /exploration-tests/jsoup_exploration-tests.patch
+RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd jsoup && mvn verify -DskipTests=true"
+
+
 # Jackson
-RUN git clone https://github.com/FasterXML/jackson-core.git
-RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd jackson-core && ./mvnw dependency:resolve dependency:resolve-plugins"
-RUN git clone https://github.com/FasterXML/jackson-databind.git
-RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd jackson-databind && ./mvnw dependency:resolve dependency:resolve-plugins"
+#RUN git clone https://github.com/FasterXML/jackson-core.git
+#RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd jackson-core && ./mvnw dependency:resolve dependency:resolve-plugins"
+#RUN git clone https://github.com/FasterXML/jackson-databind.git
+#RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd jackson-databind && ./mvnw dependency:resolve dependency:resolve-plugins"
+
 # Netty
-RUN git clone https://github.com/netty/netty.git
-RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd netty && ./mvnw dependency:resolve dependency:resolve-plugins"
+#RUN git clone https://github.com/netty/netty.git
+#RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd netty && ./mvnw dependency:resolve dependency:resolve-plugins"
 
 # Guava
-RUN git clone https://github.com/google/guava.git
-RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd guava && mvn -B -P!standard-with-extra-repos verify -U -Dmaven.javadoc.skip=true -DskipTests=true"
+#RUN git clone https://github.com/google/guava.git
+#RUN bash -c "source $HOME/.sdkman/bin/sdkman-init.sh && cd guava && mvn -B -P!standard-with-extra-repos verify -U -Dmaven.javadoc.skip=true -DskipTests=true"
 
diff --git a/dd-java-agent/agent-debugger/exploration-tests/exclude.txt b/dd-java-agent/agent-debugger/exploration-tests/exclude.txt
diff --git a/dd-java-agent/agent-debugger/exploration-tests/exclude_jackson-databind.txt b/dd-java-agent/agent-debugger/exploration-tests/exclude_jackson-databind.txt
diff --git a/dd-java-agent/agent-debugger/exploration-tests/exclude_jsoup.txt b/dd-java-agent/agent-debugger/exploration-tests/exclude_jsoup.txt
diff --git a/dd-java-agent/agent-debugger/exploration-tests/include_jsoup.txt b/dd-java-agent/agent-debugger/exploration-tests/include_jsoup.txt
@@ -0,0 +1 @@
+org/jsoup/*
diff --git a/dd-java-agent/agent-debugger/exploration-tests/jsoup_exploration-tests.patch b/dd-java-agent/agent-debugger/exploration-tests/jsoup_exploration-tests.patch
@@ -0,0 +1,26 @@
+diff --git a/src/test/java/org/jsoup/parser/HtmlParserTest.java b/src/test/java/org/jsoup/parser/HtmlParserTest.java
+index a67003a8..1201d1af 100644
+--- a/src/test/java/org/jsoup/parser/HtmlParserTest.java
++++ b/src/test/java/org/jsoup/parser/HtmlParserTest.java
+@@ -1033,7 +1033,7 @@ public class HtmlParserTest {
+
+         // Assert
+         assertEquals(50000, doc.body().childNodeSize());
+-        assertTrue(System.currentTimeMillis() - start < 1000);
++        //assertTrue(System.currentTimeMillis() - start < 10000);
+     }
+
+     @Test
+diff --git a/src/test/java/org/jsoup/parser/ParserIT.java b/src/test/java/org/jsoup/parser/ParserIT.java
+index 54d757e7..467c10bc 100644
+--- a/src/test/java/org/jsoup/parser/ParserIT.java
++++ b/src/test/java/org/jsoup/parser/ParserIT.java
+@@ -48,7 +48,7 @@ public class ParserIT {
+         // Assert
+         assertEquals(2, doc.body().childNodeSize());
+         assertEquals(25000, doc.select("dd").size());
+-        assertTrue(System.currentTimeMillis() - start < 20000); // I get ~ 1.5 seconds, but others have reported slower
++        //assertTrue(System.currentTimeMillis() - start < 20000); // I get ~ 1.5 seconds, but others have reported slower
+         // was originally much longer, or stack overflow.
+     }
+ }
diff --git a/dd-java-agent/agent-debugger/exploration-tests/run-exploration-tests.sh b/dd-java-agent/agent-debugger/exploration-tests/run-exploration-tests.sh
@@ -2,9 +2,10 @@
 set -uo pipefail
 NAME=$1
 COMMAND=$2
-PROJECT_EXCLUDE_FILE=${3:-}
+PROJECT_INCLUDE_FILE=${3:-}
+PROJECT_EXCLUDE_FILE=${4:-}
 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=true -Ddd.dynamic.instrumentation.exclude.files=/exploration-tests/exclude.txt,/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=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"
 echo "$JAVA_TOOL_OPTIONS"
 cd $NAME
 echo "Building repository $NAME..."

diff --git a/...va-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java b/...va-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java
@@ -93,8 +93,12 @@ public class DebuggerTransformer implements ClassFileTransformer {
   private final InstrumentationListener listener;
   private final DebuggerSink debuggerSink;
   private final boolean instrumentTheWorld;
-  private final Set<String> excludeClasses = new HashSet<>();
-  private final Trie excludeTrie = new Trie();
+  private final Set<String> excludeClasses;
+  private final Set<String> excludeMethods;
+  private final Trie excludeTrie;
+  private final Set<String> includeClasses;
+  private final Set<String> includeMethods;
+  private final Trie includeTrie;
   private final Map<String, LogProbe> instrumentTheWorldProbes;
 
   public interface InstrumentationListener {
@@ -115,9 +119,24 @@ public DebuggerTransformer(
     this.instrumentTheWorld = config.isDebuggerInstrumentTheWorld();
     if (this.instrumentTheWorld) {
       instrumentTheWorldProbes = new ConcurrentHashMap<>();
-      readExcludeFiles(config.getDebuggerExcludeFiles());
+      excludeTrie = new Trie();
+      excludeClasses = new HashSet<>();
+      excludeMethods = new HashSet<>();
+      includeTrie = new Trie();
+      includeClasses = new HashSet<>();
+      includeMethods = new HashSet<>();
+      processITWFiles(
+          config.getDebuggerExcludeFiles(), excludeTrie, excludeClasses, excludeMethods);
+      processITWFiles(
+          config.getDebuggerIncludeFiles(), includeTrie, includeClasses, includeMethods);
     } else {
       instrumentTheWorldProbes = null;
+      excludeTrie = null;
+      excludeClasses = null;
+      excludeMethods = null;
+      includeTrie = null;
+      includeClasses = null;
+      includeMethods = null;
     }
   }
 
@@ -137,7 +156,8 @@ public DebuggerTransformer(Config config, Configuration configuration) {
             new SymbolSink(config)));
   }
 
-  private void readExcludeFiles(String commaSeparatedFileNames) {
+  private void processITWFiles(
+      String commaSeparatedFileNames, Trie prefixes, Set<String> classes, Set<String> methods) {
     if (commaSeparatedFileNames == null) {
       return;
     }
@@ -156,10 +176,14 @@ private void readExcludeFiles(String commaSeparatedFileNames) {
                     return;
                   }
                   if (line.endsWith("*")) {
-                    excludeTrie.insert(line.substring(0, line.length() - 1));
-                  } else {
-                    excludeClasses.add(line);
+                    prefixes.insert(line.substring(0, line.length() - 1));
+                    return;
+                  }
+                  if (line.contains("::")) {
+                    methods.add(line);
+                    return;
                   }
+                  classes.add(line);
                 });
       } catch (IOException ex) {
         log.warn("Error reading exclude file '{}' for Instrument-The-World: ", fileName, ex);
@@ -263,6 +287,9 @@ private byte[] transformTheWorld(
       if (isExcludedFromTransformation(classFilePath)) {
         return null;
       }
+      if (!isIncludedForTransformation(classFilePath)) {
+        return null;
+      }
       URL location = null;
       if (protectionDomain != null) {
         CodeSource codeSource = protectionDomain.getCodeSource();
@@ -277,15 +304,32 @@ private byte[] transformTheWorld(
           loader,
           location);
       ClassNode classNode = parseClassFile(classFilePath, classfileBuffer);
+      if (classNode.superName.equals("java/lang/ClassLoader")
+          || classNode.superName.equals("java/net/URLClassLoader")
+          || excludeClasses.contains(classNode.superName)) {
+        // Skip ClassLoader classes
+        log.debug("Skipping ClassLoader class: {}", classFilePath);
+        excludeClasses.add(classFilePath);
+        return null;
+      }
       List<ProbeDefinition> probes = new ArrayList<>();
       Set<String> methodNames = new HashSet<>();
       for (MethodNode methodNode : classNode.methods) {
+        if (methodNode.name.equals("<clinit>")) {
+          // skip static class initializer
+          continue;
+        }
+        String fqnMethod = classNode.name + "::" + methodNode.name;
+        if (excludeMethods.contains(fqnMethod)) {
+          log.debug("Skipping method: {}", fqnMethod);
+          continue;
+        }
         if (methodNames.add(methodNode.name)) {
           LogProbe probe =
               LogProbe.builder()
                   .probeId(UUID.randomUUID().toString(), 0)
                   .where(classNode.name, methodNode.name)
-                  .captureSnapshot(true)
+                  .captureSnapshot(false)
                   .build();
           probes.add(probe);
           instrumentTheWorldProbes.put(probe.getProbeId().getEncodedId(), probe);
@@ -294,6 +338,8 @@ private byte[] transformTheWorld(
       boolean transformed = performInstrumentation(loader, classFilePath, probes, classNode);
       if (transformed) {
         return writeClassFile(probes, loader, classFilePath, classNode);
+      } else {
+        log.debug("Class not transformed: {}", classFilePath);
       }
     } catch (Throwable ex) {
       log.warn("Cannot transform: ", ex);
@@ -328,6 +374,16 @@ private boolean isExcludedFromTransformation(String classFilePath) {
     return false;
   }
 
+  private boolean isIncludedForTransformation(String classFilePath) {
+    if (includeClasses.contains(classFilePath)) {
+      return true;
+    }
+    if (includeTrie.hasMatchingPrefix(classFilePath)) {
+      return true;
+    }
+    return false;
+  }
+
   private boolean instrumentationIsAllowed(
       String fullyQualifiedClassName, List<ProbeDefinition> definitions) {
     if (denyListHelper.isDenied(fullyQualifiedClassName)) {

diff --git a/...ugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java b/...ugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java
@@ -64,8 +64,11 @@
 import org.objectweb.asm.tree.TryCatchBlockNode;
 import org.objectweb.asm.tree.TypeInsnNode;
 import org.objectweb.asm.tree.VarInsnNode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class CapturedContextInstrumentor extends Instrumentor {
+  private static final Logger log = LoggerFactory.getLogger(CapturedContextInstrumentor.class);
   private final boolean captureSnapshot;
   private final Limits limits;
   private final LabelNode contextInitLabel = new LabelNode();
@@ -1167,7 +1170,12 @@ private static List<FieldNode> extractStaticFields(
         }
       }
     }
-    addInheritedStaticFields(classNode, classLoader, limits, results, fieldCount);
+    if (!Config.get().isDebuggerInstrumentTheWorld()) {
+      // 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
+      addInheritedStaticFields(classNode, classLoader, limits, results, fieldCount);
+    }
     return results;
   }
 
@@ -1185,17 +1193,22 @@ private static void addInheritedStaticFields(
       } catch (ClassNotFoundException ex) {
         break;
       }
-      for (Field field : clazz.getDeclaredFields()) {
-        if (isStaticField(field) && !isFinalField(field)) {
-          String desc = Type.getDescriptor(field.getType());
-          FieldNode fieldNode =
-              new FieldNode(field.getModifiers(), field.getName(), desc, null, field);
-          results.add(fieldNode);
-          fieldCount++;
-          if (fieldCount > limits.maxFieldCount) {
-            return;
+      try {
+        for (Field field : clazz.getDeclaredFields()) {
+          if (isStaticField(field) && !isFinalField(field)) {
+            String desc = Type.getDescriptor(field.getType());
+            FieldNode fieldNode =
+                new FieldNode(field.getModifiers(), field.getName(), desc, null, field);
+            results.add(fieldNode);
+            log.debug("Adding static inherited field {}", fieldNode.name);
+            fieldCount++;
+            if (fieldCount > limits.maxFieldCount) {
+              return;
+            }
           }
         }
+      } catch (ClassCircularityError ex) {
+        break;
       }
       clazz = clazz.getSuperclass();
       superClassName = clazz.getTypeName();