diff --git a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts index bb5268cf14..8cae6cc2cc 100644 --- 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', () => { @@ -43,7 +42,6 @@ describe('transformMetrics', () => { // Histogram mockHistogram.aggregator.update(7); mockHistogram.aggregator.update(14); - (mockHistogram.aggregator as HistogramAggregator).reset(); // ValueRecorder mockValueRecorder.aggregator.update(5); @@ -51,7 +49,6 @@ describe('transformMetrics', () => { afterEach(() => { mockCounter.aggregator.update(-1); // Reset counter - (mockHistogram.aggregator as HistogramAggregator).reset(); }); it('should convert metric', () => { ensureCounterIsCorrect( diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index d7733c2c60..7437460b42 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -38,6 +38,10 @@ if (typeof Buffer === 'undefined') { }; } +type Mutable = { + -readonly [P in keyof T]: T[P]; +}; + const traceIdArr = [ 31, 16, @@ -113,7 +117,7 @@ export const mockValueRecorder: MetricRecord = { instrumentationLibrary: { name: 'default', version: '0.0.1' }, }; -export const mockHistogram: MetricRecord = { +export const mockHistogram: Mutable = { descriptor: { name: 'test-hist', description: 'sample observer description', diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts index 7f75512654..abdd1d774c 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts @@ -146,14 +146,14 @@ const testCollectorMetricExporter = (params: TestParams) => metrics[1].aggregator.update(10); metrics[2].aggregator.update(7); metrics[2].aggregator.update(14); - (metrics[2].aggregator as HistogramAggregator).reset(); metrics[3].aggregator.update(5); done(); }); afterEach(() => { - metrics[0].aggregator.update(-1); // Aggregator is not deep-copied - (metrics[2].aggregator as HistogramAggregator).reset(); + // Aggregator is not deep-copied + metrics[0].aggregator.update(-1); + mockHistogram.aggregator = new HistogramAggregator([10, 20]); exportedData = undefined; reqMetadata = undefined; }); diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporterWithJson.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporterWithJson.test.ts index 9e5155d2cd..a3e61d42f8 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporterWithJson.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporterWithJson.test.ts @@ -85,12 +85,12 @@ describe('CollectorMetricExporter - node with json over http', () => { metrics[1].aggregator.update(10); metrics[2].aggregator.update(7); metrics[2].aggregator.update(14); - (metrics[2].aggregator as HistogramAggregator).reset(); metrics[3].aggregator.update(5); }); afterEach(() => { - metrics[0].aggregator.update(-1); // Aggregator is not deep-copied - (metrics[2].aggregator as HistogramAggregator).reset(); + // Aggregator is not deep-copied + metrics[0].aggregator.update(-1); + mockHistogram.aggregator = new HistogramAggregator([10, 20]); spyRequest.restore(); spyWrite.restore(); }); diff --git a/packages/opentelemetry-metrics/src/export/aggregators/histogram.ts b/packages/opentelemetry-metrics/src/export/aggregators/Histogram.ts similarity index 71% rename from packages/opentelemetry-metrics/src/export/aggregators/histogram.ts rename to packages/opentelemetry-metrics/src/export/aggregators/Histogram.ts index 655afd8ff7..11cd33ee8f 100644 --- a/packages/opentelemetry-metrics/src/export/aggregators/histogram.ts +++ b/packages/opentelemetry-metrics/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 index 93001f73d7..9ff9f904ea 100644 --- 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/packages/opentelemetry-metrics/test/export/aggregators/histogram.test.ts b/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts similarity index 91% rename from packages/opentelemetry-metrics/test/export/aggregators/histogram.test.ts rename to packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts index 45f6fe80f6..9a2b43938c 100644 --- a/packages/opentelemetry-metrics/test/export/aggregators/histogram.test.ts +++ b/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts @@ -40,18 +40,9 @@ describe('HistogramAggregator', () => { }); describe('.update()', () => { - it('should not update checkpoint', () => { - const aggregator = new HistogramAggregator([100, 200]); - aggregator.update(150); - const point = aggregator.toPoint().value as Histogram; - assert.equal(point.count, 0); - assert.equal(point.sum, 0); - }); - 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 +54,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 +65,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 +80,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 +92,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 +113,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 +133,6 @@ describe('HistogramAggregator', () => { it('should return last checkpoint if updated', () => { const aggregator = new HistogramAggregator([100]); aggregator.update(100); - aggregator.reset(); assert( aggregator .toPoint()