DefaultLongTaskTimer class throws an IllegalArgumentException when percentiles is empty

[marcelopbarros]

Describe the bug
The DefaultLongTaskTimer class throws an IllegalArgumentException when double[] percentiles is not null, but it is empty.

Environment

• Micrometer version [1.11.6, 1.12.0]
• Micrometer registry [prometheus]
• OS: [macOS]
• Java version: [openjdk 17.0.5 2022-10-18]

To Reproduce
How to reproduce the bug:

@Test
public void testWhenPercentilesEmptyThrowsIllegalArgumentException() {
    final var id = new Meter.Id("my.metric", Tags.empty(), "seconds", null, Type.LONG_TASK_TIMER);
    final var config = DistributionStatisticConfig.builder()
            .percentiles(new double[] {})
            .build();

    final var timer = new DefaultLongTaskTimer(id, Clock.SYSTEM, TimeUnit.SECONDS, config, true);
    Assertions.assertThrowsExactly(IllegalArgumentException.class, () -> {
        timer.takeSnapshot();
    });
}

Also, in a Spring Boot 3.1.6 environment, if you set the metrics distribution to an empty configuration, the error throws when accessing /actuator/prometheus. (Empty configuration is useful when you have a default configuration and, for a specific environment you want it to be null).

management.metrics.distribution.percentiles.http.server.requests=

Expected behavior

Considering the actual implementation of DefaultLongTaskTimer, I believe that it was intended to handle empty arrays nicely, the same way when it is null.

Queue<Double> percentilesRequested = new ArrayBlockingQueue<>(
        distributionStatisticConfig.getPercentiles() == null ? 1
                : distributionStatisticConfig.getPercentiles().length);
double[] percentilesRequestedArr = distributionStatisticConfig.getPercentiles();
if (percentilesRequestedArr != null && percentilesRequestedArr.length > 0) {
    Arrays.stream(percentilesRequestedArr).sorted().boxed().forEach(percentilesRequested::add);
}

[jonatan-ivanov]

Is this a "theoretical" issue or you actually bumped into this in prod?
If so, how? You should not use DefaultLongTaskTimer directly or set the property to empty.

(I'm not saying we should not fix this I'm only curious if this has production consequences.)

[marcelopbarros]

@jonatan-ivanov I bumbped into this in a Spring Web application, while using Spring Boot 3. The application uses a default configuration of percentiles for a few environments. Then we use something like:

management.metrics.distribution.percentiles.http.server.requests=0.5,0.9,0.95,0.99

But, there is one environment that souldn't be calculating percentiles (because of the size of the metrics becoming to big). In this environment we override the configuration with an empty config.

management.metrics.distribution.percentiles.http.server.requests=

The class DefaultLongTaskTimer is never instantiated directly, but it's indirectly referenced by PrometheusMeterRegistry in the function newLongTaskTimer when it calls new CumulativeHistogramLongTaskTimer.

We realize that when there's no annotation at all, the class is loaded with null in percentiles. And it handles it well.

The reason that made me believe it was an issue it's because the code seems to check the size in the comparison percentilesRequestedArr.length > 0, but this check logically a dead code (it will never be false) because when the length is 0 the ArrayBlockingQueue constructor throws the IllegalArgumentException.

Queue<Double> percentilesRequested = new ArrayBlockingQueue<>(
        distributionStatisticConfig.getPercentiles() == null ? 1
                : distributionStatisticConfig.getPercentiles().length);
double[] percentilesRequestedArr = distributionStatisticConfig.getPercentiles();
if (percentilesRequestedArr != null && percentilesRequestedArr.length > 0) {
    Arrays.stream(percentilesRequestedArr).sorted().boxed().forEach(percentilesRequested::add);
}

So, my suggestion is to treat empty percentiles the same way as when they are null.

[github-actions]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[github-actions]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open.

[marcelopbarros]

Hi! The issue was closed, but I submitted the feedback here. Is it still going to be evaluated?

[shakuzen]

Sorry about that. It seems our bot didn't do what we expected. I see you've opened a pull request (#4483) for us as well. Thank you.