Skip to content

Commit b0a5e13

Browse files
authored
fix: correct native histogram bucket merge on scale-down (#2274)
## Summary Fix native histogram bucket remapping when the bucket width is doubled during schema scale-down. The previous merge formula used: ``` (keys[i] + 1) / 2 ``` That works for positive bucket indexes, but not for negative even indexes. Java integer division truncates toward zero, so negative even buckets were merged into the bucket immediately to the right of the correct target. This matches the behaviour described in #2270 and aligns the merge formula with client_golang: ``` (keys[i] > 0 ? keys[i] + 1 : keys[i]) / 2 ``` ## Why When `doubleBucketWidth()` runs, counts from some negative native histogram buckets could be shifted into neighbouring buckets. That can make Prometheus interpret the reshuffled bucket counters as resets, causing spikes in queries such as: ``` histogram_count(rate(my_native_histogram[1m])) ``` ## Tests Added a focused regression test for negative, zero, and positive bucket index remapping during `doubleBucketWidth(Map)`. Verified locally: ``` ./mvnw test -pl prometheus-metrics-core -am \ -Dtest=HistogramTest#testDoubleBucketWidthMergesNegativeIndexesCorrectly \ -Dcoverage.skip=true \ -Dcheckstyle.skip=true \ -Dsurefire.failIfNoSpecifiedTests=false ``` Also verified: ```bash ./mvnw install -DskipTests -Dcoverage.skip=true ``` Fixes #2270 Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>
1 parent 4080581 commit b0a5e13

2 files changed

Lines changed: 34 additions & 1 deletion

File tree

prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ private void doubleBucketWidth(Map<Integer, LongAdder> buckets) {
614614
}
615615
buckets.clear();
616616
for (i = 0; i < keys.length; i++) {
617-
int index = (keys[i] + 1) / 2;
617+
int index = (keys[i] > 0 ? keys[i] + 1 : keys[i]) / 2;
618618
LongAdder count = buckets.computeIfAbsent(index, k -> new LongAdder());
619619
count.add(values[i]);
620620
}

prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/HistogramTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
import java.util.Arrays;
2929
import java.util.Collections;
3030
import java.util.Comparator;
31+
import java.util.HashMap;
3132
import java.util.List;
33+
import java.util.Map;
3234
import java.util.Random;
3335
import java.util.concurrent.CompletionService;
3436
import java.util.concurrent.CountDownLatch;
@@ -39,6 +41,7 @@
3941
import java.util.concurrent.Future;
4042
import java.util.concurrent.TimeUnit;
4143
import java.util.concurrent.TimeoutException;
44+
import java.util.concurrent.atomic.LongAdder;
4245
import java.util.stream.Collectors;
4346
import org.junit.jupiter.api.AfterEach;
4447
import org.junit.jupiter.api.BeforeEach;
@@ -836,6 +839,36 @@ void testNativeBucketIndexToUpperBound()
836839
}
837840
}
838841

842+
@Test
843+
void testDoubleBucketWidthMergesNegativeIndexesCorrectly()
844+
throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
845+
Histogram histogram = Histogram.builder().name("test").nativeOnly().build();
846+
Histogram.DataPoint dataPoint = histogram.newDataPoint();
847+
848+
Method doubleBucketWidth =
849+
Histogram.DataPoint.class.getDeclaredMethod("doubleBucketWidth", Map.class);
850+
doubleBucketWidth.setAccessible(true);
851+
852+
int[] keys = {-5, -4, -3, -2, -1, 0, 1, 2, 3, 4, 5};
853+
int[] expectedMergedIndexes = {-2, -2, -1, -1, 0, 0, 1, 1, 2, 2, 3};
854+
855+
for (int i = 0; i < keys.length; i++) {
856+
Map<Integer, LongAdder> buckets = new HashMap<>();
857+
LongAdder adder = new LongAdder();
858+
adder.add(7);
859+
buckets.put(keys[i], adder);
860+
861+
doubleBucketWidth.invoke(dataPoint, buckets);
862+
863+
assertThat(buckets.keySet())
864+
.as("merged index for original key " + keys[i])
865+
.containsExactly(expectedMergedIndexes[i]);
866+
assertThat(buckets.get(expectedMergedIndexes[i]).sum())
867+
.as("count preserved for original key " + keys[i])
868+
.isEqualTo(7);
869+
}
870+
}
871+
839872
/**
840873
* Test if lowerBound < value <= upperBound is true for the bucket index returned by
841874
* findBucketIndex()

0 commit comments

Comments
 (0)