Remove unnecessary class loader matchers

Author: trask

.github/agents/knowledge/javaagent-module-patterns.md

@@ -39,33 +39,45 @@ public class MyLibrary10InstrumentationModule extends InstrumentationModule {
   `Arrays.asList(...)` for multiple items and `Collections.singletonList(...)` for a single
   item.
 
-### `classLoaderMatcher()` — Version-Boundary Detection
+### `classLoaderMatcher()` — Multi-Version Library Split
 
-Override `classLoaderMatcher()` only when the instrumentation targets a specific library
-version range and muzzle cannot distinguish that range on its own. Typical cases are classes
-that were added, removed, renamed, or introduced by a library's own native OpenTelemetry
-instrumentation, but are not referenced by the helper classes muzzle inspects.
+Override `classLoaderMatcher()` only when **multiple instrumentation versions exist for the
+same library** and the module still needs an extra class-presence check to keep one versioned
+module from applying where another versioned module should win. If `typeMatcher()` or
+`classLoaderOptimization()` already differentiates the versions, do not add
+`classLoaderMatcher()`.
+
+Do not add `classLoaderMatcher()` just because muzzle mismatches in cross tests, to avoid
+debug logging, or as a generic optimization. In the common case, the type instrumentation
+already short-circuits when the library is absent, and muzzle handles wrong-version rejection.
 
 **Execution order**: `classLoaderMatcher()` runs **first** and should be very cheap. It is
 followed by `typeMatcher()`, then muzzle. Use `classLoaderMatcher()` to reject obvious
-non-matches before the more expensive checks run.
+non-matches only when a versioned sibling module would otherwise overlap.
 
 **Override it when**:
 
-- A landmark class was **added** in the target version and excludes older versions.
-- A landmark class was **removed** in a newer version and excludes newer versions.
-- A newer version ships **native OpenTelemetry instrumentation** and the agent should back off.
+- There are **multiple instrumentation modules for one library** and a landmark class is needed
+  to separate their supported version ranges.
+- A landmark class was **added** in the target version and excludes older sibling modules.
+- A landmark class was **removed** in a newer version and excludes newer sibling modules.
+- A newer version ships **native OpenTelemetry instrumentation** and the older module should
+  back off.
 
 **Do not override it for**:
 
+- Single-version libraries with no competing instrumentation module.
+- Cross-test-driven preconditions whose only purpose is to avoid muzzle mismatches.
+- Library-absent fast paths that `typeMatcher()` or `classLoaderOptimization()` already cover.
 - Method signature changes.
 - Parameter type changes.
 - Field removals.
 - Package relocations that muzzle already sees.
 
 #### Pattern A: Single Landmark Class
 
-This is the most common case: one class cleanly identifies the lower bound.
+This is the most common valid case: one class cleanly identifies which sibling versioned
+module should handle the library.
 
 ```java
 @Override
@@ -176,8 +188,8 @@ the 3.7 module from activating on 4.0+ classloaders. That is why the comment mus
 
 #### Pattern C: Exclude Newer Versions with Native Instrumentation
 
-Use `.and(not(hasClassesNamed(...)))` when the newer library version ships its own
-OpenTelemetry instrumentation and the agent should opt out.
+Use `.and(not(hasClassesNamed(...)))` when a newer sibling version ships its own
+OpenTelemetry instrumentation and the older module should opt out.
 
 ```java
 @Override
@@ -204,13 +216,16 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
 
 #### Rules for `classLoaderMatcher()`
 
-- **Do NOT add `classLoaderMatcher()` for optimization.** The "skip when library is absent"
-  optimization belongs on `TypeInstrumentation.classLoaderOptimization()`, not here.
-  `classLoaderMatcher()` is only for **version-boundary detection**. Most modules do not need
-  it.
+- **Do NOT add `classLoaderMatcher()` for optimization or to silence muzzle mismatches.** The
+  "skip when library is absent" optimization belongs on
+  `TypeInstrumentation.classLoaderOptimization()`, not here. `classLoaderMatcher()` is only for
+  separating **multiple instrumentation versions of the same library** when the type
+  instrumentation does not already do that. Most modules do not need it.
 - **Do NOT flag modules that omit `classLoaderMatcher()`.** The default (`any()`) is correct
-  when muzzle can detect the version boundary on its own. Only flag a missing override when
-  the module truly depends on an added or removed landmark class that muzzle does not inspect.
+  when there is no competing sibling module, or when `typeMatcher()`,
+  `classLoaderOptimization()`, or muzzle already separates the versions. Only flag a missing
+  override when sibling versioned modules would otherwise overlap and the module truly depends
+  on an added or removed landmark class that the other checks do not inspect.
 - **Version comments are required on landmark classes.** For multi-class checks, or whenever
   the landmark version differs from the module's base version, every `hasClassesNamed()` call
   needs a role comment. When the entire return expression is a single `hasClassesNamed(...)`

instrumentation/activej-http-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/activejhttp/ActivejHttpServerInstrumentationModule.java

@@ -5,14 +5,12 @@
 
 package io.opentelemetry.javaagent.instrumentation.activejhttp;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Collections.singletonList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class ActivejHttpServerInstrumentationModule extends InstrumentationModule {
@@ -25,10 +23,4 @@ public ActivejHttpServerInstrumentationModule() {
   public List<TypeInstrumentation> typeInstrumentations() {
     return singletonList(new ActivejAsyncServletInstrumentation());
   }
-
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // added in 6.0
-    return hasClassesNamed("io.activej.http.HttpServer");
-  }
 }

instrumentation/armeria/armeria-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaInstrumentationModule.java

@@ -5,27 +5,19 @@
 
 package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Arrays.asList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class ArmeriaInstrumentationModule extends InstrumentationModule {
   public ArmeriaInstrumentationModule() {
     super("armeria", "armeria-1.3");
   }
 
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // Unrelated class which was added in Armeria 1.3.0, the minimum version we support.
-    return hasClassesNamed("com.linecorp.armeria.server.metric.PrometheusExpositionServiceBuilder");
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/clickhouse/clickhouse-client-v1-0.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/clickhouse/clientv1/v0_5/ClickHouseClientV1InstrumentationModule.java

@@ -5,15 +5,13 @@
 
 package io.opentelemetry.javaagent.instrumentation.clickhouse.clientv1.v0_5;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Collections.singletonList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class ClickHouseClientV1InstrumentationModule extends InstrumentationModule
@@ -28,12 +26,6 @@ public boolean isHelperClass(String className) {
     return "com.clickhouse.client.ClickHouseRequestAccess".equals(className);
   }
 
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // Unrelated class which was added in 0.5.0, the minimum version where tests pass
-    return hasClassesNamed("com.clickhouse.client.config.ClickHouseProxyType");
-  }
-
   @Override
   public List<String> injectedClassNames() {
     return singletonList("com.clickhouse.client.ClickHouseRequestAccess");

instrumentation/dropwizard/dropwizard-metrics-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/dropwizardmetrics/DropwizardMetricsInstrumentationModule.java

@@ -5,15 +5,12 @@
 
 package io.opentelemetry.javaagent.instrumentation.dropwizardmetrics;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Arrays.asList;
-import static net.bytebuddy.matcher.ElementMatchers.not;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class DropwizardMetricsInstrumentationModule extends InstrumentationModule {
@@ -22,12 +19,6 @@ public DropwizardMetricsInstrumentationModule() {
     super("dropwizard-metrics", "dropwizard-metrics-4.0");
   }
 
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // removed in 4.0
-    return not(hasClassesNamed("com.codahale.metrics.LongAdder"));
-  }
-
   @Override
   public boolean defaultEnabled() {
     // the Dropwizard metrics API does not have a concept of metric labels/tags/attributes, thus the

instrumentation/grizzly-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyInstrumentationModule.java

@@ -5,27 +5,19 @@
 
 package io.opentelemetry.javaagent.instrumentation.grizzly;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Arrays.asList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class GrizzlyInstrumentationModule extends InstrumentationModule {
   public GrizzlyInstrumentationModule() {
     super("grizzly", "grizzly-2.3");
   }
 
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // class added in 2.3
-    return hasClassesNamed("org.glassfish.grizzly.InputSource");
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/gwt-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtInstrumentationModule.java

@@ -5,14 +5,12 @@
 
 package io.opentelemetry.javaagent.instrumentation.gwt;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Collections.singletonList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class GwtInstrumentationModule extends InstrumentationModule {
@@ -21,12 +19,6 @@ public GwtInstrumentationModule() {
     super("gwt", "gwt-2.0");
   }
 
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // added in 2.0
-    return hasClassesNamed("com.google.gwt.uibinder.client.UiBinder");
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return singletonList(new GwtRpcInstrumentation());

instrumentation/jfinal-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jfinal/v3_2/JFinalInstrumentationModule.java

@@ -5,15 +5,12 @@
 
 package io.opentelemetry.javaagent.instrumentation.jfinal.v3_2;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Arrays.asList;
-import static net.bytebuddy.matcher.ElementMatchers.not;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class JFinalInstrumentationModule extends InstrumentationModule {
@@ -22,13 +19,6 @@ public JFinalInstrumentationModule() {
     super("jfinal", "jfinal-3.2");
   }
 
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // In version 3.2, TypeConverter is moved from com.jfinal.core
-    // to com.jfinal.core.converter
-    return not(hasClassesNamed("com.jfinal.core.TypeConverter"));
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new ActionMappingInstrumentation(), new InvocationInstrumentation());

instrumentation/kafka/kafka-connect-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaconnect/v2_6/KafkaConnectInstrumentationModule.java

@@ -5,14 +5,12 @@
 
 package io.opentelemetry.javaagent.instrumentation.kafkaconnect.v2_6;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Arrays.asList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class KafkaConnectInstrumentationModule extends InstrumentationModule {
@@ -25,10 +23,4 @@ public KafkaConnectInstrumentationModule() {
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new SinkTaskInstrumentation(), new WorkerSinkTaskInstrumentation());
   }
-
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // class added in 2.6.0
-    return hasClassesNamed("org.apache.kafka.connect.sink.SinkConnectorContext");
-  }
 }

instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/mdc/v1_2/Log4j1InstrumentationModule.java

@@ -5,27 +5,19 @@
 
 package io.opentelemetry.javaagent.instrumentation.log4j.mdc.v1_2;
 
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Arrays.asList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import java.util.List;
-import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
 public class Log4j1InstrumentationModule extends InstrumentationModule {
   public Log4j1InstrumentationModule() {
     super("log4j-mdc", "log4j-mdc-1.2");
   }
 
-  @Override
-  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // added in 1.2
-    return hasClassesNamed("org.apache.log4j.MDC");
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new CategoryInstrumentation(), new LoggingEventInstrumentation());