Browse Source

Allow disabling muzzle checks for specific methods (#7289)

Resolves
https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/2556

https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7265
made me wonder whether it would help when we could sometimes skip muzzle
checks.

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Lauri Tulmin 2 years ago
parent
commit
f3a21e86f5

+ 0 - 2
conventions/build.gradle.kts

@@ -42,8 +42,6 @@ dependencies {
   implementation("com.google.guava:guava:31.1-jre")
   implementation("gradle.plugin.com.google.protobuf:protobuf-gradle-plugin:0.8.18")
   implementation("gradle.plugin.com.github.johnrengelman:shadow:7.1.2")
-  implementation("org.ow2.asm:asm:9.4")
-  implementation("org.ow2.asm:asm-tree:9.4")
   implementation("org.apache.httpcomponents:httpclient:4.5.14")
   implementation("org.gradle:test-retry-gradle-plugin:1.5.0")
   implementation("org.owasp:dependency-check-gradle:7.4.1")

+ 3 - 0
dependencyManagement/build.gradle.kts

@@ -40,6 +40,7 @@ val autoServiceVersion = "1.0.1"
 val autoValueVersion = "1.10.1"
 val errorProneVersion = "2.16"
 val byteBuddyVersion = "1.12.19"
+val asmVersion = "9.4"
 val jmhVersion = "1.36"
 val mockitoVersion = "4.9.0"
 val slf4jVersion = "2.0.5"
@@ -57,6 +58,8 @@ val CORE_DEPENDENCIES = listOf(
   "net.bytebuddy:byte-buddy-dep:${byteBuddyVersion}",
   "net.bytebuddy:byte-buddy-agent:${byteBuddyVersion}",
   "net.bytebuddy:byte-buddy-gradle-plugin:${byteBuddyVersion}",
+  "org.ow2.asm:asm:${asmVersion}",
+  "org.ow2.asm:asm-tree:${asmVersion}",
   "org.openjdk.jmh:jmh-core:${jmhVersion}",
   "org.openjdk.jmh:jmh-generator-bytecode:${jmhVersion}",
   "org.mockito:mockito-core:${mockitoVersion}",

+ 2 - 0
instrumentation/spring/spring-kafka-2.7/library/build.gradle.kts

@@ -3,6 +3,8 @@ plugins {
 }
 
 dependencies {
+  compileOnly(project(":muzzle"))
+
   compileOnly("com.google.auto.value:auto-value-annotations")
   annotationProcessor("com.google.auto.value:auto-value")
 

+ 5 - 40
instrumentation/spring/spring-kafka-2.7/library/src/main/java/io/opentelemetry/instrumentation/spring/kafka/v2_7/InstrumentedRecordInterceptor.java

@@ -9,9 +9,7 @@ import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.util.VirtualField;
-import java.lang.invoke.MethodHandle;
-import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodType;
+import io.opentelemetry.javaagent.tooling.muzzle.NoMuzzle;
 import javax.annotation.Nullable;
 import org.apache.kafka.clients.consumer.Consumer;
 import org.apache.kafka.clients.consumer.ConsumerRecord;
@@ -23,22 +21,6 @@ final class InstrumentedRecordInterceptor<K, V> implements RecordInterceptor<K,
       VirtualField.find(ConsumerRecord.class, Context.class);
   private static final VirtualField<ConsumerRecord<?, ?>, State<ConsumerRecord<?, ?>>> stateField =
       VirtualField.find(ConsumerRecord.class, State.class);
-  private static final MethodHandle interceptRecord;
-
-  static {
-    MethodHandle interceptRecordHandle;
-    try {
-      interceptRecordHandle =
-          MethodHandles.lookup()
-              .findVirtual(
-                  RecordInterceptor.class,
-                  "intercept",
-                  MethodType.methodType(ConsumerRecord.class, ConsumerRecord.class));
-    } catch (NoSuchMethodException | IllegalAccessException e) {
-      interceptRecordHandle = null;
-    }
-    interceptRecord = interceptRecordHandle;
-  }
 
   private final Instrumenter<ConsumerRecord<?, ?>, Void> processInstrumenter;
   @Nullable private final RecordInterceptor<K, V> decorated;
@@ -50,25 +32,13 @@ final class InstrumentedRecordInterceptor<K, V> implements RecordInterceptor<K,
     this.decorated = decorated;
   }
 
-  @SuppressWarnings({
-    "deprecation",
-    "unchecked"
-  }) // implementing deprecated method (removed in 3.0) for better compatibility
+  @NoMuzzle
+  @SuppressWarnings(
+      "deprecation") // implementing deprecated method (removed in 3.0) for better compatibility
   @Override
   public ConsumerRecord<K, V> intercept(ConsumerRecord<K, V> record) {
-    if (interceptRecord == null) {
-      return null;
-    }
     start(record);
-    if (decorated == null) {
-      return null;
-    }
-    try {
-      return (ConsumerRecord<K, V>) interceptRecord.invoke(decorated, record);
-    } catch (Throwable e) {
-      rethrow(e);
-      return null; // unreachable
-    }
+    return decorated == null ? record : decorated.intercept(record);
   }
 
   @Override
@@ -77,11 +47,6 @@ final class InstrumentedRecordInterceptor<K, V> implements RecordInterceptor<K,
     return decorated == null ? record : decorated.intercept(record, consumer);
   }
 
-  @SuppressWarnings("unchecked")
-  private static <E extends Throwable> void rethrow(Throwable e) throws E {
-    throw (E) e;
-  }
-
   private void start(ConsumerRecord<K, V> record) {
     Context parentContext = getParentContext(record);
 

+ 21 - 7
licenses/licenses.md

@@ -1,7 +1,7 @@
 
 #javaagent
 ##Dependency License Report
-_2022-12-09 13:28:49 PST_
+_2022-12-12 10:32:28 PST_
 ## Apache License, Version 2.0
 
 **1** **Group:** `com.blogspot.mydailyjava` **Name:** `weak-lock-free` **Version:** `0.18`
@@ -194,31 +194,45 @@ _2022-12-09 13:28:49 PST_
 > - **POM License**: Apache License, Version 2.0 - [http://www.apache.org/licenses/LICENSE-2.0](http://www.apache.org/licenses/LICENSE-2.0)
 > - **POM License**: The 3-Clause BSD License - [https://opensource.org/licenses/BSD-3-Clause](https://opensource.org/licenses/BSD-3-Clause)
 
-**43** **Group:** `org.yaml` **Name:** `snakeyaml` **Version:** `1.33`
+**43** **Group:** `org.ow2.asm` **Name:** `asm-tree` **Version:** `9.4`
+> - **Manifest Project URL**: [http://asm.ow2.org](http://asm.ow2.org)
+> - **Manifest License**: The 3-Clause BSD License (Not Packaged)
+> - **POM Project URL**: [http://asm.ow2.io/](http://asm.ow2.io/)
+> - **POM License**: Apache License, Version 2.0 - [http://www.apache.org/licenses/LICENSE-2.0](http://www.apache.org/licenses/LICENSE-2.0)
+> - **POM License**: The 3-Clause BSD License - [https://opensource.org/licenses/BSD-3-Clause](https://opensource.org/licenses/BSD-3-Clause)
+
+**44** **Group:** `org.yaml` **Name:** `snakeyaml` **Version:** `1.33`
 > - **Manifest License**: Apache License, Version 2.0 (Not Packaged)
 > - **POM Project URL**: [https://bitbucket.org/snakeyaml/snakeyaml](https://bitbucket.org/snakeyaml/snakeyaml)
 > - **POM License**: Apache License, Version 2.0 - [https://www.apache.org/licenses/LICENSE-2.0](https://www.apache.org/licenses/LICENSE-2.0)
 
 ## MIT License
 
-**44** **Group:** `org.slf4j` **Name:** `slf4j-api` **Version:** `2.0.5`
+**45** **Group:** `org.slf4j` **Name:** `slf4j-api` **Version:** `2.0.5`
 > - **POM Project URL**: [http://www.slf4j.org](http://www.slf4j.org)
 > - **POM License**: MIT License - [https://opensource.org/licenses/MIT](https://opensource.org/licenses/MIT)
 
-**45** **Group:** `org.slf4j` **Name:** `slf4j-simple` **Version:** `2.0.5`
+**46** **Group:** `org.slf4j` **Name:** `slf4j-simple` **Version:** `2.0.5`
 > - **POM Project URL**: [http://www.slf4j.org](http://www.slf4j.org)
 > - **POM License**: MIT License - [https://opensource.org/licenses/MIT](https://opensource.org/licenses/MIT)
 
 ## The 3-Clause BSD License
 
-**46** **Group:** `org.ow2.asm` **Name:** `asm` **Version:** `9.4`
+**47** **Group:** `org.ow2.asm` **Name:** `asm` **Version:** `9.4`
+> - **Manifest Project URL**: [http://asm.ow2.org](http://asm.ow2.org)
+> - **Manifest License**: The 3-Clause BSD License (Not Packaged)
+> - **POM Project URL**: [http://asm.ow2.io/](http://asm.ow2.io/)
+> - **POM License**: Apache License, Version 2.0 - [http://www.apache.org/licenses/LICENSE-2.0](http://www.apache.org/licenses/LICENSE-2.0)
+> - **POM License**: The 3-Clause BSD License - [https://opensource.org/licenses/BSD-3-Clause](https://opensource.org/licenses/BSD-3-Clause)
+
+**48** **Group:** `org.ow2.asm` **Name:** `asm-commons` **Version:** `9.4`
 > - **Manifest Project URL**: [http://asm.ow2.org](http://asm.ow2.org)
 > - **Manifest License**: The 3-Clause BSD License (Not Packaged)
 > - **POM Project URL**: [http://asm.ow2.io/](http://asm.ow2.io/)
 > - **POM License**: Apache License, Version 2.0 - [http://www.apache.org/licenses/LICENSE-2.0](http://www.apache.org/licenses/LICENSE-2.0)
 > - **POM License**: The 3-Clause BSD License - [https://opensource.org/licenses/BSD-3-Clause](https://opensource.org/licenses/BSD-3-Clause)
 
-**47** **Group:** `org.ow2.asm` **Name:** `asm-commons` **Version:** `9.4`
+**49** **Group:** `org.ow2.asm` **Name:** `asm-tree` **Version:** `9.4`
 > - **Manifest Project URL**: [http://asm.ow2.org](http://asm.ow2.org)
 > - **Manifest License**: The 3-Clause BSD License (Not Packaged)
 > - **POM Project URL**: [http://asm.ow2.io/](http://asm.ow2.io/)
@@ -227,4 +241,4 @@ _2022-12-09 13:28:49 PST_
 
 ## Unknown
 
-**48** **Group:** `com.squareup.okio` **Name:** `okio` **Version:** `3.0.0`
+**50** **Group:** `com.squareup.okio` **Name:** `okio` **Version:** `3.0.0`

+ 1 - 0
muzzle/build.gradle.kts

@@ -14,6 +14,7 @@ dependencies {
   annotationProcessor("com.google.auto.value:auto-value")
 
   api("net.bytebuddy:byte-buddy-dep")
+  implementation("org.ow2.asm:asm-tree")
 
   implementation(project(":javaagent-bootstrap"))
   implementation(project(":instrumentation-api"))

+ 16 - 0
muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/NoMuzzle.java

@@ -0,0 +1,16 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.tooling.muzzle;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/** Skip muzzle checks for methods annotated with this annotation. */
+@Retention(RetentionPolicy.CLASS)
+@Target(ElementType.METHOD)
+public @interface NoMuzzle {}

+ 27 - 3
muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceCollectingClassVisitor.java

@@ -28,6 +28,8 @@ import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.Type;
+import org.objectweb.asm.tree.AnnotationNode;
+import org.objectweb.asm.tree.MethodNode;
 
 /** Visit a class and collect all references made by the visited class. */
 // Additional things we could check
@@ -244,11 +246,33 @@ final class ReferenceCollectingClassVisitor extends ClassVisitor {
               .build());
     }
 
+    MethodVisitor methodVisitor =
+        super.visitMethod(access, name, descriptor, signature, exceptions);
+    MethodVisitor methodNode =
+        new MethodNode(Opcodes.ASM9, access, name, descriptor, signature, exceptions) {
+          @Override
+          public void visitEnd() {
+            super.visitEnd();
+
+            boolean skip = false;
+            if (invisibleAnnotations != null) {
+              for (AnnotationNode annotationNode : invisibleAnnotations) {
+                if (Type.getDescriptor(NoMuzzle.class).equals(annotationNode.desc)) {
+                  skip = true;
+                  break;
+                }
+              }
+            }
+            MethodVisitor target =
+                skip ? methodVisitor : new AdviceReferenceMethodVisitor(methodVisitor);
+            if (target != null) {
+              accept(target);
+            }
+          }
+        };
     // Additional references we could check
     // - Classes in signature (return type, params) and visible from this package
-    return new AdviceReferenceMethodVisitor(
-        new VirtualFieldCollectingMethodVisitor(
-            super.visitMethod(access, name, descriptor, signature, exceptions)));
+    return new VirtualFieldCollectingMethodVisitor(methodNode);
   }
 
   private static VisibilityFlag computeVisibilityFlag(int access) {