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..6724cdefa60 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,7 +113,9 @@ public static ReferenceMatcher loadStaticMuzzleReferences( } /** - * @return Class names of helpers to inject into the user's classloader. + * @return Class names of helpers to inject into the user's classloader. Override this to declare + * them manually; otherwise {@code MuzzleGenerator} generates it at build time from the + * helpers inferred from the advice. *

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 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..59b00800a3c 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,22 @@ 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.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.security.CodeSource; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; 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 +27,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; @@ -70,18 +78,45 @@ public ClassVisitor wrap( throw new RuntimeException(e); } + File sourceRoot = sourceRootFor(module); + + AdviceShader adviceShader = AdviceShader.with(module.adviceShading()); + + // 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) { + Collections.addAll( + allReferences, + generateReferences( + (Instrumenter.HasMethodAdvice) instrumenter, adviceShader, adviceClasses)); + } + } + + String[] orderedHelpers = + computeInjectedHelpers(module, allReferences, adviceClasses, sourceRoot); + File muzzleClass = new File(targetDir, moduleDefinition.getInternalName() + "$Muzzle.class"); try { muzzleClass.getParentFile().mkdirs(); - Files.write(muzzleClass.toPath(), generateMuzzleClass(module)); + Files.write(muzzleClass.toPath(), generateMuzzleClass(module, allReferences, orderedHelpers)); } catch (IOException e) { throw new RuntimeException(e); } - return classVisitor; + + // Generate helperClassNames() only when neither the module nor a parent declares one + if (module.helperClassNames().length > 0) { + return classVisitor; + } + // Write the resolved helpers into the module's helperClassNames() so agent reads them directly. + return new HelperClassNamesWriter(classVisitor, orderedHelpers); } 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 +128,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 +149,17 @@ private static Reference[] generateReferences( } /** This code is generated in a separate side-class. */ - private static byte[] generateMuzzleClass(InstrumenterModule module) { - - Set ignoredClassNames = new HashSet<>(asList(module.muzzleIgnoredClassNames())); - AdviceShader adviceShader = AdviceShader.with(module.adviceShading()); + private byte[] generateMuzzleClass( + InstrumenterModule module, List allReferences, String[] orderedHelpers) { + // 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 (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); - } - } + for (Reference reference : allReferences) { + if (!ignoredClassNames.contains(reference.className)) { + references.add(reference); } } Reference[] additionalReferences = module.additionalMuzzleReferences(); @@ -182,6 +215,185 @@ private static byte[] generateMuzzleClass(InstrumenterModule module) { return cw.toByteArray(); } + /** + * Adds a {@code helperClassNames()} returning the build-time-resolved helper list to modules that + * don't declare one; a module that declares its own keeps it. + */ + private static final class HelperClassNamesWriter extends ClassVisitor { + private static final String HELPER_METHOD = "helperClassNames"; + private static final String HELPER_DESCRIPTOR = "()[Ljava/lang/String;"; + + private final String[] helpers; + private boolean declared; + + HelperClassNamesWriter(ClassVisitor classVisitor, String[] helpers) { + super(Opcodes.ASM7, classVisitor); + this.helpers = helpers; + } + + @Override + public MethodVisitor visitMethod( + int access, String name, String descriptor, String signature, String[] exceptions) { + if (HELPER_METHOD.equals(name) && HELPER_DESCRIPTOR.equals(descriptor)) { + declared = true; + } + return super.visitMethod(access, name, descriptor, signature, exceptions); + } + + @Override + public void visitEnd() { + // Only generate when the module does not declare its own manually listed helpers. + if (!declared && helpers.length > 0) { + MethodVisitor mv = + super.visitMethod(Opcodes.ACC_PUBLIC, HELPER_METHOD, HELPER_DESCRIPTOR, null, null); + mv.visitCode(); + writeStrings(mv, helpers); + mv.visitInsn(Opcodes.ARETURN); + mv.visitMaxs(0, 0); + mv.visitEnd(); + } + super.visitEnd(); + } + } + + /** Resolves the ordered set of helper classes to inject for a module. */ + String[] computeInjectedHelpers( + InstrumenterModule module, + List allReferences, + Set adviceClasses, + File sourceRoot) { + // A module that declares its own helper list uses it directly. + String[] declaredHelpers = module.helperClassNames(); + if (declaredHelpers.length > 0) { + return declaredHelpers; + } + + // Otherwise infer them + HelperClassPredicate helperPredicate = + new HelperClassPredicate(name -> isOwnOutput(sourceRoot, name)); + Set helpers = new LinkedHashSet<>(); + for (Reference reference : allReferences) { + if (!adviceClasses.contains(reference.className) + && helperPredicate.isHelperClass(reference.className)) { + helpers.add(reference.className); + } + } + for (String helper : new ArrayList<>(helpers)) { + if (isOwnOutput(sourceRoot, helper)) { + addNestedClasses(sourceRoot, helper, helpers); + } + } + ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + String[] orderedHelpers = discoverAndOrderHelpers(helpers, helperPredicate, contextClassLoader); + ClassFileLocator locator = ClassFileLocator.ForClassLoader.of(contextClassLoader); + List injectableHelpers = new ArrayList<>(orderedHelpers.length); + for (String helper : orderedHelpers) { + if (!isBuildTimeOnly(helper, locator)) { + injectableHelpers.add(helper); + } + } + return injectableHelpers.toArray(new String[0]); + } + + /** + * The subproject's compiled-output root, taken from the loaded module's code source (the + * classpath entry it was loaded from, i.e. the raw-classes folder). + */ + private static File sourceRootFor(InstrumenterModule module) { + CodeSource codeSource = module.getClass().getProtectionDomain().getCodeSource(); + if (codeSource == null || codeSource.getLocation() == null) { + throw new IllegalStateException( + "Cannot locate compiled output for " + module.getClass().getName()); + } + try { + return new File(codeSource.getLocation().toURI()); + } catch (URISyntaxException e) { + throw new IllegalStateException( + "Cannot resolve compiled output for " + codeSource.getLocation(), e); + } + } + + /** {@code true} if the class was compiled from this instrumentation subproject's own output. */ + private static boolean isOwnOutput(File sourceRoot, String className) { + return new File(sourceRoot, className.replace('.', '/') + ".class").isFile(); + } + + /** Adds the nested classes ({@code Foo$Bar}, {@code Foo$1}, ...) of an ownOutput helper. */ + private static void addNestedClasses( + File sourceRoot, String className, Set helperClasses) { + File classFile = new File(sourceRoot, 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; + } + Arrays.sort(siblings, Comparator.comparing(File::getName)); + 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, 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) && !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]); + } + 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/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..662f9df230f --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/MuzzleGeneratorFixtures.java @@ -0,0 +1,55 @@ +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 list. */ + static class ManualModule extends TestInstrumentationClasses.BaseInst { + @Override + public String[] helperClassNames() { + return new String[] {ManualHelperFixture.class.getName()}; + } + } + + /** Module with no declared helper list, so its helpers come from auto-detection. */ + static class InferredModule extends TestInstrumentationClasses.BaseInst {} +} 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..ed2bb3dd8a0 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/muzzle/MuzzleGeneratorTest.java @@ -0,0 +1,93 @@ +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.InferredHelperFixture; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.InferredModule; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.ManualHelperFixture; +import datadog.trace.agent.tooling.muzzle.MuzzleGeneratorFixtures.ManualModule; +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 declaredHelperListIsUsedAsIsWithoutMergingInference() throws Exception { + List injected = injectedHelpers(new ManualModule()); + + // A module that declares helperClassNames() gets exactly that list - nothing inferred from the + // advice is merged in. + assertTrue(injected.contains(MANUAL), "manually declared helper should be injected"); + assertFalse(injected.contains(INFERRED), "inferred helper must not be merged into manual list"); + assertFalse(injected.contains(OWNER), "ownOutput helper must not be merged into manual list"); + } + + @Test + void inferredHelpersAreUsedWhenNoListDeclaredAndMuzzleProvidersDropped() throws Exception { + List injected = injectedHelpers(new InferredModule()); + + // If no declared list, the helpers inferred from the advice are injected minus the + // build-time-only muzzle provider and advice root. + assertTrue(injected.contains(INFERRED), "inferred 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"); + assertFalse(injected.contains(MANUAL), "helper the advice never references is not inferred"); + } + + private static List injectedHelpers(InstrumenterModule module) 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(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()); + + ClassLoader previous = Thread.currentThread().getContextClassLoader(); + // computeInjectedHelpers resolves classes via the context class-loader. + Thread.currentThread().setContextClassLoader(loader); + try { + return Arrays.asList( + generator.computeInjectedHelpers(module, references, adviceClasses, sourceDir)); + } 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.