Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetty 12 detailed response metrics bug: IndexOutOfBoundsException #3911

Closed
mihalyr opened this issue Jan 19, 2024 · 1 comment · Fixed by #3912 or #3913
Closed

Jetty 12 detailed response metrics bug: IndexOutOfBoundsException #3911

mihalyr opened this issue Jan 19, 2024 · 1 comment · Fixed by #3912 or #3913
Labels
Milestone

Comments

@mihalyr
Copy link

mihalyr commented Jan 19, 2024

Jetty12 instrumentation with detailed response metrics are broken, because coarse metrics are not collected but the instrumentation still registers the gauges with hardcoded array indexes which don't exist. This breaks the complete Graphite metrics publishing in our case.

java.lang.IndexOutOfBoundsException: Index: 3
    at java.util.Collections$EmptyList.get(Collections.java:4807)
    at io.dropwizard.metrics.jetty12.AbstractInstrumentedHandler$3.getRatio(AbstractInstrumentedHandler.java:201)

Consider this code: https://github.com/dropwizard/metrics/blob/release/4.2.x/metrics-jetty12/src/main/java/io/dropwizard/metrics/jetty12/AbstractInstrumentedHandler.java#L162-L169

        this.responses = COARSE_METER_LEVELS.contains(responseMeteredLevel) ?
                Collections.unmodifiableList(Arrays.asList(
                        metricRegistry.meter(name(prefix, NAME_1XX_RESPONSES)), // 1xx
                        metricRegistry.meter(name(prefix, NAME_2XX_RESPONSES)), // 2xx
                        metricRegistry.meter(name(prefix, NAME_3XX_RESPONSES)), // 3xx
                        metricRegistry.meter(name(prefix, NAME_4XX_RESPONSES)), // 4xx
                        metricRegistry.meter(name(prefix, NAME_5XX_RESPONSES))  // 5xx
                )) : Collections.emptyList();

The responses array is only filled if coarse meter levels are enabled. In case of detailed meter levels the responses field is an emptyList.

Now consider this code later: https://github.com/dropwizard/metrics/blob/release/4.2.x/metrics-jetty12/src/main/java/io/dropwizard/metrics/jetty12/AbstractInstrumentedHandler.java#L198-L204

        metricRegistry.register(name(prefix, NAME_PERCENT_4XX_15M), new RatioGauge() {
            @Override
            protected Ratio getRatio() {
                return Ratio.of(responses.get(3).getFifteenMinuteRate(),
                        requests.getFifteenMinuteRate());
            }
        });

This registers a gauge which calls responses.get(3) on an emptyList which will fail with a runtime exception as the index is out of the array bounds. When using the GraphiteReporter, this bug breaks the metric publishing completely.

  • The coarse gauges should only be registered if the coarse meter levels are enabled
  • It would be nice to have a test case for this class which also covers detailed metrics without coarse
@joschi
Copy link
Member

joschi commented Jan 19, 2024

@mihalyr Thanks for reporting this! The fix for this bug will be included in the next version of Dropwizard Metrics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants