Skip to content

Commit 53fdbf3

Browse files
committed
Fix percentile metrics rotating too often
Before this change percentile metrics rotation interval was set to `expiry / bufferLength`. This was different from other metrics like `_max` where the interval is set to `expiry`. At the same time documentation of the `expiry` parameter says clearly that it is used as rotation interval. This inconsistent behavior was confusing for users because some metrics expired faster than the others. What's more it caused some requests to be ignored in percentile metrics if `expiry` (also called `step`) was set to the same duration as scrapping interval (e.g. 1 minute) and scrapping occurred not long after buffer rotation. In case of the default config where `bufferLength` is 3 it could result in up to 33% requests being ignored by percentile metrics. Fix this inconsistency by changing buffer rotation interval for percentiles to `expiry`. Fixes micrometer-metrics#3298
1 parent c7f1a83 commit 53fdbf3

File tree

2 files changed

+3
-3
lines changed

2 files changed

+3
-3
lines changed

Diff for: micrometer-core/src/main/java/io/micrometer/core/instrument/distribution/AbstractTimeWindowHistogram.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ abstract class AbstractTimeWindowHistogram<T, U> implements Histogram {
7474

7575
ringBuffer = (T[]) Array.newInstance(bucketType, ageBuckets);
7676

77-
durationBetweenRotatesMillis = distributionStatisticConfig.getExpiry().toMillis() / ageBuckets;
77+
durationBetweenRotatesMillis = distributionStatisticConfig.getExpiry().toMillis();
7878
if (durationBetweenRotatesMillis <= 0) {
7979
rejectHistogramConfig("expiry (" + distributionStatisticConfig.getExpiry().toMillis()
80-
+ "ms) / bufferLength (" + ageBuckets + ") must be greater than 0.");
80+
+ "ms) must be greater than 0.");
8181
}
8282

8383
currentBucket = 0;

Diff for: micrometer-core/src/test/java/io/micrometer/core/instrument/distribution/TimeWindowPercentileHistogramTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ private boolean percentileValueIsApproximately(ValueAtPercentile vp, double perc
162162
void timeBasedSlidingWindow() {
163163
final DistributionStatisticConfig config = DistributionStatisticConfig.builder()
164164
.percentiles(0.0, 0.5, 0.75, 0.9, 0.99, 0.999, 1.0)
165-
.expiry(Duration.ofSeconds(4))
165+
.expiry(Duration.ofSeconds(1))
166166
.bufferLength(4)
167167
.build()
168168
.merge(DistributionStatisticConfig.DEFAULT);

0 commit comments

Comments
 (0)