fix(sdk-metrics): allow single bucket histograms

open-telemetry · Feb 1, 2024 · 8825cbb · 8825cbb
1 parent 4655895
commit 8825cbb
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 2 deletions.
diff --git a/packages/sdk-metrics/src/view/Aggregation.ts b/packages/sdk-metrics/src/view/Aggregation.ts
@@ -130,8 +130,10 @@ export class ExplicitBucketHistogramAggregation extends Aggregation {
     private readonly _recordMinMax = true
   ) {
     super();
-    if (boundaries === undefined || boundaries.length === 0) {
-      throw new Error('HistogramAggregator should be created with boundaries.');
+    if (boundaries === undefined) {
+      throw new Error(
+        'ExplicitBucketHistogramAggregation should be created with explicit boundaries, if a single bucket histogram is required, please pass an empty array'
+      );
     }
     // Copy the boundaries array for modification.
     boundaries = boundaries.concat();

diff --git a/packages/sdk-metrics/test/Instruments.test.ts b/packages/sdk-metrics/test/Instruments.test.ts
@@ -406,6 +406,20 @@ describe('Instruments', () => {
       });
     });
 
+    it('should allow metric advice with empty explicit boundaries', function () {
+      const meter = new MeterProvider({
+        readers: [new TestMetricReader()],
+      }).getMeter('meter');
+
+      assert.doesNotThrow(() => {
+        meter.createHistogram('histogram', {
+          advice: {
+            explicitBucketBoundaries: [],
+          },
+        });
+      });
+    });
+
     it('should collect min and max', async () => {
       const { meter, deltaReader, cumulativeReader } = setup();
       const histogram = meter.createHistogram('test', {

diff --git a/packages/sdk-metrics/test/aggregator/Histogram.test.ts b/packages/sdk-metrics/test/aggregator/Histogram.test.ts
@@ -81,6 +81,27 @@ describe('HistogramAggregator', () => {
         sum: -65,
       });
     });
+
+    it('with single bucket', function () {
+      const aggregator = new HistogramAggregator([0], true);
+      const prev = aggregator.createAccumulation([0, 0]);
+      prev.record(0);
+      prev.record(1);
+
+      const delta = aggregator.createAccumulation([1, 1]);
+      delta.record(2);
+      delta.record(11);
+
+      const expected = aggregator.createAccumulation([0, 0]);
+      // replay actions on prev
+      expected.record(0);
+      expected.record(1);
+      // replay actions on delta
+      expected.record(2);
+      expected.record(11);
+
+      assert.deepStrictEqual(aggregator.merge(prev, delta), expected);
+    });
   });
 
   describe('diff', () => {
@@ -112,6 +133,35 @@ describe('HistogramAggregator', () => {
 
       assert.deepStrictEqual(aggregator.diff(prev, curr), expected);
     });
+
+    it('with single bucket', function () {
+      const aggregator = new HistogramAggregator([], true);
+      const prev = aggregator.createAccumulation([0, 0]);
+      prev.record(0);
+      prev.record(1);
+
+      const curr = aggregator.createAccumulation([1, 1]);
+      // replay actions on prev
+      curr.record(0);
+      curr.record(1);
+      // perform new actions
+      curr.record(2);
+      curr.record(11);
+
+      const expected = new HistogramAccumulation([1, 1], [], true, {
+        buckets: {
+          boundaries: [],
+          counts: [2],
+        },
+        count: 2,
+        sum: 13,
+        hasMinMax: false,
+        min: Infinity,
+        max: -Infinity,
+      });
+
+      assert.deepStrictEqual(aggregator.diff(prev, curr), expected);
+    });
   });
 
   describe('toMetricData', () => {
@@ -199,6 +249,48 @@ describe('HistogramAggregator', () => {
       );
     });
 
+    it('should transform to expected data with empty boundaries', () => {
+      const aggregator = new HistogramAggregator([], false);
+
+      const startTime: HrTime = [0, 0];
+      const endTime: HrTime = [1, 1];
+      const accumulation = aggregator.createAccumulation(startTime);
+      accumulation.record(0);
+      accumulation.record(1);
+
+      const expected: MetricData = {
+        descriptor: defaultInstrumentDescriptor,
+        aggregationTemporality: AggregationTemporality.CUMULATIVE,
+        dataPointType: DataPointType.HISTOGRAM,
+        dataPoints: [
+          {
+            attributes: {},
+            startTime,
+            endTime,
+            value: {
+              buckets: {
+                boundaries: [],
+                counts: [2],
+              },
+              count: 2,
+              sum: 1,
+              min: undefined,
+              max: undefined,
+            },
+          },
+        ],
+      };
+      assert.deepStrictEqual(
+        aggregator.toMetricData(
+          defaultInstrumentDescriptor,
+          AggregationTemporality.CUMULATIVE,
+          [[{}, accumulation]],
+          endTime
+        ),
+        expected
+      );
+    });
+
     function testSum(instrumentType: InstrumentType, expectSum: boolean) {
       const aggregator = new HistogramAggregator([1, 10, 100], true);
 

diff --git a/packages/sdk-metrics/test/view/Aggregation.test.ts b/packages/sdk-metrics/test/view/Aggregation.test.ts
@@ -158,6 +158,13 @@ describe('ExplicitBucketHistogramAggregation', () => {
     }
   });
 
+  it('construct with empty boundaries', function () {
+    const boundaries: number[] = [];
+    const aggregation = new ExplicitBucketHistogramAggregation(boundaries);
+    assert.ok(aggregation instanceof ExplicitBucketHistogramAggregation);
+    assert.deepStrictEqual(aggregation['_boundaries'], []);
+  });
+
   it('constructor should not modify inputs', () => {
     const boundaries = [100, 10, 1];
     const aggregation = new ExplicitBucketHistogramAggregation(boundaries);