feat: remove HistogramAggregator.reset

open-telemetry · Jul 13, 2020 · 901c965 · 901c965
1 parent 965e69f
commit 901c965
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 28 deletions.
diff --git a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts
@@ -29,7 +29,6 @@ import {
   ensureValueRecorderIsCorrect,
   mockValueRecorder,
 } from '../helper';
-import { HistogramAggregator } from '@opentelemetry/metrics';
 import { hrTimeToNanoseconds } from '@opentelemetry/core';
 describe('transformMetrics', () => {
   describe('toCollectorMetric', () => {
@@ -46,7 +45,6 @@ describe('transformMetrics', () => {
       );
       mockHistogram.aggregator.update(7);
       mockHistogram.aggregator.update(14);
-      (mockHistogram.aggregator as HistogramAggregator).reset();
       ensureHistogramIsCorrect(
         transform.toCollectorMetric(mockHistogram, 1592602232694000000),
         hrTimeToNanoseconds(mockHistogram.aggregator.toPoint().timestamp)

diff --git a/...trics/src/export/aggregators/histogram.ts → ...trics/src/export/aggregators/Histogram.ts b/...trics/src/export/aggregators/histogram.ts → ...trics/src/export/aggregators/Histogram.ts
@@ -23,9 +23,8 @@ import { hrTime } from '@opentelemetry/core';
  * and provides the total sum and count of all observations.
  */
 export class HistogramAggregator implements Aggregator {
-  private _lastCheckpoint: Histogram;
-  private _currentCheckpoint: Histogram;
-  private _lastCheckpointTime: HrTime;
+  private _current: Histogram;
+  private _lastUpdateTime: HrTime;
   private readonly _boundaries: number[];
 
   constructor(boundaries: number[]) {
@@ -35,36 +34,29 @@ export class HistogramAggregator implements Aggregator {
     // we need to an ordered set to be able to correctly compute count for each
     // boundary since we'll iterate on each in order.
     this._boundaries = boundaries.sort();
-    this._lastCheckpoint = this._newEmptyCheckpoint();
-    this._lastCheckpointTime = hrTime();
-    this._currentCheckpoint = this._newEmptyCheckpoint();
+    this._current = this._newEmptyCheckpoint();
+    this._lastUpdateTime = hrTime();
   }
 
   update(value: number): void {
-    this._currentCheckpoint.count += 1;
-    this._currentCheckpoint.sum += value;
+    this._current.count += 1;
+    this._current.sum += value;
 
     for (let i = 0; i < this._boundaries.length; i++) {
       if (value < this._boundaries[i]) {
-        this._currentCheckpoint.buckets.counts[i] += 1;
+        this._current.buckets.counts[i] += 1;
         return;
       }
     }
 
     // value is above all observed boundaries
-    this._currentCheckpoint.buckets.counts[this._boundaries.length] += 1;
-  }
-
-  reset(): void {
-    this._lastCheckpointTime = hrTime();
-    this._lastCheckpoint = this._currentCheckpoint;
-    this._currentCheckpoint = this._newEmptyCheckpoint();
+    this._current.buckets.counts[this._boundaries.length] += 1;
   }
 
   toPoint(): Point {
     return {
-      value: this._lastCheckpoint,
-      timestamp: this._lastCheckpointTime,
+      value: this._current,
+      timestamp: this._lastUpdateTime,
     };
   }
 

diff --git a/packages/opentelemetry-metrics/src/export/aggregators/index.ts b/packages/opentelemetry-metrics/src/export/aggregators/index.ts
@@ -14,6 +14,6 @@
  * limitations under the License.
  */
 
-export * from './histogram';
+export * from './Histogram';
 export * from './MinMaxLastSumCount';
 export * from './Sum';
diff --git a/...test/export/aggregators/histogram.test.ts → ...test/export/aggregators/Histogram.test.ts b/...test/export/aggregators/histogram.test.ts → ...test/export/aggregators/Histogram.test.ts
@@ -51,7 +51,6 @@ describe('HistogramAggregator', () => {
     it('should update the second bucket', () => {
       const aggregator = new HistogramAggregator([100, 200]);
       aggregator.update(150);
-      aggregator.reset();
       const point = aggregator.toPoint().value as Histogram;
       assert.equal(point.count, 1);
       assert.equal(point.sum, 150);
@@ -63,7 +62,6 @@ describe('HistogramAggregator', () => {
     it('should update the second bucket', () => {
       const aggregator = new HistogramAggregator([100, 200]);
       aggregator.update(50);
-      aggregator.reset();
       const point = aggregator.toPoint().value as Histogram;
       assert.equal(point.count, 1);
       assert.equal(point.sum, 50);
@@ -75,7 +73,6 @@ describe('HistogramAggregator', () => {
     it('should update the third bucket since value is above all boundaries', () => {
       const aggregator = new HistogramAggregator([100, 200]);
       aggregator.update(250);
-      aggregator.reset();
       const point = aggregator.toPoint().value as Histogram;
       assert.equal(point.count, 1);
       assert.equal(point.sum, 250);
@@ -91,7 +88,6 @@ describe('HistogramAggregator', () => {
       let point = aggregator.toPoint().value as Histogram;
       assert.equal(point.count, point.count);
       aggregator.update(10);
-      aggregator.reset();
       point = aggregator.toPoint().value as Histogram;
       assert.equal(point.count, 1);
       assert.equal(point.count, point.count);
@@ -104,7 +100,6 @@ describe('HistogramAggregator', () => {
       let point = aggregator.toPoint().value as Histogram;
       assert.equal(point.sum, point.sum);
       aggregator.update(10);
-      aggregator.reset();
       point = aggregator.toPoint().value as Histogram;
       assert.equal(point.sum, 10);
     });
@@ -126,7 +121,6 @@ describe('HistogramAggregator', () => {
     it('should update checkpoint', () => {
       const aggregator = new HistogramAggregator([100]);
       aggregator.update(10);
-      aggregator.reset();
       const point = aggregator.toPoint().value as Histogram;
       assert.equal(point.count, 1);
       assert.equal(point.sum, 10);
@@ -147,7 +141,6 @@ describe('HistogramAggregator', () => {
     it('should return last checkpoint if updated', () => {
       const aggregator = new HistogramAggregator([100]);
       aggregator.update(100);
-      aggregator.reset();
       assert(
         aggregator
           .toPoint()