diff --git a/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/ActionInstrumentation.java b/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/ActionInstrumentation.java index c16b1583031d..6d68eb947220 100644 --- a/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/ActionInstrumentation.java +++ b/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/ActionInstrumentation.java @@ -73,10 +73,13 @@ public static void stopTraceOnResponse( tracer().updateSpanName(ServerSpan.fromContextOrNull(context), req); scope.close(); - // span finished in RequestCompleteCallback if (throwable == null) { - responseFuture.onComplete( - new RequestCompleteCallback(context), ((Action) thisAction).executionContext()); + // span is finished when future completes + // not using responseFuture.onComplete() because that doesn't guarantee this handler span + // will be completed before the server span completes + responseFuture = + ResponseFutureWrapper.wrap( + responseFuture, context, ((Action) thisAction).executionContext()); } else { tracer().endExceptionally(context, throwable); } diff --git a/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/RequestCompleteCallback.java b/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/RequestCompleteCallback.java deleted file mode 100644 index c30a1b6e03d2..000000000000 --- a/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/RequestCompleteCallback.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.play.v2_6; - -import static io.opentelemetry.javaagent.instrumentation.play.v2_6.PlayTracer.tracer; - -import io.opentelemetry.context.Context; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import play.api.mvc.Result; -import scala.runtime.AbstractFunction1; -import scala.util.Try; - -public class RequestCompleteCallback extends AbstractFunction1, Object> { - - private static final Logger logger = LoggerFactory.getLogger(RequestCompleteCallback.class); - - private final Context context; - - public RequestCompleteCallback(Context context) { - this.context = context; - } - - @Override - public Object apply(Try result) { - try { - if (result.isFailure()) { - tracer().endExceptionally(context, result.failed().get()); - } else { - tracer().end(context); - } - } catch (Throwable t) { - logger.debug("error in play instrumentation", t); - } - return null; - } -} diff --git a/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/ResponseFutureWrapper.java b/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/ResponseFutureWrapper.java new file mode 100644 index 000000000000..f440359f9dec --- /dev/null +++ b/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/ResponseFutureWrapper.java @@ -0,0 +1,38 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.play.v2_6; + +import static io.opentelemetry.javaagent.instrumentation.play.v2_6.PlayTracer.tracer; + +import io.opentelemetry.context.Context; +import play.api.mvc.Result; +import scala.concurrent.ExecutionContext; +import scala.concurrent.Future; +import scala.runtime.AbstractFunction1; + +public class ResponseFutureWrapper { + + public static Future wrap( + Future future, Context context, ExecutionContext executionContext) { + + return future.transform( + new AbstractFunction1() { + @Override + public Result apply(Result result) { + tracer().end(context); + return result; + } + }, + new AbstractFunction1() { + @Override + public Throwable apply(Throwable throwable) { + tracer().endExceptionally(context, throwable); + return throwable; + } + }, + executionContext); + } +} diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/ControllerSpringWebFluxServerTest.groovy b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/ControllerSpringWebFluxServerTest.groovy index 5f0efd07c715..85952f1ae0d9 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/ControllerSpringWebFluxServerTest.groovy +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/ControllerSpringWebFluxServerTest.groovy @@ -44,4 +44,12 @@ abstract class ControllerSpringWebFluxServerTest extends SpringWebFluxServerTest boolean hasHandlerAsControllerParentSpan(HttpServerTest.ServerEndpoint endpoint) { return false } + + @Override + boolean verifyServerSpanEndTime() { + // TODO (trask) it seems like in this case ideally the controller span (which ends when the Mono + // that the controller returns completes) should end before the server span (which needs the + // result of the Mono) + false + } } diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/HandlerSpringWebFluxServerTest.groovy b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/HandlerSpringWebFluxServerTest.groovy index 0cef823425d7..442cfd22f2e3 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/HandlerSpringWebFluxServerTest.groovy +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/server/base/HandlerSpringWebFluxServerTest.groovy @@ -39,4 +39,12 @@ abstract class HandlerSpringWebFluxServerTest extends SpringWebFluxServerTest { childOf((SpanData) parent) } } + + @Override + boolean verifyServerSpanEndTime() { + // TODO (trask) it seems like in this case ideally the handler span (which ends when the Mono + // that the handler returns completes) should end before the server span (which needs the + // result of the Mono) + false + } }