From 8ce765872ba5533d2bf2c2dcfb667503cdccff2a Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 2 Dec 2020 15:51:30 -0500 Subject: [PATCH 1/2] Add Muzzle reference creation for interfaces This will allow muzzle to fail if an implemented interface is missing. --- .../tooling/muzzle/ReferenceCreator.java | 19 ++++++++++++++---- .../groovy/muzzle/ReferenceCreatorTest.groovy | 20 ++++++++++++++++++- .../src/test/java/muzzle/TestClasses.java | 5 ++--- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java index d5455ef3088..08720f4639f 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java @@ -35,6 +35,8 @@ public class ReferenceCreator extends ClassVisitor { */ private static final String REFERENCE_CREATION_PACKAGE = "datadog.trace.instrumentation."; + private static final int UNDEFINED_LINE = -1; + /** * Generate all references reachable from a given class. * @@ -180,9 +182,18 @@ public void visit( refSourceClassName = Utils.getClassName(name); refSourceType = Type.getType("L" + name + ";"); refSourceTypeInternalName = refSourceType.getInternalName(); - // Additional references we could check - // - supertype of class and visible from this package - // - interfaces of class and visible from this package + + // Add references to each of the interfaces. + for (String iface : interfaces) { + addReference( + new Reference.Builder(iface) + .withSource( + refSourceClassName, + UNDEFINED_LINE) // We don't have a specific line number to use. + .withFlag(Reference.Flag.PUBLIC) + .build()); + } + // the super type is handled by the method visitor to the constructor. super.visit(version, access, name, signature, superName, interfaces); } @@ -215,7 +226,7 @@ public MethodVisitor visitMethod( } private class AdviceReferenceMethodVisitor extends MethodVisitor { - private int currentLineNumber = -1; + private int currentLineNumber = UNDEFINED_LINE; public AdviceReferenceMethodVisitor(final MethodVisitor methodVisitor) { super(Opcodes.ASM7, methodVisitor); diff --git a/dd-java-agent/testing/src/test/groovy/muzzle/ReferenceCreatorTest.groovy b/dd-java-agent/testing/src/test/groovy/muzzle/ReferenceCreatorTest.groovy index 7af6299d2bf..7cd6edb181d 100644 --- a/dd-java-agent/testing/src/test/groovy/muzzle/ReferenceCreatorTest.groovy +++ b/dd-java-agent/testing/src/test/groovy/muzzle/ReferenceCreatorTest.groovy @@ -67,6 +67,24 @@ class ReferenceCreatorTest extends AgentTestRunner { references.get('muzzle.TestClasses$MethodBodyAdvice$A') != null } + def "interface impl creates references"() { + setup: + Map references = ReferenceCreator.createReferencesFrom(MethodBodyAdvice.SomeImplementation.getName(), this.getClass().getClassLoader()) + + expect: + references.get('muzzle.TestClasses$MethodBodyAdvice$SomeInterface') != null + references.size() == 1 + } + + def "child class creates references"() { + setup: + Map references = ReferenceCreator.createReferencesFrom(MethodBodyAdvice.A2.getName(), this.getClass().getClassLoader()) + + expect: + references.get('muzzle.TestClasses$MethodBodyAdvice$A') != null + references.size() == 1 + } + def "instanceof creates references"() { setup: Map references = ReferenceCreator.createReferencesFrom(InstanceofAdvice.getName(), this.getClass().getClassLoader()) @@ -82,7 +100,7 @@ class ReferenceCreatorTest extends AgentTestRunner { Map references = ReferenceCreator.createReferencesFrom(TestClasses.InDyAdvice.getName(), this.getClass().getClassLoader()) expect: - references.get('muzzle.TestClasses$MethodBodyAdvice$SomeImplementation') != null + references.get('muzzle.TestClasses$MethodBodyAdvice$HasMethod') != null references.get('muzzle.TestClasses$MethodBodyAdvice$B') != null } diff --git a/dd-java-agent/testing/src/test/java/muzzle/TestClasses.java b/dd-java-agent/testing/src/test/java/muzzle/TestClasses.java index 931d8b098dd..7ee27cc599b 100644 --- a/dd-java-agent/testing/src/test/java/muzzle/TestClasses.java +++ b/dd-java-agent/testing/src/test/java/muzzle/TestClasses.java @@ -95,10 +95,9 @@ public static boolean instanceofMethod(final Object a) { // Can't test this until java 7 is dropped. public static class InDyAdvice { - // public static MethodBodyAdvice.SomeInterface indyMethod( - // final MethodBodyAdvice.SomeImplementation a) { + // public static MethodBodyAdvice.HasMethod indyMethod(final MethodBodyAdvice.HasMethod a) { // Runnable aStaticMethod = MethodBodyAdvice.B::aStaticMethod; - // return a::someMethod; + // return a::requiredMethod; // } } } From 5d9ca5faecad6dcdefef3e85d15a69816715c761 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 3 Dec 2020 15:10:53 -0500 Subject: [PATCH 2/2] Add root to log --- .../src/main/java/datadog/trace/core/PendingTrace.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java b/dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java index 4347c3faabd..aa142749483 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java @@ -240,7 +240,8 @@ private void decrementRefAndMaybeWrite(boolean isRootSpan) { } } if (log.isDebugEnabled()) { - log.debug("t_id={} -> expired reference. pending count={}", traceId, count); + log.debug( + "t_id={} -> expired reference. root={} pending count={}", traceId, isRootSpan, count); } }