Better muzzle instrumentation exclusion (open-telemetry#6044)

* Better muzzle instrumentation exclusion * set
RashmiRam · May 23, 2022 · ecae165 · ecae165
1 parent ff03927
commit ecae165
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 20 deletions.
diff --git a/gradle-plugins/src/main/kotlin/io.opentelemetry.instrumentation.muzzle-check.gradle.kts b/gradle-plugins/src/main/kotlin/io.opentelemetry.instrumentation.muzzle-check.gradle.kts
@@ -287,7 +287,7 @@ fun addMuzzleTask(muzzleDirective: MuzzleDirective, versionArtifact: Artifact?,
       val userCL = createClassLoaderForTask(config)
       withContextClassLoader(instrumentationCL) {
         MuzzleGradlePluginUtil.assertInstrumentationMuzzled(instrumentationCL, userCL,
-          muzzleDirective.excludedInstrumentationModules.get(), muzzleDirective.assertPass.get())
+          muzzleDirective.excludedInstrumentationNames.get(), muzzleDirective.assertPass.get())
       }
 
       for (thread in Thread.getAllStackTraces().keys) {
@@ -351,7 +351,7 @@ fun inverseOf(muzzleDirective: MuzzleDirective, system: RepositorySystem, sessio
       assertPass.set(!muzzleDirective.assertPass.get())
       additionalDependencies.set(muzzleDirective.additionalDependencies)
       excludedDependencies.set(muzzleDirective.excludedDependencies)
-      excludedInstrumentationModules.set(muzzleDirective.excludedInstrumentationModules)
+      excludedInstrumentationNames.set(muzzleDirective.excludedInstrumentationNames)
     }
     inverseDirectives.add(inverseDirective)
   }

diff --git a/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/MuzzleDirective.kt b/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/MuzzleDirective.kt
@@ -20,7 +20,7 @@ abstract class MuzzleDirective {
   abstract val skipVersions: SetProperty<String>
   abstract val additionalDependencies: ListProperty<Any>
   abstract val excludedDependencies: ListProperty<String>
-  abstract val excludedInstrumentationModules: ListProperty<String>
+  abstract val excludedInstrumentationNames: SetProperty<String>
   abstract val assertPass: Property<Boolean>
   abstract val assertInverse: Property<Boolean>
   internal abstract val coreJdk: Property<Boolean> // use coreJdk() function below to enable
@@ -31,7 +31,7 @@ abstract class MuzzleDirective {
     skipVersions.convention(emptySet())
     additionalDependencies.convention(listOf())
     excludedDependencies.convention(listOf())
-    excludedInstrumentationModules.convention(listOf())
+    excludedInstrumentationNames.convention(listOf())
     assertPass.convention(false)
     assertInverse.convention(false)
     coreJdk.convention(false)
@@ -65,8 +65,8 @@ abstract class MuzzleDirective {
    *
    * @param excludeString An instrumentation module class name to exclude
    */
-  fun excludeInstrumentationModule(excludeString: String) {
-    excludedInstrumentationModules.add(excludeString)
+  fun excludeInstrumentationName(excludeString: String) {
+    excludedInstrumentationNames.add(excludeString)
   }
 
   fun skip(vararg version: String?) {

diff --git a/...ugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt b/...ugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt
@@ -53,7 +53,7 @@ class MuzzleGradlePluginUtil {
      */
     @Suppress("UNCHECKED_CAST")
     fun assertInstrumentationMuzzled(agentClassLoader: ClassLoader, userClassLoader: ClassLoader,
-                                     excludedInstrumentationModules: List<String>, assertPass: Boolean) {
+                                     excludedInstrumentationNames: Set<String>, assertPass: Boolean) {
 
       val matcherClass = agentClassLoader.loadClass("io.opentelemetry.javaagent.tooling.muzzle.ClassLoaderMatcher")
 
@@ -63,13 +63,11 @@ class MuzzleGradlePluginUtil {
       // We cannot reference Mismatch class directly here, because we are loaded from a different
       // classloader.
       val allMismatches = matcherClass
-        .getMethod("matchesAll", ClassLoader::class.java, Boolean::class.javaPrimitiveType)
-        .invoke(null, userClassLoader, assertPass)
+        .getMethod("matchesAll", ClassLoader::class.java, Boolean::class.javaPrimitiveType, Set::class.java)
+        .invoke(null, userClassLoader, assertPass, excludedInstrumentationNames)
         as Map<String, List<Any>>
 
-      allMismatches.filterKeys {
-        !excludedInstrumentationModules.contains(it)
-      }.forEach { (moduleName, mismatches) ->
+      allMismatches.forEach { moduleName, mismatches ->
         val passed = mismatches.isEmpty()
         if (passed && !assertPass) {
           System.err.println("MUZZLE PASSED $moduleName BUT FAILURE WAS EXPECTED")

diff --git a/instrumentation/opentelemetry-instrumentation-api/javaagent/build.gradle.kts b/instrumentation/opentelemetry-instrumentation-api/javaagent/build.gradle.kts
@@ -14,7 +14,7 @@ muzzle {
     // if you want to test them anyways, comment out "alpha" from the exclusions in AcceptableVersions.kt
     versions.set("[1.14.0-alpha,)")
     assertInverse.set(true)
-    excludeInstrumentationModule("io.opentelemetry.javaagent.instrumentation.opentelemetryapi.OpenTelemetryApiInstrumentationModule")
+    excludeInstrumentationName("opentelemetry-api")
   }
 }
 

diff --git a/instrumentation/reactor/reactor-3.1/javaagent/build.gradle.kts b/instrumentation/reactor/reactor-3.1/javaagent/build.gradle.kts
@@ -9,7 +9,7 @@ muzzle {
     versions.set("[3.1.0.RELEASE,)")
     extraDependency("io.opentelemetry:opentelemetry-api:1.0.0")
     assertInverse.set(true)
-    excludeInstrumentationModule("io.opentelemetry.javaagent.instrumentation.opentelemetryapi.OpenTelemetryApiInstrumentationModule")
+    excludeInstrumentationName("opentelemetry-api")
   }
 }
 

diff --git a/instrumentation/reactor/reactor-netty/reactor-netty-0.9/javaagent/build.gradle.kts b/instrumentation/reactor/reactor-netty/reactor-netty-0.9/javaagent/build.gradle.kts
@@ -8,14 +8,14 @@ muzzle {
     module.set("reactor-netty")
     versions.set("[0.8.2.RELEASE,1.0.0)")
     assertInverse.set(true)
-    excludeInstrumentationModule("io.opentelemetry.javaagent.instrumentation.netty.v4_1.NettyInstrumentationModule")
+    excludeInstrumentationName("netty")
   }
   fail {
     group.set("io.projectreactor.netty")
     module.set("reactor-netty-http")
     versions.set("[1.0.0,)")
     assertInverse.set(true)
-    excludeInstrumentationModule("io.opentelemetry.javaagent.instrumentation.netty.v4_1.NettyInstrumentationModule")
+    excludeInstrumentationName("netty")
   }
 }
 

diff --git a/instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/build.gradle.kts b/instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/build.gradle.kts
@@ -8,14 +8,14 @@ muzzle {
     module.set("reactor-netty")
     versions.set("[,1.0.0)")
     assertInverse.set(true)
-    excludeInstrumentationModule("io.opentelemetry.javaagent.instrumentation.netty.v4_1.NettyInstrumentationModule")
+    excludeInstrumentationName("netty")
   }
   pass {
     group.set("io.projectreactor.netty")
     module.set("reactor-netty-http")
     versions.set("[1.0.0,)")
     assertInverse.set(true)
-    excludeInstrumentationModule("io.opentelemetry.javaagent.instrumentation.netty.v4_1.NettyInstrumentationModule")
+    excludeInstrumentationName("netty")
   }
 }
 

diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java
@@ -11,6 +11,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.ServiceLoader;
+import java.util.Set;
 
 /**
  * This class verifies that a given {@link ClassLoader} satisfies all expectations of a given {@link
@@ -30,12 +31,16 @@ public class ClassLoaderMatcher {
    * <p>The returned map will be empty if and only if no instrumentation modules were found.
    */
   public static Map<String, List<Mismatch>> matchesAll(
-      ClassLoader classLoader, boolean injectHelpers) {
+      ClassLoader classLoader, boolean injectHelpers, Set<String> excludedInstrumentationNames) {
     Map<String, List<Mismatch>> result = new HashMap<>();
     ServiceLoader.load(InstrumentationModule.class)
         .forEach(
             module -> {
-              result.put(module.getClass().getName(), matches(module, classLoader, injectHelpers));
+              if (module.instrumentationNames().stream()
+                  .noneMatch(excludedInstrumentationNames::contains)) {
+                result.put(
+                    module.getClass().getName(), matches(module, classLoader, injectHelpers));
+              }
             });
     return result;
   }