From 84456420526af75ad4ce452266c967fa01af87f4 Mon Sep 17 00:00:00 2001 From: jason plumb <75337021+breedx-splk@users.noreply.github.com> Date: Wed, 24 Aug 2022 10:59:29 -0700 Subject: [PATCH] Update bucket hist defaults to match spec (#4684) * udpate default histogram bucket boundaries * spotless --- .../ExplicitBucketHistogramUtils.java | 4 +-- .../sdk/metrics/SdkDoubleHistogramTest.java | 30 +++++++------------ .../sdk/metrics/SdkLongHistogramTest.java | 30 +++++++------------ .../sdk/metrics/SdkMeterProviderTest.java | 6 ++-- 4 files changed, 23 insertions(+), 47 deletions(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtils.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtils.java index c0e0123f054..93958461736 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtils.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtils.java @@ -20,9 +20,7 @@ private ExplicitBucketHistogramUtils() {} public static final List DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES = Collections.unmodifiableList( - Arrays.asList( - 5d, 10d, 25d, 50d, 75d, 100d, 250d, 500d, 750d, 1_000d, 2_500d, 5_000d, 7_500d, - 10_000d)); + Arrays.asList(0d, 5d, 10d, 25d, 50d, 75d, 100d, 250d, 500d, 1_000d)); /** Converts bucket boundary "convenient" configuration into the "more efficient" array. */ public static double[] createBoundaryArray(List boundaries) { diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java index 9d81e469881..1f3e628b5d2 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java @@ -112,10 +112,8 @@ void collectMetrics_WithEmptyAttributes() { .hasCount(2) .hasSum(24) .hasBucketBoundaries( - 5, 10, 25, 50, 75, 100, 250, 500, 750, 1_000, 2_500, - 5_000, 7_500, 10_000) - .hasBucketCounts( - 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)))); + 0, 5, 10, 25, 50, 75, 100, 250, 500, 1_000) + .hasBucketCounts(0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0)))); } @Test @@ -149,8 +147,7 @@ void collectMetrics_WithMultipleCollects() { .hasEpochNanos(testClock.now()) .hasCount(3) .hasSum(566.3d) - .hasBucketCounts( - 0, 0, 0, 0, 0, 0, 2, 1, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 0, 0, 0, 0, 0, 2, 1, 0, 0) .hasAttributes(attributeEntry("K", "V")), point -> point @@ -158,8 +155,7 @@ void collectMetrics_WithMultipleCollects() { .hasEpochNanos(testClock.now()) .hasCount(2) .hasSum(22.2d) - .hasBucketCounts( - 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(Attributes.empty())))); // Histograms are cumulative by default. @@ -182,8 +178,7 @@ void collectMetrics_WithMultipleCollects() { .hasEpochNanos(testClock.now()) .hasCount(4) .hasSum(788.3) - .hasBucketCounts( - 0, 0, 0, 0, 0, 0, 3, 1, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 0, 0, 0, 0, 0, 3, 1, 0, 0) .hasAttributes(attributeEntry("K", "V")), point -> point @@ -191,8 +186,7 @@ void collectMetrics_WithMultipleCollects() { .hasEpochNanos(testClock.now()) .hasCount(3) .hasSum(39.2) - .hasBucketCounts( - 0, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 1, 2, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(Attributes.empty())))); } finally { bound.unbind(); @@ -386,8 +380,7 @@ void stressTest_WithDifferentLabelSet() { .hasEpochNanos(testClock.now()) .hasCount(4_000) .hasSum(40_000) - .hasBucketCounts( - 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(attributeEntry(keys[0], values[0])), point -> point @@ -395,8 +388,7 @@ void stressTest_WithDifferentLabelSet() { .hasEpochNanos(testClock.now()) .hasCount(4_000) .hasSum(40_000) - .hasBucketCounts( - 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(attributeEntry(keys[1], values[1])), point -> point @@ -404,8 +396,7 @@ void stressTest_WithDifferentLabelSet() { .hasEpochNanos(testClock.now()) .hasCount(4_000) .hasSum(40_000) - .hasBucketCounts( - 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(attributeEntry(keys[2], values[2])), point -> point @@ -413,8 +404,7 @@ void stressTest_WithDifferentLabelSet() { .hasEpochNanos(testClock.now()) .hasCount(4_000) .hasSum(40_000) - .hasBucketCounts( - 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(attributeEntry(keys[3], values[3]))))); } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java index cf564a3cbe0..6b62d479a39 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java @@ -112,10 +112,8 @@ void collectMetrics_WithEmptyAttributes() { .hasCount(2) .hasSum(24) .hasBucketBoundaries( - 5, 10, 25, 50, 75, 100, 250, 500, 750, 1_000, 2_500, - 5_000, 7_500, 10_000) - .hasBucketCounts( - 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)))); + 0, 5, 10, 25, 50, 75, 100, 250, 500, 1_000) + .hasBucketCounts(0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0)))); } @Test @@ -149,8 +147,7 @@ void collectMetrics_WithMultipleCollects() { .hasEpochNanos(testClock.now()) .hasCount(3) .hasSum(445) - .hasBucketCounts( - 1, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 1, 0, 0, 0, 0, 0, 1, 1, 0, 0) .hasAttributes(attributeEntry("K", "V")), point -> point @@ -158,8 +155,7 @@ void collectMetrics_WithMultipleCollects() { .hasEpochNanos(testClock.now()) .hasCount(2) .hasSum(23) - .hasBucketCounts( - 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(Attributes.empty())))); // Histograms are cumulative by default. @@ -182,8 +178,7 @@ void collectMetrics_WithMultipleCollects() { .hasEpochNanos(testClock.now()) .hasCount(4) .hasSum(667) - .hasBucketCounts( - 1, 0, 0, 0, 0, 0, 2, 1, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 1, 0, 0, 0, 0, 0, 2, 1, 0, 0) .hasAttributes(attributeEntry("K", "V")), point -> point @@ -191,8 +186,7 @@ void collectMetrics_WithMultipleCollects() { .hasEpochNanos(testClock.now()) .hasCount(3) .hasSum(40) - .hasBucketCounts( - 0, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 1, 2, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(Attributes.empty())))); } finally { bound.unbind(); @@ -387,8 +381,7 @@ void stressTest_WithDifferentLabelSet() { .hasEpochNanos(testClock.now()) .hasCount(2_000) .hasSum(20_000) - .hasBucketCounts( - 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(attributeEntry(keys[0], values[0])), point -> point @@ -396,8 +389,7 @@ void stressTest_WithDifferentLabelSet() { .hasEpochNanos(testClock.now()) .hasCount(2_000) .hasSum(20_000) - .hasBucketCounts( - 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(attributeEntry(keys[1], values[1])), point -> point @@ -405,8 +397,7 @@ void stressTest_WithDifferentLabelSet() { .hasEpochNanos(testClock.now()) .hasCount(2_000) .hasSum(20_000) - .hasBucketCounts( - 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(attributeEntry(keys[2], values[2])), point -> point @@ -414,8 +405,7 @@ void stressTest_WithDifferentLabelSet() { .hasEpochNanos(testClock.now()) .hasCount(2_000) .hasSum(20_000) - .hasBucketCounts( - 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) + .hasBucketCounts(0, 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0) .hasAttributes(attributeEntry(keys[3], values[3]))))); } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java index 20462630a17..9498cc83e0d 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java @@ -130,8 +130,7 @@ void collectAllSyncInstruments() { .hasAttributes(Attributes.empty()) .hasCount(1) .hasSum(10.1) - .hasBucketCounts( - 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0))), + .hasBucketCounts(0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0))), metric -> assertThat(metric) .hasName("testDoubleCounter") @@ -159,8 +158,7 @@ void collectAllSyncInstruments() { .hasAttributes(Attributes.empty()) .hasCount(1) .hasSum(10) - .hasBucketCounts( - 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0))), + .hasBucketCounts(0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0))), metric -> assertThat(metric) .hasName("testLongUpDownCounter")