Muzzle improvements: docs, javadocs, renamings and minor refactoring (#1379)

Author: Mateusz Rzeszutek

buildSrc/src/main/groovy/MuzzlePlugin.groovy

@@ -74,7 +74,7 @@ class MuzzlePlugin implements Plugin<Project> {
           project.getLogger().info('No muzzle pass directives configured. Asserting pass against instrumentation compile-time dependencies')
           ClassLoader userCL = createCompileDepsClassLoader(project, bootstrapProject)
           ClassLoader instrumentationCL = createInstrumentationClassloader(project, toolingProject)
-          Method assertionMethod = instrumentationCL.loadClass('io.opentelemetry.javaagent.tooling.muzzle.MuzzleVersionScanPlugin')
+          Method assertionMethod = instrumentationCL.loadClass('io.opentelemetry.javaagent.tooling.muzzle.matcher.MuzzleGradlePluginUtil')
             .getMethod('assertInstrumentationMuzzled', ClassLoader.class, ClassLoader.class, boolean.class)
           assertionMethod.invoke(null, instrumentationCL, userCL, true)
         }
@@ -86,7 +86,7 @@ class MuzzlePlugin implements Plugin<Project> {
       description = "Print references created by instrumentation muzzle"
       doLast {
         ClassLoader instrumentationCL = createInstrumentationClassloader(project, toolingProject)
-        Method assertionMethod = instrumentationCL.loadClass('io.opentelemetry.javaagent.tooling.muzzle.MuzzleVersionScanPlugin')
+        Method assertionMethod = instrumentationCL.loadClass('io.opentelemetry.javaagent.tooling.muzzle.matcher.MuzzleGradlePluginUtil')
           .getMethod('printMuzzleReferences', ClassLoader.class)
         assertionMethod.invoke(null, instrumentationCL)
       }
@@ -340,7 +340,7 @@ class MuzzlePlugin implements Plugin<Project> {
         ClassLoader userCL = createClassLoaderForTask(instrumentationProject, bootstrapProject, taskName)
         try {
           // find all instrumenters, get muzzle, and assert
-          Method assertionMethod = instrumentationCL.loadClass('io.opentelemetry.javaagent.tooling.muzzle.MuzzleVersionScanPlugin')
+          Method assertionMethod = instrumentationCL.loadClass('io.opentelemetry.javaagent.tooling.muzzle.matcher.MuzzleGradlePluginUtil')
             .getMethod('assertInstrumentationMuzzled', ClassLoader.class, ClassLoader.class, boolean.class)
           assertionMethod.invoke(null, instrumentationCL, userCL, muzzleDirective.assertPass)
         } finally {

docs/contributing/muzzle.md

@@ -1,42 +1,109 @@
 # Muzzle
 
-Muzzle is a feature of the Java agent that ensures API compatibility
-between libraries/symbols on the application classpath and APIs of instrumented
-3rd party libraries used by the Agent. In other words the Muzzle ensures
-that the API symbols used by the Agent are compatible with API symbols
-on the application classpath. The Muzzle will prevent loading an instrumentation
-if the APIs do not match.
+Muzzle is a safety feature of the Java agent that prevents applying instrumentation when a mismatch
+between the instrumentation code and the instrumented application code is detected.
+It ensures API compatibility between symbols (classes, methods, fields) on the application classpath
+and references to those symbols made by instrumentation advices defined in the agent.
+In other words, muzzle ensures that the API symbols used by the agent are compatible with the API
+symbols on the application classpath.
 
-## How does it work
+Muzzle will prevent loading an instrumentation if it detects any mismatch or conflict.
 
-At build time, for each instrumentation the Muzzle ByteBuddy plugin collects symbols referring to both internal
-and 3rd party APIs used by the currently processed instrumentation. The reference collection process starts
-from advice classes - values of the map returned by the `Instrumenter.Default#transformers()` method.
+## How it works
 
-All those references are then used to create a `ReferenceMatcher` instance.
-The matcher is stored in the instrumentation class in method `ReferenceMatcher getInstrumentationMuzzle()`.
+Muzzle has two phases:
+* at compile time it collects references to the third-party symbols;
+* at runtime it compares those references to the actual API symbols on the classpath.
 
-At runtime the Muzzle checks API compatibility between symbols used by the Agent
-and symbols in the application class loader. If the symbols do not match the instrumentation is not loaded.
-Because the muzzle matcher is expensive, it is only performed after a match has been made by the
-`SomeInstrumentation.classLoaderMatcher()` and `SomeInstrumentation.typeMatcher()` matchers.
+### Compile-time reference collection
+
+The compile-time reference collection and code generation process is implemented using a ByteBuddy
+plugin (called `MuzzleCodeGenerationPlugin`).
+
+For each instrumentation the ByteBuddy plugin collects symbols referring to both internal and third
+party APIs used by the currently processed instrumentation. The reference collection process starts
+from advice classes (values of the map returned by the `Instrumenter.Default#transformers()` method)
+and traverses the class graph until it encounters a reference to a non-instrumentation class.
+
+All collected references are then used to create a `ReferenceMatcher` instance. This matcher
+is stored in the instrumentation class in the method `Instrumenter.Default#getMuzzleReferenceMatcher()`.
+The bytecode of this method (basically an array of `Reference` builder calls) 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`.
+
+### Runtime reference matching
+
+The runtime reference matching process is implemented as a ByteBuddy matcher in `Instrumenter.Default`.
+`MuzzleMatcher` uses the `getMuzzleReferenceMatcher()` method generated during the compilation phase
+to verify that the class loader of the instrumented type has all necessary symbols (classes,
+methods, fields). If the `ReferenceMatcher` finds any mismatch between collected references and the
+actual application classpath types the whole instrumentation is discarded.
+
+It is worth noting that because the muzzle check is expensive, it is only performed after a match
+has been made by the `InstrumenterDefault#classLoaderMatcher()` and `Instrumenter.Default#typeMatcher()`
+matchers.
+
+The source code of the runtime muzzle matcher is located in the `javaagent-tooling` module,
+in the class `Instrumenter.Default` and under the package `io.opentelemetry.javaagent.tooling.muzzle`.
 
 ## Muzzle gradle plugin
 
-The `printReferences` task prints all API references in a given module.
+The muzzle gradle plugin allows to perform the runtime reference matching process against different
+third party library versions, when the project is built.
 
-```bash
-./gradlew :instrumentation:google-http-client-1.19:printReferences
-```
+Muzzle gradle plugin is just an additional utility for enhanced build-time checking
+to alert us when there are breaking changes in the underlying third party library
+that will cause the instrumentation not to get applied.
+**Even without using it muzzle reference matching is _always_ active in runtime**,
+it's not an optional feature.
+
+The gradle plugin defines two tasks:
+
+* `muzzle` task runs the runtime muzzle verification against different library versions:
+    ```sh
+    ./gradlew :instrumentation:google-http-client-1.19:muzzle
+    ```
+    If a new, incompatible version of the instrumented library is published it fails the build.
+
+* `printReferences` task prints all API references in a given module:
+    ```sh
+    ./gradlew :instrumentation:google-http-client-1.19:printReferences
+    ```
 
-The `muzzle` task downloads 3rd party libraries from maven central and checks API compatibility.
-If a new incompatible version is published it fails the build.
+The muzzle plugin needs to be configured in the module's `.gradle` file.
+Example:
 
-```bash
-./gradlew :instrumentation:google-http-client-1.19:muzzle
+```groovy
+muzzle {
+  // it is expected that muzzle fails the runtime check for this component
+  fail {
+    group = "commons-httpclient"
+    module = "commons-httpclient"
+    // versions from this range are checked
+    versions = "[,4.0)"
+    // this version is not checked by muzzle
+    skipVersions += '3.1-jenkins-1'
+  }
+  // it is expected that muzzle passes the runtime check for this component
+  pass {
+    group = "org.apache.httpcomponents"
+    module = "httpclient"
+    versions = "[4.0,)"
+    // verify that all other versions - [,4.0) in this case - fail the muzzle runtime check
+    assertInverse = true
+  }
+  // ...
+}
 ```
 
-## Muzzle location
+* Using either `pass` or `fail` directive allows to specify whether muzzle should treat the
+  reference check failure as expected behavior;
+* `versions` is a version range, where `[]` is inclusive and `()` is exclusive. It is not needed to
+  specify the exact version to start/end, e.g. `[1.0.0,4)` would usually behave in the same way as
+  `[1.0.0,4.0.0-Alpha)`;
+* `assertInverse` is basically a shortcut for adding an opposite directive for all library versions
+  that are not included in the specified `versions` range.
 
-* `buildSrc` - Muzzle Gradle plugin
-* `agent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle` - Muzzle ByteBuddy plugin
+The source code of the gradle plugin is located in the `buildSrc` directory.

gradle/instrumentation.gradle

@@ -13,7 +13,7 @@ byteBuddy {
   transformation {
     // Applying NoOp optimizes build by applying bytebuddy plugin to only compileJava task
     tasks = ['compileJava', 'compileScala', 'compileKotlin']
-    plugin = 'io.opentelemetry.javaagent.tooling.muzzle.MuzzleGradlePlugin$NoOp'
+    plugin = 'io.opentelemetry.javaagent.tooling.muzzle.collector.MuzzleCodeGenerationPlugin$NoOp'
   }
 }
 
@@ -32,7 +32,7 @@ afterEvaluate {
   byteBuddy {
     transformation {
       tasks = ['compileJava', 'compileScala', 'compileKotlin']
-      plugin = 'io.opentelemetry.javaagent.tooling.muzzle.MuzzleGradlePlugin'
+      plugin = 'io.opentelemetry.javaagent.tooling.muzzle.collector.MuzzleCodeGenerationPlugin'
       classPath = project(':javaagent-tooling').configurations.instrumentationMuzzle + configurations.runtimeClasspath + sourceSets.main.output
     }
   }

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java

@@ -27,7 +27,7 @@ public abstract class Config {
   private static final Config DEFAULT = Config.create(Collections.emptyMap());
 
   // INSTANCE can never be null - muzzle instantiates instrumenters when it generates
-  // getInstrumentationMuzzle() and the Instrumenter.Default constructor uses Config
+  // getMuzzleReferenceMatcher() and the Instrumenter.Default constructor uses Config
   private static volatile Config INSTANCE = DEFAULT;
 
   /**

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Instrumenter.java

@@ -17,8 +17,8 @@
 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.Reference;
-import io.opentelemetry.javaagent.tooling.muzzle.ReferenceMatcher;
+import io.opentelemetry.javaagent.tooling.muzzle.matcher.Mismatch;
+import io.opentelemetry.javaagent.tooling.muzzle.matcher.ReferenceMatcher;
 import java.security.ProtectionDomain;
 import java.util.Arrays;
 import java.util.Collections;
@@ -153,7 +153,11 @@ public int getOrder() {
       return 0;
     }
 
-    /** Matches classes for which instrumentation is not muzzled. */
+    /**
+     * A ByteBuddy matcher that decides whether this instrumentation should be applied. Calls
+     * generated {@link ReferenceMatcher}: if any mismatch with the passed {@code classLoader} is
+     * found this instrumentation is skipped.
+     */
     private class MuzzleMatcher implements AgentBuilder.RawMatcher {
       @Override
       public boolean matches(
@@ -162,49 +166,46 @@ public boolean matches(
           JavaModule module,
           Class<?> classBeingRedefined,
           ProtectionDomain protectionDomain) {
-        /* Optimization: calling getInstrumentationMuzzle() inside this method
+        /* Optimization: calling getMuzzleReferenceMatcher() inside this method
          * prevents unnecessary loading of muzzle references during agentBuilder
          * setup.
          */
-        ReferenceMatcher muzzle = getInstrumentationMuzzle();
-        if (null != muzzle) {
+        ReferenceMatcher muzzle = getMuzzleReferenceMatcher();
+        if (muzzle != null) {
           boolean isMatch = muzzle.matches(classLoader);
-          if (!isMatch) {
-            if (log.isDebugEnabled()) {
-              List<Reference.Mismatch> mismatches =
-                  muzzle.getMismatchedReferenceSources(classLoader);
-              if (log.isDebugEnabled()) {
-                log.debug(
-                    "Instrumentation muzzled: {} -- {} on {}",
-                    instrumentationNames,
-                    Instrumenter.Default.this.getClass().getName(),
-                    classLoader);
-              }
-              for (Reference.Mismatch mismatch : mismatches) {
+
+          if (log.isDebugEnabled()) {
+            if (!isMatch) {
+              log.debug(
+                  "Instrumentation skipped, mismatched references were found: {} -- {} on {}",
+                  instrumentationNames,
+                  Instrumenter.Default.this.getClass().getName(),
+                  classLoader);
+              List<Mismatch> mismatches = muzzle.getMismatchedReferenceSources(classLoader);
+              for (Mismatch mismatch : mismatches) {
                 log.debug("-- {}", mismatch);
               }
-            }
-          } else {
-            if (log.isDebugEnabled()) {
+            } else {
               log.debug(
                   "Applying instrumentation: {} -- {} on {}",
                   instrumentationPrimaryName,
                   Instrumenter.Default.this.getClass().getName(),
                   classLoader);
             }
           }
+
           return isMatch;
         }
         return true;
       }
     }
 
     /**
-     * This method is implemented dynamically by compile-time bytecode transformations.
-     *
-     * <p>{@see io.opentelemetry.javaagent.tooling.muzzle.MuzzleGradlePlugin}
+     * The actual implementation of this method is generated automatically during compilation by the
+     * {@link io.opentelemetry.javaagent.tooling.muzzle.collector.MuzzleCodeGenerationPlugin}
+     * ByteBuddy plugin.
      */
-    protected ReferenceMatcher getInstrumentationMuzzle() {
+    protected ReferenceMatcher getMuzzleReferenceMatcher() {
       return null;
     }
 

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceCreationPredicate.java renamed to javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationClassPredicate.java

@@ -5,13 +5,7 @@
 
 package io.opentelemetry.javaagent.tooling.muzzle;
 
-/**
- * Defines a set of packages for which we'll create references.
- *
- * <p>For now we're hardcoding this to the instrumentation and javaagent-tooling packages so we only
- * create references from the method advice and helper classes.
- */
-final class ReferenceCreationPredicate {
+public final class InstrumentationClassPredicate {
   // non-shaded packages
   private static final String AUTO_INSTRUMENTATION_PACKAGE =
       "io.opentelemetry.javaagent.instrumentation.";
@@ -23,7 +17,16 @@ final class ReferenceCreationPredicate {
   private static final String LIBRARY_INSTRUMENTATION_PACKAGE = "io.opentelemetry.instrumentation.";
   private static final String INSTRUMENTATION_API_PACKAGE = "io.opentelemetry.instrumentation.api.";
 
-  static boolean shouldCreateReferenceFor(String className) {
+  /**
+   * Defines which classes are treated by muzzle as "internal", "helper" instrumentation classes.
+   *
+   * <p>This set of classes is defined by a package naming convention: all automatic and manual
+   * instrumentation classes and {@code javaagent.tooling} classes are treated as "helper" classes
+   * and are subjected to the reference collection process. All others (including {@code
+   * instrumentation-api} and {@code javaagent-api} modules are not scanned for referenced (but
+   * references to them are collected).
+   */
+  public static boolean isInstrumentationClass(String className) {
     if (className.startsWith(INSTRUMENTATION_API_PACKAGE)
         || className.startsWith(AUTO_INSTRUMENTATION_API_PACKAGE)) {
       return false;
@@ -33,5 +36,5 @@ static boolean shouldCreateReferenceFor(String className) {
         || className.startsWith(LIBRARY_INSTRUMENTATION_PACKAGE);
   }
 
-  private ReferenceCreationPredicate() {}
+  private InstrumentationClassPredicate() {}
 }