Skip to content

Commit 849cc50

Browse files
committed
Address review feedback
- Remove @SInCE annotations (added during release flow) - Remove old create() overloads without recordMinMax, update callers - Remove redundant // read-only comment on final primitive field Claude claude-sonnet-4-6 assisted with this change.
1 parent 3d34d6c commit 849cc50

File tree

6 files changed

+8
-32
lines changed

6 files changed

+8
-32
lines changed

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Aggregation.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ static Aggregation explicitBucketHistogram() {
6666
* order from lowest to highest.
6767
*/
6868
static Aggregation explicitBucketHistogram(List<Double> bucketBoundaries) {
69-
return ExplicitBucketHistogramAggregation.create(bucketBoundaries);
69+
return ExplicitBucketHistogramAggregation.create(bucketBoundaries, /* recordMinMax= */ true);
7070
}
7171

7272
/**
@@ -75,7 +75,6 @@ static Aggregation explicitBucketHistogram(List<Double> bucketBoundaries) {
7575
* @param bucketBoundaries A list of (inclusive) upper bounds for the histogram. Should be in
7676
* order from lowest to highest.
7777
* @param recordMinMax whether to record min and max values
78-
* @since 1.45.0
7978
*/
8079
static Aggregation explicitBucketHistogram(List<Double> bucketBoundaries, boolean recordMinMax) {
8180
return ExplicitBucketHistogramAggregation.create(bucketBoundaries, recordMinMax);
@@ -103,7 +102,7 @@ static Aggregation base2ExponentialBucketHistogram() {
103102
* @since 1.23.0
104103
*/
105104
static Aggregation base2ExponentialBucketHistogram(int maxBuckets, int maxScale) {
106-
return Base2ExponentialHistogramAggregation.create(maxBuckets, maxScale);
105+
return Base2ExponentialHistogramAggregation.create(maxBuckets, maxScale, /* recordMinMax= */ true);
107106
}
108107

109108
/**
@@ -116,7 +115,6 @@ static Aggregation base2ExponentialBucketHistogram(int maxBuckets, int maxScale)
116115
* accommodated. Setting maxScale may reduce the number of downscales. Additionally, the
117116
* performance of computing bucket index is improved when scale is {@code <= 0}.
118117
* @param recordMinMax whether to record min and max values
119-
* @since 1.45.0
120118
*/
121119
static Aggregation base2ExponentialBucketHistogram(
122120
int maxBuckets, int maxScale, boolean recordMinMax) {

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregator.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ static final class Handle extends AggregatorHandle<HistogramPointData> {
9797
private final List<Double> boundaryList;
9898
// read-only
9999
private final double[] boundaries;
100-
// read-only
101100
private final boolean recordMinMax;
102101

103102
private final Object lock = new Object();

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,23 +49,6 @@ public static Aggregation getDefault() {
4949
return DEFAULT;
5050
}
5151

52-
/**
53-
* Aggregations measurements into an {@link MetricDataType#EXPONENTIAL_HISTOGRAM}.
54-
*
55-
* @param maxBuckets the max number of positive buckets and negative buckets (max total buckets is
56-
* 2 * {@code maxBuckets} + 1 zero bucket).
57-
* @param maxScale the maximum and initial scale. If measurements can't fit in a particular scale
58-
* given the {@code maxBuckets}, the scale is reduced until the measurements can be
59-
* accommodated. Setting maxScale may reduce the number of downscales. Additionally, the
60-
* performance of computing bucket index is improved when scale is <= 0.
61-
* @return the aggregation
62-
*/
63-
public static Aggregation create(int maxBuckets, int maxScale) {
64-
checkArgument(maxBuckets >= 2, "maxBuckets must be >= 2");
65-
checkArgument(maxScale <= 20 && maxScale >= -10, "maxScale must be -10 <= x <= 20");
66-
return new Base2ExponentialHistogramAggregation(maxBuckets, maxScale, /* recordMinMax= */ true);
67-
}
68-
6952
/**
7053
* Aggregations measurements into an {@link MetricDataType#EXPONENTIAL_HISTOGRAM}.
7154
*

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/DefaultAggregation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ private static Aggregation resolve(InstrumentDescriptor instrument, boolean with
4545
case HISTOGRAM:
4646
if (withAdvice && instrument.getAdvice().getExplicitBucketBoundaries() != null) {
4747
return ExplicitBucketHistogramAggregation.create(
48-
instrument.getAdvice().getExplicitBucketBoundaries());
48+
instrument.getAdvice().getExplicitBucketBoundaries(), /* recordMinMax= */ true);
4949
}
5050
return ExplicitBucketHistogramAggregation.getDefault();
5151
case OBSERVABLE_GAUGE:

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@ public static Aggregation getDefault() {
3535
return DEFAULT;
3636
}
3737

38-
public static Aggregation create(List<Double> bucketBoundaries) {
39-
return new ExplicitBucketHistogramAggregation(bucketBoundaries, /* recordMinMax= */ true);
40-
}
41-
4238
public static Aggregation create(List<Double> bucketBoundaries, boolean recordMinMax) {
4339
return new ExplicitBucketHistogramAggregation(bucketBoundaries, recordMinMax);
4440
}

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregationTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,25 +29,25 @@ class Base2ExponentialHistogramAggregationTest {
2929
@Test
3030
void goodConfig() {
3131
assertThat(Base2ExponentialHistogramAggregation.getDefault()).isNotNull();
32-
assertThat(Base2ExponentialHistogramAggregation.create(10, 20)).isNotNull();
32+
assertThat(Base2ExponentialHistogramAggregation.create(10, 20, /* recordMinMax= */ true)).isNotNull();
3333
}
3434

3535
@Test
3636
void invalidConfig_Throws() {
37-
assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(0, 20))
37+
assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(0, 20, /* recordMinMax= */ true))
3838
.isInstanceOf(IllegalArgumentException.class)
3939
.hasMessage("maxBuckets must be >= 2");
40-
assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(2, 21))
40+
assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(2, 21, /* recordMinMax= */ true))
4141
.isInstanceOf(IllegalArgumentException.class)
4242
.hasMessage("maxScale must be -10 <= x <= 20");
43-
assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(2, -11))
43+
assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(2, -11, /* recordMinMax= */ true))
4444
.isInstanceOf(IllegalArgumentException.class)
4545
.hasMessage("maxScale must be -10 <= x <= 20");
4646
}
4747

4848
@Test
4949
void minimumBucketsCanAccommodateMaxRange() {
50-
Aggregation aggregation = Base2ExponentialHistogramAggregation.create(2, 20);
50+
Aggregation aggregation = Base2ExponentialHistogramAggregation.create(2, 20, /* recordMinMax= */ true);
5151
Aggregator<ExponentialHistogramPointData> aggregator =
5252
((AggregatorFactory) aggregation)
5353
.createAggregator(

0 commit comments

Comments
 (0)