diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/ClassLoaderMatchers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/ClassLoaderMatchers.java index 954354880fd..2ae8cb21921 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/ClassLoaderMatchers.java +++ b/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) { diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatchersTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatchersTest.groovy index e161735d486..39cdc2f7d16 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatchersTest.groovy +++ b/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 */ diff --git a/dd-java-agent/instrumentation/drools/drools-6.0/build.gradle b/dd-java-agent/instrumentation/drools/drools-6.0/build.gradle new file mode 100644 index 00000000000..b1006d1450f --- /dev/null +++ b/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", + ] + ) + } +} diff --git a/dd-java-agent/instrumentation/drools/drools-6.0/src/test/groovy/datadog/trace/instrumentation/drools/DroolsClassLoaderExclusionTest.groovy b/dd-java-agent/instrumentation/drools/drools-6.0/src/test/groovy/datadog/trace/instrumentation/drools/DroolsClassLoaderExclusionTest.groovy new file mode 100644 index 00000000000..94000ac0772 --- /dev/null +++ b/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() + } +} + diff --git a/dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/example/Person.java b/dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/example/Person.java new file mode 100644 index 00000000000..08346b1a234 --- /dev/null +++ b/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 + "}"; + } +} diff --git a/dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/test/ConstructorAdvice.java b/dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/test/ConstructorAdvice.java new file mode 100644 index 00000000000..0a44404f343 --- /dev/null +++ b/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() {} +} diff --git a/dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/test/RuleInstrumentation.java b/dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/test/RuleInstrumentation.java new file mode 100644 index 00000000000..1241e84f079 --- /dev/null +++ b/dd-java-agent/instrumentation/drools/drools-6.0/src/test/java/test/RuleInstrumentation.java @@ -0,0 +1,34 @@ +package test; + +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumenterModule.class) +public class RuleInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { + + public RuleInstrumentation() { + super("drools-test"); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice(isConstructor(), "test.ConstructorAdvice"); + } + + @Override + public String hierarchyMarkerType() { + return null; + } + + @Override + public ElementMatcher hierarchyMatcher() { + return NameMatchers.nameStartsWith("example.Rule"); + } +} diff --git a/dd-java-agent/instrumentation/drools/drools-6.0/src/test/resources/example/rules.drl b/dd-java-agent/instrumentation/drools/drools-6.0/src/test/resources/example/rules.drl new file mode 100644 index 00000000000..ef54b5c2c51 --- /dev/null +++ b/dd-java-agent/instrumentation/drools/drools-6.0/src/test/resources/example/rules.drl @@ -0,0 +1,12 @@ +package example + +import example.Person + +rule "Mark person as adult" +when + $p : Person(age >= 18, adult == false) +then + $p.setAdult(true); + System.out.println("Rule fired for " + $p.getName()); + update($p); +end diff --git a/settings.gradle.kts b/settings.gradle.kts index 4710bcfbb5d..6d3d15d16f1 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -335,6 +335,7 @@ include( ":dd-java-agent:instrumentation:datastax-cassandra:datastax-cassandra-4.0", ":dd-java-agent:instrumentation:dropwizard:dropwizard-views-0.7", ":dd-java-agent:instrumentation:dropwizard:dropwizard-0.8", + ":dd-java-agent:instrumentation:drools:drools-6.0", ":dd-java-agent:instrumentation:elasticsearch:elasticsearch-rest:elasticsearch-rest-5.0", ":dd-java-agent:instrumentation:elasticsearch:elasticsearch-rest:elasticsearch-rest-6.4", ":dd-java-agent:instrumentation:elasticsearch:elasticsearch-rest:elasticsearch-rest-7.0",