From b63c50ad4dfe2ebc779410d07c68d21c5579137d Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sun, 10 Oct 2021 15:03:46 -0700 Subject: [PATCH 1/4] Add test --- .../test/groovy/DropwizardAsyncTest.groovy | 6 ++ .../src/test/groovy/GrizzlyAsyncTest.groovy | 6 ++ .../GrizzlyFilterchainServerTest.groovy | 6 +- .../test/groovy/server/PlayServerTest.groovy | 6 ++ .../AbstractRatpackAsyncHttpServerTest.groovy | 18 ++--- ...AbstractRatpackForkedHttpServerTest.groovy | 20 ++--- .../AbstractRatpackHttpServerTest.groovy | 18 ++--- .../tomcat/v7_0/AsyncServlet.groovy | 13 +-- ...VertxRxCircuitBreakerHttpServerTest.groovy | 12 +-- .../server/VertxRxHttpServerTest.groovy | 12 +-- ...VertxRxCircuitBreakerHttpServerTest.groovy | 12 +-- .../server/VertxRxHttpServerTest.groovy | 12 +-- .../java/server/AbstractVertxWebServer.java | 81 ++++++++----------- .../test/base/HttpServerTest.groovy | 9 +++ 14 files changed, 117 insertions(+), 114 deletions(-) diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy index 15e819cb45c6..8f3ff49c4178 100644 --- a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy +++ b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy @@ -36,6 +36,12 @@ class DropwizardAsyncTest extends DropwizardTest { AsyncServiceResource } + @Override + boolean verifyServerSpanEndTime() { + // SERVER span is ended inside of JAX-RS controller span + 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 0aada2168885..efa8c3090d16 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 span is ended inside of JAX-RS controller span + 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 dddffb261e38..64512004d984 100644 --- a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy +++ b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy @@ -116,11 +116,13 @@ class GrizzlyFilterchainServerTest extends HttpServerTest implements .header("Content-Length", valueOf(responseParameters.getResponseBody().length)) responseParameters.fillHeaders(builder) HttpResponsePacket responsePacket = builder.build() + def response controller(responseParameters.getEndpoint()) { - ctx.write(HttpContent.builder(responsePacket) + response = HttpContent.builder(responsePacket) .content(wrap(ctx.getMemoryManager(), responseParameters.getResponseBody())) - .build()) + .build() } + ctx.write(response) } } return ctx.getStopAction() 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 f4c78233064c..5a8916137835 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 @@ -87,6 +87,12 @@ class PlayServerTest extends HttpServerTest implements AgentTestTrait { return true } + @Override + boolean verifyServerSpanEndTime() { + // TODO (trask) see if the play controller instrumentation can be ended before netty server span + 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/AbstractRatpackAsyncHttpServerTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackAsyncHttpServerTest.groovy index 712edad6e686..60f727c5a0e9 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackAsyncHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackAsyncHttpServerTest.groovy @@ -37,8 +37,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer SUCCESS } then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(endpoint.body) } + context.response.status(endpoint.status).send(endpoint.body) } } } @@ -49,8 +49,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer } then { controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { context.request.queryParams.get(it) } - context.response.status(INDEXED_CHILD.status).send() } + context.response.status(INDEXED_CHILD.status).send() } } } @@ -60,8 +60,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer QUERY_PARAM } then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(context.request.query) } + context.response.status(endpoint.status).send(context.request.query) } } } @@ -71,8 +71,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer REDIRECT } then { endpoint -> controller(endpoint) { - context.redirect(endpoint.body) } + context.redirect(endpoint.body) } } } @@ -82,8 +82,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer ERROR } then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(endpoint.body) } + context.response.status(endpoint.status).send(endpoint.body) } } } @@ -104,8 +104,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer PATH_PARAM } then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(context.pathTokens.id) } + context.response.status(endpoint.status).send(context.pathTokens.id) } } } @@ -115,10 +115,10 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer CAPTURE_HEADERS } then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status) - context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) - context.response.send(endpoint.body) } + context.response.status(endpoint.status) + context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) + context.response.send(endpoint.body) } } } diff --git a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackForkedHttpServerTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackForkedHttpServerTest.groovy index 1bb6b1ba2103..cf5aaff947fd 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackForkedHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackForkedHttpServerTest.groovy @@ -45,8 +45,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe SUCCESS }.fork().then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(endpoint.body) } + context.response.status(endpoint.status).send(endpoint.body) } } } @@ -57,8 +57,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe }.fork().then { controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { context.request.queryParams.get(it) } - context.response.status(INDEXED_CHILD.status).send() } + context.response.status(INDEXED_CHILD.status).send() } } } @@ -68,8 +68,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe QUERY_PARAM }.fork().then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(context.request.query) } + context.response.status(endpoint.status).send(context.request.query) } } } @@ -79,8 +79,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe REDIRECT }.fork().then { endpoint -> controller(endpoint) { - context.redirect(endpoint.body) } + context.redirect(endpoint.body) } } } @@ -90,8 +90,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe ERROR }.fork().then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(endpoint.body) } + context.response.status(endpoint.status).send(endpoint.body) } } } @@ -112,8 +112,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe PATH_PARAM }.fork().then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(context.pathTokens.id) } + context.response.status(endpoint.status).send(context.pathTokens.id) } } } @@ -123,10 +123,10 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe CAPTURE_HEADERS }.fork().then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status) - context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) - context.response.send(endpoint.body) } + context.response.status(endpoint.status) + context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) + context.response.send(endpoint.body) } } } @@ -141,8 +141,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe Promise.sync { list.get(0).value } } then { endpoint -> controller(endpoint) { - context.response.status(endpoint.status).send(endpoint.body) } + context.response.status(endpoint.status).send(endpoint.body) } } } 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 4121b32ab78a..183fee6a0a61 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 @@ -44,37 +44,37 @@ abstract class AbstractRatpackHttpServerTest extends HttpServerTest controller(SUCCESS) { - context.response.status(SUCCESS.status).send(SUCCESS.body) } + context.response.status(SUCCESS.status).send(SUCCESS.body) } } it.prefix(INDEXED_CHILD.rawPath()) { it.all { context -> controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { context.request.queryParams.get(it) } - context.response.status(INDEXED_CHILD.status).send() } + context.response.status(INDEXED_CHILD.status).send() } } it.prefix(QUERY_PARAM.rawPath()) { it.all { context -> controller(QUERY_PARAM) { - context.response.status(QUERY_PARAM.status).send(context.request.query) } + context.response.status(QUERY_PARAM.status).send(context.request.query) } } it.prefix(REDIRECT.rawPath()) { it.all { context -> controller(REDIRECT) { - context.redirect(REDIRECT.body) } + context.redirect(REDIRECT.body) } } it.prefix(ERROR.rawPath()) { it.all { context -> controller(ERROR) { - context.response.status(ERROR.status).send(ERROR.body) } + context.response.status(ERROR.status).send(ERROR.body) } } it.prefix(EXCEPTION.rawPath()) { @@ -87,17 +87,17 @@ abstract class AbstractRatpackHttpServerTest extends HttpServerTest controller(PATH_PARAM) { - context.response.status(PATH_PARAM.status).send(context.pathTokens.id) } + context.response.status(PATH_PARAM.status).send(context.pathTokens.id) } } it.prefix(CAPTURE_HEADERS.rawPath()) { it.all { context -> controller(CAPTURE_HEADERS) { - context.response.status(CAPTURE_HEADERS.status) - context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) - context.response.send(CAPTURE_HEADERS.body) } + context.response.status(CAPTURE_HEADERS.status) + context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) + context.response.send(CAPTURE_HEADERS.body) } } } diff --git a/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy b/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy index 23695f4158a4..ee4dcf5ec6b2 100644 --- a/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy +++ b/instrumentation/tomcat/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy @@ -11,7 +11,6 @@ import io.opentelemetry.instrumentation.test.base.HttpServerTest import javax.servlet.annotation.WebServlet import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse -import java.util.concurrent.CountDownLatch import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR @@ -26,7 +25,6 @@ class AsyncServlet extends AbstractHttpServlet { @Override protected void service(HttpServletRequest req, HttpServletResponse resp) { HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath) - def latch = new CountDownLatch(1) def context = req.startAsync() context.start { try { @@ -36,44 +34,37 @@ class AsyncServlet extends AbstractHttpServlet { case SUCCESS: resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() break case INDEXED_CHILD: endpoint.collectSpanAttributes { req.getParameter(it) } resp.status = endpoint.status - context.complete() break case QUERY_PARAM: resp.status = endpoint.status resp.writer.print(req.queryString) - context.complete() break case REDIRECT: resp.sendRedirect(endpoint.body) - context.complete() break case CAPTURE_HEADERS: resp.setHeader("X-Test-Response", req.getHeader("X-Test-Request")) resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() break case ERROR: resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() break case EXCEPTION: resp.status = endpoint.status resp.writer.print(endpoint.body) - context.complete() throw new Exception(endpoint.body) } } } finally { - latch.countDown() + // complete at the end so the server span will end after the controller span + context.complete() } } - latch.await() } } diff --git a/instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy b/instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy index b8e4396f31aa..d27aa53fd651 100644 --- a/instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy +++ b/instrumentation/vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy @@ -50,8 +50,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).end(endpoint.body) } + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) }) } router.route(INDEXED_CHILD.path).handler { ctx -> @@ -64,8 +64,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { endpoint.collectSpanAttributes { ctx.request().params().get(it) } - ctx.response().setStatusCode(endpoint.status).end() } + ctx.response().setStatusCode(endpoint.status).end() }) } router.route(QUERY_PARAM.path).handler { ctx -> @@ -77,8 +77,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) } + ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) }) } router.route(REDIRECT.path).handler { ctx -> @@ -90,8 +90,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end() } + ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end() }) } router.route(ERROR.path).handler { ctx -> @@ -103,8 +103,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).end(endpoint.body) } + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) }) } router.route(EXCEPTION.path).handler { ctx -> @@ -130,8 +130,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).end(ctx.request().getParam("id")) } + ctx.response().setStatusCode(endpoint.status).end(ctx.request().getParam("id")) }) } 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 fe798cb4bb71..cbac3da68322 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 @@ -105,29 +105,29 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr router.route(SUCCESS.path).handler { ctx -> controller(SUCCESS) { - ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } + ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } router.route(INDEXED_CHILD.path).handler { ctx -> controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { ctx.request().params().get(it) } - ctx.response().setStatusCode(INDEXED_CHILD.status).end() } + ctx.response().setStatusCode(INDEXED_CHILD.status).end() } router.route(QUERY_PARAM.path).handler { ctx -> controller(QUERY_PARAM) { - ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) } + ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) } router.route(REDIRECT.path).handler { ctx -> controller(REDIRECT) { - ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() } + ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() } router.route(ERROR.path).handler { ctx -> controller(ERROR) { - ctx.response().setStatusCode(ERROR.status).end(ERROR.body) } + ctx.response().setStatusCode(ERROR.status).end(ERROR.body) } router.route(EXCEPTION.path).handler { ctx -> controller(EXCEPTION) { @@ -136,8 +136,8 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr } router.route("/path/:id/param").handler { ctx -> controller(PATH_PARAM) { - ctx.response().setStatusCode(PATH_PARAM.status).end(ctx.request().getParam("id")) } + ctx.response().setStatusCode(PATH_PARAM.status).end(ctx.request().getParam("id")) } super.@vertx.createHttpServer() diff --git a/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy b/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy index f413e8206600..75699e37b3a0 100644 --- a/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy +++ b/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy @@ -50,8 +50,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).end(endpoint.body) } + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) }) } router.route(INDEXED_CHILD.path).handler { ctx -> @@ -64,8 +64,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { endpoint.collectSpanAttributes { ctx.request().params().get(it) } - ctx.response().setStatusCode(endpoint.status).end() } + ctx.response().setStatusCode(endpoint.status).end() }) } router.route(QUERY_PARAM.path).handler { ctx -> @@ -77,8 +77,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) } + ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) }) } router.route(REDIRECT.path).handler { ctx -> @@ -90,8 +90,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end() } + ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end() }) } router.route(ERROR.path).handler { ctx -> @@ -103,8 +103,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).end(endpoint.body) } + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) }) } router.route(EXCEPTION.path).handler { ctx -> @@ -130,8 +130,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { - ctx.response().setStatusCode(endpoint.status).end(ctx.request().getParam("id")) } + ctx.response().setStatusCode(endpoint.status).end(ctx.request().getParam("id")) }) } 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 14e78aa30c80..1f3e3d34d509 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 @@ -105,29 +105,29 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr router.route(SUCCESS.path).handler { ctx -> controller(SUCCESS) { - ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } + ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } router.route(INDEXED_CHILD.path).handler { ctx -> controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { ctx.request().params().get(it) } - ctx.response().setStatusCode(INDEXED_CHILD.status).end() } + ctx.response().setStatusCode(INDEXED_CHILD.status).end() } router.route(QUERY_PARAM.path).handler { ctx -> controller(QUERY_PARAM) { - ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) } + ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) } router.route(REDIRECT.path).handler { ctx -> controller(REDIRECT) { - ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() } + ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() } router.route(ERROR.path).handler { ctx -> controller(ERROR) { - ctx.response().setStatusCode(ERROR.status).end(ERROR.body) } + ctx.response().setStatusCode(ERROR.status).end(ERROR.body) } router.route(EXCEPTION.path).handler { ctx -> controller(EXCEPTION) { @@ -136,8 +136,8 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr } router.route("/path/:id/param").handler { ctx -> controller(PATH_PARAM) { - ctx.response().setStatusCode(PATH_PARAM.status).end(ctx.request().getParam("id")) } + ctx.response().setStatusCode(PATH_PARAM.status).end(ctx.request().getParam("id")) } diff --git a/instrumentation/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java b/instrumentation/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java index 251f5bc836f6..4ed07e27b4d5 100644 --- a/instrumentation/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java +++ b/instrumentation/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java @@ -39,60 +39,46 @@ public Router buildRouter() { new Handler() { @Override public void handle(RoutingContext ctx) { - HttpServerTest.controller( - SUCCESS, - () -> { - end(ctx.response().setStatusCode(SUCCESS.getStatus()), SUCCESS.getBody()); - return null; - }); + HttpServerTest.controller(SUCCESS, () -> null); + end(ctx.response().setStatusCode(SUCCESS.getStatus()), SUCCESS.getBody()); } }); router .route(INDEXED_CHILD.getPath()) .handler( - ctx -> - HttpServerTest.controller( - INDEXED_CHILD, - () -> { - INDEXED_CHILD.collectSpanAttributes(it -> ctx.request().getParam(it)); - end(ctx.response().setStatusCode(INDEXED_CHILD.getStatus())); - return null; - })); + ctx -> { + HttpServerTest.controller( + INDEXED_CHILD, + () -> { + INDEXED_CHILD.collectSpanAttributes(it -> ctx.request().getParam(it)); + return null; + }); + end(ctx.response().setStatusCode(INDEXED_CHILD.getStatus())); + }); router .route(QUERY_PARAM.getPath()) .handler( - ctx -> - HttpServerTest.controller( - QUERY_PARAM, - () -> { - end( - ctx.response().setStatusCode(QUERY_PARAM.getStatus()), - ctx.request().query()); - return null; - })); + ctx -> { + HttpServerTest.controller(QUERY_PARAM, () -> null); + end(ctx.response().setStatusCode(QUERY_PARAM.getStatus()), ctx.request().query()); + }); router .route(REDIRECT.getPath()) .handler( - ctx -> - HttpServerTest.controller( - REDIRECT, - () -> { - end( - ctx.response() - .setStatusCode(REDIRECT.getStatus()) - .putHeader("location", REDIRECT.getBody())); - return null; - })); + ctx -> { + HttpServerTest.controller(REDIRECT, () -> null); + end( + ctx.response() + .setStatusCode(REDIRECT.getStatus()) + .putHeader("location", REDIRECT.getBody())); + }); router .route(ERROR.getPath()) .handler( - ctx -> - HttpServerTest.controller( - ERROR, - () -> { - end(ctx.response().setStatusCode(ERROR.getStatus()), ERROR.getBody()); - return null; - })); + ctx -> { + HttpServerTest.controller(ERROR, () -> null); + end(ctx.response().setStatusCode(ERROR.getStatus()), ERROR.getBody()); + }); router .route(EXCEPTION.getPath()) .handler( @@ -105,15 +91,12 @@ public void handle(RoutingContext ctx) { router .route("/path/:id/param") .handler( - ctx -> - HttpServerTest.controller( - PATH_PARAM, - () -> { - end( - ctx.response().setStatusCode(PATH_PARAM.getStatus()), - ctx.request().getParam("id")); - return null; - })); + ctx -> { + HttpServerTest.controller(PATH_PARAM, () -> null); + end( + ctx.response().setStatusCode(PATH_PARAM.getStatus()), + ctx.request().getParam("id")); + }); return router; } 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 8ca3fe373031..84b916664be5 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 @@ -131,6 +131,10 @@ abstract class HttpServerTest extends InstrumentationSpecification imple false } + boolean verifyServerSpanEndTime() { + return true + } + List> extraAttributes() { [] } @@ -502,6 +506,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) From ea57d6b6a5a3c19ac032e7b7e24bbc2f581e0146 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sun, 10 Oct 2021 14:09:06 -0700 Subject: [PATCH 2/4] Fix tomcat async spans --- .../instrumentation/tomcat/common/TomcatHelper.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHelper.java b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHelper.java index 8fe6f4986a8f..6e18ffe9f842 100644 --- a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHelper.java +++ b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatHelper.java @@ -9,6 +9,7 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.servlet.AppServerBridge; +import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer; import io.opentelemetry.javaagent.instrumentation.servlet.ServletHelper; import org.apache.coyote.Request; import org.apache.coyote.Response; @@ -32,7 +33,9 @@ public boolean shouldStart(Context parentContext, Request request) { } 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( From 5ee8e4b7c33c46f2f18def83682e79d0cada4ad9 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 11 Oct 2021 15:04:57 -0700 Subject: [PATCH 3/4] Preserve existing test controller behavior --- .../GrizzlyFilterchainServerTest.groovy | 12 ++- .../AbstractRatpackAsyncHttpServerTest.groovy | 18 ++--- ...AbstractRatpackForkedHttpServerTest.groovy | 20 ++--- .../AbstractRatpackHttpServerTest.groovy | 24 +++--- ...VertxRxCircuitBreakerHttpServerTest.groovy | 12 +-- .../server/VertxRxHttpServerTest.groovy | 18 +++-- ...VertxRxCircuitBreakerHttpServerTest.groovy | 12 +-- .../server/VertxRxHttpServerTest.groovy | 18 +++-- .../server/AbstractVertxHttpServerTest.groovy | 6 ++ .../java/server/AbstractVertxWebServer.java | 81 +++++++++++-------- 10 files changed, 133 insertions(+), 88 deletions(-) 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 64512004d984..c758d2790d14 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 span is ended inside of the controller span below + return false + } + void setUpTransport(FilterChain filterChain) { TCPNIOTransportBuilder transportBuilder = TCPNIOTransportBuilder.newInstance() .setOptimizedForMultiplexing(true) @@ -116,13 +122,11 @@ class GrizzlyFilterchainServerTest extends HttpServerTest implements .header("Content-Length", valueOf(responseParameters.getResponseBody().length)) responseParameters.fillHeaders(builder) HttpResponsePacket responsePacket = builder.build() - def response controller(responseParameters.getEndpoint()) { - response = HttpContent.builder(responsePacket) + ctx.write(HttpContent.builder(responsePacket) .content(wrap(ctx.getMemoryManager(), responseParameters.getResponseBody())) - .build() + .build()) } - ctx.write(response) } } return ctx.getStopAction() diff --git a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackAsyncHttpServerTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackAsyncHttpServerTest.groovy index 60f727c5a0e9..712edad6e686 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackAsyncHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackAsyncHttpServerTest.groovy @@ -37,8 +37,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer SUCCESS } then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(endpoint.body) } - context.response.status(endpoint.status).send(endpoint.body) } } } @@ -49,8 +49,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer } then { controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { context.request.queryParams.get(it) } + context.response.status(INDEXED_CHILD.status).send() } - context.response.status(INDEXED_CHILD.status).send() } } } @@ -60,8 +60,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer QUERY_PARAM } then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(context.request.query) } - context.response.status(endpoint.status).send(context.request.query) } } } @@ -71,8 +71,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer REDIRECT } then { endpoint -> controller(endpoint) { + context.redirect(endpoint.body) } - context.redirect(endpoint.body) } } } @@ -82,8 +82,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer ERROR } then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(endpoint.body) } - context.response.status(endpoint.status).send(endpoint.body) } } } @@ -104,8 +104,8 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer PATH_PARAM } then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(context.pathTokens.id) } - context.response.status(endpoint.status).send(context.pathTokens.id) } } } @@ -115,10 +115,10 @@ abstract class AbstractRatpackAsyncHttpServerTest extends AbstractRatpackHttpSer CAPTURE_HEADERS } then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status) + context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) + context.response.send(endpoint.body) } - context.response.status(endpoint.status) - context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) - context.response.send(endpoint.body) } } } diff --git a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackForkedHttpServerTest.groovy b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackForkedHttpServerTest.groovy index cf5aaff947fd..1bb6b1ba2103 100644 --- a/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackForkedHttpServerTest.groovy +++ b/instrumentation/ratpack-1.4/testing/src/main/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackForkedHttpServerTest.groovy @@ -45,8 +45,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe SUCCESS }.fork().then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(endpoint.body) } - context.response.status(endpoint.status).send(endpoint.body) } } } @@ -57,8 +57,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe }.fork().then { controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { context.request.queryParams.get(it) } + context.response.status(INDEXED_CHILD.status).send() } - context.response.status(INDEXED_CHILD.status).send() } } } @@ -68,8 +68,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe QUERY_PARAM }.fork().then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(context.request.query) } - context.response.status(endpoint.status).send(context.request.query) } } } @@ -79,8 +79,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe REDIRECT }.fork().then { endpoint -> controller(endpoint) { + context.redirect(endpoint.body) } - context.redirect(endpoint.body) } } } @@ -90,8 +90,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe ERROR }.fork().then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(endpoint.body) } - context.response.status(endpoint.status).send(endpoint.body) } } } @@ -112,8 +112,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe PATH_PARAM }.fork().then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(context.pathTokens.id) } - context.response.status(endpoint.status).send(context.pathTokens.id) } } } @@ -123,10 +123,10 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe CAPTURE_HEADERS }.fork().then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status) + context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) + context.response.send(endpoint.body) } - context.response.status(endpoint.status) - context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) - context.response.send(endpoint.body) } } } @@ -141,8 +141,8 @@ abstract class AbstractRatpackForkedHttpServerTest extends AbstractRatpackHttpSe Promise.sync { list.get(0).value } } then { endpoint -> controller(endpoint) { + context.response.status(endpoint.status).send(endpoint.body) } - context.response.status(endpoint.status).send(endpoint.body) } } } 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 183fee6a0a61..123b085126f9 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 @@ -44,37 +44,37 @@ abstract class AbstractRatpackHttpServerTest extends HttpServerTest controller(SUCCESS) { + context.response.status(SUCCESS.status).send(SUCCESS.body) } - context.response.status(SUCCESS.status).send(SUCCESS.body) } } it.prefix(INDEXED_CHILD.rawPath()) { it.all { context -> controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { context.request.queryParams.get(it) } + context.response.status(INDEXED_CHILD.status).send() } - context.response.status(INDEXED_CHILD.status).send() } } it.prefix(QUERY_PARAM.rawPath()) { it.all { context -> controller(QUERY_PARAM) { + context.response.status(QUERY_PARAM.status).send(context.request.query) } - context.response.status(QUERY_PARAM.status).send(context.request.query) } } it.prefix(REDIRECT.rawPath()) { it.all { context -> controller(REDIRECT) { + context.redirect(REDIRECT.body) } - context.redirect(REDIRECT.body) } } it.prefix(ERROR.rawPath()) { it.all { context -> controller(ERROR) { + context.response.status(ERROR.status).send(ERROR.body) } - context.response.status(ERROR.status).send(ERROR.body) } } it.prefix(EXCEPTION.rawPath()) { @@ -87,17 +87,17 @@ abstract class AbstractRatpackHttpServerTest extends HttpServerTest controller(PATH_PARAM) { + context.response.status(PATH_PARAM.status).send(context.pathTokens.id) } - context.response.status(PATH_PARAM.status).send(context.pathTokens.id) } } it.prefix(CAPTURE_HEADERS.rawPath()) { it.all { context -> controller(CAPTURE_HEADERS) { + context.response.status(CAPTURE_HEADERS.status) + context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) + context.response.send(CAPTURE_HEADERS.body) } - context.response.status(CAPTURE_HEADERS.status) - context.response.headers.set("X-Test-Response", context.request.headers.get("X-Test-Request")) - context.response.send(CAPTURE_HEADERS.body) } } } @@ -138,6 +138,12 @@ abstract class AbstractRatpackHttpServerTest extends HttpServerTest @@ -64,8 +64,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { endpoint.collectSpanAttributes { ctx.request().params().get(it) } + ctx.response().setStatusCode(endpoint.status).end() } - ctx.response().setStatusCode(endpoint.status).end() }) } router.route(QUERY_PARAM.path).handler { ctx -> @@ -77,8 +77,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) } - ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) }) } router.route(REDIRECT.path).handler { ctx -> @@ -90,8 +90,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end() } - ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end() }) } router.route(ERROR.path).handler { ctx -> @@ -103,8 +103,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) } - ctx.response().setStatusCode(endpoint.status).end(endpoint.body) }) } router.route(EXCEPTION.path).handler { ctx -> @@ -130,8 +130,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(ctx.request().getParam("id")) } - ctx.response().setStatusCode(endpoint.status).end(ctx.request().getParam("id")) }) } 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 cbac3da68322..16c1e711b6e1 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 @@ -85,6 +85,12 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr return true } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the controller spans + return false + } + @Override List> extraAttributes() { return [ @@ -105,29 +111,29 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr router.route(SUCCESS.path).handler { ctx -> controller(SUCCESS) { + ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } - ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } router.route(INDEXED_CHILD.path).handler { ctx -> controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { ctx.request().params().get(it) } + ctx.response().setStatusCode(INDEXED_CHILD.status).end() } - ctx.response().setStatusCode(INDEXED_CHILD.status).end() } router.route(QUERY_PARAM.path).handler { ctx -> controller(QUERY_PARAM) { + ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) } - ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) } router.route(REDIRECT.path).handler { ctx -> controller(REDIRECT) { + ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() } - ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() } router.route(ERROR.path).handler { ctx -> controller(ERROR) { + ctx.response().setStatusCode(ERROR.status).end(ERROR.body) } - ctx.response().setStatusCode(ERROR.status).end(ERROR.body) } router.route(EXCEPTION.path).handler { ctx -> controller(EXCEPTION) { @@ -136,8 +142,8 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr } router.route("/path/:id/param").handler { ctx -> controller(PATH_PARAM) { + ctx.response().setStatusCode(PATH_PARAM.status).end(ctx.request().getParam("id")) } - ctx.response().setStatusCode(PATH_PARAM.status).end(ctx.request().getParam("id")) } super.@vertx.createHttpServer() diff --git a/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy b/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy index 75699e37b3a0..f413e8206600 100644 --- a/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy +++ b/instrumentation/vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy @@ -50,8 +50,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) } - ctx.response().setStatusCode(endpoint.status).end(endpoint.body) }) } router.route(INDEXED_CHILD.path).handler { ctx -> @@ -64,8 +64,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { endpoint.collectSpanAttributes { ctx.request().params().get(it) } + ctx.response().setStatusCode(endpoint.status).end() } - ctx.response().setStatusCode(endpoint.status).end() }) } router.route(QUERY_PARAM.path).handler { ctx -> @@ -77,8 +77,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) } - ctx.response().setStatusCode(endpoint.status).end(ctx.request().query()) }) } router.route(REDIRECT.path).handler { ctx -> @@ -90,8 +90,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end() } - ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end() }) } router.route(ERROR.path).handler { ctx -> @@ -103,8 +103,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(endpoint.body) } - ctx.response().setStatusCode(endpoint.status).end(endpoint.body) }) } router.route(EXCEPTION.path).handler { ctx -> @@ -130,8 +130,8 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest { } HttpServerTest.ServerEndpoint endpoint = it.result() controller(endpoint) { + ctx.response().setStatusCode(endpoint.status).end(ctx.request().getParam("id")) } - ctx.response().setStatusCode(endpoint.status).end(ctx.request().getParam("id")) }) } 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 1f3e3d34d509..87ba0a57f648 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 @@ -85,6 +85,12 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr return true } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the controller spans + return false + } + @Override List> extraAttributes() { return [ @@ -105,29 +111,29 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr router.route(SUCCESS.path).handler { ctx -> controller(SUCCESS) { + ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } - ctx.response().setStatusCode(SUCCESS.status).end(SUCCESS.body) } router.route(INDEXED_CHILD.path).handler { ctx -> controller(INDEXED_CHILD) { INDEXED_CHILD.collectSpanAttributes { ctx.request().params().get(it) } + ctx.response().setStatusCode(INDEXED_CHILD.status).end() } - ctx.response().setStatusCode(INDEXED_CHILD.status).end() } router.route(QUERY_PARAM.path).handler { ctx -> controller(QUERY_PARAM) { + ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) } - ctx.response().setStatusCode(QUERY_PARAM.status).end(ctx.request().query()) } router.route(REDIRECT.path).handler { ctx -> controller(REDIRECT) { + ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() } - ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end() } router.route(ERROR.path).handler { ctx -> controller(ERROR) { + ctx.response().setStatusCode(ERROR.status).end(ERROR.body) } - ctx.response().setStatusCode(ERROR.status).end(ERROR.body) } router.route(EXCEPTION.path).handler { ctx -> controller(EXCEPTION) { @@ -136,8 +142,8 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr } router.route("/path/:id/param").handler { ctx -> controller(PATH_PARAM) { + ctx.response().setStatusCode(PATH_PARAM.status).end(ctx.request().getParam("id")) } - ctx.response().setStatusCode(PATH_PARAM.status).end(ctx.request().getParam("id")) } 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 f86a024a9fc6..5e38164fe0f8 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 @@ -65,6 +65,12 @@ abstract class AbstractVertxHttpServerTest extends HttpServerTest impleme false } + @Override + boolean verifyServerSpanEndTime() { + // server spans are ended inside of the controller spans + return false + } + @Override List> extraAttributes() { return [ diff --git a/instrumentation/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java b/instrumentation/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java index 4ed07e27b4d5..251f5bc836f6 100644 --- a/instrumentation/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java +++ b/instrumentation/vertx-web-3.0/testing/src/main/java/server/AbstractVertxWebServer.java @@ -39,46 +39,60 @@ public Router buildRouter() { new Handler() { @Override public void handle(RoutingContext ctx) { - HttpServerTest.controller(SUCCESS, () -> null); - end(ctx.response().setStatusCode(SUCCESS.getStatus()), SUCCESS.getBody()); + HttpServerTest.controller( + SUCCESS, + () -> { + end(ctx.response().setStatusCode(SUCCESS.getStatus()), SUCCESS.getBody()); + return null; + }); } }); router .route(INDEXED_CHILD.getPath()) .handler( - ctx -> { - HttpServerTest.controller( - INDEXED_CHILD, - () -> { - INDEXED_CHILD.collectSpanAttributes(it -> ctx.request().getParam(it)); - return null; - }); - end(ctx.response().setStatusCode(INDEXED_CHILD.getStatus())); - }); + ctx -> + HttpServerTest.controller( + INDEXED_CHILD, + () -> { + INDEXED_CHILD.collectSpanAttributes(it -> ctx.request().getParam(it)); + end(ctx.response().setStatusCode(INDEXED_CHILD.getStatus())); + return null; + })); router .route(QUERY_PARAM.getPath()) .handler( - ctx -> { - HttpServerTest.controller(QUERY_PARAM, () -> null); - end(ctx.response().setStatusCode(QUERY_PARAM.getStatus()), ctx.request().query()); - }); + ctx -> + HttpServerTest.controller( + QUERY_PARAM, + () -> { + end( + ctx.response().setStatusCode(QUERY_PARAM.getStatus()), + ctx.request().query()); + return null; + })); router .route(REDIRECT.getPath()) .handler( - ctx -> { - HttpServerTest.controller(REDIRECT, () -> null); - end( - ctx.response() - .setStatusCode(REDIRECT.getStatus()) - .putHeader("location", REDIRECT.getBody())); - }); + ctx -> + HttpServerTest.controller( + REDIRECT, + () -> { + end( + ctx.response() + .setStatusCode(REDIRECT.getStatus()) + .putHeader("location", REDIRECT.getBody())); + return null; + })); router .route(ERROR.getPath()) .handler( - ctx -> { - HttpServerTest.controller(ERROR, () -> null); - end(ctx.response().setStatusCode(ERROR.getStatus()), ERROR.getBody()); - }); + ctx -> + HttpServerTest.controller( + ERROR, + () -> { + end(ctx.response().setStatusCode(ERROR.getStatus()), ERROR.getBody()); + return null; + })); router .route(EXCEPTION.getPath()) .handler( @@ -91,12 +105,15 @@ public void handle(RoutingContext ctx) { router .route("/path/:id/param") .handler( - ctx -> { - HttpServerTest.controller(PATH_PARAM, () -> null); - end( - ctx.response().setStatusCode(PATH_PARAM.getStatus()), - ctx.request().getParam("id")); - }); + ctx -> + HttpServerTest.controller( + PATH_PARAM, + () -> { + end( + ctx.response().setStatusCode(PATH_PARAM.getStatus()), + ctx.request().getParam("id")); + return null; + })); return router; } From 81cb6cf1c7216e0d74d3fba996d97633ba98e9a9 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Mon, 11 Oct 2021 15:08:01 -0700 Subject: [PATCH 4/4] Comments --- .../src/test/groovy/DropwizardAsyncTest.groovy | 2 +- .../javaagent/src/test/groovy/GrizzlyAsyncTest.groovy | 2 +- .../src/test/groovy/GrizzlyFilterchainServerTest.groovy | 2 +- .../javaagent/src/test/groovy/server/PlayServerTest.groovy | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy index 8f3ff49c4178..e0120ba50318 100644 --- a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy +++ b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy @@ -38,7 +38,7 @@ class DropwizardAsyncTest extends DropwizardTest { @Override boolean verifyServerSpanEndTime() { - // SERVER span is ended inside of JAX-RS controller span + // server spans are ended inside of the JAX-RS controller spans return false } 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 efa8c3090d16..1d5ae00bae67 100644 --- a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyAsyncTest.groovy +++ b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyAsyncTest.groovy @@ -40,7 +40,7 @@ class GrizzlyAsyncTest extends GrizzlyTest { @Override boolean verifyServerSpanEndTime() { - // SERVER span is ended inside of JAX-RS controller span + // server spans are ended inside of the JAX-RS controller spans return false } 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 c758d2790d14..c5568524f640 100644 --- a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy +++ b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy @@ -72,7 +72,7 @@ class GrizzlyFilterchainServerTest extends HttpServerTest implements @Override boolean verifyServerSpanEndTime() { - // SERVER span is ended inside of the controller span below + // server spans are ended inside of the controller spans return false } 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 5a8916137835..0032c22c0dc4 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 @@ -89,7 +89,7 @@ class PlayServerTest extends HttpServerTest implements AgentTestTrait { @Override boolean verifyServerSpanEndTime() { - // TODO (trask) see if the play controller instrumentation can be ended before netty server span + // server spans are ended inside of the controller spans return false }