Skip to content

Commit 160af1c

Browse files
authored
Restore prior histogram defaults (#4717)
* Add property for configuring legacy explicit bucket histogram bucket boundaries * Add clarifying comment * Spotless
1 parent def1c85 commit 160af1c

File tree

9 files changed

+93
-7
lines changed

9 files changed

+93
-7
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 debugEnabledTest by registering(JvmTestSuite::class) {
35+
val configurationTest by registering(JvmTestSuite::class) {
3636
targets {
3737
all {
3838
testTask.configure {
39-
jvmArgs("-Dotel.experimental.sdk.metrics.debug=true")
39+
jvmArgs("-Dotel.experimental.sdk.metrics.debug=true", "-Dotel.java.histogram.legacy.buckets.enabled=true")
4040
}
4141
}
4242
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.sdk.metrics.internal.aggregator;
7+
8+
import static org.assertj.core.api.Assertions.assertThat;
9+
10+
import java.util.Arrays;
11+
import org.junit.jupiter.api.Test;
12+
13+
class ExplicitBucketHistogramUtilsTest {
14+
15+
/**
16+
* {@link ExplicitBucketHistogramUtils#getDefaultBucketBoundaries()} returns different values when
17+
* {@code otel.java.histogram.legacy.buckets.enabled=true}, which is set for this test in {@code
18+
* build.gradle.kts}.
19+
*/
20+
@Test
21+
void defaultBucketBoundaries() {
22+
assertThat(ExplicitBucketHistogramUtils.getDefaultBucketBoundaries())
23+
.isEqualTo(
24+
Arrays.asList(
25+
5d, 10d, 25d, 50d, 75d, 100d, 250d, 500d, 750d, 1_000d, 2_500d, 5_000d, 7_500d,
26+
10_000d));
27+
}
28+
}

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.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES),
18+
ExplicitBucketHistogramUtils.getDefaultBucketBoundaries()),
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.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES);
67+
fixedBoundaries.addAll(ExplicitBucketHistogramUtils.getDefaultBucketBoundaries());
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

+40-2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
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;
1114

1215
/**
1316
* Utilities for interacting with explicit bucket histograms.
@@ -16,11 +19,46 @@
1619
* at any time.
1720
*/
1821
public final class ExplicitBucketHistogramUtils {
19-
private ExplicitBucketHistogramUtils() {}
2022

21-
public static final List<Double> DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES =
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 =
2227
Collections.unmodifiableList(
2328
Arrays.asList(0d, 5d, 10d, 25d, 50d, 75d, 100d, 250d, 500d, 1_000d));
29+
private static final List<Double> LEGACY_HISTOGRAM_BUCKET_BOUNDARIES =
30+
Collections.unmodifiableList(
31+
Arrays.asList(
32+
5d, 10d, 25d, 50d, 75d, 100d, 250d, 500d, 750d, 1_000d, 2_500d, 5_000d, 7_500d,
33+
10_000d));
34+
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+
}
2462

2563
/** Converts bucket boundary "convenient" configuration into the "more efficient" array. */
2664
public static double[] createBoundaryArray(List<Double> boundaries) {

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.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES);
30+
ExplicitBucketHistogramUtils.getDefaultBucketBoundaries());
3131

3232
public static Aggregation getDefault() {
3333
return DEFAULT;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.sdk.metrics.internal.aggregator;
7+
8+
import static org.assertj.core.api.Assertions.assertThat;
9+
10+
import java.util.Arrays;
11+
import org.junit.jupiter.api.Test;
12+
13+
class ExplicitBucketHistogramUtilsTest {
14+
15+
@Test
16+
void defaultBucketBoundaries() {
17+
assertThat(ExplicitBucketHistogramUtils.getDefaultBucketBoundaries())
18+
.isEqualTo(Arrays.asList(0d, 5d, 10d, 25d, 50d, 75d, 100d, 250d, 500d, 1_000d));
19+
}
20+
}

0 commit comments

Comments
 (0)