diff --git a/extensions/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/standalone/VertxRequestHandler.java b/extensions/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/standalone/VertxRequestHandler.java index 77f28738c2e7c0..d57c8a4563e8de 100644 --- a/extensions/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/standalone/VertxRequestHandler.java +++ b/extensions/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/standalone/VertxRequestHandler.java @@ -72,16 +72,19 @@ public void handle(RoutingContext request) { vertx.executeBlocking(event -> { dispatch(request, is, new VertxBlockingOutput(request.request())); }, false, event -> { + if (event.failed()) { + request.fail(event.cause()); + } }); } private void dispatch(RoutingContext routingContext, InputStream is, VertxOutput output) { + ManagedContext requestContext = beanContainer.requestContext(); + requestContext.activate(); QuarkusHttpUser user = (QuarkusHttpUser) routingContext.user(); if (user != null && association != null) { association.setIdentity(user.getSecurityIdentity()); } - ManagedContext requestContext = beanContainer.requestContext(); - requestContext.activate(); currentVertxRequest.setCurrent(routingContext); try { Context ctx = vertx.getOrCreateContext(); @@ -111,15 +114,23 @@ private void dispatch(RoutingContext routingContext, InputStream is, VertxOutput } catch (Throwable ex) { routingContext.fail(ex); } + boolean suspended = vertxRequest.getAsyncContext().isSuspended(); - currentVertxRequest.initialInvocationComplete(suspended); + boolean requestContextActive = requestContext.isActive(); + if (requestContextActive) { + //it is possible that there was an async response, that then finished in the same thread + //the async response will have terminated the request context in this case + currentVertxRequest.initialInvocationComplete(suspended); + } if (!suspended) { try { vertxResponse.finish(); } catch (IOException e) { log.error("Unexpected failure", e); } finally { - requestContext.terminate(); + if (requestContextActive) { + requestContext.terminate(); + } } } else { //we need the request context to stick around @@ -129,7 +140,9 @@ private void dispatch(RoutingContext routingContext, InputStream is, VertxOutput try { routingContext.fail(t); } finally { - requestContext.terminate(); + if (requestContext.isActive()) { + requestContext.terminate(); + } } } } diff --git a/extensions/smallrye-opentracing/deployment/pom.xml b/extensions/smallrye-opentracing/deployment/pom.xml index f96c0923ac6bc5..704196b321f8ed 100644 --- a/extensions/smallrye-opentracing/deployment/pom.xml +++ b/extensions/smallrye-opentracing/deployment/pom.xml @@ -22,10 +22,6 @@ io.quarkus quarkus-resteasy-deployment - - io.quarkus - quarkus-undertow-deployment - io.quarkus quarkus-arc-deployment diff --git a/extensions/smallrye-opentracing/deployment/src/main/java/io/quarkus/smallrye/opentracing/deployment/SmallRyeOpenTracingProcessor.java b/extensions/smallrye-opentracing/deployment/src/main/java/io/quarkus/smallrye/opentracing/deployment/SmallRyeOpenTracingProcessor.java index 23ff2fd62f32b3..26306b4c2b6efc 100644 --- a/extensions/smallrye-opentracing/deployment/src/main/java/io/quarkus/smallrye/opentracing/deployment/SmallRyeOpenTracingProcessor.java +++ b/extensions/smallrye-opentracing/deployment/src/main/java/io/quarkus/smallrye/opentracing/deployment/SmallRyeOpenTracingProcessor.java @@ -8,12 +8,14 @@ import io.opentracing.contrib.interceptors.OpenTracingInterceptor; import io.opentracing.contrib.jaxrs2.server.SpanFinishingFilter; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; +import io.quarkus.deployment.Capabilities; import io.quarkus.deployment.annotations.BuildProducer; import io.quarkus.deployment.annotations.BuildStep; import io.quarkus.deployment.builditem.FeatureBuildItem; import io.quarkus.deployment.builditem.nativeimage.ReflectiveMethodBuildItem; import io.quarkus.resteasy.common.spi.ResteasyJaxrsProviderBuildItem; import io.quarkus.smallrye.opentracing.runtime.QuarkusSmallRyeTracingDynamicFeature; +import io.quarkus.smallrye.opentracing.runtime.QuarkusSmallRyeTracingStandaloneVertxDynamicFeature; import io.quarkus.smallrye.opentracing.runtime.TracerProducer; import io.quarkus.undertow.deployment.FilterBuildItem; @@ -33,21 +35,28 @@ ReflectiveMethodBuildItem registerMethod() throws Exception { @BuildStep void setupFilter(BuildProducer providers, BuildProducer filterProducer, - BuildProducer feature) { + BuildProducer feature, + Capabilities capabilities) { feature.produce(new FeatureBuildItem(FeatureBuildItem.SMALLRYE_OPENTRACING)); providers.produce(new ResteasyJaxrsProviderBuildItem(QuarkusSmallRyeTracingDynamicFeature.class.getName())); - FilterBuildItem filterInfo = FilterBuildItem.builder("tracingFilter", SpanFinishingFilter.class.getName()) - .setAsyncSupported(true) - .addFilterUrlMapping("*", DispatcherType.FORWARD) - .addFilterUrlMapping("*", DispatcherType.INCLUDE) - .addFilterUrlMapping("*", DispatcherType.REQUEST) - .addFilterUrlMapping("*", DispatcherType.ASYNC) - .addFilterUrlMapping("*", DispatcherType.ERROR) - .build(); - filterProducer.produce(filterInfo); + if (capabilities.isCapabilityPresent(Capabilities.SERVLET)) { + FilterBuildItem filterInfo = FilterBuildItem.builder("tracingFilter", SpanFinishingFilter.class.getName()) + .setAsyncSupported(true) + .addFilterUrlMapping("*", DispatcherType.FORWARD) + .addFilterUrlMapping("*", DispatcherType.INCLUDE) + .addFilterUrlMapping("*", DispatcherType.REQUEST) + .addFilterUrlMapping("*", DispatcherType.ASYNC) + .addFilterUrlMapping("*", DispatcherType.ERROR) + .build(); + filterProducer.produce(filterInfo); + } else { + //otherwise we know we have RESTeasy on vert.x + providers.produce( + new ResteasyJaxrsProviderBuildItem(QuarkusSmallRyeTracingStandaloneVertxDynamicFeature.class.getName())); + } } } diff --git a/extensions/smallrye-opentracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracerRegistrar.java b/extensions/smallrye-opentracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracerRegistrar.java index 3e86ea8b8d2161..2b8705eeca5086 100644 --- a/extensions/smallrye-opentracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracerRegistrar.java +++ b/extensions/smallrye-opentracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracerRegistrar.java @@ -1,16 +1,15 @@ package io.quarkus.smallrye.opentracing.deployment; -import javax.servlet.ServletContextEvent; -import javax.servlet.ServletContextListener; -import javax.servlet.annotation.WebListener; +import javax.enterprise.context.ApplicationScoped; +import javax.enterprise.event.Observes; import io.opentracing.util.GlobalTracer; +import io.quarkus.runtime.StartupEvent; -@WebListener -public class TracerRegistrar implements ServletContextListener { +@ApplicationScoped +public class TracerRegistrar { - @Override - public void contextInitialized(ServletContextEvent sce) { + public void start(@Observes StartupEvent start) { GlobalTracer.register(TracingTest.mockTracer); } } diff --git a/extensions/smallrye-opentracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracingTest.java b/extensions/smallrye-opentracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracingTest.java index 9ab1d26d7b9aca..1fb5563107fb75 100644 --- a/extensions/smallrye-opentracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracingTest.java +++ b/extensions/smallrye-opentracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracingTest.java @@ -1,7 +1,9 @@ package io.quarkus.smallrye.opentracing.deployment; import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import org.awaitility.Awaitility; import org.jboss.shrinkwrap.api.ShrinkWrap; @@ -46,13 +48,15 @@ public static void afterAll() { } @Test - public void testSingleServerRequest() { + public void testSingleServerRequest() throws InterruptedException { try { RestAssured.defaultParser = Parser.TEXT; RestAssured.when().get("/hello") .then() .statusCode(200); - Assertions.assertEquals(1, mockTracer.finishedSpans().size()); + //inherently racy, tracer is completed after response is sent back to the client + Awaitility.await().atMost(5, TimeUnit.SECONDS) + .until(() -> mockTracer.finishedSpans().size() == 1); Assertions.assertEquals("GET:io.quarkus.smallrye.opentracing.deployment.TestResource.hello", mockTracer.finishedSpans().get(0).operationName()); } finally { @@ -61,12 +65,15 @@ public void testSingleServerRequest() { } @Test - public void testCDI() { + public void testCDI() throws InterruptedException { try { RestAssured.defaultParser = Parser.TEXT; RestAssured.when().get("/cdi") .then() .statusCode(200); + //inherently racy, tracer is completed after response is sent back to the client + Awaitility.await().atMost(5, TimeUnit.SECONDS) + .until(() -> mockTracer.finishedSpans().size() == 2); Assertions.assertEquals(2, mockTracer.finishedSpans().size()); Assertions.assertEquals("io.quarkus.smallrye.opentracing.deployment.Service.foo", mockTracer.finishedSpans().get(0).operationName()); @@ -78,18 +85,24 @@ public void testCDI() { } @Test - public void testMPRestClient() { + public void testMPRestClient() throws InterruptedException { try { RestAssured.defaultParser = Parser.TEXT; RestAssured.when().get("/restClient") .then() .statusCode(200); + //inherently racy, tracer is completed after response is sent back to the client + Awaitility.await().atMost(5, TimeUnit.SECONDS) + .until(() -> mockTracer.finishedSpans().size() == 3); Assertions.assertEquals(3, mockTracer.finishedSpans().size()); - Assertions.assertEquals("GET:io.quarkus.smallrye.opentracing.deployment.TestResource.hello", - mockTracer.finishedSpans().get(0).operationName()); - Assertions.assertEquals("GET", mockTracer.finishedSpans().get(1).operationName()); - Assertions.assertEquals("GET:io.quarkus.smallrye.opentracing.deployment.TestResource.restClient", - mockTracer.finishedSpans().get(2).operationName()); + //these can come in any order, as the 'hello' span is finished after the request is sent back. + //this means the client might have already dealt with the response before the hello span is finished + //in practice this means that the spans might be in any order, as they are ordered by the time + //they are completed rather than the time they are started + Set results = mockTracer.finishedSpans().stream().map(MockSpan::operationName).collect(Collectors.toSet()); + Assertions.assertTrue(results.contains("GET:io.quarkus.smallrye.opentracing.deployment.TestResource.hello")); + Assertions.assertTrue(results.contains("GET")); + Assertions.assertTrue(results.contains("GET:io.quarkus.smallrye.opentracing.deployment.TestResource.restClient")); } finally { RestAssured.reset(); } diff --git a/extensions/smallrye-opentracing/runtime/pom.xml b/extensions/smallrye-opentracing/runtime/pom.xml index f68c44a410a232..3b4b099c8491bd 100644 --- a/extensions/smallrye-opentracing/runtime/pom.xml +++ b/extensions/smallrye-opentracing/runtime/pom.xml @@ -46,7 +46,7 @@ io.quarkus - quarkus-undertow + quarkus-resteasy io.quarkus diff --git a/extensions/smallrye-opentracing/runtime/src/main/java/io/quarkus/smallrye/opentracing/runtime/QuarkusSmallRyeTracingStandaloneVertxDynamicFeature.java b/extensions/smallrye-opentracing/runtime/src/main/java/io/quarkus/smallrye/opentracing/runtime/QuarkusSmallRyeTracingStandaloneVertxDynamicFeature.java new file mode 100644 index 00000000000000..2ce0f0b01aa9d0 --- /dev/null +++ b/extensions/smallrye-opentracing/runtime/src/main/java/io/quarkus/smallrye/opentracing/runtime/QuarkusSmallRyeTracingStandaloneVertxDynamicFeature.java @@ -0,0 +1,79 @@ +package io.quarkus.smallrye.opentracing.runtime; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import javax.enterprise.inject.spi.CDI; +import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.container.ContainerRequestFilter; +import javax.ws.rs.container.DynamicFeature; +import javax.ws.rs.container.ResourceInfo; +import javax.ws.rs.core.FeatureContext; +import javax.ws.rs.ext.Provider; + +import io.opentracing.Span; +import io.opentracing.contrib.jaxrs2.internal.SpanWrapper; +import io.opentracing.tag.Tags; +import io.quarkus.vertx.http.runtime.CurrentVertxRequest; +import io.vertx.ext.web.RoutingContext; + +@Provider +public class QuarkusSmallRyeTracingStandaloneVertxDynamicFeature implements DynamicFeature { + + @Override + public void configure(ResourceInfo resourceInfo, FeatureContext context) { + context.register(StandaloneFilter.class); + } + + public static class StandaloneFilter implements ContainerRequestFilter { + + volatile CurrentVertxRequest currentVertxRequest; + + CurrentVertxRequest request() { + if (currentVertxRequest == null) { + currentVertxRequest = CDI.current().select(CurrentVertxRequest.class).get(); + } + return currentVertxRequest; + } + + @Override + public void filter(ContainerRequestContext requestContext) throws IOException { + request().addRequestDoneListener(new CurrentVertxRequest.Listener() { + @Override + public void initialInvocationComplete(RoutingContext routingContext, boolean goingAsync) { + SpanWrapper wrapper = routingContext.get(SpanWrapper.PROPERTY_NAME); + if (wrapper != null) { + wrapper.getScope().close(); + } + } + + @Override + public void responseComplete(RoutingContext routingContext) { + SpanWrapper wrapper = routingContext.get(SpanWrapper.PROPERTY_NAME); + if (wrapper == null) { + return; + } + + Tags.HTTP_STATUS.set(wrapper.get(), routingContext.response().getStatusCode()); + if (routingContext.failure() != null) { + addExceptionLogs(wrapper.get(), routingContext.failure()); + } + wrapper.finish(); + + } + }); + + } + + private static void addExceptionLogs(Span span, Throwable throwable) { + Tags.ERROR.set(span, true); + if (throwable != null) { + Map errorLogs = new HashMap<>(2); + errorLogs.put("event", Tags.ERROR.getKey()); + errorLogs.put("error.object", throwable); + span.log(errorLogs); + } + } + } +}