From e60f9ba6599d23b141bbab9d4e240148326daa18 Mon Sep 17 00:00:00 2001 From: Jan Martiska Date: Fri, 9 Sep 2022 09:35:26 +0200 Subject: [PATCH] For metrics, count JAX-RS requests ending with 500 as unsuccessful (all other codes as successful) --- .../smallrye/metrics/jaxrs/JaxRsMetricsTestCase.java | 6 ++++-- .../quarkus/smallrye/metrics/jaxrs/MetricsResource.java | 2 +- .../metrics/runtime/QuarkusRestEasyMetricsFilter.java | 9 ++++++--- .../metrics/runtime/QuarkusRestMetricsFilter.java | 9 ++++----- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/extensions/smallrye-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/JaxRsMetricsTestCase.java b/extensions/smallrye-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/JaxRsMetricsTestCase.java index f64e2d1ccea06..c7ff7722a78c8 100644 --- a/extensions/smallrye-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/JaxRsMetricsTestCase.java +++ b/extensions/smallrye-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/JaxRsMetricsTestCase.java @@ -51,7 +51,9 @@ public void testMethodReturningServerError() throws InterruptedException { when() .get("/error") .then() - .statusCode(500); + .statusCode(501); + // we only consider requests ending with a 500 as "unsuccessful", + // so a 501 should count towards successful SimpleTimer metric = metricRegistry.simpleTimer("REST.request", new Tag("class", METRIC_RESOURCE_CLASS_NAME), new Tag("method", "error")); @@ -71,7 +73,7 @@ public void testMethodThrowingException() { assertEquals(0, metric.getCount()); assertEquals(0, metric.getElapsedTime().toNanos()); - // calls throwing an unmapped exception should only be reflected in the REST.request.unmappedException.total metric + // calls ending with status code 500 should only be reflected in the REST.request.unmappedException.total metric Counter exceptionCounter = metricRegistry.counter("REST.request.unmappedException.total", new Tag("class", METRIC_RESOURCE_CLASS_NAME), new Tag("method", "exception")); diff --git a/extensions/smallrye-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/MetricsResource.java b/extensions/smallrye-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/MetricsResource.java index 5090b369d0b5f..04122ffcdba25 100644 --- a/extensions/smallrye-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/MetricsResource.java +++ b/extensions/smallrye-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/MetricsResource.java @@ -22,7 +22,7 @@ public String hello(@PathParam("name") String name) { @Path("/error") @GET public Response error() { - return Response.serverError().build(); + return Response.status(501).build(); } @Path("/exception") diff --git a/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestEasyMetricsFilter.java b/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestEasyMetricsFilter.java index 474a8dae954d4..84968332ca457 100644 --- a/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestEasyMetricsFilter.java +++ b/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestEasyMetricsFilter.java @@ -28,6 +28,7 @@ import jakarta.ws.rs.container.ContainerResponseFilter; import jakarta.ws.rs.container.ResourceInfo; import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.Response; import io.quarkus.vertx.http.runtime.CurrentVertxRequest; import io.vertx.ext.web.RoutingContext; @@ -63,9 +64,11 @@ public void filter(final ContainerRequestContext requestContext) { @Override public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) { - // mark the request as successful if it finished without exception or with a mapped exception - // if it ended with an unmapped exception, this filter is not called by RESTEasy - requestContext.setProperty("smallrye.metrics.jaxrs.successful", true); + Response.StatusType statusInfo = responseContext.getStatusInfo(); + // consider all requests ending with 500 as unsuccessful, and anything else as successful + if (statusInfo != null && statusInfo.getStatusCode() != 500) { + requestContext.setProperty("smallrye.metrics.jaxrs.successful", true); + } } } diff --git a/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestMetricsFilter.java b/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestMetricsFilter.java index f720c5c9539e2..dee84c3521a3a 100644 --- a/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestMetricsFilter.java +++ b/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestMetricsFilter.java @@ -4,6 +4,7 @@ import java.lang.reflect.Method; +import jakarta.ws.rs.container.ContainerResponseContext; import jakarta.ws.rs.container.ResourceInfo; import org.jboss.resteasy.reactive.server.ServerResponseFilter; @@ -16,7 +17,7 @@ public class QuarkusRestMetricsFilter { @ServerResponseFilter - public void filter(ResourceInfo resourceInfo, Throwable throwable) { + public void filter(ResourceInfo resourceInfo, ContainerResponseContext responseContext) { Class resourceClass = resourceInfo.getResourceClass(); Method resourceMethod = resourceInfo.getResourceMethod(); if ((resourceClass == null) || (resourceMethod == null)) { @@ -26,9 +27,7 @@ public void filter(ResourceInfo resourceInfo, Throwable throwable) { FilterUtil.finishRequest(System.nanoTime(), resourceInfo.getResourceClass(), resourceInfo.getResourceMethod().getName(), resourceInfo.getResourceMethod().getParameterTypes(), - // FIXME: we need to know whether the exception is mapped or not, how to find out? - // for now let's assume all are unmapped, and therefore if there was an exception, - // increment the failure counter rather than the successful calls counter - () -> throwable == null); + // consider all requests ending with 500 as unsuccessful, and anything else as successful + () -> responseContext.getStatus() != 500); } }