From 25856b85317afff760fad1324640ae2518a32380 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Fri, 18 Mar 2022 09:57:02 -0700 Subject: [PATCH] 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 * Simpler * Add link to comment Co-authored-by: Anuraag Agrawal --- .github/workflows/muzzle.yml | 51 +++++-------------- .github/workflows/pr.yml | 2 + ...ry.instrumentation.muzzle-check.gradle.kts | 5 +- instrumentation/build.gradle.kts | 28 +++++----- .../api/internal/InClassLoaderMatcher.java | 26 ++++------ 5 files changed, 42 insertions(+), 70 deletions(-) diff --git a/.github/workflows/muzzle.yml b/.github/workflows/muzzle.yml index e845a73c23..8c4a43d2d6 100644 --- a/.github/workflows/muzzle.yml +++ b/.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 }} diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 402045b1ed..0d1f776b8d 100644 --- a/.github/workflows/pr.yml +++ b/.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 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 index 5726c3c6e2..96f88d2ad8 100644 --- 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 @@ -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) { diff --git a/instrumentation/build.gradle.kts b/instrumentation/build.gradle.kts index 09ca2c94e6..7e09c04d48 100644 --- a/instrumentation/build.gradle.kts +++ b/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")) } } diff --git a/javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/InClassLoaderMatcher.java b/javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/InClassLoaderMatcher.java index b7dd183d5e..8d297906a1 100644 --- a/javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/InClassLoaderMatcher.java +++ b/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 inClassLoaderMatcher = - ThreadLocal.withInitial(MutableBoolean::new); + // using boolean[] to avoid an extra thread local lookup for getAndSet() below + private static final ThreadLocal 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; } }