Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better muzzle instrumentation exclusion #6044

Merged
merged 3 commits into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: ListProperty<String>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could be a SetProperty

abstract val assertPass: Property<Boolean>
abstract val assertInverse: Property<Boolean>
internal abstract val coreJdk: Property<Boolean> // use coreJdk() function below to enable
Expand All @@ -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)
Expand Down Expand Up @@ -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?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class MuzzleGradlePluginUtil {
*/
@Suppress("UNCHECKED_CAST")
fun assertInstrumentationMuzzled(agentClassLoader: ClassLoader, userClassLoader: ClassLoader,
excludedInstrumentationModules: List<String>, assertPass: Boolean) {
excludedInstrumentationNames: List<String>, assertPass: Boolean) {

val matcherClass = agentClassLoader.loadClass("io.opentelemetry.javaagent.tooling.muzzle.ClassLoaderMatcher")

Expand All @@ -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<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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,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, List<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;
}
Expand Down