Make sure muzzle build-time check actually validates anything (#2599)

* Check if muzzle build-time check actually validates anything

* Force class files generation before muzzle

* Apply muzzle only to javaagent projects

* Apply muzzle only to javaagent projects

* More fixes

* More fixes

* PR review comments

* polish

* Exclude "broken" version

* Polish

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

buildSrc/src/main/groovy/MuzzlePlugin.groovy

@@ -50,24 +50,18 @@ class MuzzlePlugin implements Plugin<Project> {
     // compileMuzzle compiles all projects required to run muzzle validation.
     // Not adding group and description to keep this task from showing in `gradle tasks`.
     def compileMuzzle = project.task('compileMuzzle')
+    compileMuzzle.dependsOn(bootstrapProject.tasks.classes, toolingProject.tasks.classes, project.tasks.classes)
+
     def muzzle = project.task('muzzle') {
       group = 'Muzzle'
       description = "Run instrumentation muzzle on compile time dependencies"
-      doLast {
-        if (!project.muzzle.directives.any { it.assertPass }) {
-          project.getLogger().info('No muzzle pass directives configured. Asserting pass against instrumentation compile-time dependencies')
-          ClassLoader userCL = createCompileDepsClassLoader(project)
-          ClassLoader instrumentationCL = createInstrumentationClassloader(project)
-          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)
-        }
-        println "Muzzle executing for $project"
-      }
+      dependsOn(compileMuzzle)
     }
-    def printReferences = project.task('printMuzzleReferences') {
+
+    project.task('printMuzzleReferences') {
       group = 'Muzzle'
       description = "Print references created by instrumentation muzzle"
+      dependsOn(compileMuzzle)
       doLast {
         ClassLoader instrumentationCL = createInstrumentationClassloader(project)
         Method assertionMethod = instrumentationCL.loadClass('io.opentelemetry.javaagent.tooling.muzzle.matcher.MuzzleGradlePluginUtil')
@@ -75,16 +69,6 @@ class MuzzlePlugin implements Plugin<Project> {
         assertionMethod.invoke(null, instrumentationCL)
       }
     }
-    project.tasks.compileMuzzle.dependsOn(bootstrapProject.tasks.compileJava)
-    project.tasks.compileMuzzle.dependsOn(toolingProject.tasks.compileJava)
-    project.afterEvaluate {
-      project.tasks.compileMuzzle.dependsOn(project.tasks.compileJava)
-      if (project.tasks.getNames().contains('compileScala')) {
-        project.tasks.compileMuzzle.dependsOn(project.tasks.compileScala)
-      }
-    }
-    project.tasks.muzzle.dependsOn(project.tasks.compileMuzzle)
-    project.tasks.printMuzzleReferences.dependsOn(project.tasks.compileMuzzle)
 
     def hasRelevantTask = project.gradle.startParameter.taskNames.any { taskName ->
       // removing leading ':' if present
@@ -102,7 +86,7 @@ class MuzzlePlugin implements Plugin<Project> {
 
     project.afterEvaluate {
       // use runAfter to set up task finalizers in version order
-      Task runAfter = project.tasks.muzzle
+      Task runAfter = muzzle
 
       for (MuzzleDirective muzzleDirective : project.muzzle.directives) {
         project.getLogger().info("configured $muzzleDirective")

gradle/instrumentation.gradle

@@ -2,7 +2,6 @@
 import io.opentelemetry.instrumentation.gradle.bytebuddy.ByteBuddyPluginConfigurator
 
 apply plugin: 'net.bytebuddy.byte-buddy'
-apply plugin: 'muzzle'
 apply plugin: 'com.github.johnrengelman.shadow'
 
 ext {
@@ -19,6 +18,8 @@ if (project.ext.find("skipPublish") != true) {
 apply from: "$rootDir/gradle/instrumentation-common.gradle"
 
 if (projectDir.name == 'javaagent') {
+  apply plugin: 'muzzle'
+
   archivesBaseName = projectDir.parentFile.name
 }
 

instrumentation/elasticsearch/elasticsearch-rest-5.0/javaagent/elasticsearch-rest-5.0-javaagent.gradle

@@ -23,7 +23,7 @@ testSets {
 dependencies {
   compileOnly group: 'org.elasticsearch.client', name: 'rest', version: '5.0.0'
 
-  implementation project(':instrumentation:elasticsearch:elasticsearch-rest-common:javaagent')
+  implementation project(':instrumentation:elasticsearch:elasticsearch-rest-common:library')
 
   testInstrumentation project(':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent')
   testInstrumentation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')

instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent/elasticsearch-rest-6.4-javaagent.gradle

@@ -18,7 +18,7 @@ muzzle {
 dependencies {
   library group: 'org.elasticsearch.client', name: 'elasticsearch-rest-client', version: '6.4.0'
 
-  implementation project(':instrumentation:elasticsearch:elasticsearch-rest-common:javaagent')
+  implementation project(':instrumentation:elasticsearch:elasticsearch-rest-common:library')
 
   testInstrumentation project(':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent')
   testInstrumentation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')

instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent/elasticsearch-rest-7.0-javaagent.gradle

@@ -18,7 +18,7 @@ muzzle {
 dependencies {
   library group: 'org.elasticsearch.client', name: 'elasticsearch-rest-client', version: '7.0.0'
 
-  implementation project(':instrumentation:elasticsearch:elasticsearch-rest-common:javaagent')
+  implementation project(':instrumentation:elasticsearch:elasticsearch-rest-common:library')
 
   testInstrumentation project(':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent')
   testInstrumentation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')

instrumentation/elasticsearch/elasticsearch-rest-common/library/elasticsearch-rest-common-library.gradle

@@ -1,4 +1,4 @@
-apply from: "$rootDir/gradle/instrumentation.gradle"
+apply from: "$rootDir/gradle/instrumentation-library.gradle"
 
 dependencies {
   compileOnly group: 'org.elasticsearch.client', name: 'rest', version: '5.0.0'

instrumentation/elasticsearch/elasticsearch-transport-5.0/javaagent/elasticsearch-transport-5.0-javaagent.gradle

@@ -26,7 +26,7 @@ muzzle {
 dependencies {
   compileOnly group: 'org.elasticsearch.client', name: 'transport', version: '5.0.0'
 
-  implementation project(':instrumentation:elasticsearch:elasticsearch-transport-common:javaagent')
+  implementation project(':instrumentation:elasticsearch:elasticsearch-transport-common:library')
 
   // Ensure no cross interference
   testInstrumentation project(':instrumentation:elasticsearch:elasticsearch-rest-5.0:javaagent')

instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/elasticsearch-transport-5.3-javaagent.gradle

@@ -26,7 +26,7 @@ muzzle {
 dependencies {
   library group: 'org.elasticsearch.client', name: 'transport', version: '5.3.0'
 
-  implementation project(':instrumentation:elasticsearch:elasticsearch-transport-common:javaagent')
+  implementation project(':instrumentation:elasticsearch:elasticsearch-transport-common:library')
 
   testInstrumentation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')
   testInstrumentation project(':instrumentation:netty:netty-4.1:javaagent')

instrumentation/elasticsearch/elasticsearch-transport-6.0/javaagent/elasticsearch-transport-6.0-javaagent.gradle

@@ -20,7 +20,7 @@ muzzle {
 dependencies {
   library group: 'org.elasticsearch.client', name: 'transport', version: '6.0.0'
 
-  implementation project(':instrumentation:elasticsearch:elasticsearch-transport-common:javaagent')
+  implementation project(':instrumentation:elasticsearch:elasticsearch-transport-common:library')
 
   // Ensure no cross interference
   testInstrumentation project(':instrumentation:elasticsearch:elasticsearch-rest-5.0:javaagent')

instrumentation/elasticsearch/elasticsearch-transport-common/library/elasticsearch-transport-common-library.gradle

@@ -1,4 +1,4 @@
-apply from: "$rootDir/gradle/instrumentation.gradle"
+apply from: "$rootDir/gradle/instrumentation-library.gradle"
 
 dependencies {
   compileOnly group: 'org.elasticsearch', name: 'elasticsearch', version: '2.0.0'

instrumentation/hibernate/hibernate-common/javaagent/hibernate-common-javaagent.gradle

@@ -2,4 +2,8 @@
  * Classes that are common to all versions of the Hibernate instrumentation.
  */
 
-apply from: "$rootDir/gradle/instrumentation.gradle"
+apply from: "$rootDir/gradle/instrumentation-library.gradle"
+
+dependencies {
+  compileOnly project(':javaagent-api')
+}

instrumentation/jaxws/jaxws-2.0/javaagent/jaxws-2.0-javaagent.gradle

@@ -11,5 +11,5 @@ muzzle {
 
 dependencies {
   library group: 'javax.xml.ws', name: 'jaxws-api', version: '2.0'
-  implementation project(":instrumentation:jaxws:jaxws-common:javaagent")
+  implementation project(":instrumentation:jaxws:jaxws-common:library")
 }

instrumentation/jaxws/jaxws-common/javaagent/jaxws-common-javaagent.gradle

@@ -1,2 +0,0 @@
-apply from: "$rootDir/gradle/instrumentation.gradle"
-

instrumentation/jaxws/jaxws-common/library/jaxws-common-library.gradle

@@ -0,0 +1 @@
+apply from: "$rootDir/gradle/instrumentation-library.gradle"

instrumentation/jaxws/jws-1.1/javaagent/jws-1.1-javaagent.gradle

@@ -10,5 +10,5 @@ muzzle {
 
 dependencies {
   library group: 'javax.jws', name: 'javax.jws-api', version: '1.1'
-  implementation project(":instrumentation:jaxws:jaxws-common:javaagent")
+  implementation project(":instrumentation:jaxws:jaxws-common:library")
 }

instrumentation/rocketmq-client-4.8/javaagent/rocketmq-client-4.8-javaagent.gradle

@@ -4,7 +4,7 @@ muzzle {
   pass {
     group = "org.apache.rocketmq"
     module = 'rocketmq-client'
-    versions = "[4.8.0,)"
+    versions = "[4.0.0,)"
     assertInverse = true
   }
 }

instrumentation/rxjava-2.0/javaagent/rxjava-2.0-javaagent.gradle

@@ -4,7 +4,9 @@ muzzle {
   pass {
     group = "io.reactivex.rxjava2"
     module = "rxjava"
-    versions = "[2.0.0,)"
+    versions = "[2.0.6,)"
+    extraDependency "io.reactivex.rxjava2:rxjava:2.0.6"
+    assertInverse true
   }
 }
 

instrumentation/rxjava-2.0/javaagent/src/main/java/io/opentelemetry/instrumentation/rxjava2/RxJava2InstrumentationModule.java

@@ -8,6 +8,7 @@ package io.opentelemetry.instrumentation.rxjava2;
 import static net.bytebuddy.matcher.ElementMatchers.isTypeInitializer;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 
+import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.tooling.InstrumentationModule;
 import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
 import java.util.Collections;
@@ -18,6 +19,7 @@ import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 
+@AutoService(InstrumentationModule.class)
 public class RxJava2InstrumentationModule extends InstrumentationModule {
 
   public RxJava2InstrumentationModule() {

instrumentation/vertx-reactive-3.5/javaagent/vertx-reactive-3.5-javaagent.gradle

@@ -17,7 +17,7 @@ dependencies {
 
   testInstrumentation project(':instrumentation:jdbc:javaagent')
   testInstrumentation project(':instrumentation:netty:netty-4.1:javaagent')
-  testInstrumentation project(':instrumentation:vertx-web-3.0')
+  testInstrumentation project(':instrumentation:vertx-web-3.0:javaagent')
   //TODO we should include rjxava2 instrumentation here as well
 
   testLibrary group: 'io.vertx', name: 'vertx-web-client', version: vertxVersion

instrumentation/vertx-web-3.0/javaagent/vertx-web-3.0-javaagent.gradle

@@ -5,7 +5,9 @@ muzzle {
     group = 'io.vertx'
     module = 'vertx-web'
     versions = "[3.0.0,4.0.0)"
-    assertInverse = true
+    //TODO we should split this module into client and server
+    //They have different version applicability
+//    assertInverse = true
   }
 }
 

javaagent-tooling/javaagent-tooling.gradle

@@ -47,7 +47,7 @@ dependencies {
   implementation deps.opentelemetryZipkin
 
   implementation group: 'com.blogspot.mydailyjava', name: 'weak-lock-free', version: '0.15'
-  implementation deps.bytebuddy
+  api deps.bytebuddy
   implementation deps.bytebuddyagent
   annotationProcessor deps.autoservice
   implementation deps.autoservice

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/matcher/MuzzleGradlePluginUtil.java

@@ -46,6 +46,7 @@ public final class MuzzleGradlePluginUtil {
       ClassLoader agentClassLoader, ClassLoader userClassLoader, boolean assertPass)
       throws Exception {
     // muzzle validate all instrumenters
+    int validatedModulesCount = 0;
     for (InstrumentationModule instrumentationModule :
         ServiceLoader.load(InstrumentationModule.class, agentClassLoader)) {
       Method getMuzzleReferenceMatcher = null;
@@ -87,6 +88,8 @@ public final class MuzzleGradlePluginUtil {
           getMuzzleReferenceMatcher.setAccessible(false);
         }
       }
+
+      validatedModulesCount++;
     }
     // run helper injector on all instrumenters
     if (assertPass) {
@@ -108,6 +111,11 @@ public final class MuzzleGradlePluginUtil {
         }
       }
     }
+    if (validatedModulesCount == 0) {
+      String errorMessage = "Did not found any InstrumentationModule to validate!";
+      System.err.println(errorMessage);
+      throw new RuntimeException(errorMessage);
+    }
   }
 
   private static Map<String, byte[]> createHelperMap(

settings.gradle

@@ -97,11 +97,11 @@ include ':instrumentation:couchbase:couchbase-testing'
 include ':instrumentation:dropwizard-views-0.7:javaagent'
 include ':instrumentation:dropwizard-testing'
 include ':instrumentation:eclipse-osgi-3.6:javaagent'
-include ':instrumentation:elasticsearch:elasticsearch-rest-common:javaagent'
+include ':instrumentation:elasticsearch:elasticsearch-rest-common:library'
 include ':instrumentation:elasticsearch:elasticsearch-rest-5.0:javaagent'
 include ':instrumentation:elasticsearch:elasticsearch-rest-6.4:javaagent'
 include ':instrumentation:elasticsearch:elasticsearch-rest-7.0:javaagent'
-include ':instrumentation:elasticsearch:elasticsearch-transport-common:javaagent'
+include ':instrumentation:elasticsearch:elasticsearch-transport-common:library'
 include ':instrumentation:elasticsearch:elasticsearch-transport-5.0:javaagent'
 include ':instrumentation:elasticsearch:elasticsearch-transport-5.3:javaagent'
 include ':instrumentation:elasticsearch:elasticsearch-transport-6.0:javaagent'
@@ -144,7 +144,7 @@ include ':instrumentation:jaxws:jaxws-2.0-cxf-3.0:javaagent'
 include ':instrumentation:jaxws:jaxws-2.0-cxf-3.0:library'
 include ':instrumentation:jaxws:jaxws-2.0-metro-2.2:javaagent'
 include ':instrumentation:jaxws:jaxws-2.0-testing'
-include ':instrumentation:jaxws:jaxws-common:javaagent'
+include ':instrumentation:jaxws:jaxws-common:library'
 include ':instrumentation:jaxws:jws-1.1:javaagent'
 include ':instrumentation:jdbc:javaagent'
 include ':instrumentation:jdbc:javaagent-unittests'
@@ -257,7 +257,7 @@ include ':instrumentation:struts-2.3:javaagent'
 include ':instrumentation:tomcat-7.0:javaagent'
 include ':instrumentation:twilio-6.6:javaagent'
 include ':instrumentation:undertow-1.4:javaagent'
-include ':instrumentation:vertx-web-3.0'
+include ':instrumentation:vertx-web-3.0:javaagent'
 include ':instrumentation:vertx-reactive-3.5:javaagent'
 include ':instrumentation:wicket-8.0:javaagent'
 include ':instrumentation:rocketmq-client-4.8:javaagent'