From a5513a3c601a219b434aae7851b8a09ccc989771 Mon Sep 17 00:00:00 2001 From: HaloFour Date: Fri, 6 Aug 2021 13:42:18 -0400 Subject: [PATCH] Port opentelemetry-annotations-1.0 to Instrumenter API (#3738) * Port WithSpanInstrumentation to Instrumenter API * Unit tests, clean up attribute binding APIs * Remove AsyncSpanEndStrategies and fix weak reference purging * Move tryToGetResponse to AsyncOperationEndSupport * Address PR comments * ParameterAttributeNamesExtractor can no longer return a null array --- .../build.gradle.kts | 1 + .../support/BaseAttributeBinder.java | 106 ---------- .../MethodSpanAttributesExtractor.java | 96 +++++++-- .../ParameterAttributeNamesExtractor.java | 5 +- .../async/AsyncOperationEndSupport.java | 11 +- .../async/Jdk8AsyncOperationEndStrategy.java | 11 +- .../tracer/async/AsyncSpanEndStrategies.java | 52 ----- .../tracer/async/AsyncSpanEndStrategy.java | 33 --- .../async/Jdk8AsyncSpanEndStrategy.java | 75 ------- .../api/tracer/async/package-info.java | 8 - .../support/BaseAttributeBinderTest.groovy | 122 ----------- .../MethodSpanAttributesExtractorTest.groovy | 200 ++++++++++++++++++ .../async/AsyncOperationEndSupportTest.java | 4 +- .../async/Jdk8AsyncSpanEndStrategyTest.groovy | 80 ------- .../annotation/support/MethodCacheTest.java | 88 ++++++++ .../guava/InstrumentationHelper.java | 2 - .../guava/GuavaAsyncOperationEndStrategy.java | 55 ++--- .../otelannotations/MethodRequest.java | 26 +++ .../WithSpanInstrumentation.java | 52 ++--- ...SpanParameterAttributeNamesExtractor.java} | 20 +- .../otelannotations/WithSpanSingletons.java | 88 ++++++++ .../otelannotations/WithSpanTracer.java | 125 ----------- .../ReactorAsyncOperationEndStrategy.java | 38 +--- .../reactor/TracingOperator.java | 18 +- .../RxJava2AsyncOperationEndStrategy.java | 40 +--- .../rxjava2/TracingAssembly.java | 6 - .../RxJava3AsyncOperationEndStrategy.java | 40 +--- .../rxjava3/TracingAssembly.java | 4 - .../WeakRefAsyncOperationEndStrategies.java | 36 ++-- 29 files changed, 592 insertions(+), 850 deletions(-) delete mode 100644 instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/BaseAttributeBinder.java delete mode 100644 instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/AsyncSpanEndStrategies.java delete mode 100644 instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/AsyncSpanEndStrategy.java delete mode 100644 instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/Jdk8AsyncSpanEndStrategy.java delete mode 100644 instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java delete mode 100644 instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/BaseAttributeBinderTest.groovy create mode 100644 instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractorTest.groovy delete mode 100644 instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/Jdk8AsyncSpanEndStrategyTest.groovy create mode 100644 instrumentation-api-annotation-support/src/test/java/io/opentelemetry/instrumentation/api/annotation/support/MethodCacheTest.java create mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/MethodRequest.java rename instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/{WithSpanAttributeBinder.java => WithSpanParameterAttributeNamesExtractor.java} (72%) create mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanSingletons.java delete mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanTracer.java diff --git a/instrumentation-api-annotation-support/build.gradle.kts b/instrumentation-api-annotation-support/build.gradle.kts index e3f1537105..0a4987c86c 100644 --- a/instrumentation-api-annotation-support/build.gradle.kts +++ b/instrumentation-api-annotation-support/build.gradle.kts @@ -12,6 +12,7 @@ dependencies { // this only exists to make Intellij happy since it doesn't (currently at least) understand our // inclusion of this artifact inside of :instrumentation-api compileOnly(project(":instrumentation-api-caching")) + testCompileOnly(project(":instrumentation-api-caching")) api("io.opentelemetry:opentelemetry-api") api("io.opentelemetry:opentelemetry-semconv") diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/BaseAttributeBinder.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/BaseAttributeBinder.java deleted file mode 100644 index c76b556356..0000000000 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/BaseAttributeBinder.java +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.annotation.support; - -import io.opentelemetry.instrumentation.api.tracer.AttributeSetter; -import java.lang.reflect.Method; -import java.lang.reflect.Parameter; -import org.checkerframework.checker.nullness.qual.Nullable; - -/** Base class for instrumentation-specific attribute binding for traced methods. */ -public abstract class BaseAttributeBinder { - - /** - * Creates a binding of the parameters of the traced method to span attributes. - * - * @param method the traced method - * @return the bindings of the parameters - */ - public AttributeBindings bind(Method method) { - AttributeBindings bindings = EmptyAttributeBindings.INSTANCE; - - Parameter[] parameters = method.getParameters(); - if (parameters == null || parameters.length == 0) { - return bindings; - } - - String[] attributeNames = attributeNamesForParameters(method, parameters); - if (attributeNames == null || attributeNames.length != parameters.length) { - return bindings; - } - - for (int i = 0; i < parameters.length; i++) { - Parameter parameter = parameters[i]; - String attributeName = attributeNames[i]; - if (attributeName == null || attributeName.isEmpty()) { - continue; - } - - bindings = - new CombinedAttributeBindings( - bindings, - i, - AttributeBindingFactory.createBinding( - attributeName, parameter.getParameterizedType())); - } - - return bindings; - } - - /** - * Returns an array of the names of the attributes for the parameters of the traced method. The - * array should be the same length as the array of the method parameters. An element may be {@code - * null} to indicate that the parameter should not be bound to an attribute. The array may also be - * {@code null} to indicate that the method has no parameters to bind to attributes. - * - * @param method the traced method - * @param parameters the method parameters - * @return an array of the attribute names - */ - @Nullable - protected abstract String[] attributeNamesForParameters(Method method, Parameter[] parameters); - - protected enum EmptyAttributeBindings implements AttributeBindings { - INSTANCE; - - @Override - public boolean isEmpty() { - return true; - } - - @Override - public void apply(AttributeSetter setter, Object[] args) {} - } - - private static final class CombinedAttributeBindings implements AttributeBindings { - private final AttributeBindings parent; - private final int index; - private final AttributeBinding binding; - - public CombinedAttributeBindings( - AttributeBindings parent, int index, AttributeBinding binding) { - this.parent = parent; - this.index = index; - this.binding = binding; - } - - @Override - public boolean isEmpty() { - return false; - } - - @Override - public void apply(AttributeSetter setter, Object[] args) { - parent.apply(setter, args); - if (args != null && args.length > index) { - Object arg = args[index]; - if (arg != null) { - binding.apply(setter, arg); - } - } - } - } -} diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractor.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractor.java index 7f71c226da..ac24f517a5 100644 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractor.java +++ b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractor.java @@ -8,6 +8,7 @@ package io.opentelemetry.instrumentation.api.annotation.support; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.instrumentation.api.caching.Cache; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.tracer.AttributeSetter; import java.lang.reflect.Method; import java.lang.reflect.Parameter; import org.checkerframework.checker.nullness.qual.Nullable; @@ -16,10 +17,10 @@ import org.checkerframework.checker.nullness.qual.Nullable; public final class MethodSpanAttributesExtractor extends AttributesExtractor { - private final BaseAttributeBinder binder; private final MethodExtractor methodExtractor; private final MethodArgumentsExtractor methodArgumentsExtractor; private final Cache cache; + private final ParameterAttributeNamesExtractor parameterAttributeNamesExtractor; public static MethodSpanAttributesExtractor newInstance( MethodExtractor methodExtractor, @@ -27,23 +28,27 @@ public final class MethodSpanAttributesExtractor MethodArgumentsExtractor methodArgumentsExtractor) { return new MethodSpanAttributesExtractor<>( - methodExtractor, parameterAttributeNamesExtractor, methodArgumentsExtractor); + methodExtractor, + parameterAttributeNamesExtractor, + methodArgumentsExtractor, + new MethodCache<>()); } MethodSpanAttributesExtractor( MethodExtractor methodExtractor, ParameterAttributeNamesExtractor parameterAttributeNamesExtractor, - MethodArgumentsExtractor methodArgumentsExtractor) { + MethodArgumentsExtractor methodArgumentsExtractor, + Cache cache) { this.methodExtractor = methodExtractor; this.methodArgumentsExtractor = methodArgumentsExtractor; - this.binder = new MethodSpanAttributeBinder(parameterAttributeNamesExtractor); - this.cache = new MethodCache<>(); + this.parameterAttributeNamesExtractor = parameterAttributeNamesExtractor; + this.cache = cache; } @Override protected void onStart(AttributesBuilder attributes, REQUEST request) { Method method = methodExtractor.extract(request); - AttributeBindings bindings = cache.computeIfAbsent(method, binder::bind); + AttributeBindings bindings = cache.computeIfAbsent(method, this::bind); if (!bindings.isEmpty()) { Object[] args = methodArgumentsExtractor.extract(request); bindings.apply(attributes::put, args); @@ -54,18 +59,81 @@ public final class MethodSpanAttributesExtractor protected void onEnd( AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {} - private static class MethodSpanAttributeBinder extends BaseAttributeBinder { - private final ParameterAttributeNamesExtractor parameterAttributeNamesExtractor; + /** + * Creates a binding of the parameters of the traced method to span attributes. + * + * @param method the traced method + * @return the bindings of the parameters + */ + private AttributeBindings bind(Method method) { + AttributeBindings bindings = EmptyAttributeBindings.INSTANCE; - public MethodSpanAttributeBinder( - ParameterAttributeNamesExtractor parameterAttributeNamesExtractor) { - this.parameterAttributeNamesExtractor = parameterAttributeNamesExtractor; + Parameter[] parameters = method.getParameters(); + if (parameters.length == 0) { + return bindings; + } + + String[] attributeNames = parameterAttributeNamesExtractor.extract(method, parameters); + if (attributeNames.length != parameters.length) { + return bindings; + } + + for (int i = 0; i < parameters.length; i++) { + Parameter parameter = parameters[i]; + String attributeName = attributeNames[i]; + if (attributeName == null || attributeName.isEmpty()) { + continue; + } + + bindings = + new CombinedAttributeBindings( + bindings, + i, + AttributeBindingFactory.createBinding( + attributeName, parameter.getParameterizedType())); + } + + return bindings; + } + + protected enum EmptyAttributeBindings implements AttributeBindings { + INSTANCE; + + @Override + public boolean isEmpty() { + return true; } @Override - protected @Nullable String[] attributeNamesForParameters( - Method method, Parameter[] parameters) { - return parameterAttributeNamesExtractor.extract(method, parameters); + public void apply(AttributeSetter setter, Object[] args) {} + } + + private static final class CombinedAttributeBindings implements AttributeBindings { + private final AttributeBindings parent; + private final int index; + private final AttributeBinding binding; + + public CombinedAttributeBindings( + AttributeBindings parent, int index, AttributeBinding binding) { + this.parent = parent; + this.index = index; + this.binding = binding; + } + + @Override + public boolean isEmpty() { + return false; + } + + @Override + public void apply(AttributeSetter setter, Object[] args) { + parent.apply(setter, args); + if (args != null && args.length > index) { + Object arg = args[index]; + if (arg != null) { + binding.apply(setter, arg); + } + } } } } diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/ParameterAttributeNamesExtractor.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/ParameterAttributeNamesExtractor.java index 64d406ab54..5bda21b45b 100644 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/ParameterAttributeNamesExtractor.java +++ b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/ParameterAttributeNamesExtractor.java @@ -14,9 +14,8 @@ import org.checkerframework.checker.nullness.qual.Nullable; public interface ParameterAttributeNamesExtractor { /** * Returns an array of the names of the attributes for the parameters of the traced method. The - * array should be the same length as the array of the method parameters. An element may be {@code - * null} to indicate that the parameter should not be bound to an attribute. The array may also be - * {@code null} to indicate that the method has no parameters to bind to attributes. + * array must be the same length as the array of the method parameters. An element may be {@code + * null} to indicate that the parameter should not be bound to an attribute. * * @param method the traced method * @param parameters the method parameters diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/async/AsyncOperationEndSupport.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/async/AsyncOperationEndSupport.java index 4eddd97325..b34756458c 100644 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/async/AsyncOperationEndSupport.java +++ b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/async/AsyncOperationEndSupport.java @@ -79,7 +79,16 @@ public final class AsyncOperationEndSupport { } // fall back to sync end() if asyncValue type doesn't match - instrumenter.end(context, request, null, null); + instrumenter.end(context, request, tryToGetResponse(responseType, asyncValue), null); return asyncValue; } + + @Nullable + public static RESPONSE tryToGetResponse( + Class responseType, @Nullable Object asyncValue) { + if (responseType.isInstance(asyncValue)) { + return responseType.cast(asyncValue); + } + return null; + } } diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/async/Jdk8AsyncOperationEndStrategy.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/async/Jdk8AsyncOperationEndStrategy.java index 66f6f1bdb3..c1eea3a419 100644 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/async/Jdk8AsyncOperationEndStrategy.java +++ b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/async/Jdk8AsyncOperationEndStrategy.java @@ -5,11 +5,12 @@ package io.opentelemetry.instrumentation.api.annotation.support.async; +import static io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport.tryToGetResponse; + import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; -import org.checkerframework.checker.nullness.qual.Nullable; public enum Jdk8AsyncOperationEndStrategy implements AsyncOperationEndStrategy { INSTANCE; @@ -76,12 +77,4 @@ public enum Jdk8AsyncOperationEndStrategy implements AsyncOperationEndStrategy { (result, exception) -> instrumenter.end(context, request, tryToGetResponse(responseType, result), exception)); } - - @Nullable - private static RESPONSE tryToGetResponse(Class responseType, Object result) { - if (responseType.isInstance(result)) { - return responseType.cast(result); - } - return null; - } } diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/AsyncSpanEndStrategies.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/AsyncSpanEndStrategies.java deleted file mode 100644 index 9d7a0dd88d..0000000000 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/AsyncSpanEndStrategies.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.tracer.async; - -import java.util.List; -import java.util.Objects; -import java.util.concurrent.CopyOnWriteArrayList; -import org.checkerframework.checker.nullness.qual.Nullable; - -/** - * Registry of {@link AsyncSpanEndStrategy} implementations for tracing the asynchronous operations - * represented by the return type of a traced method. - */ -public class AsyncSpanEndStrategies { - private static final AsyncSpanEndStrategies instance = new AsyncSpanEndStrategies(); - - public static AsyncSpanEndStrategies getInstance() { - return instance; - } - - private final List strategies = new CopyOnWriteArrayList<>(); - - private AsyncSpanEndStrategies() { - strategies.add(Jdk8AsyncSpanEndStrategy.INSTANCE); - } - - public void registerStrategy(AsyncSpanEndStrategy strategy) { - Objects.requireNonNull(strategy); - strategies.add(strategy); - } - - public void unregisterStrategy(AsyncSpanEndStrategy strategy) { - strategies.remove(strategy); - } - - public void unregisterStrategy(Class strategyClass) { - strategies.removeIf(strategy -> strategy.getClass() == strategyClass); - } - - @Nullable - public AsyncSpanEndStrategy resolveStrategy(Class returnType) { - for (AsyncSpanEndStrategy strategy : strategies) { - if (strategy.supports(returnType)) { - return strategy; - } - } - return null; - } -} diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/AsyncSpanEndStrategy.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/AsyncSpanEndStrategy.java deleted file mode 100644 index 5981da300c..0000000000 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/AsyncSpanEndStrategy.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.tracer.async; - -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; - -/** - * Represents an implementation of a strategy for composing over the return value of an asynchronous - * traced method which can compose or register for notification of completion at which point the - * span representing the invocation of the method will be ended. - */ -public interface AsyncSpanEndStrategy { - boolean supports(Class returnType); - - /** - * Denotes the end of the invocation of the traced method with a successful result which will end - * the span stored in the passed {@code context}. The span will remain open until the asynchronous - * operation has completed. - * - * @param tracer {@link BaseTracer} tracer to be used to end the span stored in the {@code - * context}. - * @param returnValue Return value from the traced method. Must be an instance of a {@code - * returnType} for which {@link #supports(Class)} returned true (in particular it must not be - * {@code null}). - * @return Either {@code returnValue} or a value composing over {@code returnValue} for - * notification of completion. - */ - Object end(BaseTracer tracer, Context context, Object returnValue); -} diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/Jdk8AsyncSpanEndStrategy.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/Jdk8AsyncSpanEndStrategy.java deleted file mode 100644 index e0af4b7d6d..0000000000 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/Jdk8AsyncSpanEndStrategy.java +++ /dev/null @@ -1,75 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.tracer.async; - -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; - -enum Jdk8AsyncSpanEndStrategy implements AsyncSpanEndStrategy { - INSTANCE; - - @Override - public boolean supports(Class returnType) { - return returnType == CompletionStage.class || returnType == CompletableFuture.class; - } - - @Override - public Object end(BaseTracer tracer, Context context, Object returnValue) { - if (returnValue instanceof CompletableFuture) { - CompletableFuture future = (CompletableFuture) returnValue; - if (endSynchronously(future, tracer, context)) { - return future; - } - return endWhenComplete(future, tracer, context); - } - CompletionStage stage = (CompletionStage) returnValue; - return endWhenComplete(stage, tracer, context); - } - - /** - * Checks to see if the {@link CompletableFuture} has already been completed and if so - * synchronously ends the span to avoid additional allocations and overhead registering for - * notification of completion. - */ - private static boolean endSynchronously( - CompletableFuture future, BaseTracer tracer, Context context) { - - if (!future.isDone()) { - return false; - } - - if (future.isCompletedExceptionally()) { - // If the future completed exceptionally then join to catch the exception - // so that it can be recorded to the span - try { - future.join(); - } catch (Throwable t) { - tracer.endExceptionally(context, t); - return true; - } - } - tracer.end(context); - return true; - } - - /** - * Registers for notification of the completion of the {@link CompletionStage} at which time the - * span will be ended. - */ - private CompletionStage endWhenComplete( - CompletionStage stage, BaseTracer tracer, Context context) { - return stage.whenComplete( - (result, exception) -> { - if (exception != null) { - tracer.endExceptionally(context, exception); - } else { - tracer.end(context); - } - }); - } -} diff --git a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java b/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java deleted file mode 100644 index 8ca6a986d6..0000000000 --- a/instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java +++ /dev/null @@ -1,8 +0,0 @@ -/** - * Provides implementations of strategies for tracing methods that return asynchronous and reactive - * values so that the span can be ended when the asynchronous operation completes. - */ -@UnstableApi -package io.opentelemetry.instrumentation.api.tracer.async; - -import io.opentelemetry.instrumentation.api.annotations.UnstableApi; diff --git a/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/BaseAttributeBinderTest.groovy b/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/BaseAttributeBinderTest.groovy deleted file mode 100644 index 6dd1f72855..0000000000 --- a/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/BaseAttributeBinderTest.groovy +++ /dev/null @@ -1,122 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.annotation.support - -import io.opentelemetry.instrumentation.api.tracer.AttributeSetter -import spock.lang.Shared -import spock.lang.Specification - -import java.lang.reflect.Method -import java.lang.reflect.Parameter - -class BaseAttributeBinderTest extends Specification { - @Shared - Method method = TestClass.getDeclaredMethod("method", String, String, String) - - @Shared - Object[] args = [ "a", "b", "c" ] - - AttributeSetter setter = Mock() - - def "returns empty bindings for null attribute names array"() { - given: - def binder = new TestAttributeBinder(null) - - when: - AttributeBindings bindings = binder.bind(method) - bindings.apply(setter, args) - - then: - bindings.isEmpty() - 0 * setter.setAttribute(*spock.lang.Specification._) - } - - def "returns empty bindings for empty attribute names array"() { - given: - def binder = new TestAttributeBinder(new String[0]) - - when: - AttributeBindings bindings = binder.bind(method) - bindings.apply(setter, args) - - then: - bindings.isEmpty() - 0 * setter.setAttribute(*spock.lang.Specification._) - } - - def "returns empty bindings for attribute names array with all null elements"() { - given: - def binder = new TestAttributeBinder([ null, null, null ] as String[]) - - when: - AttributeBindings bindings = binder.bind(method) - bindings.apply(setter, args) - - then: - bindings.isEmpty() - 0 * setter.setAttribute(*spock.lang.Specification._) - } - - def "returns empty bindings for attribute names array with fewer elements than parameters"() { - given: - def binder = new TestAttributeBinder([ "x", "y" ] as String[]) - - when: - AttributeBindings bindings = binder.bind(method) - bindings.apply(setter, args) - - then: - bindings.isEmpty() - 0 * setter.setAttribute(*spock.lang.Specification._) - } - - def "returns bindings for attribute names array"() { - given: - def binder = new TestAttributeBinder([ "x", "y", "z" ] as String[]) - - when: - AttributeBindings bindings = binder.bind(method) - bindings.apply(setter, args) - - then: - !bindings.isEmpty() - 1 * setter.setAttribute({ it.getKey() == "x" }, "a") - 1 * setter.setAttribute({ it.getKey() == "y" }, "b") - 1 * setter.setAttribute({ it.getKey() == "z" }, "c") - } - - def "returns bindings for attribute names with null name"() { - given: - def binder = new TestAttributeBinder([ "x", null, "z" ] as String[]) - - when: - AttributeBindings bindings = binder.bind(method) - bindings.apply(setter, args) - - then: - !bindings.isEmpty() - 1 * setter.setAttribute({ it.getKey() == "x" }, "a") - 0 * setter.setAttribute(spock.lang.Specification._, "b") - 1 * setter.setAttribute({ it.getKey() == "z" }, "c") - } - - class TestAttributeBinder extends BaseAttributeBinder { - final String[] attributeNames - - TestAttributeBinder(String[] attributeNames) { - this.attributeNames = attributeNames - } - - @Override - protected String[] attributeNamesForParameters(Method method, Parameter[] parameters) { - return attributeNames - } - } - - class TestClass { - void method(String x, String y, String z) { } - } -} diff --git a/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractorTest.groovy b/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractorTest.groovy new file mode 100644 index 0000000000..7e1e04548e --- /dev/null +++ b/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractorTest.groovy @@ -0,0 +1,200 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.annotation.support + +import io.opentelemetry.api.common.AttributesBuilder +import io.opentelemetry.instrumentation.api.caching.Cache +import spock.lang.Specification + +import java.lang.reflect.Method +import java.util.function.Function + +class MethodSpanAttributesExtractorTest extends Specification { + + def "extracts attributes for method with attribute names"() { + given: + def request = new Object() + def method = TestClass.getDeclaredMethod("method", String, String, String) + AttributesBuilder builder = Mock() + + Cache cache = Mock { + 1 * computeIfAbsent(method, _ as Function) >> { m, fn -> fn.apply(m) } + } + + def extractor = new MethodSpanAttributesExtractor( + { r -> method }, + { m, p -> [ "x", "y", "z" ] as String[] }, + { r -> [ "a", "b", "c" ] as String[] }, + cache + ) + + when: + extractor.onStart(builder, request) + + then: + 1 * builder.put({ it.getKey() == "x" }, "a") + 1 * builder.put({ it.getKey() == "y" }, "b") + 1 * builder.put({ it.getKey() == "z" }, "c") + } + + def "does not extract attributes for empty attribute name array"() { + given: + def request = new Object() + def method = TestClass.getDeclaredMethod("method", String, String, String) + AttributesBuilder builder = Mock() + + Cache cache = Mock { + 1 * computeIfAbsent(method, _ as Function) >> { m, fn -> fn.apply(m) } + } + + def extractor = new MethodSpanAttributesExtractor( + { r -> method }, + { m, p -> new String[0] }, + { r -> [ "a", "b", "c" ] as String[] }, + cache + ) + + when: + extractor.onStart(builder, request) + + then: + 0 * builder.put(*_) + } + + def "does not extract attributes for method with attribute names array with fewer elements than parameters"() { + given: + def request = new Object() + def method = TestClass.getDeclaredMethod("method", String, String, String) + AttributesBuilder builder = Mock() + + Cache cache = Mock { + 1 * computeIfAbsent(method, _ as Function) >> { m, fn -> fn.apply(m) } + } + + def extractor = new MethodSpanAttributesExtractor( + { r -> method }, + { m, p -> [ "x", "y" ] as String[] }, + { r -> [ "a", "b", "c" ] as String[] }, + cache + ) + + when: + extractor.onStart(builder, request) + + then: + 0 * builder.put(*_) + } + + def "extracts attributes for method with attribute names array with null element"() { + given: + def request = new Object() + def method = TestClass.getDeclaredMethod("method", String, String, String) + AttributesBuilder builder = Mock() + + Cache cache = Mock { + 1 * computeIfAbsent(method, _ as Function) >> { m, fn -> fn.apply(m) } + } + + def extractor = new MethodSpanAttributesExtractor( + { r -> method }, + { m, p -> [ "x", null, "z" ] as String[] }, + { r -> [ "a", "b", "c" ] as String[] }, + cache + ) + + when: + extractor.onStart(builder, request) + + then: + 1 * builder.put({ it.getKey() == "x" }, "a") + 1 * builder.put({ it.getKey() == "z" }, "c") + 0 * builder.put(_, "b") + } + + def "does not extracts attribute for method with null argument"() { + given: + def request = new Object() + def method = TestClass.getDeclaredMethod("method", String, String, String) + AttributesBuilder builder = Mock() + + Cache cache = Mock { + 1 * computeIfAbsent(method, _ as Function) >> { m, fn -> fn.apply(m) } + } + + def extractor = new MethodSpanAttributesExtractor( + { r -> method }, + { m, p -> [ "x", "y", "z" ] as String[] }, + { r -> [ "a", "b", null ] as String[] }, + cache + ) + + when: + extractor.onStart(builder, request) + + then: + 1 * builder.put({ it.getKey() == "x" }, "a") + 1 * builder.put({ it.getKey() == "y" }, "b") + 0 * builder.put({ it.getKey() == "z" }, _) + } + + def "applies cached bindings"() { + given: + def request = new Object() + def method = TestClass.getDeclaredMethod("method", String, String, String) + AttributesBuilder builder = Mock() + + AttributeBindings bindings = Mock { + 1 * isEmpty() >> false + } + Cache cache = Mock { + 1 * computeIfAbsent(method, _ as Function) >> bindings + } + + def extractor = new MethodSpanAttributesExtractor( + { r -> method }, + { m, p -> throw new Exception() }, + { r -> [ "a", "b", "c" ] as String[] }, + cache + ) + + when: + extractor.onStart(builder, request) + + then: + 1 * bindings.apply(_, [ "a", "b", "c" ]) + } + + def "does not apply cached empty bindings"() { + given: + def request = new Object() + def method = TestClass.getDeclaredMethod("method", String, String, String) + AttributesBuilder builder = Mock() + + AttributeBindings bindings = Mock { + 1 * isEmpty() >> true + } + Cache cache = Mock { + 1 * computeIfAbsent(method, _ as Function) >> bindings + } + + def extractor = new MethodSpanAttributesExtractor( + { r -> method }, + { m, p -> throw new Exception() }, + { r -> throw new Exception() }, + cache + ) + + when: + extractor.onStart(builder, request) + + then: + 0 * bindings.apply(_, _) + } + + class TestClass { + void method(String x, String y, String z) { } + } +} diff --git a/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/async/AsyncOperationEndSupportTest.java b/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/async/AsyncOperationEndSupportTest.java index 3dd78da3fe..667a2b5093 100644 --- a/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/async/AsyncOperationEndSupportTest.java +++ b/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/annotation/support/async/AsyncOperationEndSupportTest.java @@ -63,7 +63,7 @@ class AsyncOperationEndSupportTest { } @Test - void shouldEndImmediatelyWhenAsyncWrapperisOfWrongType() { + void shouldEndImmediatelyWhenAsyncWrapperIsOfWrongType() { // given AsyncOperationEndSupport underTest = AsyncOperationEndSupport.create(instrumenter, String.class, CompletableFuture.class); @@ -76,7 +76,7 @@ class AsyncOperationEndSupportTest { // then assertSame("not async", result); - verify(instrumenter).end(context, "request", null, null); + verify(instrumenter).end(context, "request", "not async", null); } @Test diff --git a/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/Jdk8AsyncSpanEndStrategyTest.groovy b/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/Jdk8AsyncSpanEndStrategyTest.groovy deleted file mode 100644 index bb52b09f37..0000000000 --- a/instrumentation-api-annotation-support/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/Jdk8AsyncSpanEndStrategyTest.groovy +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.tracer.async - -import io.opentelemetry.context.Context -import io.opentelemetry.instrumentation.api.tracer.BaseTracer -import spock.lang.Specification - -import java.util.concurrent.CompletableFuture -import java.util.concurrent.CompletionException - -class Jdk8AsyncSpanEndStrategyTest extends Specification { - BaseTracer tracer - - Context context - - def underTest = Jdk8AsyncSpanEndStrategy.INSTANCE - - void setup() { - tracer = Mock() - context = Mock() - } - - def "ends span on completed future"() { - when: - underTest.end(tracer, context, CompletableFuture.completedFuture("completed")) - - then: - 1 * tracer.end(context) - } - - def "ends span exceptionally on failed future"() { - given: - def exception = new CompletionException() - def future = new CompletableFuture() - future.completeExceptionally(exception) - - when: - underTest.end(tracer, context, future) - - then: - 1 * tracer.endExceptionally(context, exception) - } - - def "ends span on future when complete"() { - def future = new CompletableFuture() - - when: - underTest.end(tracer, context, future) - - then: - 0 * tracer._ - - when: - future.complete("completed") - - then: - 1 * tracer.end(context) - } - - def "ends span exceptionally on future when completed exceptionally"() { - def future = new CompletableFuture() - def exception = new Exception() - - when: - underTest.end(tracer, context, future) - - then: - 0 * tracer._ - - when: - future.completeExceptionally(exception) - - then: - 1 * tracer.endExceptionally(context, exception) - } -} diff --git a/instrumentation-api-annotation-support/src/test/java/io/opentelemetry/instrumentation/api/annotation/support/MethodCacheTest.java b/instrumentation-api-annotation-support/src/test/java/io/opentelemetry/instrumentation/api/annotation/support/MethodCacheTest.java new file mode 100644 index 0000000000..9521f709fe --- /dev/null +++ b/instrumentation-api-annotation-support/src/test/java/io/opentelemetry/instrumentation/api/annotation/support/MethodCacheTest.java @@ -0,0 +1,88 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.annotation.support; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import io.opentelemetry.instrumentation.api.caching.Cache; +import java.lang.reflect.Method; +import java.util.function.Function; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +public class MethodCacheTest { + + @Mock private Function fn; + + @Test + public void getItemFromCache() throws Exception { + Cache cache = new MethodCache<>(); + Method key = TestClass.class.getDeclaredMethod("method"); + String value = "Value"; + + cache.put(key, value); + + assertThat(cache.get(key)).isEqualTo("Value"); + } + + @Test + void getItemFromCacheWithEquivalentMethod() throws Exception { + Cache cache = new MethodCache<>(); + Method key = TestClass.class.getDeclaredMethod("method"); + String value = "Value"; + + cache.put(key, value); + + Method otherKey = TestClass.class.getDeclaredMethod("method"); + assertThat(otherKey).isNotSameAs(key); + assertThat(cache.get(otherKey)).isEqualTo(value); + } + + @Test + void returnNullWhenNotInCache() throws Exception { + Cache cache = new MethodCache<>(); + Method key = TestClass.class.getDeclaredMethod("method"); + + assertThat(cache.get(key)).isNull(); + } + + @Test + void computesItemIfAbsent() throws Exception { + Cache cache = new MethodCache<>(); + Method key = TestClass.class.getDeclaredMethod("method"); + String value = "Value"; + when(fn.apply(key)).thenReturn(value); + + assertThat(cache.computeIfAbsent(key, fn)).isEqualTo(value); + verify(fn).apply(key); + + Method otherKey = TestClass.class.getDeclaredMethod("method"); + assertThat(cache.computeIfAbsent(otherKey, fn)).isEqualTo(value); + verifyNoMoreInteractions(fn); + } + + @Test + void doesNotComputeItemIfPresent() throws Exception { + Cache cache = new MethodCache<>(); + Method key = TestClass.class.getDeclaredMethod("method"); + String value = "Value"; + cache.put(key, value); + + assertThat(cache.computeIfAbsent(key, fn)).isEqualTo(value); + verifyNoInteractions(fn); + } + + static class TestClass { + public static void method() {} + } +} diff --git a/instrumentation/guava-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/guava/InstrumentationHelper.java b/instrumentation/guava-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/guava/InstrumentationHelper.java index b6f53a5434..4abd3a3778 100644 --- a/instrumentation/guava-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/guava/InstrumentationHelper.java +++ b/instrumentation/guava-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/guava/InstrumentationHelper.java @@ -7,7 +7,6 @@ package io.opentelemetry.javaagent.instrumentation.guava; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategies; import io.opentelemetry.instrumentation.api.config.Config; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategies; import io.opentelemetry.instrumentation.guava.GuavaAsyncOperationEndStrategy; public final class InstrumentationHelper { @@ -26,7 +25,6 @@ public final class InstrumentationHelper { private static final GuavaAsyncOperationEndStrategy asyncOperationEndStrategy; private static void registerAsyncSpanEndStrategy() { - AsyncSpanEndStrategies.getInstance().registerStrategy(asyncOperationEndStrategy); AsyncOperationEndStrategies.instance().registerStrategy(asyncOperationEndStrategy); } diff --git a/instrumentation/guava-10.0/library/src/main/java/io/opentelemetry/instrumentation/guava/GuavaAsyncOperationEndStrategy.java b/instrumentation/guava-10.0/library/src/main/java/io/opentelemetry/instrumentation/guava/GuavaAsyncOperationEndStrategy.java index e7581aa2e8..578c1548b8 100644 --- a/instrumentation/guava-10.0/library/src/main/java/io/opentelemetry/instrumentation/guava/GuavaAsyncOperationEndStrategy.java +++ b/instrumentation/guava-10.0/library/src/main/java/io/opentelemetry/instrumentation/guava/GuavaAsyncOperationEndStrategy.java @@ -5,6 +5,8 @@ package io.opentelemetry.instrumentation.guava; +import static io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport.tryToGetResponse; + import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.Uninterruptibles; import io.opentelemetry.api.common.AttributeKey; @@ -12,13 +14,8 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategy; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategy; -import java.util.function.BiConsumer; -import org.checkerframework.checker.nullness.qual.Nullable; -public final class GuavaAsyncOperationEndStrategy - implements AsyncOperationEndStrategy, AsyncSpanEndStrategy { +public final class GuavaAsyncOperationEndStrategy implements AsyncOperationEndStrategy { private static final AttributeKey CANCELED_ATTRIBUTE_KEY = AttributeKey.booleanKey("guava.canceled"); @@ -50,55 +47,33 @@ public final class GuavaAsyncOperationEndStrategy Class responseType) { ListenableFuture future = (ListenableFuture) asyncValue; - end( - context, - future, - (result, error) -> - instrumenter.end(context, request, tryToGetResponse(responseType, result), error)); + end(instrumenter, context, request, future, responseType); return future; } - @Override - public Object end(BaseTracer tracer, Context context, Object returnValue) { - ListenableFuture future = (ListenableFuture) returnValue; - end( - context, - future, - (result, error) -> { - if (error == null) { - tracer.end(context); - } else { - tracer.endExceptionally(context, error); - } - }); - return future; - } - - private void end(Context context, ListenableFuture future, BiConsumer end) { + private void end( + Instrumenter instrumenter, + Context context, + REQUEST request, + ListenableFuture future, + Class responseType) { if (future.isDone()) { if (future.isCancelled()) { if (captureExperimentalSpanAttributes) { Span.fromContext(context).setAttribute(CANCELED_ATTRIBUTE_KEY, true); } - end.accept(null, null); + instrumenter.end(context, request, null, null); } else { try { Object response = Uninterruptibles.getUninterruptibly(future); - end.accept(response, null); + instrumenter.end(context, request, tryToGetResponse(responseType, response), null); } catch (Throwable exception) { - end.accept(null, exception); + instrumenter.end(context, request, null, exception); } } } else { - future.addListener(() -> end(context, future, end), Runnable::run); + future.addListener( + () -> end(instrumenter, context, request, future, responseType), Runnable::run); } } - - @Nullable - private static RESPONSE tryToGetResponse(Class responseType, Object result) { - if (responseType.isInstance(result)) { - return responseType.cast(result); - } - return null; - } } diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/MethodRequest.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/MethodRequest.java new file mode 100644 index 0000000000..d886fc72ff --- /dev/null +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/MethodRequest.java @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.otelannotations; + +import java.lang.reflect.Method; + +public final class MethodRequest { + private final Method method; + private final Object[] args; + + public MethodRequest(Method method, Object[] args) { + this.method = method; + this.args = args; + } + + public Method method() { + return this.method; + } + + public Object[] args() { + return this.args; + } +} diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanInstrumentation.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanInstrumentation.java index 0ab0d32db2..6672b36cc9 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanInstrumentation.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanInstrumentation.java @@ -5,7 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.otelannotations; -import static io.opentelemetry.javaagent.instrumentation.otelannotations.WithSpanTracer.tracer; +import static io.opentelemetry.javaagent.instrumentation.otelannotations.WithSpanSingletons.instrumenter; +import static io.opentelemetry.javaagent.instrumentation.otelannotations.WithSpanSingletons.instrumenterWithAttributes; import static net.bytebuddy.matcher.ElementMatchers.declaresMethod; import static net.bytebuddy.matcher.ElementMatchers.hasParameters; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; @@ -15,11 +16,11 @@ import static net.bytebuddy.matcher.ElementMatchers.none; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.whereAny; -import application.io.opentelemetry.extension.annotations.WithSpan; -import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport; import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; @@ -115,23 +116,27 @@ public class WithSpanInstrumentation implements TypeInstrumentation { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.Origin Method method, + @Advice.Local("otelOperationEndSupport") + AsyncOperationEndSupport operationEndSupport, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - WithSpan applicationAnnotation = method.getAnnotation(WithSpan.class); - SpanKind kind = tracer().extractSpanKind(applicationAnnotation); + Instrumenter instrumenter = instrumenter(); Context current = Java8BytecodeBridge.currentContext(); - // don't create a nested span if you're not supposed to. - if (tracer().shouldStartSpan(current, kind)) { - context = tracer().startSpan(current, applicationAnnotation, method, kind, null); + if (instrumenter.shouldStart(current, method)) { + context = instrumenter.start(current, method); scope = context.makeCurrent(); + operationEndSupport = + AsyncOperationEndSupport.create(instrumenter, Object.class, method.getReturnType()); } } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Origin Method method, + @Advice.Local("otelOperationEndSupport") + AsyncOperationEndSupport operationEndSupport, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope, @Advice.Return(typing = Assigner.Typing.DYNAMIC, readOnly = false) Object returnValue, @@ -140,12 +145,7 @@ public class WithSpanInstrumentation implements TypeInstrumentation { return; } scope.close(); - - if (throwable != null) { - tracer().endExceptionally(context, throwable); - } else { - returnValue = tracer().end(context, method.getReturnType(), returnValue); - } + returnValue = operationEndSupport.asyncEnd(context, method, returnValue, throwable); } } @@ -156,23 +156,30 @@ public class WithSpanInstrumentation implements TypeInstrumentation { public static void onEnter( @Advice.Origin Method method, @Advice.AllArguments(typing = Assigner.Typing.DYNAMIC) Object[] args, + @Advice.Local("otelOperationEndSupport") + AsyncOperationEndSupport operationEndSupport, + @Advice.Local("otelRequest") MethodRequest request, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - WithSpan applicationAnnotation = method.getAnnotation(WithSpan.class); - SpanKind kind = tracer().extractSpanKind(applicationAnnotation); + Instrumenter instrumenter = instrumenterWithAttributes(); Context current = Java8BytecodeBridge.currentContext(); + request = new MethodRequest(method, args); - // don't create a nested span if you're not supposed to. - if (tracer().shouldStartSpan(current, kind)) { - context = tracer().startSpan(current, applicationAnnotation, method, kind, args); + if (instrumenter.shouldStart(current, request)) { + context = instrumenter.start(current, request); scope = context.makeCurrent(); + operationEndSupport = + AsyncOperationEndSupport.create(instrumenter, Object.class, method.getReturnType()); } } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Origin Method method, + @Advice.Local("otelOperationEndSupport") + AsyncOperationEndSupport operationEndSupport, + @Advice.Local("otelRequest") MethodRequest request, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope, @Advice.Return(typing = Assigner.Typing.DYNAMIC, readOnly = false) Object returnValue, @@ -181,12 +188,7 @@ public class WithSpanInstrumentation implements TypeInstrumentation { return; } scope.close(); - - if (throwable != null) { - tracer().endExceptionally(context, throwable); - } else { - returnValue = tracer().end(context, method.getReturnType(), returnValue); - } + returnValue = operationEndSupport.asyncEnd(context, request, returnValue, throwable); } } } diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAttributeBinder.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanParameterAttributeNamesExtractor.java similarity index 72% rename from instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAttributeBinder.java rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanParameterAttributeNamesExtractor.java index 0663bfc706..ab35653d30 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAttributeBinder.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanParameterAttributeNamesExtractor.java @@ -6,9 +6,7 @@ package io.opentelemetry.javaagent.instrumentation.otelannotations; import io.opentelemetry.instrumentation.api.annotation.support.AnnotationReflectionHelper; -import io.opentelemetry.instrumentation.api.annotation.support.AttributeBindings; -import io.opentelemetry.instrumentation.api.annotation.support.BaseAttributeBinder; -import io.opentelemetry.instrumentation.api.caching.Cache; +import io.opentelemetry.instrumentation.api.annotation.support.ParameterAttributeNamesExtractor; import java.lang.annotation.Annotation; import java.lang.invoke.MethodHandles; import java.lang.reflect.Method; @@ -16,15 +14,14 @@ import java.lang.reflect.Parameter; import java.util.function.Function; import org.checkerframework.checker.nullness.qual.Nullable; -class WithSpanAttributeBinder extends BaseAttributeBinder { +public enum WithSpanParameterAttributeNamesExtractor implements ParameterAttributeNamesExtractor { + INSTANCE; - private static final Cache bindings = - Cache.newBuilder().setWeakKeys().build(); private static final Class spanAttributeAnnotation; private static final Function spanAttributeValueFunction; static { - ClassLoader classLoader = WithSpanAttributeBinder.class.getClassLoader(); + ClassLoader classLoader = WithSpanParameterAttributeNamesExtractor.class.getClassLoader(); spanAttributeAnnotation = AnnotationReflectionHelper.forNameOrNull( classLoader, "io.opentelemetry.extension.annotations.SpanAttribute"); @@ -46,14 +43,7 @@ class WithSpanAttributeBinder extends BaseAttributeBinder { } @Override - public AttributeBindings bind(Method method) { - return spanAttributeAnnotation != null - ? bindings.computeIfAbsent(method, super::bind) - : EmptyAttributeBindings.INSTANCE; - } - - @Override - protected @Nullable String[] attributeNamesForParameters(Method method, Parameter[] parameters) { + public @Nullable String[] extract(Method method, Parameter[] parameters) { String[] attributeNames = new String[parameters.length]; for (int i = 0; i < parameters.length; i++) { attributeNames[i] = attributeName(parameters[i]); diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanSingletons.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanSingletons.java new file mode 100644 index 0000000000..17b6adcecc --- /dev/null +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanSingletons.java @@ -0,0 +1,88 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.otelannotations; + +import application.io.opentelemetry.extension.annotations.WithSpan; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.api.annotation.support.MethodSpanAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.tracer.SpanNames; +import java.lang.reflect.Method; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public final class WithSpanSingletons { + private static final String INSTRUMENTATION_NAME = + "io.opentelemetry.opentelemetry-annotations-1.0"; + + private static final Logger logger = LoggerFactory.getLogger(WithSpanSingletons.class); + private static final Instrumenter INSTRUMENTER = createInstrumenter(); + private static final Instrumenter INSTRUMENTER_WITH_ATTRIBUTES = + createInstrumenterWithAttributes(); + + public static Instrumenter instrumenter() { + return INSTRUMENTER; + } + + public static Instrumenter instrumenterWithAttributes() { + return INSTRUMENTER_WITH_ATTRIBUTES; + } + + private static Instrumenter createInstrumenter() { + return Instrumenter.newBuilder( + GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, WithSpanSingletons::spanNameFromMethod) + .newInstrumenter(WithSpanSingletons::spanKindFromMethod); + } + + private static Instrumenter createInstrumenterWithAttributes() { + return Instrumenter.newBuilder( + GlobalOpenTelemetry.get(), + INSTRUMENTATION_NAME, + WithSpanSingletons::spanNameFromMethodRequest) + .addAttributesExtractor( + MethodSpanAttributesExtractor.newInstance( + MethodRequest::method, + WithSpanParameterAttributeNamesExtractor.INSTANCE, + MethodRequest::args)) + .newInstrumenter(WithSpanSingletons::spanKindFromMethodRequest); + } + + private static SpanKind spanKindFromMethodRequest(MethodRequest request) { + return spanKindFromMethod(request.method()); + } + + private static SpanKind spanKindFromMethod(Method method) { + WithSpan annotation = method.getDeclaredAnnotation(WithSpan.class); + if (annotation == null) { + return SpanKind.INTERNAL; + } + return toAgentOrNull(annotation.kind()); + } + + private static SpanKind toAgentOrNull( + application.io.opentelemetry.api.trace.SpanKind applicationSpanKind) { + try { + return SpanKind.valueOf(applicationSpanKind.name()); + } catch (IllegalArgumentException e) { + logger.debug("unexpected span kind: {}", applicationSpanKind.name()); + return SpanKind.INTERNAL; + } + } + + private static String spanNameFromMethodRequest(MethodRequest request) { + return spanNameFromMethod(request.method()); + } + + private static String spanNameFromMethod(Method method) { + WithSpan annotation = method.getDeclaredAnnotation(WithSpan.class); + String spanName = annotation.value(); + if (spanName.isEmpty()) { + spanName = SpanNames.fromMethod(method); + } + return spanName; + } +} diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanTracer.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanTracer.java deleted file mode 100644 index 519ee5aa0c..0000000000 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanTracer.java +++ /dev/null @@ -1,125 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.otelannotations; - -import application.io.opentelemetry.extension.annotations.WithSpan; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanBuilder; -import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.annotation.support.AttributeBindings; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.SpanNames; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategies; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategy; -import java.lang.reflect.Method; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class WithSpanTracer extends BaseTracer { - private static final WithSpanTracer TRACER = new WithSpanTracer(); - - public static WithSpanTracer tracer() { - return TRACER; - } - - private static final Logger logger = LoggerFactory.getLogger(WithSpanTracer.class); - - private final WithSpanAttributeBinder attributeBinder = new WithSpanAttributeBinder(); - private final AsyncSpanEndStrategies asyncSpanEndStrategies = - AsyncSpanEndStrategies.getInstance(); - - public Context startSpan( - Context parentContext, - WithSpan applicationAnnotation, - Method method, - SpanKind kind, - Object[] args) { - - SpanBuilder spanBuilder = - spanBuilder( - parentContext, spanNameForMethodWithAnnotation(applicationAnnotation, method), kind); - Span span = withSpanAttributes(spanBuilder, method, args).startSpan(); - - if (kind == SpanKind.SERVER) { - return withServerSpan(parentContext, span); - } - if (kind == SpanKind.CLIENT) { - return withClientSpan(parentContext, span); - } - return parentContext.with(span); - } - - /** - * This method is used to generate an acceptable span (operation) name based on a given method - * reference. It first checks for existence of {@link WithSpan} annotation. If it is present, then - * tries to derive name from its {@code value} attribute. Otherwise delegates to {@link - * SpanNames#fromMethod(Method)}. - */ - public String spanNameForMethodWithAnnotation(WithSpan applicationAnnotation, Method method) { - if (applicationAnnotation != null && !applicationAnnotation.value().isEmpty()) { - return applicationAnnotation.value(); - } - return SpanNames.fromMethod(method); - } - - public SpanKind extractSpanKind(WithSpan applicationAnnotation) { - application.io.opentelemetry.api.trace.SpanKind applicationKind = - applicationAnnotation != null - ? applicationAnnotation.kind() - : application.io.opentelemetry.api.trace.SpanKind.INTERNAL; - return toAgentOrNull(applicationKind); - } - - public static SpanKind toAgentOrNull( - application.io.opentelemetry.api.trace.SpanKind applicationSpanKind) { - try { - return SpanKind.valueOf(applicationSpanKind.name()); - } catch (IllegalArgumentException e) { - logger.debug("unexpected span kind: {}", applicationSpanKind.name()); - return SpanKind.INTERNAL; - } - } - - public SpanBuilder withSpanAttributes(SpanBuilder spanBuilder, Method method, Object[] args) { - if (args != null && args.length > 0) { - AttributeBindings bindings = attributeBinder.bind(method); - if (!bindings.isEmpty()) { - bindings.apply(spanBuilder::setAttribute, args); - } - } - return spanBuilder; - } - - /** - * Denotes the end of the invocation of the traced method with a successful result which will end - * the span stored in the passed {@code context}. If the method returned a value representing an - * asynchronous operation then the span will not be finished until the asynchronous operation has - * completed. - * - * @param returnType Return type of the traced method. - * @param returnValue Return value from the traced method. - * @return Either {@code returnValue} or a value composing over {@code returnValue} for - * notification of completion. - * @throws ClassCastException if returnValue is not an instance of returnType - */ - public Object end(Context context, Class returnType, Object returnValue) { - if (returnType.isInstance(returnValue)) { - AsyncSpanEndStrategy asyncSpanEndStrategy = - asyncSpanEndStrategies.resolveStrategy(returnType); - if (asyncSpanEndStrategy != null) { - return asyncSpanEndStrategy.end(this, context, returnValue); - } - } - end(context); - return returnValue; - } - - @Override - protected String getInstrumentationName() { - return "io.opentelemetry.opentelemetry-annotations-1.0"; - } -} diff --git a/instrumentation/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/ReactorAsyncOperationEndStrategy.java b/instrumentation/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/ReactorAsyncOperationEndStrategy.java index 000fe0c9bd..5e2f7aab0d 100644 --- a/instrumentation/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/ReactorAsyncOperationEndStrategy.java +++ b/instrumentation/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/ReactorAsyncOperationEndStrategy.java @@ -5,22 +5,20 @@ package io.opentelemetry.instrumentation.reactor; +import static io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport.tryToGetResponse; + import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategy; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategy; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; -import org.checkerframework.checker.nullness.qual.Nullable; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; -public final class ReactorAsyncOperationEndStrategy - implements AsyncOperationEndStrategy, AsyncSpanEndStrategy { +public final class ReactorAsyncOperationEndStrategy implements AsyncOperationEndStrategy { private static final AttributeKey CANCELED_ATTRIBUTE_KEY = AttributeKey.booleanKey("reactor.canceled"); @@ -43,23 +41,6 @@ public final class ReactorAsyncOperationEndStrategy return returnType == Publisher.class || returnType == Mono.class || returnType == Flux.class; } - @Override - public Object end(BaseTracer tracer, Context context, Object returnValue) { - - EndOnFirstNotificationConsumer notificationConsumer = - new EndOnFirstNotificationConsumer(context) { - @Override - protected void end(Object result, Throwable error) { - if (error == null) { - tracer.end(context); - } else { - tracer.endExceptionally(context, error); - } - } - }; - return end(returnValue, notificationConsumer); - } - @Override public Object end( Instrumenter instrumenter, @@ -75,11 +56,6 @@ public final class ReactorAsyncOperationEndStrategy instrumenter.end(context, request, tryToGetResponse(responseType, result), error); } }; - return end(asyncValue, notificationConsumer); - } - - private static Object end( - Object asyncValue, EndOnFirstNotificationConsumer notificationConsumer) { if (asyncValue instanceof Mono) { Mono mono = (Mono) asyncValue; @@ -94,14 +70,6 @@ public final class ReactorAsyncOperationEndStrategy } } - @Nullable - private static RESPONSE tryToGetResponse(Class responseType, Object result) { - if (responseType.isInstance(result)) { - return responseType.cast(result); - } - return null; - } - /** * Helper class to ensure that the span is ended exactly once regardless of how many OnComplete or * OnError notifications are received. Multiple notifications can happen anytime multiple diff --git a/instrumentation/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/TracingOperator.java b/instrumentation/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/TracingOperator.java index 0731d5e68e..cedbadae81 100644 --- a/instrumentation/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/TracingOperator.java +++ b/instrumentation/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/TracingOperator.java @@ -24,7 +24,6 @@ package io.opentelemetry.instrumentation.reactor; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategies; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategies; import java.util.function.BiFunction; import java.util.function.Function; import org.reactivestreams.Publisher; @@ -61,25 +60,32 @@ public final class TracingOperator { * application. */ public void registerOnEachOperator() { - Hooks.onEachOperator(TracingSubscriber.class.getName(), tracingLift()); - AsyncSpanEndStrategies.getInstance().registerStrategy(asyncOperationEndStrategy); + Hooks.onEachOperator(TracingSubscriber.class.getName(), tracingLift(asyncOperationEndStrategy)); AsyncOperationEndStrategies.instance().registerStrategy(asyncOperationEndStrategy); } /** Unregisters the hook registered by {@link #registerOnEachOperator()}. */ public void resetOnEachOperator() { Hooks.resetOnEachOperator(TracingSubscriber.class.getName()); - AsyncSpanEndStrategies.getInstance().unregisterStrategy(asyncOperationEndStrategy); AsyncOperationEndStrategies.instance().unregisterStrategy(asyncOperationEndStrategy); } - private static Function, ? extends Publisher> tracingLift() { - return Operators.lift(new Lifter<>()); + private static Function, ? extends Publisher> tracingLift( + ReactorAsyncOperationEndStrategy asyncOperationEndStrategy) { + return Operators.lift(new Lifter<>(asyncOperationEndStrategy)); } public static class Lifter implements BiFunction, CoreSubscriber> { + /** Holds reference to strategy to prevent it from being collected. */ + @SuppressWarnings("FieldCanBeLocal") + private final ReactorAsyncOperationEndStrategy asyncOperationEndStrategy; + + public Lifter(ReactorAsyncOperationEndStrategy asyncOperationEndStrategy) { + this.asyncOperationEndStrategy = asyncOperationEndStrategy; + } + @Override public CoreSubscriber apply(Scannable publisher, CoreSubscriber sub) { // if Flux/Mono #just, #empty, #error diff --git a/instrumentation/rxjava/rxjava-2.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava2/RxJava2AsyncOperationEndStrategy.java b/instrumentation/rxjava/rxjava-2.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava2/RxJava2AsyncOperationEndStrategy.java index 865ad70fc7..8b91c95afc 100644 --- a/instrumentation/rxjava/rxjava-2.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava2/RxJava2AsyncOperationEndStrategy.java +++ b/instrumentation/rxjava/rxjava-2.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava2/RxJava2AsyncOperationEndStrategy.java @@ -5,13 +5,13 @@ package io.opentelemetry.instrumentation.rxjava2; +import static io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport.tryToGetResponse; + import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategy; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategy; import io.reactivex.Completable; import io.reactivex.Flowable; import io.reactivex.Maybe; @@ -22,11 +22,9 @@ import io.reactivex.functions.BiConsumer; import io.reactivex.functions.Consumer; import io.reactivex.parallel.ParallelFlowable; import java.util.concurrent.atomic.AtomicBoolean; -import org.checkerframework.checker.nullness.qual.Nullable; import org.reactivestreams.Publisher; -public final class RxJava2AsyncOperationEndStrategy - implements AsyncOperationEndStrategy, AsyncSpanEndStrategy { +public final class RxJava2AsyncOperationEndStrategy implements AsyncOperationEndStrategy { private static final AttributeKey CANCELED_ATTRIBUTE_KEY = AttributeKey.booleanKey("rxjava.canceled"); @@ -63,34 +61,14 @@ public final class RxJava2AsyncOperationEndStrategy Object asyncValue, Class responseType) { - return end( - asyncValue, + EndOnFirstNotificationConsumer notificationConsumer = new EndOnFirstNotificationConsumer(context) { @Override protected void end(Object response, Throwable error) { instrumenter.end(context, request, tryToGetResponse(responseType, response), error); } - }); - } + }; - @Override - public Object end(BaseTracer tracer, Context context, Object returnValue) { - return end( - returnValue, - new EndOnFirstNotificationConsumer(context) { - @Override - protected void end(Object response, Throwable error) { - if (error != null) { - tracer.endExceptionally(context, error); - } else { - tracer.end(context); - } - } - }); - } - - private static Object end( - Object asyncValue, EndOnFirstNotificationConsumer notificationConsumer) { if (asyncValue instanceof Completable) { return endWhenComplete((Completable) asyncValue, notificationConsumer); } else if (asyncValue instanceof Maybe) { @@ -153,14 +131,6 @@ public final class RxJava2AsyncOperationEndStrategy .doOnCancel(notificationConsumer::onCancelOrDispose); } - @Nullable - private static RESPONSE tryToGetResponse(Class responseType, Object result) { - if (responseType.isInstance(result)) { - return responseType.cast(result); - } - return null; - } - /** * Helper class to ensure that the span is ended exactly once regardless of how many OnComplete or * OnError notifications are received. Multiple notifications can happen anytime multiple 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 4f4cde6f4f..0e023f0236 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,7 +25,6 @@ package io.opentelemetry.instrumentation.rxjava2; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategies; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategies; import io.reactivex.Completable; import io.reactivex.CompletableObserver; import io.reactivex.Flowable; @@ -254,8 +253,6 @@ public final class TracingAssembly { .setCaptureExperimentalSpanAttributes(captureExperimentalSpanAttributes) .build(); - AsyncSpanEndStrategies.getInstance().registerStrategy(asyncOperationEndStrategy); - AsyncOperationEndStrategies.instance().registerStrategy(asyncOperationEndStrategy); } @@ -293,10 +290,7 @@ public final class TracingAssembly { private static void disableWithSpanStrategy() { if (asyncOperationEndStrategy != null) { - AsyncSpanEndStrategies.getInstance().unregisterStrategy(asyncOperationEndStrategy); - AsyncOperationEndStrategies.instance().unregisterStrategy(asyncOperationEndStrategy); - asyncOperationEndStrategy = null; } } diff --git a/instrumentation/rxjava/rxjava-3.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava3/RxJava3AsyncOperationEndStrategy.java b/instrumentation/rxjava/rxjava-3.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava3/RxJava3AsyncOperationEndStrategy.java index 4f8321427d..d65854451a 100644 --- a/instrumentation/rxjava/rxjava-3.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava3/RxJava3AsyncOperationEndStrategy.java +++ b/instrumentation/rxjava/rxjava-3.0/library/src/main/java/io/opentelemetry/instrumentation/rxjava3/RxJava3AsyncOperationEndStrategy.java @@ -5,13 +5,13 @@ package io.opentelemetry.instrumentation.rxjava3; +import static io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport.tryToGetResponse; + import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategy; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategy; import io.reactivex.rxjava3.core.Completable; import io.reactivex.rxjava3.core.Flowable; import io.reactivex.rxjava3.core.Maybe; @@ -22,11 +22,9 @@ import io.reactivex.rxjava3.functions.BiConsumer; import io.reactivex.rxjava3.functions.Consumer; import io.reactivex.rxjava3.parallel.ParallelFlowable; import java.util.concurrent.atomic.AtomicBoolean; -import org.checkerframework.checker.nullness.qual.Nullable; import org.reactivestreams.Publisher; -public final class RxJava3AsyncOperationEndStrategy - implements AsyncOperationEndStrategy, AsyncSpanEndStrategy { +public final class RxJava3AsyncOperationEndStrategy implements AsyncOperationEndStrategy { private static final AttributeKey CANCELED_ATTRIBUTE_KEY = AttributeKey.booleanKey("rxjava.canceled"); @@ -63,34 +61,14 @@ public final class RxJava3AsyncOperationEndStrategy Object asyncValue, Class responseType) { - return end( - asyncValue, + EndOnFirstNotificationConsumer notificationConsumer = new EndOnFirstNotificationConsumer(context) { @Override protected void end(Object response, Throwable error) { instrumenter.end(context, request, tryToGetResponse(responseType, response), error); } - }); - } + }; - @Override - public Object end(BaseTracer tracer, Context context, Object returnValue) { - return end( - returnValue, - new EndOnFirstNotificationConsumer(context) { - @Override - protected void end(Object response, Throwable error) { - if (error != null) { - tracer.endExceptionally(context, error); - } else { - tracer.end(context); - } - } - }); - } - - private static Object end( - Object asyncValue, EndOnFirstNotificationConsumer notificationConsumer) { if (asyncValue instanceof Completable) { return endWhenComplete((Completable) asyncValue, notificationConsumer); } else if (asyncValue instanceof Maybe) { @@ -153,14 +131,6 @@ public final class RxJava3AsyncOperationEndStrategy .doOnCancel(notificationConsumer::onCancelOrDispose); } - @Nullable - private static RESPONSE tryToGetResponse(Class responseType, Object result) { - if (responseType.isInstance(result)) { - return responseType.cast(result); - } - return null; - } - /** * Helper class to ensure that the span is ended exactly once regardless of how many OnComplete or * OnError notifications are received. Multiple notifications can happen anytime multiple 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 817b678574..4cf3ace47f 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,7 +25,6 @@ package io.opentelemetry.instrumentation.rxjava3; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndStrategies; -import io.opentelemetry.instrumentation.api.tracer.async.AsyncSpanEndStrategies; import io.reactivex.rxjava3.core.Completable; import io.reactivex.rxjava3.core.CompletableObserver; import io.reactivex.rxjava3.core.Flowable; @@ -252,8 +251,6 @@ public final class TracingAssembly { .setCaptureExperimentalSpanAttributes(captureExperimentalSpanAttributes) .build(); - AsyncSpanEndStrategies.getInstance().registerStrategy(asyncOperationEndStrategy); - AsyncOperationEndStrategies.instance().registerStrategy(asyncOperationEndStrategy); } @@ -291,7 +288,6 @@ public final class TracingAssembly { private static void disableWithSpanStrategy() { if (asyncOperationEndStrategy != null) { - AsyncSpanEndStrategies.getInstance().unregisterStrategy(asyncOperationEndStrategy); AsyncOperationEndStrategies.instance().unregisterStrategy(asyncOperationEndStrategy); asyncOperationEndStrategy = null; } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/asyncannotationsupport/WeakRefAsyncOperationEndStrategies.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/asyncannotationsupport/WeakRefAsyncOperationEndStrategies.java index 1cfa2b8c0c..1341ecc82a 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/asyncannotationsupport/WeakRefAsyncOperationEndStrategies.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/asyncannotationsupport/WeakRefAsyncOperationEndStrategies.java @@ -10,7 +10,6 @@ import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperat import io.opentelemetry.instrumentation.api.annotation.support.async.Jdk8AsyncOperationEndStrategy; import java.lang.ref.WeakReference; import java.util.List; -import java.util.ListIterator; import java.util.concurrent.CopyOnWriteArrayList; import org.checkerframework.checker.nullness.qual.Nullable; @@ -39,28 +38,31 @@ public final class WeakRefAsyncOperationEndStrategies extends AsyncOperationEndS @Override public void unregisterStrategy(AsyncOperationEndStrategy strategy) { - ListIterator> it = strategies.listIterator(); - while (it.hasNext()) { - AsyncOperationEndStrategy s = it.next().get(); - if (s == null || s == strategy) { - it.remove(); - break; - } - } + strategies.removeIf( + ref -> { + AsyncOperationEndStrategy s = ref.get(); + return s == null || s == strategy; + }); } @Nullable @Override public AsyncOperationEndStrategy resolveStrategy(Class returnType) { - ListIterator> it = strategies.listIterator(); - while (it.hasNext()) { - AsyncOperationEndStrategy s = it.next().get(); - if (s == null) { - it.remove(); - } else if (s.supports(returnType)) { - return s; + boolean purgeCollectedWeakReferences = false; + try { + for (WeakReference ref : strategies) { + AsyncOperationEndStrategy s = ref.get(); + if (s == null) { + purgeCollectedWeakReferences = true; + } else if (s.supports(returnType)) { + return s; + } + } + return null; + } finally { + if (purgeCollectedWeakReferences) { + strategies.removeIf(ref -> ref.get() == null); } } - return null; } }