Skip to content

Commit

Permalink
Fix percentile metrics rotating too often
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rafalh committed Jul 1, 2024
1 parent c7f1a83 commit 177becd
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ abstract class AbstractTimeWindowHistogram<T, U> implements Histogram {

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

durationBetweenRotatesMillis = distributionStatisticConfig.getExpiry().toMillis() / ageBuckets;
durationBetweenRotatesMillis = distributionStatisticConfig.getExpiry().toMillis();
if (durationBetweenRotatesMillis <= 0) {
rejectHistogramConfig("expiry (" + distributionStatisticConfig.getExpiry().toMillis()
+ "ms) / bufferLength (" + ageBuckets + ") must be greater than 0.");
+ "ms) must be greater than 0.");
}

currentBucket = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ private boolean percentileValueIsApproximately(ValueAtPercentile vp, double perc
void timeBasedSlidingWindow() {
final DistributionStatisticConfig config = DistributionStatisticConfig.builder()
.percentiles(0.0, 0.5, 0.75, 0.9, 0.99, 0.999, 1.0)
.expiry(Duration.ofSeconds(4))
.expiry(Duration.ofSeconds(1))
.bufferLength(4)
.build()
.merge(DistributionStatisticConfig.DEFAULT);
Expand Down

0 comments on commit 177becd

Please sign in to comment.