From adce42d8cd1ebaec68401cea74ca114ca4086f45 Mon Sep 17 00:00:00 2001 From: Jochen Schalanda Date: Sat, 20 Jan 2024 00:02:55 +0100 Subject: [PATCH] Fix IndexOutOfBoundsException in Jetty 9, 10, 11, 12 InstrumentedHandler (#3913) Fixes #3911 Refs #3133 Refs #3174 Refs #3175 --- .../metrics5/jetty10/InstrumentedHandler.java | 115 +++++++++--------- .../jetty10/InstrumentedHandlerTest.java | 20 +++ .../metrics5/jetty11/InstrumentedHandler.java | 114 ++++++++--------- .../jetty11/InstrumentedHandlerTest.java | 20 +++ 4 files changed, 158 insertions(+), 111 deletions(-) diff --git a/metrics-jetty10/src/main/java/io/dropwizard/metrics5/jetty10/InstrumentedHandler.java b/metrics-jetty10/src/main/java/io/dropwizard/metrics5/jetty10/InstrumentedHandler.java index 64e3f7acfe..906dc13f54 100644 --- a/metrics-jetty10/src/main/java/io/dropwizard/metrics5/jetty10/InstrumentedHandler.java +++ b/metrics-jetty10/src/main/java/io/dropwizard/metrics5/jetty10/InstrumentedHandler.java @@ -170,14 +170,6 @@ protected void doStart() throws Exception { this.asyncTimeouts = metricRegistry.meter(prefix.resolve(NAME_ASYNC_TIMEOUTS)); this.responseCodeMeters = DETAILED_METER_LEVELS.contains(responseMeteredLevel) ? new ConcurrentHashMap<>() : Collections.emptyMap(); - this.responses = COARSE_METER_LEVELS.contains(responseMeteredLevel) ? - Collections.unmodifiableList(Arrays.asList( - metricRegistry.meter(prefix.resolve(NAME_1XX_RESPONSES)), // 1xx - metricRegistry.meter(prefix.resolve(NAME_2XX_RESPONSES)), // 2xx - metricRegistry.meter(prefix.resolve(NAME_3XX_RESPONSES)), // 3xx - metricRegistry.meter(prefix.resolve(NAME_4XX_RESPONSES)), // 4xx - metricRegistry.meter(prefix.resolve(NAME_5XX_RESPONSES)) // 5xx - )) : Collections.emptyList(); this.getRequests = metricRegistry.timer(prefix.resolve(NAME_GET_REQUESTS)); this.postRequests = metricRegistry.timer(prefix.resolve(NAME_POST_REQUESTS)); @@ -190,54 +182,65 @@ protected void doStart() throws Exception { this.moveRequests = metricRegistry.timer(prefix.resolve(NAME_MOVE_REQUESTS)); this.otherRequests = metricRegistry.timer(prefix.resolve(NAME_OTHER_REQUESTS)); - metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_1M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(3).getOneMinuteRate(), - requests.getOneMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_5M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(3).getFiveMinuteRate(), - requests.getFiveMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_15M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(3).getFifteenMinuteRate(), - requests.getFifteenMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_1M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(4).getOneMinuteRate(), - requests.getOneMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_5M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(4).getFiveMinuteRate(), - requests.getFiveMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_15M), new RatioGauge() { - @Override - public Ratio getRatio() { - return Ratio.of(responses.get(4).getFifteenMinuteRate(), - requests.getFifteenMinuteRate()); - } - }); - + if (COARSE_METER_LEVELS.contains(responseMeteredLevel)) { + this.responses = Collections.unmodifiableList(Arrays.asList( + metricRegistry.meter(prefix.resolve(NAME_1XX_RESPONSES)), // 1xx + metricRegistry.meter(prefix.resolve(NAME_2XX_RESPONSES)), // 2xx + metricRegistry.meter(prefix.resolve(NAME_3XX_RESPONSES)), // 3xx + metricRegistry.meter(prefix.resolve(NAME_4XX_RESPONSES)), // 4xx + metricRegistry.meter(prefix.resolve(NAME_5XX_RESPONSES)) // 5xx + )); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_1M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(3).getOneMinuteRate(), + requests.getOneMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_5M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(3).getFiveMinuteRate(), + requests.getFiveMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_15M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(3).getFifteenMinuteRate(), + requests.getFifteenMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_1M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(4).getOneMinuteRate(), + requests.getOneMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_5M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(4).getFiveMinuteRate(), + requests.getFiveMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_15M), new RatioGauge() { + @Override + public Ratio getRatio() { + return Ratio.of(responses.get(4).getFifteenMinuteRate(), + requests.getFifteenMinuteRate()); + } + }); + } else { + this.responses = Collections.emptyList(); + } this.listener = new AsyncAttachingListener(); } diff --git a/metrics-jetty10/src/test/java/io/dropwizard/metrics5/jetty10/InstrumentedHandlerTest.java b/metrics-jetty10/src/test/java/io/dropwizard/metrics5/jetty10/InstrumentedHandlerTest.java index 1e08412d69..64dd0dd597 100644 --- a/metrics-jetty10/src/test/java/io/dropwizard/metrics5/jetty10/InstrumentedHandlerTest.java +++ b/metrics-jetty10/src/test/java/io/dropwizard/metrics5/jetty10/InstrumentedHandlerTest.java @@ -24,6 +24,8 @@ import java.util.concurrent.TimeUnit; import static io.dropwizard.metrics5.annotation.ResponseMeteredLevel.ALL; +import static io.dropwizard.metrics5.annotation.ResponseMeteredLevel.COARSE; +import static io.dropwizard.metrics5.annotation.ResponseMeteredLevel.DETAILED; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; @@ -121,6 +123,24 @@ void doStopDoesNotThrowNPE() throws Exception { assertThatCode(handler::doStop).doesNotThrowAnyException(); } + @Test + public void gaugesAreRegisteredWithResponseMeteredLevelCoarse() throws Exception { + InstrumentedHandler handler = new InstrumentedHandler(registry, "coarse", COARSE); + handler.setHandler(new TestHandler()); + handler.setName("handler"); + handler.doStart(); + assertThat(registry.getGauges()).containsKey(MetricName.build("coarse", "handler", "percent-4xx-1m")); + } + + @Test + public void gaugesAreNotRegisteredWithResponseMeteredLevelDetailed() throws Exception { + InstrumentedHandler handler = new InstrumentedHandler(registry, "detailed", DETAILED); + handler.setHandler(new TestHandler()); + handler.setName("handler"); + handler.doStart(); + assertThat(registry.getGauges()).doesNotContainKey(MetricName.build("detailed", "handler", "percent-4xx-1m")); + } + @Test @Disabled("flaky on virtual machines") void responseTimesAreRecordedForAsyncResponses() throws Exception { diff --git a/metrics-jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHandler.java b/metrics-jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHandler.java index 82a80be83b..ae8cb296a4 100644 --- a/metrics-jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHandler.java +++ b/metrics-jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHandler.java @@ -170,14 +170,6 @@ protected void doStart() throws Exception { this.asyncTimeouts = metricRegistry.meter(prefix.resolve(NAME_ASYNC_TIMEOUTS)); this.responseCodeMeters = DETAILED_METER_LEVELS.contains(responseMeteredLevel) ? new ConcurrentHashMap<>() : Collections.emptyMap(); - this.responses = COARSE_METER_LEVELS.contains(responseMeteredLevel) ? - Collections.unmodifiableList(Arrays.asList( - metricRegistry.meter(prefix.resolve(NAME_1XX_RESPONSES)), // 1xx - metricRegistry.meter(prefix.resolve(NAME_2XX_RESPONSES)), // 2xx - metricRegistry.meter(prefix.resolve(NAME_3XX_RESPONSES)), // 3xx - metricRegistry.meter(prefix.resolve(NAME_4XX_RESPONSES)), // 4xx - metricRegistry.meter(prefix.resolve(NAME_5XX_RESPONSES)) // 5xx - )) : Collections.emptyList(); this.getRequests = metricRegistry.timer(prefix.resolve(NAME_GET_REQUESTS)); this.postRequests = metricRegistry.timer(prefix.resolve(NAME_POST_REQUESTS)); @@ -190,53 +182,65 @@ protected void doStart() throws Exception { this.moveRequests = metricRegistry.timer(prefix.resolve(NAME_MOVE_REQUESTS)); this.otherRequests = metricRegistry.timer(prefix.resolve(NAME_OTHER_REQUESTS)); - metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_1M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(3).getOneMinuteRate(), - requests.getOneMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_5M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(3).getFiveMinuteRate(), - requests.getFiveMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_15M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(3).getFifteenMinuteRate(), - requests.getFifteenMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_1M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(4).getOneMinuteRate(), - requests.getOneMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_5M), new RatioGauge() { - @Override - protected Ratio getRatio() { - return Ratio.of(responses.get(4).getFiveMinuteRate(), - requests.getFiveMinuteRate()); - } - }); - - metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_15M), new RatioGauge() { - @Override - public Ratio getRatio() { - return Ratio.of(responses.get(4).getFifteenMinuteRate(), - requests.getFifteenMinuteRate()); - } - }); + if (COARSE_METER_LEVELS.contains(responseMeteredLevel)) { + this.responses = Collections.unmodifiableList(Arrays.asList( + metricRegistry.meter(prefix.resolve(NAME_1XX_RESPONSES)), // 1xx + metricRegistry.meter(prefix.resolve(NAME_2XX_RESPONSES)), // 2xx + metricRegistry.meter(prefix.resolve(NAME_3XX_RESPONSES)), // 3xx + metricRegistry.meter(prefix.resolve(NAME_4XX_RESPONSES)), // 4xx + metricRegistry.meter(prefix.resolve(NAME_5XX_RESPONSES)) // 5xx + )); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_1M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(3).getOneMinuteRate(), + requests.getOneMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_5M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(3).getFiveMinuteRate(), + requests.getFiveMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_4XX_15M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(3).getFifteenMinuteRate(), + requests.getFifteenMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_1M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(4).getOneMinuteRate(), + requests.getOneMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_5M), new RatioGauge() { + @Override + protected Ratio getRatio() { + return Ratio.of(responses.get(4).getFiveMinuteRate(), + requests.getFiveMinuteRate()); + } + }); + + metricRegistry.register(prefix.resolve(NAME_PERCENT_5XX_15M), new RatioGauge() { + @Override + public Ratio getRatio() { + return Ratio.of(responses.get(4).getFifteenMinuteRate(), + requests.getFifteenMinuteRate()); + } + }); + } else { + this.responses = Collections.emptyList(); + } this.listener = new AsyncAttachingListener(); } diff --git a/metrics-jetty11/src/test/java/io/dropwizard/metrics5/jetty11/InstrumentedHandlerTest.java b/metrics-jetty11/src/test/java/io/dropwizard/metrics5/jetty11/InstrumentedHandlerTest.java index 36313ce902..94f0f9533f 100644 --- a/metrics-jetty11/src/test/java/io/dropwizard/metrics5/jetty11/InstrumentedHandlerTest.java +++ b/metrics-jetty11/src/test/java/io/dropwizard/metrics5/jetty11/InstrumentedHandlerTest.java @@ -24,6 +24,8 @@ import java.util.concurrent.TimeUnit; import static io.dropwizard.metrics5.annotation.ResponseMeteredLevel.ALL; +import static io.dropwizard.metrics5.annotation.ResponseMeteredLevel.COARSE; +import static io.dropwizard.metrics5.annotation.ResponseMeteredLevel.DETAILED; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; @@ -121,6 +123,24 @@ void doStopDoesNotThrowNPE() throws Exception { assertThatCode(handler::doStop).doesNotThrowAnyException(); } + @Test + public void gaugesAreRegisteredWithResponseMeteredLevelCoarse() throws Exception { + InstrumentedHandler handler = new InstrumentedHandler(registry, "coarse", COARSE); + handler.setHandler(new TestHandler()); + handler.setName("handler"); + handler.doStart(); + assertThat(registry.getGauges()).containsKey(MetricName.build("coarse", "handler", "percent-4xx-1m")); + } + + @Test + public void gaugesAreNotRegisteredWithResponseMeteredLevelDetailed() throws Exception { + InstrumentedHandler handler = new InstrumentedHandler(registry, "detailed", DETAILED); + handler.setHandler(new TestHandler()); + handler.setName("handler"); + handler.doStart(); + assertThat(registry.getGauges()).doesNotContainKey(MetricName.build("detailed", "handler", "percent-4xx-1m")); + } + @Test @Disabled("flaky on virtual machines") void responseTimesAreRecordedForAsyncResponses() throws Exception {