From 7021e2434669f8bfdff3889bedcd89206a063c9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Wed, 10 Aug 2022 17:43:14 +0200 Subject: [PATCH 1/2] Add CallSiteInstrumenter to perform CSI in the tracer --- .../tooling/AgentTransformerBuilder.java | 2 + .../agent-tooling/agent-tooling.gradle | 21 ++ .../bytebuddy/csi/CallSiteBenchmark.java | 89 ++++++ .../csi/CallSiteBenchmarkHelper.java | 17 ++ .../csi/CallSiteBenchmarkInstrumenter.java | 88 ++++++ .../csi/CalleeBenchmarkInstrumenter.java | 52 ++++ .../jmh/java/foo/bar/DummyApplication.java | 6 + .../src/jmh/java/foo/bar/DummyController.java | 14 + .../datadog.trace.agent.tooling.Instrumenter | 2 + .../trace/agent/tooling/Instrumenter.java | 5 + .../agent/tooling/bytebuddy/csi/Advices.java | 274 ++++++++++++++++++ .../bytebuddy/csi/CallSiteInstrumenter.java | 57 ++++ .../bytebuddy/csi/CallSiteTransformer.java | 115 ++++++++ .../agent/tooling/csi/CallSiteAdvice.java | 25 ++ .../trace/agent/tooling/csi/Pointcut.java | 9 + .../agent/tooling/csi/AdvicesTest.groovy | 137 +++++++++ .../agent/tooling/csi/BaseCallSiteTest.groovy | 123 ++++++++ .../csi/CallSiteInstrumenterTest.groovy | 107 +++++++ .../csi/CallSiteTransformerTest.groovy | 55 ++++ .../tooling/csi/StringConcatExample.java | 10 + .../agent/tooling/csi/TestCallSiteAdvice.java | 3 + ...trace.agent.tooling.csi.TestCallSiteAdvice | 1 + .../tooling/bytebuddy/csi/call_site.trie | 41 +++ 23 files changed, 1253 insertions(+) create mode 100644 dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmark.java create mode 100644 dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkHelper.java create mode 100644 dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkInstrumenter.java create mode 100644 dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CalleeBenchmarkInstrumenter.java create mode 100644 dd-java-agent/agent-tooling/src/jmh/java/foo/bar/DummyApplication.java create mode 100644 dd-java-agent/agent-tooling/src/jmh/java/foo/bar/DummyController.java create mode 100644 dd-java-agent/agent-tooling/src/jmh/resources/META-INF/services/datadog.trace.agent.tooling.Instrumenter create mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/Advices.java create mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteInstrumenter.java create mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteTransformer.java create mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/csi/CallSiteAdvice.java create mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/csi/Pointcut.java create mode 100644 dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/AdvicesTest.groovy create mode 100644 dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/BaseCallSiteTest.groovy create mode 100644 dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/CallSiteInstrumenterTest.groovy create mode 100644 dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/CallSiteTransformerTest.groovy create mode 100644 dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/StringConcatExample.java create mode 100644 dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/TestCallSiteAdvice.java create mode 100644 dd-java-agent/agent-tooling/src/test/resources/META-INF/services/datadog.trace.agent.tooling.csi.TestCallSiteAdvice create mode 100644 dd-java-agent/agent-tooling/src/test/resources/datadog/trace/agent/tooling/bytebuddy/csi/call_site.trie diff --git a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentTransformerBuilder.java b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentTransformerBuilder.java index 9dd69499f01..846d87acb00 100644 --- a/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentTransformerBuilder.java +++ b/dd-java-agent/agent-builder/src/main/java/datadog/trace/agent/tooling/AgentTransformerBuilder.java @@ -140,6 +140,8 @@ private AgentBuilder.RawMatcher matcher(Instrumenter.Default instrumenter) { typeMatcher = ((Instrumenter.ForTypeHierarchy) instrumenter).hierarchyMatcher(); } else if (instrumenter instanceof Instrumenter.ForConfiguredType) { typeMatcher = none(); // handle below, just like when it's combined with other matchers + } else if (instrumenter instanceof Instrumenter.ForCallSite) { + typeMatcher = ((Instrumenter.ForCallSite) instrumenter).callerType(); } else { return AgentBuilder.RawMatcher.Trivial.NON_MATCHING; } diff --git a/dd-java-agent/agent-tooling/agent-tooling.gradle b/dd-java-agent/agent-tooling/agent-tooling.gradle index 345d8e960df..af501d0cdc0 100644 --- a/dd-java-agent/agent-tooling/agent-tooling.gradle +++ b/dd-java-agent/agent-tooling/agent-tooling.gradle @@ -1,3 +1,6 @@ +plugins { + id 'me.champeau.jmh' +} apply from: "$rootDir/gradle/java.gradle" apply from: "$rootDir/gradle/tries.gradle" @@ -33,4 +36,22 @@ dependencies { testImplementation project(':dd-java-agent:testing') 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' +} + +jmh { + jmhVersion = '1.32' + includeTests = true } + +forbiddenApisJmh { + ignoreFailures = true +} + +final compileTestJava = project.tasks.compileTestJava +compileTestJava.dependsOn(project.tasks.generateTestClassNameTries) + +final jmh = project.tasks.jmh +jmh.outputs.upToDateWhen { false } +jmh.dependsOn(compileTestJava) diff --git a/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmark.java b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmark.java new file mode 100644 index 00000000000..18a52949d6d --- /dev/null +++ b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmark.java @@ -0,0 +1,89 @@ +package datadog.trace.agent.tooling.bytebuddy.csi; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +import datadog.trace.agent.tooling.AgentInstaller; +import java.lang.instrument.Instrumentation; +import net.bytebuddy.agent.ByteBuddyAgent; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.springframework.boot.SpringApplication; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.web.client.RestTemplate; + +@State(Scope.Benchmark) +@BenchmarkMode(Mode.SingleShotTime) +@OutputTimeUnit(MILLISECONDS) +@Fork(value = 10) +public class CallSiteBenchmark { + + private Instrumentation instrumentation; + + @Setup(Level.Trial) + public void setUp() { + instrumentation = ByteBuddyAgent.install(); + } + + @Benchmark + public void none() throws Exception { + runBenchmark(Type.NONE); + } + + @Benchmark + public void callee() throws Exception { + runBenchmark(Type.CALLEE); + } + + @Benchmark + public void callSite() throws Exception { + runBenchmark(Type.CALL_SITE); + } + + private void runBenchmark(final Type type) throws Exception { + type.apply(instrumentation); + final Class server = Class.forName("foo.bar.DummyApplication"); + try (ConfigurableApplicationContext context = SpringApplication.run(server)) { + final RestTemplate template = new RestTemplate(); + final String url = "http://localhost:8080/benchmark?param=Hello!"; + final String response = template.getForObject(url, String.class); + type.validate(response); + } + } + + enum Type { + NONE(null), + CALL_SITE("callSite"), + CALLEE("callee"); + + private final String type; + + Type(final String type) { + this.type = type; + } + + public void apply(final Instrumentation instrumentation) { + if (type != null) { + System.setProperty("dd.benchmark.instrumentation", type); + AgentInstaller.installBytebuddyAgent(instrumentation); + } + } + + public void validate(final String response) { + if (response == null) { + throw new RuntimeException("Empty response received"); + } + if (type != null && !response.contains("[Transformed]")) { + throw new RuntimeException( + String.format( + "Wrong response, expected to contain '[Transformed]' and received '%s'", response)); + } + } + } +} diff --git a/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkHelper.java b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkHelper.java new file mode 100644 index 00000000000..362c8b391bb --- /dev/null +++ b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkHelper.java @@ -0,0 +1,17 @@ +package datadog.trace.agent.tooling.bytebuddy.csi; + +import javax.servlet.ServletRequest; +import net.bytebuddy.asm.Advice; + +public class CallSiteBenchmarkHelper { + + @Advice.OnMethodExit + public static void adviceCallee(@Advice.Return(readOnly = false) String result) { + final String currentValue = result; + result = currentValue + " [Transformed]"; + } + + public static String adviceCallSite(final ServletRequest request, final String parameter) { + return request.getParameter(parameter) + " [Transformed]"; + } +} diff --git a/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkInstrumenter.java b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkInstrumenter.java new file mode 100644 index 00000000000..7222aea75b5 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkInstrumenter.java @@ -0,0 +1,88 @@ +package datadog.trace.agent.tooling.bytebuddy.csi; + +import datadog.trace.agent.tooling.csi.CallSiteAdvice; +import datadog.trace.agent.tooling.csi.CallSiteAdvice.HasHelpers; +import datadog.trace.agent.tooling.csi.Pointcut; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.jar.asm.MethodVisitor; +import net.bytebuddy.jar.asm.Opcodes; +import net.bytebuddy.matcher.ElementMatcher; + +public class CallSiteBenchmarkInstrumenter extends CallSiteInstrumenter + implements ElementMatcher { + + public CallSiteBenchmarkInstrumenter() { + super(buildCallSites(), "call-site"); + } + + @Override + public ElementMatcher callerType() { + return this; + } + + @Override + public boolean matches(final TypeDescription target) { + return CallSiteTrie.apply(target.getName()) != 1; + } + + @Override + public boolean isEnabled() { + return "callSite".equals(System.getProperty("dd.benchmark.instrumentation", "")); + } + + @Override + public boolean isApplicable(final Set enabledSystems) { + return true; + } + + private static List buildCallSites() { + return Collections.singletonList(new GetParameterCallSite()); + } + + private static class GetParameterCallSite implements CallSiteAdvice, HasHelpers, Pointcut { + + @Override + public Pointcut pointcut() { + return this; + } + + @Override + public void apply( + final MethodVisitor mv, + final int opcode, + final String owner, + final String name, + final String descriptor, + final boolean isInterface) { + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + "datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmarkHelper", + "adviceCallSite", + "(Ljavax/servlet/ServletRequest;Ljava/lang/String;)Ljava/lang/String;", + false); + } + + @Override + public String[] helperClassNames() { + return new String[] {CallSiteBenchmarkHelper.class.getName()}; + } + + @Override + public String type() { + return "javax/servlet/ServletRequest"; + } + + @Override + public String method() { + return "getParameter"; + } + + @Override + public String descriptor() { + return "(Ljava/lang/String;)Ljava/lang/String;"; + } + } +} diff --git a/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CalleeBenchmarkInstrumenter.java b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CalleeBenchmarkInstrumenter.java new file mode 100644 index 00000000000..8eed96a4608 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CalleeBenchmarkInstrumenter.java @@ -0,0 +1,52 @@ +package datadog.trace.agent.tooling.bytebuddy.csi; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassesNamed; +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Set; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +public class CalleeBenchmarkInstrumenter extends Instrumenter.Default + implements Instrumenter.ForTypeHierarchy { + + public CalleeBenchmarkInstrumenter() { + super("callee"); + } + + @Override + public ElementMatcher classLoaderMatcher() { + return hasClassesNamed("javax.servlet.http.HttpServlet"); + } + + @Override + public ElementMatcher hierarchyMatcher() { + return implementsInterface(named("javax.servlet.ServletRequest")); + } + + @Override + public void adviceTransformations(final AdviceTransformation transformation) { + transformation.applyAdvice( + named("getParameter").and(takesArguments(String.class)).and(returns(String.class)), + CallSiteBenchmarkHelper.class.getName()); + } + + @Override + public String[] helperClassNames() { + return new String[] {CallSiteBenchmarkHelper.class.getName()}; + } + + @Override + public boolean isEnabled() { + return "callee".equals(System.getProperty("dd.benchmark.instrumentation", "")); + } + + @Override + public boolean isApplicable(final Set enabledSystems) { + return true; + } +} diff --git a/dd-java-agent/agent-tooling/src/jmh/java/foo/bar/DummyApplication.java b/dd-java-agent/agent-tooling/src/jmh/java/foo/bar/DummyApplication.java new file mode 100644 index 00000000000..94f1b6b7e34 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/jmh/java/foo/bar/DummyApplication.java @@ -0,0 +1,6 @@ +package foo.bar; + +import org.springframework.boot.autoconfigure.SpringBootApplication; + +@SpringBootApplication(scanBasePackages = "foo.bar") +public class DummyApplication {} diff --git a/dd-java-agent/agent-tooling/src/jmh/java/foo/bar/DummyController.java b/dd-java-agent/agent-tooling/src/jmh/java/foo/bar/DummyController.java new file mode 100644 index 00000000000..4ad78536707 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/jmh/java/foo/bar/DummyController.java @@ -0,0 +1,14 @@ +package foo.bar; + +import javax.servlet.ServletRequest; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class DummyController { + + @GetMapping("/benchmark") + public String index(final ServletRequest request) { + return request.getParameter("param"); + } +} diff --git a/dd-java-agent/agent-tooling/src/jmh/resources/META-INF/services/datadog.trace.agent.tooling.Instrumenter b/dd-java-agent/agent-tooling/src/jmh/resources/META-INF/services/datadog.trace.agent.tooling.Instrumenter new file mode 100644 index 00000000000..8a30c7be295 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/jmh/resources/META-INF/services/datadog.trace.agent.tooling.Instrumenter @@ -0,0 +1,2 @@ +datadog.trace.agent.tooling.bytebuddy.csi.CalleeBenchmarkInstrumenter +datadog.trace.agent.tooling.bytebuddy.csi.CallSiteBenchmarkInstrumenter diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java index 13db6863253..163c36cd173 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java @@ -72,6 +72,11 @@ interface ForTypeHierarchy { ElementMatcher hierarchyMatcher(); } + /** Instrumentation that matches based on the caller of an instruction. */ + interface ForCallSite { + ElementMatcher callerType(); + } + /** Instrumentation that can optionally widen matching to consider the type hierarchy. */ interface CanShortcutTypeMatching extends ForKnownTypes, ForTypeHierarchy { boolean onlyMatchKnownTypes(); diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/Advices.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/Advices.java new file mode 100644 index 00000000000..1c84493e806 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/Advices.java @@ -0,0 +1,274 @@ +package datadog.trace.agent.tooling.bytebuddy.csi; + +import datadog.trace.agent.tooling.bytebuddy.ClassFileLocators; +import datadog.trace.agent.tooling.csi.CallSiteAdvice; +import datadog.trace.agent.tooling.csi.Pointcut; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import javax.annotation.Nonnull; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.dynamic.ClassFileLocator; +import net.bytebuddy.jar.asm.ClassReader; +import net.bytebuddy.utility.OpenedClassReader; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Class holding a collection of {@link CallSiteAdvice} indexed by the declaring type, method name + * and method description, it stores the collection of helper classes required by the advices. + */ +public class Advices { + + private static final Logger LOG = LoggerFactory.getLogger(Advices.class); + + public static final Advices EMPTY = + new Advices( + Collections.>>emptyMap(), + new String[0], + AdviceIntrospector.NoOpAdviceInstrospector.INSTANCE) { + @Override + public boolean isEmpty() { + return true; + } + }; + + private final Map>> advices; + + private final String[] helpers; + + private final AdviceIntrospector introspector; + + private Advices( + final Map>> advices, + final String[] helpers, + final AdviceIntrospector introspector) { + this.advices = Collections.unmodifiableMap(advices); + this.helpers = helpers; + this.introspector = introspector; + } + + public static Advices fromCallSites(@Nonnull final CallSiteAdvice... advices) { + return fromCallSites(Arrays.asList(advices)); + } + + public static Advices fromCallSites(@Nonnull final Iterable advices) { + return fromCallSites(advices, AdviceIntrospector.ConstantPoolInstrospector.INSTANCE); + } + + public static Advices fromCallSites( + @Nonnull final Iterable advices, + @Nonnull final AdviceIntrospector introspector) { + final Map>> adviceMap = new HashMap<>(); + final Set helperSet = new HashSet<>(); + for (CallSiteAdvice advice : advices) { + addAdvice(adviceMap, helperSet, advice); + } + return adviceMap.isEmpty() + ? EMPTY + : new Advices(adviceMap, helperSet.toArray(new String[0]), introspector); + } + + private static void addAdvice( + @Nonnull final Map>> advices, + @Nonnull final Set helpers, + @Nonnull final CallSiteAdvice advice) { + final Pointcut pointcut = advice.pointcut(); + Map> typeAdvices = advices.get(pointcut.type()); + if (typeAdvices == null) { + typeAdvices = new HashMap<>(); + advices.put(pointcut.type(), typeAdvices); + } + Map methodAdvices = typeAdvices.get(pointcut.method()); + if (methodAdvices == null) { + methodAdvices = new HashMap<>(); + typeAdvices.put(pointcut.method(), methodAdvices); + } + final CallSiteAdvice oldAdvice = methodAdvices.put(pointcut.descriptor(), advice); + if (oldAdvice != null) { + throw new UnsupportedOperationException( + String.format( + "Advice %s and %s match the same pointcut, this is not yet supported", + oldAdvice, advice)); + } + if (advice instanceof CallSiteAdvice.HasHelpers) { + final String[] helperClassNames = ((CallSiteAdvice.HasHelpers) advice).helperClassNames(); + if (helperClassNames != null) { + Collections.addAll(helpers, helperClassNames); + } + } + } + + /** + * The method will try to discover the {@link CallSiteAdvice} that should be applied to a specific + * type by using the assigned {@link AdviceIntrospector} + */ + public Advices findAdvices(@Nonnull final TypeDescription type, final ClassLoader loader) { + if (advices.isEmpty()) { + return this; + } + return introspector.findAdvices(this, type, loader); + } + + // used for testing + public CallSiteAdvice findAdvice(@Nonnull final Pointcut pointcut) { + return findAdvice(pointcut.type(), pointcut.method(), pointcut.descriptor()); + } + + /** + * The method tries to find the configured advice for the specific method call + * + * @param callee type owning the method + * @param method method name + * @param descriptor descriptor of the method + * @return the advice or {@code null} if none found + */ + public CallSiteAdvice findAdvice( + @Nonnull final String callee, + @Nonnull final String method, + @Nonnull final String descriptor) { + if (advices.isEmpty()) { + return null; + } + final Map> typeAdvices = advices.get(callee); + if (typeAdvices == null) { + return null; + } + final Map methodAdvices = typeAdvices.get(method); + if (methodAdvices == null) { + return null; + } + return methodAdvices.get(descriptor); + } + + public String[] getHelpers() { + return helpers; + } + + public boolean isEmpty() { + return advices.isEmpty(); + } + + /** + * Instance of this class will try to discover the advices required to instrument a class before + * visiting it + */ + public interface AdviceIntrospector { + + @Nonnull + Advices findAdvices( + @Nonnull final Advices advices, + @Nonnull final TypeDescription type, + final ClassLoader classLoader); + + class NoOpAdviceInstrospector implements AdviceIntrospector { + + public static final AdviceIntrospector INSTANCE = new NoOpAdviceInstrospector(); + + @Override + public @Nonnull Advices findAdvices( + final @Nonnull Advices advices, + final @Nonnull TypeDescription type, + final ClassLoader classLoader) { + return advices; + } + } + + /** + * This class will try to parse the constant pool of the class in order to discover the required + * advices to parse it. + */ + class ConstantPoolInstrospector implements AdviceIntrospector { + + public static final AdviceIntrospector INSTANCE = new ConstantPoolInstrospector(); + + private static final int CONSTANT_METHOD_REF = 10; + private static final int CONSTANT_INTERFACE_METHOD_REF = 11; + + private static final Map CP_HANDLERS; + + static { + final Map handlers = new HashMap<>(2); + final ConstantPoolHandler methodRefHandler = new MethodRefHandler(); + handlers.put(CONSTANT_METHOD_REF, methodRefHandler); + handlers.put(CONSTANT_INTERFACE_METHOD_REF, methodRefHandler); + CP_HANDLERS = Collections.unmodifiableMap(handlers); + } + + @Override + public @Nonnull Advices findAdvices( + final @Nonnull Advices advices, + final @Nonnull TypeDescription type, + final ClassLoader classLoader) { + try (final ClassFileLocator classFile = ClassFileLocators.classFileLocator(classLoader)) { + final ClassFileLocator.Resolution resolution = classFile.locate(type.getName()); + if (!resolution.isResolved()) { + return advices; // cannot resolve class file so don't apply any filtering + } + final Map>> adviceMap = new HashMap<>(); + final Set helperSet = new HashSet<>(); + final ClassReader reader = OpenedClassReader.of(resolution.resolve()); + final char[] buffer = new char[reader.getMaxStringLength()]; + for (int i = 0; i < reader.getItemCount(); i++) { + final int offset = reader.getItem(i); + if (offset > 0) { + final int referenceType = reader.readByte(offset - 1); + final ConstantPoolHandler handler = CP_HANDLERS.get(referenceType); + if (handler != null) { + final CallSiteAdvice advice = handler.findAdvice(advices, reader, offset, buffer); + if (advice != null) { + addAdvice(adviceMap, helperSet, advice); + } + } + } + } + return adviceMap.isEmpty() + ? EMPTY + : new Advices(adviceMap, helperSet.toArray(new String[0]), this); + } catch (Throwable e) { + if (LOG.isErrorEnabled()) { + LOG.error(String.format("Failed to introspect %s constant pool", type), e); + } + return advices; // in case of error we should continue without filtering + } + } + + /** Handler for a particular type of constant pool type (MethodRef, InvokeDynamic, ...) */ + private interface ConstantPoolHandler { + CallSiteAdvice findAdvice( + @Nonnull Advices advices, + @Nonnull ClassReader reader, + int offset, + @Nonnull char[] buffer); + } + + /** + * Extracts advices by looking at method refs in the constant pool + * + * @see Methodref_info + */ + private static class MethodRefHandler implements ConstantPoolHandler { + + @Override + public CallSiteAdvice findAdvice( + @Nonnull final Advices advices, + @Nonnull final ClassReader reader, + final int offset, + @Nonnull final char[] buffer) { + final int classIndex = reader.readUnsignedShort(offset); + final int classOffset = reader.getItem(classIndex); + final String className = reader.readUTF8(classOffset, buffer); + final int nameAndTypeIndex = reader.readUnsignedShort(offset + 2); + final int nameAndTypeOffset = reader.getItem(nameAndTypeIndex); + final String name = reader.readUTF8(nameAndTypeOffset, buffer); + final String descriptor = reader.readUTF8(nameAndTypeOffset + 2, buffer); + return advices.findAdvice(className, name, descriptor); + } + } + } + } +} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteInstrumenter.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteInstrumenter.java new file mode 100644 index 00000000000..0e1b5f202a1 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteInstrumenter.java @@ -0,0 +1,57 @@ +package datadog.trace.agent.tooling.bytebuddy.csi; + +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.csi.CallSiteAdvice; +import java.util.ServiceLoader; +import javax.annotation.Nonnull; + +/** + * Instrumented using a {@link CallSiteTransformer} to perform the actual instrumentation. It will + * fetch the instance of the {@link CallSiteAdvice} from SPI according to the specified marker + * interface. + */ +public abstract class CallSiteInstrumenter extends Instrumenter.Default + implements Instrumenter.ForCallSite { + + private final Advices advices; + + /** + * Create a new instance of the instrumenter. + * + * @param spiInterface marker interface implemented by the chosen {@link CallSiteAdvice} instances + */ + public CallSiteInstrumenter( + @Nonnull final Class spiInterface, + @Nonnull final String name, + @Nonnull final String... additionalNames) { + this(fetchAdvicesFromSpi(spiInterface), name, additionalNames); + } + + protected CallSiteInstrumenter( + @Nonnull final Iterable advices, + @Nonnull final String name, + @Nonnull final String... additionalNames) { + super(name, additionalNames); + this.advices = Advices.fromCallSites(advices); + } + + @SuppressWarnings("unchecked") + private static Iterable fetchAdvicesFromSpi( + @Nonnull final Class spiInterface) { + return (ServiceLoader) + ServiceLoader.load(spiInterface, spiInterface.getClassLoader()); + } + + @Override + public String[] helperClassNames() { + return advices.getHelpers(); + } + + @Override + public void adviceTransformations(final AdviceTransformation transformation) {} + + @Override + public AdviceTransformer transformer() { + return new CallSiteTransformer(advices); + } +} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteTransformer.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteTransformer.java new file mode 100644 index 00000000000..55c53841a9f --- /dev/null +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteTransformer.java @@ -0,0 +1,115 @@ +package datadog.trace.agent.tooling.bytebuddy.csi; + +import static net.bytebuddy.jar.asm.ClassWriter.COMPUTE_MAXS; + +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.csi.CallSiteAdvice; +import javax.annotation.Nonnull; +import net.bytebuddy.asm.AsmVisitorWrapper; +import net.bytebuddy.description.field.FieldDescription; +import net.bytebuddy.description.field.FieldList; +import net.bytebuddy.description.method.MethodList; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.implementation.Implementation; +import net.bytebuddy.jar.asm.ClassVisitor; +import net.bytebuddy.jar.asm.MethodVisitor; +import net.bytebuddy.jar.asm.Opcodes; +import net.bytebuddy.pool.TypePool; +import net.bytebuddy.utility.JavaModule; + +public class CallSiteTransformer implements Instrumenter.AdviceTransformer { + + public static final int ASM_API = Opcodes.ASM8; + + private final Advices advices; + + public CallSiteTransformer(@Nonnull final Advices advices) { + this.advices = advices; + } + + @Override + public @Nonnull DynamicType.Builder transform( + @Nonnull final DynamicType.Builder builder, + @Nonnull final TypeDescription type, + final ClassLoader classLoader, + final JavaModule module) { + Advices discovered = advices.findAdvices(type, classLoader); + return discovered.isEmpty() ? builder : builder.visit(new CallSiteVisitorWrapper(discovered)); + } + + private static class CallSiteVisitorWrapper extends AsmVisitorWrapper.AbstractBase { + + private final Advices advices; + + private CallSiteVisitorWrapper(@Nonnull final Advices advices) { + this.advices = advices; + } + + @Override + public int mergeWriter(final int flags) { + return flags | COMPUTE_MAXS; + } + + @Override + public @Nonnull ClassVisitor wrap( + @Nonnull final TypeDescription instrumentedType, + @Nonnull final ClassVisitor classVisitor, + @Nonnull final Implementation.Context implementationContext, + @Nonnull final TypePool typePool, + @Nonnull final FieldList fields, + @Nonnull final MethodList methods, + final int writerFlags, + final int readerFlags) { + return new CallSiteClassVisitor(advices, classVisitor); + } + } + + private static class CallSiteClassVisitor extends ClassVisitor { + + private final Advices advices; + + private CallSiteClassVisitor( + @Nonnull final Advices advices, @Nonnull final ClassVisitor delegated) { + super(ASM_API, delegated); + this.advices = advices; + } + + @Override + public MethodVisitor visitMethod( + final int access, + final String name, + final String descriptor, + final String signature, + final String[] exceptions) { + final MethodVisitor delegated = + super.visitMethod(access, name, descriptor, signature, exceptions); + return new CallSiteMethodVisitor(advices, delegated); + } + } + + private static class CallSiteMethodVisitor extends MethodVisitor { + private final Advices advices; + + private CallSiteMethodVisitor( + @Nonnull final Advices advices, @Nonnull final MethodVisitor delegated) { + super(ASM_API, delegated); + this.advices = advices; + } + + @Override + public void visitMethodInsn( + final int opcode, + final String owner, + final String name, + final String descriptor, + final boolean isInterface) { + CallSiteAdvice advice = advices.findAdvice(owner, name, descriptor); + if (advice != null) { + advice.apply(mv, opcode, owner, name, descriptor, isInterface); + } else { + mv.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + } + } + } +} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/csi/CallSiteAdvice.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/csi/CallSiteAdvice.java new file mode 100644 index 00000000000..c518dbb6331 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/csi/CallSiteAdvice.java @@ -0,0 +1,25 @@ +package datadog.trace.agent.tooling.csi; + +import net.bytebuddy.jar.asm.MethodVisitor; + +/** + * Interface to implement by call site advices, the {@link CallSiteAdvice#apply(MethodVisitor, int, + * String, String, String, boolean)} method will be used by ByteBuddy to perform the actual + * instrumentation. + */ +public interface CallSiteAdvice { + + Pointcut pointcut(); + + void apply( + MethodVisitor mv, + int opcode, + String owner, + String name, + String descriptor, + boolean isInterface); + + interface HasHelpers { + String[] helperClassNames(); + } +} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/csi/Pointcut.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/csi/Pointcut.java new file mode 100644 index 00000000000..fb1b5fdd58d --- /dev/null +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/csi/Pointcut.java @@ -0,0 +1,9 @@ +package datadog.trace.agent.tooling.csi; + +public interface Pointcut { + String type(); + + String method(); + + String descriptor(); +} diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/AdvicesTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/AdvicesTest.groovy new file mode 100644 index 00000000000..817331fd312 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/AdvicesTest.groovy @@ -0,0 +1,137 @@ +package datadog.trace.agent.tooling.csi + +import datadog.trace.agent.tooling.bytebuddy.csi.Advices +import net.bytebuddy.description.type.TypeDescription + +import java.security.MessageDigest + +class AdvicesTest extends BaseCallSiteTest { + + def 'test empty advices'() { + setup: + final advices = Advices.fromCallSites() + + when: + final empty = advices.empty + + then: + empty + advices == Advices.EMPTY + } + + def 'test non empty advices'() { + setup: + final pointcut = buildPointcut(String.getDeclaredMethod('concat', String)) + final advices = Advices.fromCallSites(mockAdvice(pointcut)) + + when: + final empty = advices.empty + + then: + !empty + } + + def 'test advices for type'() { + setup: + final pointcut = buildPointcut(String.getDeclaredMethod('concat', String)) + final advices = Advices.fromCallSites(mockAdvice(pointcut)) + + when: + final existing = advices.findAdvice(pointcut) + + then: + existing != null + + when: + final notFound = advices.findAdvice('java/lang/Integer', pointcut.method(), pointcut.descriptor()) + + then: + notFound == null + } + + def 'test multiple advices for type'() { + setup: + final startsWith1 = buildPointcut(String.getDeclaredMethod('startsWith', String)) + final startsWith2 = buildPointcut(String.getDeclaredMethod('startsWith', String, int.class)) + final advices = Advices.fromCallSites(mockAdvice(startsWith1), mockAdvice(startsWith2)) + + when: + final startsWith1Found = advices.findAdvice(startsWith1) + + then: + startsWith1Found != null + + when: + final startsWith2Found = advices.findAdvice(startsWith2) + + then: + startsWith2Found != null + } + + def 'test helper class names'() { + setup: + final concatAdvice = mockAdvice(buildPointcut(String.getDeclaredMethod('concat', String)), 'foo.bar.Helper1', 'foo.bar.Helper2') + final digestAdvice = mockAdvice(buildPointcut(MessageDigest.getDeclaredMethod('getInstance', String)), 'foo.bar.Helper3') + final advices = Advices.fromCallSites([concatAdvice, digestAdvice]) + + when: + final helpers = advices.helpers + + then: + helpers.toList().containsAll(['foo.bar.Helper1', 'foo.bar.Helper2', 'foo.bar.Helper3']) + } + + def 'test advices with duplicated pointcut'() { + setup: + final pointcut = buildPointcut(String.getDeclaredMethod('concat', String)) + + when: + Advices.fromCallSites(mockAdvice(pointcut), mockAdvice(pointcut)) + + then: + thrown(UnsupportedOperationException) + } + + def 'test no op advice instrospector'() { + setup: + final introspector = Advices.AdviceIntrospector.NoOpAdviceInstrospector.INSTANCE + final advices = Advices.fromCallSites() + + when: + final result = introspector.findAdvices(advices, Mock(TypeDescription), Mock(ClassLoader)) + + then: + result == advices + } + + def 'test constant pool introspector'() { + setup: + final target = StringConcatExample + final targetType = Mock(TypeDescription) { + getName() >> target.name + } + final advice = mockAdvice(pointcutMock) + final advices = Advices.fromCallSites(advice) + final introspector = Advices.AdviceIntrospector.ConstantPoolInstrospector.INSTANCE + + when: 'find is called without a class loader' + def result = introspector.findAdvices(advices, targetType, null) + + then: 'no filtering should happen' + result == advices + + when: 'find is called with a class loader' + result = introspector.findAdvices(advices, targetType, StringConcatExample.classLoader) + final found = result.findAdvice(pointcutMock) != null + + then: 'constant pool should be used to filter' + result != advices + result.empty == emptyAdvices + found == adviceFound + + where: + pointcutMock | emptyAdvices | adviceFound + buildPointcut(String.getDeclaredMethod('concat', String)) | false | true + buildPointcut(MessageDigest.getDeclaredMethod('getInstance', String)) | true | false + } +} diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/BaseCallSiteTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/BaseCallSiteTest.groovy new file mode 100644 index 00000000000..e8fc2792e6f --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/BaseCallSiteTest.groovy @@ -0,0 +1,123 @@ +package datadog.trace.agent.tooling.csi + +import datadog.trace.agent.tooling.bytebuddy.csi.Advices +import datadog.trace.agent.tooling.bytebuddy.csi.CallSiteInstrumenter +import datadog.trace.agent.tooling.bytebuddy.csi.CallSiteTransformer +import datadog.trace.test.util.DDSpecification +import net.bytebuddy.agent.builder.AgentBuilder +import net.bytebuddy.description.type.TypeDescription +import net.bytebuddy.dynamic.DynamicType +import net.bytebuddy.dynamic.loading.ByteArrayClassLoader +import net.bytebuddy.jar.asm.Type +import net.bytebuddy.matcher.ElementMatcher +import net.bytebuddy.utility.JavaModule +import net.bytebuddy.utility.nullability.MaybeNull + +import java.lang.reflect.Method + +import static net.bytebuddy.matcher.ElementMatchers.any +import static net.bytebuddy.matcher.ElementMatchers.named + +class BaseCallSiteTest extends DDSpecification { + + protected CallSiteAdvice mockAdvice(final Pointcut target) { + return Mock(CallSiteAdvice) { + pointcut() >> target + } + } + + protected CallSiteAdvice mockAdvice(final Pointcut target, final String... helpers) { + return Mock(CallSiteAdviceWithHelpers) { + pointcut() >> target + helperClassNames() >> helpers + } + } + + protected Advices mockAdvices(final Collection advices) { + return Mock(Advices) { + isEmpty() >> advices.isEmpty() + findAdvices(_ as TypeDescription, _ as ClassLoader) >> it + findAdvice(_ as String, _ as String, _ as String) >> { params -> + final Object[] args = params as Object[] + advices.find { + final pointcut = it.pointcut() + return pointcut.type() == args[0] as String && + pointcut.method() == args[1] as String && + pointcut.descriptor() == args[2] as String + } + } + } + } + + protected static Pointcut buildPointcut(final Method executable) { + return new Pointcut() { + @Override + String type() { + return Type.getType(executable.getDeclaringClass()).internalName + } + + @Override + String method() { + return executable.name + } + + @Override + String descriptor() { + return Type.getType(executable).descriptor + } + } + } + + protected static CallSiteInstrumenter buildInstrumenter(final Iterable advices, + final ElementMatcher callerType = any()) { + return new CallSiteInstrumenter(advices, 'csi') { + @Override + ElementMatcher callerType() { + return callerType + } + } + } + + protected static CallSiteInstrumenter buildInstrumenter(final Class spiClass, + final ElementMatcher callerType = any()) { + return new CallSiteInstrumenter(spiClass, 'csi') { + @Override + ElementMatcher callerType() { + return callerType + } + } + } + + protected static Object loadType(final Type type, + final byte[] data, + final ClassLoader loader = Thread.currentThread().contextClassLoader) { + final classLoader = new ByteArrayClassLoader(loader, [(type.className): data]) + final Class clazz = classLoader.loadClass(type.className) + return clazz.getConstructor().newInstance() + } + + protected static byte[] transformType(final Type source, + final Type target, + final CallSiteTransformer transformer, + final ClassLoader loader = Thread.currentThread().contextClassLoader) { + final classContent = loader.getResourceAsStream("${source.getInternalName()}.class").bytes + final classFileTransformer = new AgentBuilder.Default() + .type(named(source.className)) + .transform(new AgentBuilder.Transformer() { + @Override + DynamicType.Builder transform(final DynamicType.Builder builder, + final TypeDescription typeDescription, + final @MaybeNull ClassLoader classLoader, + final @MaybeNull JavaModule module) { + return transformer + .transform(builder, typeDescription, classLoader, module) + .name(target.className) + } + }) + .makeRaw() + return classFileTransformer.transform(loader, source.className, null, null, classContent) + } + + interface CallSiteAdviceWithHelpers extends CallSiteAdvice, CallSiteAdvice.HasHelpers { + } +} diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/CallSiteInstrumenterTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/CallSiteInstrumenterTest.groovy new file mode 100644 index 00000000000..ae06a1a6cf0 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/CallSiteInstrumenterTest.groovy @@ -0,0 +1,107 @@ +package datadog.trace.agent.tooling.csi + +import datadog.trace.agent.tooling.Instrumenter +import net.bytebuddy.asm.AsmVisitorWrapper +import net.bytebuddy.description.type.TypeDescription +import net.bytebuddy.dynamic.DynamicType +import net.bytebuddy.jar.asm.MethodVisitor + +class CallSiteInstrumenterTest extends BaseCallSiteTest { + + def 'test instrumenter creates transformer'() { + setup: + final advice = mockAdvice(buildPointcut(String.getDeclaredMethod('concat', String))) + final instrumenter = buildInstrumenter([advice]) + final builder = Mock(DynamicType.Builder) + final type = Mock(TypeDescription) { + getName() >> StringConcatExample.name + } + + when: + final transformer = instrumenter.transformer() + final result = transformer.transform(builder, type, getClass().getClassLoader(), null) + + then: + result == builder + 1 * builder.visit(_ as AsmVisitorWrapper) >> builder + } + + def 'test instrumenter adds no transformations'() { + setup: + final advice = mockAdvice(buildPointcut(String.getDeclaredMethod('concat', String))) + final instrumenter = buildInstrumenter([advice]) + final mock = Mock(Instrumenter.AdviceTransformation) + + when: + instrumenter.adviceTransformations(mock) + + then: + 0 * mock._ + } + + def 'test helper class names'() { + setup: + final advice1 = mockAdvice(buildPointcut(String.getDeclaredMethod('concat', String)), 'foo.bar.Helper1') + final advice2 = mockAdvice(buildPointcut(StringBuilder.getDeclaredMethod('append', String)), 'foo.bar.Helper1', 'foo.bar.Helper2', 'foo.bar.Helper3') + final instrumenter = buildInstrumenter([advice1, advice2]) + + when: + final helpers = instrumenter.helperClassNames() + + then: + helpers.length == 3 + helpers.toList().containsAll('foo.bar.Helper1', 'foo.bar.Helper2', 'foo.bar.Helper3') + } + + def 'test fetch advices from spi with custom class'() { + setup: + final builder = Mock(DynamicType.Builder) + final type = Mock(TypeDescription) { + getName() >> StringConcatExample.name + } + + when: + final instrumenter = buildInstrumenter(TestCallSiteAdvice) + final transformer = instrumenter.transformer() + transformer.transform(builder, type, getClass().getClassLoader(), null) + + then: + transformer != null + 1 * builder.visit(_ as AsmVisitorWrapper) >> builder + } + + def 'test fetch advices from spi with no implementations'() { + setup: + final builder = Mock(DynamicType.Builder) + final type = Mock(TypeDescription) { + getName() >> StringConcatExample.name + } + + when: + final instrumenter = buildInstrumenter(CallSiteAdvice) + final transformer = instrumenter.transformer() + transformer.transform(builder, type, getClass().getClassLoader(), null) + + then: + 0 * builder.visit(_ as AsmVisitorWrapper) >> builder + } + + static class StringConcatAdvice implements TestCallSiteAdvice { + + @Override + Pointcut pointcut() { + return buildPointcut(String.getDeclaredMethod('concat', String)) + } + + @Override + void apply( + final MethodVisitor mv, + final int opcode, + final String owner, + final String name, + final String descriptor, + final boolean isInterface) { + mv.visitMethodInsn(opcode, owner, name, descriptor, isInterface) + } + } +} diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/CallSiteTransformerTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/CallSiteTransformerTest.groovy new file mode 100644 index 00000000000..a4b77f93d0f --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/csi/CallSiteTransformerTest.groovy @@ -0,0 +1,55 @@ +package datadog.trace.agent.tooling.csi + +import datadog.trace.agent.tooling.bytebuddy.csi.CallSiteTransformer +import datadog.trace.api.function.BiFunction +import net.bytebuddy.jar.asm.MethodVisitor +import net.bytebuddy.jar.asm.Opcodes +import net.bytebuddy.jar.asm.Type + +class CallSiteTransformerTest extends BaseCallSiteTest { + + def 'test call site transformer'() { + setup: + final source = Type.getType(StringConcatExample) + final target = renameType(source, 'Test') + final pointcut = buildPointcut(String.getDeclaredMethod('concat', String)) + final callSite = mockAdvice(pointcut) + final callSiteTransformer = new CallSiteTransformer(mockAdvices([callSite])) + + when: + final transformedClass = transformType(source, target, callSiteTransformer) + final instance = loadType(target, transformedClass) as BiFunction + final result = instance.apply("Hello ", "World!") + + then: + 1 * callSite.apply(_ as MethodVisitor, Opcodes.INVOKEVIRTUAL, pointcut.type(), pointcut.method(), pointcut.descriptor(), false) >> { params -> + final args = params as Object[] + final mv = args[0] as MethodVisitor + mv.visitInsn(Opcodes.SWAP) + mv.visitInsn(Opcodes.POP) + mv.visitLdcInsn("Goodbye ") + mv.visitInsn(Opcodes.SWAP) + mv.visitMethodInsn(args[1] as int, args[2] as String, args[3] as String, args[4] as String, args[5] as Boolean) + } + result == "Goodbye World!" + } + + def 'test call site with non matching advice'() { + setup: + final source = Type.getType(StringConcatExample) + final target = renameType(source, 'TestNoAdvices') + final callSiteTransformer = new CallSiteTransformer(mockAdvices([])) + + when: + final transformedClass = transformType(source, target, callSiteTransformer) + final instance = loadType(target, transformedClass) as BiFunction + final result = instance.apply("Hello ", "World!") + + then: + result == "Hello World!" + } + + private static Type renameType(final Type sourceType, final String suffix) { + return Type.getType(sourceType.descriptor.replace('StringConcatExample', "StringConcatExample${suffix}")) + } +} diff --git a/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/StringConcatExample.java b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/StringConcatExample.java new file mode 100644 index 00000000000..78e850f2a07 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/StringConcatExample.java @@ -0,0 +1,10 @@ +package datadog.trace.agent.tooling.csi; + +import datadog.trace.api.function.BiFunction; + +public class StringConcatExample implements BiFunction { + + public String apply(final String first, final String second) { + return first.concat(second); + } +} diff --git a/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/TestCallSiteAdvice.java b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/TestCallSiteAdvice.java new file mode 100644 index 00000000000..ae773f0a871 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/java/datadog/trace/agent/tooling/csi/TestCallSiteAdvice.java @@ -0,0 +1,3 @@ +package datadog.trace.agent.tooling.csi; + +public interface TestCallSiteAdvice extends CallSiteAdvice {} diff --git a/dd-java-agent/agent-tooling/src/test/resources/META-INF/services/datadog.trace.agent.tooling.csi.TestCallSiteAdvice b/dd-java-agent/agent-tooling/src/test/resources/META-INF/services/datadog.trace.agent.tooling.csi.TestCallSiteAdvice new file mode 100644 index 00000000000..49fca3c5039 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/resources/META-INF/services/datadog.trace.agent.tooling.csi.TestCallSiteAdvice @@ -0,0 +1 @@ +datadog.trace.agent.tooling.csi.CallSiteInstrumenterTest$StringConcatAdvice diff --git a/dd-java-agent/agent-tooling/src/test/resources/datadog/trace/agent/tooling/bytebuddy/csi/call_site.trie b/dd-java-agent/agent-tooling/src/test/resources/datadog/trace/agent/tooling/bytebuddy/csi/call_site.trie new file mode 100644 index 00000000000..3b48412e8c0 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/resources/datadog/trace/agent/tooling/bytebuddy/csi/call_site.trie @@ -0,0 +1,41 @@ +# Generates 'CallSiteTrie.java' + +# 0 = allowed elements +# 1 = ignored elements + +# -------- IGNORED ELEMENTS -------- + +# jdk +1 java.util.* +1 java.lang.* +1 java.net.* +1 javax.accessibility.* +1 javax.activation.* +1 javax.annotation.* +1 javax.management.* +1 javax.naming.* +1 javax.net.* +1 javax.script.* +1 javax.servlet.* +1 javax.security.* +1 javax.sql.* +1 javax.swing.* +1 javax.websocket.* +1 sun.* + +# jmh +1 org.openjdk.* + +# libraries +1 ch.qos.* +1 com.blogspot.* +1 com.googlecode.* +1 groovy.* +1 joptsimple.* +1 org.apache.* +1 org.eclipse.* +1 org.codehaus.* +1 org.ietf.* +1 org.slf4j.* +1 org.springframework.* +1 org.w3c.* From 884e615b14de5b2f80dfc3d20e29102a80e1cd74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Tue, 6 Sep 2022 11:10:24 +0200 Subject: [PATCH 2/2] Take care of the PR feedback --- .../tooling/bytebuddy/csi/CallSiteBenchmark.java | 16 ++++++++-------- .../csi/CalleeBenchmarkInstrumenter.java | 3 +-- .../agent/tooling/bytebuddy/csi/Advices.java | 4 ++-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmark.java b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmark.java index 18a52949d6d..89f99cb1fed 100644 --- a/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmark.java +++ b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CallSiteBenchmark.java @@ -62,15 +62,15 @@ enum Type { CALL_SITE("callSite"), CALLEE("callee"); - private final String type; + private final String instrumenter; - Type(final String type) { - this.type = type; + Type(final String instrumenter) { + this.instrumenter = instrumenter; } public void apply(final Instrumentation instrumentation) { - if (type != null) { - System.setProperty("dd.benchmark.instrumentation", type); + if (instrumenter != null) { + System.setProperty("dd.benchmark.instrumentation", instrumenter); AgentInstaller.installBytebuddyAgent(instrumentation); } } @@ -79,10 +79,10 @@ public void validate(final String response) { if (response == null) { throw new RuntimeException("Empty response received"); } - if (type != null && !response.contains("[Transformed]")) { + String expected = instrumenter == null ? "Hello!" : "Hello! [Transformed]"; + if (!expected.equals(response)) { throw new RuntimeException( - String.format( - "Wrong response, expected to contain '[Transformed]' and received '%s'", response)); + String.format("Wrong response, expected '%s' but received '%s'", expected, response)); } } } diff --git a/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CalleeBenchmarkInstrumenter.java b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CalleeBenchmarkInstrumenter.java index 8eed96a4608..8b31c037958 100644 --- a/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CalleeBenchmarkInstrumenter.java +++ b/dd-java-agent/agent-tooling/src/jmh/java/datadog/trace/agent/tooling/bytebuddy/csi/CalleeBenchmarkInstrumenter.java @@ -1,7 +1,6 @@ package datadog.trace.agent.tooling.bytebuddy.csi; import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassesNamed; -import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -25,7 +24,7 @@ public ElementMatcher classLoaderMatcher() { @Override public ElementMatcher hierarchyMatcher() { - return implementsInterface(named("javax.servlet.ServletRequest")); + return named("org.apache.catalina.connector.Request"); } @Override diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/Advices.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/Advices.java index 1c84493e806..b20211b2ffe 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/Advices.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/csi/Advices.java @@ -178,8 +178,8 @@ class NoOpAdviceInstrospector implements AdviceIntrospector { } /** - * This class will try to parse the constant pool of the class in order to discover the required - * advices to parse it. + * This class will try to parse the constant pool of the class in order to discover if any + * configured advices should be applied. */ class ConstantPoolInstrospector implements AdviceIntrospector {