From a5faf500b00f2cad9aa423b8a536a6397f0f49b7 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Mon, 13 Jan 2025 12:05:27 +0100 Subject: [PATCH 1/3] Mule: do not crash with JPMS (cherry picked from commit 0a7bf86f4cb19d5055cde14d54f22daf14a58e8b) --- .../instrumentation/mule-4/build.gradle | 29 ++++++++++++- .../mule4/JpmsMuleInstrumentation.java | 43 +++++++++++++++++++ .../mule4/JpmsAdvisingHelper.java | 13 ++++++ .../mule4/JpmsClearanceAdvice.java | 20 +++++++++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java create mode 100644 dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java create mode 100644 dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java diff --git a/dd-java-agent/instrumentation/mule-4/build.gradle b/dd-java-agent/instrumentation/mule-4/build.gradle index 70ea3c665d3..ead93c8cc67 100644 --- a/dd-java-agent/instrumentation/mule-4/build.gradle +++ b/dd-java-agent/instrumentation/mule-4/build.gradle @@ -43,6 +43,7 @@ muzzle { } apply from: "$rootDir/gradle/java.gradle" +apply plugin: "idea" addTestSuiteForDir('mule46ForkedTest', 'test') addTestSuiteForDir('latestDepForkedTest', 'test') @@ -81,6 +82,9 @@ configurations.all { } sourceSets { + main_java11 { + java.srcDirs "${project.projectDir}/src/main/java11" + } test { output.dir("$buildDir/generated-resources/test", builtBy: 'generateAppResources') } @@ -92,6 +96,20 @@ sourceSets { } } +compileMain_java11Java.configure { + setJavaVersion(it, 11) + sourceCompatibility = JavaVersion.VERSION_1_8 + targetCompatibility = JavaVersion.VERSION_1_8 +} + +jar { + from sourceSets.main_java11.output +} + +forbiddenApisMain_java11 { + failOnMissingClasses = false +} + tasks.named("compileTestGroovy").configure { dependsOn 'mvnPackage', 'extractMuleServices' } @@ -112,7 +130,10 @@ tasks.named("compileLatestDepForkedTestJava").configure { dependencies { compileOnly group: 'org.mule.runtime', name: 'mule-core', version: muleVersion compileOnly group: 'org.mule.runtime', name: 'mule-tracer-customization-impl', version: muleVersion - + main_java11CompileOnly project(':internal-api') + main_java11CompileOnly project(':dd-java-agent:agent-tooling') + main_java11CompileOnly project(':dd-java-agent:agent-bootstrap') + main_java11CompileOnly sourceSets.main.output testImplementation project(':dd-java-agent:instrumentation:aws-common') testImplementation project(':dd-java-agent:instrumentation:reactor-core-3.1') testImplementation project(':dd-java-agent:instrumentation:reactive-streams') @@ -234,3 +255,9 @@ spotless { target "**/*.java" } } + +idea { + module { + jdkName = '11' + } +} diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java b/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java new file mode 100644 index 00000000000..4e2391dc510 --- /dev/null +++ b/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java @@ -0,0 +1,43 @@ +package datadog.trace.instrumentation.mule4; + +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.api.Platform; + +@AutoService(InstrumenterModule.class) +public class JpmsMuleInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.HasMethodAdvice, Instrumenter.ForKnownTypes { + public JpmsMuleInstrumentation() { + super("mule", "mule-jpms"); + } + + @Override + public boolean isEnabled() { + return super.isEnabled() && Platform.isJavaVersionAtLeast(9); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + // same module but they can be initialized in any order + "org.mule.runtime.tracer.customization.impl.info.ExecutionInitialSpanInfo", + "org.mule.runtime.tracer.customization.impl.provider.LazyInitialSpanInfo", + }; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".JpmsAdvisingHelper", + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + // it does not work with typeInitializer() + transformer.applyAdvice(isConstructor(), packageName + ".JpmsClearanceAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java new file mode 100644 index 00000000000..aee99b4fae5 --- /dev/null +++ b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java @@ -0,0 +1,13 @@ +package datadog.trace.instrumentation.mule4; + +import java.util.WeakHashMap; + +public class JpmsAdvisingHelper { + private static final WeakHashMap ALREADY_PROCESSED_CACHE = new WeakHashMap<>(); + + public static boolean isModuleAlreadyProcessed(final Module module) { + return Boolean.TRUE.equals(ALREADY_PROCESSED_CACHE.putIfAbsent(module, Boolean.TRUE)); + } + + private JpmsAdvisingHelper() {} +} diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java new file mode 100644 index 00000000000..684b9ccff3b --- /dev/null +++ b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java @@ -0,0 +1,20 @@ +package datadog.trace.instrumentation.mule4; + +import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; + +public class JpmsClearanceAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void openOnReturn(@Advice.This(typing = Assigner.Typing.DYNAMIC) Object self) { + final Module module = self.getClass().getModule(); + if (module == null || JpmsAdvisingHelper.isModuleAlreadyProcessed(module)) { + return; + } + for (String pn : module.getPackages()) { + try { + module.addExports(pn, module.getClassLoader().getUnnamedModule()); + } catch (Throwable ignored) { + } + } + } +} From 57f258db4ce258003269903545d2979b9c2791e3 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Mon, 13 Jan 2025 13:39:50 +0100 Subject: [PATCH 2/3] muzzle (cherry picked from commit 4af59aacb517849f4fd8c8df7f891e67e459fbba) --- .../instrumentation/mule-4/build.gradle | 17 +++++++++++----- .../mule4/JpmsMuleInstrumentation.java | 17 +++++++++++++++- .../mule4/JpmsAdvisingHelper.java | 15 +++++++++++--- .../mule4/JpmsClearanceAdvice.java | 20 ------------------- 4 files changed, 40 insertions(+), 29 deletions(-) delete mode 100644 dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java diff --git a/dd-java-agent/instrumentation/mule-4/build.gradle b/dd-java-agent/instrumentation/mule-4/build.gradle index ead93c8cc67..5f347448893 100644 --- a/dd-java-agent/instrumentation/mule-4/build.gradle +++ b/dd-java-agent/instrumentation/mule-4/build.gradle @@ -35,11 +35,19 @@ muzzle { module = 'mule-core' versions = "[$muleVersion,)" javaVersion = "17" - assertInverse true excludeDependency 'om.google.guava:guava' excludeDependency 'com.google.code.findbugs:jsr305' additionalDependencies +="org.mule.runtime:mule-tracer-customization-impl:$muleVersion" } + + fail { + name = 'before-4.5.0' + group = 'org.mule.runtime' + module = 'mule-core' + versions = "[, $muleVersion)" + excludeDependency 'om.google.guava:guava' + excludeDependency 'com.google.code.findbugs:jsr305' + } } apply from: "$rootDir/gradle/java.gradle" @@ -130,10 +138,9 @@ tasks.named("compileLatestDepForkedTestJava").configure { dependencies { compileOnly group: 'org.mule.runtime', name: 'mule-core', version: muleVersion compileOnly group: 'org.mule.runtime', name: 'mule-tracer-customization-impl', version: muleVersion - main_java11CompileOnly project(':internal-api') - main_java11CompileOnly project(':dd-java-agent:agent-tooling') - main_java11CompileOnly project(':dd-java-agent:agent-bootstrap') - main_java11CompileOnly sourceSets.main.output + compileOnly sourceSets.main_java11.output + testImplementation sourceSets.main_java11.output + testImplementation project(':dd-java-agent:instrumentation:aws-common') testImplementation project(':dd-java-agent:instrumentation:reactor-core-3.1') testImplementation project(':dd-java-agent:instrumentation:reactive-streams') diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java b/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java index 4e2391dc510..b079cc2f722 100644 --- a/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java +++ b/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java @@ -6,6 +6,9 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.api.Platform; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; +import org.mule.runtime.tracer.api.EventTracer; @AutoService(InstrumenterModule.class) public class JpmsMuleInstrumentation extends InstrumenterModule.Tracing @@ -38,6 +41,18 @@ public String[] helperClassNames() { @Override public void methodAdvice(MethodTransformer transformer) { // it does not work with typeInitializer() - transformer.applyAdvice(isConstructor(), packageName + ".JpmsClearanceAdvice"); + transformer.applyAdvice(isConstructor(), getClass().getName() + "$JpmsClearanceAdvice"); + } + + public static class JpmsClearanceAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void openOnReturn(@Advice.This(typing = Assigner.Typing.DYNAMIC) Object self) { + JpmsAdvisingHelper.allowAccessOnModuleClass(self.getClass()); + } + + private static void muzzleCheck(final EventTracer tracer) { + // introduced in 4.5.0 + tracer.endCurrentSpan(null); + } } } diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java index aee99b4fae5..a275ae54bb9 100644 --- a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java +++ b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java @@ -3,10 +3,19 @@ import java.util.WeakHashMap; public class JpmsAdvisingHelper { - private static final WeakHashMap ALREADY_PROCESSED_CACHE = new WeakHashMap<>(); + private static final WeakHashMap, Boolean> ALREADY_PROCESSED_CACHE = new WeakHashMap<>(); - public static boolean isModuleAlreadyProcessed(final Module module) { - return Boolean.TRUE.equals(ALREADY_PROCESSED_CACHE.putIfAbsent(module, Boolean.TRUE)); + public static void allowAccessOnModuleClass(final Class cls) { + if (Boolean.TRUE.equals(ALREADY_PROCESSED_CACHE.putIfAbsent(cls, Boolean.TRUE))) { + return; + } + final Module module = cls.getModule(); + if (module != null) { + try { + module.addExports(cls.getPackageName(), module.getClassLoader().getUnnamedModule()); + } catch (Throwable ignored) { + } + } } private JpmsAdvisingHelper() {} diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java deleted file mode 100644 index 684b9ccff3b..00000000000 --- a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java +++ /dev/null @@ -1,20 +0,0 @@ -package datadog.trace.instrumentation.mule4; - -import net.bytebuddy.asm.Advice; -import net.bytebuddy.implementation.bytecode.assign.Assigner; - -public class JpmsClearanceAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - public static void openOnReturn(@Advice.This(typing = Assigner.Typing.DYNAMIC) Object self) { - final Module module = self.getClass().getModule(); - if (module == null || JpmsAdvisingHelper.isModuleAlreadyProcessed(module)) { - return; - } - for (String pn : module.getPackages()) { - try { - module.addExports(pn, module.getClassLoader().getUnnamedModule()); - } catch (Throwable ignored) { - } - } - } -} From 47a521144623ec5ec6f54507bdec0826a3781820 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 14 Jan 2025 12:02:49 +0100 Subject: [PATCH 3/3] Applying suggestions and use ClassValue (cherry picked from commit 817c5ed1e7ea6ab183dffdf2543f61ff19271e4b) --- .../instrumentation/mule-4/build.gradle | 6 +++- .../mule4/JpmsMuleInstrumentation.java | 33 ++++++++++--------- .../mule4/JpmsAdvisingHelper.java | 19 +++-------- .../mule4/JpmsClearanceAdvice.java | 26 +++++++++++++++ 4 files changed, 52 insertions(+), 32 deletions(-) create mode 100644 dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java diff --git a/dd-java-agent/instrumentation/mule-4/build.gradle b/dd-java-agent/instrumentation/mule-4/build.gradle index 5f347448893..e1f19f311c8 100644 --- a/dd-java-agent/instrumentation/mule-4/build.gradle +++ b/dd-java-agent/instrumentation/mule-4/build.gradle @@ -139,8 +139,12 @@ dependencies { compileOnly group: 'org.mule.runtime', name: 'mule-core', version: muleVersion compileOnly group: 'org.mule.runtime', name: 'mule-tracer-customization-impl', version: muleVersion compileOnly sourceSets.main_java11.output - testImplementation sourceSets.main_java11.output + main_java11CompileOnly project(':internal-api') + main_java11CompileOnly project(':dd-java-agent:agent-tooling') + main_java11CompileOnly project(':dd-java-agent:agent-bootstrap') + + testImplementation sourceSets.main_java11.output testImplementation project(':dd-java-agent:instrumentation:aws-common') testImplementation project(':dd-java-agent:instrumentation:reactor-core-3.1') testImplementation project(':dd-java-agent:instrumentation:reactive-streams') diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java b/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java index b079cc2f722..c915a1a393d 100644 --- a/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java +++ b/dd-java-agent/instrumentation/mule-4/src/main/java/datadog/trace/instrumentation/mule4/JpmsMuleInstrumentation.java @@ -5,10 +5,8 @@ import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.agent.tooling.muzzle.Reference; import datadog.trace.api.Platform; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.implementation.bytecode.assign.Assigner; -import org.mule.runtime.tracer.api.EventTracer; @AutoService(InstrumenterModule.class) public class JpmsMuleInstrumentation extends InstrumenterModule.Tracing @@ -39,20 +37,23 @@ public String[] helperClassNames() { } @Override - public void methodAdvice(MethodTransformer transformer) { - // it does not work with typeInitializer() - transformer.applyAdvice(isConstructor(), getClass().getName() + "$JpmsClearanceAdvice"); + public Reference[] additionalMuzzleReferences() { + return new Reference[] { + // added in 4.5.0 + new Reference.Builder("org.mule.runtime.tracer.api.EventTracer") + .withMethod( + new String[0], + Reference.EXPECTS_NON_STATIC | Reference.EXPECTS_PUBLIC, + "endCurrentSpan", + "V", + "Lorg/mule/runtime/api/event/Event;") + .build(), + }; } - public static class JpmsClearanceAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - public static void openOnReturn(@Advice.This(typing = Assigner.Typing.DYNAMIC) Object self) { - JpmsAdvisingHelper.allowAccessOnModuleClass(self.getClass()); - } - - private static void muzzleCheck(final EventTracer tracer) { - // introduced in 4.5.0 - tracer.endCurrentSpan(null); - } + @Override + public void methodAdvice(MethodTransformer transformer) { + // it does not work with typeInitializer() + transformer.applyAdvice(isConstructor(), packageName + ".JpmsClearanceAdvice"); } } diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java index a275ae54bb9..22313a64889 100644 --- a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java +++ b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsAdvisingHelper.java @@ -1,22 +1,11 @@ package datadog.trace.instrumentation.mule4; -import java.util.WeakHashMap; +import datadog.trace.api.GenericClassValue; +import java.util.concurrent.atomic.AtomicBoolean; public class JpmsAdvisingHelper { - private static final WeakHashMap, Boolean> ALREADY_PROCESSED_CACHE = new WeakHashMap<>(); - - public static void allowAccessOnModuleClass(final Class cls) { - if (Boolean.TRUE.equals(ALREADY_PROCESSED_CACHE.putIfAbsent(cls, Boolean.TRUE))) { - return; - } - final Module module = cls.getModule(); - if (module != null) { - try { - module.addExports(cls.getPackageName(), module.getClassLoader().getUnnamedModule()); - } catch (Throwable ignored) { - } - } - } + public static final ClassValue ALREADY_PROCESSED_CACHE = + GenericClassValue.constructing(AtomicBoolean.class); private JpmsAdvisingHelper() {} } diff --git a/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java new file mode 100644 index 00000000000..a479ae4ec4c --- /dev/null +++ b/dd-java-agent/instrumentation/mule-4/src/main/java11/datadog/trace/instrumentation/mule4/JpmsClearanceAdvice.java @@ -0,0 +1,26 @@ +package datadog.trace.instrumentation.mule4; + +import static datadog.trace.instrumentation.mule4.JpmsAdvisingHelper.ALREADY_PROCESSED_CACHE; + +import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; + +public class JpmsClearanceAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void openOnReturn(@Advice.This(typing = Assigner.Typing.DYNAMIC) Object self) { + final Class cls = self.getClass(); + if (ALREADY_PROCESSED_CACHE.get(cls).compareAndSet(false, true)) { + final Module module = cls.getModule(); + if (module != null) { + try { + // This call needs imperatively to be done from the same module we're adding exports + // because the jdk is checking that the caller belongs to the same module. + // The code of this advice is getting inlined into the constructor of the class belonging + // to that package so it will work. Moving the same to a helper won't. + module.addExports(cls.getPackageName(), module.getClassLoader().getUnnamedModule()); + } catch (Throwable ignored) { + } + } + } + } +}