From fd02a85a63ba37befde324bf0b9b6067ba5d8d42 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Mon, 8 Mar 2021 11:03:33 -0500 Subject: [PATCH 01/19] Add asynchronous tracing for Java 8 CompletableFuture in Spring WithSpanAspect --- .../autoconfigure/aspects/WithSpanAspect.java | 3 +- .../aspects/WithSpanAspectTracer.java | 33 +++++++++++++++++-- .../CompletableFutureMethodSpanStrategy.java | 30 +++++++++++++++++ .../CompletionStageMethodSpanStrategy.java | 30 +++++++++++++++++ .../aspects/async/MethodSpanStrategy.java | 25 ++++++++++++++ .../async/MethodSpanStrategyContextKey.java | 14 ++++++++ .../async/SynchronousMethodSpanStrategy.java | 19 +++++++++++ 7 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletableFutureMethodSpanStrategy.java create mode 100644 instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletionStageMethodSpanStrategy.java create mode 100644 instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategy.java create mode 100644 instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java create mode 100644 instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/SynchronousMethodSpanStrategy.java diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java index 89d04ee7bc97..163d5d20161a 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java @@ -47,8 +47,7 @@ public Object traceMethod(ProceedingJoinPoint pjp) throws Throwable { Context context = tracer.startSpan(parentContext, withSpan, method); try (Scope ignored = context.makeCurrent()) { Object result = pjp.proceed(); - tracer.end(context); - return result; + return tracer.end(context, result); } catch (Throwable t) { tracer.endExceptionally(context, t); throw t; diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java index 190c91bfc0a7..8adea5acabbd 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java @@ -10,7 +10,13 @@ import io.opentelemetry.context.Context; import io.opentelemetry.extension.annotations.WithSpan; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; +import io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async.CompletableFutureMethodSpanStrategy; +import io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async.CompletionStageMethodSpanStrategy; +import io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async.MethodSpanStrategy; +import io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async.SynchronousMethodSpanStrategy; import java.lang.reflect.Method; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; class WithSpanAspectTracer extends BaseTracer { WithSpanAspectTracer(OpenTelemetry openTelemetry) { @@ -22,16 +28,27 @@ protected String getInstrumentationName() { return "io.opentelemetry.spring-boot-autoconfigure-aspect"; } + public Object end(Context context, Object result) { + MethodSpanStrategy methodSpanStrategy = MethodSpanStrategy.fromContextOrNull(context); + if (methodSpanStrategy != null) { + return methodSpanStrategy.end(result, this, context); + } + end(context); + return result; + } + Context startSpan(Context parentContext, WithSpan annotation, Method method) { + Context spanStrategyContext = withMethodSpanStrategy(parentContext, method); Span span = spanBuilder(parentContext, spanName(annotation, method), annotation.kind()).startSpan(); + switch (annotation.kind()) { case SERVER: - return withServerSpan(parentContext, span); + return withServerSpan(spanStrategyContext, span); case CLIENT: - return withClientSpan(parentContext, span); + return withClientSpan(spanStrategyContext, span); default: - return parentContext.with(span); + return spanStrategyContext.with(span); } } @@ -42,4 +59,14 @@ private String spanName(WithSpan annotation, Method method) { } return spanName; } + + private Context withMethodSpanStrategy(Context parentContext, Method method) { + Class returnType = method.getReturnType(); + if (returnType == CompletionStage.class) { + parentContext.with(CompletionStageMethodSpanStrategy.INSTANCE); + } else if (returnType == CompletableFuture.class) { + parentContext.with(CompletableFutureMethodSpanStrategy.INSTANCE); + } + return parentContext.with(SynchronousMethodSpanStrategy.INSTANCE); + } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletableFutureMethodSpanStrategy.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletableFutureMethodSpanStrategy.java new file mode 100644 index 000000000000..e099359d710d --- /dev/null +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletableFutureMethodSpanStrategy.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.tracer.BaseTracer; +import java.util.concurrent.CompletableFuture; + +public enum CompletableFutureMethodSpanStrategy implements MethodSpanStrategy { + INSTANCE; + + @Override + public Object end(Object result, BaseTracer tracer, Context context) { + if (result instanceof CompletableFuture) { + CompletableFuture future = (CompletableFuture) result; + return future.whenComplete( + (value, error) -> { + if (error != null) { + tracer.endExceptionally(context, error); + } else { + tracer.end(context); + } + }); + } + return result; + } +} diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletionStageMethodSpanStrategy.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletionStageMethodSpanStrategy.java new file mode 100644 index 000000000000..064d419bfeba --- /dev/null +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletionStageMethodSpanStrategy.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.tracer.BaseTracer; +import java.util.concurrent.CompletionStage; + +public enum CompletionStageMethodSpanStrategy implements MethodSpanStrategy { + INSTANCE; + + @Override + public Object end(Object result, BaseTracer tracer, Context context) { + if (result instanceof CompletionStage) { + CompletionStage stage = (CompletionStage) result; + return stage.whenComplete( + (value, error) -> { + if (error != null) { + tracer.endExceptionally(context, error); + } else { + tracer.end(context); + } + }); + } + return result; + } +} diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategy.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategy.java new file mode 100644 index 000000000000..a74d62b931f5 --- /dev/null +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategy.java @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ImplicitContextKeyed; +import io.opentelemetry.instrumentation.api.tracer.BaseTracer; +import javax.annotation.Nullable; + +public interface MethodSpanStrategy extends ImplicitContextKeyed { + Object end(Object result, BaseTracer tracer, Context context); + + @Override + default Context storeInContext(Context context) { + return context.with(MethodSpanStrategyContextKey.KEY, this); + } + + @Nullable + static MethodSpanStrategy fromContextOrNull(Context context) { + return context.get(MethodSpanStrategyContextKey.KEY); + } +} diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java new file mode 100644 index 000000000000..a47fce30bc52 --- /dev/null +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java @@ -0,0 +1,14 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; + +import io.opentelemetry.context.ContextKey; + +final class MethodSpanStrategyContextKey { + public static ContextKey KEY = ContextKey.named("opentelemetry-spring-autoconfigure-aspects-method-span-strategy"); + + private MethodSpanStrategyContextKey() {} +} diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/SynchronousMethodSpanStrategy.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/SynchronousMethodSpanStrategy.java new file mode 100644 index 000000000000..980a4c45c723 --- /dev/null +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/SynchronousMethodSpanStrategy.java @@ -0,0 +1,19 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.tracer.BaseTracer; + +public enum SynchronousMethodSpanStrategy implements MethodSpanStrategy { + INSTANCE; + + @Override + public Object end(Object result, BaseTracer tracer, Context context) { + tracer.end(context); + return result; + } +} From ee201f352f3c8207630417a54d0a5de7bc58664c Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Mon, 8 Mar 2021 11:36:59 -0500 Subject: [PATCH 02/19] Add unit tests, fix bugs --- .../aspects/WithSpanAspectTracer.java | 11 +- .../async/MethodSpanStrategyContextKey.java | 3 +- .../aspects/WithSpanAspectTest.java | 171 ++++++++++++++++++ 3 files changed, 180 insertions(+), 5 deletions(-) diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java index 8adea5acabbd..f3a7e014d093 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java @@ -61,12 +61,15 @@ private String spanName(WithSpan annotation, Method method) { } private Context withMethodSpanStrategy(Context parentContext, Method method) { - Class returnType = method.getReturnType(); + final Class returnType = method.getReturnType(); + final MethodSpanStrategy methodSpanStrategy; if (returnType == CompletionStage.class) { - parentContext.with(CompletionStageMethodSpanStrategy.INSTANCE); + methodSpanStrategy = CompletionStageMethodSpanStrategy.INSTANCE; } else if (returnType == CompletableFuture.class) { - parentContext.with(CompletableFutureMethodSpanStrategy.INSTANCE); + methodSpanStrategy = CompletableFutureMethodSpanStrategy.INSTANCE; + } else { + methodSpanStrategy = SynchronousMethodSpanStrategy.INSTANCE; } - return parentContext.with(SynchronousMethodSpanStrategy.INSTANCE); + return parentContext.with(methodSpanStrategy); } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java index a47fce30bc52..54f6254f0149 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java @@ -8,7 +8,8 @@ import io.opentelemetry.context.ContextKey; final class MethodSpanStrategyContextKey { - public static ContextKey KEY = ContextKey.named("opentelemetry-spring-autoconfigure-aspects-method-span-strategy"); + public static ContextKey KEY = + ContextKey.named("opentelemetry-spring-autoconfigure-aspects-method-span-strategy"); private MethodSpanStrategyContextKey() {} } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java index d5d5bb1ceef4..16e6d68321ee 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java @@ -19,9 +19,14 @@ import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.data.StatusData; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.springframework.aop.aspectj.annotation.AspectJProxyFactory; @@ -63,6 +68,16 @@ public String testWithServerSpan(Runnable nestedCall) { } return "Span with name testWithServerSpan and SpanKind.SERVER was created"; } + + @WithSpan + public CompletionStage testAsyncCompletionStage(CompletionStage stage) { + return stage; + } + + @WithSpan + public CompletableFuture testAsyncCompletableFuture(CompletableFuture stage) { + return stage; + } } private WithSpanTester withSpanTester; @@ -208,4 +223,160 @@ void suppressServerSpan() throws Throwable { parentSpan -> parentSpan.hasName("WithSpanTester.testWithServerSpan").hasKind(SERVER))); } + + @Nested + @DisplayName("with a method annotated with @WithSpan returns CompletionStage") + class withCompletionStage { + + @Test + @DisplayName("should end Span on complete") + void onComplete() throws Throwable { + CompletableFuture future = new CompletableFuture<>(); + + // when + runUnderTrace( + "parent", + () -> { + withSpanTester.testAsyncCompletionStage(future); + return null; + }); + + try { + instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); + } catch (TimeoutException exception) { + // do nothing, expected + } + + future.complete("DONE"); + + // then + List> traces = instrumentation.waitForTraces(1); + assertThat(traces) + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), + span -> + span.hasName("WithSpanTester.testAsyncCompletionStage") + .hasKind(INTERNAL) + // otel SDK assertions need some work before we can comfortably use + // them in this project... + .hasParentSpanId(traces.get(0).get(0).getSpanId()))); + } + + @Test + @DisplayName("should end Span on completeException AND should record the exception") + void onCompleteExceptionally() throws Throwable { + CompletableFuture future = new CompletableFuture<>(); + + // when + runUnderTrace( + "parent", + () -> { + withSpanTester.testAsyncCompletionStage(future); + return null; + }); + + try { + instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); + } catch (TimeoutException exception) { + // do nothing, expected + } + + future.completeExceptionally(new Exception("Test @WithSpan With completeExceptionally")); + + // then + List> traces = instrumentation.waitForTraces(1); + assertThat(traces) + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), + span -> + span.hasName("WithSpanTester.testAsyncCompletionStage") + .hasKind(INTERNAL) + .hasStatus(StatusData.error()) + // otel SDK assertions need some work before we can comfortably use + // them in this project... + .hasParentSpanId(traces.get(0).get(0).getSpanId()))); + } + } + + @Nested + @DisplayName("with a method annotated with @WithSpan returns CompletableFuture") + class withCompletableFuture { + + @Test + @DisplayName("should end Span on complete") + void onComplete() throws Throwable { + CompletableFuture future = new CompletableFuture<>(); + + // when + runUnderTrace( + "parent", + () -> { + withSpanTester.testAsyncCompletableFuture(future); + return null; + }); + + try { + instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); + } catch (TimeoutException exception) { + // do nothing, expected + } + + future.complete("DONE"); + + // then + List> traces = instrumentation.waitForTraces(1); + assertThat(traces) + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), + span -> + span.hasName("WithSpanTester.testAsyncCompletableFuture") + .hasKind(INTERNAL) + // otel SDK assertions need some work before we can comfortably use + // them in this project... + .hasParentSpanId(traces.get(0).get(0).getSpanId()))); + } + + @Test + @DisplayName("should end Span on completeException AND should record the exception") + void onCompleteExceptionally() throws Throwable { + CompletableFuture future = new CompletableFuture<>(); + + // when + runUnderTrace( + "parent", + () -> { + withSpanTester.testAsyncCompletableFuture(future); + return null; + }); + + try { + instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); + } catch (TimeoutException exception) { + // do nothing, expected + } + + future.completeExceptionally(new Exception("Test @WithSpan With completeExceptionally")); + + // then + List> traces = instrumentation.waitForTraces(1); + assertThat(traces) + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), + span -> + span.hasName("WithSpanTester.testAsyncCompletableFuture") + .hasKind(INTERNAL) + .hasStatus(StatusData.error()) + // otel SDK assertions need some work before we can comfortably use + // them in this project... + .hasParentSpanId(traces.get(0).get(0).getSpanId()))); + } + } } From 6b8f4f1e91cf8bcc7b8f0be5ad870360461b2213 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Mon, 8 Mar 2021 11:55:44 -0500 Subject: [PATCH 03/19] Placate spotless --- .../spring/autoconfigure/aspects/WithSpanAspectTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java index 16e6d68321ee..42feca25a171 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java @@ -226,7 +226,7 @@ void suppressServerSpan() throws Throwable { @Nested @DisplayName("with a method annotated with @WithSpan returns CompletionStage") - class withCompletionStage { + class WithCompletionStage { @Test @DisplayName("should end Span on complete") @@ -304,7 +304,7 @@ void onCompleteExceptionally() throws Throwable { @Nested @DisplayName("with a method annotated with @WithSpan returns CompletableFuture") - class withCompletableFuture { + class WithCompletableFuture { @Test @DisplayName("should end Span on complete") From 7ac98d5c15d401ca4185875020e43932b0b4bb38 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Mon, 8 Mar 2021 14:32:52 -0500 Subject: [PATCH 04/19] Move MethodSpanStrategies to instrumentation-api, enable registration of strategies --- .../CompletableFutureMethodSpanStrategy.java | 4 +-- .../CompletionStageMethodSpanStrategy.java | 4 +-- .../tracer/async/MethodSpanStrategies.java | 33 +++++++++++++++++ .../api/tracer/async/MethodSpanStrategy.java | 36 +++++++++++++++++++ .../async/MethodSpanStrategyContextKey.java | 2 +- .../async/SynchronousMethodSpanStrategy.java | 4 +-- .../aspects/WithSpanAspectTracer.java | 26 +++----------- .../aspects/async/MethodSpanStrategy.java | 25 ------------- 8 files changed, 81 insertions(+), 53 deletions(-) rename {instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects => instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer}/async/CompletableFutureMethodSpanStrategy.java (82%) rename {instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects => instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer}/async/CompletionStageMethodSpanStrategy.java (82%) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java rename {instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects => instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer}/async/MethodSpanStrategyContextKey.java (82%) rename {instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects => instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer}/async/SynchronousMethodSpanStrategy.java (69%) delete mode 100644 instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategy.java diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletableFutureMethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java similarity index 82% rename from instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletableFutureMethodSpanStrategy.java rename to instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java index e099359d710d..7662792a4496 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletableFutureMethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java @@ -3,13 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; +package io.opentelemetry.instrumentation.api.tracer.async; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import java.util.concurrent.CompletableFuture; -public enum CompletableFutureMethodSpanStrategy implements MethodSpanStrategy { +enum CompletableFutureMethodSpanStrategy implements MethodSpanStrategy { INSTANCE; @Override diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletionStageMethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java similarity index 82% rename from instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletionStageMethodSpanStrategy.java rename to instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java index 064d419bfeba..07d4e2e3f64b 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/CompletionStageMethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java @@ -3,13 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; +package io.opentelemetry.instrumentation.api.tracer.async; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import java.util.concurrent.CompletionStage; -public enum CompletionStageMethodSpanStrategy implements MethodSpanStrategy { +enum CompletionStageMethodSpanStrategy implements MethodSpanStrategy { INSTANCE; @Override diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java new file mode 100644 index 000000000000..9b750b915dec --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.tracer.async; + +import java.lang.reflect.Method; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +public class MethodSpanStrategies { + private static final ConcurrentMap, MethodSpanStrategy> strategies = + new ConcurrentHashMap<>(); + + static { + registerStrategy(CompletionStage.class, MethodSpanStrategy.forCompletionStage()); + registerStrategy(CompletableFuture.class, MethodSpanStrategy.forCompletableFuture()); + } + + public static MethodSpanStrategy resolveStrategy(Method method) { + Class returnType = method.getReturnType(); + return strategies.getOrDefault(returnType, MethodSpanStrategy.synchronous()); + } + + public static void registerStrategy(Class returnType, MethodSpanStrategy strategy) { + strategies.put(returnType, strategy); + } + + private MethodSpanStrategies() {} +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java new file mode 100644 index 000000000000..4883a1615999 --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java @@ -0,0 +1,36 @@ +/* + * 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.context.ImplicitContextKeyed; +import io.opentelemetry.instrumentation.api.tracer.BaseTracer; + +public interface MethodSpanStrategy extends ImplicitContextKeyed { + Object end(Object result, BaseTracer tracer, Context context); + + @Override + default Context storeInContext(Context context) { + return context.with(MethodSpanStrategyContextKey.KEY, this); + } + + static MethodSpanStrategy fromContext(Context context) { + MethodSpanStrategy methodSpanStrategy = context.get(MethodSpanStrategyContextKey.KEY); + return methodSpanStrategy != null ? methodSpanStrategy : synchronous(); + } + + static MethodSpanStrategy synchronous() { + return SynchronousMethodSpanStrategy.INSTANCE; + } + + static MethodSpanStrategy forCompletionStage() { + return CompletionStageMethodSpanStrategy.INSTANCE; + } + + static MethodSpanStrategy forCompletableFuture() { + return CompletableFutureMethodSpanStrategy.INSTANCE; + } +} diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategyContextKey.java similarity index 82% rename from instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java rename to instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategyContextKey.java index 54f6254f0149..4dacbcf4a5f8 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategyContextKey.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategyContextKey.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; +package io.opentelemetry.instrumentation.api.tracer.async; import io.opentelemetry.context.ContextKey; diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/SynchronousMethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java similarity index 69% rename from instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/SynchronousMethodSpanStrategy.java rename to instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java index 980a4c45c723..641657b6d0cb 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/SynchronousMethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java @@ -3,12 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; +package io.opentelemetry.instrumentation.api.tracer.async; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -public enum SynchronousMethodSpanStrategy implements MethodSpanStrategy { +enum SynchronousMethodSpanStrategy implements MethodSpanStrategy { INSTANCE; @Override diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java index f3a7e014d093..bf88fad99fdc 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java @@ -10,13 +10,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.extension.annotations.WithSpan; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async.CompletableFutureMethodSpanStrategy; -import io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async.CompletionStageMethodSpanStrategy; -import io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async.MethodSpanStrategy; -import io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async.SynchronousMethodSpanStrategy; +import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategies; +import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategy; import java.lang.reflect.Method; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; class WithSpanAspectTracer extends BaseTracer { WithSpanAspectTracer(OpenTelemetry openTelemetry) { @@ -29,12 +25,8 @@ protected String getInstrumentationName() { } public Object end(Context context, Object result) { - MethodSpanStrategy methodSpanStrategy = MethodSpanStrategy.fromContextOrNull(context); - if (methodSpanStrategy != null) { - return methodSpanStrategy.end(result, this, context); - } - end(context); - return result; + MethodSpanStrategy methodSpanStrategy = MethodSpanStrategy.fromContext(context); + return methodSpanStrategy.end(result, this, context); } Context startSpan(Context parentContext, WithSpan annotation, Method method) { @@ -61,15 +53,7 @@ private String spanName(WithSpan annotation, Method method) { } private Context withMethodSpanStrategy(Context parentContext, Method method) { - final Class returnType = method.getReturnType(); - final MethodSpanStrategy methodSpanStrategy; - if (returnType == CompletionStage.class) { - methodSpanStrategy = CompletionStageMethodSpanStrategy.INSTANCE; - } else if (returnType == CompletableFuture.class) { - methodSpanStrategy = CompletableFutureMethodSpanStrategy.INSTANCE; - } else { - methodSpanStrategy = SynchronousMethodSpanStrategy.INSTANCE; - } + final MethodSpanStrategy methodSpanStrategy = MethodSpanStrategies.resolveStrategy(method); return parentContext.with(methodSpanStrategy); } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategy.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategy.java deleted file mode 100644 index a74d62b931f5..000000000000 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/async/MethodSpanStrategy.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.spring.autoconfigure.aspects.async; - -import io.opentelemetry.context.Context; -import io.opentelemetry.context.ImplicitContextKeyed; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import javax.annotation.Nullable; - -public interface MethodSpanStrategy extends ImplicitContextKeyed { - Object end(Object result, BaseTracer tracer, Context context); - - @Override - default Context storeInContext(Context context) { - return context.with(MethodSpanStrategyContextKey.KEY, this); - } - - @Nullable - static MethodSpanStrategy fromContextOrNull(Context context) { - return context.get(MethodSpanStrategyContextKey.KEY); - } -} From 44a019a806fd47f456b468fac18d3f3231661960 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Mon, 8 Mar 2021 15:06:12 -0500 Subject: [PATCH 05/19] Switch to withSpan --- .../aspects/WithSpanAspectTest.java | 36 +++---------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java index db460f9a355e..25a3df8dd4b8 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java @@ -213,12 +213,7 @@ void onComplete() throws Throwable { CompletableFuture future = new CompletableFuture<>(); // when - runUnderTrace( - "parent", - () -> { - withSpanTester.testAsyncCompletionStage(future); - return null; - }); + withSpan("parent", () -> withSpanTester.testAsyncCompletionStage(future)); try { instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); @@ -238,8 +233,6 @@ void onComplete() throws Throwable { span -> span.hasName("WithSpanTester.testAsyncCompletionStage") .hasKind(INTERNAL) - // otel SDK assertions need some work before we can comfortably use - // them in this project... .hasParentSpanId(traces.get(0).get(0).getSpanId()))); } @@ -249,12 +242,7 @@ void onCompleteExceptionally() throws Throwable { CompletableFuture future = new CompletableFuture<>(); // when - runUnderTrace( - "parent", - () -> { - withSpanTester.testAsyncCompletionStage(future); - return null; - }); + withSpan("parent", () -> withSpanTester.testAsyncCompletionStage(future)); try { instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); @@ -275,8 +263,6 @@ void onCompleteExceptionally() throws Throwable { span.hasName("WithSpanTester.testAsyncCompletionStage") .hasKind(INTERNAL) .hasStatus(StatusData.error()) - // otel SDK assertions need some work before we can comfortably use - // them in this project... .hasParentSpanId(traces.get(0).get(0).getSpanId()))); } } @@ -291,12 +277,7 @@ void onComplete() throws Throwable { CompletableFuture future = new CompletableFuture<>(); // when - runUnderTrace( - "parent", - () -> { - withSpanTester.testAsyncCompletableFuture(future); - return null; - }); + withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); try { instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); @@ -316,8 +297,6 @@ void onComplete() throws Throwable { span -> span.hasName("WithSpanTester.testAsyncCompletableFuture") .hasKind(INTERNAL) - // otel SDK assertions need some work before we can comfortably use - // them in this project... .hasParentSpanId(traces.get(0).get(0).getSpanId()))); } @@ -327,12 +306,7 @@ void onCompleteExceptionally() throws Throwable { CompletableFuture future = new CompletableFuture<>(); // when - runUnderTrace( - "parent", - () -> { - withSpanTester.testAsyncCompletableFuture(future); - return null; - }); + withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); try { instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); @@ -353,8 +327,6 @@ void onCompleteExceptionally() throws Throwable { span.hasName("WithSpanTester.testAsyncCompletableFuture") .hasKind(INTERNAL) .hasStatus(StatusData.error()) - // otel SDK assertions need some work before we can comfortably use - // them in this project... .hasParentSpanId(traces.get(0).get(0).getSpanId()))); } } From 0f5a370650a79ee459556772fa42d4524318bc89 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 9 Mar 2021 09:42:01 -0500 Subject: [PATCH 06/19] Fix unit test, refactor MethodSpanStrategy interface --- .../CompletableFutureMethodSpanStrategy.java | 2 +- .../CompletionStageMethodSpanStrategy.java | 2 +- .../api/tracer/async/MethodSpanStrategy.java | 10 ++-- .../async/MethodSpanStrategyContextKey.java | 15 ------ .../async/SynchronousMethodSpanStrategy.java | 2 +- .../aspects/WithSpanAspectTracer.java | 2 +- .../aspects/WithSpanAspectTest.java | 54 +++++++++++-------- 7 files changed, 43 insertions(+), 44 deletions(-) delete mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategyContextKey.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java index 7662792a4496..fe9162d2abaa 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java @@ -13,7 +13,7 @@ enum CompletableFutureMethodSpanStrategy implements MethodSpanStrategy { INSTANCE; @Override - public Object end(Object result, BaseTracer tracer, Context context) { + public Object end(BaseTracer tracer, Context context, Object result) { if (result instanceof CompletableFuture) { CompletableFuture future = (CompletableFuture) result; return future.whenComplete( diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java index 07d4e2e3f64b..7371b981bc6b 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java @@ -13,7 +13,7 @@ enum CompletionStageMethodSpanStrategy implements MethodSpanStrategy { INSTANCE; @Override - public Object end(Object result, BaseTracer tracer, Context context) { + public Object end(BaseTracer tracer, Context context, Object result) { if (result instanceof CompletionStage) { CompletionStage stage = (CompletionStage) result; return stage.whenComplete( diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java index 4883a1615999..0afa552bf904 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java @@ -6,19 +6,23 @@ package io.opentelemetry.instrumentation.api.tracer.async; import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; import io.opentelemetry.context.ImplicitContextKeyed; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; public interface MethodSpanStrategy extends ImplicitContextKeyed { - Object end(Object result, BaseTracer tracer, Context context); + ContextKey CONTEXT_KEY = + ContextKey.named("opentelemetry-spring-autoconfigure-aspects-method-span-strategy"); + + Object end(BaseTracer tracer, Context context, Object result); @Override default Context storeInContext(Context context) { - return context.with(MethodSpanStrategyContextKey.KEY, this); + return context.with(CONTEXT_KEY, this); } static MethodSpanStrategy fromContext(Context context) { - MethodSpanStrategy methodSpanStrategy = context.get(MethodSpanStrategyContextKey.KEY); + MethodSpanStrategy methodSpanStrategy = context.get(CONTEXT_KEY); return methodSpanStrategy != null ? methodSpanStrategy : synchronous(); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategyContextKey.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategyContextKey.java deleted file mode 100644 index 4dacbcf4a5f8..000000000000 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategyContextKey.java +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.tracer.async; - -import io.opentelemetry.context.ContextKey; - -final class MethodSpanStrategyContextKey { - public static ContextKey KEY = - ContextKey.named("opentelemetry-spring-autoconfigure-aspects-method-span-strategy"); - - private MethodSpanStrategyContextKey() {} -} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java index 641657b6d0cb..0aa18e1e5644 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java @@ -12,7 +12,7 @@ enum SynchronousMethodSpanStrategy implements MethodSpanStrategy { INSTANCE; @Override - public Object end(Object result, BaseTracer tracer, Context context) { + public Object end(BaseTracer tracer, Context context, Object result) { tracer.end(context); return result; } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java index bf88fad99fdc..ebd6cdf34f27 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java @@ -26,7 +26,7 @@ protected String getInstrumentationName() { public Object end(Context context, Object result) { MethodSpanStrategy methodSpanStrategy = MethodSpanStrategy.fromContext(context); - return methodSpanStrategy.end(result, this, context); + return methodSpanStrategy.end(this, context, result); } Context startSpan(Context parentContext, WithSpan annotation, Method method) { diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java index 25a3df8dd4b8..55ad3ce400f3 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java @@ -23,8 +23,6 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -215,12 +213,15 @@ void onComplete() throws Throwable { // when withSpan("parent", () -> withSpanTester.testAsyncCompletionStage(future)); - try { - instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); - } catch (TimeoutException exception) { - // do nothing, expected - } + // then + assertThat(instrumentation.waitForTraces(1)) + .hasTracesSatisfyingExactly( + trace -> + trace + .hasSize(1) + .hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL))); + // when future.complete("DONE"); // then @@ -244,12 +245,15 @@ void onCompleteExceptionally() throws Throwable { // when withSpan("parent", () -> withSpanTester.testAsyncCompletionStage(future)); - try { - instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); - } catch (TimeoutException exception) { - // do nothing, expected - } + // then + assertThat(instrumentation.waitForTraces(1)) + .hasTracesSatisfyingExactly( + trace -> + trace + .hasSize(1) + .hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL))); + // when future.completeExceptionally(new Exception("Test @WithSpan With completeExceptionally")); // then @@ -279,12 +283,15 @@ void onComplete() throws Throwable { // when withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); - try { - instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); - } catch (TimeoutException exception) { - // do nothing, expected - } + // then + assertThat(instrumentation.waitForTraces(1)) + .hasTracesSatisfyingExactly( + trace -> + trace + .hasSize(1) + .hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL))); + // when future.complete("DONE"); // then @@ -308,12 +315,15 @@ void onCompleteExceptionally() throws Throwable { // when withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); - try { - instrumentation.waitForTraces(1, 20, TimeUnit.SECONDS); - } catch (TimeoutException exception) { - // do nothing, expected - } + // then + assertThat(instrumentation.waitForTraces(1)) + .hasTracesSatisfyingExactly( + trace -> + trace + .hasSize(1) + .hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL))); + // when future.completeExceptionally(new Exception("Test @WithSpan With completeExceptionally")); // then From ef099f58daa2593e3935baca7a1bfa3d64e51242 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 9 Mar 2021 10:17:27 -0500 Subject: [PATCH 07/19] Refactor to instrumentation-api, add docs --- .../api/tracer/BaseMethodTracer.java | 39 +++++++++++++++++++ .../tracer/async/MethodSpanStrategies.java | 8 +++- .../api/tracer/async/MethodSpanStrategy.java | 19 +++++++++ .../api/tracer/async/package-info.java | 5 +++ .../aspects/WithSpanAspectTracer.java | 16 +------- 5 files changed, 72 insertions(+), 15 deletions(-) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java new file mode 100644 index 000000000000..0fafbe689adf --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java @@ -0,0 +1,39 @@ +package io.opentelemetry.instrumentation.api.tracer; + +import java.lang.reflect.Method; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategies; +import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategy; + +/** + * Base class for instrumentation specific tracer implementations that trace the invocation of a method. + */ +public abstract class BaseMethodTracer extends BaseTracer { + + public BaseMethodTracer() { + } + + public BaseMethodTracer(OpenTelemetry openTelemetry) { + super(openTelemetry); + } + + /** + * Resolves the {@link MethodSpanStrategy} for tracing the specified {@code method} and stores that strategy in the returned {@code Context}. + */ + protected Context withMethodSpanStrategy(Context context, Method method) { + MethodSpanStrategy methodSpanStrategy = MethodSpanStrategies.resolveStrategy(method); + return methodSpanStrategy.storeInContext(context); + } + + /** + * 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 remain open until the asynchronous operation has completed. + * + * @param result Return value from the traced method. + * @return Either {@code result} or a value composing over {@code result} for notification of completion. + */ + public Object end(Context context, Object result) { + return MethodSpanStrategy.fromContext(context) + .end(this, context, result); + } +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java index 9b750b915dec..a8abdb388821 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java @@ -11,6 +11,9 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +/** + * Registry of {@link MethodSpanStrategy} implementations for tracing the asynchronous operations represented by the return type of a traced method. + */ public class MethodSpanStrategies { private static final ConcurrentMap, MethodSpanStrategy> strategies = new ConcurrentHashMap<>(); @@ -21,7 +24,10 @@ public class MethodSpanStrategies { } public static MethodSpanStrategy resolveStrategy(Method method) { - Class returnType = method.getReturnType(); + return resolveStrategy(method.getReturnType()); + } + + public static MethodSpanStrategy resolveStrategy(Class returnType) { return strategies.getOrDefault(returnType, MethodSpanStrategy.synchronous()); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java index 0afa552bf904..821cbb03d0d2 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java @@ -10,10 +10,20 @@ import io.opentelemetry.context.ImplicitContextKeyed; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; +/** + * Represents an implementation of a strategy for composing over the return value of a traced method. If the return value represents the result of an asynchronous operation the implementation can compose or register for notification of completion at which point the span representing the invocation of the method will be ended. + */ public interface MethodSpanStrategy extends ImplicitContextKeyed { ContextKey CONTEXT_KEY = ContextKey.named("opentelemetry-spring-autoconfigure-aspects-method-span-strategy"); + /** + * 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 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 result Return value of the traced method. + * @return Either {@code result} or a value composing over {@code result} for notification of completion. + */ Object end(BaseTracer tracer, Context context, Object result); @Override @@ -26,14 +36,23 @@ static MethodSpanStrategy fromContext(Context context) { return methodSpanStrategy != null ? methodSpanStrategy : synchronous(); } + /** + * @return A {@link MethodSpanStrategy} for tracing synchronous methods where the return value does not represent the completion of an asynchronous operation. + */ static MethodSpanStrategy synchronous() { return SynchronousMethodSpanStrategy.INSTANCE; } + /** + * @return a {@link MethodSpanStrategy} for tracing a method that returns a {@link java.util.concurrent.CompletionStage} representing the completion of an asynchronous operation. + */ static MethodSpanStrategy forCompletionStage() { return CompletionStageMethodSpanStrategy.INSTANCE; } + /** + * @return a {@link MethodSpanStrategy} for tracing a method that returns a {@link java.util.concurrent.CompletableFuture} representing the completion of an asynchronous operation. + */ static MethodSpanStrategy forCompletableFuture() { return CompletableFutureMethodSpanStrategy.INSTANCE; } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java new file mode 100644 index 000000000000..e48560a54630 --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java @@ -0,0 +1,5 @@ +/** + * 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. + */ +package io.opentelemetry.instrumentation.api.tracer.async; diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java index ebd6cdf34f27..202b0c32ed84 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java @@ -9,12 +9,10 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.extension.annotations.WithSpan; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategies; -import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategy; +import io.opentelemetry.instrumentation.api.tracer.BaseMethodTracer; import java.lang.reflect.Method; -class WithSpanAspectTracer extends BaseTracer { +class WithSpanAspectTracer extends BaseMethodTracer { WithSpanAspectTracer(OpenTelemetry openTelemetry) { super(openTelemetry); } @@ -24,11 +22,6 @@ protected String getInstrumentationName() { return "io.opentelemetry.spring-boot-autoconfigure-aspect"; } - public Object end(Context context, Object result) { - MethodSpanStrategy methodSpanStrategy = MethodSpanStrategy.fromContext(context); - return methodSpanStrategy.end(this, context, result); - } - Context startSpan(Context parentContext, WithSpan annotation, Method method) { Context spanStrategyContext = withMethodSpanStrategy(parentContext, method); Span span = @@ -51,9 +44,4 @@ private String spanName(WithSpan annotation, Method method) { } return spanName; } - - private Context withMethodSpanStrategy(Context parentContext, Method method) { - final MethodSpanStrategy methodSpanStrategy = MethodSpanStrategies.resolveStrategy(method); - return parentContext.with(methodSpanStrategy); - } } From b465fbdc1e6679d5374e106d477f6c1e427e2dfd Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 9 Mar 2021 11:45:19 -0500 Subject: [PATCH 08/19] spotless, handle null and already done scenarios --- .../api/tracer/BaseMethodTracer.java | 27 ++++-- .../CompletableFutureMethodSpanStrategy.java | 37 +++++++-- .../CompletionStageMethodSpanStrategy.java | 1 + .../tracer/async/MethodSpanStrategies.java | 3 +- .../api/tracer/async/MethodSpanStrategy.java | 27 ++++-- .../aspects/WithSpanAspectTest.java | 82 +++++++++++++++++++ 6 files changed, 152 insertions(+), 25 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java index 0fafbe689adf..74a7e40bf94f 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java @@ -1,25 +1,31 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.instrumentation.api.tracer; -import java.lang.reflect.Method; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategies; import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategy; +import java.lang.reflect.Method; /** - * Base class for instrumentation specific tracer implementations that trace the invocation of a method. + * Base class for instrumentation specific tracer implementations that trace the invocation of a + * method. */ public abstract class BaseMethodTracer extends BaseTracer { - public BaseMethodTracer() { - } + public BaseMethodTracer() {} public BaseMethodTracer(OpenTelemetry openTelemetry) { super(openTelemetry); } /** - * Resolves the {@link MethodSpanStrategy} for tracing the specified {@code method} and stores that strategy in the returned {@code Context}. + * Resolves the {@link MethodSpanStrategy} for tracing the specified {@code method} and stores + * that strategy in the returned {@code Context}. */ protected Context withMethodSpanStrategy(Context context, Method method) { MethodSpanStrategy methodSpanStrategy = MethodSpanStrategies.resolveStrategy(method); @@ -27,13 +33,16 @@ protected Context withMethodSpanStrategy(Context context, Method method) { } /** - * 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 remain open until the asynchronous operation has completed. + * 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 remain open until the asynchronous operation has + * completed. * * @param result Return value from the traced method. - * @return Either {@code result} or a value composing over {@code result} for notification of completion. + * @return Either {@code result} or a value composing over {@code result} for notification of + * completion. */ public Object end(Context context, Object result) { - return MethodSpanStrategy.fromContext(context) - .end(this, context, result); + return MethodSpanStrategy.fromContext(context).end(this, context, result); } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java index fe9162d2abaa..e44120736e5c 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java @@ -16,15 +16,36 @@ enum CompletableFutureMethodSpanStrategy implements MethodSpanStrategy { public Object end(BaseTracer tracer, Context context, Object result) { if (result instanceof CompletableFuture) { CompletableFuture future = (CompletableFuture) result; - return future.whenComplete( - (value, error) -> { - if (error != null) { - tracer.endExceptionally(context, error); - } else { - tracer.end(context); - } - }); + if (future.isDone()) { + return endSynchronously(tracer, context, future); + } else { + return endOnCompletion(tracer, context, future); + } } + tracer.end(context); return result; } + + private CompletableFuture endSynchronously( + BaseTracer tracer, Context context, CompletableFuture future) { + try { + future.join(); + tracer.end(context); + } catch (Exception exception) { + tracer.endExceptionally(context, exception); + } + return future; + } + + private CompletableFuture endOnCompletion( + BaseTracer tracer, Context context, CompletableFuture future) { + return future.whenComplete( + (value, error) -> { + if (error != null) { + tracer.endExceptionally(context, error); + } else { + tracer.end(context); + } + }); + } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java index 7371b981bc6b..6e7214fc24f7 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java @@ -25,6 +25,7 @@ public Object end(BaseTracer tracer, Context context, Object result) { } }); } + tracer.end(context); return result; } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java index a8abdb388821..3f54bc6b9575 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java @@ -12,7 +12,8 @@ import java.util.concurrent.ConcurrentMap; /** - * Registry of {@link MethodSpanStrategy} implementations for tracing the asynchronous operations represented by the return type of a traced method. + * Registry of {@link MethodSpanStrategy} implementations for tracing the asynchronous operations + * represented by the return type of a traced method. */ public class MethodSpanStrategies { private static final ConcurrentMap, MethodSpanStrategy> strategies = diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java index 821cbb03d0d2..cf52b04c31a1 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java @@ -11,18 +11,26 @@ import io.opentelemetry.instrumentation.api.tracer.BaseTracer; /** - * Represents an implementation of a strategy for composing over the return value of a traced method. If the return value represents the result of an asynchronous operation the implementation can compose or register for notification of completion at which point the span representing the invocation of the method will be ended. + * Represents an implementation of a strategy for composing over the return value of a traced + * method. If the return value represents the result of an asynchronous operation the implementation + * can compose or register for notification of completion at which point the span representing the + * invocation of the method will be ended. */ public interface MethodSpanStrategy extends ImplicitContextKeyed { ContextKey CONTEXT_KEY = ContextKey.named("opentelemetry-spring-autoconfigure-aspects-method-span-strategy"); /** - * 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 remain open until the asynchronous operation has completed. + * 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 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 tracer {@link BaseTracer} tracer to be used to end the span stored in the {@code + * context}. * @param result Return value of the traced method. - * @return Either {@code result} or a value composing over {@code result} for notification of completion. + * @return Either {@code result} or a value composing over {@code result} for notification of + * completion. */ Object end(BaseTracer tracer, Context context, Object result); @@ -37,21 +45,26 @@ static MethodSpanStrategy fromContext(Context context) { } /** - * @return A {@link MethodSpanStrategy} for tracing synchronous methods where the return value does not represent the completion of an asynchronous operation. + * @return A {@link MethodSpanStrategy} for tracing synchronous methods where the return value + * does not represent the completion of an asynchronous operation. */ static MethodSpanStrategy synchronous() { return SynchronousMethodSpanStrategy.INSTANCE; } /** - * @return a {@link MethodSpanStrategy} for tracing a method that returns a {@link java.util.concurrent.CompletionStage} representing the completion of an asynchronous operation. + * @return a {@link MethodSpanStrategy} for tracing a method that returns a {@link + * java.util.concurrent.CompletionStage} representing the completion of an asynchronous + * operation. */ static MethodSpanStrategy forCompletionStage() { return CompletionStageMethodSpanStrategy.INSTANCE; } /** - * @return a {@link MethodSpanStrategy} for tracing a method that returns a {@link java.util.concurrent.CompletableFuture} representing the completion of an asynchronous operation. + * @return a {@link MethodSpanStrategy} for tracing a method that returns a {@link + * java.util.concurrent.CompletableFuture} representing the completion of an asynchronous + * operation. */ static MethodSpanStrategy forCompletableFuture() { return CompletableFutureMethodSpanStrategy.INSTANCE; diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java index 55ad3ce400f3..54f780e16e86 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java @@ -269,6 +269,25 @@ void onCompleteExceptionally() throws Throwable { .hasStatus(StatusData.error()) .hasParentSpanId(traces.get(0).get(0).getSpanId()))); } + + @Test + @DisplayName("should end Span on incompatible return value") + void onIncompatibleReturnValue() throws Throwable { + // when + withSpan("parent", () -> withSpanTester.testAsyncCompletionStage(null)); + + // then + List> traces = instrumentation.waitForTraces(1); + assertThat(traces) + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), + span -> + span.hasName("WithSpanTester.testAsyncCompletionStage") + .hasKind(INTERNAL) + .hasParentSpanId(traces.get(0).get(0).getSpanId()))); + } } @Nested @@ -339,5 +358,68 @@ void onCompleteExceptionally() throws Throwable { .hasStatus(StatusData.error()) .hasParentSpanId(traces.get(0).get(0).getSpanId()))); } + + @Test + @DisplayName("should end the Span when already complete") + void onCompletedFuture() throws Throwable { + CompletableFuture future = CompletableFuture.completedFuture("Done"); + + // when + withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); + + // then + List> traces = instrumentation.waitForTraces(1); + assertThat(traces) + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), + span -> + span.hasName("WithSpanTester.testAsyncCompletableFuture") + .hasKind(INTERNAL) + .hasParentSpanId(traces.get(0).get(0).getSpanId()))); + } + + @Test + @DisplayName("should end the Span when already failed") + void onFailedFuture() throws Throwable { + CompletableFuture future = new CompletableFuture<>(); + future.completeExceptionally(new Exception("Test @WithSpan With completeExceptionally")); + + // when + withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); + + // then + List> traces = instrumentation.waitForTraces(1); + assertThat(traces) + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), + span -> + span.hasName("WithSpanTester.testAsyncCompletableFuture") + .hasKind(INTERNAL) + .hasStatus(StatusData.error()) + .hasParentSpanId(traces.get(0).get(0).getSpanId()))); + } + + @Test + @DisplayName("should end Span on incompatible return value") + void onIncompatibleReturnValue() throws Throwable { + // when + withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(null)); + + // then + List> traces = instrumentation.waitForTraces(1); + assertThat(traces) + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), + span -> + span.hasName("WithSpanTester.testAsyncCompletableFuture") + .hasKind(INTERNAL) + .hasParentSpanId(traces.get(0).get(0).getSpanId()))); + } } } From 7c7d7f1acc00c3ae33d3725e6b47cac89a903a1f Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 9 Mar 2021 14:24:35 -0500 Subject: [PATCH 09/19] Minor refactorings, add unit tests --- .../api/tracer/BaseMethodTracer.java | 20 +++- .../tracer/async/MethodSpanStrategies.java | 30 +++--- .../api/tracer/BaseMethodTracerTest.groovy | 56 +++++++++++ ...letableFutureMethodSpanStrategyTest.groovy | 96 +++++++++++++++++++ ...mpletionStageMethodSpanStrategyTest.groovy | 96 +++++++++++++++++++ .../SynchronousMethodSpanStrategyTest.groovy | 39 ++++++++ 6 files changed, 321 insertions(+), 16 deletions(-) create mode 100644 instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy create mode 100644 instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategyTest.groovy create mode 100644 instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategyTest.groovy create mode 100644 instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategyTest.groovy diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java index 74a7e40bf94f..df58d9ea11d1 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java @@ -17,10 +17,23 @@ */ public abstract class BaseMethodTracer extends BaseTracer { - public BaseMethodTracer() {} + private final MethodSpanStrategies methodSpanStrategies; + + public BaseMethodTracer() { + this(MethodSpanStrategies.getInstance()); + } + + public BaseMethodTracer(MethodSpanStrategies methodSpanStrategies) { + this.methodSpanStrategies = methodSpanStrategies; + } public BaseMethodTracer(OpenTelemetry openTelemetry) { + this(openTelemetry, MethodSpanStrategies.getInstance()); + } + + public BaseMethodTracer(OpenTelemetry openTelemetry, MethodSpanStrategies methodSpanStrategies) { super(openTelemetry); + this.methodSpanStrategies = methodSpanStrategies; } /** @@ -28,8 +41,9 @@ public BaseMethodTracer(OpenTelemetry openTelemetry) { * that strategy in the returned {@code Context}. */ protected Context withMethodSpanStrategy(Context context, Method method) { - MethodSpanStrategy methodSpanStrategy = MethodSpanStrategies.resolveStrategy(method); - return methodSpanStrategy.storeInContext(context); + Class returnType = method.getReturnType(); + MethodSpanStrategy methodSpanStrategy = methodSpanStrategies.resolveStrategy(returnType); + return context.with(methodSpanStrategy); } /** diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java index 3f54bc6b9575..79518a48c99d 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java @@ -5,36 +5,40 @@ package io.opentelemetry.instrumentation.api.tracer.async; -import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; /** * Registry of {@link MethodSpanStrategy} implementations for tracing the asynchronous operations * represented by the return type of a traced method. */ public class MethodSpanStrategies { - private static final ConcurrentMap, MethodSpanStrategy> strategies = - new ConcurrentHashMap<>(); + private static final MethodSpanStrategies instance; static { - registerStrategy(CompletionStage.class, MethodSpanStrategy.forCompletionStage()); - registerStrategy(CompletableFuture.class, MethodSpanStrategy.forCompletableFuture()); + Map, MethodSpanStrategy> strategies = new HashMap<>(); + strategies.put(CompletionStage.class, MethodSpanStrategy.forCompletionStage()); + strategies.put(CompletableFuture.class, MethodSpanStrategy.forCompletableFuture()); + instance = new MethodSpanStrategies(strategies); } - public static MethodSpanStrategy resolveStrategy(Method method) { - return resolveStrategy(method.getReturnType()); + public static MethodSpanStrategies getInstance() { + return instance; } - public static MethodSpanStrategy resolveStrategy(Class returnType) { - return strategies.getOrDefault(returnType, MethodSpanStrategy.synchronous()); + private final Map, MethodSpanStrategy> strategies; + + private MethodSpanStrategies(Map, MethodSpanStrategy> strategies) { + this.strategies = strategies; } - public static void registerStrategy(Class returnType, MethodSpanStrategy strategy) { + public void registerStrategy(Class returnType, MethodSpanStrategy strategy) { strategies.put(returnType, strategy); } - private MethodSpanStrategies() {} + public MethodSpanStrategy resolveStrategy(Class returnType) { + return strategies.getOrDefault(returnType, MethodSpanStrategy.synchronous()); + } } diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy new file mode 100644 index 000000000000..2b1fb12e58c8 --- /dev/null +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy @@ -0,0 +1,56 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.tracer + +import io.opentelemetry.context.Context +import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategy +import spock.lang.Specification + +import java.util.concurrent.CompletableFuture + +class BaseMethodTracerTest extends Specification { + + BaseMethodTracer tracer + + void setup() { + tracer = new BaseMethodTracer() { + @Override + protected String getInstrumentationName() { + return "BaseMethodTracerTest" + } + } + } + + def "resolves method strategy and stores in returned Context"() { + given: + def strategy = MethodSpanStrategy.forCompletableFuture() + def method = TestMethods.class.getMethod("completableFuture") + + when: + def context = tracer.withMethodSpanStrategy(Context.root(), method) + + then: + context.get(MethodSpanStrategy.CONTEXT_KEY) == strategy + } + + def "ends span through method strategy in Context"() { + given: + MethodSpanStrategy strategy = Mock() + def context = Context.root().with(MethodSpanStrategy.CONTEXT_KEY, strategy) + + when: + tracer.end(context, "Result") + + then: + 1 * strategy.end(tracer, context, "Result") + } + + class TestMethods { + CompletableFuture completableFuture() { + return CompletableFuture.completedFuture("Value") + } + } +} diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategyTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategyTest.groovy new file mode 100644 index 000000000000..1a2e85707810 --- /dev/null +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategyTest.groovy @@ -0,0 +1,96 @@ +/* + * 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 CompletableFutureMethodSpanStrategyTest extends Specification { + BaseTracer tracer + + Context context + + def underTest = CompletableFutureMethodSpanStrategy.INSTANCE + + void setup() { + tracer = Mock() + context = Mock() + } + + def "ends span on incompatible result"() { + when: + underTest.end(tracer, context, "Incompatible") + + then: + 1 * tracer.end(context) + } + + def "ends span on null result"() { + when: + underTest.end(tracer, context, null) + + then: + 1 * tracer.end(context) + } + + 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/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategyTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategyTest.groovy new file mode 100644 index 000000000000..8e94e271ea8f --- /dev/null +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategyTest.groovy @@ -0,0 +1,96 @@ +/* + * 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 CompletionStageMethodSpanStrategyTest extends Specification { + BaseTracer tracer + + Context context + + def underTest = CompletionStageMethodSpanStrategy.INSTANCE + + void setup() { + tracer = Mock() + context = Mock() + } + + def "ends span on incompatible result"() { + when: + underTest.end(tracer, context, "Incompatible") + + then: + 1 * tracer.end(context) + } + + def "ends span on null result"() { + when: + underTest.end(tracer, context, null) + + then: + 1 * tracer.end(context) + } + + 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/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategyTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategyTest.groovy new file mode 100644 index 000000000000..b27271960cc5 --- /dev/null +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategyTest.groovy @@ -0,0 +1,39 @@ +/* + * 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 + +class SynchronousMethodSpanStrategyTest extends Specification { + BaseTracer tracer + + Context context + + def underTest = SynchronousMethodSpanStrategy.INSTANCE + + void setup() { + tracer = Mock() + context = Mock() + } + + def "ends span on any result"() { + when: + underTest.end(tracer, context, "any result") + + then: + 1 * tracer.end(context) + } + + def "ends span on null result"() { + when: + underTest.end(tracer, context, null) + + then: + 1 * tracer.end(context) + } +} From cbe3ec62d60383055e44156e623f6ac52e712cb8 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 9 Mar 2021 15:40:31 -0500 Subject: [PATCH 10/19] Placate checkstyle+spotless --- .../api/tracer/async/MethodSpanStrategy.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java index cf52b04c31a1..92acd1dc5141 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java @@ -45,26 +45,25 @@ static MethodSpanStrategy fromContext(Context context) { } /** - * @return A {@link MethodSpanStrategy} for tracing synchronous methods where the return value - * does not represent the completion of an asynchronous operation. + * Returns a {@link MethodSpanStrategy} for tracing synchronous methods where the return value + * does not represent the completion of an asynchronous operation. */ static MethodSpanStrategy synchronous() { return SynchronousMethodSpanStrategy.INSTANCE; } /** - * @return a {@link MethodSpanStrategy} for tracing a method that returns a {@link - * java.util.concurrent.CompletionStage} representing the completion of an asynchronous - * operation. + * Returns a {@link MethodSpanStrategy} for tracing a method that returns a {@link + * java.util.concurrent.CompletionStage} representing the completion of an asynchronous operation. */ static MethodSpanStrategy forCompletionStage() { return CompletionStageMethodSpanStrategy.INSTANCE; } /** - * @return a {@link MethodSpanStrategy} for tracing a method that returns a {@link - * java.util.concurrent.CompletableFuture} representing the completion of an asynchronous - * operation. + * Returns a {@link MethodSpanStrategy} for tracing a method that returns a {@link + * java.util.concurrent.CompletableFuture} representing the completion of an asynchronous + * operation. */ static MethodSpanStrategy forCompletableFuture() { return CompletableFutureMethodSpanStrategy.INSTANCE; From 19781607a58b60b74f740fdddc1014bec59eee57 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 9 Mar 2021 16:26:51 -0500 Subject: [PATCH 11/19] placate codeNarc --- .../instrumentation/api/tracer/BaseMethodTracerTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy index 2b1fb12e58c8..2a23c5876ba0 100644 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy @@ -27,7 +27,7 @@ class BaseMethodTracerTest extends Specification { def "resolves method strategy and stores in returned Context"() { given: def strategy = MethodSpanStrategy.forCompletableFuture() - def method = TestMethods.class.getMethod("completableFuture") + def method = TestMethods.getMethod("completableFuture") when: def context = tracer.withMethodSpanStrategy(Context.root(), method) From 850f3a8e197de1cb48b4f8206ba7d6e1027a3885 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 9 Mar 2021 23:17:25 -0500 Subject: [PATCH 12/19] Isolate changes to only otelannotations instrumentation --- .../api/tracer/BaseMethodTracer.java | 62 ----- .../api/tracer/BaseMethodTracerTest.groovy | 56 ----- .../otelannotations/WithSpanAdvice.java | 4 +- .../otelannotations/WithSpanTracer.java | 36 ++- .../CompletableFutureMethodSpanStrategy.java | 2 +- .../CompletionStageMethodSpanStrategy.java | 2 +- .../async/MethodSpanStrategies.java | 2 +- .../async/MethodSpanStrategy.java | 2 +- .../async/SynchronousMethodSpanStrategy.java | 2 +- .../otelannotations}/async/package-info.java | 2 +- .../groovy/WithSpanInstrumentationTest.groovy | 100 ++++++++ ...letableFutureMethodSpanStrategyTest.groovy | 2 +- ...mpletionStageMethodSpanStrategyTest.groovy | 2 +- .../SynchronousMethodSpanStrategyTest.groovy | 2 +- .../test/annotation/TracedWithSpan.java | 12 + .../autoconfigure/aspects/WithSpanAspect.java | 3 +- .../aspects/WithSpanAspectTracer.java | 12 +- .../aspects/WithSpanAspectTest.java | 235 ------------------ 18 files changed, 164 insertions(+), 374 deletions(-) delete mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java delete mode 100644 instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy rename {instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/CompletableFutureMethodSpanStrategy.java (94%) rename {instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/CompletionStageMethodSpanStrategy.java (91%) rename {instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/MethodSpanStrategies.java (94%) rename {instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/MethodSpanStrategy.java (97%) rename {instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/SynchronousMethodSpanStrategy.java (84%) rename {instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/package-info.java (72%) rename {instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/CompletableFutureMethodSpanStrategyTest.groovy (96%) rename {instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/CompletionStageMethodSpanStrategyTest.groovy (96%) rename {instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer => instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations}/async/SynchronousMethodSpanStrategyTest.groovy (90%) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java deleted file mode 100644 index df58d9ea11d1..000000000000 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracer.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.tracer; - -import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategies; -import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategy; -import java.lang.reflect.Method; - -/** - * Base class for instrumentation specific tracer implementations that trace the invocation of a - * method. - */ -public abstract class BaseMethodTracer extends BaseTracer { - - private final MethodSpanStrategies methodSpanStrategies; - - public BaseMethodTracer() { - this(MethodSpanStrategies.getInstance()); - } - - public BaseMethodTracer(MethodSpanStrategies methodSpanStrategies) { - this.methodSpanStrategies = methodSpanStrategies; - } - - public BaseMethodTracer(OpenTelemetry openTelemetry) { - this(openTelemetry, MethodSpanStrategies.getInstance()); - } - - public BaseMethodTracer(OpenTelemetry openTelemetry, MethodSpanStrategies methodSpanStrategies) { - super(openTelemetry); - this.methodSpanStrategies = methodSpanStrategies; - } - - /** - * Resolves the {@link MethodSpanStrategy} for tracing the specified {@code method} and stores - * that strategy in the returned {@code Context}. - */ - protected Context withMethodSpanStrategy(Context context, Method method) { - Class returnType = method.getReturnType(); - MethodSpanStrategy methodSpanStrategy = methodSpanStrategies.resolveStrategy(returnType); - return context.with(methodSpanStrategy); - } - - /** - * 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 remain open until the asynchronous operation has - * completed. - * - * @param result Return value from the traced method. - * @return Either {@code result} or a value composing over {@code result} for notification of - * completion. - */ - public Object end(Context context, Object result) { - return MethodSpanStrategy.fromContext(context).end(this, context, result); - } -} diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy deleted file mode 100644 index 2a23c5876ba0..000000000000 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseMethodTracerTest.groovy +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.tracer - -import io.opentelemetry.context.Context -import io.opentelemetry.instrumentation.api.tracer.async.MethodSpanStrategy -import spock.lang.Specification - -import java.util.concurrent.CompletableFuture - -class BaseMethodTracerTest extends Specification { - - BaseMethodTracer tracer - - void setup() { - tracer = new BaseMethodTracer() { - @Override - protected String getInstrumentationName() { - return "BaseMethodTracerTest" - } - } - } - - def "resolves method strategy and stores in returned Context"() { - given: - def strategy = MethodSpanStrategy.forCompletableFuture() - def method = TestMethods.getMethod("completableFuture") - - when: - def context = tracer.withMethodSpanStrategy(Context.root(), method) - - then: - context.get(MethodSpanStrategy.CONTEXT_KEY) == strategy - } - - def "ends span through method strategy in Context"() { - given: - MethodSpanStrategy strategy = Mock() - def context = Context.root().with(MethodSpanStrategy.CONTEXT_KEY, strategy) - - when: - tracer.end(context, "Result") - - then: - 1 * strategy.end(tracer, context, "Result") - } - - class TestMethods { - CompletableFuture completableFuture() { - return CompletableFuture.completedFuture("Value") - } - } -} diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java index b89d5c6b4068..c7fd5bf9e820 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java @@ -13,6 +13,7 @@ import io.opentelemetry.context.Scope; import java.lang.reflect.Method; import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; /** * Instrumentation for methods annotated with {@link WithSpan} annotation. @@ -42,6 +43,7 @@ public static void onEnter( public static void stopSpan( @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope, + @Advice.Return(typing = Assigner.Typing.DYNAMIC) Object returnValue, @Advice.Thrown Throwable throwable) { if (scope == null) { return; @@ -51,7 +53,7 @@ public static void stopSpan( if (throwable != null) { tracer().endExceptionally(context, throwable); } else { - tracer().end(context); + tracer().end(context, returnValue); } } } 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 index 6f56531cab33..d738737d0ffa 100644 --- 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 @@ -10,6 +10,8 @@ import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; +import io.opentelemetry.javaagent.instrumentation.otelannotations.async.MethodSpanStrategies; +import io.opentelemetry.javaagent.instrumentation.otelannotations.async.MethodSpanStrategy; import java.lang.reflect.Method; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -23,19 +25,33 @@ public static WithSpanTracer tracer() { private static final Logger log = LoggerFactory.getLogger(WithSpanTracer.class); + private final MethodSpanStrategies methodSpanStrategies = MethodSpanStrategies.getInstance(); + public Context startSpan( Context parentContext, WithSpan applicationAnnotation, Method method, SpanKind kind) { + + Context spanStrategyContext = withMethodSpanStrategy(parentContext, method); Span span = spanBuilder( parentContext, spanNameForMethodWithAnnotation(applicationAnnotation, method), kind) .startSpan(); if (kind == SpanKind.SERVER) { - return withServerSpan(parentContext, span); + return withServerSpan(spanStrategyContext, span); } if (kind == SpanKind.CLIENT) { - return withClientSpan(parentContext, span); + return withClientSpan(spanStrategyContext, span); } - return parentContext.with(span); + return spanStrategyContext.with(span); + } + + /** + * Resolves the {@link MethodSpanStrategy} for tracing the specified {@code method} and stores + * that strategy in the returned {@code Context}. + */ + protected Context withMethodSpanStrategy(Context context, Method method) { + Class returnType = method.getReturnType(); + MethodSpanStrategy methodSpanStrategy = methodSpanStrategies.resolveStrategy(returnType); + return context.with(methodSpanStrategy); } /** @@ -69,6 +85,20 @@ public static SpanKind toAgentOrNull( } } + /** + * 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 remain open until the asynchronous operation has + * completed. + * + * @param result Return value from the traced method. + * @return Either {@code result} or a value composing over {@code result} for notification of + * completion. + */ + public Object end(Context context, Object result) { + return MethodSpanStrategy.fromContext(context).end(this, context, result); + } + @Override protected String getInstrumentationName() { return "io.opentelemetry.javaagent.opentelemetry-annotations-1.0"; diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategy.java similarity index 94% rename from instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategy.java index e44120736e5c..cbaf651a7ee3 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategy.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.tracer.async; +package io.opentelemetry.javaagent.instrumentation.otelannotations.async; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategy.java similarity index 91% rename from instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategy.java index 6e7214fc24f7..b4a7ec3e6ecc 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategy.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.tracer.async; +package io.opentelemetry.javaagent.instrumentation.otelannotations.async; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategies.java similarity index 94% rename from instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategies.java index 79518a48c99d..0aaabaccc815 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategies.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategies.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.tracer.async; +package io.opentelemetry.javaagent.instrumentation.otelannotations.async; import java.util.HashMap; import java.util.Map; diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java similarity index 97% rename from instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java index 92acd1dc5141..b543ba9bef21 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/MethodSpanStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.tracer.async; +package io.opentelemetry.javaagent.instrumentation.otelannotations.async; import io.opentelemetry.context.Context; import io.opentelemetry.context.ContextKey; diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java similarity index 84% rename from instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java index 0aa18e1e5644..728f5f5ea67b 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.tracer.async; +package io.opentelemetry.javaagent.instrumentation.otelannotations.async; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/package-info.java similarity index 72% rename from instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/package-info.java index e48560a54630..0667913f73bd 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/async/package-info.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/package-info.java @@ -2,4 +2,4 @@ * 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. */ -package io.opentelemetry.instrumentation.api.tracer.async; +package io.opentelemetry.javaagent.instrumentation.otelannotations.async; diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy index feaefd632fe4..653328536cc5 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy @@ -3,6 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +import java.util.concurrent.CompletableFuture + import static io.opentelemetry.api.trace.SpanKind.CLIENT import static io.opentelemetry.api.trace.SpanKind.PRODUCER import static io.opentelemetry.api.trace.SpanKind.SERVER @@ -144,4 +146,102 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { Thread.sleep(500) // sleep a bit just to make sure no span is captured assertTraces(0) {} } + + def "should capture span for CompletionStage"() { + setup: + def future = new CompletableFuture() + new TracedWithSpan().completionStage(future) + + expect: + Thread.sleep(500) // sleep a bit just to make sure no span is captured + assertTraces(0) {} + + future.complete("Done") + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completionStage" + kind SpanKind.INTERNAL + hasNoParent() + errored false + attributes { + } + } + } + } + } + + def "should capture span for CompletionStage on completed exceptionally"() { + setup: + def future = new CompletableFuture() + new TracedWithSpan().completionStage(future) + + expect: + Thread.sleep(500) // sleep a bit just to make sure no span is captured + assertTraces(0) {} + + future.completeExceptionally(new IllegalArgumentException("Boom")) + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completionStage" + kind SpanKind.INTERNAL + hasNoParent() + errored true + errorEvent(IllegalArgumentException, "Boom") + attributes { + } + } + } + } + } + + def "should capture span for CompletableFuture"() { + setup: + def future = new CompletableFuture() + new TracedWithSpan().completableFuture(future) + + expect: + Thread.sleep(500) // sleep a bit just to make sure no span is captured + assertTraces(0) {} + + future.complete("Done") + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completableFuture" + kind SpanKind.INTERNAL + hasNoParent() + errored false + attributes { + } + } + } + } + } + + def "should capture span for CompletableFuture on completed exceptionally"() { + setup: + def future = new CompletableFuture() + new TracedWithSpan().completableFuture(future) + + expect: + Thread.sleep(500) // sleep a bit just to make sure no span is captured + assertTraces(0) {} + + future.completeExceptionally(new IllegalArgumentException("Boom")) + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completableFuture" + kind SpanKind.INTERNAL + hasNoParent() + errored true + errorEvent(IllegalArgumentException, "Boom") + attributes { + } + } + } + } + } } diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategyTest.groovy b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategyTest.groovy similarity index 96% rename from instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategyTest.groovy rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategyTest.groovy index 1a2e85707810..8e02d6d3e8ff 100644 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletableFutureMethodSpanStrategyTest.groovy +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategyTest.groovy @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.tracer.async +package io.opentelemetry.javaagent.instrumentation.otelannotations.async import io.opentelemetry.context.Context import io.opentelemetry.instrumentation.api.tracer.BaseTracer diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategyTest.groovy b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategyTest.groovy similarity index 96% rename from instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategyTest.groovy rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategyTest.groovy index 8e94e271ea8f..bb890acf39db 100644 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/CompletionStageMethodSpanStrategyTest.groovy +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategyTest.groovy @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.tracer.async +package io.opentelemetry.javaagent.instrumentation.otelannotations.async import io.opentelemetry.context.Context import io.opentelemetry.instrumentation.api.tracer.BaseTracer diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategyTest.groovy b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategyTest.groovy similarity index 90% rename from instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategyTest.groovy rename to instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategyTest.groovy index b27271960cc5..55b0888ed71e 100644 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/async/SynchronousMethodSpanStrategyTest.groovy +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategyTest.groovy @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.tracer.async +package io.opentelemetry.javaagent.instrumentation.otelannotations.async import io.opentelemetry.context.Context import io.opentelemetry.instrumentation.api.tracer.BaseTracer diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/java/io/opentelemetry/test/annotation/TracedWithSpan.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/java/io/opentelemetry/test/annotation/TracedWithSpan.java index 03c39a97981c..8cfc2c5986a2 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/java/io/opentelemetry/test/annotation/TracedWithSpan.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/java/io/opentelemetry/test/annotation/TracedWithSpan.java @@ -7,6 +7,8 @@ import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.extension.annotations.WithSpan; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; public class TracedWithSpan { @@ -54,4 +56,14 @@ public String nestedClients() { public String innerClient() { return "hello!"; } + + @WithSpan + public CompletionStage completionStage(CompletableFuture future) { + return future; + } + + @WithSpan + public CompletableFuture completableFuture(CompletableFuture future) { + return future; + } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java index 163d5d20161a..89d04ee7bc97 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java @@ -47,7 +47,8 @@ public Object traceMethod(ProceedingJoinPoint pjp) throws Throwable { Context context = tracer.startSpan(parentContext, withSpan, method); try (Scope ignored = context.makeCurrent()) { Object result = pjp.proceed(); - return tracer.end(context, result); + tracer.end(context); + return result; } catch (Throwable t) { tracer.endExceptionally(context, t); throw t; diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java index 202b0c32ed84..190c91bfc0a7 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTracer.java @@ -9,10 +9,10 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.extension.annotations.WithSpan; -import io.opentelemetry.instrumentation.api.tracer.BaseMethodTracer; +import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import java.lang.reflect.Method; -class WithSpanAspectTracer extends BaseMethodTracer { +class WithSpanAspectTracer extends BaseTracer { WithSpanAspectTracer(OpenTelemetry openTelemetry) { super(openTelemetry); } @@ -23,17 +23,15 @@ protected String getInstrumentationName() { } Context startSpan(Context parentContext, WithSpan annotation, Method method) { - Context spanStrategyContext = withMethodSpanStrategy(parentContext, method); Span span = spanBuilder(parentContext, spanName(annotation, method), annotation.kind()).startSpan(); - switch (annotation.kind()) { case SERVER: - return withServerSpan(spanStrategyContext, span); + return withServerSpan(parentContext, span); case CLIENT: - return withClientSpan(spanStrategyContext, span); + return withClientSpan(parentContext, span); default: - return spanStrategyContext.with(span); + return parentContext.with(span); } } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java index 54f780e16e86..1896a09e8ef9 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java @@ -21,12 +21,9 @@ import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.data.StatusData; import java.util.List; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.springframework.aop.aspectj.annotation.AspectJProxyFactory; @@ -62,16 +59,6 @@ public String testWithClientSpan() { public String testWithServerSpan() { return "Span with name testWithServerSpan and SpanKind.SERVER was created"; } - - @WithSpan - public CompletionStage testAsyncCompletionStage(CompletionStage stage) { - return stage; - } - - @WithSpan - public CompletableFuture testAsyncCompletableFuture(CompletableFuture stage) { - return stage; - } } private WithSpanTester withSpanTester; @@ -200,226 +187,4 @@ void suppressServerSpan() throws Throwable { trace.hasSpansSatisfyingExactly( parentSpan -> parentSpan.hasName("parent").hasKind(SERVER))); } - - @Nested - @DisplayName("with a method annotated with @WithSpan returns CompletionStage") - class WithCompletionStage { - - @Test - @DisplayName("should end Span on complete") - void onComplete() throws Throwable { - CompletableFuture future = new CompletableFuture<>(); - - // when - withSpan("parent", () -> withSpanTester.testAsyncCompletionStage(future)); - - // then - assertThat(instrumentation.waitForTraces(1)) - .hasTracesSatisfyingExactly( - trace -> - trace - .hasSize(1) - .hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL))); - - // when - future.complete("DONE"); - - // then - List> traces = instrumentation.waitForTraces(1); - assertThat(traces) - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), - span -> - span.hasName("WithSpanTester.testAsyncCompletionStage") - .hasKind(INTERNAL) - .hasParentSpanId(traces.get(0).get(0).getSpanId()))); - } - - @Test - @DisplayName("should end Span on completeException AND should record the exception") - void onCompleteExceptionally() throws Throwable { - CompletableFuture future = new CompletableFuture<>(); - - // when - withSpan("parent", () -> withSpanTester.testAsyncCompletionStage(future)); - - // then - assertThat(instrumentation.waitForTraces(1)) - .hasTracesSatisfyingExactly( - trace -> - trace - .hasSize(1) - .hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL))); - - // when - future.completeExceptionally(new Exception("Test @WithSpan With completeExceptionally")); - - // then - List> traces = instrumentation.waitForTraces(1); - assertThat(traces) - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), - span -> - span.hasName("WithSpanTester.testAsyncCompletionStage") - .hasKind(INTERNAL) - .hasStatus(StatusData.error()) - .hasParentSpanId(traces.get(0).get(0).getSpanId()))); - } - - @Test - @DisplayName("should end Span on incompatible return value") - void onIncompatibleReturnValue() throws Throwable { - // when - withSpan("parent", () -> withSpanTester.testAsyncCompletionStage(null)); - - // then - List> traces = instrumentation.waitForTraces(1); - assertThat(traces) - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), - span -> - span.hasName("WithSpanTester.testAsyncCompletionStage") - .hasKind(INTERNAL) - .hasParentSpanId(traces.get(0).get(0).getSpanId()))); - } - } - - @Nested - @DisplayName("with a method annotated with @WithSpan returns CompletableFuture") - class WithCompletableFuture { - - @Test - @DisplayName("should end Span on complete") - void onComplete() throws Throwable { - CompletableFuture future = new CompletableFuture<>(); - - // when - withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); - - // then - assertThat(instrumentation.waitForTraces(1)) - .hasTracesSatisfyingExactly( - trace -> - trace - .hasSize(1) - .hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL))); - - // when - future.complete("DONE"); - - // then - List> traces = instrumentation.waitForTraces(1); - assertThat(traces) - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), - span -> - span.hasName("WithSpanTester.testAsyncCompletableFuture") - .hasKind(INTERNAL) - .hasParentSpanId(traces.get(0).get(0).getSpanId()))); - } - - @Test - @DisplayName("should end Span on completeException AND should record the exception") - void onCompleteExceptionally() throws Throwable { - CompletableFuture future = new CompletableFuture<>(); - - // when - withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); - - // then - assertThat(instrumentation.waitForTraces(1)) - .hasTracesSatisfyingExactly( - trace -> - trace - .hasSize(1) - .hasSpansSatisfyingExactly(span -> span.hasName("parent").hasKind(INTERNAL))); - - // when - future.completeExceptionally(new Exception("Test @WithSpan With completeExceptionally")); - - // then - List> traces = instrumentation.waitForTraces(1); - assertThat(traces) - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), - span -> - span.hasName("WithSpanTester.testAsyncCompletableFuture") - .hasKind(INTERNAL) - .hasStatus(StatusData.error()) - .hasParentSpanId(traces.get(0).get(0).getSpanId()))); - } - - @Test - @DisplayName("should end the Span when already complete") - void onCompletedFuture() throws Throwable { - CompletableFuture future = CompletableFuture.completedFuture("Done"); - - // when - withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); - - // then - List> traces = instrumentation.waitForTraces(1); - assertThat(traces) - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), - span -> - span.hasName("WithSpanTester.testAsyncCompletableFuture") - .hasKind(INTERNAL) - .hasParentSpanId(traces.get(0).get(0).getSpanId()))); - } - - @Test - @DisplayName("should end the Span when already failed") - void onFailedFuture() throws Throwable { - CompletableFuture future = new CompletableFuture<>(); - future.completeExceptionally(new Exception("Test @WithSpan With completeExceptionally")); - - // when - withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(future)); - - // then - List> traces = instrumentation.waitForTraces(1); - assertThat(traces) - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), - span -> - span.hasName("WithSpanTester.testAsyncCompletableFuture") - .hasKind(INTERNAL) - .hasStatus(StatusData.error()) - .hasParentSpanId(traces.get(0).get(0).getSpanId()))); - } - - @Test - @DisplayName("should end Span on incompatible return value") - void onIncompatibleReturnValue() throws Throwable { - // when - withSpan("parent", () -> withSpanTester.testAsyncCompletableFuture(null)); - - // then - List> traces = instrumentation.waitForTraces(1); - assertThat(traces) - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - parentSpan -> parentSpan.hasName("parent").hasKind(INTERNAL), - span -> - span.hasName("WithSpanTester.testAsyncCompletableFuture") - .hasKind(INTERNAL) - .hasParentSpanId(traces.get(0).get(0).getSpanId()))); - } - } } From 3c8a635dd00b7872bb650fb92e63c670a650cc83 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 9 Mar 2021 23:34:00 -0500 Subject: [PATCH 13/19] nix unit tests --- ...letableFutureMethodSpanStrategyTest.groovy | 96 ------------------- ...mpletionStageMethodSpanStrategyTest.groovy | 96 ------------------- .../SynchronousMethodSpanStrategyTest.groovy | 39 -------- 3 files changed, 231 deletions(-) delete mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategyTest.groovy delete mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategyTest.groovy delete mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategyTest.groovy diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategyTest.groovy b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategyTest.groovy deleted file mode 100644 index 8e02d6d3e8ff..000000000000 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategyTest.groovy +++ /dev/null @@ -1,96 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.otelannotations.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 CompletableFutureMethodSpanStrategyTest extends Specification { - BaseTracer tracer - - Context context - - def underTest = CompletableFutureMethodSpanStrategy.INSTANCE - - void setup() { - tracer = Mock() - context = Mock() - } - - def "ends span on incompatible result"() { - when: - underTest.end(tracer, context, "Incompatible") - - then: - 1 * tracer.end(context) - } - - def "ends span on null result"() { - when: - underTest.end(tracer, context, null) - - then: - 1 * tracer.end(context) - } - - 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/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategyTest.groovy b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategyTest.groovy deleted file mode 100644 index bb890acf39db..000000000000 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategyTest.groovy +++ /dev/null @@ -1,96 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.otelannotations.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 CompletionStageMethodSpanStrategyTest extends Specification { - BaseTracer tracer - - Context context - - def underTest = CompletionStageMethodSpanStrategy.INSTANCE - - void setup() { - tracer = Mock() - context = Mock() - } - - def "ends span on incompatible result"() { - when: - underTest.end(tracer, context, "Incompatible") - - then: - 1 * tracer.end(context) - } - - def "ends span on null result"() { - when: - underTest.end(tracer, context, null) - - then: - 1 * tracer.end(context) - } - - 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/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategyTest.groovy b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategyTest.groovy deleted file mode 100644 index 55b0888ed71e..000000000000 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategyTest.groovy +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.otelannotations.async - -import io.opentelemetry.context.Context -import io.opentelemetry.instrumentation.api.tracer.BaseTracer -import spock.lang.Specification - -class SynchronousMethodSpanStrategyTest extends Specification { - BaseTracer tracer - - Context context - - def underTest = SynchronousMethodSpanStrategy.INSTANCE - - void setup() { - tracer = Mock() - context = Mock() - } - - def "ends span on any result"() { - when: - underTest.end(tracer, context, "any result") - - then: - 1 * tracer.end(context) - } - - def "ends span on null result"() { - when: - underTest.end(tracer, context, null) - - then: - 1 * tracer.end(context) - } -} From b1981de7b5b9892d55ce33b729b6c42da54a83a5 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Wed, 10 Mar 2021 08:15:49 -0500 Subject: [PATCH 14/19] Consolidate JDK8 strategies, remove use of context --- .../otelannotations/WithSpanAdvice.java | 3 +- .../otelannotations/WithSpanTracer.java | 24 ++------ .../CompletableFutureMethodSpanStrategy.java | 51 ---------------- .../CompletionStageMethodSpanStrategy.java | 31 ---------- .../async/Jdk8MethodStrategy.java | 59 +++++++++++++++++++ .../async/MethodSpanStrategies.java | 34 +++++------ .../async/MethodSpanStrategy.java | 36 +---------- .../async/SynchronousMethodSpanStrategy.java | 7 ++- 8 files changed, 92 insertions(+), 153 deletions(-) delete mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategy.java delete mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategy.java create mode 100644 instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java index c7fd5bf9e820..f65ff3cf4b40 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java @@ -41,6 +41,7 @@ public static void onEnter( @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( + @Advice.Origin Method method, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope, @Advice.Return(typing = Assigner.Typing.DYNAMIC) Object returnValue, @@ -53,7 +54,7 @@ public static void stopSpan( if (throwable != null) { tracer().endExceptionally(context, throwable); } else { - tracer().end(context, returnValue); + tracer().end(context, method, returnValue); } } } 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 index d738737d0ffa..d732b67467a9 100644 --- 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 @@ -11,7 +11,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import io.opentelemetry.javaagent.instrumentation.otelannotations.async.MethodSpanStrategies; -import io.opentelemetry.javaagent.instrumentation.otelannotations.async.MethodSpanStrategy; import java.lang.reflect.Method; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,29 +28,17 @@ public static WithSpanTracer tracer() { public Context startSpan( Context parentContext, WithSpan applicationAnnotation, Method method, SpanKind kind) { - - Context spanStrategyContext = withMethodSpanStrategy(parentContext, method); Span span = spanBuilder( parentContext, spanNameForMethodWithAnnotation(applicationAnnotation, method), kind) .startSpan(); if (kind == SpanKind.SERVER) { - return withServerSpan(spanStrategyContext, span); + return withServerSpan(parentContext, span); } if (kind == SpanKind.CLIENT) { - return withClientSpan(spanStrategyContext, span); + return withClientSpan(parentContext, span); } - return spanStrategyContext.with(span); - } - - /** - * Resolves the {@link MethodSpanStrategy} for tracing the specified {@code method} and stores - * that strategy in the returned {@code Context}. - */ - protected Context withMethodSpanStrategy(Context context, Method method) { - Class returnType = method.getReturnType(); - MethodSpanStrategy methodSpanStrategy = methodSpanStrategies.resolveStrategy(returnType); - return context.with(methodSpanStrategy); + return parentContext.with(span); } /** @@ -95,8 +82,9 @@ public static SpanKind toAgentOrNull( * @return Either {@code result} or a value composing over {@code result} for notification of * completion. */ - public Object end(Context context, Object result) { - return MethodSpanStrategy.fromContext(context).end(this, context, result); + public Object end(Context context, Method method, Object result) { + Class returnType = method.getReturnType(); + return methodSpanStrategies.resolveStrategy(returnType).end(this, context, returnType, result); } @Override diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategy.java deleted file mode 100644 index cbaf651a7ee3..000000000000 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletableFutureMethodSpanStrategy.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.otelannotations.async; - -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import java.util.concurrent.CompletableFuture; - -enum CompletableFutureMethodSpanStrategy implements MethodSpanStrategy { - INSTANCE; - - @Override - public Object end(BaseTracer tracer, Context context, Object result) { - if (result instanceof CompletableFuture) { - CompletableFuture future = (CompletableFuture) result; - if (future.isDone()) { - return endSynchronously(tracer, context, future); - } else { - return endOnCompletion(tracer, context, future); - } - } - tracer.end(context); - return result; - } - - private CompletableFuture endSynchronously( - BaseTracer tracer, Context context, CompletableFuture future) { - try { - future.join(); - tracer.end(context); - } catch (Exception exception) { - tracer.endExceptionally(context, exception); - } - return future; - } - - private CompletableFuture endOnCompletion( - BaseTracer tracer, Context context, CompletableFuture future) { - return future.whenComplete( - (value, error) -> { - if (error != null) { - tracer.endExceptionally(context, error); - } else { - tracer.end(context); - } - }); - } -} diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategy.java deleted file mode 100644 index b4a7ec3e6ecc..000000000000 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/CompletionStageMethodSpanStrategy.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.otelannotations.async; - -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import java.util.concurrent.CompletionStage; - -enum CompletionStageMethodSpanStrategy implements MethodSpanStrategy { - INSTANCE; - - @Override - public Object end(BaseTracer tracer, Context context, Object result) { - if (result instanceof CompletionStage) { - CompletionStage stage = (CompletionStage) result; - return stage.whenComplete( - (value, error) -> { - if (error != null) { - tracer.endExceptionally(context, error); - } else { - tracer.end(context); - } - }); - } - tracer.end(context); - return result; - } -} diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java new file mode 100644 index 000000000000..03ba3bd7f928 --- /dev/null +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java @@ -0,0 +1,59 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.otelannotations.async; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.tracer.BaseTracer; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; + +enum Jdk8MethodStrategy implements MethodSpanStrategy { + INSTANCE; + + @Override + public boolean supports(Class returnType) { + return returnType == CompletionStage.class || returnType == CompletableFuture.class; + } + + @Override + public Object end(BaseTracer tracer, Context context, Class returnType, Object result) { + if (result instanceof CompletableFuture) { + CompletableFuture future = (CompletableFuture) result; + if (future.isDone()) { + return endSynchronously(future, tracer, context); + } + return endWhenComplete(future, tracer, context); + } else if (result instanceof CompletionStage) { + CompletionStage stage = (CompletionStage) result; + return endWhenComplete(stage, tracer, context); + } + tracer.end(context); + return result; + } + + private CompletableFuture endSynchronously( + CompletableFuture future, BaseTracer tracer, Context context) { + try { + future.join(); + tracer.end(context); + } catch (Exception exception) { + tracer.endExceptionally(context, exception); + } + return future; + } + + 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/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategies.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategies.java index 0aaabaccc815..0fc27b633a4b 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategies.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategies.java @@ -5,40 +5,38 @@ package io.opentelemetry.javaagent.instrumentation.otelannotations.async; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.CopyOnWriteArrayList; /** * Registry of {@link MethodSpanStrategy} implementations for tracing the asynchronous operations * represented by the return type of a traced method. */ public class MethodSpanStrategies { - private static final MethodSpanStrategies instance; - - static { - Map, MethodSpanStrategy> strategies = new HashMap<>(); - strategies.put(CompletionStage.class, MethodSpanStrategy.forCompletionStage()); - strategies.put(CompletableFuture.class, MethodSpanStrategy.forCompletableFuture()); - instance = new MethodSpanStrategies(strategies); - } + private static final MethodSpanStrategies instance = new MethodSpanStrategies(); public static MethodSpanStrategies getInstance() { return instance; } - private final Map, MethodSpanStrategy> strategies; + private final List strategies = new CopyOnWriteArrayList<>(); - private MethodSpanStrategies(Map, MethodSpanStrategy> strategies) { - this.strategies = strategies; + private MethodSpanStrategies() { + strategies.add(Jdk8MethodStrategy.INSTANCE); } - public void registerStrategy(Class returnType, MethodSpanStrategy strategy) { - strategies.put(returnType, strategy); + public void registerStrategy(MethodSpanStrategy strategy) { + Objects.requireNonNull(strategy); + strategies.add(strategy); } public MethodSpanStrategy resolveStrategy(Class returnType) { - return strategies.getOrDefault(returnType, MethodSpanStrategy.synchronous()); + for (MethodSpanStrategy strategy : strategies) { + if (strategy.supports(returnType)) { + return strategy; + } + } + return MethodSpanStrategy.synchronous(); } } diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java index b543ba9bef21..5a86bd72cd07 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java @@ -6,8 +6,6 @@ package io.opentelemetry.javaagent.instrumentation.otelannotations.async; import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; -import io.opentelemetry.context.ImplicitContextKeyed; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; /** @@ -16,9 +14,8 @@ * can compose or register for notification of completion at which point the span representing the * invocation of the method will be ended. */ -public interface MethodSpanStrategy extends ImplicitContextKeyed { - ContextKey CONTEXT_KEY = - ContextKey.named("opentelemetry-spring-autoconfigure-aspects-method-span-strategy"); +public interface MethodSpanStrategy { + boolean supports(Class returnType); /** * Denotes the end of the invocation of the traced method with a successful result which will end @@ -32,17 +29,7 @@ public interface MethodSpanStrategy extends ImplicitContextKeyed { * @return Either {@code result} or a value composing over {@code result} for notification of * completion. */ - Object end(BaseTracer tracer, Context context, Object result); - - @Override - default Context storeInContext(Context context) { - return context.with(CONTEXT_KEY, this); - } - - static MethodSpanStrategy fromContext(Context context) { - MethodSpanStrategy methodSpanStrategy = context.get(CONTEXT_KEY); - return methodSpanStrategy != null ? methodSpanStrategy : synchronous(); - } + Object end(BaseTracer tracer, Context context, Class returnType, Object result); /** * Returns a {@link MethodSpanStrategy} for tracing synchronous methods where the return value @@ -51,21 +38,4 @@ static MethodSpanStrategy fromContext(Context context) { static MethodSpanStrategy synchronous() { return SynchronousMethodSpanStrategy.INSTANCE; } - - /** - * Returns a {@link MethodSpanStrategy} for tracing a method that returns a {@link - * java.util.concurrent.CompletionStage} representing the completion of an asynchronous operation. - */ - static MethodSpanStrategy forCompletionStage() { - return CompletionStageMethodSpanStrategy.INSTANCE; - } - - /** - * Returns a {@link MethodSpanStrategy} for tracing a method that returns a {@link - * java.util.concurrent.CompletableFuture} representing the completion of an asynchronous - * operation. - */ - static MethodSpanStrategy forCompletableFuture() { - return CompletableFutureMethodSpanStrategy.INSTANCE; - } } diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java index 728f5f5ea67b..2465f0355baa 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java @@ -12,7 +12,12 @@ enum SynchronousMethodSpanStrategy implements MethodSpanStrategy { INSTANCE; @Override - public Object end(BaseTracer tracer, Context context, Object result) { + public boolean supports(Class returnType) { + return true; + } + + @Override + public Object end(BaseTracer tracer, Context context, Class returnType, Object result) { tracer.end(context); return result; } From 272a0f7153daf9056626780136bcf71b3830f630 Mon Sep 17 00:00:00 2001 From: HaloFour Date: Fri, 12 Mar 2021 09:05:46 -0500 Subject: [PATCH 15/19] Clarify verbiage in Javadoc Co-authored-by: Mateusz Rzeszutek --- .../instrumentation/otelannotations/WithSpanTracer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index d732b67467a9..83c13850eafc 100644 --- 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 @@ -75,7 +75,7 @@ public static SpanKind toAgentOrNull( /** * 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 remain open until the asynchronous operation has + * asynchronous operation then the span will not be finished until the asynchronous operation has * completed. * * @param result Return value from the traced method. From 9f74fea07f784c453fa36c539b0fc5676795a5a9 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Fri, 12 Mar 2021 10:41:26 -0500 Subject: [PATCH 16/19] Refactor synchronous completion and add comments, tests --- .../async/Jdk8MethodStrategy.java | 35 +++-- .../groovy/WithSpanInstrumentationTest.groovy | 134 +++++++++++++++++- 2 files changed, 157 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java index 03ba3bd7f928..e03701a71448 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java @@ -22,8 +22,8 @@ public boolean supports(Class returnType) { public Object end(BaseTracer tracer, Context context, Class returnType, Object result) { if (result instanceof CompletableFuture) { CompletableFuture future = (CompletableFuture) result; - if (future.isDone()) { - return endSynchronously(future, tracer, context); + if (endSynchronously(future, tracer, context)) { + return future; } return endWhenComplete(future, tracer, context); } else if (result instanceof CompletionStage) { @@ -34,17 +34,36 @@ public Object end(BaseTracer tracer, Context context, Class returnType, Objec return result; } - private CompletableFuture endSynchronously( + /** + * 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 boolean endSynchronously( CompletableFuture future, BaseTracer tracer, Context context) { - try { - future.join(); + + if (future.isDone()) { + 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 (Exception exception) { + tracer.endExceptionally(context, exception); + return true; + } + } tracer.end(context); - } catch (Exception exception) { - tracer.endExceptionally(context, exception); + return true; + } else { + return false; } - return future; } + /** + * 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( diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy index 653328536cc5..8d49dacc2c8d 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy @@ -147,7 +147,27 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { assertTraces(0) {} } - def "should capture span for CompletionStage"() { + def "should capture span for already completed CompletionStage"() { + setup: + def future = CompletableFuture.completedFuture("Done") + new TracedWithSpan().completionStage(future) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completionStage" + kind SpanKind.INTERNAL + hasNoParent() + errored false + attributes { + } + } + } + } + } + + def "should capture span for eventually completed CompletionStage"() { setup: def future = new CompletableFuture() new TracedWithSpan().completionStage(future) @@ -157,6 +177,7 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { assertTraces(0) {} future.complete("Done") + assertTraces(1) { trace(0, 1) { span(0) { @@ -171,7 +192,29 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { } } - def "should capture span for CompletionStage on completed exceptionally"() { + def "should capture span for already exceptionally completed CompletionStage"() { + setup: + def future = new CompletableFuture() + future.completeExceptionally(new IllegalArgumentException("Boom")) + new TracedWithSpan().completionStage(future) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completionStage" + kind SpanKind.INTERNAL + hasNoParent() + errored true + errorEvent(IllegalArgumentException, "Boom") + attributes { + } + } + } + } + } + + def "should capture span for eventually exceptionally completed CompletionStage"() { setup: def future = new CompletableFuture() new TracedWithSpan().completionStage(future) @@ -181,6 +224,7 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { assertTraces(0) {} future.completeExceptionally(new IllegalArgumentException("Boom")) + assertTraces(1) { trace(0, 1) { span(0) { @@ -196,7 +240,46 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { } } - def "should capture span for CompletableFuture"() { + def "should capture span for null CompletionStage"() { + setup: + new TracedWithSpan().completionStage(null) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completionStage" + kind SpanKind.INTERNAL + hasNoParent() + errored false + attributes { + } + } + } + } + } + + def "should capture span for already completed CompletableFuture"() { + setup: + def future = CompletableFuture.completedFuture("Done") + new TracedWithSpan().completableFuture(future) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completableFuture" + kind SpanKind.INTERNAL + hasNoParent() + errored false + attributes { + } + } + } + } + } + + def "should capture span for eventually completed CompletableFuture"() { setup: def future = new CompletableFuture() new TracedWithSpan().completableFuture(future) @@ -206,6 +289,7 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { assertTraces(0) {} future.complete("Done") + assertTraces(1) { trace(0, 1) { span(0) { @@ -220,7 +304,29 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { } } - def "should capture span for CompletableFuture on completed exceptionally"() { + def "should capture span for already exceptionally completed CompletableFuture"() { + setup: + def future = new CompletableFuture() + future.completeExceptionally(new IllegalArgumentException("Boom")) + new TracedWithSpan().completableFuture(future) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completableFuture" + kind SpanKind.INTERNAL + hasNoParent() + errored true + errorEvent(IllegalArgumentException, "Boom") + attributes { + } + } + } + } + } + + def "should capture span for eventually exceptionally completed CompletableFuture"() { setup: def future = new CompletableFuture() new TracedWithSpan().completableFuture(future) @@ -230,6 +336,7 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { assertTraces(0) {} future.completeExceptionally(new IllegalArgumentException("Boom")) + assertTraces(1) { trace(0, 1) { span(0) { @@ -244,4 +351,23 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification { } } } + + def "should capture span for null CompletableFuture"() { + setup: + new TracedWithSpan().completableFuture(null) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + name "TracedWithSpan.completableFuture" + kind SpanKind.INTERNAL + hasNoParent() + errored false + attributes { + } + } + } + } + } } From 16025a450866689615d468d82ecc6c700ae217f0 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Tue, 16 Mar 2021 09:22:27 -0400 Subject: [PATCH 17/19] Early return on uncompleted future --- .../async/Jdk8MethodStrategy.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java index e03701a71448..9f93cedc512f 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java @@ -42,22 +42,22 @@ public Object end(BaseTracer tracer, Context context, Class returnType, Objec private boolean endSynchronously( CompletableFuture future, BaseTracer tracer, Context context) { - if (future.isDone()) { - 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 (Exception exception) { - tracer.endExceptionally(context, exception); - return true; - } - } - tracer.end(context); - return true; - } else { + 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 (Exception exception) { + tracer.endExceptionally(context, exception); + return true; + } + } + tracer.end(context); + return true; } /** From 72594ae32515b14946b8fdae5cb7137406637849 Mon Sep 17 00:00:00 2001 From: "Spindler, Justin" Date: Wed, 17 Mar 2021 08:10:20 -0400 Subject: [PATCH 18/19] Add check to Jdk8MethodStrategy to ensure return type of method is compatible --- .../async/Jdk8MethodStrategy.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java index 9f93cedc512f..c3b9aed8cceb 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java @@ -20,15 +20,17 @@ public boolean supports(Class returnType) { @Override public Object end(BaseTracer tracer, Context context, Class returnType, Object result) { - if (result instanceof CompletableFuture) { - CompletableFuture future = (CompletableFuture) result; - if (endSynchronously(future, tracer, context)) { - return future; + if (supports(returnType)) { + if (result instanceof CompletableFuture) { + CompletableFuture future = (CompletableFuture) result; + if (endSynchronously(future, tracer, context)) { + return future; + } + return endWhenComplete(future, tracer, context); + } else if (result instanceof CompletionStage) { + CompletionStage stage = (CompletionStage) result; + return endWhenComplete(stage, tracer, context); } - return endWhenComplete(future, tracer, context); - } else if (result instanceof CompletionStage) { - CompletionStage stage = (CompletionStage) result; - return endWhenComplete(stage, tracer, context); } tracer.end(context); return result; From da48a91d96a9247da4eff660b83b13661bcc5ced Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sun, 21 Mar 2021 16:37:12 -0700 Subject: [PATCH 19/19] A couple of suggestions (#1) --- .../otelannotations/WithSpanAdvice.java | 2 +- .../otelannotations/WithSpanTracer.java | 17 +++++++++------ .../async/Jdk8MethodStrategy.java | 21 +++++++------------ .../async/MethodSpanStrategy.java | 10 +++++---- .../async/SynchronousMethodSpanStrategy.java | 4 ++-- 5 files changed, 28 insertions(+), 26 deletions(-) diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java index f65ff3cf4b40..a63df037dea6 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanAdvice.java @@ -54,7 +54,7 @@ public static void stopSpan( if (throwable != null) { tracer().endExceptionally(context, throwable); } else { - tracer().end(context, method, returnValue); + tracer().end(context, method.getReturnType(), returnValue); } } } 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 index 83c13850eafc..90294e913451 100644 --- 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 @@ -78,13 +78,18 @@ public static SpanKind toAgentOrNull( * asynchronous operation then the span will not be finished until the asynchronous operation has * completed. * - * @param result Return value from the traced method. - * @return Either {@code result} or a value composing over {@code result} for notification of - * completion. + * @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, Method method, Object result) { - Class returnType = method.getReturnType(); - return methodSpanStrategies.resolveStrategy(returnType).end(this, context, returnType, result); + public Object end(Context context, Class returnType, Object returnValue) { + if (!returnType.isInstance(returnValue)) { + end(context); + return returnValue; + } + return methodSpanStrategies.resolveStrategy(returnType).end(this, context, returnValue); } @Override diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java index c3b9aed8cceb..9f969ea9dce3 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/Jdk8MethodStrategy.java @@ -19,21 +19,16 @@ public boolean supports(Class returnType) { } @Override - public Object end(BaseTracer tracer, Context context, Class returnType, Object result) { - if (supports(returnType)) { - if (result instanceof CompletableFuture) { - CompletableFuture future = (CompletableFuture) result; - if (endSynchronously(future, tracer, context)) { - return future; - } - return endWhenComplete(future, tracer, context); - } else if (result instanceof CompletionStage) { - CompletionStage stage = (CompletionStage) result; - return endWhenComplete(stage, tracer, context); + 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); } - tracer.end(context); - return result; + CompletionStage stage = (CompletionStage) returnValue; + return endWhenComplete(stage, tracer, context); } /** diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java index 5a86bd72cd07..e2bc16052f6b 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/MethodSpanStrategy.java @@ -25,11 +25,13 @@ public interface MethodSpanStrategy { * * @param tracer {@link BaseTracer} tracer to be used to end the span stored in the {@code * context}. - * @param result Return value of the traced method. - * @return Either {@code result} or a value composing over {@code result} for notification of - * completion. + * @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, Class returnType, Object result); + Object end(BaseTracer tracer, Context context, Object returnValue); /** * Returns a {@link MethodSpanStrategy} for tracing synchronous methods where the return value diff --git a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java index 2465f0355baa..96d1feb3e143 100644 --- a/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java +++ b/instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/async/SynchronousMethodSpanStrategy.java @@ -17,8 +17,8 @@ public boolean supports(Class returnType) { } @Override - public Object end(BaseTracer tracer, Context context, Class returnType, Object result) { + public Object end(BaseTracer tracer, Context context, Object returnValue) { tracer.end(context); - return result; + return returnValue; } }