From fe41a77e812aebb7cdb991bc29b1044f22252086 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 16 May 2022 21:15:01 -0700 Subject: [PATCH 1/2] Better muzzle instrumentation exclusion --- ...entelemetry.instrumentation.muzzle-check.gradle.kts | 4 ++-- .../opentelemetry/javaagent/muzzle/MuzzleDirective.kt | 8 ++++---- .../javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt | 10 ++++------ .../javaagent/build.gradle.kts | 2 +- .../reactor/reactor-3.1/javaagent/build.gradle.kts | 2 +- .../reactor-netty-0.9/javaagent/build.gradle.kts | 4 ++-- .../reactor-netty-1.0/javaagent/build.gradle.kts | 4 ++-- .../javaagent/tooling/muzzle/ClassLoaderMatcher.java | 8 ++++++-- 8 files changed, 22 insertions(+), 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 index 5bcb7a43b4b8..2a70092f71f1 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 @@ -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 index 7491e1979168..e32754306ccf 100644 --- 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 abstract val additionalDependencies: ListProperty abstract val excludedDependencies: ListProperty - abstract val excludedInstrumentationModules: ListProperty + abstract val excludedInstrumentationNames: ListProperty abstract val assertPass: Property abstract val assertInverse: Property internal abstract val coreJdk: Property // 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/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt b/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt index b512210e45d4..e5d98529b34a 100644 --- a/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt +++ b/gradle-plugins/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, assertPass: Boolean) { + excludedInstrumentationNames: List, 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, List::class.java) + .invoke(null, userClassLoader, assertPass, excludedInstrumentationNames) as Map> - 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 index 5b5e0173ba1b..1eb6814658f8 100644 --- 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 index 7a74b35f689b..f803681491c8 100644 --- 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 index d09df7f39573..7b6aca282d9c 100644 --- 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 index c4947e3d44d2..8ba906388363 100644 --- 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 index 7ff102e24a7f..40912d43d894 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java @@ -30,12 +30,16 @@ public class ClassLoaderMatcher { *

The returned map will be empty if and only if no instrumentation modules were found. */ public static Map> matchesAll( - ClassLoader classLoader, boolean injectHelpers) { + ClassLoader classLoader, boolean injectHelpers, List excludedInstrumentationNames) { Map> 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; } From 9b9a2bcdf1173f93249b4c20f29bf085a33766e2 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 17 May 2022 14:22:05 -0700 Subject: [PATCH 2/2] set --- .../io/opentelemetry/javaagent/muzzle/MuzzleDirective.kt | 2 +- .../javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt | 4 ++-- .../javaagent/tooling/muzzle/ClassLoaderMatcher.java | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) 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 index e32754306ccf..96223740ce40 100644 --- 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 abstract val additionalDependencies: ListProperty abstract val excludedDependencies: ListProperty - abstract val excludedInstrumentationNames: ListProperty + abstract val excludedInstrumentationNames: SetProperty abstract val assertPass: Property abstract val assertInverse: Property internal abstract val coreJdk: Property // use coreJdk() function below to enable diff --git a/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt b/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt index e5d98529b34a..a79254366c24 100644 --- a/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt +++ b/gradle-plugins/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, - excludedInstrumentationNames: List, assertPass: Boolean) { + excludedInstrumentationNames: Set, assertPass: Boolean) { val matcherClass = agentClassLoader.loadClass("io.opentelemetry.javaagent.tooling.muzzle.ClassLoaderMatcher") @@ -63,7 +63,7 @@ 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, List::class.java) + .getMethod("matchesAll", ClassLoader::class.java, Boolean::class.javaPrimitiveType, Set::class.java) .invoke(null, userClassLoader, assertPass, excludedInstrumentationNames) as Map> 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 index 40912d43d894..609a8c2aa083 100644 --- 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,7 +31,7 @@ public class ClassLoaderMatcher { *

The returned map will be empty if and only if no instrumentation modules were found. */ public static Map> matchesAll( - ClassLoader classLoader, boolean injectHelpers, List excludedInstrumentationNames) { + ClassLoader classLoader, boolean injectHelpers, Set excludedInstrumentationNames) { Map> result = new HashMap<>(); ServiceLoader.load(InstrumentationModule.class) .forEach(