diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index 4e57a6edbb..7607beab93 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -26,7 +26,7 @@ import { MetricRecord, MetricKind, SumAggregator, - LastValueAggregator, + MinMaxLastSumCountAggregator, } from '@opentelemetry/metrics'; if (typeof Buffer === 'undefined') { @@ -85,7 +85,7 @@ export const mockObserver: MetricRecord = { valueType: ValueType.DOUBLE, }, labels: {}, - aggregator: new LastValueAggregator(), + aggregator: new MinMaxLastSumCountAggregator(), resource: new Resource({ service: 'ui', version: 1, diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 2951ddeeee..af0c551dba 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -161,10 +161,10 @@ export class PrometheusExporter implements MetricExporter { (point.value as Histogram).sum, hrTimeToMilliseconds(point.timestamp) ); - } else if (typeof (point.value as Distribution).max === 'number') { + } else if (typeof (point.value as Distribution).last === 'number') { metric.set( labels, - (point.value as Distribution).sum, + (point.value as Distribution).last, hrTimeToMilliseconds(point.timestamp) ); } diff --git a/packages/opentelemetry-metrics/src/export/Batcher.ts b/packages/opentelemetry-metrics/src/export/Batcher.ts index 9bcf574d02..18e871aa63 100644 --- a/packages/opentelemetry-metrics/src/export/Batcher.ts +++ b/packages/opentelemetry-metrics/src/export/Batcher.ts @@ -57,9 +57,9 @@ export class UngroupedBatcher extends Batcher { return new aggregators.SumAggregator(); case MetricKind.VALUE_RECORDER: case MetricKind.VALUE_OBSERVER: - return new aggregators.LastValueAggregator(); + return new aggregators.MinMaxLastSumCountAggregator(); default: - return new aggregators.MinMaxSumCountAggregator(); + return new aggregators.MinMaxLastSumCountAggregator(); } } diff --git a/packages/opentelemetry-metrics/src/export/aggregators/LastValue.ts b/packages/opentelemetry-metrics/src/export/aggregators/LastValue.ts deleted file mode 100644 index 20c049c842..0000000000 --- a/packages/opentelemetry-metrics/src/export/aggregators/LastValue.ts +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { Aggregator, Point } from '../types'; -import { HrTime } from '@opentelemetry/api'; -import { hrTime } from '@opentelemetry/core'; - -/** Basic aggregator for LastValue which keeps the last recorded value. */ -export class LastValueAggregator implements Aggregator { - private _current: number = 0; - private _lastUpdateTime: HrTime = [0, 0]; - - update(value: number): void { - this._current = value; - this._lastUpdateTime = hrTime(); - } - - toPoint(): Point { - return { - value: this._current, - timestamp: this._lastUpdateTime, - }; - } -} diff --git a/packages/opentelemetry-metrics/src/export/aggregators/MinMaxSumCount.ts b/packages/opentelemetry-metrics/src/export/aggregators/MinMaxLastSumCount.ts similarity index 87% rename from packages/opentelemetry-metrics/src/export/aggregators/MinMaxSumCount.ts rename to packages/opentelemetry-metrics/src/export/aggregators/MinMaxLastSumCount.ts index 9be0fe8350..7f5a9df431 100644 --- a/packages/opentelemetry-metrics/src/export/aggregators/MinMaxSumCount.ts +++ b/packages/opentelemetry-metrics/src/export/aggregators/MinMaxLastSumCount.ts @@ -19,8 +19,10 @@ import { HrTime } from '@opentelemetry/api'; import { hrTime } from '@opentelemetry/core'; import { Distribution } from '../types'; -/** Basic aggregator keeping all raw values (events, sum, max and min). */ -export class MinMaxSumCountAggregator implements Aggregator { +/** + * Basic aggregator keeping all raw values (events, sum, max, last and min). + */ +export class MinMaxLastSumCountAggregator implements Aggregator { private _distribution: Distribution; private _lastUpdateTime: HrTime = [0, 0]; @@ -28,6 +30,7 @@ export class MinMaxSumCountAggregator implements Aggregator { this._distribution = { min: Infinity, max: -Infinity, + last: 0, sum: 0, count: 0, }; @@ -36,6 +39,7 @@ export class MinMaxSumCountAggregator implements Aggregator { update(value: number): void { this._distribution.count++; this._distribution.sum += value; + this._distribution.last = value; this._distribution.min = Math.min(this._distribution.min, value); this._distribution.max = Math.max(this._distribution.max, value); this._lastUpdateTime = hrTime(); diff --git a/packages/opentelemetry-metrics/src/export/aggregators/index.ts b/packages/opentelemetry-metrics/src/export/aggregators/index.ts index 0d3f8155b7..93001f73d7 100644 --- a/packages/opentelemetry-metrics/src/export/aggregators/index.ts +++ b/packages/opentelemetry-metrics/src/export/aggregators/index.ts @@ -15,6 +15,5 @@ */ export * from './histogram'; -export * from './MinMaxSumCount'; -export * from './LastValue'; +export * from './MinMaxLastSumCount'; export * from './Sum'; diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts index 192cabd414..66c1262e37 100644 --- a/packages/opentelemetry-metrics/src/export/types.ts +++ b/packages/opentelemetry-metrics/src/export/types.ts @@ -37,6 +37,7 @@ export type LastValue = number; export interface Distribution { min: number; max: number; + last: number; count: number; sum: number; } diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 2cd8c42937..95f985a903 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -27,8 +27,9 @@ import { MetricRecord, Aggregator, MetricDescriptor, - LastValueAggregator, UpDownCounterMetric, + Distribution, + MinMaxLastSumCountAggregator, } from '../src'; import * as api from '@opentelemetry/api'; import { NoopLogger, hrTime, hrTimeToNanoseconds } from '@opentelemetry/core'; @@ -562,7 +563,16 @@ describe('Meter', () => { await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual(record1.aggregator.toPoint().value as number, 0); + assert.deepStrictEqual( + record1.aggregator.toPoint().value as Distribution, + { + count: 0, + last: 0, + max: -Infinity, + min: Infinity, + sum: 0, + } + ); }); it('should not set the instrument data when disabled', async () => { @@ -574,7 +584,16 @@ describe('Meter', () => { await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual(record1.aggregator.toPoint().value as number, 0); + assert.deepStrictEqual( + record1.aggregator.toPoint().value as Distribution, + { + count: 0, + last: 0, + max: -Infinity, + min: Infinity, + sum: 0, + } + ); }); it( @@ -591,8 +610,14 @@ describe('Meter', () => { await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); assert.deepStrictEqual( - record1.aggregator.toPoint().value as number, - 50 + record1.aggregator.toPoint().value as Distribution, + { + count: 2, + last: 50, + max: 50, + min: -10, + sum: 40, + } ); assert.ok( hrTimeToNanoseconds(record1.aggregator.toPoint().timestamp) > @@ -612,8 +637,14 @@ describe('Meter', () => { await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); assert.deepStrictEqual( - record1.aggregator.toPoint().value as number, - 100 + record1.aggregator.toPoint().value as Distribution, + { + count: 2, + last: 100, + max: 100, + min: 10, + sum: 110, + } ); assert.strictEqual(boundValueRecorder1, boundValueRecorder2); }); @@ -809,10 +840,34 @@ describe('Meter', () => { assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1'); assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2'); - ensureMetric(metric1, 'cpu_temp_per_app', 67); - ensureMetric(metric2, 'cpu_temp_per_app', 69); - ensureMetric(metric3, 'cpu_temp_per_app', 67); - ensureMetric(metric4, 'cpu_temp_per_app', 69); + ensureMetric(metric1, 'cpu_temp_per_app', { + count: 1, + last: 67, + max: 67, + min: 67, + sum: 67, + }); + ensureMetric(metric2, 'cpu_temp_per_app', { + count: 1, + last: 69, + max: 69, + min: 69, + sum: 69, + }); + ensureMetric(metric3, 'cpu_temp_per_app', { + count: 1, + last: 67, + max: 67, + min: 67, + sum: 67, + }); + ensureMetric(metric4, 'cpu_temp_per_app', { + count: 1, + last: 69, + max: 69, + min: 69, + sum: 69, + }); const metric5 = cpuUsageMetricRecords[0]; const metric6 = cpuUsageMetricRecords[1]; @@ -823,10 +878,34 @@ describe('Meter', () => { assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1'); assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2'); - ensureMetric(metric5, 'cpu_usage_per_app', 2.1); - ensureMetric(metric6, 'cpu_usage_per_app', 3.1); - ensureMetric(metric7, 'cpu_usage_per_app', 1.2); - ensureMetric(metric8, 'cpu_usage_per_app', 4.5); + ensureMetric(metric5, 'cpu_usage_per_app', { + count: 1, + last: 2.1, + max: 2.1, + min: 2.1, + sum: 2.1, + }); + ensureMetric(metric6, 'cpu_usage_per_app', { + count: 1, + last: 3.1, + max: 3.1, + min: 3.1, + sum: 3.1, + }); + ensureMetric(metric7, 'cpu_usage_per_app', { + count: 1, + last: 1.2, + max: 1.2, + min: 1.2, + sum: 1.2, + }); + ensureMetric(metric8, 'cpu_usage_per_app', { + count: 1, + last: 4.5, + max: 4.5, + min: 4.5, + sum: 4.5, + }); }); it('should not observe values when timeout', done => { @@ -856,9 +935,15 @@ describe('Meter', () => { const value = cpuUsageMetric .bind({ foo: 'bar' }) .getAggregator() - .toPoint().value as number; - - assert.strictEqual(value, 0); + .toPoint().value as Distribution; + + assert.deepStrictEqual(value, { + count: 0, + last: 0, + max: -Infinity, + min: Infinity, + sum: 0, + }); assert.strictEqual(cpuUsageMetricRecords.length, 0); done(); }); @@ -961,13 +1046,17 @@ class CustomBatcher extends Batcher { } } -function ensureMetric(metric: MetricRecord, name?: string, value?: number) { - assert.ok(metric.aggregator instanceof LastValueAggregator); - const lastValue = metric.aggregator.toPoint().value; - if (typeof value === 'number') { - assert.strictEqual(lastValue, value); +function ensureMetric( + metric: MetricRecord, + name?: string, + value?: Distribution +) { + assert.ok(metric.aggregator instanceof MinMaxLastSumCountAggregator); + const distribution = metric.aggregator.toPoint().value as Distribution; + if (value) { + assert.deepStrictEqual(distribution, value); } else { - assert.ok(lastValue >= 0 && lastValue <= 1); + assert.ok(distribution.last >= 0 && distribution.last <= 1); } const descriptor = metric.descriptor; assert.strictEqual(descriptor.name, name || 'name');