Base2ExponentialHistogramAggregation maxBuckets must be >= 2

open-telemetry · Dec 21, 2023 · 332a50c · 332a50c
1 parent eea1fe0
commit 332a50c
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 8 deletions.
diff --git a/...test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java b/...test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java
@@ -62,8 +62,8 @@ private static Stream<Arguments> createTestCases() {
         Arguments.of(
             new Aggregation()
                 .withBase2ExponentialBucketHistogram(
-                    new Base2ExponentialBucketHistogram().withMaxSize(1).withMaxScale(2)),
-            io.opentelemetry.sdk.metrics.Aggregation.base2ExponentialBucketHistogram(1, 2)),
+                    new Base2ExponentialBucketHistogram().withMaxSize(2).withMaxScale(2)),
+            io.opentelemetry.sdk.metrics.Aggregation.base2ExponentialBucketHistogram(2, 2)),
         Arguments.of(
             new Aggregation()
                 .withExplicitBucketHistogram(new ExplicitBucketHistogram().withBoundaries(null)),

diff --git a/...java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java b/...java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java
@@ -58,7 +58,7 @@ public static Aggregation getDefault() {
    * @return the aggregation
    */
   public static Aggregation create(int maxBuckets, int maxScale) {
-    checkArgument(maxBuckets >= 1, "maxBuckets must be > 0");
+    checkArgument(maxBuckets >= 2, "maxBuckets must be >= 2");
     checkArgument(maxScale <= 20 && maxScale >= -10, "maxScale must be -10 <= x <= 20");
     return new Base2ExponentialHistogramAggregation(maxBuckets, maxScale);
   }

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
@@ -31,9 +31,9 @@ void haveToString() {
     assertThat(Aggregation.base2ExponentialBucketHistogram())
         .asString()
         .isEqualTo("Base2ExponentialHistogramAggregation{maxBuckets=160,maxScale=20}");
-    assertThat(Aggregation.base2ExponentialBucketHistogram(1, 0))
+    assertThat(Aggregation.base2ExponentialBucketHistogram(2, 0))
         .asString()
-        .isEqualTo("Base2ExponentialHistogramAggregation{maxBuckets=1,maxScale=0}");
+        .isEqualTo("Base2ExponentialHistogramAggregation{maxBuckets=2,maxScale=0}");
   }
 
   @Test

diff --git a/.../io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregationTest.java b/.../io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregationTest.java
@@ -8,6 +8,18 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.sdk.metrics.Aggregation;
+import io.opentelemetry.sdk.metrics.InstrumentType;
+import io.opentelemetry.sdk.metrics.InstrumentValueType;
+import io.opentelemetry.sdk.metrics.data.ExemplarData;
+import io.opentelemetry.sdk.metrics.data.ExponentialHistogramPointData;
+import io.opentelemetry.sdk.metrics.internal.aggregator.Aggregator;
+import io.opentelemetry.sdk.metrics.internal.aggregator.AggregatorFactory;
+import io.opentelemetry.sdk.metrics.internal.aggregator.AggregatorHandle;
+import io.opentelemetry.sdk.metrics.internal.descriptor.Advice;
+import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
+import io.opentelemetry.sdk.metrics.internal.exemplar.ExemplarFilter;
 import org.junit.jupiter.api.Test;
 
 class Base2ExponentialHistogramAggregationTest {
@@ -22,12 +34,38 @@ void goodConfig() {
   void invalidConfig_Throws() {
     assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(0, 20))
         .isInstanceOf(IllegalArgumentException.class)
-        .hasMessage("maxBuckets must be > 0");
-    assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(1, 21))
+        .hasMessage("maxBuckets must be >= 2");
+    assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(2, 21))
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessage("maxScale must be -10 <= x <= 20");
-    assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(1, -11))
+    assertThatThrownBy(() -> Base2ExponentialHistogramAggregation.create(2, -11))
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessage("maxScale must be -10 <= x <= 20");
   }
+
+  @Test
+  void minimumBucketsCanAccommodateMaxRange() {
+    Aggregation aggregation = Base2ExponentialHistogramAggregation.create(2, 20);
+    Aggregator<ExponentialHistogramPointData, ExemplarData> aggregator =
+        ((AggregatorFactory) aggregation)
+            .createAggregator(
+                InstrumentDescriptor.create(
+                    "foo",
+                    "description",
+                    "unit",
+                    InstrumentType.HISTOGRAM,
+                    InstrumentValueType.DOUBLE,
+                    Advice.empty()),
+                ExemplarFilter.alwaysOff());
+    AggregatorHandle<ExponentialHistogramPointData, ExemplarData> handle =
+        aggregator.createHandle();
+    // Record max range
+    handle.recordDouble(Double.MIN_VALUE);
+    handle.recordDouble(Double.MAX_VALUE);
+
+    ExponentialHistogramPointData pointData =
+        handle.aggregateThenMaybeReset(0, 1, Attributes.empty(), /* reset= */ true);
+    assertThat(pointData.getCount()).isEqualTo(2);
+    assertThat(pointData.getScale()).isEqualTo(-11);
+  }
 }