Merge branch 'master' into alexeyk/test-base-windows

Author: AlexeyKuznetsov-DD

.github/workflows/run-system-tests.yaml

@@ -75,7 +75,7 @@ jobs:
       desired_execution_time: 900  # 15 minutes
       scenarios_groups: tracer-release
       excluded_scenarios: APM_TRACING_E2E_OTEL,APM_TRACING_E2E_SINGLE_SPAN,PROFILING  # exclude flaky scenarios
-      skip_empty_scenarios: true
+      skip_empty_scenarios: ${{ github.event_name != 'push' && github.event_name != 'schedule' }}
       push_to_test_optimization: true
     secrets:
       DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}

communication/src/main/java/datadog/communication/ddagent/DDAgentFeaturesDiscovery.java

@@ -22,7 +22,6 @@
 import datadog.trace.api.telemetry.LogCollector;
 import datadog.trace.util.Strings;
 import java.nio.ByteBuffer;
-import java.security.NoSuchAlgorithmException;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -311,7 +310,7 @@ private boolean processInfoResponse(State newState, String response) {
       }
       try {
         newState.state = Strings.sha256(response);
-      } catch (NoSuchAlgorithmException ex) {
+      } catch (Throwable ex) {
         log.debug(
             "Failed to hash trace agent /info response. Will probe {}", newState.traceEndpoint, ex);
       }

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

@@ -59,6 +59,7 @@ private void populateDefinitionFileNamesMap(Collection<ProbeDefinition> definiti
       if (fileName == null) {
         continue;
       }
+      fileName = normalizeWindowsToUnixPath(fileName);
       Map<String, List<ProbeDefinition>> targetMap =
           fileName.indexOf('/') != -1
               ? definitionsByQualifiedFileNames
@@ -67,6 +68,10 @@ private void populateDefinitionFileNamesMap(Collection<ProbeDefinition> definiti
     }
   }
 
+  private static String normalizeWindowsToUnixPath(String fileName) {
+    return fileName.replace('\\', '/');
+  }
+
   private Trie buildDefinitionFileNamesTrie(
       Map<String, List<ProbeDefinition>> definitionsByFileNames) {
     Trie resultTrie = new Trie();

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

@@ -74,6 +74,26 @@ public void sourceFileAbsoluteFileName() {
     assertEquals(PROBE_ID1, probeDefinitions.get(0).getProbeId());
   }
 
+  @Test
+  public void sourceFileWindowsStyleFileName() {
+    LogProbe probe = createProbe(PROBE_ID1, "src\\main\\java\\java\\lang\\String.java", 23);
+    TransformerDefinitionMatcher matcher = createMatcher(probe);
+    List<ProbeDefinition> probeDefinitions = match(matcher, String.class);
+    assertEquals(1, probeDefinitions.size());
+    assertEquals(PROBE_ID1, probeDefinitions.get(0).getProbeId());
+  }
+
+  @Test
+  public void sourceFileWindowsStyleAbsoluteFileName() {
+    LogProbe probe =
+        createProbe(
+            PROBE_ID1, "C:\\Users\\user\\project\\src\\main\\java\\java\\lang\\String.java", 23);
+    TransformerDefinitionMatcher matcher = createMatcher(probe);
+    List<ProbeDefinition> probeDefinitions = match(matcher, String.class);
+    assertEquals(1, probeDefinitions.size());
+    assertEquals(PROBE_ID1, probeDefinitions.get(0).getProbeId());
+  }
+
   @Test
   public void sourceFileSimpleFileName() {
     LogProbe probe = createProbe(PROBE_ID1, "String.java", 23);

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/ClassLoaderMatchers.java

@@ -54,6 +54,10 @@ public static boolean canSkipClassLoaderByName(final ClassLoader loader) {
       case "com.alibaba.fastjson.util.ASMClassLoader":
       case "datadog.trace.bootstrap.DatadogClassLoader":
       case "datadog.trace.bootstrap.InstrumentationClassLoader":
+      // drools
+      case "org.drools.core.rule.PackageClassLoader":
+      case "org.drools.wiring.dynamic.PackageClassLoader":
+      case "org.drools.core.rule.JavaDialectRuntimeData$PackageClassLoader":
         return true;
     }
     if (CHECK_EXCLUDES) {

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatchersTest.groovy

@@ -5,6 +5,9 @@ import datadog.trace.bootstrap.instrumentation.log.LogContextScopeListener
 import datadog.trace.bootstrap.DatadogClassLoader
 import datadog.trace.test.util.DDSpecification
 import groovy.transform.CompileStatic
+import net.bytebuddy.ByteBuddy
+import net.bytebuddy.dynamic.loading.ClassLoadingStrategy
+import spock.lang.Unroll
 
 class ClassLoaderMatchersTest extends DDSpecification {
 
@@ -44,6 +47,29 @@ class ClassLoaderMatchersTest extends DDSpecification {
     LogContextScopeListener.name == "datadog.trace.bootstrap.instrumentation.log.LogContextScopeListener"
   }
 
+  @Unroll
+  def "skips drools classloader: #loaderName"() {
+    given:
+    ClassLoader loader = new ByteBuddy()
+      .subclass(ClassLoader)
+      .name(loaderName)
+      .make()
+      .load(ClassLoader.getSystemClassLoader(), ClassLoadingStrategy.Default.WRAPPER)
+      .getLoaded()
+      .getDeclaredConstructor(ClassLoader)
+      .newInstance(ClassLoader.getSystemClassLoader())
+
+    expect:
+    ClassLoaderMatchers.canSkipClassLoaderByName(loader)
+
+    where:
+    loaderName << [
+      "org.drools.core.rule.PackageClassLoader",
+      "org.drools.wiring.dynamic.PackageClassLoader",
+      "org.drools.core.rule.JavaDialectRuntimeData\$PackageClassLoader"
+    ]
+  }
+
   /*
    * A URLClassloader which only delegates java.* classes
    */

dd-java-agent/instrumentation/drools/drools-6.0/build.gradle

@@ -0,0 +1,80 @@
+apply from: "$rootDir/gradle/java.gradle"
+
+addTestSuiteForDir("latest7Test", "test")
+addTestSuiteForDir("latest8Test", "test")
+addTestSuiteForDir("latestDepTest", "test")
+
+testJvmConstraints {
+}
+repositories {
+  maven {
+    url = uri("https://repository.jboss.org/nexus/content/repositories/releases/")
+  }
+}
+dependencies {
+  testImplementation project(':dd-java-agent:testing')
+  testImplementation group: 'org.drools', name: 'drools-compiler', version: '6.0.0.Final'
+  testImplementation group: 'org.drools', name: 'drools-core', version: '6.0.0.Final'
+  testImplementation group: 'org.kie', name: 'kie-api', version: '6.0.0.Final'
+
+  latest7TestImplementation group: 'org.drools', name: 'drools-compiler', version: '7.+'
+  latest7TestImplementation group: 'org.drools', name: 'drools-core', version: '7.+'
+  latest7TestImplementation group: 'org.drools', name: 'drools-mvel', version: '7.+'
+  latest7TestImplementation group: 'org.kie', name: 'kie-api', version: '7.+'
+
+  latest8TestImplementation group: 'org.drools', name: 'drools-compiler', version: '8.+'
+  latest8TestImplementation group: 'org.drools', name: 'drools-core', version: '8.+'
+  latest8TestImplementation group: 'org.drools', name: 'drools-mvel', version: '8.+'
+  latest8TestImplementation group: 'org.kie', name: 'kie-api', version: '8.+'
+
+  // for now this is locked to 10.x not to add maintenance burden
+  latestDepTestImplementation group: 'org.drools', name: 'drools-compiler', version: '10.+'
+  latestDepTestImplementation group: 'org.drools', name: 'drools-core', version: '10.+'
+  latestDepTestImplementation group: 'org.drools', name: 'drools-mvel', version: '10.+'
+  latestDepTestImplementation group: 'org.kie', name: 'kie-api', version: '10.+'
+}
+
+configurations.matching { it.name.startsWith('latestDepTest') }.configureEach {
+  it.resolutionStrategy {
+    force group: 'org.slf4j', name: 'slf4j-api', version: libs.versions.slf4j.get()
+  }
+}
+
+tasks.named("test") {
+  testJvmConstraints {
+    maxJavaVersion = JavaVersion.VERSION_1_8
+  }
+}
+
+tasks.named("latest7Test") {
+  testJvmConstraints {
+    maxJavaVersion = JavaVersion.VERSION_11
+  }
+}
+
+tasks.named("latest8Test") {
+  testJvmConstraints {
+    minJavaVersion = JavaVersion.VERSION_11
+    maxJavaVersion = JavaVersion.VERSION_17
+  }
+}
+
+tasks.named("latestDepTest") {
+  testJvmConstraints {
+    minJavaVersion = JavaVersion.VERSION_17
+  }
+}
+
+project.afterEvaluate {
+  tasks.withType(Test).configureEach {
+    conditionalJvmArgs(
+      it,
+      JavaVersion.VERSION_16,
+      [
+        "--add-opens=java.base/java.lang.reflect=ALL-UNNAMED",
+        "--add-opens=java.base/java.text=ALL-UNNAMED",
+        "--add-opens=java.desktop/java.awt.font=ALL-UNNAMED",
+      ]
+      )
+  }
+}

dd-java-agent/instrumentation/drools/drools-6.0/src/test/groovy/datadog/trace/instrumentation/drools/DroolsClassLoaderExclusionTest.groovy

@@ -0,0 +1,62 @@
+package datadog.trace.instrumentation.drools
+
+import datadog.trace.agent.test.InstrumentationSpecification
+import datadog.trace.config.inversion.ConfigHelper
+import example.Person
+import org.kie.api.KieServices
+import org.kie.api.builder.KieFileSystem
+import org.kie.api.runtime.KieContainer
+import org.kie.api.runtime.KieSession
+
+class DroolsClassLoaderExclusionTest extends InstrumentationSpecification {
+  static ConfigHelper.StrictnessPolicy strictness
+
+  @Override
+  protected void configurePreAgent() {
+    // this is required otherwise checks will fail...
+    strictness = ConfigHelper.get().configInversionStrictFlag()
+    ConfigHelper.get().setConfigInversionStrict(ConfigHelper.StrictnessPolicy.TEST)
+    super.configurePreAgent()
+  }
+
+  def cleanup() {
+    ConfigHelper.get().setConfigInversionStrict(strictness)
+  }
+
+  def "should not instrument drools generated classes"() {
+    setup:
+    KieServices ks = KieServices.Factory.get()
+
+    KieFileSystem kfs = ks.newKieFileSystem()
+
+    kfs.write(
+      "src/main/resources/example/rules.drl",
+      ks.getResources().newClassPathResource("example/rules.drl")
+      )
+
+    ks.newKieBuilder(kfs).buildAll()
+    KieContainer kc = ks.newKieContainer(ks.getRepository().getDefaultReleaseId())
+    KieSession ksession = kc.newKieSession()
+
+    when:
+    Person john = new Person("John", 20)
+    Person bob = new Person("Bob", 15)
+
+    ksession.insert(john)
+    ksession.insert(bob)
+    int fired = ksession.fireAllRules()
+
+    then:
+    fired == 1
+    !bob.isAdult()
+    john.isAdult()
+
+    and:
+    // assert we do not transform the generated rule class (RuleInstrumentation would but the classloader should be ignored)
+    TRANSFORMED_CLASSES_TYPES.findAll { it.getName().startsWith("example.") }.isEmpty()
+
+    cleanup:
+    ksession?.dispose()
+  }
+}
+

dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/example/Person.java

@@ -0,0 +1,44 @@
+package example;
+
+public class Person {
+  private String name;
+  private int age;
+  private boolean adult;
+
+  public Person() {}
+
+  public Person(String name, int age) {
+    this.name = name;
+    this.age = age;
+    this.adult = false;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
+
+  public int getAge() {
+    return age;
+  }
+
+  public void setAge(int age) {
+    this.age = age;
+  }
+
+  public boolean isAdult() {
+    return adult;
+  }
+
+  public void setAdult(boolean adult) {
+    this.adult = adult;
+  }
+
+  @Override
+  public String toString() {
+    return "Person{name='" + name + "', age=" + age + ", adult=" + adult + "}";
+  }
+}

dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/test/ConstructorAdvice.java

@@ -0,0 +1,8 @@
+package test;
+
+import net.bytebuddy.asm.Advice;
+
+public class ConstructorAdvice {
+  @Advice.OnMethodExit(suppress = Throwable.class)
+  public static void onExit() {}
+}