Skip to content

Commit 928d112

Browse files
jack-bergjack-berg
authored
Revert histogram buckets (#4736)
* Revert "Restore prior histogram defaults (#4717)" This reverts commit 160af1c. * Revert "Update bucket hist defaults to match spec (#4684)" This reverts commit 8445642.
1 parent b5ef538 commit 928d112

File tree

12 files changed

+51
-113
lines changed

12 files changed

+51
-113
lines changed

‎sdk/metrics/build.gradle.kts

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ dependencies {
3232

3333
testing {
3434
suites {
35-
val configurationTest by registering(JvmTestSuite::class) {
35+
val debugEnabledTest by registering(JvmTestSuite::class) {
3636
targets {
3737
all {
3838
testTask.configure {
39-
jvmArgs("-Dotel.experimental.sdk.metrics.debug=true", "-Dotel.java.histogram.legacy.buckets.enabled=true")
39+
jvmArgs("-Dotel.experimental.sdk.metrics.debug=true")
4040
}
4141
}
4242
}

‎sdk/metrics/src/configurationTest/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtilsTest.java

-28
This file was deleted.

‎sdk/metrics/src/configurationTest/java/io/opentelemetry/sdk/metrics/internal/descriptor/InstrumentDescriptorTest.java renamed to ‎sdk/metrics/src/debugEnabledTest/java/io/opentelemetry/sdk/metrics/internal/descriptor/InstrumentDescriptorTest.java

File renamed without changes.

‎sdk/metrics/src/configurationTest/java/io/opentelemetry/testing/SourceInfoTest.java renamed to ‎sdk/metrics/src/debugEnabledTest/java/io/opentelemetry/testing/SourceInfoTest.java

File renamed without changes.

‎sdk/metrics/src/jmh/java/io/opentelemetry/sdk/metrics/internal/aggregator/HistogramAggregationParam.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public enum HistogramAggregationParam {
1515
EXPLICIT_DEFAULT_BUCKET(
1616
new DoubleExplicitBucketHistogramAggregator(
1717
ExplicitBucketHistogramUtils.createBoundaryArray(
18-
ExplicitBucketHistogramUtils.getDefaultBucketBoundaries()),
18+
ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES),
1919
ExemplarReservoir::doubleNoSamples)),
2020
EXPLICIT_SINGLE_BUCKET(
2121
new DoubleExplicitBucketHistogramAggregator(

‎sdk/metrics/src/jmh/java/io/opentelemetry/sdk/metrics/internal/aggregator/HistogramValueGenerator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ private static double[] explicitDefaultBucketPool() {
6464
// Add minimal recording value.
6565
fixedBoundaries.add(0.0);
6666
// Add the bucket LE bucket boundaries (starts at 5).
67-
fixedBoundaries.addAll(ExplicitBucketHistogramUtils.getDefaultBucketBoundaries());
67+
fixedBoundaries.addAll(ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES);
6868
// Add Double max value as our other extreme.
6969
fixedBoundaries.add(Double.MAX_VALUE);
7070
return ExplicitBucketHistogramUtils.createBoundaryArray(fixedBoundaries);

‎sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtils.java

+2-38
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
import java.util.Arrays;
99
import java.util.Collections;
1010
import java.util.List;
11-
import java.util.Locale;
12-
import java.util.logging.Level;
13-
import java.util.logging.Logger;
1411

1512
/**
1613
* Utilities for interacting with explicit bucket histograms.
@@ -19,47 +16,14 @@
1916
* at any time.
2017
*/
2118
public final class ExplicitBucketHistogramUtils {
19+
private ExplicitBucketHistogramUtils() {}
2220

23-
private static final Logger logger =
24-
Logger.getLogger(ExplicitBucketHistogramUtils.class.getName());
25-
private static final String LEGACY_BUCKETS_ENABLED = "otel.java.histogram.legacy.buckets.enabled";
26-
private static final List<Double> DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES =
27-
Collections.unmodifiableList(
28-
Arrays.asList(0d, 5d, 10d, 25d, 50d, 75d, 100d, 250d, 500d, 1_000d));
29-
private static final List<Double> LEGACY_HISTOGRAM_BUCKET_BOUNDARIES =
21+
public static final List<Double> DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES =
3022
Collections.unmodifiableList(
3123
Arrays.asList(
3224
5d, 10d, 25d, 50d, 75d, 100d, 250d, 500d, 750d, 1_000d, 2_500d, 5_000d, 7_500d,
3325
10_000d));
3426

35-
private static final List<Double> defaultBucketBoundaries;
36-
37-
static {
38-
// TODO: remove support for configuring legacy bucket boundaries after 1.24.0
39-
boolean legacyBucketsEnabled =
40-
Boolean.parseBoolean(System.getProperty(LEGACY_BUCKETS_ENABLED))
41-
|| Boolean.parseBoolean(
42-
System.getenv(LEGACY_BUCKETS_ENABLED.toUpperCase(Locale.ROOT).replace(".", "_")));
43-
if (legacyBucketsEnabled) {
44-
logger.log(
45-
Level.WARNING,
46-
"Legacy explicit bucket histogram buckets have been enabled. Support will be removed "
47-
+ "after version 1.24.0. If you depend on the legacy bucket boundaries, please "
48-
+ "use the View API as described in "
49-
+ "https://opentelemetry.io/docs/instrumentation/java/manual/#views.");
50-
defaultBucketBoundaries = LEGACY_HISTOGRAM_BUCKET_BOUNDARIES;
51-
} else {
52-
defaultBucketBoundaries = DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES;
53-
}
54-
}
55-
56-
private ExplicitBucketHistogramUtils() {}
57-
58-
/** Returns the default explicit bucket histogram bucket boundaries. */
59-
public static List<Double> getDefaultBucketBoundaries() {
60-
return defaultBucketBoundaries;
61-
}
62-
6327
/** Converts bucket boundary "convenient" configuration into the "more efficient" array. */
6428
public static double[] createBoundaryArray(List<Double> boundaries) {
6529
return validateBucketBoundaries(boundaries.stream().mapToDouble(i -> i).toArray());

‎sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public final class ExplicitBucketHistogramAggregation implements Aggregation, Ag
2727

2828
private static final Aggregation DEFAULT =
2929
new ExplicitBucketHistogramAggregation(
30-
ExplicitBucketHistogramUtils.getDefaultBucketBoundaries());
30+
ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES);
3131

3232
public static Aggregation getDefault() {
3333
return DEFAULT;

‎sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java

+20-10
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,10 @@ void collectMetrics_WithEmptyAttributes() {
112112
.hasCount(2)
113113
.hasSum(24)
114114
.hasBucketBoundaries(
115-
0, 5, 10, 25, 50, 75, 100, 250, 500, 1_000)
116-
.hasBucketCounts(0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0))));
115+
5, 10, 25, 50, 75, 100, 250, 500, 750, 1_000, 2_500,
116+
5_000, 7_500, 10_000)
117+
.hasBucketCounts(
118+
0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0))));
117119
}
118120

119121
@Test
@@ -147,15 +149,17 @@ void collectMetrics_WithMultipleCollects() {
147149
.hasEpochNanos(testClock.now())
148150
.hasCount(3)
149151
.hasSum(566.3d)
150-
.hasBucketCounts(0, 0, 0, 0, 0, 0, 0, 2, 1, 0, 0)
152+
.hasBucketCounts(
153+
0, 0, 0, 0, 0, 0, 2, 1, 0, 0, 0, 0, 0, 0, 0)
151154
.hasAttributes(attributeEntry("K", "V")),
152155
point ->
153156
point
154157
.hasStartEpochNanos(startTime)
155158
.hasEpochNanos(testClock.now())
156159
.hasCount(2)
157160
.hasSum(22.2d)
158-
.hasBucketCounts(0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0)
161+
.hasBucketCounts(
162+
0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
159163
.hasAttributes(Attributes.empty()))));
160164

161165
// Histograms are cumulative by default.
@@ -178,15 +182,17 @@ void collectMetrics_WithMultipleCollects() {
178182
.hasEpochNanos(testClock.now())
179183
.hasCount(4)
180184
.hasSum(788.3)
181-
.hasBucketCounts(0, 0, 0, 0, 0, 0, 0, 3, 1, 0, 0)
185+
.hasBucketCounts(
186+
0, 0, 0, 0, 0, 0, 3, 1, 0, 0, 0, 0, 0, 0, 0)
182187
.hasAttributes(attributeEntry("K", "V")),
183188
point ->
184189
point
185190
.hasStartEpochNanos(startTime)
186191
.hasEpochNanos(testClock.now())
187192
.hasCount(3)
188193
.hasSum(39.2)
189-
.hasBucketCounts(0, 0, 1, 2, 0, 0, 0, 0, 0, 0, 0)
194+
.hasBucketCounts(
195+
0, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
190196
.hasAttributes(Attributes.empty()))));
191197
} finally {
192198
bound.unbind();
@@ -380,31 +386,35 @@ void stressTest_WithDifferentLabelSet() {
380386
.hasEpochNanos(testClock.now())
381387
.hasCount(4_000)
382388
.hasSum(40_000)
383-
.hasBucketCounts(0, 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0)
389+
.hasBucketCounts(
390+
0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
384391
.hasAttributes(attributeEntry(keys[0], values[0])),
385392
point ->
386393
point
387394
.hasStartEpochNanos(testClock.now())
388395
.hasEpochNanos(testClock.now())
389396
.hasCount(4_000)
390397
.hasSum(40_000)
391-
.hasBucketCounts(0, 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0)
398+
.hasBucketCounts(
399+
0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
392400
.hasAttributes(attributeEntry(keys[1], values[1])),
393401
point ->
394402
point
395403
.hasStartEpochNanos(testClock.now())
396404
.hasEpochNanos(testClock.now())
397405
.hasCount(4_000)
398406
.hasSum(40_000)
399-
.hasBucketCounts(0, 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0)
407+
.hasBucketCounts(
408+
0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
400409
.hasAttributes(attributeEntry(keys[2], values[2])),
401410
point ->
402411
point
403412
.hasStartEpochNanos(testClock.now())
404413
.hasEpochNanos(testClock.now())
405414
.hasCount(4_000)
406415
.hasSum(40_000)
407-
.hasBucketCounts(0, 0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0)
416+
.hasBucketCounts(
417+
0, 2000, 2000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
408418
.hasAttributes(attributeEntry(keys[3], values[3])))));
409419
}
410420

‎sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java

+20-10
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,10 @@ void collectMetrics_WithEmptyAttributes() {
112112
.hasCount(2)
113113
.hasSum(24)
114114
.hasBucketBoundaries(
115-
0, 5, 10, 25, 50, 75, 100, 250, 500, 1_000)
116-
.hasBucketCounts(0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0))));
115+
5, 10, 25, 50, 75, 100, 250, 500, 750, 1_000, 2_500,
116+
5_000, 7_500, 10_000)
117+
.hasBucketCounts(
118+
0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0))));
117119
}
118120

119121
@Test
@@ -147,15 +149,17 @@ void collectMetrics_WithMultipleCollects() {
147149
.hasEpochNanos(testClock.now())
148150
.hasCount(3)
149151
.hasSum(445)
150-
.hasBucketCounts(0, 1, 0, 0, 0, 0, 0, 1, 1, 0, 0)
152+
.hasBucketCounts(
153+
1, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0)
151154
.hasAttributes(attributeEntry("K", "V")),
152155
point ->
153156
point
154157
.hasStartEpochNanos(startTime)
155158
.hasEpochNanos(testClock.now())
156159
.hasCount(2)
157160
.hasSum(23)
158-
.hasBucketCounts(0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0)
161+
.hasBucketCounts(
162+
0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
159163
.hasAttributes(Attributes.empty()))));
160164

161165
// Histograms are cumulative by default.
@@ -178,15 +182,17 @@ void collectMetrics_WithMultipleCollects() {
178182
.hasEpochNanos(testClock.now())
179183
.hasCount(4)
180184
.hasSum(667)
181-
.hasBucketCounts(0, 1, 0, 0, 0, 0, 0, 2, 1, 0, 0)
185+
.hasBucketCounts(
186+
1, 0, 0, 0, 0, 0, 2, 1, 0, 0, 0, 0, 0, 0, 0)
182187
.hasAttributes(attributeEntry("K", "V")),
183188
point ->
184189
point
185190
.hasStartEpochNanos(startTime)
186191
.hasEpochNanos(testClock.now())
187192
.hasCount(3)
188193
.hasSum(40)
189-
.hasBucketCounts(0, 0, 1, 2, 0, 0, 0, 0, 0, 0, 0)
194+
.hasBucketCounts(
195+
0, 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
190196
.hasAttributes(Attributes.empty()))));
191197
} finally {
192198
bound.unbind();
@@ -381,31 +387,35 @@ void stressTest_WithDifferentLabelSet() {
381387
.hasEpochNanos(testClock.now())
382388
.hasCount(2_000)
383389
.hasSum(20_000)
384-
.hasBucketCounts(0, 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0)
390+
.hasBucketCounts(
391+
0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
385392
.hasAttributes(attributeEntry(keys[0], values[0])),
386393
point ->
387394
point
388395
.hasStartEpochNanos(testClock.now())
389396
.hasEpochNanos(testClock.now())
390397
.hasCount(2_000)
391398
.hasSum(20_000)
392-
.hasBucketCounts(0, 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0)
399+
.hasBucketCounts(
400+
0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
393401
.hasAttributes(attributeEntry(keys[1], values[1])),
394402
point ->
395403
point
396404
.hasStartEpochNanos(testClock.now())
397405
.hasEpochNanos(testClock.now())
398406
.hasCount(2_000)
399407
.hasSum(20_000)
400-
.hasBucketCounts(0, 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0)
408+
.hasBucketCounts(
409+
0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
401410
.hasAttributes(attributeEntry(keys[2], values[2])),
402411
point ->
403412
point
404413
.hasStartEpochNanos(testClock.now())
405414
.hasEpochNanos(testClock.now())
406415
.hasCount(2_000)
407416
.hasSum(20_000)
408-
.hasBucketCounts(0, 0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0)
417+
.hasBucketCounts(
418+
0, 1000, 1000, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
409419
.hasAttributes(attributeEntry(keys[3], values[3])))));
410420
}
411421

‎sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ void collectAllSyncInstruments() {
130130
.hasAttributes(Attributes.empty())
131131
.hasCount(1)
132132
.hasSum(10.1)
133-
.hasBucketCounts(0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0))),
133+
.hasBucketCounts(
134+
0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0))),
134135
metric ->
135136
assertThat(metric)
136137
.hasName("testDoubleCounter")
@@ -158,7 +159,8 @@ void collectAllSyncInstruments() {
158159
.hasAttributes(Attributes.empty())
159160
.hasCount(1)
160161
.hasSum(10)
161-
.hasBucketCounts(0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0))),
162+
.hasBucketCounts(
163+
0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0))),
162164
metric ->
163165
assertThat(metric)
164166
.hasName("testLongUpDownCounter")

‎sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/ExplicitBucketHistogramUtilsTest.java

-20
This file was deleted.

0 commit comments

Comments
 (0)