Skip to content

Commit

Permalink
Update bucket hist defaults to match spec (open-telemetry#4684)
Browse files Browse the repository at this point in the history 
* udpate default histogram bucket boundaries

* spotless
  • Loading branch information
@breedx-splk
breedx-splk authored Aug 24, 2022
1 parent 198b2c2 commit 8445642
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 47 deletions.
4 changes: 1 addition & 3 deletions ...n/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ private ExplicitBucketHistogramUtils() {}

public static final List<Double> 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<Double> boundaries) {
Expand Down
30 changes: 10 additions & 20 deletions sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -149,17 +147,15 @@ 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
.hasStartEpochNanos(startTime)
.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.
Expand All @@ -182,17 +178,15 @@ 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
.hasStartEpochNanos(startTime)
.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();
Expand Down Expand Up @@ -386,35 +380,31 @@ 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
.hasStartEpochNanos(testClock.now())
.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
.hasStartEpochNanos(testClock.now())
.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
.hasStartEpochNanos(testClock.now())
.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])))));
}

Expand Down
30 changes: 10 additions & 20 deletions sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -149,17 +147,15 @@ 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
.hasStartEpochNanos(startTime)
.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.
Expand All @@ -182,17 +178,15 @@ 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
.hasStartEpochNanos(startTime)
.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();
Expand Down Expand Up @@ -387,35 +381,31 @@ 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
.hasStartEpochNanos(testClock.now())
.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
.hasStartEpochNanos(testClock.now())
.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
.hasStartEpochNanos(testClock.now())
.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])))));
}

Expand Down
6 changes: 2 additions & 4 deletions sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 8445642

Please sign in to comment.