For metrics, count JAX-RS requests ending with 500 as unsuccessful (a…

…ll other codes as successful)
aloubyansky · Sep 9, 2022 · e60f9ba · e60f9ba
1 parent 9134fa8
commit e60f9ba
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 11 deletions.
diff --git a/...rics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/JaxRsMetricsTestCase.java b/...rics/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/...e-metrics/deployment/src/test/java/io/quarkus/smallrye/metrics/jaxrs/MetricsResource.java b/...e-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/...ntime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestEasyMetricsFilter.java b/...ntime/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/...s/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/QuarkusRestMetricsFilter.java b/...s/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);
     }
 }