Muzzle optimization
MODIFY
javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/muzzle/Reference.java
javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/muzzle/Reference.java
@@ -22,6 +22,11 @@ import org.checkerframework.checker.nullness.qual.Nullable;
* bytecode generated based on them would also be deterministic.
*/
public final class Reference {
+
+ // this could be exposed as a system property if needed, but for now it's just helpful to be able
+ // to change manually here when reviewing/optimizing the generated getMuzzleReferences() method
+ private static final boolean COLLECT_SOURCES = true;
+
private final Set<Source> sources;
private final String className;
private final String superName;
@@ -198,6 +203,7 @@ public final class Reference {
boolean matches(int asmFlags);
// This method is internally used to generate the getMuzzleReferenceMatcher() implementation
+
/** Same as {@link Enum#name()}. */
String name();
@@ -351,7 +357,7 @@ public final class Reference {
public Method(
Source[] sources, Flag[] flags, String name, Type returnType, Type[] parameterTypes) {
this(
- new LinkedHashSet<>(Arrays.asList(sources)),
+ COLLECT_SOURCES ? new LinkedHashSet<>(Arrays.asList(sources)) : new LinkedHashSet<>(),
new LinkedHashSet<>(Arrays.asList(flags)),
name,
returnType,
@@ -442,7 +448,8 @@ public final class Reference {
private final boolean declared;
public Field(Source[] sources, Flag[] flags, String name, Type fieldType, boolean declared) {
- this.sources = new LinkedHashSet<>(Arrays.asList(sources));
+ this.sources =
+ COLLECT_SOURCES ? new LinkedHashSet<>(Arrays.asList(sources)) : new LinkedHashSet<>();
this.flags = new LinkedHashSet<>(Arrays.asList(flags));
this.name = name;
this.type = fieldType;
@@ -541,7 +548,9 @@ public final class Reference {
}
public Builder withSource(String sourceName, int line) {
+ if (COLLECT_SOURCES) {
sources.add(new Source(sourceName, line));
+ }
return this;
}

MODIFY
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationClassPredicate.java
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationClassPredicate.java
@@ -42,6 +42,20 @@ public final class InstrumentationClassPredicate {
|| additionalLibraryInstrumentationPredicate.test(className);
}
+ public boolean isProvidedByLibrary(String className) {
+ return !isInstrumentationClass(className) && !isProvidedByJavaagent(className);
+ }
+
+ private boolean isProvidedByJavaagent(String className) {
+ return className.startsWith(JAVAAGENT_API_PACKAGE)
+ || className.startsWith(INSTRUMENTATION_API_PACKAGE)
+ || className.startsWith("io.opentelemetry.javaagent.bootstrap.")
+ || className.startsWith("io.opentelemetry.api.")
+ || className.startsWith("io.opentelemetry.context.")
+ || className.startsWith("io.opentelemetry.semconv.")
+ || className.startsWith("org.slf4j.");
+ }
+
private static boolean isJavaagentInstrumentationClass(String className) {
return className.startsWith(JAVAAGENT_INSTRUMENTATION_PACKAGE)
&& !className.startsWith(JAVAAGENT_API_PACKAGE);

MODIFY
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/MuzzleCodeGenerator.java
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/MuzzleCodeGenerator.java
@@ -190,6 +190,7 @@ class MuzzleCodeGenerator implements AsmVisitorWrapper {
for (String resource : instrumentationModule.helperResourceNames()) {
collector.collectReferencesFromResource(resource);
}
+ collector.prune();
return collector;
}

MODIFY
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollectingClassVisitor.java
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollectingClassVisitor.java
@@ -21,7 +21,6 @@ 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;
@@ -233,10 +232,6 @@ class ReferenceCollectingClassVisitor extends ClassVisitor {
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)
@@ -248,7 +243,6 @@ class ReferenceCollectingClassVisitor extends ClassVisitor {
methodType.getArgumentTypes())
.build());
}
- }
// Additional references we could check
// - Classes in signature (return type, params) and visible from this package

MODIFY
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/collector/ReferenceCollector.java
@@ -26,6 +26,7 @@ import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
@@ -34,7 +35,9 @@ import java.util.Queue;
import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Pattern;
+import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.jar.asm.ClassReader;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* {@link LinkedHashMap} is used for reference map to guarantee a deterministic order of iteration,
@@ -201,6 +204,95 @@ public class ReferenceCollector {
return references;
}
+ public void prune() {
+ // helper classes that may help another helper class implement an abstract library method
+ // must be retained
+ // for example if helper class A extends helper class B, and A also implements a library
+ // interface L, then B needs to be retained so that it can be used at runtime to verify that A
+ // implements all of L's methods.
+ // Super types of A that are not also helper classes do not need to be retained because they can
+ // be looked up on the classpath at runtime, see HelperReferenceWrapper.create().
+ Set<Reference> helperClassesParticipatingInLibrarySuperType =
+ getHelperClassesParticipatingInLibrarySuperType();
+
+ for (Iterator<Reference> i = references.values().iterator(); i.hasNext(); ) {
+ Reference reference = i.next();
+ if (instrumentationClassPredicate.isProvidedByLibrary(reference.getClassName())) {
+ // these are the references to library classes which need to be checked at runtime
+ continue;
+ }
+ if (helperClassesParticipatingInLibrarySuperType.contains(reference)) {
+ // these need to be kept in order to check that abstract methods are implemented,
+ // and to check that declared super class fields are present
+ //
+ // can at least prune constructors, private, and static methods, since those cannot be used
+ // to help implement an abstract library method
+ reference
+ .getMethods()
+ .removeIf(
+ method ->
+ method.getName().equals(MethodDescription.CONSTRUCTOR_INTERNAL_NAME)
+ || method.getFlags().contains(Reference.Flag.VisibilityFlag.PRIVATE)
+ || method.getFlags().contains(Reference.Flag.OwnershipFlag.STATIC));
+ continue;
+ }
+ i.remove();
+ }
+ }
+
+ private Set<Reference> getHelperClassesParticipatingInLibrarySuperType() {
+ Set<Reference> helperClassesParticipatingInLibrarySuperType = new HashSet<>();
+ for (Reference reference : getHelperClassesWithLibrarySuperType()) {
+ addSuperTypesThatAreAlsoHelperClasses(
+ reference.getClassName(), helperClassesParticipatingInLibrarySuperType);
+ }
+ return helperClassesParticipatingInLibrarySuperType;
+ }
+
+ private Set<Reference> getHelperClassesWithLibrarySuperType() {
+ Set<Reference> helperClassesWithLibrarySuperType = new HashSet<>();
+ for (Reference reference : references.values()) {
+ if (instrumentationClassPredicate.isInstrumentationClass(reference.getClassName())
+ && hasLibrarySuperType(reference.getClassName())) {
+ helperClassesWithLibrarySuperType.add(reference);
+ }
+ }
+ return helperClassesWithLibrarySuperType;
+ }
+
+ private void addSuperTypesThatAreAlsoHelperClasses(
+ @Nullable String className, Set<Reference> superTypes) {
+ if (className != null && instrumentationClassPredicate.isInstrumentationClass(className)) {
+ Reference reference = references.get(className);
+ superTypes.add(reference);
+
+ addSuperTypesThatAreAlsoHelperClasses(reference.getSuperName(), superTypes);
+ // need to keep interfaces too since they may have default methods
+ for (String superType : reference.getInterfaces()) {
+ addSuperTypesThatAreAlsoHelperClasses(superType, superTypes);
+ }
+ }
+ }
+
+ private boolean hasLibrarySuperType(@Nullable String typeName) {
+ if (typeName == null || typeName.startsWith("java.")) {
+ return false;
+ }
+ if (instrumentationClassPredicate.isProvidedByLibrary(typeName)) {
+ return true;
+ }
+ Reference reference = references.get(typeName);
+ if (hasLibrarySuperType(reference.getSuperName())) {
+ return true;
+ }
+ for (String type : reference.getInterfaces()) {
+ if (hasLibrarySuperType(type)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
// see https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm
public List<String> getSortedHelperClasses() {
MutableGraph<String> dependencyGraph = Graphs.copyOf(Graphs.transpose(helperSuperClassGraph));

MODIFY
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/HelperReferenceWrapper.java
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/HelperReferenceWrapper.java
@@ -142,6 +142,10 @@ interface HelperReferenceWrapper {
if (resolution.isResolved()) {
return new ClasspathType(resolution.resolve());
}
+ // checking helper references is needed when one helper class A extends another helper class B
+ // and the subclass A also implements a library interface C
+ // B needs to be resolved as part of checking that A implements all required methods of C
+ // but B cannot be resolved on the classpath, so B needs to be resolved from helper references
if (helperReferences.containsKey(className)) {
return new ReferenceType(helperReferences.get(className));
}

MODIFY
testing-common/src/test/groovy/muzzle/ReferenceCollectorTest.groovy
testing-common/src/test/groovy/muzzle/ReferenceCollectorTest.groovy
package muzzle
-import static io.opentelemetry.javaagent.extension.muzzle.Reference.Flag.*
-import static muzzle.TestClasses.*
+
+import static io.opentelemetry.javaagent.extension.muzzle.Reference.Flag.ManifestationFlag
+import static io.opentelemetry.javaagent.extension.muzzle.Reference.Flag.MinimumVisibilityFlag
+import static io.opentelemetry.javaagent.extension.muzzle.Reference.Flag.OwnershipFlag
+import static io.opentelemetry.javaagent.extension.muzzle.Reference.Flag.VisibilityFlag
+import static muzzle.TestClasses.HelperAdvice
+import static muzzle.TestClasses.LdcAdvice
+import static muzzle.TestClasses.MethodBodyAdvice
import external.instrumentation.ExternalHelper
import io.opentelemetry.context.Context
@@ -24,6 +30,7 @@ class ReferenceCollectorTest extends Specification {
setup:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(MethodBodyAdvice.name)
+ collector.prune()
def references = collector.getReferences()
expect:
@@ -70,6 +77,7 @@ class ReferenceCollectorTest extends Specification {
setup:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(MethodBodyAdvice.B2.name)
+ collector.prune()
def references = collector.getReferences()
expect:
@@ -82,6 +90,7 @@ class ReferenceCollectorTest extends Specification {
setup:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(LdcAdvice.name)
+ collector.prune()
def references = collector.getReferences()
expect:
@@ -92,6 +101,7 @@ class ReferenceCollectorTest extends Specification {
setup:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(TestClasses.InstanceofAdvice.name)
+ collector.prune()
def references = collector.getReferences()
expect:
@@ -102,6 +112,7 @@ class ReferenceCollectorTest extends Specification {
setup:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(TestClasses.InvokeDynamicAdvice.name)
+ collector.prune()
def references = collector.getReferences()
expect:
@@ -147,6 +158,7 @@ class ReferenceCollectorTest extends Specification {
when:
def collector = new ReferenceCollector({ it == DeclaredFieldTestClass.Helper.name })
collector.collectReferencesFromAdvice(DeclaredFieldTestClass.Advice.name)
+ collector.prune()
def references = collector.references
then:
@@ -169,6 +181,7 @@ class ReferenceCollectorTest extends Specification {
when:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(HelperAdvice.name)
+ collector.prune()
def helperClasses = collector.getSortedHelperClasses()
then:
@@ -187,6 +200,7 @@ class ReferenceCollectorTest extends Specification {
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(TestClasses.HelperAdvice.name)
collector.collectReferencesFromAdvice(TestClasses.HelperOtherAdvice.name)
+ collector.prune()
def helperClasses = collector.getSortedHelperClasses()
then:
@@ -218,10 +232,10 @@ class ReferenceCollectorTest extends Specification {
when:
def collector = new ReferenceCollector({ it.startsWith("external.instrumentation") })
collector.collectReferencesFromAdvice(TestClasses.ExternalInstrumentationAdvice.name)
+ collector.prune()
then: "should collect references"
def references = collector.getReferences()
- references['external.instrumentation.ExternalHelper'] != null
references['external.NotInstrumentation'] != null
then: "should collect helper classes"
@@ -234,14 +248,7 @@ class ReferenceCollectorTest extends Specification {
when:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromResource(resource)
-
- then: "SPI classes are collected as references"
- def references = collector.references
- references.keySet() == [
- TestHelperClasses.Helper.name,
- TestHelperClasses.HelperSuperClass.name,
- TestHelperClasses.HelperInterface.name
- ] as Set
+ collector.prune()
then: "SPI classes are included in helper classes"
def helperClasses = collector.sortedHelperClasses
@@ -269,6 +276,7 @@ class ReferenceCollectorTest extends Specification {
when:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromResource("application.properties")
+ collector.prune()
then:
collector.references.isEmpty()
@@ -279,6 +287,7 @@ class ReferenceCollectorTest extends Specification {
when:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(InstrumentationContextTestClasses.ValidAdvice.name)
+ collector.prune()
then:
def contextStore = collector.getContextStoreClasses()
@@ -292,6 +301,7 @@ class ReferenceCollectorTest extends Specification {
when:
def collector = new ReferenceCollector({ false })
collector.collectReferencesFromAdvice(adviceClassName)
+ collector.prune()
then:
thrown(MuzzleCompilationException)

Muzzle optimization (#2903)