From 13c405c1740a3075fe1688fccec543c4d4e3db65 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 24 Nov 2020 19:07:22 +0100 Subject: [PATCH] Make muzzle generate helperClassNames() method (#1714) --- docs/contributing/muzzle.md | 10 +- .../ClassLoaderInstrumentationModule.java | 5 + .../v2_0/CouchbaseInstrumentationModule.java | 7 + ...5TransportClientInstrumentationModule.java | 12 + ...3TransportClientInstrumentationModule.java | 12 + ...6TransportClientInstrumentationModule.java | 12 + .../hystrix/HystrixInstrumentationModule.java | 7 + ...KubernetesClientInstrumentationModule.java | 7 + .../tooling/InstrumentationModule.java | 52 +- .../muzzle/collector/MuzzleCodeGenerator.java | 586 ++++++++++-------- .../ReferenceCollectingClassVisitor.java | 456 ++++++++++++++ .../muzzle/collector/ReferenceCollector.java | 492 +++------------ .../matcher/MuzzleGradlePluginUtil.java | 16 +- .../muzzle/matcher/ReferenceMatcher.java | 14 +- .../muzzle/ReferenceCollectorTest.groovy | 89 ++- .../groovy/muzzle/ReferenceMatcherTest.groovy | 26 +- .../OtherTestHelperClasses.java | 33 + .../java/muzzle/MuzzleWeakReferenceTest.java | 7 +- .../src/test/java/muzzle/TestClasses.java | 7 + 19 files changed, 1114 insertions(+), 736 deletions(-) create mode 100644 javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollectingClassVisitor.java create mode 100644 testing-common/src/test/java/io/opentelemetry/instrumentation/OtherTestHelperClasses.java diff --git a/docs/contributing/muzzle.md b/docs/contributing/muzzle.md index 834d1f6d79..67259d363b 100644 --- a/docs/contributing/muzzle.md +++ b/docs/contributing/muzzle.md @@ -12,7 +12,7 @@ Muzzle will prevent loading an instrumentation if it detects any mismatch or con ## How it works Muzzle has two phases: -* at compile time it collects references to the third-party symbols; +* at compile time it collects references to the third-party symbols and used helper classes; * at runtime it compares those references to the actual API symbols on the classpath. ### Compile-time reference collection @@ -24,12 +24,16 @@ For each instrumentation module the ByteBuddy plugin collects symbols referring third party APIs used by the currently processed module's type instrumentations (`InstrumentationModule#typeInstrumentations()`). The reference collection process starts from advice classes (values of the map returned by the `TypeInstrumentation#transformers()`method) and traverses the class graph until it encounters -a reference to a non-instrumentation class. +a reference to a non-instrumentation class (determined by `InstrumentationClassPredicate`). +Aside from references, the collection process also builds a graph of dependencies between internal +instrumentation helper classes - this dependency graph is later used to construct a list of helper +classes that will be injected to the application classloader (`InstrumentationModule#getMuzzleHelperClassNames()`). All collected references are then used to create a `ReferenceMatcher` instance. This matcher is stored in the instrumentation module class in the method `InstrumentationModule#getMuzzleReferenceMatcher()` and is shared between all type instrumentations. The bytecode of this method (basically an array of -`Reference` builder calls) is generated automatically by the ByteBuddy plugin using an ASM code visitor. +`Reference` builder calls) and the `getMuzzleHelperClassNames()` is generated automatically by the +ByteBuddy plugin using an ASM code visitor. The source code of the compile-time plugin is located in the `javaagent-tooling` module, package `io.opentelemetry.javaagent.tooling.muzzle.collector`. diff --git a/instrumentation/classloaders/src/main/java/io/opentelemetry/javaagent/instrumentation/javaclassloader/ClassLoaderInstrumentationModule.java b/instrumentation/classloaders/src/main/java/io/opentelemetry/javaagent/instrumentation/javaclassloader/ClassLoaderInstrumentationModule.java index df491266ce..0b4b4b5b76 100644 --- a/instrumentation/classloaders/src/main/java/io/opentelemetry/javaagent/instrumentation/javaclassloader/ClassLoaderInstrumentationModule.java +++ b/instrumentation/classloaders/src/main/java/io/opentelemetry/javaagent/instrumentation/javaclassloader/ClassLoaderInstrumentationModule.java @@ -23,6 +23,11 @@ public class ClassLoaderInstrumentationModule extends InstrumentationModule { return new String[] {"io.opentelemetry.javaagent.tooling.Constants"}; } + @Override + protected String[] additionalHelperClassNames() { + return new String[] {"io.opentelemetry.javaagent.tooling.Constants"}; + } + @Override public List typeInstrumentations() { return asList(new ClassLoaderInstrumentation(), new ResourceInjectionInstrumentation()); diff --git a/instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseInstrumentationModule.java b/instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseInstrumentationModule.java index 6f1282467d..dc2adf4eab 100644 --- a/instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseInstrumentationModule.java +++ b/instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseInstrumentationModule.java @@ -18,6 +18,13 @@ public class CouchbaseInstrumentationModule extends InstrumentationModule { super("couchbase", "couchbase-2.0"); } + @Override + protected String[] additionalHelperClassNames() { + return new String[] { + "rx.__OpenTelemetryTracingUtil", + }; + } + @Override public String[] helperClassNames() { return new String[] { diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.0/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_0/Elasticsearch5TransportClientInstrumentationModule.java b/instrumentation/elasticsearch/elasticsearch-transport-5.0/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_0/Elasticsearch5TransportClientInstrumentationModule.java index 4414b5f40e..ddf8fd2678 100644 --- a/instrumentation/elasticsearch/elasticsearch-transport-5.0/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_0/Elasticsearch5TransportClientInstrumentationModule.java +++ b/instrumentation/elasticsearch/elasticsearch-transport-5.0/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_0/Elasticsearch5TransportClientInstrumentationModule.java @@ -34,6 +34,18 @@ public class Elasticsearch5TransportClientInstrumentationModule extends Instrume super("elasticsearch-transport", "elasticsearch-transport-5.0", "elasticsearch"); } + @Override + protected String[] additionalHelperClassNames() { + return new String[] { + // TODO: use Java 8 Collectors.joining() instead + "com.google.common.base.Preconditions", + "com.google.common.base.Joiner", + "com.google.common.base.Joiner$1", + "com.google.common.base.Joiner$2", + "com.google.common.base.Joiner$MapJoiner", + }; + } + @Override public String[] helperClassNames() { return new String[] { diff --git a/instrumentation/elasticsearch/elasticsearch-transport-5.3/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/Elasticsearch53TransportClientInstrumentationModule.java b/instrumentation/elasticsearch/elasticsearch-transport-5.3/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/Elasticsearch53TransportClientInstrumentationModule.java index dd04693a45..a5b2a43c33 100644 --- a/instrumentation/elasticsearch/elasticsearch-transport-5.3/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/Elasticsearch53TransportClientInstrumentationModule.java +++ b/instrumentation/elasticsearch/elasticsearch-transport-5.3/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v5_3/Elasticsearch53TransportClientInstrumentationModule.java @@ -35,6 +35,18 @@ public class Elasticsearch53TransportClientInstrumentationModule extends Instrum super("elasticsearch-transport", "elasticsearch-transport-5.3", "elasticsearch"); } + @Override + protected String[] additionalHelperClassNames() { + return new String[] { + // TODO: use Java 8 Collectors.joining() instead + "com.google.common.base.Preconditions", + "com.google.common.base.Joiner", + "com.google.common.base.Joiner$1", + "com.google.common.base.Joiner$2", + "com.google.common.base.Joiner$MapJoiner", + }; + } + @Override public String[] helperClassNames() { return new String[] { diff --git a/instrumentation/elasticsearch/elasticsearch-transport-6.0/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v6_0/Elasticsearch6TransportClientInstrumentationModule.java b/instrumentation/elasticsearch/elasticsearch-transport-6.0/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v6_0/Elasticsearch6TransportClientInstrumentationModule.java index 7149969686..c7a83f56c0 100644 --- a/instrumentation/elasticsearch/elasticsearch-transport-6.0/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v6_0/Elasticsearch6TransportClientInstrumentationModule.java +++ b/instrumentation/elasticsearch/elasticsearch-transport-6.0/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/v6_0/Elasticsearch6TransportClientInstrumentationModule.java @@ -38,6 +38,18 @@ public class Elasticsearch6TransportClientInstrumentationModule extends Instrume super("elasticsearch-transport", "elasticsearch-transport-6.0", "elasticsearch"); } + @Override + protected String[] additionalHelperClassNames() { + return new String[] { + // TODO: use Java 8 Collectors.joining() instead + "com.google.common.base.Preconditions", + "com.google.common.base.Joiner", + "com.google.common.base.Joiner$1", + "com.google.common.base.Joiner$2", + "com.google.common.base.Joiner$MapJoiner", + }; + } + @Override public String[] helperClassNames() { return new String[] { diff --git a/instrumentation/hystrix-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/hystrix/HystrixInstrumentationModule.java b/instrumentation/hystrix-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/hystrix/HystrixInstrumentationModule.java index 154861bc84..28719b1088 100644 --- a/instrumentation/hystrix-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/hystrix/HystrixInstrumentationModule.java +++ b/instrumentation/hystrix-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/hystrix/HystrixInstrumentationModule.java @@ -38,6 +38,13 @@ public class HystrixInstrumentationModule extends InstrumentationModule { super("hystrix", "hystrix-1.4"); } + @Override + protected String[] additionalHelperClassNames() { + return new String[] { + "rx.__OpenTelemetryTracingUtil", + }; + } + @Override public String[] helperClassNames() { return new String[] { diff --git a/instrumentation/kubernetes-client-7.0/src/main/java/io/opentelemetry/javaagent/instrumentation/kubernetesclient/KubernetesClientInstrumentationModule.java b/instrumentation/kubernetes-client-7.0/src/main/java/io/opentelemetry/javaagent/instrumentation/kubernetesclient/KubernetesClientInstrumentationModule.java index 31a5987adb..41dded1643 100644 --- a/instrumentation/kubernetes-client-7.0/src/main/java/io/opentelemetry/javaagent/instrumentation/kubernetesclient/KubernetesClientInstrumentationModule.java +++ b/instrumentation/kubernetes-client-7.0/src/main/java/io/opentelemetry/javaagent/instrumentation/kubernetesclient/KubernetesClientInstrumentationModule.java @@ -31,6 +31,13 @@ public class KubernetesClientInstrumentationModule extends InstrumentationModule super("kubernetes-client", "kubernetes-client-3.0"); } + @Override + protected String[] additionalHelperClassNames() { + return new String[] { + "com.google.common.base.Strings", + }; + } + @Override public String[] helperClassNames() { return new String[] { diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/InstrumentationModule.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/InstrumentationModule.java index e07140ef78..23b62314da 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/InstrumentationModule.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/InstrumentationModule.java @@ -19,6 +19,7 @@ import io.opentelemetry.javaagent.tooling.bytebuddy.ExceptionHandlers; import io.opentelemetry.javaagent.tooling.context.FieldBackedProvider; import io.opentelemetry.javaagent.tooling.context.InstrumentationContextProvider; import io.opentelemetry.javaagent.tooling.context.NoopContextProvider; +import io.opentelemetry.javaagent.tooling.muzzle.InstrumentationClassPredicate; import io.opentelemetry.javaagent.tooling.muzzle.matcher.Mismatch; import io.opentelemetry.javaagent.tooling.muzzle.matcher.ReferenceMatcher; import java.security.ProtectionDomain; @@ -55,6 +56,12 @@ public abstract class InstrumentationModule { private final Set instrumentationNames; protected final boolean enabled; + /** + * Deprecated, will be removed. + * + * @deprecated Will be removed together with {@link #helperClassNames()} + */ + @Deprecated protected final String packageName = getClass().getPackage() == null ? "" : getClass().getPackage().getName(); @@ -113,7 +120,7 @@ public abstract class InstrumentationModule { return parentAgentBuilder; } - List helperClassNames = asList(helperClassNames()); + List helperClassNames = getAllHelperClassNames(); List helperResourceNames = asList(helperResourceNames()); List typeInstrumentations = typeInstrumentations(); if (typeInstrumentations.isEmpty()) { @@ -160,6 +167,17 @@ public abstract class InstrumentationModule { return agentBuilder; } + /** + * Returns all helper classes that will be injected into the application classloader, both ones + * provided by the implementation and ones that were collected by muzzle during compilation. + */ + public final List getAllHelperClassNames() { + List helperClassNames = new ArrayList<>(); + helperClassNames.addAll(asList(additionalHelperClassNames())); + helperClassNames.addAll(asList(getMuzzleHelperClassNames())); + return helperClassNames; + } + private AgentBuilder.Identified.Extendable applyInstrumentationTransformers( Map, String> transformers, AgentBuilder.Identified.Extendable agentBuilder) { @@ -246,6 +264,30 @@ public abstract class InstrumentationModule { return null; } + /** + * Returns a list of instrumentation helper classes, automatically detected by muzzle during + * compilation. Those helpers will be injected into the application classloader. + * + *

The actual implementation of this method is generated automatically during compilation by + * the {@link io.opentelemetry.javaagent.tooling.muzzle.collector.MuzzleCodeGenerationPlugin} + * ByteBuddy plugin. + * + *

This method is generated automatically, do not override it. + */ + protected String[] getMuzzleHelperClassNames() { + return EMPTY; + } + + /** + * Instrumentation modules can override this method to provide additional helper classes that are + * not located in instrumentation packages described in {@link InstrumentationClassPredicate} (and + * not automatically detected by muzzle). These additional classes will be injected into the + * application classloader first. + */ + protected String[] additionalHelperClassNames() { + return EMPTY; + } + /** * Order of adding instrumentation to ByteBuddy. For example instrumentation with order 1 runs * after an instrumentation with order 0 (default) matched on the same API. @@ -256,7 +298,13 @@ public abstract class InstrumentationModule { return 0; } - /** Returns class names of helpers to inject into the user's classloader. */ + /** + * Deprecated, will be removed. + * + * @deprecated This method is replaced by {@link #getMuzzleHelperClassNames()} and {@link + * #additionalHelperClassNames()}, extending it provides no effect. + */ + @Deprecated public String[] helperClassNames() { return EMPTY; } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/MuzzleCodeGenerator.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/MuzzleCodeGenerator.java index c41ac58719..994eeaec07 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/MuzzleCodeGenerator.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/MuzzleCodeGenerator.java @@ -9,8 +9,7 @@ import io.opentelemetry.javaagent.tooling.InstrumentationModule; import io.opentelemetry.javaagent.tooling.Utils; import io.opentelemetry.javaagent.tooling.muzzle.Reference; import io.opentelemetry.javaagent.tooling.muzzle.matcher.ReferenceMatcher; -import java.util.HashMap; -import java.util.Map; +import java.util.List; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -40,8 +39,9 @@ import net.bytebuddy.pool.TypePool; *

This class is run at compile time by the {@link MuzzleCodeGenerationPlugin} ByteBuddy plugin. */ class MuzzleCodeGenerator implements AsmVisitorWrapper { - public static final String MUZZLE_FIELD_NAME = "muzzleReferenceMatcher"; - public static final String MUZZLE_METHOD_NAME = "getMuzzleReferenceMatcher"; + public static final String MUZZLE_REF_MATCHER_FIELD_NAME = "muzzleReferenceMatcher"; + public static final String MUZZLE_REF_MATCHER_METHOD_NAME = "getMuzzleReferenceMatcher"; + public static final String MUZZLE_HELPER_CLASSES_METHOD_NAME = "getMuzzleHelperClassNames"; @Override public int mergeWriter(int flags) { @@ -106,7 +106,7 @@ class MuzzleCodeGenerator implements AsmVisitorWrapper { @Override public FieldVisitor visitField( int access, String name, String descriptor, String signature, Object value) { - if (MUZZLE_FIELD_NAME.equals(name)) { + if (MUZZLE_REF_MATCHER_FIELD_NAME.equals(name)) { // muzzle field has been generated // by previous compilation // ignore and recompute in visitEnd @@ -118,7 +118,7 @@ class MuzzleCodeGenerator implements AsmVisitorWrapper { @Override public MethodVisitor visitMethod( int access, String name, String descriptor, String signature, String[] exceptions) { - if (MUZZLE_METHOD_NAME.equals(name)) { + if (MUZZLE_REF_MATCHER_METHOD_NAME.equals(name)) { // muzzle getter has been generated // by previous compilation // ignore and recompute in visitEnd @@ -132,212 +132,183 @@ class MuzzleCodeGenerator implements AsmVisitorWrapper { return methodVisitor; } - public Reference[] generateReferences() { - Map references = new HashMap<>(); + @Override + public void visitEnd() { + ReferenceCollector collector = collectReferences(); + generateMuzzleHelperClassNamesMethod(collector); + generateMuzzleReferenceMatcherMethod(collector); + generateMuzzleReferenceMatcherField(); + super.visitEnd(); + } + + private ReferenceCollector collectReferences() { Set adviceClassNames = instrumenter.typeInstrumentations().stream() .flatMap(typeInstrumentation -> typeInstrumentation.transformers().values().stream()) .collect(Collectors.toSet()); + ReferenceCollector collector = new ReferenceCollector(); for (String adviceClass : adviceClassNames) { - for (Map.Entry entry : - ReferenceCollector.collectReferencesFrom(adviceClass).entrySet()) { - if (references.containsKey(entry.getKey())) { - references.put(entry.getKey(), references.get(entry.getKey()).merge(entry.getValue())); - } else { - references.put(entry.getKey(), entry.getValue()); - } - } + collector.collectReferencesFrom(adviceClass); } - return references.values().toArray(new Reference[0]); + return collector; } - @Override - public void visitEnd() { - { // generate getMuzzleReferenceMatcher method - /* - * protected synchronized ReferenceMatcher getMuzzleReferenceMatcher() { - * if (null == this.muzzleReferenceMatcher) { - * this.muzzleReferenceMatcher = new ReferenceMatcher(this.helperClassNames(), - * new Reference[]{ - * //reference builders - * }); - * } - * return this.muzzleReferenceMatcher; - * } - */ - try { - MethodVisitor mv = - super.visitMethod( - Opcodes.ACC_PROTECTED + Opcodes.ACC_SYNCHRONIZED, - MUZZLE_METHOD_NAME, - "()Lio/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher;", - null, - null); - - mv.visitCode(); - Label start = new Label(); - Label ret = new Label(); - Label finish = new Label(); - - mv.visitLabel(start); - mv.visitInsn(Opcodes.ACONST_NULL); - mv.visitVarInsn(Opcodes.ALOAD, 0); - mv.visitFieldInsn( - Opcodes.GETFIELD, - instrumentationClassName, - MUZZLE_FIELD_NAME, - "Lio/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher;"); - mv.visitJumpInsn(Opcodes.IF_ACMPNE, ret); - - mv.visitVarInsn(Opcodes.ALOAD, 0); - - mv.visitTypeInsn( - Opcodes.NEW, "io/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher"); - mv.visitInsn(Opcodes.DUP); - - mv.visitVarInsn(Opcodes.ALOAD, 0); - mv.visitMethodInsn( - Opcodes.INVOKEVIRTUAL, - instrumentationClassName, - "helperClassNames", + private void generateMuzzleHelperClassNamesMethod(ReferenceCollector collector) { + /* + * protected String[] getMuzzleHelperClassNames() { + * return new String[] { + * // sorted helper class names + * }; + * } + */ + MethodVisitor mv = + super.visitMethod( + Opcodes.ACC_PROTECTED, + MUZZLE_HELPER_CLASSES_METHOD_NAME, "()[Ljava/lang/String;", - false); + null, + null); + mv.visitCode(); - Reference[] references = generateReferences(); - mv.visitLdcInsn(references.length); + List helperClassNames = collector.getSortedHelperClasses(); + + mv.visitLdcInsn(helperClassNames.size()); + // stack: size + mv.visitTypeInsn(Opcodes.ANEWARRAY, "java/lang/String"); + // stack: array + for (int i = 0; i < helperClassNames.size(); ++i) { + String helperClassName = helperClassNames.get(i); + + mv.visitInsn(Opcodes.DUP); + // stack: array, array + mv.visitLdcInsn(i); + // stack: array, array, i + mv.visitLdcInsn(helperClassName); + // stack: array, array, i, helperClassName + mv.visitInsn(Opcodes.AASTORE); + // stack: array + } + mv.visitInsn(Opcodes.ARETURN); + + mv.visitMaxs(0, 0); + mv.visitEnd(); + } + + private void generateMuzzleReferenceMatcherMethod(ReferenceCollector collector) { + /* + * protected synchronized ReferenceMatcher getMuzzleReferenceMatcher() { + * if (null == this.muzzleReferenceMatcher) { + * this.muzzleReferenceMatcher = new ReferenceMatcher(this.helperClassNames(), + * new Reference[]{ + * //reference builders + * }); + * } + * return this.muzzleReferenceMatcher; + * } + */ + try { + MethodVisitor mv = + super.visitMethod( + Opcodes.ACC_PROTECTED + Opcodes.ACC_SYNCHRONIZED, + MUZZLE_REF_MATCHER_METHOD_NAME, + "()Lio/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher;", + null, + null); + + mv.visitCode(); + Label start = new Label(); + Label ret = new Label(); + Label finish = new Label(); + + mv.visitLabel(start); + mv.visitInsn(Opcodes.ACONST_NULL); + mv.visitVarInsn(Opcodes.ALOAD, 0); + mv.visitFieldInsn( + Opcodes.GETFIELD, + instrumentationClassName, + MUZZLE_REF_MATCHER_FIELD_NAME, + "Lio/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher;"); + mv.visitJumpInsn(Opcodes.IF_ACMPNE, ret); + + mv.visitVarInsn(Opcodes.ALOAD, 0); + + mv.visitTypeInsn( + Opcodes.NEW, "io/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher"); + mv.visitInsn(Opcodes.DUP); + + mv.visitVarInsn(Opcodes.ALOAD, 0); + mv.visitMethodInsn( + Opcodes.INVOKEVIRTUAL, + instrumentationClassName, + "getAllHelperClassNames", + "()Ljava/util/List;", + false); + + Reference[] references = collector.getReferences().values().toArray(new Reference[0]); + mv.visitLdcInsn(references.length); + mv.visitTypeInsn(Opcodes.ANEWARRAY, "io/opentelemetry/javaagent/tooling/muzzle/Reference"); + + for (int i = 0; i < references.length; ++i) { + mv.visitInsn(Opcodes.DUP); + mv.visitLdcInsn(i); mv.visitTypeInsn( - Opcodes.ANEWARRAY, "io/opentelemetry/javaagent/tooling/muzzle/Reference"); - - for (int i = 0; i < references.length; ++i) { - mv.visitInsn(Opcodes.DUP); - mv.visitLdcInsn(i); - mv.visitTypeInsn( - Opcodes.NEW, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder"); - mv.visitInsn(Opcodes.DUP); - mv.visitLdcInsn(references[i].getClassName()); + Opcodes.NEW, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder"); + mv.visitInsn(Opcodes.DUP); + mv.visitLdcInsn(references[i].getClassName()); + mv.visitMethodInsn( + Opcodes.INVOKESPECIAL, + "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", + "", + "(Ljava/lang/String;)V", + false); + for (Reference.Source source : references[i].getSources()) { + mv.visitLdcInsn(source.getName()); + mv.visitLdcInsn(source.getLine()); mv.visitMethodInsn( - Opcodes.INVOKESPECIAL, + Opcodes.INVOKEVIRTUAL, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", - "", - "(Ljava/lang/String;)V", + "withSource", + "(Ljava/lang/String;I)Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Builder;", false); - for (Reference.Source source : references[i].getSources()) { - mv.visitLdcInsn(source.getName()); - mv.visitLdcInsn(source.getLine()); - mv.visitMethodInsn( - Opcodes.INVOKEVIRTUAL, - "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", - "withSource", - "(Ljava/lang/String;I)Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Builder;", - false); - } - for (Reference.Flag flag : references[i].getFlags()) { - String enumClassName = getEnumClassInternalName(flag); - mv.visitFieldInsn( - Opcodes.GETSTATIC, enumClassName, flag.name(), "L" + enumClassName + ";"); - mv.visitMethodInsn( - Opcodes.INVOKEVIRTUAL, - "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", - "withFlag", - "(Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Flag;)Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Builder;", - false); - } - if (null != references[i].getSuperName()) { - mv.visitLdcInsn(references[i].getSuperName()); - mv.visitMethodInsn( - Opcodes.INVOKEVIRTUAL, - "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", - "withSuperName", - "(Ljava/lang/String;)Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Builder;", - false); - } - for (String interfaceName : references[i].getInterfaces()) { - mv.visitLdcInsn(interfaceName); - mv.visitMethodInsn( - Opcodes.INVOKEVIRTUAL, - "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", - "withInterface", - "(Ljava/lang/String;)Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Builder;", - false); - } - for (Reference.Field field : references[i].getFields()) { - { // sources - mv.visitLdcInsn(field.getSources().size()); - mv.visitTypeInsn( - Opcodes.ANEWARRAY, - "io/opentelemetry/javaagent/tooling/muzzle/Reference$Source"); - - int j = 0; - for (Reference.Source source : field.getSources()) { - mv.visitInsn(Opcodes.DUP); - mv.visitLdcInsn(j); - - mv.visitTypeInsn( - Opcodes.NEW, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Source"); - mv.visitInsn(Opcodes.DUP); - mv.visitLdcInsn(source.getName()); - mv.visitLdcInsn(source.getLine()); - mv.visitMethodInsn( - Opcodes.INVOKESPECIAL, - "io/opentelemetry/javaagent/tooling/muzzle/Reference$Source", - "", - "(Ljava/lang/String;I)V", - false); - - mv.visitInsn(Opcodes.AASTORE); - ++j; - } - } - - { // flags - mv.visitLdcInsn(field.getFlags().size()); - mv.visitTypeInsn( - Opcodes.ANEWARRAY, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Flag"); - - int j = 0; - for (Reference.Flag flag : field.getFlags()) { - mv.visitInsn(Opcodes.DUP); - mv.visitLdcInsn(j); - String enumClassName = getEnumClassInternalName(flag); - mv.visitFieldInsn( - Opcodes.GETSTATIC, enumClassName, flag.name(), "L" + enumClassName + ";"); - mv.visitInsn(Opcodes.AASTORE); - ++j; - } - } - - mv.visitLdcInsn(field.getName()); - - { // field type - mv.visitLdcInsn(field.getType().getDescriptor()); - mv.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(Type.class), - "getType", - Type.getMethodDescriptor(Type.class.getMethod("getType", String.class)), - false); - } - - mv.visitMethodInsn( - Opcodes.INVOKEVIRTUAL, - "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", - "withField", - Type.getMethodDescriptor( - Reference.Builder.class.getMethod( - "withField", - Reference.Source[].class, - Reference.Flag[].class, - String.class, - Type.class)), - false); - } - for (Reference.Method method : references[i].getMethods()) { - mv.visitLdcInsn(method.getSources().size()); + } + for (Reference.Flag flag : references[i].getFlags()) { + String enumClassName = getEnumClassInternalName(flag); + mv.visitFieldInsn( + Opcodes.GETSTATIC, enumClassName, flag.name(), "L" + enumClassName + ";"); + mv.visitMethodInsn( + Opcodes.INVOKEVIRTUAL, + "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", + "withFlag", + "(Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Flag;)Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Builder;", + false); + } + if (null != references[i].getSuperName()) { + mv.visitLdcInsn(references[i].getSuperName()); + mv.visitMethodInsn( + Opcodes.INVOKEVIRTUAL, + "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", + "withSuperName", + "(Ljava/lang/String;)Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Builder;", + false); + } + for (String interfaceName : references[i].getInterfaces()) { + mv.visitLdcInsn(interfaceName); + mv.visitMethodInsn( + Opcodes.INVOKEVIRTUAL, + "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", + "withInterface", + "(Ljava/lang/String;)Lio/opentelemetry/javaagent/tooling/muzzle/Reference$Builder;", + false); + } + for (Reference.Field field : references[i].getFields()) { + { // sources + mv.visitLdcInsn(field.getSources().size()); mv.visitTypeInsn( Opcodes.ANEWARRAY, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Source"); + int j = 0; - for (Reference.Source source : method.getSources()) { + for (Reference.Source source : field.getSources()) { mv.visitInsn(Opcodes.DUP); mv.visitLdcInsn(j); @@ -356,12 +327,15 @@ class MuzzleCodeGenerator implements AsmVisitorWrapper { mv.visitInsn(Opcodes.AASTORE); ++j; } + } - mv.visitLdcInsn(method.getFlags().size()); + { // flags + mv.visitLdcInsn(field.getFlags().size()); mv.visitTypeInsn( Opcodes.ANEWARRAY, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Flag"); - j = 0; - for (Reference.Flag flag : method.getFlags()) { + + int j = 0; + for (Reference.Flag flag : field.getFlags()) { mv.visitInsn(Opcodes.DUP); mv.visitLdcInsn(j); String enumClassName = getEnumClassInternalName(flag); @@ -370,102 +344,166 @@ class MuzzleCodeGenerator implements AsmVisitorWrapper { mv.visitInsn(Opcodes.AASTORE); ++j; } + } - mv.visitLdcInsn(method.getName()); - - { // return type - mv.visitLdcInsn(method.getReturnType().getDescriptor()); - mv.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(Type.class), - "getType", - Type.getMethodDescriptor(Type.class.getMethod("getType", String.class)), - false); - } - - mv.visitLdcInsn(method.getParameterTypes().size()); - mv.visitTypeInsn(Opcodes.ANEWARRAY, Type.getInternalName(Type.class)); - j = 0; - for (Type parameterType : method.getParameterTypes()) { - mv.visitInsn(Opcodes.DUP); - mv.visitLdcInsn(j); - - mv.visitLdcInsn(parameterType.getDescriptor()); - mv.visitMethodInsn( - Opcodes.INVOKESTATIC, - Type.getInternalName(Type.class), - "getType", - Type.getMethodDescriptor(Type.class.getMethod("getType", String.class)), - false); - - mv.visitInsn(Opcodes.AASTORE); - j++; - } + mv.visitLdcInsn(field.getName()); + { // field type + mv.visitLdcInsn(field.getType().getDescriptor()); mv.visitMethodInsn( - Opcodes.INVOKEVIRTUAL, - "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", - "withMethod", - Type.getMethodDescriptor( - Reference.Builder.class.getMethod( - "withMethod", - Reference.Source[].class, - Reference.Flag[].class, - String.class, - Type.class, - Type[].class)), + Opcodes.INVOKESTATIC, + Type.getInternalName(Type.class), + "getType", + Type.getMethodDescriptor(Type.class.getMethod("getType", String.class)), false); } + mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", - "build", - "()Lio/opentelemetry/javaagent/tooling/muzzle/Reference;", + "withField", + Type.getMethodDescriptor( + Reference.Builder.class.getMethod( + "withField", + Reference.Source[].class, + Reference.Flag[].class, + String.class, + Type.class)), false); - mv.visitInsn(Opcodes.AASTORE); } + for (Reference.Method method : references[i].getMethods()) { + mv.visitLdcInsn(method.getSources().size()); + mv.visitTypeInsn( + Opcodes.ANEWARRAY, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Source"); + int j = 0; + for (Reference.Source source : method.getSources()) { + mv.visitInsn(Opcodes.DUP); + mv.visitLdcInsn(j); + mv.visitTypeInsn( + Opcodes.NEW, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Source"); + mv.visitInsn(Opcodes.DUP); + mv.visitLdcInsn(source.getName()); + mv.visitLdcInsn(source.getLine()); + mv.visitMethodInsn( + Opcodes.INVOKESPECIAL, + "io/opentelemetry/javaagent/tooling/muzzle/Reference$Source", + "", + "(Ljava/lang/String;I)V", + false); + + mv.visitInsn(Opcodes.AASTORE); + ++j; + } + + mv.visitLdcInsn(method.getFlags().size()); + mv.visitTypeInsn( + Opcodes.ANEWARRAY, "io/opentelemetry/javaagent/tooling/muzzle/Reference$Flag"); + j = 0; + for (Reference.Flag flag : method.getFlags()) { + mv.visitInsn(Opcodes.DUP); + mv.visitLdcInsn(j); + String enumClassName = getEnumClassInternalName(flag); + mv.visitFieldInsn( + Opcodes.GETSTATIC, enumClassName, flag.name(), "L" + enumClassName + ";"); + mv.visitInsn(Opcodes.AASTORE); + ++j; + } + + mv.visitLdcInsn(method.getName()); + + { // return type + mv.visitLdcInsn(method.getReturnType().getDescriptor()); + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(Type.class), + "getType", + Type.getMethodDescriptor(Type.class.getMethod("getType", String.class)), + false); + } + + mv.visitLdcInsn(method.getParameterTypes().size()); + mv.visitTypeInsn(Opcodes.ANEWARRAY, Type.getInternalName(Type.class)); + j = 0; + for (Type parameterType : method.getParameterTypes()) { + mv.visitInsn(Opcodes.DUP); + mv.visitLdcInsn(j); + + mv.visitLdcInsn(parameterType.getDescriptor()); + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(Type.class), + "getType", + Type.getMethodDescriptor(Type.class.getMethod("getType", String.class)), + false); + + mv.visitInsn(Opcodes.AASTORE); + j++; + } + + mv.visitMethodInsn( + Opcodes.INVOKEVIRTUAL, + "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", + "withMethod", + Type.getMethodDescriptor( + Reference.Builder.class.getMethod( + "withMethod", + Reference.Source[].class, + Reference.Flag[].class, + String.class, + Type.class, + Type[].class)), + false); + } mv.visitMethodInsn( - Opcodes.INVOKESPECIAL, - "io/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher", - "", - "([Ljava/lang/String;[Lio/opentelemetry/javaagent/tooling/muzzle/Reference;)V", + Opcodes.INVOKEVIRTUAL, + "io/opentelemetry/javaagent/tooling/muzzle/Reference$Builder", + "build", + "()Lio/opentelemetry/javaagent/tooling/muzzle/Reference;", false); - mv.visitFieldInsn( - Opcodes.PUTFIELD, - instrumentationClassName, - MUZZLE_FIELD_NAME, - "Lio/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher;"); - - mv.visitLabel(ret); - if (frames) { - mv.visitFrame(Opcodes.F_SAME, 1, null, 0, null); - } - mv.visitVarInsn(Opcodes.ALOAD, 0); - mv.visitFieldInsn( - Opcodes.GETFIELD, - instrumentationClassName, - MUZZLE_FIELD_NAME, - "Lio/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher;"); - mv.visitInsn(Opcodes.ARETURN); - mv.visitLabel(finish); - - mv.visitLocalVariable( - "this", "L" + instrumentationClassName + ";", null, start, finish, 0); - mv.visitMaxs(0, 0); // recomputed - mv.visitEnd(); - } catch (Exception e) { - throw new RuntimeException(e); + mv.visitInsn(Opcodes.AASTORE); } - } + mv.visitMethodInsn( + Opcodes.INVOKESPECIAL, + "io/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher", + "", + "(Ljava/util/List;[Lio/opentelemetry/javaagent/tooling/muzzle/Reference;)V", + false); + mv.visitFieldInsn( + Opcodes.PUTFIELD, + instrumentationClassName, + MUZZLE_REF_MATCHER_FIELD_NAME, + "Lio/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher;"); + + mv.visitLabel(ret); + if (frames) { + mv.visitFrame(Opcodes.F_SAME, 1, null, 0, null); + } + mv.visitVarInsn(Opcodes.ALOAD, 0); + mv.visitFieldInsn( + Opcodes.GETFIELD, + instrumentationClassName, + MUZZLE_REF_MATCHER_FIELD_NAME, + "Lio/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher;"); + mv.visitInsn(Opcodes.ARETURN); + mv.visitLabel(finish); + + mv.visitLocalVariable("this", "L" + instrumentationClassName + ";", null, start, finish, 0); + mv.visitMaxs(0, 0); // recomputed + mv.visitEnd(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + private void generateMuzzleReferenceMatcherField() { super.visitField( Opcodes.ACC_PRIVATE + Opcodes.ACC_VOLATILE, - MUZZLE_FIELD_NAME, + MUZZLE_REF_MATCHER_FIELD_NAME, Type.getDescriptor(ReferenceMatcher.class), null, null); - super.visitEnd(); } private static final Pattern ANONYMOUS_ENUM_CONSTANT_CLASS = @@ -494,7 +532,7 @@ class MuzzleCodeGenerator implements AsmVisitorWrapper { super.visitFieldInsn( Opcodes.PUTFIELD, instrumentationClassName, - MUZZLE_FIELD_NAME, + MUZZLE_REF_MATCHER_FIELD_NAME, Type.getDescriptor(ReferenceMatcher.class)); } super.visitInsn(opcode); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollectingClassVisitor.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollectingClassVisitor.java new file mode 100644 index 0000000000..bf2c036ea4 --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollectingClassVisitor.java @@ -0,0 +1,456 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.muzzle.collector; + +import static io.opentelemetry.javaagent.tooling.muzzle.InstrumentationClassPredicate.isInstrumentationClass; + +import io.opentelemetry.javaagent.tooling.Utils; +import io.opentelemetry.javaagent.tooling.muzzle.Reference; +import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag; +import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag.ManifestationFlag; +import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag.MinimumVisibilityFlag; +import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag.OwnershipFlag; +import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag.VisibilityFlag; +import io.opentelemetry.javaagent.tooling.muzzle.Reference.Source; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.jar.asm.ClassVisitor; +import net.bytebuddy.jar.asm.FieldVisitor; +import net.bytebuddy.jar.asm.Handle; +import net.bytebuddy.jar.asm.Label; +import net.bytebuddy.jar.asm.MethodVisitor; +import net.bytebuddy.jar.asm.Opcodes; +import net.bytebuddy.jar.asm.Type; + +/** Visit a class and collect all references made by the visited class. */ +// Additional things we could check +// - annotations on class +// - outer class +// - inner class +// - cast opcodes in method bodies +class ReferenceCollectingClassVisitor extends ClassVisitor { + + /** + * Get the package of an internal class name. + * + *

foo/bar/Baz -> foo/bar/ + */ + private static String internalPackageName(String internalName) { + return internalName.replaceAll("/[^/]+$", ""); + } + + /** + * Compute the minimum required access for FROM class to access the TO class. + * + * @return A reference flag with the required level of access. + */ + private static MinimumVisibilityFlag computeMinimumClassAccess(Type from, Type to) { + if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { + return MinimumVisibilityFlag.PRIVATE_OR_HIGHER; + } else if (internalPackageName(from.getInternalName()) + .equals(internalPackageName(to.getInternalName()))) { + return MinimumVisibilityFlag.PACKAGE_OR_HIGHER; + } else { + return MinimumVisibilityFlag.PUBLIC; + } + } + + /** + * Compute the minimum required access for FROM class to access a field on the TO class. + * + * @return A reference flag with the required level of access. + */ + private static MinimumVisibilityFlag computeMinimumFieldAccess(Type from, Type to) { + if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { + return MinimumVisibilityFlag.PRIVATE_OR_HIGHER; + } else if (internalPackageName(from.getInternalName()) + .equals(internalPackageName(to.getInternalName()))) { + return MinimumVisibilityFlag.PACKAGE_OR_HIGHER; + } else { + // Additional references: check the type hierarchy of FROM to distinguish public from + // protected + return MinimumVisibilityFlag.PROTECTED_OR_HIGHER; + } + } + + /** + * Compute the minimum required access for FROM class to access METHODTYPE on the TO class. + * + * @return A reference flag with the required level of access. + */ + private static MinimumVisibilityFlag computeMinimumMethodAccess( + Type from, Type to, Type methodType) { + if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { + return MinimumVisibilityFlag.PRIVATE_OR_HIGHER; + } else { + // Additional references: check the type hierarchy of FROM to distinguish public from + // protected + return MinimumVisibilityFlag.PROTECTED_OR_HIGHER; + } + } + + /** + * If TYPE is an array, returns the underlying type. If TYPE is not an array simply return the + * type. + */ + private static Type underlyingType(Type type) { + while (type.getSort() == Type.ARRAY) { + type = type.getElementType(); + } + return type; + } + + private final boolean isAdviceClass; + private final Map references = new LinkedHashMap<>(); + private final Set helperClasses = new HashSet<>(); + // helper super classes which are themselves also helpers + // this is needed for injecting the helper classes into the class loader in the correct order + private final Set helperSuperClasses = new HashSet<>(); + private String refSourceClassName; + private Type refSourceType; + + ReferenceCollectingClassVisitor(boolean isAdviceClass) { + super(Opcodes.ASM7); + this.isAdviceClass = isAdviceClass; + } + + Map getReferences() { + return references; + } + + Set getHelperClasses() { + return helperClasses; + } + + Set getHelperSuperClasses() { + return helperSuperClasses; + } + + private void addExtendsReference(Reference ref) { + addReference(ref); + if (isInstrumentationClass(ref.getClassName())) { + helperSuperClasses.add(ref.getClassName()); + } + } + + private void addReference(Reference ref) { + if (!ref.getClassName().startsWith("java.")) { + Reference reference = references.get(ref.getClassName()); + if (null == reference) { + references.put(ref.getClassName(), ref); + } else { + references.put(ref.getClassName(), reference.merge(ref)); + } + } + if (isInstrumentationClass(ref.getClassName())) { + helperClasses.add(ref.getClassName()); + } + } + + @Override + public void visit( + int version, + int access, + String name, + String signature, + String superName, + String[] interfaces) { + refSourceClassName = Utils.getClassName(name); + refSourceType = Type.getType("L" + name + ";"); + + // class references are not generated for advice classes, only for helper classes + if (!isAdviceClass) { + String fixedSuperClassName = Utils.getClassName(superName); + + addExtendsReference( + new Reference.Builder(fixedSuperClassName).withSource(refSourceClassName).build()); + + List fixedInterfaceNames = new ArrayList<>(interfaces.length); + for (String interfaceName : interfaces) { + String fixedInterfaceName = Utils.getClassName(interfaceName); + fixedInterfaceNames.add(fixedInterfaceName); + + addExtendsReference( + new Reference.Builder(fixedInterfaceName).withSource(refSourceClassName).build()); + } + + addReference( + new Reference.Builder(refSourceClassName) + .withSource(refSourceClassName) + .withSuperName(fixedSuperClassName) + .withInterfaces(fixedInterfaceNames) + .withFlag(computeTypeManifestationFlag(access)) + .build()); + } + + super.visit(version, access, name, signature, superName, interfaces); + } + + @Override + public FieldVisitor visitField( + int access, String name, String descriptor, String signature, Object value) { + // Additional references we could check + // - annotations on field + + // intentionally not creating refs to fields here. + // Will create refs in method instructions to include line numbers. + return super.visitField(access, name, descriptor, signature, value); + } + + @Override + public MethodVisitor visitMethod( + int access, String name, String descriptor, String signature, String[] exceptions) { + + // declared method references are not generated for advice classes, only for helper classes + if (!isAdviceClass) { + Type methodType = Type.getMethodType(descriptor); + + Flag visibilityFlag = computeVisibilityFlag(access); + Flag ownershipFlag = computeOwnershipFlag(access); + Flag manifestationFlag = computeTypeManifestationFlag(access); + + // as an optimization skip constructors, private and static methods + if (!(visibilityFlag == VisibilityFlag.PRIVATE + || ownershipFlag == OwnershipFlag.STATIC + || MethodDescription.CONSTRUCTOR_INTERNAL_NAME.equals(name))) { + addReference( + new Reference.Builder(refSourceClassName) + .withSource(refSourceClassName) + .withMethod( + new Source[0], + new Flag[] {visibilityFlag, ownershipFlag, manifestationFlag}, + name, + methodType.getReturnType(), + methodType.getArgumentTypes()) + .build()); + } + } + + // Additional references we could check + // - Classes in signature (return type, params) and visible from this package + return new AdviceReferenceMethodVisitor( + super.visitMethod(access, name, descriptor, signature, exceptions)); + } + + private static VisibilityFlag computeVisibilityFlag(int access) { + if (VisibilityFlag.PUBLIC.matches(access)) { + return VisibilityFlag.PUBLIC; + } else if (VisibilityFlag.PROTECTED.matches(access)) { + return VisibilityFlag.PROTECTED; + } else if (VisibilityFlag.PACKAGE.matches(access)) { + return VisibilityFlag.PACKAGE; + } else { + return VisibilityFlag.PRIVATE; + } + } + + private static OwnershipFlag computeOwnershipFlag(int access) { + if (OwnershipFlag.STATIC.matches(access)) { + return OwnershipFlag.STATIC; + } else { + return OwnershipFlag.NON_STATIC; + } + } + + private static ManifestationFlag computeTypeManifestationFlag(int access) { + if (ManifestationFlag.ABSTRACT.matches(access)) { + return ManifestationFlag.ABSTRACT; + } else if (ManifestationFlag.FINAL.matches(access)) { + return ManifestationFlag.FINAL; + } else { + return ManifestationFlag.NON_FINAL; + } + } + + private class AdviceReferenceMethodVisitor extends MethodVisitor { + private int currentLineNumber = -1; + + public AdviceReferenceMethodVisitor(MethodVisitor methodVisitor) { + super(Opcodes.ASM7, methodVisitor); + } + + @Override + public void visitLineNumber(int line, Label start) { + currentLineNumber = line; + super.visitLineNumber(line, start); + } + + @Override + public void visitFieldInsn(int opcode, String owner, String name, String descriptor) { + // Additional references we could check + // * DONE owner class + // * DONE owner class has a field (name) + // * DONE field is static or non-static + // * DONE field's visibility from this point (NON_PRIVATE?) + // * DONE owner class's visibility from this point (NON_PRIVATE?) + // + // * DONE field-source class (descriptor) + // * DONE field-source visibility from this point (PRIVATE?) + + Type ownerType = + owner.startsWith("[") + ? underlyingType(Type.getType(owner)) + : Type.getType("L" + owner + ";"); + Type fieldType = Type.getType(descriptor); + + List fieldFlags = new ArrayList<>(); + fieldFlags.add(computeMinimumFieldAccess(refSourceType, ownerType)); + fieldFlags.add( + opcode == Opcodes.GETSTATIC || opcode == Opcodes.PUTSTATIC + ? OwnershipFlag.STATIC + : OwnershipFlag.NON_STATIC); + + addReference( + new Reference.Builder(ownerType.getInternalName()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag(computeMinimumClassAccess(refSourceType, ownerType)) + .withField( + new Reference.Source[] { + new Reference.Source(refSourceClassName, currentLineNumber) + }, + fieldFlags.toArray(new Reference.Flag[0]), + name, + fieldType) + .build()); + + Type underlyingFieldType = underlyingType(fieldType); + if (underlyingFieldType.getSort() == Type.OBJECT) { + addReference( + new Reference.Builder(underlyingFieldType.getInternalName()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag(computeMinimumClassAccess(refSourceType, underlyingFieldType)) + .build()); + } + super.visitFieldInsn(opcode, owner, name, descriptor); + } + + @Override + public void visitMethodInsn( + int opcode, String owner, String name, String descriptor, boolean isInterface) { + // Additional references we could check + // * DONE name of method owner's class + // * DONE is the owner an interface? + // * DONE owner's access from here (PRIVATE?) + // * DONE method on the owner class + // * DONE is the method static? Is it visible from here? + // * Class names from the method descriptor + // * params classes + // * return type + Type methodType = Type.getMethodType(descriptor); + + { // ref for method return type + Type returnType = underlyingType(methodType.getReturnType()); + if (returnType.getSort() == Type.OBJECT) { + addReference( + new Reference.Builder(returnType.getInternalName()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag(computeMinimumClassAccess(refSourceType, returnType)) + .build()); + } + } + // refs for method param types + for (Type paramType : methodType.getArgumentTypes()) { + paramType = underlyingType(paramType); + if (paramType.getSort() == Type.OBJECT) { + addReference( + new Reference.Builder(paramType.getInternalName()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag(computeMinimumClassAccess(refSourceType, paramType)) + .build()); + } + } + + Type ownerType = + owner.startsWith("[") + ? underlyingType(Type.getType(owner)) + : Type.getType("L" + owner + ";"); + + List methodFlags = new ArrayList<>(); + methodFlags.add( + opcode == Opcodes.INVOKESTATIC ? OwnershipFlag.STATIC : OwnershipFlag.NON_STATIC); + methodFlags.add(computeMinimumMethodAccess(refSourceType, ownerType, methodType)); + + addReference( + new Reference.Builder(ownerType.getInternalName()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag(isInterface ? ManifestationFlag.INTERFACE : ManifestationFlag.NON_INTERFACE) + .withFlag(computeMinimumClassAccess(refSourceType, ownerType)) + .withMethod( + new Reference.Source[] { + new Reference.Source(refSourceClassName, currentLineNumber) + }, + methodFlags.toArray(new Reference.Flag[0]), + name, + methodType.getReturnType(), + methodType.getArgumentTypes()) + .build()); + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + } + + @Override + public void visitTypeInsn(int opcode, String type) { + Type typeObj = underlyingType(Type.getObjectType(type)); + if (typeObj.getSort() == Type.OBJECT) { + addReference( + new Reference.Builder(typeObj.getInternalName()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag(computeMinimumClassAccess(refSourceType, typeObj)) + .build()); + } + super.visitTypeInsn(opcode, type); + } + + @Override + public void visitInvokeDynamicInsn( + String name, + String descriptor, + Handle bootstrapMethodHandle, + Object... bootstrapMethodArguments) { + // This part might be unnecessary... + addReference( + new Reference.Builder(bootstrapMethodHandle.getOwner()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag( + computeMinimumClassAccess( + refSourceType, Type.getObjectType(bootstrapMethodHandle.getOwner()))) + .build()); + for (Object arg : bootstrapMethodArguments) { + if (arg instanceof Handle) { + Handle handle = (Handle) arg; + addReference( + new Reference.Builder(handle.getOwner()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag( + computeMinimumClassAccess( + refSourceType, Type.getObjectType(handle.getOwner()))) + .build()); + } + } + super.visitInvokeDynamicInsn( + name, descriptor, bootstrapMethodHandle, bootstrapMethodArguments); + } + + @Override + public void visitLdcInsn(Object value) { + if (value instanceof Type) { + Type type = underlyingType((Type) value); + if (type.getSort() == Type.OBJECT) { + addReference( + new Reference.Builder(type.getInternalName()) + .withSource(refSourceClassName, currentLineNumber) + .withFlag(computeMinimumClassAccess(refSourceType, type)) + .build()); + } + } + super.visitLdcInsn(value); + } + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java index ee500b53c2..aaf61c7ed9 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java @@ -8,44 +8,33 @@ package io.opentelemetry.javaagent.tooling.muzzle.collector; import static com.google.common.base.Preconditions.checkNotNull; import static io.opentelemetry.javaagent.tooling.muzzle.InstrumentationClassPredicate.isInstrumentationClass; +import com.google.common.graph.Graph; +import com.google.common.graph.GraphBuilder; +import com.google.common.graph.Graphs; +import com.google.common.graph.MutableGraph; import io.opentelemetry.javaagent.tooling.Utils; import io.opentelemetry.javaagent.tooling.muzzle.Reference; -import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag; -import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag.ManifestationFlag; -import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag.MinimumVisibilityFlag; -import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag.OwnershipFlag; -import io.opentelemetry.javaagent.tooling.muzzle.Reference.Flag.VisibilityFlag; -import io.opentelemetry.javaagent.tooling.muzzle.Reference.Source; import java.io.IOException; import java.io.InputStream; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Queue; import java.util.Set; -import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.jar.asm.ClassReader; -import net.bytebuddy.jar.asm.ClassVisitor; -import net.bytebuddy.jar.asm.FieldVisitor; -import net.bytebuddy.jar.asm.Handle; -import net.bytebuddy.jar.asm.Label; -import net.bytebuddy.jar.asm.MethodVisitor; -import net.bytebuddy.jar.asm.Opcodes; -import net.bytebuddy.jar.asm.Type; -/** Visit a class and collect all references made by the visited class. */ -// Additional things we could check -// - annotations on class -// - outer class -// - inner class -// - cast opcodes in method bodies -public class ReferenceCollector extends ClassVisitor { +public class ReferenceCollector { + private final Map references = new HashMap<>(); + private final MutableGraph helperSuperClassGraph = GraphBuilder.directed().build(); + private final Set visitedClasses = new HashSet<>(); + /** - * Traverse a graph of classes starting from {@code entryPointClassName} (usually an advice class) - * and collect all references to both internal (instrumentation) and external classes. + * Traverse a graph of classes starting from {@code adviceClassName} and collect all references to + * both internal (instrumentation) and external classes. * *

The graph of classes is traversed until a non-instrumentation (external) class is * encountered. @@ -53,452 +42,113 @@ public class ReferenceCollector extends ClassVisitor { *

This class is only called at compile time by the {@link MuzzleCodeGenerationPlugin} * ByteBuddy plugin. * - * @param entryPointClassName Starting point for generating references. - * @return Map of [referenceClassName to Reference] + * @param adviceClassName Starting point for generating references. * @see io.opentelemetry.javaagent.tooling.muzzle.InstrumentationClassPredicate */ - public static Map collectReferencesFrom(String entryPointClassName) { - Set visitedSources = new HashSet<>(); - Map references = new HashMap<>(); - + public void collectReferencesFrom(String adviceClassName) { Queue instrumentationQueue = new ArrayDeque<>(); - instrumentationQueue.add(entryPointClassName); + instrumentationQueue.add(adviceClassName); - boolean isEntryPoint = true; + boolean isAdviceClass = true; while (!instrumentationQueue.isEmpty()) { - String className = instrumentationQueue.remove(); - visitedSources.add(className); + String visitedClassName = instrumentationQueue.remove(); + visitedClasses.add(visitedClassName); try (InputStream in = checkNotNull( ReferenceCollector.class .getClassLoader() - .getResourceAsStream(Utils.getResourceName(className)), + .getResourceAsStream(Utils.getResourceName(visitedClassName)), "Couldn't find class file %s", - className)) { + visitedClassName)) { - // only start from method bodies for entry point class (skips class/method references) - ReferenceCollector cv = new ReferenceCollector(isEntryPoint); + // only start from method bodies for the advice class (skips class/method references) + ReferenceCollectingClassVisitor cv = new ReferenceCollectingClassVisitor(isAdviceClass); ClassReader reader = new ClassReader(in); reader.accept(cv, ClassReader.SKIP_FRAMES); - Map instrumentationReferences = cv.getReferences(); - for (Map.Entry entry : instrumentationReferences.entrySet()) { - String key = entry.getKey(); + for (Map.Entry entry : cv.getReferences().entrySet()) { + String refClassName = entry.getKey(); + Reference reference = entry.getValue(); + // Don't generate references created outside of the instrumentation package. - if (!visitedSources.contains(entry.getKey()) && isInstrumentationClass(key)) { - instrumentationQueue.add(key); - } - if (references.containsKey(key)) { - references.put(key, references.get(key).merge(entry.getValue())); - } else { - references.put(key, entry.getValue()); + if (!visitedClasses.contains(refClassName) && isInstrumentationClass(refClassName)) { + instrumentationQueue.add(refClassName); } + addReference(refClassName, reference); } + collectHelperClasses( + isAdviceClass, visitedClassName, cv.getHelperClasses(), cv.getHelperSuperClasses()); } catch (IOException e) { - throw new IllegalStateException("Error reading class " + className, e); + throw new IllegalStateException("Error reading class " + visitedClassName, e); } - if (isEntryPoint) { - isEntryPoint = false; + if (isAdviceClass) { + isAdviceClass = false; } } - return references; } - /** - * Get the package of an internal class name. - * - *

foo/bar/Baz -> foo/bar/ - */ - private static String internalPackageName(String internalName) { - return internalName.replaceAll("/[^/]+$", ""); - } - - /** - * Compute the minimum required access for FROM class to access the TO class. - * - * @return A reference flag with the required level of access. - */ - private static MinimumVisibilityFlag computeMinimumClassAccess(Type from, Type to) { - if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { - return MinimumVisibilityFlag.PRIVATE_OR_HIGHER; - } else if (internalPackageName(from.getInternalName()) - .equals(internalPackageName(to.getInternalName()))) { - return MinimumVisibilityFlag.PACKAGE_OR_HIGHER; + private void addReference(String refClassName, Reference reference) { + if (references.containsKey(refClassName)) { + references.put(refClassName, references.get(refClassName).merge(reference)); } else { - return MinimumVisibilityFlag.PUBLIC; + references.put(refClassName, reference); } } - /** - * Compute the minimum required access for FROM class to access a field on the TO class. - * - * @return A reference flag with the required level of access. - */ - private static MinimumVisibilityFlag computeMinimumFieldAccess(Type from, Type to) { - if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { - return MinimumVisibilityFlag.PRIVATE_OR_HIGHER; - } else if (internalPackageName(from.getInternalName()) - .equals(internalPackageName(to.getInternalName()))) { - return MinimumVisibilityFlag.PACKAGE_OR_HIGHER; - } else { - // Additional references: check the type hierarchy of FROM to distinguish public from - // protected - return MinimumVisibilityFlag.PROTECTED_OR_HIGHER; + private void collectHelperClasses( + boolean isAdviceClass, + String className, + Set helperClasses, + Set helperSuperClasses) { + for (String helperClass : helperClasses) { + helperSuperClassGraph.addNode(helperClass); } - } - - /** - * Compute the minimum required access for FROM class to access METHODTYPE on the TO class. - * - * @return A reference flag with the required level of access. - */ - private static MinimumVisibilityFlag computeMinimumMethodAccess( - Type from, Type to, Type methodType) { - if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { - return MinimumVisibilityFlag.PRIVATE_OR_HIGHER; - } else { - // Additional references: check the type hierarchy of FROM to distinguish public from - // protected - return MinimumVisibilityFlag.PROTECTED_OR_HIGHER; + if (!isAdviceClass) { + for (String helperSuperClass : helperSuperClasses) { + helperSuperClassGraph.putEdge(className, helperSuperClass); + } } } - /** - * If TYPE is an array, returns the underlying type. If TYPE is not an array simply return the - * type. - */ - private static Type underlyingType(Type type) { - while (type.getSort() == Type.ARRAY) { - type = type.getElementType(); - } - return type; - } - - private final boolean skipClassReferenceGeneration; - private final Map references = new HashMap<>(); - private String refSourceClassName; - private Type refSourceType; - - private ReferenceCollector(boolean skipClassReferenceGeneration) { - super(Opcodes.ASM7); - this.skipClassReferenceGeneration = skipClassReferenceGeneration; - } - public Map getReferences() { return references; } - private void addReference(Reference ref) { - if (!ref.getClassName().startsWith("java.")) { - Reference reference = references.get(ref.getClassName()); - if (null == reference) { - references.put(ref.getClassName(), ref); - } else { - references.put(ref.getClassName(), reference.merge(ref)); - } - } - } + // see https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm + public List getSortedHelperClasses() { + MutableGraph dependencyGraph = Graphs.copyOf(Graphs.transpose(helperSuperClassGraph)); + List helperClasses = new ArrayList<>(dependencyGraph.nodes().size()); - @Override - public void visit( - int version, - int access, - String name, - String signature, - String superName, - String[] interfaces) { - refSourceClassName = Utils.getClassName(name); - refSourceType = Type.getType("L" + name + ";"); + Queue helpersWithNoDeps = findAllHelperClassesWithoutDependencies(dependencyGraph); - // class references are not generated for advice classes, only for helper classes - if (!skipClassReferenceGeneration) { - String fixedSuperClassName = Utils.getClassName(superName); + while (!helpersWithNoDeps.isEmpty()) { + String helperClass = helpersWithNoDeps.remove(); + helperClasses.add(helperClass); - addReference( - new Reference.Builder(fixedSuperClassName).withSource(refSourceClassName).build()); - - List fixedInterfaceNames = new ArrayList<>(interfaces.length); - for (String interfaceName : interfaces) { - String fixedInterfaceName = Utils.getClassName(interfaceName); - fixedInterfaceNames.add(fixedInterfaceName); - - addReference( - new Reference.Builder(fixedInterfaceName).withSource(refSourceClassName).build()); - } - - addReference( - new Reference.Builder(refSourceClassName) - .withSource(refSourceClassName) - .withSuperName(fixedSuperClassName) - .withInterfaces(fixedInterfaceNames) - .withFlag(computeTypeManifestationFlag(access)) - .build()); - } - - super.visit(version, access, name, signature, superName, interfaces); - } - - @Override - public FieldVisitor visitField( - int access, String name, String descriptor, String signature, Object value) { - // Additional references we could check - // - annotations on field - - // intentionally not creating refs to fields here. - // Will create refs in method instructions to include line numbers. - return super.visitField(access, name, descriptor, signature, value); - } - - @Override - public MethodVisitor visitMethod( - int access, String name, String descriptor, String signature, String[] exceptions) { - - // declared method references are not generated for advice classes, only for helper classes - if (!skipClassReferenceGeneration) { - Type methodType = Type.getMethodType(descriptor); - - Flag visibilityFlag = computeVisibilityFlag(access); - Flag ownershipFlag = computeOwnershipFlag(access); - Flag manifestationFlag = computeTypeManifestationFlag(access); - - // as an optimization skip constructors, private and static methods - if (!(visibilityFlag == VisibilityFlag.PRIVATE - || ownershipFlag == OwnershipFlag.STATIC - || MethodDescription.CONSTRUCTOR_INTERNAL_NAME.equals(name))) { - addReference( - new Reference.Builder(refSourceClassName) - .withSource(refSourceClassName) - .withMethod( - new Source[0], - new Flag[] {visibilityFlag, ownershipFlag, manifestationFlag}, - name, - methodType.getReturnType(), - methodType.getArgumentTypes()) - .build()); - } - } - - // Additional references we could check - // - Classes in signature (return type, params) and visible from this package - return new AdviceReferenceMethodVisitor( - super.visitMethod(access, name, descriptor, signature, exceptions)); - } - - private static VisibilityFlag computeVisibilityFlag(int access) { - if (VisibilityFlag.PUBLIC.matches(access)) { - return VisibilityFlag.PUBLIC; - } else if (VisibilityFlag.PROTECTED.matches(access)) { - return VisibilityFlag.PROTECTED; - } else if (VisibilityFlag.PACKAGE.matches(access)) { - return VisibilityFlag.PACKAGE; - } else { - return VisibilityFlag.PRIVATE; - } - } - - private static OwnershipFlag computeOwnershipFlag(int access) { - if (OwnershipFlag.STATIC.matches(access)) { - return OwnershipFlag.STATIC; - } else { - return OwnershipFlag.NON_STATIC; - } - } - - private static ManifestationFlag computeTypeManifestationFlag(int access) { - if (ManifestationFlag.ABSTRACT.matches(access)) { - return ManifestationFlag.ABSTRACT; - } else if (ManifestationFlag.FINAL.matches(access)) { - return ManifestationFlag.FINAL; - } else { - return ManifestationFlag.NON_FINAL; - } - } - - private class AdviceReferenceMethodVisitor extends MethodVisitor { - private int currentLineNumber = -1; - - public AdviceReferenceMethodVisitor(MethodVisitor methodVisitor) { - super(Opcodes.ASM7, methodVisitor); - } - - @Override - public void visitLineNumber(int line, Label start) { - currentLineNumber = line; - super.visitLineNumber(line, start); - } - - @Override - public void visitFieldInsn(int opcode, String owner, String name, String descriptor) { - // Additional references we could check - // * DONE owner class - // * DONE owner class has a field (name) - // * DONE field is static or non-static - // * DONE field's visibility from this point (NON_PRIVATE?) - // * DONE owner class's visibility from this point (NON_PRIVATE?) - // - // * DONE field-source class (descriptor) - // * DONE field-source visibility from this point (PRIVATE?) - - Type ownerType = - owner.startsWith("[") - ? underlyingType(Type.getType(owner)) - : Type.getType("L" + owner + ";"); - Type fieldType = Type.getType(descriptor); - - List fieldFlags = new ArrayList<>(); - fieldFlags.add(computeMinimumFieldAccess(refSourceType, ownerType)); - fieldFlags.add( - opcode == Opcodes.GETSTATIC || opcode == Opcodes.PUTSTATIC - ? OwnershipFlag.STATIC - : OwnershipFlag.NON_STATIC); - - addReference( - new Reference.Builder(ownerType.getInternalName()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag(computeMinimumClassAccess(refSourceType, ownerType)) - .withField( - new Reference.Source[] { - new Reference.Source(refSourceClassName, currentLineNumber) - }, - fieldFlags.toArray(new Reference.Flag[0]), - name, - fieldType) - .build()); - - Type underlyingFieldType = underlyingType(fieldType); - if (underlyingFieldType.getSort() == Type.OBJECT) { - addReference( - new Reference.Builder(underlyingFieldType.getInternalName()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag(computeMinimumClassAccess(refSourceType, underlyingFieldType)) - .build()); - } - super.visitFieldInsn(opcode, owner, name, descriptor); - } - - @Override - public void visitMethodInsn( - int opcode, String owner, String name, String descriptor, boolean isInterface) { - // Additional references we could check - // * DONE name of method owner's class - // * DONE is the owner an interface? - // * DONE owner's access from here (PRIVATE?) - // * DONE method on the owner class - // * DONE is the method static? Is it visible from here? - // * Class names from the method descriptor - // * params classes - // * return type - Type methodType = Type.getMethodType(descriptor); - - { // ref for method return type - Type returnType = underlyingType(methodType.getReturnType()); - if (returnType.getSort() == Type.OBJECT) { - addReference( - new Reference.Builder(returnType.getInternalName()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag(computeMinimumClassAccess(refSourceType, returnType)) - .build()); + Set dependencies = new HashSet<>(dependencyGraph.successors(helperClass)); + for (String dependency : dependencies) { + dependencyGraph.removeEdge(helperClass, dependency); + if (dependencyGraph.predecessors(dependency).isEmpty()) { + helpersWithNoDeps.add(dependency); } } - // refs for method param types - for (Type paramType : methodType.getArgumentTypes()) { - paramType = underlyingType(paramType); - if (paramType.getSort() == Type.OBJECT) { - addReference( - new Reference.Builder(paramType.getInternalName()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag(computeMinimumClassAccess(refSourceType, paramType)) - .build()); - } - } - - Type ownerType = - owner.startsWith("[") - ? underlyingType(Type.getType(owner)) - : Type.getType("L" + owner + ";"); - - List methodFlags = new ArrayList<>(); - methodFlags.add( - opcode == Opcodes.INVOKESTATIC ? OwnershipFlag.STATIC : OwnershipFlag.NON_STATIC); - methodFlags.add(computeMinimumMethodAccess(refSourceType, ownerType, methodType)); - - addReference( - new Reference.Builder(ownerType.getInternalName()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag(isInterface ? ManifestationFlag.INTERFACE : ManifestationFlag.NON_INTERFACE) - .withFlag(computeMinimumClassAccess(refSourceType, ownerType)) - .withMethod( - new Reference.Source[] { - new Reference.Source(refSourceClassName, currentLineNumber) - }, - methodFlags.toArray(new Reference.Flag[0]), - name, - methodType.getReturnType(), - methodType.getArgumentTypes()) - .build()); - super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); } - @Override - public void visitTypeInsn(int opcode, String type) { - Type typeObj = underlyingType(Type.getObjectType(type)); - if (typeObj.getSort() == Type.OBJECT) { - addReference( - new Reference.Builder(typeObj.getInternalName()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag(computeMinimumClassAccess(refSourceType, typeObj)) - .build()); - } - super.visitTypeInsn(opcode, type); - } + return helperClasses; + } - @Override - public void visitInvokeDynamicInsn( - String name, - String descriptor, - Handle bootstrapMethodHandle, - Object... bootstrapMethodArguments) { - // This part might be unnecessary... - addReference( - new Reference.Builder(bootstrapMethodHandle.getOwner()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag( - computeMinimumClassAccess( - refSourceType, Type.getObjectType(bootstrapMethodHandle.getOwner()))) - .build()); - for (Object arg : bootstrapMethodArguments) { - if (arg instanceof Handle) { - Handle handle = (Handle) arg; - addReference( - new Reference.Builder(handle.getOwner()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag( - computeMinimumClassAccess( - refSourceType, Type.getObjectType(handle.getOwner()))) - .build()); - } + private static Queue findAllHelperClassesWithoutDependencies( + Graph dependencyGraph) { + Queue helpersWithNoDeps = new LinkedList<>(); + for (String helperClass : dependencyGraph.nodes()) { + if (dependencyGraph.predecessors(helperClass).isEmpty()) { + helpersWithNoDeps.add(helperClass); } - super.visitInvokeDynamicInsn( - name, descriptor, bootstrapMethodHandle, bootstrapMethodArguments); - } - - @Override - public void visitLdcInsn(Object value) { - if (value instanceof Type) { - Type type = underlyingType((Type) value); - if (type.getSort() == Type.OBJECT) { - addReference( - new Reference.Builder(type.getInternalName()) - .withSource(refSourceClassName, currentLineNumber) - .withFlag(computeMinimumClassAccess(refSourceType, type)) - .build()); - } - } - super.visitLdcInsn(value); } + return helpersWithNoDeps; } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/MuzzleGradlePluginUtil.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/MuzzleGradlePluginUtil.java index 7260b723a3..cd63e2dd6e 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/MuzzleGradlePluginUtil.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/MuzzleGradlePluginUtil.java @@ -10,6 +10,7 @@ import io.opentelemetry.javaagent.tooling.InstrumentationModule; import io.opentelemetry.javaagent.tooling.muzzle.Reference; import java.io.IOException; import java.lang.reflect.Method; +import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -30,8 +31,9 @@ public final class MuzzleGradlePluginUtil { *

  • {@code userClassLoader} is not matched by the {@link * InstrumentationModule#classLoaderMatcher()} method *
  • {@link ReferenceMatcher} of any instrumentation module finds any mismatch - *
  • any helper class defined in {@link InstrumentationModule#helperClassNames()} fails to be - * injected into {@code userClassLoader} + *
  • any helper class defined in {@link InstrumentationModule#getMuzzleHelperClassNames()} or + * {@link InstrumentationModule#additionalHelperClassNames()} fails to be injected into + * {@code userClassLoader} * * *

    When {@code assertPass = false} this method behaves in an opposite way: failure in any of @@ -92,11 +94,11 @@ public final class MuzzleGradlePluginUtil { ServiceLoader.load(InstrumentationModule.class, agentClassLoader)) { try { // verify helper injector works - String[] helperClassNames = instrumentationModule.helperClassNames(); - if (helperClassNames.length > 0) { + List allHelperClasses = instrumentationModule.getAllHelperClassNames(); + if (!allHelperClasses.isEmpty()) { new HelperInjector( MuzzleGradlePluginUtil.class.getSimpleName(), - createHelperMap(helperClassNames, agentClassLoader)) + createHelperMap(allHelperClasses, agentClassLoader)) .transform(null, null, userClassLoader, null); } } catch (Exception e) { @@ -109,8 +111,8 @@ public final class MuzzleGradlePluginUtil { } private static Map createHelperMap( - String[] helperClassNames, ClassLoader agentClassLoader) throws IOException { - Map helperMap = new LinkedHashMap<>(helperClassNames.length); + Collection helperClassNames, ClassLoader agentClassLoader) throws IOException { + Map helperMap = new LinkedHashMap<>(helperClassNames.size()); for (String helperName : helperClassNames) { ClassFileLocator locator = ClassFileLocator.ForClassLoader.of(agentClassLoader); byte[] classBytes = locator.locate(helperName).resolve(); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher.java index c8a944cf6f..2281913326 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/ReferenceMatcher.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.tooling.muzzle.matcher; import static io.opentelemetry.javaagent.tooling.muzzle.InstrumentationClassPredicate.isInstrumentationClass; +import static java.util.Collections.emptyList; import static net.bytebuddy.dynamic.loading.ClassLoadingStrategy.BOOTSTRAP_LOADER; import com.google.common.collect.Sets; @@ -17,7 +18,6 @@ import io.opentelemetry.javaagent.tooling.muzzle.Reference.Source; import io.opentelemetry.javaagent.tooling.muzzle.matcher.HelperReferenceWrapper.Factory; import io.opentelemetry.javaagent.tooling.muzzle.matcher.HelperReferenceWrapper.Method; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -38,15 +38,15 @@ public final class ReferenceMatcher { private final Set helperClassNames; public ReferenceMatcher(Reference... references) { - this(new String[0], references); + this(emptyList(), references); } - public ReferenceMatcher(String[] helperClassNames, Reference[] references) { + public ReferenceMatcher(List helperClassNames, Reference[] references) { this.references = new HashMap<>(references.length); for (Reference reference : references) { this.references.put(reference.getClassName(), reference); } - this.helperClassNames = new HashSet<>(Arrays.asList(helperClassNames)); + this.helperClassNames = new HashSet<>(helperClassNames); } Collection getReferences() { @@ -88,7 +88,7 @@ public final class ReferenceMatcher { loader = Utils.getBootstrapProxy(); } - List mismatches = Collections.emptyList(); + List mismatches = emptyList(); for (Reference reference : references.values()) { mismatches = lazyAddAll(mismatches, checkMatch(reference, loader)); @@ -119,7 +119,7 @@ public final class ReferenceMatcher { } else if (helperClassNames.contains(reference.getClassName())) { // skip muzzle check for those helper classes that are not in instrumentation packages; e.g. // some instrumentations inject guava types as helper classes - return Collections.emptyList(); + return emptyList(); } else { TypePool.Resolution resolution = typePool.describe(reference.getClassName()); if (!resolution.isResolved()) { @@ -146,7 +146,7 @@ public final class ReferenceMatcher { // for helper classes we make sure that all abstract methods from super classes and interfaces are // implemented private List checkHelperClassMatch(Reference helperClass, TypePool typePool) { - List mismatches = Collections.emptyList(); + List mismatches = emptyList(); HelperReferenceWrapper helperWrapper = new Factory(typePool, references).create(helperClass); diff --git a/testing-common/src/test/groovy/muzzle/ReferenceCollectorTest.groovy b/testing-common/src/test/groovy/muzzle/ReferenceCollectorTest.groovy index d2e5d5ebc0..af55535836 100644 --- a/testing-common/src/test/groovy/muzzle/ReferenceCollectorTest.groovy +++ b/testing-common/src/test/groovy/muzzle/ReferenceCollectorTest.groovy @@ -13,6 +13,7 @@ import static muzzle.TestClasses.HelperAdvice import static muzzle.TestClasses.LdcAdvice import static muzzle.TestClasses.MethodBodyAdvice +import io.opentelemetry.instrumentation.OtherTestHelperClasses import io.opentelemetry.instrumentation.TestHelperClasses import io.opentelemetry.instrumentation.test.AgentTestRunner import io.opentelemetry.javaagent.tooling.muzzle.Reference @@ -21,7 +22,9 @@ import io.opentelemetry.javaagent.tooling.muzzle.collector.ReferenceCollector class ReferenceCollectorTest extends AgentTestRunner { def "method body creates references"() { setup: - def references = ReferenceCollector.collectReferencesFrom(MethodBodyAdvice.name) + def collector = new ReferenceCollector() + collector.collectReferencesFrom(MethodBodyAdvice.name) + def references = collector.getReferences() expect: references.keySet() == [ @@ -65,7 +68,9 @@ class ReferenceCollectorTest extends AgentTestRunner { def "protected ref test"() { setup: - def references = ReferenceCollector.collectReferencesFrom(MethodBodyAdvice.B2.name) + def collector = new ReferenceCollector() + collector.collectReferencesFrom(MethodBodyAdvice.B2.name) + def references = collector.getReferences() expect: assertMethod references[MethodBodyAdvice.B.name], 'protectedMethod', '()V', @@ -75,7 +80,9 @@ class ReferenceCollectorTest extends AgentTestRunner { def "ldc creates references"() { setup: - def references = ReferenceCollector.collectReferencesFrom(LdcAdvice.name) + def collector = new ReferenceCollector() + collector.collectReferencesFrom(LdcAdvice.name) + def references = collector.getReferences() expect: references[MethodBodyAdvice.A.name] != null @@ -83,7 +90,9 @@ class ReferenceCollectorTest extends AgentTestRunner { def "instanceof creates references"() { setup: - def references = ReferenceCollector.collectReferencesFrom(TestClasses.InstanceofAdvice.name) + def collector = new ReferenceCollector() + collector.collectReferencesFrom(TestClasses.InstanceofAdvice.name) + def references = collector.getReferences() expect: references[MethodBodyAdvice.A.name] != null @@ -91,7 +100,9 @@ class ReferenceCollectorTest extends AgentTestRunner { def "invokedynamic creates references"() { setup: - def references = ReferenceCollector.collectReferencesFrom(TestClasses.InvokeDynamicAdvice.name) + def collector = new ReferenceCollector() + collector.collectReferencesFrom(TestClasses.InvokeDynamicAdvice.name) + def references = collector.getReferences() expect: references['muzzle.TestClasses$MethodBodyAdvice$SomeImplementation'] != null @@ -100,7 +111,9 @@ class ReferenceCollectorTest extends AgentTestRunner { def "should create references for helper classes"() { when: - def references = ReferenceCollector.collectReferencesFrom(HelperAdvice.name) + def collector = new ReferenceCollector() + collector.collectReferencesFrom(HelperAdvice.name) + def references = collector.getReferences() then: references.keySet() == [ @@ -130,6 +143,55 @@ class ReferenceCollectorTest extends AgentTestRunner { } } + def "should find all helper classes"() { + when: + def collector = new ReferenceCollector() + collector.collectReferencesFrom(HelperAdvice.name) + def helperClasses = collector.getSortedHelperClasses() + + then: + assertThatContainsInOrder helperClasses, [ + TestHelperClasses.HelperInterface.name, + TestHelperClasses.Helper.name + ] + assertThatContainsInOrder helperClasses, [ + TestHelperClasses.HelperSuperClass.name, + TestHelperClasses.Helper.name + ] + } + + def "should correctly find helper classes from multiple advice classes"() { + when: + def collector = new ReferenceCollector() + collector.collectReferencesFrom(TestClasses.HelperAdvice.name) + collector.collectReferencesFrom(TestClasses.HelperOtherAdvice.name) + def helperClasses = collector.getSortedHelperClasses() + + then: + assertThatContainsInOrder helperClasses, [ + TestHelperClasses.HelperInterface.name, + TestHelperClasses.Helper.name + ] + assertThatContainsInOrder helperClasses, [ + TestHelperClasses.HelperSuperClass.name, + TestHelperClasses.Helper.name + ] + assertThatContainsInOrder helperClasses, [ + OtherTestHelperClasses.TestEnum.name, + OtherTestHelperClasses.TestEnum.name + '$1', + ] + new HashSet<>(helperClasses) == new HashSet([ + TestHelperClasses.HelperSuperClass.name, + TestHelperClasses.HelperInterface.name, + TestHelperClasses.Helper.name, + OtherTestHelperClasses.Bar.name, + OtherTestHelperClasses.Foo.name, + OtherTestHelperClasses.TestEnum.name, + OtherTestHelperClasses.TestEnum.name + '$1', + OtherTestHelperClasses.name + '$1', + ]) + } + private static assertHelperSuperClassMethod(Reference reference, boolean isAbstract) { assertMethod reference, 'abstractMethod', '()I', VisibilityFlag.PROTECTED, @@ -171,4 +233,19 @@ class ReferenceCollectorTest extends AgentTestRunner { } return null } + + private static assertThatContainsInOrder(List list, List sublist) { + def listIt = list.iterator() + def sublistIt = sublist.iterator() + while (listIt.hasNext() && sublistIt.hasNext()) { + def sublistElem = sublistIt.next() + while (listIt.hasNext()) { + def listElem = listIt.next() + if (listElem == sublistElem) { + break + } + } + } + return !sublistIt.hasNext() + } } diff --git a/testing-common/src/test/groovy/muzzle/ReferenceMatcherTest.groovy b/testing-common/src/test/groovy/muzzle/ReferenceMatcherTest.groovy index 914578fec1..9a81dd3904 100644 --- a/testing-common/src/test/groovy/muzzle/ReferenceMatcherTest.groovy +++ b/testing-common/src/test/groovy/muzzle/ReferenceMatcherTest.groovy @@ -46,9 +46,9 @@ class ReferenceMatcherTest extends AgentTestRunner { def "match safe classpaths"() { setup: - Reference[] refs = ReferenceCollector.collectReferencesFrom(MethodBodyAdvice.name) - .values() - .toArray(new Reference[0]) + def collector = new ReferenceCollector() + collector.collectReferencesFrom(MethodBodyAdvice.name) + Reference[] refs = collector.getReferences().values().toArray(new Reference[0]) def refMatcher = new ReferenceMatcher(refs) expect: @@ -83,9 +83,11 @@ class ReferenceMatcherTest extends AgentTestRunner { MethodBodyAdvice.SomeInterface, MethodBodyAdvice.SomeImplementation)] as URL[], (ClassLoader) null) - Reference[] refs = ReferenceCollector.collectReferencesFrom(MethodBodyAdvice.name) - .values() - .toArray(new Reference[0]) + + def collector = new ReferenceCollector() + collector.collectReferencesFrom(MethodBodyAdvice.name) + Reference[] refs = collector.getReferences().values().toArray(new Reference[0]) + def refMatcher1 = new ReferenceMatcher(refs) def refMatcher2 = new ReferenceMatcher(refs) assert getMismatchClassSet(refMatcher1.getMismatchedReferenceSources(cl)).empty @@ -170,7 +172,7 @@ class ReferenceMatcherTest extends AgentTestRunner { .build() when: - def mismatches = new ReferenceMatcher([reference.className] as String[], [reference] as Reference[]) + def mismatches = new ReferenceMatcher([reference.className], [reference] as Reference[]) .getMismatchedReferenceSources(emptyClassLoader) then: @@ -186,7 +188,7 @@ class ReferenceMatcherTest extends AgentTestRunner { .build() when: - def mismatches = new ReferenceMatcher([reference.className] as String[], [reference] as Reference[]) + def mismatches = new ReferenceMatcher([reference.className], [reference] as Reference[]) .getMismatchedReferenceSources(this.class.classLoader) then: @@ -201,7 +203,7 @@ class ReferenceMatcherTest extends AgentTestRunner { .build() when: - def mismatches = new ReferenceMatcher([reference.className] as String[], [reference] as Reference[]) + def mismatches = new ReferenceMatcher([reference.className], [reference] as Reference[]) .getMismatchedReferenceSources(this.class.classLoader) then: @@ -216,7 +218,7 @@ class ReferenceMatcherTest extends AgentTestRunner { .build() when: - def mismatches = new ReferenceMatcher([reference.className] as String[], [reference] as Reference[]) + def mismatches = new ReferenceMatcher([reference.className], [reference] as Reference[]) .getMismatchedReferenceSources(this.class.classLoader) then: @@ -233,7 +235,7 @@ class ReferenceMatcherTest extends AgentTestRunner { .build() when: - def mismatches = new ReferenceMatcher([reference.className, emptySuperClassRef.className] as String[], [reference, emptySuperClassRef] as Reference[]) + def mismatches = new ReferenceMatcher([reference.className, emptySuperClassRef.className], [reference, emptySuperClassRef] as Reference[]) .getMismatchedReferenceSources(this.class.classLoader) then: @@ -255,7 +257,7 @@ class ReferenceMatcherTest extends AgentTestRunner { .build() when: - def mismatches = new ReferenceMatcher([helper.className, baseHelper.className] as String[], [helper, baseHelper] as Reference[]) + def mismatches = new ReferenceMatcher([helper.className, baseHelper.className], [helper, baseHelper] as Reference[]) .getMismatchedReferenceSources(this.class.classLoader) then: diff --git a/testing-common/src/test/java/io/opentelemetry/instrumentation/OtherTestHelperClasses.java b/testing-common/src/test/java/io/opentelemetry/instrumentation/OtherTestHelperClasses.java new file mode 100644 index 0000000000..a71afc8cc4 --- /dev/null +++ b/testing-common/src/test/java/io/opentelemetry/instrumentation/OtherTestHelperClasses.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation; + +import muzzle.TestClasses; + +public class OtherTestHelperClasses { + public static class Foo implements TestClasses.MethodBodyAdvice.SomeInterface { + @Override + public void someMethod() {} + } + + public static class Bar { + public void doSomething() { + new Foo().someMethod(); + TestEnum.INSTANCE.getAnswer(); + } + } + + public enum TestEnum { + INSTANCE { + @Override + int getAnswer() { + return 42; + } + }; + + abstract int getAnswer(); + } +} diff --git a/testing-common/src/test/java/muzzle/MuzzleWeakReferenceTest.java b/testing-common/src/test/java/muzzle/MuzzleWeakReferenceTest.java index fbad69a135..0def39505c 100644 --- a/testing-common/src/test/java/muzzle/MuzzleWeakReferenceTest.java +++ b/testing-common/src/test/java/muzzle/MuzzleWeakReferenceTest.java @@ -22,10 +22,9 @@ public class MuzzleWeakReferenceTest { public static boolean classLoaderRefIsGarbageCollected() throws InterruptedException { ClassLoader loader = new URLClassLoader(new URL[0], null); WeakReference clRef = new WeakReference<>(loader); - Reference[] refs = - ReferenceCollector.collectReferencesFrom(TestClasses.MethodBodyAdvice.class.getName()) - .values() - .toArray(new Reference[0]); + ReferenceCollector collector = new ReferenceCollector(); + collector.collectReferencesFrom(TestClasses.MethodBodyAdvice.class.getName()); + Reference[] refs = collector.getReferences().values().toArray(new Reference[0]); ReferenceMatcher refMatcher = new ReferenceMatcher(refs); refMatcher.getMismatchedReferenceSources(loader); loader = null; diff --git a/testing-common/src/test/java/muzzle/TestClasses.java b/testing-common/src/test/java/muzzle/TestClasses.java index a0168188b9..3ce2310806 100644 --- a/testing-common/src/test/java/muzzle/TestClasses.java +++ b/testing-common/src/test/java/muzzle/TestClasses.java @@ -5,6 +5,7 @@ package muzzle; +import io.opentelemetry.instrumentation.OtherTestHelperClasses; import io.opentelemetry.instrumentation.TestHelperClasses.Helper; import net.bytebuddy.asm.Advice; @@ -100,4 +101,10 @@ public class TestClasses { Helper h = new Helper(); } } + + public static class HelperOtherAdvice { + public static void adviceMethod() { + new OtherTestHelperClasses.Bar().doSomething(); + } + } }