diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-sdk-metrics.txt b/docs/apidiffs/current_vs_latest/opentelemetry-sdk-metrics.txt index d339d345d3c..16f6cb84754 100644 --- a/docs/apidiffs/current_vs_latest/opentelemetry-sdk-metrics.txt +++ b/docs/apidiffs/current_vs_latest/opentelemetry-sdk-metrics.txt @@ -1,2 +1,5 @@ Comparing source compatibility of opentelemetry-sdk-metrics-1.60.0-SNAPSHOT.jar against opentelemetry-sdk-metrics-1.59.0.jar -No changes. \ No newline at end of file +*** MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.sdk.metrics.Aggregation (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + +++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.Aggregation base2ExponentialBucketHistogram(int, int, boolean) + +++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.Aggregation explicitBucketHistogram(java.util.List, boolean) diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactory.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactory.java index ca2441f12f9..69e2c80dd4b 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactory.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactory.java @@ -10,6 +10,7 @@ import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Base2ExponentialBucketHistogramAggregationModel; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExplicitBucketHistogramAggregationModel; import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.internal.aggregator.ExplicitBucketHistogramUtils; import java.util.List; final class AggregationFactory implements Factory { @@ -44,8 +45,10 @@ public Aggregation create(AggregationModel model, DeclarativeConfigContext conte if (maxSize == null) { maxSize = 160; } + Boolean recordMinMax = exponentialBucketHistogram.getRecordMinMax(); + boolean shouldRecordMinMax = recordMinMax != null ? recordMinMax : true; try { - return Aggregation.base2ExponentialBucketHistogram(maxSize, maxScale); + return Aggregation.base2ExponentialBucketHistogram(maxSize, maxScale, shouldRecordMinMax); } catch (IllegalArgumentException e) { throw new DeclarativeConfigException("Invalid exponential bucket histogram", e); } @@ -54,11 +57,15 @@ public Aggregation create(AggregationModel model, DeclarativeConfigContext conte model.getExplicitBucketHistogram(); if (explicitBucketHistogram != null) { List boundaries = explicitBucketHistogram.getBoundaries(); + Boolean recordMinMax = explicitBucketHistogram.getRecordMinMax(); + boolean shouldRecordMinMax = recordMinMax != null ? recordMinMax : true; if (boundaries == null) { - return Aggregation.explicitBucketHistogram(); + // Use default boundaries with recordMinMax parameter + return Aggregation.explicitBucketHistogram( + ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES, shouldRecordMinMax); } try { - return Aggregation.explicitBucketHistogram(boundaries); + return Aggregation.explicitBucketHistogram(boundaries, shouldRecordMinMax); } catch (IllegalArgumentException e) { throw new DeclarativeConfigException("Invalid explicit bucket histogram", e); } diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java index 8cf4ccc6187..043d1d96797 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java @@ -62,6 +62,49 @@ private static Stream createTestCases() { .withExplicitBucketHistogram( new ExplicitBucketHistogramAggregationModel() .withBoundaries(Arrays.asList(1.0, 2.0))), - Aggregation.explicitBucketHistogram(Arrays.asList(1.0, 2.0)))); + Aggregation.explicitBucketHistogram(Arrays.asList(1.0, 2.0))), + // Test recordMinMax parameter for explicit bucket histogram + Arguments.of( + new AggregationModel() + .withExplicitBucketHistogram( + new ExplicitBucketHistogramAggregationModel() + .withBoundaries(Arrays.asList(1.0, 2.0)) + .withRecordMinMax(true)), + Aggregation.explicitBucketHistogram(Arrays.asList(1.0, 2.0), true)), + Arguments.of( + new AggregationModel() + .withExplicitBucketHistogram( + new ExplicitBucketHistogramAggregationModel() + .withBoundaries(Arrays.asList(1.0, 2.0)) + .withRecordMinMax(false)), + Aggregation.explicitBucketHistogram(Arrays.asList(1.0, 2.0), false)), + Arguments.of( + new AggregationModel() + .withExplicitBucketHistogram( + new ExplicitBucketHistogramAggregationModel() + .withBoundaries(null) + .withRecordMinMax(false)), + Aggregation.explicitBucketHistogram( + Arrays.asList( + 0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 750.0, 1000.0, 2500.0, + 5000.0, 7500.0, 10000.0), + false)), + // Test recordMinMax parameter for exponential bucket histogram + Arguments.of( + new AggregationModel() + .withBase2ExponentialBucketHistogram( + new Base2ExponentialBucketHistogramAggregationModel() + .withMaxSize(2) + .withMaxScale(2) + .withRecordMinMax(true)), + Aggregation.base2ExponentialBucketHistogram(2, 2, true)), + Arguments.of( + new AggregationModel() + .withBase2ExponentialBucketHistogram( + new Base2ExponentialBucketHistogramAggregationModel() + .withMaxSize(2) + .withMaxScale(2) + .withRecordMinMax(false)), + Aggregation.base2ExponentialBucketHistogram(2, 2, false))); } } diff --git a/sdk/metrics/src/jmh/java/io/opentelemetry/sdk/metrics/internal/aggregator/HistogramAggregationParam.java b/sdk/metrics/src/jmh/java/io/opentelemetry/sdk/metrics/internal/aggregator/HistogramAggregationParam.java index 71bf89803cc..30c78a0500e 100644 --- a/sdk/metrics/src/jmh/java/io/opentelemetry/sdk/metrics/internal/aggregator/HistogramAggregationParam.java +++ b/sdk/metrics/src/jmh/java/io/opentelemetry/sdk/metrics/internal/aggregator/HistogramAggregationParam.java @@ -17,19 +17,21 @@ public enum HistogramAggregationParam { new DoubleExplicitBucketHistogramAggregator( ExplicitBucketHistogramUtils.createBoundaryArray( ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES), + /* recordMinMax= */ true, ExemplarReservoirFactory.noSamples(), IMMUTABLE_DATA)), EXPLICIT_SINGLE_BUCKET( new DoubleExplicitBucketHistogramAggregator( ExplicitBucketHistogramUtils.createBoundaryArray(Collections.emptyList()), + /* recordMinMax= */ true, ExemplarReservoirFactory.noSamples(), IMMUTABLE_DATA)), EXPONENTIAL_SMALL_CIRCULAR_BUFFER( new DoubleBase2ExponentialHistogramAggregator( - ExemplarReservoirFactory.noSamples(), 20, 0, IMMUTABLE_DATA)), + ExemplarReservoirFactory.noSamples(), 20, 0, /* recordMinMax= */ true, IMMUTABLE_DATA)), EXPONENTIAL_CIRCULAR_BUFFER( new DoubleBase2ExponentialHistogramAggregator( - ExemplarReservoirFactory.noSamples(), 160, 0, IMMUTABLE_DATA)); + ExemplarReservoirFactory.noSamples(), 160, 0, /* recordMinMax= */ true, IMMUTABLE_DATA)); private final Aggregator aggregator; diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Aggregation.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Aggregation.java index ba8470a8ed9..65aa97873b8 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Aggregation.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Aggregation.java @@ -66,7 +66,18 @@ static Aggregation explicitBucketHistogram() { * order from lowest to highest. */ static Aggregation explicitBucketHistogram(List bucketBoundaries) { - return ExplicitBucketHistogramAggregation.create(bucketBoundaries); + return ExplicitBucketHistogramAggregation.create(bucketBoundaries, /* recordMinMax= */ true); + } + + /** + * Aggregates measurements into an explicit bucket {@link MetricDataType#HISTOGRAM}. + * + * @param bucketBoundaries A list of (inclusive) upper bounds for the histogram. Should be in + * order from lowest to highest. + * @param recordMinMax whether to record min and max values + */ + static Aggregation explicitBucketHistogram(List bucketBoundaries, boolean recordMinMax) { + return ExplicitBucketHistogramAggregation.create(bucketBoundaries, recordMinMax); } /** @@ -91,6 +102,23 @@ static Aggregation base2ExponentialBucketHistogram() { * @since 1.23.0 */ static Aggregation base2ExponentialBucketHistogram(int maxBuckets, int maxScale) { - return Base2ExponentialHistogramAggregation.create(maxBuckets, maxScale); + return Base2ExponentialHistogramAggregation.create( + maxBuckets, maxScale, /* recordMinMax= */ true); + } + + /** + * Aggregates measurements into a base-2 {@link MetricDataType#EXPONENTIAL_HISTOGRAM}. + * + * @param maxBuckets the max number of positive buckets and negative buckets (max total buckets is + * 2 * {@code maxBuckets} + 1 zero bucket). + * @param maxScale the maximum and initial scale. If measurements can't fit in a particular scale + * given the {@code maxBuckets}, the scale is reduced until the measurements can be + * accommodated. Setting maxScale may reduce the number of downscales. Additionally, the + * performance of computing bucket index is improved when scale is {@code <= 0}. + * @param recordMinMax whether to record min and max values + */ + static Aggregation base2ExponentialBucketHistogram( + int maxBuckets, int maxScale, boolean recordMinMax) { + return Base2ExponentialHistogramAggregation.create(maxBuckets, maxScale, recordMinMax); } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleBase2ExponentialHistogramAggregator.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleBase2ExponentialHistogramAggregator.java index 28c18a78d46..072f79a3e1c 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleBase2ExponentialHistogramAggregator.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleBase2ExponentialHistogramAggregator.java @@ -40,27 +40,34 @@ public final class DoubleBase2ExponentialHistogramAggregator private final ExemplarReservoirFactory reservoirFactory; private final int maxBuckets; private final int maxScale; + private final boolean recordMinMax; private final MemoryMode memoryMode; /** * Constructs an exponential histogram aggregator. * * @param reservoirFactory Supplier of exemplar reservoirs per-stream. + * @param maxBuckets maximum number of buckets in each of the positive and negative ranges + * @param maxScale maximum scale factor + * @param recordMinMax whether to record min and max values + * @param memoryMode The {@link MemoryMode} to use in this aggregator. */ public DoubleBase2ExponentialHistogramAggregator( ExemplarReservoirFactory reservoirFactory, int maxBuckets, int maxScale, + boolean recordMinMax, MemoryMode memoryMode) { this.reservoirFactory = reservoirFactory; this.maxBuckets = maxBuckets; this.maxScale = maxScale; + this.recordMinMax = recordMinMax; this.memoryMode = memoryMode; } @Override public AggregatorHandle createHandle() { - return new Handle(reservoirFactory, maxBuckets, maxScale, memoryMode); + return new Handle(reservoirFactory, maxBuckets, maxScale, recordMinMax, memoryMode); } @Override @@ -82,6 +89,7 @@ public MetricData toMetricData( static final class Handle extends AggregatorHandle { private final int maxBuckets; private final int maxScale; + private final boolean recordMinMax; @Nullable private DoubleBase2ExponentialHistogramBuckets positiveBuckets; @Nullable private DoubleBase2ExponentialHistogramBuckets negativeBuckets; private long zeroCount; @@ -99,10 +107,12 @@ static final class Handle extends AggregatorHandle 0, - this.min, - this.count > 0, - this.max, + recordMinMax && this.count > 0, + recordMinMax ? this.min : 0, + recordMinMax && this.count > 0, + recordMinMax ? this.max : 0, resolveBuckets( this.positiveBuckets, currentScale, reset, /* reusableBuckets= */ null), resolveBuckets( @@ -149,10 +159,10 @@ protected synchronized ExponentialHistogramPointData doAggregateThenMaybeResetDo currentScale, sum, zeroCount, - this.count > 0, - this.min, - this.count > 0, - this.max, + recordMinMax && this.count > 0, + recordMinMax ? this.min : 0, + recordMinMax && this.count > 0, + recordMinMax ? this.max : 0, resolveBuckets( this.positiveBuckets, currentScale, reset, reusablePoint.getPositiveBuckets()), resolveBuckets( @@ -222,8 +232,10 @@ protected synchronized void doRecordDouble(double value) { sum += value; - this.min = Math.min(this.min, value); - this.max = Math.max(this.max, value); + if (recordMinMax) { + this.min = Math.min(this.min, value); + this.max = Math.max(this.max, value); + } count++; int c = Double.compare(value, 0); diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregator.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregator.java index 0d3ce15c1bf..3d7cf1ceb31 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregator.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregator.java @@ -38,6 +38,7 @@ public final class DoubleExplicitBucketHistogramAggregator implements Aggregator { private final double[] boundaries; + private final boolean recordMinMax; private final MemoryMode memoryMode; // a cache for converting to MetricData @@ -49,12 +50,17 @@ public final class DoubleExplicitBucketHistogramAggregator * Constructs an explicit bucket histogram aggregator. * * @param boundaries Bucket boundaries, in-order. + * @param recordMinMax whether to record min and max values * @param reservoirFactory Supplier of exemplar reservoirs per-stream. * @param memoryMode The {@link MemoryMode} to use in this aggregator. */ public DoubleExplicitBucketHistogramAggregator( - double[] boundaries, ExemplarReservoirFactory reservoirFactory, MemoryMode memoryMode) { + double[] boundaries, + boolean recordMinMax, + ExemplarReservoirFactory reservoirFactory, + MemoryMode memoryMode) { this.boundaries = boundaries; + this.recordMinMax = recordMinMax; this.memoryMode = memoryMode; List boundaryList = new ArrayList<>(this.boundaries.length); @@ -67,7 +73,7 @@ public DoubleExplicitBucketHistogramAggregator( @Override public AggregatorHandle createHandle() { - return new Handle(boundaryList, boundaries, reservoirFactory, memoryMode); + return new Handle(boundaryList, boundaries, recordMinMax, reservoirFactory, memoryMode); } @Override @@ -91,6 +97,7 @@ static final class Handle extends AggregatorHandle { private final List boundaryList; // read-only private final double[] boundaries; + private final boolean recordMinMax; private final Object lock = new Object(); @@ -115,11 +122,13 @@ static final class Handle extends AggregatorHandle { Handle( List boundaryList, double[] boundaries, + boolean recordMinMax, ExemplarReservoirFactory reservoirFactory, MemoryMode memoryMode) { super(reservoirFactory, /* isDoubleType= */ true); this.boundaryList = boundaryList; this.boundaries = boundaries; + this.recordMinMax = recordMinMax; this.counts = new long[this.boundaries.length + 1]; this.sum = 0; this.min = Double.MAX_VALUE; @@ -156,10 +165,10 @@ protected HistogramPointData doAggregateThenMaybeResetDoubles( epochNanos, attributes, sum, - this.count > 0, - this.min, - this.count > 0, - this.max, + recordMinMax && this.count > 0, + recordMinMax ? this.min : 0, + recordMinMax && this.count > 0, + recordMinMax ? this.max : 0, boundaryList, PrimitiveLongList.wrap(Arrays.copyOf(counts, counts.length)), exemplars); @@ -170,10 +179,10 @@ protected HistogramPointData doAggregateThenMaybeResetDoubles( epochNanos, attributes, sum, - this.count > 0, - this.min, - this.count > 0, - this.max, + recordMinMax && this.count > 0, + recordMinMax ? this.min : 0, + recordMinMax && this.count > 0, + recordMinMax ? this.max : 0, boundaryList, counts, exemplars); @@ -195,8 +204,10 @@ protected void doRecordDouble(double value) { synchronized (lock) { this.sum += value; - this.min = Math.min(this.min, value); - this.max = Math.max(this.max, value); + if (recordMinMax) { + this.min = Math.min(this.min, value); + this.max = Math.max(this.max, value); + } this.count++; this.counts[bucketIndex]++; } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java index 6e354d7069c..c69edcbe2bd 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java @@ -32,14 +32,17 @@ public final class Base2ExponentialHistogramAggregation implements Aggregation, private static final int DEFAULT_MAX_SCALE = 20; private static final Aggregation DEFAULT = - new Base2ExponentialHistogramAggregation(DEFAULT_MAX_BUCKETS, DEFAULT_MAX_SCALE); + new Base2ExponentialHistogramAggregation( + DEFAULT_MAX_BUCKETS, DEFAULT_MAX_SCALE, /* recordMinMax= */ true); private final int maxBuckets; private final int maxScale; + private final boolean recordMinMax; - private Base2ExponentialHistogramAggregation(int maxBuckets, int maxScale) { + private Base2ExponentialHistogramAggregation(int maxBuckets, int maxScale, boolean recordMinMax) { this.maxBuckets = maxBuckets; this.maxScale = maxScale; + this.recordMinMax = recordMinMax; } public static Aggregation getDefault() { @@ -55,12 +58,13 @@ public static Aggregation getDefault() { * given the {@code maxBuckets}, the scale is reduced until the measurements can be * accommodated. Setting maxScale may reduce the number of downscales. Additionally, the * performance of computing bucket index is improved when scale is <= 0. + * @param recordMinMax whether to record min and max values * @return the aggregation */ - public static Aggregation create(int maxBuckets, int maxScale) { + public static Aggregation create(int maxBuckets, int maxScale, boolean recordMinMax) { checkArgument(maxBuckets >= 2, "maxBuckets must be >= 2"); checkArgument(maxScale <= 20 && maxScale >= -10, "maxScale must be -10 <= x <= 20"); - return new Base2ExponentialHistogramAggregation(maxBuckets, maxScale); + return new Base2ExponentialHistogramAggregation(maxBuckets, maxScale, recordMinMax); } @Override @@ -79,6 +83,7 @@ public Aggregator createAggregator( RandomSupplier.platformDefault())), maxBuckets, maxScale, + recordMinMax, memoryMode); } @@ -99,6 +104,8 @@ public String toString() { + maxBuckets + ",maxScale=" + maxScale + + ",recordMinMax=" + + recordMinMax + "}"; } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/DefaultAggregation.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/DefaultAggregation.java index d44a48269bf..dac425543c2 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/DefaultAggregation.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/DefaultAggregation.java @@ -45,7 +45,7 @@ private static Aggregation resolve(InstrumentDescriptor instrument, boolean with case HISTOGRAM: if (withAdvice && instrument.getAdvice().getExplicitBucketBoundaries() != null) { return ExplicitBucketHistogramAggregation.create( - instrument.getAdvice().getExplicitBucketBoundaries()); + instrument.getAdvice().getExplicitBucketBoundaries(), /* recordMinMax= */ true); } return ExplicitBucketHistogramAggregation.getDefault(); case OBSERVABLE_GAUGE: diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java index d4af55fd0ad..0a9f13f2f1f 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java @@ -28,23 +28,26 @@ public final class ExplicitBucketHistogramAggregation implements Aggregation, Ag private static final Aggregation DEFAULT = new ExplicitBucketHistogramAggregation( - ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES); + ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES, + /* recordMinMax= */ true); public static Aggregation getDefault() { return DEFAULT; } - public static Aggregation create(List bucketBoundaries) { - return new ExplicitBucketHistogramAggregation(bucketBoundaries); + public static Aggregation create(List bucketBoundaries, boolean recordMinMax) { + return new ExplicitBucketHistogramAggregation(bucketBoundaries, recordMinMax); } private final List bucketBoundaries; private final double[] bucketBoundaryArray; + private final boolean recordMinMax; - private ExplicitBucketHistogramAggregation(List bucketBoundaries) { + private ExplicitBucketHistogramAggregation(List bucketBoundaries, boolean recordMinMax) { this.bucketBoundaries = bucketBoundaries; // We need to fail here if our bucket boundaries are ill-configured. this.bucketBoundaryArray = ExplicitBucketHistogramUtils.createBoundaryArray(bucketBoundaries); + this.recordMinMax = recordMinMax; } @Override @@ -56,6 +59,7 @@ public Aggregator createAggregator( return (Aggregator) new DoubleExplicitBucketHistogramAggregator( bucketBoundaryArray, + recordMinMax, ExemplarReservoirFactory.filtered( exemplarFilter, ExemplarReservoirFactory.histogramBucketReservoir( @@ -76,6 +80,11 @@ public boolean isCompatibleWithInstrument(InstrumentDescriptor instrumentDescrip @Override public String toString() { - return "ExplicitBucketHistogramAggregation(" + bucketBoundaries.toString() + ")"; + return "ExplicitBucketHistogramAggregation{" + + "bucketBoundaries=" + + bucketBoundaries + + ",recordMinMax=" + + recordMinMax + + "}"; } } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AggregationTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AggregationTest.java index be2502ec054..8a8fe924d1b 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AggregationTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AggregationTest.java @@ -30,10 +30,12 @@ void haveToString() { .contains("ExplicitBucketHistogramAggregation"); assertThat(Aggregation.base2ExponentialBucketHistogram()) .asString() - .isEqualTo("Base2ExponentialHistogramAggregation{maxBuckets=160,maxScale=20}"); + .isEqualTo( + "Base2ExponentialHistogramAggregation{maxBuckets=160,maxScale=20,recordMinMax=true}"); assertThat(Aggregation.base2ExponentialBucketHistogram(2, 0)) .asString() - .isEqualTo("Base2ExponentialHistogramAggregation{maxBuckets=2,maxScale=0}"); + .isEqualTo( + "Base2ExponentialHistogramAggregation{maxBuckets=2,maxScale=0,recordMinMax=true}"); } @Test diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleBase2ExponentialHistogramAggregatorTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleBase2ExponentialHistogramAggregatorTest.java index 763d36fbec6..47486fab69e 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleBase2ExponentialHistogramAggregatorTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleBase2ExponentialHistogramAggregatorTest.java @@ -82,10 +82,14 @@ private static Stream provideAggregat for (MemoryMode memoryMode : MemoryMode.values()) { parameters.add( new DoubleBase2ExponentialHistogramAggregator( - ExemplarReservoirFactory.noSamples(), 160, 20, memoryMode)); + ExemplarReservoirFactory.noSamples(), 160, 20, /* recordMinMax= */ true, memoryMode)); parameters.add( new DoubleBase2ExponentialHistogramAggregator( - ExemplarReservoirFactory.noSamples(), 160, MAX_SCALE, memoryMode)); + ExemplarReservoirFactory.noSamples(), + 160, + MAX_SCALE, + /* recordMinMax= */ true, + memoryMode)); } return parameters.stream(); } @@ -98,7 +102,7 @@ private static int valueToIndex(int scale, double value) { private void initialize(MemoryMode memoryMode) { aggregator = new DoubleBase2ExponentialHistogramAggregator( - ExemplarReservoirFactory.noSamples(), 160, 20, memoryMode); + ExemplarReservoirFactory.noSamples(), 160, 20, /* recordMinMax= */ true, memoryMode); } @ParameterizedTest @@ -230,7 +234,8 @@ void testRecordingsAtLimits(DoubleBase2ExponentialHistogramAggregator aggregator @EnumSource(MemoryMode.class) void aggregateThenMaybeReset_WithExemplars(MemoryMode memoryMode) { DoubleBase2ExponentialHistogramAggregator agg = - new DoubleBase2ExponentialHistogramAggregator(reservoirFactory, 160, MAX_SCALE, memoryMode); + new DoubleBase2ExponentialHistogramAggregator( + reservoirFactory, 160, MAX_SCALE, /* recordMinMax= */ true, memoryMode); Attributes attributes = Attributes.builder().put("test", "value").build(); DoubleExemplarData exemplar = @@ -348,7 +353,8 @@ void testToMetricData(MemoryMode memoryMode) { .thenReturn(Collections.singletonList(exemplar)); DoubleBase2ExponentialHistogramAggregator cumulativeAggregator = - new DoubleBase2ExponentialHistogramAggregator(reservoirFactory, 160, MAX_SCALE, memoryMode); + new DoubleBase2ExponentialHistogramAggregator( + reservoirFactory, 160, MAX_SCALE, /* recordMinMax= */ true, memoryMode); AggregatorHandle aggregatorHandle = cumulativeAggregator.createHandle(); @@ -568,4 +574,27 @@ public void reusablePoint_emptyFirstThenRecordAndCheck() { assertThat(point.getPositiveBuckets().getBucketCounts()).isNotEmpty(); assertThat(point.getNegativeBuckets().getBucketCounts()).isNotEmpty(); } + + @ParameterizedTest + @EnumSource(MemoryMode.class) + void testRecordMinMaxDisabled(MemoryMode memoryMode) { + DoubleBase2ExponentialHistogramAggregator aggregator = + new DoubleBase2ExponentialHistogramAggregator( + ExemplarReservoirFactory.noSamples(), 160, 20, /* recordMinMax= */ false, memoryMode); + AggregatorHandle aggregatorHandle = aggregator.createHandle(); + aggregatorHandle.recordDouble(0.5, Attributes.empty(), Context.current()); + aggregatorHandle.recordDouble(1.0, Attributes.empty(), Context.current()); + aggregatorHandle.recordDouble(12.0, Attributes.empty(), Context.current()); + aggregatorHandle.recordDouble(15.213, Attributes.empty(), Context.current()); + aggregatorHandle.recordDouble(-13.2, Attributes.empty(), Context.current()); + aggregatorHandle.recordDouble(-2.01, Attributes.empty(), Context.current()); + + ExponentialHistogramPointData point = + aggregatorHandle.aggregateThenMaybeReset(0, 1, Attributes.empty(), /* reset= */ true); + assertThat(point).isNotNull(); + assertThat(point.hasMin()).isFalse(); + assertThat(point.hasMax()).isFalse(); + assertThat(point.getSum()).isCloseTo(13.503, Offset.offset(0.0001)); + assertThat(point.getCount()).isEqualTo(6); + } } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregatorTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregatorTest.java index 2e425e5cd0c..66fd7dd43b6 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregatorTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/aggregator/DoubleExplicitBucketHistogramAggregatorTest.java @@ -70,7 +70,7 @@ public LongExemplarReservoir createLongExemplarReservoir() { private void init(MemoryMode memoryMode) { aggregator = new DoubleExplicitBucketHistogramAggregator( - boundaries, ExemplarReservoirFactory.noSamples(), memoryMode); + boundaries, /* recordMinMax= */ true, ExemplarReservoirFactory.noSamples(), memoryMode); } @ParameterizedTest @@ -123,7 +123,8 @@ void aggregateThenMaybeReset_WithExemplars(MemoryMode memoryMode) { List exemplars = Collections.singletonList(exemplar); Mockito.when(reservoir.collectAndResetDoubles(Attributes.empty())).thenReturn(exemplars); DoubleExplicitBucketHistogramAggregator aggregator = - new DoubleExplicitBucketHistogramAggregator(boundaries, reservoirFactory, memoryMode); + new DoubleExplicitBucketHistogramAggregator( + boundaries, /* recordMinMax= */ true, reservoirFactory, memoryMode); AggregatorHandle aggregatorHandle = aggregator.createHandle(); aggregatorHandle.recordDouble(0, attributes, Context.root()); assertThat( @@ -286,4 +287,27 @@ void testReusableDataMemoryMode() { // The point data instance should be reused assertThat(anotherPointData).isSameAs(pointData); } + + @ParameterizedTest + @EnumSource(MemoryMode.class) + void testRecordMinMaxDisabled(MemoryMode memoryMode) { + DoubleExplicitBucketHistogramAggregator aggregator = + new DoubleExplicitBucketHistogramAggregator( + boundaries, + /* recordMinMax= */ false, + ExemplarReservoirFactory.noSamples(), + memoryMode); + AggregatorHandle aggregatorHandle = aggregator.createHandle(); + aggregatorHandle.recordLong(20, Attributes.empty(), Context.current()); + aggregatorHandle.recordLong(5, Attributes.empty(), Context.current()); + aggregatorHandle.recordLong(150, Attributes.empty(), Context.current()); + aggregatorHandle.recordLong(2000, Attributes.empty(), Context.current()); + HistogramPointData point = + aggregatorHandle.aggregateThenMaybeReset(0, 1, Attributes.empty(), /* reset= */ true); + assertThat(point.getSum()).isEqualTo(2175); + assertThat(point.hasMin()).isFalse(); + assertThat(point.hasMax()).isFalse(); + assertThat(point.getBoundaries()).isEqualTo(boundariesList); + assertThat(point.getCounts()).isEqualTo(Arrays.asList(1L, 1L, 1L, 1L)); + } } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregationTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregationTest.java index ee10e75d679..ff33359ddcb 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregationTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregationTest.java @@ -29,25 +29,30 @@ class Base2ExponentialHistogramAggregationTest { @Test void goodConfig() { assertThat(Base2ExponentialHistogramAggregation.getDefault()).isNotNull(); - assertThat(Base2ExponentialHistogramAggregation.create(10, 20)).isNotNull(); + assertThat(Base2ExponentialHistogramAggregation.create(10, 20, /* recordMinMax= */ true)) + .isNotNull(); } @Test void invalidConfig_Throws() { - assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(0, 20)) + assertThatThrownBy( + () -> Base2ExponentialHistogramAggregation.create(0, 20, /* recordMinMax= */ true)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("maxBuckets must be >= 2"); - assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(2, 21)) + assertThatThrownBy( + () -> Base2ExponentialHistogramAggregation.create(2, 21, /* recordMinMax= */ true)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("maxScale must be -10 <= x <= 20"); - assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(2, -11)) + assertThatThrownBy( + () -> Base2ExponentialHistogramAggregation.create(2, -11, /* recordMinMax= */ true)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("maxScale must be -10 <= x <= 20"); } @Test void minimumBucketsCanAccommodateMaxRange() { - Aggregation aggregation = Base2ExponentialHistogramAggregation.create(2, 20); + Aggregation aggregation = + Base2ExponentialHistogramAggregation.create(2, 20, /* recordMinMax= */ true); Aggregator aggregator = ((AggregatorFactory) aggregation) .createAggregator(