From b53546a02e85f6347502bdef4b728083e0bd01d2 Mon Sep 17 00:00:00 2001 From: Denny Abraham Cheriyan Date: Wed, 21 Dec 2022 18:01:31 -0800 Subject: [PATCH] Add ResponseMeteredLevel --- docs/source/manual/jersey.rst | 4 +- .../metrics/annotation/ResponseMetered.java | 11 ++- ...ntedResourceMethodApplicationListener.java | 44 ++++++--- .../jersey2/SingletonMetricsJerseyTest.java | 96 ++++++++++--------- ...ricsResponseMeteredPerClassJerseyTest.java | 38 +++----- .../resources/InstrumentedResource.java | 24 +++-- ...mentedResourceResponseMeteredPerClass.java | 10 +- ...tedSubResourceResponseMeteredPerClass.java | 6 +- 8 files changed, 127 insertions(+), 106 deletions(-) diff --git a/docs/source/manual/jersey.rst b/docs/source/manual/jersey.rst index 609454c138..1e47a13b1b 100644 --- a/docs/source/manual/jersey.rst +++ b/docs/source/manual/jersey.rst @@ -56,7 +56,7 @@ application's ``ResourceConfig`` as a singleton provider for this to work. } @GET - @ResponseMetered + @ResponseMetered(level = ResponseMeteredLevel.ALL) @Path("/response-metered") public Response responseMetered(@QueryParam("invalid") @DefaultValue("false") boolean invalid) { if (invalid) { @@ -74,6 +74,6 @@ If the annotation is placed on the class, it will apply to all its resource meth * ``@Timed`` adds a timer and measures time spent in that method. * ``@Metered`` adds a meter and measures the rate at which the resource method is accessed. -* ``@ResponseMetered`` adds a meter and measures rate for each class of response codes (1xx/2xx/3xx/4xx/5xx). +* ``@ResponseMetered`` adds meters and measures rate for response codes based on the selected level. * ``@ExceptionMetered`` adds a meter and measures how often the specified exception occurs when processing the resource. If the ``cause`` is not specified, the default is ``Exception.class``. diff --git a/metrics-annotation/src/main/java/com/codahale/metrics/annotation/ResponseMetered.java b/metrics-annotation/src/main/java/com/codahale/metrics/annotation/ResponseMetered.java index cb3274463c..ea5d8766d5 100644 --- a/metrics-annotation/src/main/java/com/codahale/metrics/annotation/ResponseMetered.java +++ b/metrics-annotation/src/main/java/com/codahale/metrics/annotation/ResponseMetered.java @@ -12,14 +12,14 @@ *

* Given a method like this: * 


- *     {@literal @}ResponseMetered(name = "fancyName")
+ *     {@literal @}ResponseMetered(name = "fancyName", level = ResponseMeteredLevel.ALL)
  *     public String fancyName(String name) {
  *         return "Sir Captain " + name;
  *     }
  *
*

- * A meter for the defining class with the name {@code fancyName} will be created for every response code - * in addition to 1xx/2xx/3xx/4xx/5xx responses. Each time the {@code #fancyName(String)} method is invoked, + * Meters for the defining class with the name {@code fancyName} will be created for response codes + * based on the ResponseMeteredLevel selected. Each time the {@code #fancyName(String)} method is invoked, * the appropriate response meter will be marked. */ @Inherited @@ -37,4 +37,9 @@ * relative to the annotated class. When annotating a class, this must be {@code false}. */ boolean absolute() default false; + + /** + * @return the ResponseMeteredLevel which decides which response code meters are marked. + */ + ResponseMeteredLevel level() default ResponseMeteredLevel.COARSE; } diff --git a/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java b/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java index 7e4667c60b..28ff2d666d 100644 --- a/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java +++ b/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java @@ -9,6 +9,7 @@ import com.codahale.metrics.annotation.ExceptionMetered; import com.codahale.metrics.annotation.Metered; import com.codahale.metrics.annotation.ResponseMetered; +import com.codahale.metrics.annotation.ResponseMeteredLevel; import com.codahale.metrics.annotation.Timed; import org.glassfish.jersey.server.ContainerResponse; import org.glassfish.jersey.server.model.ModelProcessor; @@ -28,12 +29,17 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.EnumSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import static com.codahale.metrics.MetricRegistry.name; +import static com.codahale.metrics.annotation.ResponseMeteredLevel.COARSE; +import static com.codahale.metrics.annotation.ResponseMeteredLevel.DETAILED; +import static com.codahale.metrics.annotation.ResponseMeteredLevel.ALL; /** * An application event listener that listens for Jersey application initialization to @@ -127,25 +133,44 @@ public ExceptionMeterMetric(final MetricRegistry registry, * different response codes */ private static class ResponseMeterMetric { + private static final Set COARSE_METER_LEVELS = EnumSet.of(COARSE, ALL); + private static final Set DETAILED_METER_LEVELS = EnumSet.of(DETAILED, ALL); public final List meters; - private final ConcurrentMap responseCodeMeters = new ConcurrentHashMap<>(); + private final Map responseCodeMeters; private final MetricRegistry metricRegistry; private final String metricName; + private final ResponseMeteredLevel level; public ResponseMeterMetric(final MetricRegistry registry, final ResourceMethod method, final ResponseMetered responseMetered) { this.metricName = chooseName(responseMetered.name(), responseMetered.absolute(), method); - this.meters = Collections.unmodifiableList(Arrays.asList( + this.level = responseMetered.level(); + this.meters = COARSE_METER_LEVELS.contains(level) ? + Collections.unmodifiableList(Arrays.asList( registry.meter(name(metricName, "1xx-responses")), // 1xx registry.meter(name(metricName, "2xx-responses")), // 2xx registry.meter(name(metricName, "3xx-responses")), // 3xx registry.meter(name(metricName, "4xx-responses")), // 4xx registry.meter(name(metricName, "5xx-responses")) // 5xx - )); + )) : Collections.emptyList(); + this.responseCodeMeters = DETAILED_METER_LEVELS.contains(level) ? new ConcurrentHashMap<>() : Collections.emptyMap(); this.metricRegistry = registry; } + public void mark(int statusCode) { + if (DETAILED_METER_LEVELS.contains(level)) { + getResponseCodeMeter(statusCode).mark(); + } + + if (COARSE_METER_LEVELS.contains(level)) { + final int responseStatus = statusCode / 100; + if (responseStatus >= 1 && responseStatus <= 5) { + meters.get(responseStatus - 1).mark(); + } + } + } + public Meter getResponseCodeMeter(int statusCode) { return responseCodeMeters .computeIfAbsent(statusCode, sc -> metricRegistry @@ -281,17 +306,10 @@ public void onEvent(RequestEvent event) { if (metric != null) { ContainerResponse containerResponse = event.getContainerResponse(); - if (containerResponse == null) { - if (event.getException() != null) { - metric.meters.get(4).mark(); - metric.getResponseCodeMeter(500).mark(); - } + if (containerResponse == null && event.getException() != null) { + metric.mark(500); } else { - final int responseStatus = containerResponse.getStatus() / 100; - if (responseStatus >= 1 && responseStatus <= 5) { - metric.meters.get(responseStatus - 1).mark(); - metric.getResponseCodeMeter(containerResponse.getStatus()).mark(); - } + metric.mark(containerResponse.getStatus()); } } } diff --git a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/SingletonMetricsJerseyTest.java b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/SingletonMetricsJerseyTest.java index 7c13385a12..bb1170395c 100644 --- a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/SingletonMetricsJerseyTest.java +++ b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/SingletonMetricsJerseyTest.java @@ -95,69 +95,73 @@ public void exceptionMeteredMethodsAreExceptionMetered() { } @Test - public void responseMeteredMethodsAreMetered() { + public void responseMeteredMethodsAreMeteredWithCoarseLevel() { final Meter meter2xx = registry.meter(name(InstrumentedResource.class, - "response2xxMetered", + "responseMeteredCoarse", "2xx-responses")); final Meter meter200 = registry.meter(name(InstrumentedResource.class, - "response2xxMetered", + "responseMeteredCoarse", "200-responses")); - final Meter meter4xx = registry.meter(name(InstrumentedResource.class, - "response4xxMetered", - "4xx-responses")); - final Meter meter400 = registry.meter(name(InstrumentedResource.class, - "response4xxMetered", - "400-responses")); - final Meter meter401 = registry.meter(name(InstrumentedResource.class, - "response4xxMetered", - "401-responses")); - final Meter meter5xx = registry.meter(name(InstrumentedResource.class, - "response5xxMetered", - "5xx-responses")); - final Meter meter500 = registry.meter(name(InstrumentedResource.class, - "response5xxMetered", - "500-responses")); - final Meter meter503 = registry.meter(name(InstrumentedResource.class, - "response5xxMetered", - "503-responses")); assertThat(meter2xx.getCount()).isZero(); - assertThat(target("response-2xx-metered") + assertThat(meter200.getCount()).isZero(); + assertThat(target("response-metered-coarse") .request() .get().getStatus()) .isEqualTo(200); - assertThat(meter4xx.getCount()).isZero(); - assertThat(target("response-4xx-metered") - .request() - .get().getStatus()) - .isEqualTo(400); + assertThat(meter2xx.getCount()).isOne(); + assertThat(meter200.getCount()).isZero(); + } - assertThat(target("response-4xx-metered") - .queryParam("status_code", 401) + @Test + public void responseMeteredMethodsAreMeteredWithDetailedLevel() { + final Meter meter2xx = registry.meter(name(InstrumentedResource.class, + "responseMeteredDetailed", + "2xx-responses")); + final Meter meter200 = registry.meter(name(InstrumentedResource.class, + "responseMeteredDetailed", + "200-responses")); + final Meter meter201 = registry.meter(name(InstrumentedResource.class, + "responseMeteredDetailed", + "201-responses")); + + assertThat(meter2xx.getCount()).isZero(); + assertThat(meter200.getCount()).isZero(); + assertThat(meter201.getCount()).isZero(); + assertThat(target("response-metered-detailed") .request() .get().getStatus()) - .isEqualTo(401); - - assertThat(meter5xx.getCount()).isZero(); - assertThat(target("response-5xx-metered") + .isEqualTo(200); + assertThat(target("response-metered-detailed") + .queryParam("status_code", 201) .request() .get().getStatus()) - .isEqualTo(500); - assertThat(target("response-5xx-metered") - .queryParam("status_code", 503) + .isEqualTo(201); + + assertThat(meter2xx.getCount()).isZero(); + assertThat(meter200.getCount()).isOne(); + assertThat(meter201.getCount()).isOne(); + } + + @Test + public void responseMeteredMethodsAreMeteredWithAllLevel() { + final Meter meter2xx = registry.meter(name(InstrumentedResource.class, + "responseMeteredAll", + "2xx-responses")); + final Meter meter200 = registry.meter(name(InstrumentedResource.class, + "responseMeteredAll", + "200-responses")); + + assertThat(meter2xx.getCount()).isZero(); + assertThat(meter200.getCount()).isZero(); + assertThat(target("response-metered-all") .request() .get().getStatus()) - .isEqualTo(503); - - assertThat(meter2xx.getCount()).isEqualTo(1); - assertThat(meter4xx.getCount()).isEqualTo(2); - assertThat(meter5xx.getCount()).isEqualTo(2); - assertThat(meter200.getCount()).isEqualTo(1); - assertThat(meter400.getCount()).isEqualTo(1); - assertThat(meter401.getCount()).isEqualTo(1); - assertThat(meter500.getCount()).isEqualTo(1); - assertThat(meter503.getCount()).isEqualTo(1); + .isEqualTo(200); + + assertThat(meter2xx.getCount()).isOne(); + assertThat(meter200.getCount()).isOne(); } @Test diff --git a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/SingletonMetricsResponseMeteredPerClassJerseyTest.java b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/SingletonMetricsResponseMeteredPerClassJerseyTest.java index 2554234a96..acb8153770 100644 --- a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/SingletonMetricsResponseMeteredPerClassJerseyTest.java +++ b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/SingletonMetricsResponseMeteredPerClassJerseyTest.java @@ -62,11 +62,6 @@ public void responseMetered4xxPerClassMethodsAreMetered() { .request() .get().getStatus()) .isEqualTo(400); - assertThat(target("responseMetered4xxPerClass") - .queryParam("status_code", 401) - .request() - .get().getStatus()) - .isEqualTo(401); assertThat(target("responseMeteredBadRequestPerClass") .request() .get().getStatus()) @@ -75,15 +70,11 @@ public void responseMetered4xxPerClassMethodsAreMetered() { final Meter meter4xx = registry.meter(name(InstrumentedResourceResponseMeteredPerClass.class, "responseMetered4xxPerClass", "4xx-responses")); - final Meter meter401 = registry.meter(name(InstrumentedResourceResponseMeteredPerClass.class, - "responseMetered4xxPerClass", - "401-responses")); final Meter meterException4xx = registry.meter(name(InstrumentedResourceResponseMeteredPerClass.class, "responseMeteredBadRequestPerClass", "4xx-responses")); - assertThat(meter4xx.getCount()).isEqualTo(2); - assertThat(meter401.getCount()).isEqualTo(1); + assertThat(meter4xx.getCount()).isEqualTo(1); assertThat(meterException4xx.getCount()).isEqualTo(1); } @@ -93,22 +84,12 @@ public void responseMetered5xxPerClassMethodsAreMetered() { .request() .get().getStatus()) .isEqualTo(500); - assertThat(target("responseMetered5xxPerClass") - .queryParam("status_code", 503) - .request() - .get().getStatus()) - .isEqualTo(503); final Meter meter5xx = registry.meter(name(InstrumentedResourceResponseMeteredPerClass.class, "responseMetered5xxPerClass", "5xx-responses")); - final Meter meter503 = registry.meter(name(InstrumentedResourceResponseMeteredPerClass.class, - "responseMetered5xxPerClass", - "503-responses")); - - assertThat(meter5xx.getCount()).isEqualTo(2); - assertThat(meter503.getCount()).isEqualTo(1); + assertThat(meter5xx.getCount()).isEqualTo(1); } @Test @@ -145,14 +126,21 @@ public void responseMeteredUnmappedExceptionPerClassMethodsAreMetered() { @Test public void subresourcesFromLocatorsRegisterMetrics() { + final Meter meter2xx = registry.meter(name(InstrumentedSubResourceResponseMeteredPerClass.class, + "responseMeteredPerClass", + "2xx-responses")); + final Meter meter201 = registry.meter(name(InstrumentedSubResourceResponseMeteredPerClass.class, + "responseMeteredPerClass", + "200-responses")); + + assertThat(meter2xx.getCount()).isZero(); + assertThat(meter2xx.getCount()).isZero(); assertThat(target("subresource/responseMeteredPerClass") .request() .get().getStatus()) .isEqualTo(200); - final Meter meter = registry.meter(name(InstrumentedSubResourceResponseMeteredPerClass.class, - "responseMeteredPerClass", - "2xx-responses")); - assertThat(meter.getCount()).isEqualTo(1); + assertThat(meter2xx.getCount()).isOne(); + assertThat(meter2xx.getCount()).isOne(); } } diff --git a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedResource.java b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedResource.java index 47ef945061..963ac82c63 100644 --- a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedResource.java +++ b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedResource.java @@ -14,6 +14,10 @@ import javax.ws.rs.core.Response; import java.io.IOException; +import static com.codahale.metrics.annotation.ResponseMeteredLevel.COARSE; +import static com.codahale.metrics.annotation.ResponseMeteredLevel.DETAILED; +import static com.codahale.metrics.annotation.ResponseMeteredLevel.ALL; + @Path("/") @Produces(MediaType.TEXT_PLAIN) public class InstrumentedResource { @@ -42,23 +46,23 @@ public String exceptionMetered(@QueryParam("splode") @DefaultValue("false") bool } @GET - @ResponseMetered - @Path("/response-2xx-metered") - public Response response2xxMetered() { - return Response.ok().build(); + @ResponseMetered(level = DETAILED) + @Path("/response-metered-detailed") + public Response responseMeteredDetailed(@QueryParam("status_code") @DefaultValue("200") int statusCode) { + return Response.status(Response.Status.fromStatusCode(statusCode)).build(); } @GET - @ResponseMetered - @Path("/response-4xx-metered") - public Response response4xxMetered(@QueryParam("status_code") @DefaultValue("400") int statusCode) { + @ResponseMetered(level = COARSE) + @Path("/response-metered-coarse") + public Response responseMeteredCoarse(@QueryParam("status_code") @DefaultValue("200") int statusCode) { return Response.status(Response.Status.fromStatusCode(statusCode)).build(); } @GET - @ResponseMetered - @Path("/response-5xx-metered") - public Response response5xxMetered(@QueryParam("status_code") @DefaultValue("500") int statusCode) { + @ResponseMetered(level = ALL) + @Path("/response-metered-all") + public Response responseMeteredAll(@QueryParam("status_code") @DefaultValue("200") int statusCode) { return Response.status(Response.Status.fromStatusCode(statusCode)).build(); } diff --git a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedResourceResponseMeteredPerClass.java b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedResourceResponseMeteredPerClass.java index 2f0f4684d0..e03e8e9553 100644 --- a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedResourceResponseMeteredPerClass.java +++ b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedResourceResponseMeteredPerClass.java @@ -6,8 +6,6 @@ import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.Produces; -import javax.ws.rs.QueryParam; -import javax.ws.rs.DefaultValue; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -24,14 +22,14 @@ public Response responseMetered2xxPerClass() { @GET @Path("/responseMetered4xxPerClass") - public Response responseMetered4xxPerClass(@QueryParam("status_code") @DefaultValue("400") int statusCode) { - return Response.status(Response.Status.fromStatusCode(statusCode)).build(); + public Response responseMetered4xxPerClass() { + return Response.status(Response.Status.BAD_REQUEST).build(); } @GET @Path("/responseMetered5xxPerClass") - public Response responseMetered5xxPerClass(@QueryParam("status_code") @DefaultValue("500") int statusCode) { - return Response.status(Response.Status.fromStatusCode(statusCode)).build(); + public Response responseMetered5xxPerClass() { + return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build(); } @GET diff --git a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedSubResourceResponseMeteredPerClass.java b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedSubResourceResponseMeteredPerClass.java index 88d3a2892d..ae131ee404 100644 --- a/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedSubResourceResponseMeteredPerClass.java +++ b/metrics-jersey2/src/test/java/com/codahale/metrics/jersey2/resources/InstrumentedSubResourceResponseMeteredPerClass.java @@ -1,13 +1,17 @@ package com.codahale.metrics.jersey2.resources; import com.codahale.metrics.annotation.ResponseMetered; +import com.codahale.metrics.annotation.ResponseMeteredLevel; + import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; -@ResponseMetered +import static com.codahale.metrics.annotation.ResponseMeteredLevel.ALL; + +@ResponseMetered(level = ALL) @Produces(MediaType.TEXT_PLAIN) public class InstrumentedSubResourceResponseMeteredPerClass { @GET