From 87236cc519c85b0bb760832d70fee3c03817fc0e Mon Sep 17 00:00:00 2001 From: Mayur Kale Date: Fri, 27 Mar 2020 10:00:38 -0700 Subject: [PATCH] Metrics: Add lastUpdateTimestamp associated with point (#893) * Metrics: Add lastUpdateTimestamp associated with point * update lastUpdateTime in MeasureExactAggregator * add tests Co-authored-by: Daniel Dyla --- .../src/prometheus.ts | 15 ++- .../src/export/Aggregator.ts | 31 +++-- .../src/export/ConsoleMetricExporter.ts | 5 +- .../opentelemetry-metrics/src/export/types.ts | 11 +- .../opentelemetry-metrics/test/Meter.test.ts | 114 ++++++++++++------ 5 files changed, 119 insertions(+), 57 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 27c4bcf7a1..b3a8aac621 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -15,7 +15,7 @@ */ import { ExportResult } from '@opentelemetry/base'; -import { NoopLogger } from '@opentelemetry/core'; +import { NoopLogger, hrTimeToMilliseconds } from '@opentelemetry/core'; import { CounterSumAggregator, LastValue, @@ -124,27 +124,30 @@ export class PrometheusExporter implements MetricExporter { if (!metric) return; const labelKeys = record.descriptor.labelKeys; - const value = record.aggregator.value(); + const point = record.aggregator.toPoint(); if (metric instanceof Counter) { // Prometheus counter saves internal state and increments by given value. // MetricRecord value is the current state, not the delta to be incremented by. // Currently, _registerMetric creates a new counter every time the value changes, // so the increment here behaves as a set value (increment from 0) - metric.inc(this._getLabelValues(labelKeys, record.labels), value as Sum); + metric.inc( + this._getLabelValues(labelKeys, record.labels), + point.value as Sum + ); } if (metric instanceof Gauge) { if (record.aggregator instanceof CounterSumAggregator) { metric.set( this._getLabelValues(labelKeys, record.labels), - value as Sum + point.value as Sum ); } else if (record.aggregator instanceof ObserverAggregator) { metric.set( this._getLabelValues(labelKeys, record.labels), - value as LastValue, - new Date() + point.value as LastValue, + hrTimeToMilliseconds(point.timestamp) ); } } diff --git a/packages/opentelemetry-metrics/src/export/Aggregator.ts b/packages/opentelemetry-metrics/src/export/Aggregator.ts index f9edf8ef93..d8bd4373b9 100644 --- a/packages/opentelemetry-metrics/src/export/Aggregator.ts +++ b/packages/opentelemetry-metrics/src/export/Aggregator.ts @@ -14,37 +14,50 @@ * limitations under the License. */ -import { Aggregator, Distribution, LastValue, Sum } from './types'; +import { Aggregator, Distribution, Point } from './types'; +import { HrTime } from '@opentelemetry/api'; +import { hrTime } from '@opentelemetry/core'; /** Basic aggregator which calculates a Sum from individual measurements. */ export class CounterSumAggregator implements Aggregator { private _current: number = 0; + private _lastUpdateTime: HrTime = [0, 0]; update(value: number): void { this._current += value; + this._lastUpdateTime = hrTime(); } - value(): Sum { - return this._current; + toPoint(): Point { + return { + value: this._current, + timestamp: this._lastUpdateTime, + }; } } /** Basic aggregator for Observer which keeps the last recorded value. */ export class ObserverAggregator implements Aggregator { private _current: number = 0; + private _lastUpdateTime: HrTime = [0, 0]; update(value: number): void { this._current = value; + this._lastUpdateTime = hrTime(); } - value(): LastValue { - return this._current; + toPoint(): Point { + return { + value: this._current, + timestamp: this._lastUpdateTime, + }; } } /** Basic aggregator keeping all raw values (events, sum, max and min). */ export class MeasureExactAggregator implements Aggregator { private _distribution: Distribution; + private _lastUpdateTime: HrTime = [0, 0]; constructor() { this._distribution = { @@ -60,9 +73,13 @@ export class MeasureExactAggregator implements Aggregator { this._distribution.sum += value; this._distribution.min = Math.min(this._distribution.min, value); this._distribution.max = Math.max(this._distribution.max, value); + this._lastUpdateTime = hrTime(); } - value(): Distribution { - return this._distribution; + toPoint(): Point { + return { + value: this._distribution, + timestamp: this._lastUpdateTime, + }; } } diff --git a/packages/opentelemetry-metrics/src/export/ConsoleMetricExporter.ts b/packages/opentelemetry-metrics/src/export/ConsoleMetricExporter.ts index 1d848dd968..ba0858be1c 100644 --- a/packages/opentelemetry-metrics/src/export/ConsoleMetricExporter.ts +++ b/packages/opentelemetry-metrics/src/export/ConsoleMetricExporter.ts @@ -37,11 +37,12 @@ export class ConsoleMetricExporter implements MetricExporter { console.log(metric.labels.labels); switch (metric.descriptor.metricKind) { case MetricKind.COUNTER: - const sum = metric.aggregator.value() as Sum; + const sum = metric.aggregator.toPoint().value as Sum; console.log('value: ' + sum); break; default: - const distribution = metric.aggregator.value() as Distribution; + const distribution = metric.aggregator.toPoint() + .value as Distribution; console.log( 'min: ' + distribution.min + diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts index b57ab562b7..32e0c077a5 100644 --- a/packages/opentelemetry-metrics/src/export/types.ts +++ b/packages/opentelemetry-metrics/src/export/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ValueType } from '@opentelemetry/api'; +import { ValueType, HrTime } from '@opentelemetry/api'; import { ExportResult } from '@opentelemetry/base'; import { LabelSet } from '../LabelSet'; @@ -76,6 +76,11 @@ export interface Aggregator { /** Updates the current with the new value. */ update(value: number): void; - /** Returns snapshot of the current value. */ - value(): Sum | Distribution; + /** Returns snapshot of the current point (value with timestamp). */ + toPoint(): Point; +} + +export interface Point { + value: Sum | LastValue | Distribution; + timestamp: HrTime; } diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index f0ed7b161d..2a91e2bd82 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -29,7 +29,7 @@ import { } from '../src'; import * as types from '@opentelemetry/api'; import { LabelSet } from '../src/LabelSet'; -import { NoopLogger } from '@opentelemetry/core'; +import { NoopLogger, hrTime, hrTimeToNanoseconds } from '@opentelemetry/core'; import { CounterSumAggregator, ObserverAggregator, @@ -63,6 +63,8 @@ describe('Meter', () => { }); describe('#counter', () => { + const performanceTimeOrigin = hrTime(); + it('should create a counter', () => { const counter = meter.createCounter('name'); assert.ok(counter instanceof Metric); @@ -84,9 +86,19 @@ describe('Meter', () => { meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.strictEqual(record1.aggregator.value(), 10); + assert.strictEqual(record1.aggregator.toPoint().value, 10); + const lastTimestamp = record1.aggregator.toPoint().timestamp; + assert.ok( + hrTimeToNanoseconds(lastTimestamp) > + hrTimeToNanoseconds(performanceTimeOrigin) + ); counter.add(10, labelSet); - assert.strictEqual(record1.aggregator.value(), 20); + assert.strictEqual(record1.aggregator.toPoint().value, 20); + + assert.ok( + hrTimeToNanoseconds(record1.aggregator.toPoint().timestamp) > + hrTimeToNanoseconds(lastTimestamp) + ); }); it('should return counter with resource', () => { @@ -102,9 +114,9 @@ describe('Meter', () => { meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.strictEqual(record1.aggregator.value(), 10); + assert.strictEqual(record1.aggregator.toPoint().value, 10); boundCounter.add(10); - assert.strictEqual(record1.aggregator.value(), 20); + assert.strictEqual(record1.aggregator.toPoint().value, 20); }); it('should return the aggregator', () => { @@ -123,9 +135,9 @@ describe('Meter', () => { meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.strictEqual(record1.aggregator.value(), 10); + assert.strictEqual(record1.aggregator.toPoint().value, 10); boundCounter.add(-100); - assert.strictEqual(record1.aggregator.value(), 10); + assert.strictEqual(record1.aggregator.toPoint().value, 10); }); it('should not add the instrument data when disabled', () => { @@ -136,7 +148,7 @@ describe('Meter', () => { boundCounter.add(10); meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.strictEqual(record1.aggregator.value(), 0); + assert.strictEqual(record1.aggregator.toPoint().value, 0); }); it('should add negative value when monotonic is set to false', () => { @@ -147,7 +159,7 @@ describe('Meter', () => { boundCounter.add(-10); meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.strictEqual(record1.aggregator.value(), -10); + assert.strictEqual(record1.aggregator.toPoint().value, -10); }); it('should return same instrument on same label values', () => { @@ -159,7 +171,7 @@ describe('Meter', () => { meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.strictEqual(record1.aggregator.value(), 20); + assert.strictEqual(record1.aggregator.toPoint().value, 20); assert.strictEqual(boundCounter, boundCounter1); }); }); @@ -214,7 +226,7 @@ describe('Meter', () => { unit: '1', valueType: ValueType.DOUBLE, }); - assert.strictEqual(record[0].aggregator.value(), 10); + assert.strictEqual(record[0].aggregator.toPoint().value, 10); }); }); @@ -306,6 +318,8 @@ describe('Meter', () => { }); describe('.bind()', () => { + const performanceTimeOrigin = hrTime(); + it('should create a measure instrument', () => { const measure = meter.createMeasure('name') as MeasureMetric; const boundMeasure = measure.bind(labelSet); @@ -319,12 +333,15 @@ describe('Meter', () => { meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual(record1.aggregator.value() as Distribution, { - count: 0, - max: -Infinity, - min: Infinity, - sum: 0, - }); + assert.deepStrictEqual( + record1.aggregator.toPoint().value as Distribution, + { + count: 0, + max: -Infinity, + min: Infinity, + sum: 0, + } + ); }); it('should not set the instrument data when disabled', () => { @@ -336,12 +353,15 @@ describe('Meter', () => { meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual(record1.aggregator.value() as Distribution, { - count: 0, - max: -Infinity, - min: Infinity, - sum: 0, - }); + assert.deepStrictEqual( + record1.aggregator.toPoint().value as Distribution, + { + count: 0, + max: -Infinity, + min: Infinity, + sum: 0, + } + ); }); it('should accept negative (and positive) values when absolute is set to false', () => { @@ -354,12 +374,19 @@ describe('Meter', () => { meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual(record1.aggregator.value() as Distribution, { - count: 2, - max: 50, - min: -10, - sum: 40, - }); + assert.deepStrictEqual( + record1.aggregator.toPoint().value as Distribution, + { + count: 2, + max: 50, + min: -10, + sum: 40, + } + ); + assert.ok( + hrTimeToNanoseconds(record1.aggregator.toPoint().timestamp) > + hrTimeToNanoseconds(performanceTimeOrigin) + ); }); it('should return same instrument on same label values', () => { @@ -370,12 +397,15 @@ describe('Meter', () => { boundMeasure2.record(100); meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual(record1.aggregator.value() as Distribution, { - count: 2, - max: 100, - min: 10, - sum: 110, - }); + assert.deepStrictEqual( + record1.aggregator.toPoint().value as Distribution, + { + count: 2, + max: 100, + min: 10, + sum: 110, + } + ); assert.strictEqual(boundMeasure1, boundMeasure2); }); }); @@ -500,7 +530,7 @@ describe('Meter', () => { labelKeys: ['key'], }); assert.strictEqual(record[0].labels, labelSet); - const value = record[0].aggregator.value() as Sum; + const value = record[0].aggregator.toPoint().value as Sum; assert.strictEqual(value, 10.45); }); @@ -529,7 +559,7 @@ describe('Meter', () => { labelKeys: ['key'], }); assert.strictEqual(record[0].labels, labelSet); - const value = record[0].aggregator.value() as Sum; + const value = record[0].aggregator.toPoint().value as Sum; assert.strictEqual(value, 10); }); }); @@ -537,8 +567,14 @@ describe('Meter', () => { function ensureMetric(metric: MetricRecord) { assert.ok(metric.aggregator instanceof ObserverAggregator); - assert.ok(metric.aggregator.value() >= 0 && metric.aggregator.value() <= 1); - assert.ok(metric.aggregator.value() >= 0 && metric.aggregator.value() <= 1); + assert.ok( + metric.aggregator.toPoint().value >= 0 && + metric.aggregator.toPoint().value <= 1 + ); + assert.ok( + metric.aggregator.toPoint().value >= 0 && + metric.aggregator.toPoint().value <= 1 + ); const descriptor = metric.descriptor; assert.strictEqual(descriptor.name, 'name'); assert.strictEqual(descriptor.description, 'desc');