diff --git a/buildSrc/src/main/kotlin/datadog/gradle/plugin/instrument/BuildTimeInstrumentationPlugin.kt b/buildSrc/src/main/kotlin/datadog/gradle/plugin/instrument/BuildTimeInstrumentationPlugin.kt index e89a7f13ea0..44c954ac4a9 100644 --- a/buildSrc/src/main/kotlin/datadog/gradle/plugin/instrument/BuildTimeInstrumentationPlugin.kt +++ b/buildSrc/src/main/kotlin/datadog/gradle/plugin/instrument/BuildTimeInstrumentationPlugin.kt @@ -41,7 +41,8 @@ import org.gradle.kotlin.dsl.withType * * Requirements for ByteBuddy plugins: * 1. Must implement [net.bytebuddy.build.Plugin] - * 2. Must have a constructor accepting a [java.io.File] parameter (target directory) + * 2. Must have a constructor accepting a [java.io.File] (target directory), or a two-`File` + * `(sourceDirectory, targetDirectory)` constructor when the plugin also needs the source folder * 3. Plugin classes must be available on `buildTimeInstrumentationPlugin` configuration * * @see BuildTimeInstrumentationExtension diff --git a/buildSrc/src/main/kotlin/datadog/gradle/plugin/instrument/ByteBuddyInstrumenter.kt b/buildSrc/src/main/kotlin/datadog/gradle/plugin/instrument/ByteBuddyInstrumenter.kt index 576e0b01c2a..0035ae84a1a 100644 --- a/buildSrc/src/main/kotlin/datadog/gradle/plugin/instrument/ByteBuddyInstrumenter.kt +++ b/buildSrc/src/main/kotlin/datadog/gradle/plugin/instrument/ByteBuddyInstrumenter.kt @@ -33,7 +33,14 @@ object ByteBuddyInstrumenter { val factories = plugins.map { try { val pluginClass = instrumentingLoader.loadClass(it).asSubclass(Plugin::class.java) - val loadedPlugin = pluginClass.getConstructor(File::class.java).newInstance(targetDirectory) + // Fall back to just the targetDirectory instance if no sourceDirectory is provided. + // SourceDirectory is needed to know whether the subproject compiled a given class (signal for injecting it as a helper). + val loadedPlugin = try { + pluginClass.getConstructor(File::class.java, File::class.java) + .newInstance(sourceDirectory, targetDirectory) + } catch (_: NoSuchMethodException) { + pluginClass.getConstructor(File::class.java).newInstance(targetDirectory) + } Plugin.Factory.Simple(loadedPlugin) } catch (throwable: Throwable) { throw IllegalStateException("Cannot resolve plugin: $it", throwable) diff --git a/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/CombiningTransformerBuilder.java b/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/CombiningTransformerBuilder.java index 5a7e4b9c3df..415d4886503 100644 --- a/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/CombiningTransformerBuilder.java +++ b/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/CombiningTransformerBuilder.java @@ -130,7 +130,12 @@ private void prepareInstrumentation(InstrumenterModule module, int instrumentati adviceShader = AdviceShader.with(module); - String[] helperClassNames = module.helperClassNames(); + String[] helperClassNames = + InstrumenterModule.loadStaticMuzzleHelperClassNames( + Utils.getExtendedClassLoader(), module.getClass().getName()); + if (null == helperClassNames) { + helperClassNames = module.helperClassNames(); + } if (module.injectHelperDependencies()) { helperClassNames = HelperScanner.withClassDependencies(helperClassNames); } diff --git a/dd-java-agent/agent-tooling/build.gradle b/dd-java-agent/agent-tooling/build.gradle index c32d4824c34..8430e878af0 100644 --- a/dd-java-agent/agent-tooling/build.gradle +++ b/dd-java-agent/agent-tooling/build.gradle @@ -49,6 +49,7 @@ dependencies { testImplementation project(':dd-java-agent:testing') testImplementation libs.bytebuddy + testImplementation libs.bundles.junit5 testImplementation group: 'com.google.guava', name: 'guava-testlib', version: '20.0' jmhImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '2.3.5.RELEASE' diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperScanner.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperScanner.java index 3ec0fe7c82e..5cad8113ebf 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperScanner.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperScanner.java @@ -24,8 +24,7 @@ public final class HelperScanner extends ClassVisitor { static final int READER_OPTIONS = ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES; - static final ClassFileLocator locator = - ClassFileLocator.ForClassLoader.of(Utils.getAgentClassLoader()); + final ClassFileLocator locator; final MethodScanner methodScanner = new MethodScanner(); @@ -41,7 +40,12 @@ public final class HelperScanner extends ClassVisitor { Set uses; HelperScanner() { + this(ClassFileLocator.ForClassLoader.of(Utils.getAgentClassLoader())); + } + + HelperScanner(ClassFileLocator locator) { super(Opcodes.ASM7, null); + this.locator = locator; } /** Expands helper class names to include any non-bootstrap classes they depend on. */ @@ -49,6 +53,15 @@ public static String[] withClassDependencies(String... helperClassNames) { return new HelperScanner().simulateClassLoading(helperClassNames); } + /** + * Same as above, but reads bytecode via the passed locator (e.g. during build time when the agent + * loader is absent). + */ + public static String[] withClassDependencies( + ClassFileLocator locator, String... helperClassNames) { + return new HelperScanner(locator).simulateClassLoading(helperClassNames); + } + /** * Simulates class-loading by finding all classes required to load the helper classes as well as * optional classes used in method instructions that may be needed later when invoking the method. diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterModule.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterModule.java index d2abbc265e5..833d573cfde 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterModule.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterModule.java @@ -113,13 +113,23 @@ public static ReferenceMatcher loadStaticMuzzleReferences( } /** - * @return Class names of helpers to inject into the user's classloader. - *

NOTE: The order of the returned helper classes matters. If a muzzle check fails - * with a NoClassDefFoundError, as logged in build/reports/muzzle-*.txt, it is likely that one - * helper class depends on another that appears later in the list. In this case, the returned - * list must be reordered so that the referred helper class appears before the one that refers - * to it. + * @return the build-time inferred and manually-declared helper class names captured by {@code + * $Muzzle}, or {@code null} when none are available and fall back to {@link + * #helperClassNames()}. */ + public static String[] loadStaticMuzzleHelperClassNames( + ClassLoader classLoader, String instrumentationClass) { + String muzzleClass = instrumentationClass + "$Muzzle"; + try { + // helper class names captured at build-time; see MuzzleGenerator + return (String[]) + classLoader.loadClass(muzzleClass).getMethod("helperClassNames").invoke(null); + } catch (Throwable e) { + return null; + } + } + + /** Optional manual additions to the injected helper set. */ public String[] helperClassNames() { return NO_HELPERS; } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/HelperClassPredicate.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/HelperClassPredicate.java new file mode 100644 index 00000000000..c6dff32f810 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/HelperClassPredicate.java @@ -0,0 +1,65 @@ +package datadog.trace.agent.tooling.muzzle; + +import datadog.trace.bootstrap.Constants; +import java.util.function.Predicate; + +/** + * Classifies a referenced class as an injectable tracer helper, a bootstrap class, or a library + * class — similar to OpenTelemetry's {@code HelperClassPredicate#isHelperClass}. The primary signal + * is {@code ownOutput}: a class the instrumentation subproject compiled itself. + * + *

A subproject only injects helpers it owns; a helper owned by another subproject must be + * declared explicitly via {@code helperClassNames()}. {@link #HELPER_PREFIXES} lists the shared + * infrastructure subprojects that are not owned by a specific subproject and so are always treated + * as helpers. + */ +public final class HelperClassPredicate { + + static final String[] HELPER_PREFIXES = { + "datadog.opentelemetry.shim.", + "datadog.trace.agent.tooling.iast.", + "datadog.trace.agent.tooling.nativeimage.", + }; + + private final Predicate ownOutput; + + /** + * @param ownOutput tests whether a class name was compiled by the instrumentation subproject + * itself; injected so this classifier stays independent of the build directory layout. + */ + public HelperClassPredicate(final Predicate ownOutput) { + this.ownOutput = ownOutput; + } + + public boolean isHelperClass(final String className) { + return !isBootstrap(className) && (ownOutput.test(className) || matchesHelperPrefix(className)); + } + + private static boolean matchesHelperPrefix(final String className) { + for (final String prefix : HELPER_PREFIXES) { + if (className.startsWith(prefix)) { + return true; + } + } + return false; + } + + /** Whether the class is on the bootstrap class-path and so never injected. */ + public static boolean isBootstrap(final String className) { + if (className.startsWith("java.") + || className.startsWith("javax.") + || className.startsWith("jdk.") + || className.startsWith("com.sun.") + || className.startsWith("sun.") + || className.startsWith("org.slf4j.") + || className.startsWith("datadog.slf4j.")) { + return true; + } + for (final String prefix : Constants.BOOTSTRAP_PACKAGE_PREFIXES) { + if (className.startsWith(prefix)) { + return true; + } + } + return false; + } +} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleGenerator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleGenerator.java index 69421f6d16a..50cc27526de 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleGenerator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleGenerator.java @@ -3,15 +3,18 @@ import static java.util.Arrays.asList; import datadog.trace.agent.tooling.AdviceShader; +import datadog.trace.agent.tooling.HelperScanner; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -20,6 +23,7 @@ import net.bytebuddy.description.field.FieldList; import net.bytebuddy.description.method.MethodList; import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.dynamic.ClassFileLocator; import net.bytebuddy.implementation.Implementation; import net.bytebuddy.jar.asm.ClassVisitor; import net.bytebuddy.jar.asm.ClassWriter; @@ -30,9 +34,12 @@ /** Generates a 'Muzzle' side-class for each {@link InstrumenterModule}. */ public class MuzzleGenerator implements AsmVisitorWrapper { + private final File sourceDir; + private final File targetDir; - public MuzzleGenerator(File targetDir) { + public MuzzleGenerator(File sourceDir, File targetDir) { + this.sourceDir = sourceDir; this.targetDir = targetDir; } @@ -81,7 +88,9 @@ public ClassVisitor wrap( } private static Reference[] generateReferences( - Instrumenter.HasMethodAdvice instrumenter, AdviceShader adviceShader) { + Instrumenter.HasMethodAdvice instrumenter, + AdviceShader adviceShader, + Set allAdviceClasses) { // track sources we've generated references from to avoid recursion final Set referenceSources = new HashSet<>(); final Map references = new LinkedHashMap<>(); @@ -93,6 +102,8 @@ private static Reference[] generateReferences( adviceClasses.addAll(asList(additionalClasses)); } }); + // remember the advice roots so callers can exclude them from the injected helper set + allAdviceClasses.addAll(adviceClasses); ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); for (String adviceClass : adviceClasses) { if (referenceSources.add(adviceClass)) { @@ -112,21 +123,32 @@ private static Reference[] generateReferences( } /** This code is generated in a separate side-class. */ - private static byte[] generateMuzzleClass(InstrumenterModule module) { + private byte[] generateMuzzleClass(InstrumenterModule module) { - Set ignoredClassNames = new HashSet<>(asList(module.muzzleIgnoredClassNames())); AdviceShader adviceShader = AdviceShader.with(module.adviceShading()); - List references = new ArrayList<>(); + // Collect the muzzle references from every advice the module defines. + Set adviceClasses = new HashSet<>(); + List allReferences = new ArrayList<>(); for (Instrumenter instrumenter : module.typeInstrumentations()) { if (instrumenter instanceof Instrumenter.HasMethodAdvice) { - for (Reference reference : - generateReferences((Instrumenter.HasMethodAdvice) instrumenter, adviceShader)) { - // ignore helper classes, they will be injected by the instrumentation's HelperInjector. - if (!ignoredClassNames.contains(reference.className)) { - references.add(reference); - } - } + Collections.addAll( + allReferences, + generateReferences( + (Instrumenter.HasMethodAdvice) instrumenter, adviceShader, adviceClasses)); + } + } + + String[] orderedHelpers = computeInjectedHelpers(module, allReferences, adviceClasses); + + // Injected helpers are our own classes, so they don't need to be asserted as library + // references. + Set ignoredClassNames = new HashSet<>(asList(orderedHelpers)); + Collections.addAll(ignoredClassNames, module.muzzleIgnoredClassNames()); + List references = new ArrayList<>(); + for (Reference reference : allReferences) { + if (!ignoredClassNames.contains(reference.className)) { + references.add(reference); } } Reference[] additionalReferences = module.additionalMuzzleReferences(); @@ -179,9 +201,199 @@ private static byte[] generateMuzzleClass(InstrumenterModule module) { mv.visitMaxs(0, 0); mv.visitEnd(); + // Generate helperClassNames() with resolved helpers for the agent to read at load time; + // skip the method entirely when the module injects nothing. + if (orderedHelpers.length > 0) { + MethodVisitor hv = + cw.visitMethod( + Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, + "helperClassNames", + "()[Ljava/lang/String;", + null, + null); + hv.visitCode(); + writeStrings(hv, orderedHelpers); + hv.visitInsn(Opcodes.ARETURN); + hv.visitMaxs(0, 0); + hv.visitEnd(); + } + return cw.toByteArray(); } + /** Resolves the ordered set of helper classes to inject for a module. */ + String[] computeInjectedHelpers( + InstrumenterModule module, List allReferences, Set adviceClasses) { + HelperClassPredicate helperPredicate = new HelperClassPredicate(this::isOwnOutput); + // Inferred helpers = our classes referenced from the advice minus the advice roots. + Set inferredHelpers = new LinkedHashSet<>(); + for (Reference reference : allReferences) { + if (!adviceClasses.contains(reference.className) + && helperPredicate.isHelperClass(reference.className)) { + inferredHelpers.add(reference.className); + } + } + + // Add manually defined helpers. + Set manualHelpers = new LinkedHashSet<>(asList(module.helperClassNames())); + Set initialHelpers = new LinkedHashSet<>(inferredHelpers); + initialHelpers.addAll(manualHelpers); + for (String helper : new ArrayList<>(initialHelpers)) { + if (isOwnOutput(helper)) { + addNestedClasses(helper, initialHelpers); + } + } + + ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + String[] orderedHelpers = + discoverAndOrderHelpers(initialHelpers, manualHelpers, helperPredicate, contextClassLoader); + + // Drop build-time-only muzzle providers. + ClassFileLocator locator = ClassFileLocator.ForClassLoader.of(contextClassLoader); + List injectableHelpers = new ArrayList<>(orderedHelpers.length); + for (String helper : orderedHelpers) { + if (!isBuildTimeOnly(helper, locator)) { + injectableHelpers.add(helper); + } + } + orderedHelpers = injectableHelpers.toArray(new String[0]); + + writeInferenceReport(module, adviceClasses.isEmpty(), inferredHelpers, orderedHelpers); + return orderedHelpers; + } + + /** {@code true} if the class was compiled from this instrumentation subproject's own output. */ + private boolean isOwnOutput(String className) { + return new File(sourceDir, className.replace('.', '/') + ".class").isFile(); + } + + /** Adds the nested classes ({@code Foo$Bar}, {@code Foo$1}, ...) of an ownOutput helper. */ + private void addNestedClasses(String className, Set helperClasses) { + File classFile = new File(sourceDir, className.replace('.', '/') + ".class"); + File dir = classFile.getParentFile(); + if (dir == null || !dir.isDirectory()) { + return; + } + int lastDot = className.lastIndexOf('.'); + String pkg = lastDot < 0 ? "" : className.substring(0, lastDot + 1); + String prefix = (lastDot < 0 ? className : className.substring(lastDot + 1)) + "$"; + File[] siblings = dir.listFiles(); + if (siblings == null) { + return; + } + for (File sibling : siblings) { + String fileName = sibling.getName(); + if (fileName.startsWith(prefix) && fileName.endsWith(".class")) { + helperClasses.add(pkg + fileName.substring(0, fileName.length() - ".class".length())); + } + } + } + + private static final String MUZZLE_REFERENCE_API = "datadog/trace/agent/tooling/muzzle/Reference"; + + /** + * {@code true} if the class uses the muzzle {@link Reference} API (as a {@link ReferenceProvider} + * or via {@code compileReferences}). This method is used to avoid injecting build-time-only + * classes. + */ + static boolean isBuildTimeOnly(String className, ClassFileLocator locator) { + try { + ClassFileLocator.Resolution resolution = locator.locate(className); + if (!resolution.isResolved()) { + return false; + } + // The muzzle type appears as a constant-pool entry when the class references it. + return new String(resolution.resolve(), StandardCharsets.ISO_8859_1) + .contains(MUZZLE_REFERENCE_API); + } catch (IOException e) { + return false; + } + } + + /** + * Expands the given helpers with any helper classes they depend on and returns them in + * dependency-first load order (required by {@link datadog.trace.agent.tooling.HelperInjector}) + * via {@link HelperScanner}. Library classes the scanner pulls in are dropped, but helpers that + * could not be located are kept (appended, unordered). + */ + private static String[] discoverAndOrderHelpers( + Set initialHelpers, + Set manualHelpers, + HelperClassPredicate helperPredicate, + ClassLoader loader) { + if (initialHelpers.isEmpty()) { + return new String[0]; + } + List ordered = new ArrayList<>(); + try { + for (String name : + HelperScanner.withClassDependencies( + ClassFileLocator.ForClassLoader.of(loader), initialHelpers.toArray(new String[0]))) { + if ((helperPredicate.isHelperClass(name) || manualHelpers.contains(name)) + && !ordered.contains(name)) { + ordered.add(name); + } + } + } catch (Throwable ignore) { + // best-effort ordering; unlocatable helpers are appended below + } + for (String helper : initialHelpers) { + if (!ordered.contains(helper)) { + ordered.add(helper); + } + } + return ordered.toArray(new String[0]); + } + + /** + * TODO: Remove this method after all instrumentations are migrated! Writes an advisory report + * classifying each declared helper as inferred vs. manual-only. Used to help with migration from + * manually listed helpers to auto-inferred ones. + */ + private void writeInferenceReport( + InstrumenterModule module, + boolean adviceLess, + Set inferredHelpers, + String[] injectedHelpers) { + try { + Set declared = new LinkedHashSet<>(asList(module.helperClassNames())); + if (declared.isEmpty() && inferredHelpers.isEmpty()) { + return; + } + File buildDir = targetDir; + while (buildDir != null && !"build".equals(buildDir.getName())) { + buildDir = buildDir.getParentFile(); + } + if (buildDir == null) { + return; + } + File reportDir = new File(buildDir, "reports/helper-inference"); + reportDir.mkdirs(); + StringBuilder sb = new StringBuilder(); + sb.append("module: ").append(module.getClass().getName()).append('\n'); + sb.append("has-advice: ").append(!adviceLess).append('\n'); + sb.append("injected-helpers: ").append(injectedHelpers.length).append('\n'); + if (adviceLess && !declared.isEmpty()) { + sb.append("crawl-cannot-cover: advice-less module; declared helpers are all manual-only\n"); + } + for (String name : declared) { + sb.append(inferredHelpers.contains(name) ? " inferred " : " manual-only ") + .append(name) + .append('\n'); + } + for (String name : inferredHelpers) { + if (!declared.contains(name)) { + sb.append(" inferred-only ").append(name).append('\n'); + } + } + Files.write( + new File(reportDir, module.getClass().getName() + ".txt").toPath(), + sb.toString().getBytes(StandardCharsets.UTF_8)); + } catch (Throwable ignore) { + // report is advisory; never fail the build over it + } + } + private static void writeReference(MethodVisitor mv, Reference reference) { if (reference instanceof OrReference) { mv.visitTypeInsn(Opcodes.NEW, "datadog/trace/agent/tooling/muzzle/OrReference"); diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleGradlePlugin.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleGradlePlugin.java index 5a22ac484be..55ede9c8c9d 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleGradlePlugin.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleGradlePlugin.java @@ -25,10 +25,16 @@ public class MuzzleGradlePlugin extends Plugin.ForElementMatcher { HierarchyMatchers.registerIfAbsent(HierarchyMatchers.simpleChecks()); } + private final File sourceDir; private final File targetDir; - public MuzzleGradlePlugin(File targetDir) { + /** + * @param sourceDir the engine's source folder, used to decide which helpers this subproject owns + * @param targetDir the engine's target folder, where {@code $Muzzle} classes are written + */ + public MuzzleGradlePlugin(File sourceDir, File targetDir) { super(concreteClass().and(extendsClass(named(InstrumenterModule.class.getName())))); + this.sourceDir = sourceDir; this.targetDir = targetDir; } @@ -37,7 +43,7 @@ public DynamicType.Builder apply( final DynamicType.Builder builder, final TypeDescription typeDescription, final ClassFileLocator classFileLocator) { - return builder.visit(new MuzzleGenerator(targetDir)); + return builder.visit(new MuzzleGenerator(sourceDir, targetDir)); } @Override diff --git a/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/HelperClassPredicateTest.java b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/HelperClassPredicateTest.java new file mode 100644 index 00000000000..90a3ad6fa98 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/HelperClassPredicateTest.java @@ -0,0 +1,79 @@ +package datadog.trace.agent.tooling.muzzle; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.HashSet; +import java.util.Set; +import org.junit.jupiter.api.Test; + +class HelperClassPredicateTest { + + private static HelperClassPredicate predicateWithOwnOutput(String... ownOutput) { + Set own = new HashSet<>(); + for (String className : ownOutput) { + own.add(className); + } + return new HelperClassPredicate(own::contains); + } + + @Test + void instrumentationPackageRequiresOwnOutput() { + // The same instrumentation-package class is a helper only when this subproject compiled it + String className = "datadog.trace.instrumentation.servlet.ServletBlockingHelper"; + assertTrue(predicateWithOwnOutput(className).isHelperClass(className)); + assertFalse(predicateWithOwnOutput().isHelperClass(className)); + } + + @Test + void allowlistedSharedSubprojectsAreHelpers() { + // helpers that live in other dd-owned tracer subprojects + HelperClassPredicate predicate = predicateWithOwnOutput(); + assertTrue(predicate.isHelperClass("datadog.opentelemetry.shim.context.OtelContext")); + assertTrue(predicate.isHelperClass("datadog.trace.agent.tooling.iast.TaintableEnumeration")); + assertTrue(predicate.isHelperClass("datadog.trace.agent.tooling.nativeimage.TracerActivation")); + } + + @Test + void ownOutputInLibraryPackageIsHelper() { + // helpers deliberately placed in a library's package are detected via ownOutput + HelperClassPredicate predicate = + predicateWithOwnOutput("redis.clients.jedis.JedisClientDecorator"); + assertTrue(predicate.isHelperClass("redis.clients.jedis.JedisClientDecorator")); + // the real library class in the same package is not ours and must remain a library reference + assertFalse(predicate.isHelperClass("redis.clients.jedis.Jedis")); + } + + @Test + void bootstrapAndJdkAreNeverHelpers() { + // bootstrap wins even if a class is reported as ownOutput + HelperClassPredicate predicate = + predicateWithOwnOutput("datadog.trace.bootstrap.InstrumentationContext"); + assertFalse(predicate.isHelperClass("datadog.trace.bootstrap.InstrumentationContext")); + assertFalse(predicate.isHelperClass("datadog.trace.api.Config")); + // datadog.trace.instrumentation.api is a bootstrap sub-package, not an injectable helper + assertFalse(predicate.isHelperClass("datadog.trace.instrumentation.api.SomeApi")); + assertFalse(predicate.isHelperClass("java.lang.String")); + assertFalse(predicate.isHelperClass("javax.servlet.http.HttpServletRequest")); + assertFalse(predicate.isHelperClass("org.slf4j.Logger")); + assertFalse(predicate.isHelperClass("datadog.slf4j.Logger")); + } + + @Test + void plainLibraryClassIsNotHelper() { + HelperClassPredicate predicate = predicateWithOwnOutput(); + assertFalse(predicate.isHelperClass("org.apache.http.HttpRequest")); + assertFalse(predicate.isHelperClass("com.datastax.oss.driver.api.core.CqlSession")); + } + + @Test + void isBootstrapMatchesBootstrapPrefixesAndJdk() { + assertTrue(HelperClassPredicate.isBootstrap("datadog.trace.instrumentation.api.X")); + assertTrue(HelperClassPredicate.isBootstrap("datadog.trace.bootstrap.Y")); + assertTrue(HelperClassPredicate.isBootstrap("datadog.trace.api.Z")); + assertTrue(HelperClassPredicate.isBootstrap("java.util.List")); + assertTrue(HelperClassPredicate.isBootstrap("jdk.internal.Foo")); + assertFalse(HelperClassPredicate.isBootstrap("datadog.trace.instrumentation.foo.Bar")); + assertFalse(HelperClassPredicate.isBootstrap("com.example.Lib")); + } +} diff --git a/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/MuzzleGeneratorFixtures.java b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/MuzzleGeneratorFixtures.java new file mode 100644 index 00000000000..2a979cb6c40 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/MuzzleGeneratorFixtures.java @@ -0,0 +1,52 @@ +package datadog.trace.agent.tooling.muzzle; + +import java.util.Collections; +import net.bytebuddy.pool.TypePool; + +/** Fixtures for {@link MuzzleGeneratorTest}. */ +final class MuzzleGeneratorFixtures { + private MuzzleGeneratorFixtures() {} + + /** Helper referenced by the advice. */ + static class InferredHelperFixture {} + + /** + * Helper the advice does not reference; reachable only via manual definition in {@code + * helperClassNames()}. + */ + static class ManualHelperFixture {} + + /** Helper whose nested build-time {@code MuzzleHelper} must not be injected. */ + static class OwnerWithMuzzleFixture { + static final class MuzzleHelper implements ReferenceProvider { + @Override + public Iterable buildReferences(TypePool typePool) { + return Collections.emptyList(); + } + } + } + + /** Build-time muzzle provider that must never be injected into the application. */ + static final class BuildTimeProviderFixture implements ReferenceProvider { + @Override + public Iterable buildReferences(TypePool typePool) { + return Collections.emptyList(); + } + } + + /** Advice referencing the inferred helper and the owner but not the manual helper. */ + static class CombineAdvice { + static void apply() { + new InferredHelperFixture(); + new OwnerWithMuzzleFixture(); + } + } + + /** Module declaring a manual helper the advice crawl cannot see. */ + static class CombineModule extends TestInstrumentationClasses.BaseInst { + @Override + public String[] helperClassNames() { + return new String[] {ManualHelperFixture.class.getName()}; + } + } +} diff --git a/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/MuzzleGeneratorTest.java b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/MuzzleGeneratorTest.java new file mode 100644 index 00000000000..e05d547a9e4 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/MuzzleGeneratorTest.java @@ -0,0 +1,83 @@ +package datadog.trace.agent.tooling.muzzle; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.BuildTimeProviderFixture; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.CombineAdvice; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.CombineModule; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.InferredHelperFixture; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.ManualHelperFixture; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.OwnerWithMuzzleFixture; +import java.io.File; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import net.bytebuddy.dynamic.ClassFileLocator; +import org.junit.jupiter.api.Test; + +class MuzzleGeneratorTest { + + private static final String INFERRED = InferredHelperFixture.class.getName(); + private static final String MANUAL = ManualHelperFixture.class.getName(); + private static final String OWNER = OwnerWithMuzzleFixture.class.getName(); + private static final String MUZZLE_HELPER = OwnerWithMuzzleFixture.MuzzleHelper.class.getName(); + + @Test + void isBuildTimeOnlyDetectsMuzzleReferenceProviders() { + ClassFileLocator locator = ClassFileLocator.ForClassLoader.of(getClass().getClassLoader()); + // classes that use the muzzle Reference API are build-time only and must not be injected + assertTrue(MuzzleGenerator.isBuildTimeOnly(BuildTimeProviderFixture.class.getName(), locator)); + assertTrue(MuzzleGenerator.isBuildTimeOnly(MUZZLE_HELPER, locator)); + // ordinary helper is injectable + assertFalse(MuzzleGenerator.isBuildTimeOnly(INFERRED, locator)); + } + + @Test + void combinesInferredAndManualHelpersAndDropsMuzzleProviders() throws Exception { + // Point the generator's "ownOutput" at this module's compiled test classes so the fixtures + // count as this subproject's own helpers. + File sourceDir = classesRootOf(MuzzleGeneratorFixtures.class); + File targetDir = Files.createTempDirectory("muzzle-generator-test").toFile(); + MuzzleGenerator generator = new MuzzleGenerator(sourceDir, targetDir); + + ClassLoader loader = MuzzleGeneratorTest.class.getClassLoader(); + Map crawled = + ReferenceCreator.createReferencesFrom(CombineAdvice.class.getName(), loader); + List references = new ArrayList<>(crawled.values()); + Set adviceClasses = Collections.singleton(CombineAdvice.class.getName()); + + List injected = + injectedHelpers(generator, new CombineModule(), references, adviceClasses); + + assertTrue(injected.contains(INFERRED), "inferred helper should be injected"); + assertTrue(injected.contains(MANUAL), "manually declared helper should be injected"); + assertTrue(injected.contains(OWNER), "ownOutput helper should be injected"); + assertFalse(injected.contains(MUZZLE_HELPER), "build-time MuzzleHelper must not be injected"); + assertFalse(injected.contains(CombineAdvice.class.getName()), "advice root is not a helper"); + } + + private static List injectedHelpers( + MuzzleGenerator generator, + InstrumenterModule module, + List references, + Set adviceClasses) { + ClassLoader previous = Thread.currentThread().getContextClassLoader(); + // computeInjectedHelpers resolves classes via the context class-loader. + Thread.currentThread().setContextClassLoader(MuzzleGeneratorTest.class.getClassLoader()); + try { + return Arrays.asList(generator.computeInjectedHelpers(module, references, adviceClasses)); + } finally { + Thread.currentThread().setContextClassLoader(previous); + } + } + + private static File classesRootOf(Class type) throws Exception { + return new File(type.getProtectionDomain().getCodeSource().getLocation().toURI()); + } +} diff --git a/dd-java-agent/instrumentation/datastax-cassandra/datastax-cassandra-4.0/src/main/java/datadog/trace/instrumentation/datastax/cassandra4/CassandraClientInstrumentation.java b/dd-java-agent/instrumentation/datastax-cassandra/datastax-cassandra-4.0/src/main/java/datadog/trace/instrumentation/datastax/cassandra4/CassandraClientInstrumentation.java index 963ee8c5439..f9e0f865ad3 100644 --- a/dd-java-agent/instrumentation/datastax-cassandra/datastax-cassandra-4.0/src/main/java/datadog/trace/instrumentation/datastax/cassandra4/CassandraClientInstrumentation.java +++ b/dd-java-agent/instrumentation/datastax-cassandra/datastax-cassandra-4.0/src/main/java/datadog/trace/instrumentation/datastax/cassandra4/CassandraClientInstrumentation.java @@ -24,15 +24,6 @@ public String instrumentedType() { return "com.datastax.oss.driver.internal.core.session.DefaultSession"; } - @Override - public String[] helperClassNames() { - return new String[] { - packageName + ".CassandraClientDecorator", - packageName + ".TracingSession", - packageName + ".ContactPointsUtil", - }; - } - @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( diff --git a/dd-java-agent/instrumentation/google-http-client-1.19/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java b/dd-java-agent/instrumentation/google-http-client-1.19/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java index 769976bfc7a..84c8196b787 100644 --- a/dd-java-agent/instrumentation/google-http-client-1.19/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/google-http-client-1.19/src/main/java/datadog/trace/instrumentation/googlehttpclient/GoogleHttpClientInstrumentation.java @@ -39,13 +39,6 @@ public String instrumentedType() { return "com.google.api.client.http.HttpRequest"; } - @Override - public String[] helperClassNames() { - return new String[] { - packageName + ".GoogleHttpClientDecorator", packageName + ".HeadersInjectAdapter" - }; - } - @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvices( diff --git a/dd-java-agent/instrumentation/jedis/jedis-4.0/src/main/java/datadog/trace/instrumentation/jedis40/JedisInstrumentation.java b/dd-java-agent/instrumentation/jedis/jedis-4.0/src/main/java/datadog/trace/instrumentation/jedis40/JedisInstrumentation.java index 24350a6a3a6..6c525ebb6ab 100644 --- a/dd-java-agent/instrumentation/jedis/jedis-4.0/src/main/java/datadog/trace/instrumentation/jedis40/JedisInstrumentation.java +++ b/dd-java-agent/instrumentation/jedis/jedis-4.0/src/main/java/datadog/trace/instrumentation/jedis40/JedisInstrumentation.java @@ -33,13 +33,6 @@ public String instrumentedType() { return "redis.clients.jedis.Connection"; } - @Override - public String[] helperClassNames() { - return new String[] { - "redis.clients.jedis.JedisClientDecorator", - }; - } - @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( diff --git a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-common/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-common/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java index b1af9e2ea13..d409628ca29 100644 --- a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-common/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-common/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java @@ -45,13 +45,6 @@ public ElementMatcher hierarchyMatcher() { return extendsClass(named(hierarchyMarkerType())); } - @Override - public String[] helperClassNames() { - return new String[] { - "datadog.trace.instrumentation.servlet.SpanNameCache", packageName + ".HttpServletDecorator", - }; - } - /** * Here we are instrumenting the protected method for HttpServlet. This should ensure that this * advice is always called after Servlet3Instrumentation which is instrumenting the public method. diff --git a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-common/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-common/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java index 10e6c530212..e708d76e79a 100644 --- a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-common/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-common/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java @@ -42,7 +42,6 @@ public ElementMatcher hierarchyMatcher() { public String[] helperClassNames() { return new String[] { "datadog.trace.instrumentation.servlet.ServletRequestSetter", - packageName + ".HttpServletResponseDecorator", }; }