From 53a639bbba82db7cdb0fcdc27b87b61f5e189180 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 13 Oct 2021 13:04:23 -0700 Subject: [PATCH] Fix tomcat async spans (#4339) * Add test * Fix tomcat async spans * Preserve existing test controller behavior * Comments --- .../src/test/groovy/DropwizardAsyncTest.groovy | 6 ++++++ .../src/test/groovy/GrizzlyAsyncTest.groovy | 6 ++++++ .../test/groovy/GrizzlyFilterchainServerTest.groovy | 6 ++++++ .../src/test/groovy/server/PlayServerTest.groovy | 6 ++++++ .../server/AbstractRatpackHttpServerTest.groovy | 6 ++++++ .../instrumentation/tomcat/v7_0/AsyncServlet.groovy | 13 ++----------- .../instrumentation/tomcat/common/TomcatHelper.java | 5 ++++- .../groovy/server/VertxRxHttpServerTest.groovy | 6 ++++++ .../groovy/server/VertxRxHttpServerTest.groovy | 6 ++++++ .../server/AbstractVertxHttpServerTest.groovy | 6 ++++++ .../instrumentation/test/base/HttpServerTest.groovy | 9 +++++++++ 11 files changed, 63 insertions(+), 12 deletions(-) diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy index 1b8752f14f..8e9fc566e0 100644 --- a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy +++ b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy @@ -35,6 +35,12 @@ class DropwizardAsyncTest extends DropwizardTest { AsyncServiceResource } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the JAX-RS controller spans + return false + } + static class AsyncTestApp extends Application { @Override void initialize(Bootstrap bootstrap) { diff --git a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyAsyncTest.groovy b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyAsyncTest.groovy index 0aada21688..1d5ae00bae 100644 --- a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyAsyncTest.groovy +++ b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyAsyncTest.groovy @@ -38,6 +38,12 @@ class GrizzlyAsyncTest extends GrizzlyTest { false } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the JAX-RS controller spans + return false + } + @Path("/") static class AsyncServiceResource { private ExecutorService executor = Executors.newSingleThreadExecutor() diff --git a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy index dddffb261e..c5568524f6 100644 --- a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy +++ b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy @@ -70,6 +70,12 @@ class GrizzlyFilterchainServerTest extends HttpServerTest implements false } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the controller spans + return false + } + void setUpTransport(FilterChain filterChain) { TCPNIOTransportBuilder transportBuilder = TCPNIOTransportBuilder.newInstance() .setOptimizedForMultiplexing(true) diff --git a/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy b/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy index 0700abe51c..122f2908ab 100644 --- a/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy +++ b/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy @@ -86,6 +86,12 @@ class PlayServerTest extends HttpServerTest implements AgentTestTrait { return true } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the controller spans + return false + } + @Override void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { diff --git a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackHttpServerTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackHttpServerTest.groovy index 86dc0c71d9..e79b952d8a 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackHttpServerTest.groovy @@ -137,6 +137,12 @@ abstract class AbstractRatpackHttpServerTest extends HttpServerTest { } public Context start(Context parentContext, Request request) { - return instrumenter.start(parentContext, request); + Context context = instrumenter.start(parentContext, request); + request.setAttribute(HttpServerTracer.CONTEXT_ATTRIBUTE, context); + return context; } public void end( diff --git a/instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxHttpServerTest.groovy b/instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxHttpServerTest.groovy index e2d3fdc40d..29cef8b807 100644 --- a/instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxHttpServerTest.groovy +++ b/instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxHttpServerTest.groovy @@ -84,6 +84,12 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr return true } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the controller spans + return false + } + protected Class verticle() { return VertxReactiveWebServer } diff --git a/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy b/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy index b3b829ce25..34ce2a26f3 100644 --- a/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy +++ b/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy @@ -84,6 +84,12 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr return true } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the controller spans + return false + } + protected Class verticle() { return VertxReactiveWebServer } diff --git a/instrumentation/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy b/instrumentation/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy index beb2fa641c..d65aaef3ff 100644 --- a/instrumentation/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy +++ b/instrumentation/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy @@ -63,6 +63,12 @@ abstract class AbstractVertxHttpServerTest extends HttpServerTest impleme false } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the controller spans + return false + } + @Override String expectedServerSpanName(ServerEndpoint endpoint) { switch (endpoint) { diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy index 4adbe9385a..fff0b2c587 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy @@ -135,6 +135,10 @@ abstract class HttpServerTest extends InstrumentationSpecification imple false } + boolean verifyServerSpanEndTime() { + return true + } + List> extraAttributes() { [] } @@ -506,6 +510,11 @@ abstract class HttpServerTest extends InstrumentationSpecification imple (0..size - 1).each { trace(it, spanCount) { def spanIndex = 0 + if (verifyServerSpanEndTime() && spanCount > 1) { + (1..spanCount - 1).each { index -> + assert it.span(0).endEpochNanos - it.span(index).endEpochNanos >= 0 + } + } serverSpan(it, spanIndex++, traceID, parentID, method, response?.content()?.length(), endpoint) if (hasHandlerSpan(endpoint)) { handlerSpan(it, spanIndex++, span(0), method, endpoint)