Browse Source

Muzzle CI experiment (#5594)

* Muzzle CI experiment

* Fix class loader leak

* --max-workers=4

* Concurrent jobs

* Update .github/workflows/muzzle.yml

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>

* Simpler

* Add link to comment

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
Trask Stalnaker 3 years ago
parent
commit
25856b8531

+ 14 - 37
.github/workflows/muzzle.yml

@@ -2,36 +2,21 @@ name: Muzzle
 
 on:
   workflow_call:
+    inputs:
+      cache-read-only:
+        type: boolean
+        required: false
 
 jobs:
-  setup-matrix:
-    runs-on: ubuntu-latest
-    outputs:
-      matrix: ${{ steps.set-matrix.outputs.matrix }}
-    steps:
-      - uses: actions/checkout@v3
-
-      - name: Set up JDK 11 for running Gradle
-        uses: actions/setup-java@v2
-        with:
-          distribution: temurin
-          java-version: 11
-
-      - name: List instrumentations to file
-        uses: gradle/gradle-build-action@v2
-        with:
-          arguments: instrumentation:listInstrumentations
-          cache-read-only: true
-
-      - id: set-matrix
-        run: echo "::set-output name=matrix::{\"module\":[\"$(cat instrumentation-list.txt | xargs echo | sed 's/ /","/g')\"]}"
-
-  run:
-    needs: setup-matrix
+  muzzle:
     runs-on: ubuntu-latest
     strategy:
-      matrix: ${{fromJson(needs.setup-matrix.outputs.matrix)}}
-      fail-fast: false
+      matrix:
+        task:
+          - :instrumentation:muzzle1
+          - :instrumentation:muzzle2
+          - :instrumentation:muzzle3
+          - :instrumentation:muzzle4
     steps:
       - uses: actions/checkout@v3
 
@@ -41,16 +26,8 @@ jobs:
           distribution: temurin
           java-version: 11
 
-      # don't use gradle-build-action because all the parallel muzzle jobs cause the cache service
-      # to respond with 429 (to both muzzle and other jobs running in parallel)
-
       - name: Run muzzle
-        # using retry because of sporadic gradle download failures
-        uses: nick-invision/retry@v2.6.0
+        uses: gradle/gradle-build-action@v2
         with:
-          # timing out has not been a problem, these jobs typically finish in 2-3 minutes
-          timeout_minutes: 15
-          # give maven central some time to hopefully recover (120 seconds has not proven to be enough)
-          retry_wait_seconds: 300
-          max_attempts: 5
-          command: ./gradlew ${{ matrix.module }}:muzzle
+          arguments: ${{ matrix.task }}
+          cache-read-only: ${{ inputs.cache-read-only }}

+ 2 - 0
.github/workflows/pr.yml

@@ -49,6 +49,8 @@ jobs:
     # which requires unnecessary release branch maintenance, especially for patches
     if: ${{ !startsWith(github.base_ref, 'v') }}
     uses: ./.github/workflows/muzzle.yml
+    with:
+      cache-read-only: true
 
   examples:
     uses: ./.github/workflows/examples.yml

+ 2 - 3
gradle-plugins/src/main/kotlin/io.opentelemetry.instrumentation.muzzle-check.gradle.kts

@@ -154,9 +154,8 @@ val hasRelevantTask = gradle.startParameter.taskNames.any {
   // removing leading ':' if present
   val taskName = it.removePrefix(":")
   val projectPath = project.path.substring(1)
-  // Either the specific muzzle task in this project or the top level, full-project
-  // muzzle task.
-  taskName == "${projectPath}:muzzle" || taskName == "muzzle"
+  // Either the specific muzzle task in this project or a top level muzzle task.
+  taskName == "${projectPath}:muzzle" || taskName.startsWith("instrumentation:muzzle")
 }
 
 if (hasRelevantTask) {

+ 15 - 13
instrumentation/build.gradle.kts

@@ -4,6 +4,16 @@ plugins {
 
 val instrumentationProjectTest = tasks.named("test")
 
+// batching up the muzzle tasks alphabetically into 4 chunks
+// to split them up into separate CI jobs (but not too many CI job)
+val instrumentationProjectMuzzle = listOf(
+  tasks.create("muzzle1"),
+  tasks.create("muzzle2"),
+  tasks.create("muzzle3"),
+  tasks.create("muzzle4")
+)
+
+var counter = 0
 subprojects {
   val subProj = this
   plugins.withId("java") {
@@ -18,19 +28,11 @@ subprojects {
       testCompileOnly(project(path = ":testing:armeria-shaded-for-testing", configuration = "shadow"))
     }
   }
-}
 
-tasks {
-  register("listInstrumentations") {
-    group = "Help"
-    description = "List all available instrumentation modules"
-    doFirst {
-      File("instrumentation-list.txt").printWriter().use { out ->
-        subprojects
-          .filter { it.plugins.hasPlugin("io.opentelemetry.instrumentation.muzzle-check") }
-          .map { it.path }
-          .forEach { out.println(it) }
-      }
-    }
+  plugins.withId("io.opentelemetry.instrumentation.muzzle-check") {
+    // relying on predictable ordering of subprojects
+    // (see https://docs.gradle.org/current/dsl/org.gradle.api.Project.html#N14CB4)
+    // since we are splitting these muzzleX tasks across different github action jobs
+    instrumentationProjectMuzzle[counter++ % 4].dependsOn(subProj.tasks.named("muzzle"))
   }
 }

+ 9 - 17
javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/InClassLoaderMatcher.java

@@ -11,8 +11,9 @@ package io.opentelemetry.javaagent.instrumentation.api.internal;
  */
 public final class InClassLoaderMatcher {
 
-  private static final ThreadLocal<MutableBoolean> inClassLoaderMatcher =
-      ThreadLocal.withInitial(MutableBoolean::new);
+  // using boolean[] to avoid an extra thread local lookup for getAndSet() below
+  private static final ThreadLocal<boolean[]> inClassLoaderMatcher =
+      ThreadLocal.withInitial(() -> new boolean[1]);
 
   private InClassLoaderMatcher() {}
 
@@ -24,7 +25,7 @@ public final class InClassLoaderMatcher {
    * calls ClassLoader.getResource(). See {@code EclipseOsgiInstrumentationModule} for more details.
    */
   public static boolean get() {
-    return inClassLoaderMatcher.get().value;
+    return inClassLoaderMatcher.get()[0];
   }
 
   /**
@@ -36,7 +37,10 @@ public final class InClassLoaderMatcher {
    * agent class loader.
    */
   public static boolean getAndSet(boolean value) {
-    return inClassLoaderMatcher.get().getAndSet(value);
+    boolean[] arr = inClassLoaderMatcher.get();
+    boolean curr = arr[0];
+    arr[0] = value;
+    return curr;
   }
 
   /**
@@ -48,18 +52,6 @@ public final class InClassLoaderMatcher {
    * agent class loader.
    */
   public static void set(boolean value) {
-    inClassLoaderMatcher.get().value = value;
-  }
-
-  // using MutableBoolean to avoid an extra thread local lookup for getAndSet()
-  private static class MutableBoolean {
-
-    private boolean value;
-
-    private boolean getAndSet(boolean value) {
-      boolean oldValue = this.value;
-      this.value = value;
-      return oldValue;
-    }
+    inClassLoaderMatcher.get()[0] = value;
   }
 }