From 70c541814adcc1ac34ddeea66606e2a79d0e44e0 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 21 Oct 2021 14:47:50 -0700 Subject: [PATCH] Remove unshaded javax.annotation classes from bootstrap class loader (#4454) * Fix shading * Revert "Fix shading" This reverts commit 2aad3cfe211d384101aa9acc92c7986f796ed6a9. * Make javax.annotations compileOnly * Replace checker GuardedBy with otel api internal GuardedBy * Fix build * Fix errorprone failures * Remove extra newline * Move internal GuardedBy to instrumentation-api * empty commit * empty commit --- benchmark-jfr-analyzer/build.gradle.kts | 4 -- .../kotlin/otel.java-conventions.gradle.kts | 2 +- dependencyManagement/build.gradle.kts | 2 +- .../build.gradle.kts | 1 - instrumentation-api-caching/build.gradle.kts | 3 +- instrumentation-api/build.gradle.kts | 1 - .../api/internal/GuardedBy.java | 46 +++++++++++++++++++ .../rxjava2/TracingAssembly.java | 14 +++++- .../rxjava3/TracingAssembly.java | 14 +++++- javaagent-bootstrap/build.gradle.kts | 1 - javaagent-extension-api/build.gradle.kts | 1 - .../build.gradle.kts | 1 - .../ClassLoaderMatcherCacheHolder.java | 2 +- 13 files changed, 76 insertions(+), 16 deletions(-) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/GuardedBy.java diff --git a/benchmark-jfr-analyzer/build.gradle.kts b/benchmark-jfr-analyzer/build.gradle.kts index abdb3914644f..07d0987696f3 100644 --- a/benchmark-jfr-analyzer/build.gradle.kts +++ b/benchmark-jfr-analyzer/build.gradle.kts @@ -7,7 +7,3 @@ tasks.withType().configureEach { release.set(11) } } - -dependencies { - implementation("com.google.code.findbugs:jsr305:3.0.2") -} diff --git a/conventions/src/main/kotlin/otel.java-conventions.gradle.kts b/conventions/src/main/kotlin/otel.java-conventions.gradle.kts index 11772581714d..6cc656678535 100644 --- a/conventions/src/main/kotlin/otel.java-conventions.gradle.kts +++ b/conventions/src/main/kotlin/otel.java-conventions.gradle.kts @@ -94,7 +94,7 @@ dependencies { components.all() - compileOnly("org.checkerframework:checker-qual") + compileOnly("com.google.code.findbugs:jsr305") testImplementation("org.junit.jupiter:junit-jupiter-api") testImplementation("org.junit.jupiter:junit-jupiter-params") diff --git a/dependencyManagement/build.gradle.kts b/dependencyManagement/build.gradle.kts index 87be4a27f3c7..032a4dd35e42 100644 --- a/dependencyManagement/build.gradle.kts +++ b/dependencyManagement/build.gradle.kts @@ -109,7 +109,7 @@ val DEPENDENCIES = listOf( "io.netty:netty:3.10.6.Final", "org.assertj:assertj-core:3.21.0", "org.awaitility:awaitility:4.1.0", - "org.checkerframework:checker-qual:3.14.0", + "com.google.code.findbugs:jsr305:3.0.2", "org.codehaus.groovy:groovy-all:${groovyVersion}", "org.objenesis:objenesis:3.2", "org.spockframework:spock-core:1.3-groovy-2.5", diff --git a/instrumentation-api-annotation-support/build.gradle.kts b/instrumentation-api-annotation-support/build.gradle.kts index 09e0c01de8af..446635d9de33 100644 --- a/instrumentation-api-annotation-support/build.gradle.kts +++ b/instrumentation-api-annotation-support/build.gradle.kts @@ -20,7 +20,6 @@ dependencies { implementation("io.opentelemetry:opentelemetry-api-metrics") implementation("org.slf4j:slf4j-api") - implementation("com.google.code.findbugs:jsr305:3.0.2") compileOnly("com.google.auto.value:auto-value-annotations") annotationProcessor("com.google.auto.value:auto-value") diff --git a/instrumentation-api-caching/build.gradle.kts b/instrumentation-api-caching/build.gradle.kts index 8f91fcf41f34..2a30d047f22b 100644 --- a/instrumentation-api-caching/build.gradle.kts +++ b/instrumentation-api-caching/build.gradle.kts @@ -20,11 +20,10 @@ val shadowInclude by configurations.creating { } dependencies { - implementation("com.google.code.findbugs:jsr305:3.0.2") - compileOnly(project(":instrumentation-api-caching:caffeine2", configuration = "shadow")) compileOnly(project(":instrumentation-api-caching:caffeine3", configuration = "shadow")) + compileOnly("org.checkerframework:checker-qual:3.14.0") compileOnly("com.blogspot.mydailyjava:weak-lock-free") shadowInclude("com.blogspot.mydailyjava:weak-lock-free") } diff --git a/instrumentation-api/build.gradle.kts b/instrumentation-api/build.gradle.kts index 32098c085991..155fee8acd9c 100644 --- a/instrumentation-api/build.gradle.kts +++ b/instrumentation-api/build.gradle.kts @@ -30,7 +30,6 @@ dependencies { implementation("io.opentelemetry:opentelemetry-api-metrics") implementation("org.slf4j:slf4j-api") - implementation("com.google.code.findbugs:jsr305:3.0.2") compileOnly("com.google.auto.value:auto-value-annotations") annotationProcessor("com.google.auto.value:auto-value") diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/GuardedBy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/GuardedBy.java new file mode 100644 index 000000000000..df8e0be4b2c8 --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/GuardedBy.java @@ -0,0 +1,46 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.internal; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * The field or method to which this annotation is applied can only be accessed when holding a + * particular lock, which may be a built-in (synchronization) lock, or may be an explicit {@link + * java.util.concurrent.locks.Lock}. + * + *

The argument determines which lock guards the annotated field or method: + * + *

    + *
  • this : The string literal "this" means that this field is guarded by the class in which it + * is defined. + *
  • class-name.this : For inner classes, it may be necessary to disambiguate 'this'; the + * class-name.this designation allows you to specify which 'this' reference is intended + *
  • itself : For reference fields only; the object to which the field refers. + *
  • field-name : The lock object is referenced by the (instance or static) field specified by + * field-name. + *
  • class-name.field-name : The lock object is reference by the static field specified by + * class-name.field-name. + *
  • method-name() : The lock object is returned by calling the named nil-ary method. + *
  • class-name.class : The Class object for the specified class should be used as the lock + * object. + *
+ * + *

This annotation is similar to {@link javax.annotation.concurrent.GuardedBy} but has {@link + * RetentionPolicy#SOURCE} so it is not in published artifacts. We only apply this to private + * members, so there is no reason to publish them and we avoid requiring end users to have to depend + * on the annotations in their own build. See the original issue for more info. + */ +@Target({ElementType.FIELD, ElementType.METHOD}) +@Retention(RetentionPolicy.SOURCE) +public @interface GuardedBy { + /** The name of the object guarding the target. */ + String value(); +} diff --git a/instrumentation/rxjava/rxjava-2.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava2/TracingAssembly.java b/instrumentation/rxjava/rxjava-2.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava2/TracingAssembly.java index 84049d001b4c..fb83abef8a5d 100644 --- a/instrumentation/rxjava/rxjava-2.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava2/TracingAssembly.java +++ b/instrumentation/rxjava/rxjava-2.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava2/TracingAssembly.java @@ -25,6 +25,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategies; +import io.opentelemetry.instrumentation.api.internal.GuardedBy; import io.reactivex.Completable; import io.reactivex.CompletableObserver; import io.reactivex.Flowable; @@ -40,7 +41,6 @@ import io.reactivex.parallel.ParallelFlowable; import io.reactivex.plugins.RxJavaPlugins; import javax.annotation.Nullable; -import org.checkerframework.checker.lock.qual.GuardedBy; import org.reactivestreams.Subscriber; /** @@ -158,6 +158,7 @@ public void disable() { } } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableParallel() { oldOnParallelAssembly = RxJavaPlugins.getOnParallelAssembly(); @@ -167,6 +168,7 @@ private static void enableParallel() { parallelFlowable -> new TracingParallelFlowable(parallelFlowable, Context.current()))); } + @GuardedBy("TracingAssembly.class") private static void enableCompletable() { oldOnCompletableSubscribe = RxJavaPlugins.getOnCompletableSubscribe(); RxJavaPlugins.setOnCompletableSubscribe( @@ -180,6 +182,7 @@ private static void enableCompletable() { })); } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableFlowable() { oldOnFlowableSubscribe = RxJavaPlugins.getOnFlowableSubscribe(); @@ -199,6 +202,7 @@ private static void enableFlowable() { })); } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableObservable() { if (TracingObserver.canEnable()) { @@ -215,6 +219,7 @@ private static void enableObservable() { } } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableSingle() { oldOnSingleSubscribe = RxJavaPlugins.getOnSingleSubscribe(); @@ -229,6 +234,7 @@ private static void enableSingle() { })); } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableMaybe() { oldOnMaybeSubscribe = RxJavaPlugins.getOnMaybeSubscribe(); @@ -256,31 +262,37 @@ private static void enableWithSpanStrategy(boolean captureExperimentalSpanAttrib AsyncOperationEndStrategies.instance().registerStrategy(asyncOperationEndStrategy); } + @GuardedBy("TracingAssembly.class") private static void disableParallel() { RxJavaPlugins.setOnParallelAssembly(oldOnParallelAssembly); oldOnParallelAssembly = null; } + @GuardedBy("TracingAssembly.class") private static void disableObservable() { RxJavaPlugins.setOnObservableSubscribe(oldOnObservableSubscribe); oldOnObservableSubscribe = null; } + @GuardedBy("TracingAssembly.class") private static void disableCompletable() { RxJavaPlugins.setOnCompletableSubscribe(oldOnCompletableSubscribe); oldOnCompletableSubscribe = null; } + @GuardedBy("TracingAssembly.class") private static void disableFlowable() { RxJavaPlugins.setOnFlowableSubscribe(oldOnFlowableSubscribe); oldOnFlowableSubscribe = null; } + @GuardedBy("TracingAssembly.class") private static void disableSingle() { RxJavaPlugins.setOnSingleSubscribe(oldOnSingleSubscribe); oldOnSingleSubscribe = null; } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void disableMaybe() { RxJavaPlugins.setOnMaybeSubscribe( diff --git a/instrumentation/rxjava/rxjava-3.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava3/TracingAssembly.java b/instrumentation/rxjava/rxjava-3.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava3/TracingAssembly.java index 593feb96f34b..58ab9c06aa15 100644 --- a/instrumentation/rxjava/rxjava-3.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava3/TracingAssembly.java +++ b/instrumentation/rxjava/rxjava-3.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava3/TracingAssembly.java @@ -25,6 +25,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategies; +import io.opentelemetry.instrumentation.api.internal.GuardedBy; import io.reactivex.rxjava3.core.Completable; import io.reactivex.rxjava3.core.CompletableObserver; import io.reactivex.rxjava3.core.Flowable; @@ -40,7 +41,6 @@ import io.reactivex.rxjava3.parallel.ParallelFlowable; import io.reactivex.rxjava3.plugins.RxJavaPlugins; import javax.annotation.Nullable; -import org.checkerframework.checker.lock.qual.GuardedBy; import org.reactivestreams.Subscriber; /** @@ -158,6 +158,7 @@ public void disable() { } } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableParallel() { oldOnParallelAssembly = RxJavaPlugins.getOnParallelAssembly(); @@ -167,6 +168,7 @@ private static void enableParallel() { parallelFlowable -> new TracingParallelFlowable(parallelFlowable, Context.current()))); } + @GuardedBy("TracingAssembly.class") private static void enableCompletable() { oldOnCompletableSubscribe = RxJavaPlugins.getOnCompletableSubscribe(); RxJavaPlugins.setOnCompletableSubscribe( @@ -180,6 +182,7 @@ private static void enableCompletable() { })); } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableFlowable() { oldOnFlowableSubscribe = RxJavaPlugins.getOnFlowableSubscribe(); @@ -199,6 +202,7 @@ private static void enableFlowable() { })); } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableObservable() { oldOnObservableSubscribe = RxJavaPlugins.getOnObservableSubscribe(); @@ -213,6 +217,7 @@ private static void enableObservable() { })); } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableSingle() { oldOnSingleSubscribe = RxJavaPlugins.getOnSingleSubscribe(); @@ -227,6 +232,7 @@ private static void enableSingle() { })); } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void enableMaybe() { oldOnMaybeSubscribe = RxJavaPlugins.getOnMaybeSubscribe(); @@ -254,31 +260,37 @@ private static void enableWithSpanStrategy(boolean captureExperimentalSpanAttrib AsyncOperationEndStrategies.instance().registerStrategy(asyncOperationEndStrategy); } + @GuardedBy("TracingAssembly.class") private static void disableParallel() { RxJavaPlugins.setOnParallelAssembly(oldOnParallelAssembly); oldOnParallelAssembly = null; } + @GuardedBy("TracingAssembly.class") private static void disableObservable() { RxJavaPlugins.setOnObservableSubscribe(oldOnObservableSubscribe); oldOnObservableSubscribe = null; } + @GuardedBy("TracingAssembly.class") private static void disableCompletable() { RxJavaPlugins.setOnCompletableSubscribe(oldOnCompletableSubscribe); oldOnCompletableSubscribe = null; } + @GuardedBy("TracingAssembly.class") private static void disableFlowable() { RxJavaPlugins.setOnFlowableSubscribe(oldOnFlowableSubscribe); oldOnFlowableSubscribe = null; } + @GuardedBy("TracingAssembly.class") private static void disableSingle() { RxJavaPlugins.setOnSingleSubscribe(oldOnSingleSubscribe); oldOnSingleSubscribe = null; } + @GuardedBy("TracingAssembly.class") @SuppressWarnings({"rawtypes", "unchecked"}) private static void disableMaybe() { RxJavaPlugins.setOnMaybeSubscribe( diff --git a/javaagent-bootstrap/build.gradle.kts b/javaagent-bootstrap/build.gradle.kts index e1d5c3edf674..da07d66ec13e 100644 --- a/javaagent-bootstrap/build.gradle.kts +++ b/javaagent-bootstrap/build.gradle.kts @@ -8,7 +8,6 @@ group = "io.opentelemetry.javaagent" dependencies { implementation(project(":instrumentation-api")) implementation("org.slf4j:slf4j-api") - implementation("com.google.code.findbugs:jsr305:3.0.2") testImplementation(project(":testing-common")) testImplementation("org.mockito:mockito-core") diff --git a/javaagent-extension-api/build.gradle.kts b/javaagent-extension-api/build.gradle.kts index 96188b276cc1..0ac62d41639e 100644 --- a/javaagent-extension-api/build.gradle.kts +++ b/javaagent-extension-api/build.gradle.kts @@ -13,7 +13,6 @@ dependencies { implementation(project(":instrumentation-api")) implementation(project(":javaagent-instrumentation-api")) implementation("org.slf4j:slf4j-api") - implementation("com.google.code.findbugs:jsr305:3.0.2") // metrics are unstable, do not expose as api implementation("io.opentelemetry:opentelemetry-sdk-metrics") diff --git a/javaagent-instrumentation-api/build.gradle.kts b/javaagent-instrumentation-api/build.gradle.kts index 77c0aa6467cc..ca4f0114f13a 100644 --- a/javaagent-instrumentation-api/build.gradle.kts +++ b/javaagent-instrumentation-api/build.gradle.kts @@ -11,7 +11,6 @@ dependencies { api(project(":instrumentation-api")) implementation("org.slf4j:slf4j-api") - implementation("com.google.code.findbugs:jsr305:3.0.2") compileOnly("com.google.auto.value:auto-value-annotations") annotationProcessor("com.google.auto.value:auto-value") diff --git a/javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/ClassLoaderMatcherCacheHolder.java b/javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/ClassLoaderMatcherCacheHolder.java index 90298591b233..fbaecc2985e1 100644 --- a/javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/ClassLoaderMatcherCacheHolder.java +++ b/javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/ClassLoaderMatcherCacheHolder.java @@ -6,10 +6,10 @@ package io.opentelemetry.javaagent.instrumentation.api.internal; import io.opentelemetry.instrumentation.api.caching.Cache; +import io.opentelemetry.instrumentation.api.internal.GuardedBy; import java.net.URL; import java.util.ArrayList; import java.util.List; -import org.checkerframework.checker.lock.qual.GuardedBy; /** * A holder of all ClassLoaderMatcher caches. We store them in the bootstrap classloader so that