From 154c76ba85eb7cbc8f4a2f8e3927cf7d76e82583 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Mon, 18 Apr 2022 13:33:22 +0300 Subject: [PATCH 1/7] feat: aggregation temporality controls --- .../src/OTLPMetricExporter.ts | 6 +---- .../test/OTLPMetricExporter.test.ts | 10 ++++---- .../test/metricsHelper.ts | 6 ++++- .../src/OTLPMetricExporterBase.ts | 18 ++++++++++---- .../src/OTLPMetricExporterOptions.ts | 4 ++-- .../platform/browser/OTLPMetricExporter.ts | 7 ++---- .../src/platform/node/OTLPMetricExporter.ts | 7 ++---- .../src/transformMetrics.ts | 15 ++++-------- .../browser/CollectorMetricExporter.test.ts | 10 ++++---- .../test/common/transformMetrics.test.ts | 14 +++++------ .../test/metricsHelper.ts | 12 +++++++++- .../test/node/CollectorMetricExporter.test.ts | 4 ++-- .../src/OTLPMetricExporter.ts | 6 +---- .../test/OTLPMetricExporter.test.ts | 2 +- .../test/metricsHelper.ts | 6 +++++ .../src/PrometheusExporter.ts | 2 +- .../test/PrometheusSerializer.test.ts | 2 +- .../src/aggregator/Drop.ts | 2 ++ .../src/aggregator/Histogram.ts | 3 +++ .../src/aggregator/LastValue.ts | 3 +++ .../src/aggregator/Sum.ts | 3 +++ .../src/aggregator/types.ts | 2 ++ .../src/export/AggregationTemporality.ts | 19 +++++++++++++++ .../src/export/MetricData.ts | 2 ++ .../src/export/MetricExporter.ts | 12 ++++++---- .../src/export/MetricReader.ts | 9 +++---- .../export/PeriodicExportingMetricReader.ts | 2 +- .../src/state/MetricCollector.ts | 11 +++++---- .../src/state/TemporalMetricProcessor.ts | 3 ++- .../test/Instruments.test.ts | 4 ++-- .../test/aggregator/Drop.test.ts | 2 ++ .../test/aggregator/Histogram.test.ts | 3 +++ .../test/aggregator/LastValue.test.ts | 3 +++ .../test/aggregator/Sum.test.ts | 3 +++ .../PeriodicExportingMetricReader.test.ts | 13 +++++----- .../test/export/TestMetricExporter.ts | 4 ++-- .../test/export/TestMetricReader.ts | 6 ++++- .../test/state/AsyncMetricStorage.test.ts | 4 ++-- .../test/state/MeterSharedState.test.ts | 7 +++--- .../test/state/MetricCollector.test.ts | 8 +++---- .../test/state/SyncMetricStorage.test.ts | 4 ++-- .../state/TemporalMetricProcessor.test.ts | 6 ++--- .../otlp-transformer/src/metrics/index.ts | 6 ++--- .../otlp-transformer/src/metrics/internal.ts | 4 ++-- .../otlp-transformer/test/metrics.test.ts | 24 +++++++++---------- 45 files changed, 180 insertions(+), 123 deletions(-) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts index 5ec71ef7ed..2e8917c0d6 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts @@ -16,12 +16,11 @@ import { otlpTypes } from '@opentelemetry/exporter-trace-otlp-http'; import { - defaultExporterTemporality, defaultOptions, OTLPMetricExporterBase, OTLPMetricExporterOptions, toOTLPExportMetricServiceRequest } from '@opentelemetry/exporter-metrics-otlp-http'; -import { AggregationTemporality, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; +import { ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { OTLPExporterConfigNode, OTLPExporterNodeBase, @@ -36,7 +35,6 @@ const DEFAULT_COLLECTOR_URL = 'localhost:4317'; class OTLPMetricExporterProxy extends OTLPExporterNodeBase { - protected readonly _aggregationTemporality: AggregationTemporality; constructor(config: OTLPExporterConfigNode & OTLPMetricExporterOptions= defaultOptions) { super(config); @@ -45,7 +43,6 @@ class OTLPMetricExporterProxy extends OTLPExporterNodeBase url: 'grpcs://' + address, credentials, metadata: params.metadata, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); setUp(); @@ -182,7 +182,7 @@ const testOTLPMetricExporter = (params: TestParams) => headers: { foo: 'bar', }, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); const args = warnStub.args[0]; assert.strictEqual(args[0], 'Headers cannot be set when using grpc'); @@ -190,7 +190,7 @@ const testOTLPMetricExporter = (params: TestParams) => it('should warn about path in url', () => { collectorExporter = new OTLPMetricExporter({ url: `http://${address}/v1/metrics`, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); const args = warnStub.args[0]; assert.strictEqual( @@ -264,7 +264,7 @@ describe('OTLPMetricExporter - node (getDefaultUrl)', () => { const url = 'http://foo.bar.com'; const collectorExporter = new OTLPMetricExporter({ url, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); setTimeout(() => { assert.strictEqual(collectorExporter._otlpExporter.url, 'foo.bar.com'); @@ -309,7 +309,7 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const collectorExporter = new OTLPMetricExporter({ metadata, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); assert.deepStrictEqual(collectorExporter._otlpExporter.metadata?.get('foo'), ['boo']); assert.deepStrictEqual(collectorExporter._otlpExporter.metadata?.get('bar'), ['foo']); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts index fcb0b4db50..071f67e51a 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts @@ -20,9 +20,13 @@ import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; import * as grpc from '@grpc/grpc-js'; import { VERSION } from '@opentelemetry/core'; -import { ExplicitBucketHistogramAggregation, MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics-base'; +import { AggregationTemporalitySelector, CumulativeTemporalitySelector, ExplicitBucketHistogramAggregation, MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics-base'; export class TestMetricReader extends MetricReader { + constructor(temporalitySelector?: AggregationTemporalitySelector) { + super(temporalitySelector ?? CumulativeTemporalitySelector); + } + protected onForceFlush(): Promise { return Promise.resolve(undefined); } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts index 39b238c39b..432442c243 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts @@ -15,21 +15,29 @@ */ import { ExportResult } from '@opentelemetry/core'; -import { AggregationTemporality, PushMetricExporter, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; +import { AggregationTemporality, AggregationTemporalitySelector, CumulativeTemporalitySelector, DeltaTemporalitySelector, InstrumentType, PushMetricExporter, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { OTLPExporterBase, otlpTypes } from '@opentelemetry/exporter-trace-otlp-http'; import { defaultOptions, OTLPMetricExporterOptions } from './OTLPMetricExporterOptions'; +function chooseTemporalitySelector(preferredAggregationTemporality?: AggregationTemporality): AggregationTemporalitySelector { + if (preferredAggregationTemporality === AggregationTemporality.DELTA) { + return DeltaTemporalitySelector; + } + + return CumulativeTemporalitySelector; +} + export class OTLPMetricExporterBase> implements PushMetricExporter { public _otlpExporter: T; - protected _preferredAggregationTemporality: AggregationTemporality; + protected _aggregationTemporalitySelector: AggregationTemporalitySelector; constructor(exporter: T, config: OTLPMetricExporterOptions = defaultOptions) { this._otlpExporter = exporter; - this._preferredAggregationTemporality = config.aggregationTemporality ?? AggregationTemporality.CUMULATIVE; + this._aggregationTemporalitySelector = chooseTemporalitySelector(config.preferredAggregationTemporality); } export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void): void { @@ -44,8 +52,8 @@ export class OTLPMetricExporterBase { - protected readonly _aggregationTemporality: AggregationTemporality; constructor(config: OTLPMetricExporterOptions & otlpTypes.OTLPExporterConfigBase = defaultOptions) { super(config); @@ -40,7 +39,6 @@ class OTLPExporterBrowserProxy extends OTLPExporterBrowserBase { - protected readonly _aggregationTemporality: AggregationTemporality; constructor(config: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions = defaultOptions) { super(config); @@ -41,7 +40,6 @@ class OTLPExporterNodeProxy extends OTLPExporterNodeBase( metrics: ResourceMetrics, - aggregationTemporality: AggregationTemporality, collectorExporterBase: OTLPExporterBase @@ -186,7 +183,6 @@ export function toOTLPExportMetricServiceRequest toCollectorMetric(metric, aggregationTemporality)), + metrics: metrics.map(metric => toCollectorMetric(metric)), instrumentationLibrary, }; } @@ -217,7 +212,6 @@ function toCollectorInstrumentationLibraryMetrics( function toCollectorResourceMetrics( resourceMetrics: ResourceMetrics, baseAttributes: SpanAttributes, - aggregationTemporality: AggregationTemporality ): otlpTypes.opentelemetryProto.metrics.v1.ResourceMetrics[] { return [{ resource: toCollectorResource(resourceMetrics.resource, baseAttributes), @@ -225,7 +219,6 @@ function toCollectorResourceMetrics( instrumentationLibraryMetrics => toCollectorInstrumentationLibraryMetrics( instrumentationLibraryMetrics.instrumentationLibrary, instrumentationLibraryMetrics.metrics, - aggregationTemporality ))) }]; } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts index f40b1cab84..35eafe5129 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts @@ -93,7 +93,7 @@ describe('OTLPMetricExporter - web', () => { beforeEach(() => { collectorExporter = new OTLPMetricExporter({ url: 'http://foo.bar.com', - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); }); it('should successfully send metrics using sendBeacon', done => { @@ -201,7 +201,7 @@ describe('OTLPMetricExporter - web', () => { (window.navigator as any).sendBeacon = false; collectorExporter = new OTLPMetricExporter({ url: 'http://foo.bar.com', - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); // Overwrites the start time to make tests consistent Object.defineProperty(collectorExporter, '_startTime', { @@ -337,7 +337,7 @@ describe('OTLPMetricExporter - web', () => { beforeEach(() => { collectorExporterConfig = { headers: customHeaders, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }; server = sinon.fakeServer.create(); }); @@ -429,7 +429,7 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const collectorExporter = new OTLPMetricExporter({ headers: {}, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].foo, 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; @@ -439,7 +439,7 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const collectorExporter = new OTLPMetricExporter({ headers: {}, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].foo, 'boo'); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].bar, 'foo'); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/transformMetrics.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/transformMetrics.test.ts index f5b401f304..c9250fb0ce 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/transformMetrics.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/transformMetrics.test.ts @@ -56,7 +56,7 @@ describe('transformMetrics', () => { const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; ensureCounterIsCorrect( - transform.toCollectorMetric(metric, AggregationTemporality.CUMULATIVE), + transform.toCollectorMetric(metric), hrTimeToNanoseconds(metric.dataPoints[0].endTime), hrTimeToNanoseconds(metric.dataPoints[0].startTime) ); @@ -70,7 +70,7 @@ describe('transformMetrics', () => { const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; ensureDoubleCounterIsCorrect( - transform.toCollectorMetric(metric, AggregationTemporality.CUMULATIVE), + transform.toCollectorMetric(metric), hrTimeToNanoseconds(metric.dataPoints[0].endTime), hrTimeToNanoseconds(metric.dataPoints[0].startTime), ); @@ -91,7 +91,7 @@ describe('transformMetrics', () => { const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; ensureObservableGaugeIsCorrect( - transform.toCollectorMetric(metric, AggregationTemporality.CUMULATIVE), + transform.toCollectorMetric(metric), hrTimeToNanoseconds(metric.dataPoints[0].endTime), hrTimeToNanoseconds(metric.dataPoints[0].startTime), -1, @@ -113,7 +113,7 @@ describe('transformMetrics', () => { const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; ensureObservableCounterIsCorrect( - transform.toCollectorMetric(metric, AggregationTemporality.CUMULATIVE), + transform.toCollectorMetric(metric), hrTimeToNanoseconds(metric.dataPoints[0].endTime), hrTimeToNanoseconds(metric.dataPoints[0].startTime), 2, @@ -134,7 +134,7 @@ describe('transformMetrics', () => { const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; ensureObservableUpDownCounterIsCorrect( - transform.toCollectorMetric(metric, AggregationTemporality.CUMULATIVE), + transform.toCollectorMetric(metric), hrTimeToNanoseconds(metric.dataPoints[0].endTime), hrTimeToNanoseconds(metric.dataPoints[0].startTime), 2, @@ -151,7 +151,7 @@ describe('transformMetrics', () => { const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; ensureHistogramIsCorrect( - transform.toCollectorMetric(metric, AggregationTemporality.CUMULATIVE), + transform.toCollectorMetric(metric), hrTimeToNanoseconds(metric.dataPoints[0].endTime), hrTimeToNanoseconds(metric.dataPoints[0].startTime), [0, 100], @@ -170,6 +170,7 @@ describe('transformMetrics', () => { type: InstrumentType.COUNTER, valueType: 0, }, + aggregationTemporality: AggregationTemporality.CUMULATIVE, dataPoints: [ { value: 1, @@ -180,7 +181,6 @@ describe('transformMetrics', () => { ], dataPointType: DataPointType.SINGULAR, }, - AggregationTemporality.CUMULATIVE ); const collectorMetric = metric.intSum?.dataPoints[0]; assert.strictEqual(collectorMetric?.labels[0].value, '1'); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts index a2852771a4..53edae12ed 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts @@ -16,7 +16,13 @@ import { Counter, Histogram, ObservableResult, ValueType, } from '@opentelemetry/api-metrics'; import { InstrumentationLibrary, VERSION } from '@opentelemetry/core'; -import { ExplicitBucketHistogramAggregation, MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics-base'; +import { + AggregationTemporalitySelector, + CumulativeTemporalitySelector, + ExplicitBucketHistogramAggregation, + MeterProvider, + MetricReader +} from '@opentelemetry/sdk-metrics-base'; import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; import { otlpTypes } from '@opentelemetry/exporter-trace-otlp-http'; @@ -30,6 +36,10 @@ if (typeof Buffer === 'undefined') { } export class TestMetricReader extends MetricReader { + constructor(temporalitySelector?: AggregationTemporalitySelector) { + super(temporalitySelector ?? CumulativeTemporalitySelector); + } + protected onForceFlush(): Promise { return Promise.resolve(undefined); } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index 1e11f733d4..36971fed4f 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -162,7 +162,7 @@ describe('OTLPMetricExporter - node with json over http', () => { url: 'http://foo.bar.com', keepAlive: true, httpAgentOptions: { keepAliveMsecs: 2000 }, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }; collectorExporter = new OTLPMetricExporter(collectorExporterConfig); @@ -333,7 +333,7 @@ describe('OTLPMetricExporter - node with json over http', () => { const url = 'http://foo.bar.com'; const collectorExporter = new OTLPMetricExporter({ url, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }); setTimeout(() => { assert.strictEqual(collectorExporter._otlpExporter.url, url); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts index 95648bb35e..1247fa7b68 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts @@ -20,14 +20,13 @@ import { appendResourcePathToUrlIfNotPresent, } from '@opentelemetry/exporter-trace-otlp-http'; import { - defaultExporterTemporality, defaultOptions, OTLPMetricExporterOptions, toOTLPExportMetricServiceRequest } from '@opentelemetry/exporter-metrics-otlp-http'; import { ServiceClientType, OTLPExporterNodeBase } from '@opentelemetry/exporter-trace-otlp-proto'; import { getEnv, baggageUtils} from '@opentelemetry/core'; -import { AggregationTemporality, ResourceMetrics} from '@opentelemetry/sdk-metrics-base'; +import { ResourceMetrics} from '@opentelemetry/sdk-metrics-base'; import { OTLPMetricExporterBase } from '@opentelemetry/exporter-metrics-otlp-http'; const DEFAULT_COLLECTOR_RESOURCE_PATH = '/v1/metrics'; @@ -36,7 +35,6 @@ const DEFAULT_COLLECTOR_URL = `http://localhost:4318${DEFAULT_COLLECTOR_RESOURCE class OTLPMetricExporterNodeProxy extends OTLPExporterNodeBase { - protected readonly _aggregationTemporality: AggregationTemporality; constructor(config: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions = defaultOptions) { super(config); @@ -46,7 +44,6 @@ class OTLPMetricExporterNodeProxy extends OTLPExporterNodeBase { url: 'http://foo.bar.com', keepAlive: true, httpAgentOptions: { keepAliveMsecs: 2000 }, - aggregationTemporality: AggregationTemporality.CUMULATIVE + preferredAggregationTemporality: AggregationTemporality.CUMULATIVE }; collectorExporter = new OTLPMetricExporter(collectorExporterConfig); setUp(); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts index 8daa508726..ac1407eaa2 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts @@ -25,12 +25,18 @@ import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; import { Stream } from 'stream'; import { + AggregationTemporalitySelector, + CumulativeTemporalitySelector, ExplicitBucketHistogramAggregation, MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics-base'; export class TestMetricReader extends MetricReader { + constructor(temporalitySelector?: AggregationTemporalitySelector) { + super(temporalitySelector ?? CumulativeTemporalitySelector); + } + protected onForceFlush(): Promise { return Promise.resolve(undefined); } diff --git a/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts b/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts index 0be01d6c75..1e67931b4c 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts @@ -55,7 +55,7 @@ export class PrometheusExporter extends MetricReader { * @param callback Callback to be called after a server was started */ constructor(config: ExporterConfig = {}, callback?: () => void) { - super(AggregationTemporality.CUMULATIVE); + super(() => AggregationTemporality.CUMULATIVE); this._host = config.host || process.env.OTEL_EXPORTER_PROMETHEUS_HOST || diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index ec83b35e14..6b25729e2f 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -37,7 +37,7 @@ const attributes = { class TestMetricReader extends MetricReader { constructor() { - super(AggregationTemporality.CUMULATIVE); + super(() => AggregationTemporality.CUMULATIVE); } async onForceFlush() {} async onShutdown() {} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Drop.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Drop.ts index 441ca6e8e4..56ef74e554 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Drop.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Drop.ts @@ -15,6 +15,7 @@ */ import { HrTime } from '@opentelemetry/api'; +import { AggregationTemporality } from '../export/AggregationTemporality'; import { MetricData } from '../export/MetricData'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; @@ -42,6 +43,7 @@ export class DropAggregator implements Aggregator { toMetricData( _descriptor: InstrumentDescriptor, + _aggregationTemporality: AggregationTemporality, _accumulationByAttributes: AccumulationRecord[], _startTime: HrTime, _endTime: HrTime): Maybe { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Histogram.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Histogram.ts index f3f1782d31..bcfa4f69eb 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Histogram.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Histogram.ts @@ -25,6 +25,7 @@ import { HistogramMetricData, DataPointType } from '../export/MetricData'; import { HrTime } from '@opentelemetry/api'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; +import { AggregationTemporality } from '../export/AggregationTemporality'; function createNewEmptyCheckpoint(boundaries: number[]): Histogram { const counts = boundaries.map(() => 0); @@ -134,11 +135,13 @@ export class HistogramAggregator implements Aggregator { toMetricData( descriptor: InstrumentDescriptor, + aggregationTemporality: AggregationTemporality, accumulationByAttributes: AccumulationRecord[], startTime: HrTime, endTime: HrTime): Maybe { return { descriptor, + aggregationTemporality, dataPointType: DataPointType.HISTOGRAM, dataPoints: accumulationByAttributes.map(([attributes, accumulation]) => { return { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/LastValue.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/LastValue.ts index f73891afe2..a6ec036564 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/LastValue.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/LastValue.ts @@ -20,6 +20,7 @@ import { hrTime, hrTimeToMicroseconds } from '@opentelemetry/core'; import { DataPointType, SingularMetricData } from '../export/MetricData'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; +import { AggregationTemporality } from '../export/AggregationTemporality'; export class LastValueAccumulation implements Accumulation { constructor(private _current: number = 0, public sampleTime: HrTime = [0, 0]) {} @@ -67,11 +68,13 @@ export class LastValueAggregator implements Aggregator { toMetricData( descriptor: InstrumentDescriptor, + aggregationTemporality: AggregationTemporality, accumulationByAttributes: AccumulationRecord[], startTime: HrTime, endTime: HrTime): Maybe { return { descriptor, + aggregationTemporality, dataPointType: DataPointType.SINGULAR, dataPoints: accumulationByAttributes.map(([attributes, accumulation]) => { return { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Sum.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Sum.ts index 0b21c7a417..3b8647d730 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Sum.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Sum.ts @@ -19,6 +19,7 @@ import { HrTime } from '@opentelemetry/api'; import { DataPointType, SingularMetricData } from '../export/MetricData'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; +import { AggregationTemporality } from '../export/AggregationTemporality'; export class SumAccumulation implements Accumulation { constructor(private _current: number = 0) {} @@ -56,11 +57,13 @@ export class SumAggregator implements Aggregator { toMetricData( descriptor: InstrumentDescriptor, + aggregationTemporality: AggregationTemporality, accumulationByAttributes: AccumulationRecord[], startTime: HrTime, endTime: HrTime): Maybe { return { descriptor, + aggregationTemporality, dataPointType: DataPointType.SINGULAR, dataPoints: accumulationByAttributes.map(([attributes, accumulation]) => { return { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/types.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/types.ts index 7c972070ba..403eb65155 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/types.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/types.ts @@ -16,6 +16,7 @@ import { HrTime } from '@opentelemetry/api'; import { MetricAttributes } from '@opentelemetry/api-metrics'; +import { AggregationTemporality } from '../export/AggregationTemporality'; import { MetricData } from '../export/MetricData'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; @@ -113,6 +114,7 @@ export interface Aggregator { * @return the {@link MetricData} that this {@link Aggregator} will produce. */ toMetricData(descriptor: InstrumentDescriptor, + aggregationTemporality: AggregationTemporality, accumulationByAttributes: AccumulationRecord[], startTime: HrTime, endTime: HrTime): Maybe; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts index 6cc6d1231b..4c998fab8a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import { InstrumentType } from '../InstrumentDescriptor'; + /** * AggregationTemporality indicates the way additive quantities are expressed. */ @@ -21,3 +23,20 @@ export enum AggregationTemporality { DELTA, CUMULATIVE, } + +export type AggregationTemporalitySelector = (instrumentType: InstrumentType) => AggregationTemporality; + +export const CumulativeTemporalitySelector: AggregationTemporalitySelector = () => AggregationTemporality.CUMULATIVE; + +export const DeltaTemporalitySelector: AggregationTemporalitySelector = (instrumentType: InstrumentType) => { + switch (instrumentType) { + case InstrumentType.COUNTER: + case InstrumentType.OBSERVABLE_COUNTER: + case InstrumentType.HISTOGRAM: + case InstrumentType.OBSERVABLE_GAUGE: + return AggregationTemporality.DELTA; + case InstrumentType.UP_DOWN_COUNTER: + case InstrumentType.OBSERVABLE_UP_DOWN_COUNTER: + return AggregationTemporality.CUMULATIVE; + } +}; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts index 620039cc23..85a5db05ec 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts @@ -20,12 +20,14 @@ import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Histogram } from '../aggregator/types'; +import { AggregationTemporality } from './AggregationTemporality'; /** * Basic metric data fields. */ export interface BaseMetricData { readonly descriptor: InstrumentDescriptor; + readonly aggregationTemporality: AggregationTemporality; /** * DataPointType of the metric instrument. */ diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts index 2d84d94eac..e429c6da22 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts @@ -14,12 +14,13 @@ * limitations under the License. */ -import { AggregationTemporality } from './AggregationTemporality'; +import { AggregationTemporality, AggregationTemporalitySelector, CumulativeTemporalitySelector } from './AggregationTemporality'; import { ResourceMetrics } from './MetricData'; import { ExportResult, ExportResultCode, } from '@opentelemetry/core'; +import { InstrumentType } from '../InstrumentDescriptor'; // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricexporter @@ -30,7 +31,7 @@ export interface PushMetricExporter { forceFlush(): Promise; - getPreferredAggregationTemporality(): AggregationTemporality; + getAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality; shutdown(): Promise; @@ -39,6 +40,9 @@ export interface PushMetricExporter { export class ConsoleMetricExporter implements PushMetricExporter { protected _shutdown = true; + constructor(private _aggregationTemporalitySelector: AggregationTemporalitySelector = CumulativeTemporalitySelector) { + } + export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void) { return resultCallback({ code: ExportResultCode.FAILED, @@ -46,8 +50,8 @@ export class ConsoleMetricExporter implements PushMetricExporter { }); } - getPreferredAggregationTemporality() { - return AggregationTemporality.CUMULATIVE; + getAggregationTemporality(instrumentType: InstrumentType) { + return this._aggregationTemporalitySelector(instrumentType); } // nothing to do diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts index 166819df92..c9932c58cd 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts @@ -15,10 +15,11 @@ */ import * as api from '@opentelemetry/api'; -import { AggregationTemporality } from './AggregationTemporality'; +import { AggregationTemporality, AggregationTemporalitySelector } from './AggregationTemporality'; import { MetricProducer } from './MetricProducer'; import { ResourceMetrics } from './MetricData'; import { callWithTimeout, Maybe } from '../utils'; +import { InstrumentType } from '../InstrumentDescriptor'; import { CollectionOptions, ForceFlushOptions, ShutdownOptions } from '../types'; // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricreader @@ -34,7 +35,7 @@ export abstract class MetricReader { // MetricProducer used by this instance. private _metricProducer?: MetricProducer; - constructor(private readonly _preferredAggregationTemporality = AggregationTemporality.CUMULATIVE) { + constructor(private readonly _aggregationTemporalitySelector: AggregationTemporalitySelector) { } /** @@ -53,8 +54,8 @@ export abstract class MetricReader { /** * Get the {@link AggregationTemporality} preferred by this {@link MetricReader} */ - getPreferredAggregationTemporality(): AggregationTemporality { - return this._preferredAggregationTemporality; + getAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality { + return this._aggregationTemporalitySelector(instrumentType); } /** diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts index bd647f4029..5af051a7cc 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts @@ -40,7 +40,7 @@ export class PeriodicExportingMetricReader extends MetricReader { private readonly _exportTimeout: number; constructor(options: PeriodicExportingMetricReaderOptions) { - super(options.exporter.getPreferredAggregationTemporality()); + super(options.exporter.getAggregationTemporality); if (options.exportIntervalMillis !== undefined && options.exportIntervalMillis <= 0) { throw Error('exportIntervalMillis must be greater than 0'); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts index ac522a293a..e3c36c8cd4 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts @@ -15,10 +15,11 @@ */ import { hrTime } from '@opentelemetry/core'; -import { AggregationTemporality } from '../export/AggregationTemporality'; +import { AggregationTemporalitySelector } from '../export/AggregationTemporality'; import { ResourceMetrics } from '../export/MetricData'; import { MetricProducer } from '../export/MetricProducer'; import { MetricReader } from '../export/MetricReader'; +import { InstrumentType } from '../InstrumentDescriptor'; import { ForceFlushOptions, ShutdownOptions } from '../types'; import { MeterProviderSharedState } from './MeterProviderSharedState'; @@ -28,9 +29,7 @@ import { MeterProviderSharedState } from './MeterProviderSharedState'; * state for each MetricReader. */ export class MetricCollector implements MetricProducer { - public readonly aggregatorTemporality: AggregationTemporality; constructor(private _sharedState: MeterProviderSharedState, private _metricReader: MetricReader) { - this.aggregatorTemporality = this._metricReader.getPreferredAggregationTemporality(); } async collect(): Promise { @@ -57,6 +56,10 @@ export class MetricCollector implements MetricProducer { async shutdown(options?: ShutdownOptions): Promise { await this._metricReader.shutdown(options); } + + getAggregationTemporality(instrumentType: InstrumentType) { + return this._metricReader.getAggregationTemporality(instrumentType); + } } /** @@ -64,5 +67,5 @@ export class MetricCollector implements MetricProducer { * information for metric collection. */ export interface MetricCollectorHandle { - aggregatorTemporality: AggregationTemporality; + getAggregationTemporality: AggregationTemporalitySelector; } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts index c916523a6a..0fe5da40b0 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts @@ -69,7 +69,7 @@ export class TemporalMetricProcessor { sdkStartTime: HrTime, collectionTime: HrTime, ): Maybe { - const aggregationTemporality = collector.aggregatorTemporality; + const aggregationTemporality = collector.getAggregationTemporality(instrumentDescriptor.type); // In case it's our first collection, default to start timestamp (see below for explanation). let lastCollectionTime = sdkStartTime; @@ -108,6 +108,7 @@ export class TemporalMetricProcessor { // 2. Delta Aggregation time span: (lastCollectionTime, collectionTime] return this._aggregator.toMetricData( instrumentDescriptor, + aggregationTemporality, AttributesMapToAccumulationRecords(result), /* startTime */ aggregationTemporality === AggregationTemporality.CUMULATIVE ? sdkStartTime : lastCollectionTime, /* endTime */ collectionTime); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts index 56c16dbac4..247773fe31 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts @@ -521,9 +521,9 @@ function setup() { const meter = meterProvider.getMeter(defaultInstrumentationLibrary.name, defaultInstrumentationLibrary.version, { schemaUrl: defaultInstrumentationLibrary.schemaUrl, }); - const deltaReader = new TestMetricReader(AggregationTemporality.DELTA); + const deltaReader = new TestMetricReader(() => AggregationTemporality.DELTA); meterProvider.addMetricReader(deltaReader); - const cumulativeReader = new TestMetricReader(AggregationTemporality.CUMULATIVE); + const cumulativeReader = new TestMetricReader(() => AggregationTemporality.CUMULATIVE); meterProvider.addMetricReader(cumulativeReader); return { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Drop.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Drop.test.ts index 05242cc192..d378a165c9 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Drop.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Drop.test.ts @@ -16,6 +16,7 @@ import { HrTime } from '@opentelemetry/api'; import * as assert from 'assert'; +import { AggregationTemporality } from '../../src'; import { DropAggregator } from '../../src/aggregator'; import { defaultInstrumentDescriptor } from '../util'; @@ -55,6 +56,7 @@ describe('DropAggregator', () => { assert.strictEqual(aggregator.toMetricData( defaultInstrumentDescriptor, + AggregationTemporality.CUMULATIVE, [[{}, undefined]], startTime, endTime, diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Histogram.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Histogram.test.ts index 08bfca8889..2b4e685915 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Histogram.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Histogram.test.ts @@ -16,6 +16,7 @@ import { HrTime } from '@opentelemetry/api'; import * as assert from 'assert'; +import { AggregationTemporality } from '../../src'; import { HistogramAccumulation, HistogramAggregator } from '../../src/aggregator'; import { MetricData, DataPointType } from '../../src/export/MetricData'; import { commonValues, defaultInstrumentDescriptor } from '../util'; @@ -93,6 +94,7 @@ describe('HistogramAggregator', () => { const expected: MetricData = { descriptor: defaultInstrumentDescriptor, + aggregationTemporality: AggregationTemporality.CUMULATIVE, dataPointType: DataPointType.HISTOGRAM, dataPoints: [ { @@ -112,6 +114,7 @@ describe('HistogramAggregator', () => { }; assert.deepStrictEqual(aggregator.toMetricData( defaultInstrumentDescriptor, + AggregationTemporality.CUMULATIVE, [[{}, accumulation]], startTime, endTime, diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/LastValue.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/LastValue.test.ts index 03c75a7b2d..63df2357c8 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/LastValue.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/LastValue.test.ts @@ -16,6 +16,7 @@ import { HrTime } from '@opentelemetry/api'; import * as assert from 'assert'; +import { AggregationTemporality } from '../../src'; import { LastValueAccumulation, LastValueAggregator } from '../../src/aggregator'; import { MetricData, DataPointType } from '../../src/export/MetricData'; import { commonValues, defaultInstrumentDescriptor, sleep } from '../util'; @@ -101,6 +102,7 @@ describe('LastValueAggregator', () => { const expected: MetricData = { descriptor: defaultInstrumentDescriptor, + aggregationTemporality: AggregationTemporality.CUMULATIVE, dataPointType: DataPointType.SINGULAR, dataPoints: [ { @@ -113,6 +115,7 @@ describe('LastValueAggregator', () => { }; assert.deepStrictEqual(aggregator.toMetricData( defaultInstrumentDescriptor, + AggregationTemporality.CUMULATIVE, [[{}, accumulation]], startTime, endTime, diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Sum.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Sum.test.ts index 3c9e0a48b4..64db048d4e 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Sum.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Sum.test.ts @@ -16,6 +16,7 @@ import { HrTime } from '@opentelemetry/api'; import * as assert from 'assert'; +import { AggregationTemporality } from '../../src'; import { SumAccumulation, SumAggregator } from '../../src/aggregator'; import { MetricData, DataPointType } from '../../src/export/MetricData'; import { commonValues, defaultInstrumentDescriptor } from '../util'; @@ -79,6 +80,7 @@ describe('SumAggregator', () => { const expected: MetricData = { descriptor: defaultInstrumentDescriptor, + aggregationTemporality: AggregationTemporality.CUMULATIVE, dataPointType: DataPointType.SINGULAR, dataPoints: [ { @@ -91,6 +93,7 @@ describe('SumAggregator', () => { }; assert.deepStrictEqual(aggregator.toMetricData( defaultInstrumentDescriptor, + AggregationTemporality.CUMULATIVE, [[{}, accumulation]], startTime, endTime, diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts index 1cde6184c2..64538f984f 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts @@ -16,7 +16,7 @@ import { PeriodicExportingMetricReader } from '../../src/export/PeriodicExportingMetricReader'; import { AggregationTemporality } from '../../src/export/AggregationTemporality'; -import { PushMetricExporter } from '../../src'; +import { InstrumentType, PushMetricExporter } from '../../src'; import { ResourceMetrics } from '../../src/export/MetricData'; import * as assert from 'assert'; import * as sinon from 'sinon'; @@ -77,13 +77,13 @@ class TestMetricExporter implements PushMetricExporter { return this._batches.slice(0, numberOfExports); } - getPreferredAggregationTemporality(): AggregationTemporality { + getAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality { return AggregationTemporality.CUMULATIVE; } } class TestDeltaMetricExporter extends TestMetricExporter { - override getPreferredAggregationTemporality(): AggregationTemporality { + override getAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality { return AggregationTemporality.DELTA; } } @@ -107,13 +107,12 @@ describe('PeriodicExportingMetricReader', () => { describe('constructor', () => { it('should construct PeriodicExportingMetricReader without exceptions', () => { const exporter = new TestDeltaMetricExporter(); - const reader = new PeriodicExportingMetricReader({ - exporter: exporter, + assert.doesNotThrow(() => new PeriodicExportingMetricReader({ + exporter, exportIntervalMillis: 4000, exportTimeoutMillis: 3000 } - ); - assert.strictEqual(reader.getPreferredAggregationTemporality(), exporter.getPreferredAggregationTemporality()); + )); }); it('should throw when interval less or equal to 0', () => { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts index 43df025473..55b04a71dd 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts @@ -27,13 +27,13 @@ export class TestMetricExporter implements PushMetricExporter { async forceFlush(): Promise {} async shutdown(): Promise {} - getPreferredAggregationTemporality(): AggregationTemporality { + getAggregationTemporality(): AggregationTemporality { return AggregationTemporality.CUMULATIVE; } } export class TestDeltaMetricExporter extends TestMetricExporter { - override getPreferredAggregationTemporality(): AggregationTemporality { + override getAggregationTemporality(): AggregationTemporality { return AggregationTemporality.DELTA; } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts index 172301d958..4607bf575c 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts @@ -14,13 +14,17 @@ * limitations under the License. */ -import { MetricReader } from '../../src'; +import { AggregationTemporalitySelector, CumulativeTemporalitySelector, MetricReader } from '../../src'; import { MetricCollector } from '../../src/state/MetricCollector'; /** * A test metric reader that implements no-op onForceFlush() and onShutdown() handlers. */ export class TestMetricReader extends MetricReader { + constructor(aggregationTemporalitySelector?: AggregationTemporalitySelector) { + super(aggregationTemporalitySelector ?? CumulativeTemporalitySelector); + } + protected onForceFlush(): Promise { return Promise.resolve(undefined); } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/AsyncMetricStorage.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/AsyncMetricStorage.test.ts index aa6e824666..12914ee1b8 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/AsyncMetricStorage.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/AsyncMetricStorage.test.ts @@ -27,11 +27,11 @@ import { assertMetricData, assertDataPoint, defaultInstrumentDescriptor } from ' import { ObservableCallback } from '@opentelemetry/api-metrics'; const deltaCollector: MetricCollectorHandle = { - aggregatorTemporality: AggregationTemporality.DELTA, + getAggregationTemporality: () => AggregationTemporality.DELTA, }; const cumulativeCollector: MetricCollectorHandle = { - aggregatorTemporality: AggregationTemporality.CUMULATIVE, + getAggregationTemporality: () => AggregationTemporality.CUMULATIVE, }; const sdkStartTime = hrTime(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts index 1e0cd68f99..941c33103b 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts @@ -16,10 +16,9 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; -import { Meter, MeterProvider, DataPointType } from '../../src'; +import { Meter, MeterProvider, DataPointType, CumulativeTemporalitySelector, DeltaTemporalitySelector } from '../../src'; import { assertMetricData, defaultInstrumentationLibrary, defaultResource } from '../util'; import { TestMetricReader } from '../export/TestMetricReader'; -import { TestDeltaMetricExporter, TestMetricExporter } from '../export/TestMetricExporter'; import { MeterSharedState } from '../../src/state/MeterSharedState'; describe('MeterSharedState', () => { @@ -31,11 +30,11 @@ describe('MeterSharedState', () => { function setupInstruments() { const meterProvider = new MeterProvider({ resource: defaultResource }); - const cumulativeReader = new TestMetricReader(new TestMetricExporter().getPreferredAggregationTemporality()); + const cumulativeReader = new TestMetricReader(CumulativeTemporalitySelector); meterProvider.addMetricReader(cumulativeReader); const cumulativeCollector = cumulativeReader.getMetricCollector(); - const deltaReader = new TestMetricReader(new TestDeltaMetricExporter().getPreferredAggregationTemporality()); + const deltaReader = new TestMetricReader(DeltaTemporalitySelector); meterProvider.addMetricReader(deltaReader); const deltaCollector = deltaReader.getMetricCollector(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts index 79d29f8560..709c3d3de0 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts @@ -35,10 +35,8 @@ describe('MetricCollector', () => { const meterProviderSharedState = new MeterProviderSharedState(defaultResource); const exporters = [ new TestMetricExporter(), new TestDeltaMetricExporter() ]; for (const exporter of exporters) { - const reader = new TestMetricReader(exporter.getPreferredAggregationTemporality()); - const metricCollector = new MetricCollector(meterProviderSharedState, reader); - - assert.strictEqual(metricCollector.aggregatorTemporality, exporter.getPreferredAggregationTemporality()); + const reader = new TestMetricReader(exporter.getAggregationTemporality); + assert.doesNotThrow(() => new MetricCollector(meterProviderSharedState, reader)); } }); }); @@ -48,7 +46,7 @@ describe('MetricCollector', () => { function setupInstruments(exporter: PushMetricExporter) { const meterProvider = new MeterProvider({ resource: defaultResource }); - const reader = new TestMetricReader(exporter.getPreferredAggregationTemporality()); + const reader = new TestMetricReader(exporter.getAggregationTemporality); meterProvider.addMetricReader(reader); const metricCollector = reader.getMetricCollector(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/SyncMetricStorage.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/SyncMetricStorage.test.ts index 6b7fe30c8c..935dadacfa 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/SyncMetricStorage.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/SyncMetricStorage.test.ts @@ -27,11 +27,11 @@ import { NoopAttributesProcessor } from '../../src/view/AttributesProcessor'; import { assertMetricData, assertDataPoint, commonAttributes, commonValues, defaultInstrumentDescriptor } from '../util'; const deltaCollector: MetricCollectorHandle = { - aggregatorTemporality: AggregationTemporality.DELTA, + getAggregationTemporality: () => AggregationTemporality.DELTA, }; const cumulativeCollector: MetricCollectorHandle = { - aggregatorTemporality: AggregationTemporality.CUMULATIVE, + getAggregationTemporality: () => AggregationTemporality.CUMULATIVE, }; const sdkStartTime = hrTime(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/TemporalMetricProcessor.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/TemporalMetricProcessor.test.ts index db0b50e033..dc8160be75 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/TemporalMetricProcessor.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/TemporalMetricProcessor.test.ts @@ -26,15 +26,15 @@ import { TemporalMetricProcessor } from '../../src/state/TemporalMetricProcessor import { assertMetricData, assertDataPoint, defaultInstrumentDescriptor } from '../util'; const deltaCollector1: MetricCollectorHandle = { - aggregatorTemporality: AggregationTemporality.DELTA, + getAggregationTemporality: () => AggregationTemporality.DELTA, }; const deltaCollector2: MetricCollectorHandle = { - aggregatorTemporality: AggregationTemporality.DELTA, + getAggregationTemporality: () => AggregationTemporality.DELTA, }; const cumulativeCollector1: MetricCollectorHandle = { - aggregatorTemporality: AggregationTemporality.CUMULATIVE, + getAggregationTemporality: () => AggregationTemporality.CUMULATIVE, }; const sdkStartTime = hrTime(); diff --git a/experimental/packages/otlp-transformer/src/metrics/index.ts b/experimental/packages/otlp-transformer/src/metrics/index.ts index e5a8560ded..af14b98d11 100644 --- a/experimental/packages/otlp-transformer/src/metrics/index.ts +++ b/experimental/packages/otlp-transformer/src/metrics/index.ts @@ -17,10 +17,8 @@ import type { ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { toAttributes } from '../common/internal'; import { toMetric } from './internal'; import type { IExportMetricsServiceRequest } from './types'; -import { AggregationTemporality } from '@opentelemetry/sdk-metrics-base'; -export function createExportMetricsServiceRequest(resourceMetrics: ResourceMetrics, - aggregationTemporality: AggregationTemporality): IExportMetricsServiceRequest | null { +export function createExportMetricsServiceRequest(resourceMetrics: ResourceMetrics): IExportMetricsServiceRequest | null { return { resourceMetrics: [{ resource: { @@ -34,7 +32,7 @@ export function createExportMetricsServiceRequest(resourceMetrics: ResourceMetri name: metrics.instrumentationLibrary.name, version: metrics.instrumentationLibrary.version, }, - metrics: metrics.metrics.map(metricData => toMetric(metricData, aggregationTemporality)), + metrics: metrics.metrics.map(metricData => toMetric(metricData)), schemaUrl: metrics.instrumentationLibrary.schemaUrl }; })) diff --git a/experimental/packages/otlp-transformer/src/metrics/internal.ts b/experimental/packages/otlp-transformer/src/metrics/internal.ts index 1dab2f4417..6370fcf51c 100644 --- a/experimental/packages/otlp-transformer/src/metrics/internal.ts +++ b/experimental/packages/otlp-transformer/src/metrics/internal.ts @@ -27,14 +27,14 @@ import { toAttributes } from '../common/internal'; import { EAggregationTemporality, IHistogramDataPoint, IMetric, INumberDataPoint } from './types'; -export function toMetric(metricData: MetricData, metricTemporality: AggregationTemporality): IMetric { +export function toMetric(metricData: MetricData): IMetric { const out: IMetric = { name: metricData.descriptor.name, description: metricData.descriptor.description, unit: metricData.descriptor.unit, }; - const aggregationTemporality = toAggregationTemporality(metricTemporality); + const aggregationTemporality = toAggregationTemporality(metricData.aggregationTemporality); if (metricData.dataPointType === DataPointType.SINGULAR) { const dataPoints = toSingularDataPoints(metricData); diff --git a/experimental/packages/otlp-transformer/test/metrics.test.ts b/experimental/packages/otlp-transformer/test/metrics.test.ts index 37ecdaf560..c8006f17b3 100644 --- a/experimental/packages/otlp-transformer/test/metrics.test.ts +++ b/experimental/packages/otlp-transformer/test/metrics.test.ts @@ -32,7 +32,7 @@ const END_TIME = hrTime(); describe('Metrics', () => { describe('createExportMetricsServiceRequest', () => { - function createCounterData(value: number): MetricData { + function createCounterData(value: number, aggregationTemporality: AggregationTemporality): MetricData { return { descriptor: { description: 'this is a description', @@ -41,6 +41,7 @@ describe('Metrics', () => { unit: '1', valueType: ValueType.INT, }, + aggregationTemporality, dataPointType: DataPointType.SINGULAR, dataPoints: [ { @@ -53,7 +54,7 @@ describe('Metrics', () => { }; } - function createObservableCounterData(value: number): MetricData { + function createObservableCounterData(value: number, aggregationTemporality: AggregationTemporality): MetricData { return { descriptor: { description: 'this is a description', @@ -62,6 +63,7 @@ describe('Metrics', () => { unit: '1', valueType: ValueType.INT, }, + aggregationTemporality, dataPointType: DataPointType.SINGULAR, dataPoints: [ { @@ -83,6 +85,7 @@ describe('Metrics', () => { unit: '1', valueType: ValueType.DOUBLE, }, + aggregationTemporality: AggregationTemporality.CUMULATIVE, dataPointType: DataPointType.SINGULAR, dataPoints: [ { @@ -95,7 +98,7 @@ describe('Metrics', () => { }; } - function createHistogramMetrics(count: number, sum: number, boundaries: number[], counts: number[]): MetricData { + function createHistogramMetrics(count: number, sum: number, boundaries: number[], counts: number[], aggregationTemporality: AggregationTemporality): MetricData { return { descriptor: { description: 'this is a description', @@ -104,6 +107,7 @@ describe('Metrics', () => { unit: '1', valueType: ValueType.INT, }, + aggregationTemporality, dataPointType: DataPointType.HISTOGRAM, dataPoints: [ { @@ -144,11 +148,8 @@ describe('Metrics', () => { } it('serializes a sum metric record', () => { - const metrics = createResourceMetrics([createCounterData(10)]); - const exportRequest = createExportMetricsServiceRequest( - metrics, - AggregationTemporality.DELTA - ); + const metrics = createResourceMetrics([createCounterData(10,AggregationTemporality.DELTA)]); + const exportRequest = createExportMetricsServiceRequest(metrics); assert.ok(exportRequest); assert.deepStrictEqual(exportRequest, { @@ -208,8 +209,7 @@ describe('Metrics', () => { it('serializes an observable sum metric record', () => { const exportRequest = createExportMetricsServiceRequest( - createResourceMetrics([createObservableCounterData(10)]), - AggregationTemporality.DELTA + createResourceMetrics([createObservableCounterData(10, AggregationTemporality.DELTA)]) ); assert.ok(exportRequest); @@ -271,7 +271,6 @@ describe('Metrics', () => { it('serializes a gauge metric record', () => { const exportRequest = createExportMetricsServiceRequest( createResourceMetrics([createObservableGaugeData(10.5)]), - AggregationTemporality.DELTA ); assert.ok(exportRequest); @@ -330,8 +329,7 @@ describe('Metrics', () => { it('serializes a histogram metric record', () => { const exportRequest = createExportMetricsServiceRequest( - createResourceMetrics([createHistogramMetrics(2, 9, [5], [1,1])]), - AggregationTemporality.CUMULATIVE + createResourceMetrics([createHistogramMetrics(2, 9, [5], [1,1], AggregationTemporality.CUMULATIVE)]) ); assert.ok(exportRequest); From 391dcb89021827d772d14a4aef0ed445b5114491 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Mon, 18 Apr 2022 14:03:36 +0300 Subject: [PATCH 2/7] chore: update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58c9a285f2..9f645a40e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable changes to this project will be documented in this file. ### :boom: Breaking Change +* feat: metric aggregation temporality controls [#2902](https://github.com/open-telemetry/opentelemetry-js/pull/2902) @seemk + ### :rocket: (Enhancement) ### :bug: (Bug Fix) From dda47542fc97bf931b9f193aebb3366bf8c94920 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 22 Apr 2022 17:01:10 +0300 Subject: [PATCH 3/7] refactor: move named otlp temporality selectors to OTLPMetricExporterBase --- .../test/metricsHelper.ts | 13 +++++++++---- .../src/OTLPMetricExporterBase.ts | 19 ++++++++++++++++--- .../test/metricsHelper.ts | 13 ++++++------- .../test/metricsHelper.ts | 7 +++---- .../src/PrometheusExporter.ts | 6 +++++- .../test/PrometheusSerializer.test.ts | 9 +++++++-- .../src/export/AggregationTemporality.ts | 15 --------------- .../src/export/MetricExporter.ts | 12 +++++++----- .../src/export/MetricReader.ts | 11 +++-------- .../export/PeriodicExportingMetricReader.ts | 8 +++++++- .../src/state/MetricCollector.ts | 6 +++--- .../src/state/TemporalMetricProcessor.ts | 2 +- .../PeriodicExportingMetricReader.test.ts | 4 ++-- .../test/export/TestMetricExporter.ts | 4 ++-- .../test/export/TestMetricReader.ts | 16 ++++++++++++++-- .../test/state/AsyncMetricStorage.test.ts | 4 ++-- .../test/state/MeterSharedState.test.ts | 6 +++--- .../test/state/MetricCollector.test.ts | 4 ++-- .../test/state/SyncMetricStorage.test.ts | 4 ++-- .../state/TemporalMetricProcessor.test.ts | 6 +++--- 20 files changed, 97 insertions(+), 72 deletions(-) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts index 071f67e51a..51e93e875d 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts @@ -20,11 +20,16 @@ import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; import * as grpc from '@grpc/grpc-js'; import { VERSION } from '@opentelemetry/core'; -import { AggregationTemporalitySelector, CumulativeTemporalitySelector, ExplicitBucketHistogramAggregation, MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics-base'; +import { + AggregationTemporality, + ExplicitBucketHistogramAggregation, + MeterProvider, + MetricReader, +} from '@opentelemetry/sdk-metrics-base'; -export class TestMetricReader extends MetricReader { - constructor(temporalitySelector?: AggregationTemporalitySelector) { - super(temporalitySelector ?? CumulativeTemporalitySelector); +class TestMetricReader extends MetricReader { + selectAggregationTemporality() { + return AggregationTemporality.CUMULATIVE; } protected onForceFlush(): Promise { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts index a79ce87098..e87c0ad392 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts @@ -18,8 +18,6 @@ import { ExportResult } from '@opentelemetry/core'; import { AggregationTemporality, AggregationTemporalitySelector, - CumulativeTemporalitySelector, - DeltaTemporalitySelector, InstrumentType, PushMetricExporter, ResourceMetrics @@ -28,6 +26,21 @@ import { otlpTypes } from '@opentelemetry/exporter-trace-otlp-http'; import { defaultOptions, OTLPMetricExporterOptions } from './OTLPMetricExporterOptions'; import { OTLPExporterBase } from '@opentelemetry/otlp-exporter-base'; +export const CumulativeTemporalitySelector: AggregationTemporalitySelector = () => AggregationTemporality.CUMULATIVE; + +export const DeltaTemporalitySelector: AggregationTemporalitySelector = (instrumentType: InstrumentType) => { + switch (instrumentType) { + case InstrumentType.COUNTER: + case InstrumentType.OBSERVABLE_COUNTER: + case InstrumentType.HISTOGRAM: + case InstrumentType.OBSERVABLE_GAUGE: + return AggregationTemporality.DELTA; + case InstrumentType.UP_DOWN_COUNTER: + case InstrumentType.OBSERVABLE_UP_DOWN_COUNTER: + return AggregationTemporality.CUMULATIVE; + } +}; + function chooseTemporalitySelector(preferredAggregationTemporality?: AggregationTemporality): AggregationTemporalitySelector { if (preferredAggregationTemporality === AggregationTemporality.DELTA) { return DeltaTemporalitySelector; @@ -61,7 +74,7 @@ export class OTLPMetricExporterBase { return Promise.resolve(undefined); } @@ -47,6 +42,10 @@ export class TestMetricReader extends MetricReader { protected onShutdown(): Promise { return Promise.resolve(undefined); } + + selectAggregationTemporality() { + return AggregationTemporality.CUMULATIVE; + } } const defaultResource = Resource.default().merge(new Resource({ diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts index ac1407eaa2..b70fb1a9dc 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts @@ -25,16 +25,15 @@ import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; import { Stream } from 'stream'; import { - AggregationTemporalitySelector, - CumulativeTemporalitySelector, + AggregationTemporality, ExplicitBucketHistogramAggregation, MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics-base'; export class TestMetricReader extends MetricReader { - constructor(temporalitySelector?: AggregationTemporalitySelector) { - super(temporalitySelector ?? CumulativeTemporalitySelector); + selectAggregationTemporality() { + return AggregationTemporality.CUMULATIVE; } protected onForceFlush(): Promise { diff --git a/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts b/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts index 1e67931b4c..aa7491bc77 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts @@ -55,7 +55,7 @@ export class PrometheusExporter extends MetricReader { * @param callback Callback to be called after a server was started */ constructor(config: ExporterConfig = {}, callback?: () => void) { - super(() => AggregationTemporality.CUMULATIVE); + super(); this._host = config.host || process.env.OTEL_EXPORTER_PROMETHEUS_HOST || @@ -90,6 +90,10 @@ export class PrometheusExporter extends MetricReader { } } + selectAggregationTemporality(): AggregationTemporality { + return AggregationTemporality.CUMULATIVE; + } + override async onForceFlush(): Promise { /** do nothing */ } diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index 6b25729e2f..bd1a6b4602 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -37,8 +37,13 @@ const attributes = { class TestMetricReader extends MetricReader { constructor() { - super(() => AggregationTemporality.CUMULATIVE); -} + super(); + } + + selectAggregationTemporality() { + return AggregationTemporality.CUMULATIVE; + } + async onForceFlush() {} async onShutdown() {} } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts index 4c998fab8a..0b93671472 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts @@ -25,18 +25,3 @@ export enum AggregationTemporality { } export type AggregationTemporalitySelector = (instrumentType: InstrumentType) => AggregationTemporality; - -export const CumulativeTemporalitySelector: AggregationTemporalitySelector = () => AggregationTemporality.CUMULATIVE; - -export const DeltaTemporalitySelector: AggregationTemporalitySelector = (instrumentType: InstrumentType) => { - switch (instrumentType) { - case InstrumentType.COUNTER: - case InstrumentType.OBSERVABLE_COUNTER: - case InstrumentType.HISTOGRAM: - case InstrumentType.OBSERVABLE_GAUGE: - return AggregationTemporality.DELTA; - case InstrumentType.UP_DOWN_COUNTER: - case InstrumentType.OBSERVABLE_UP_DOWN_COUNTER: - return AggregationTemporality.CUMULATIVE; - } -}; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts index e429c6da22..71a7fb26a5 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { AggregationTemporality, AggregationTemporalitySelector, CumulativeTemporalitySelector } from './AggregationTemporality'; +import { AggregationTemporality } from './AggregationTemporality'; import { ResourceMetrics } from './MetricData'; import { ExportResult, @@ -31,7 +31,7 @@ export interface PushMetricExporter { forceFlush(): Promise; - getAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality; + selectAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality; shutdown(): Promise; @@ -39,8 +39,10 @@ export interface PushMetricExporter { export class ConsoleMetricExporter implements PushMetricExporter { protected _shutdown = true; + private _aggregationTemporality: AggregationTemporality; - constructor(private _aggregationTemporalitySelector: AggregationTemporalitySelector = CumulativeTemporalitySelector) { + constructor(aggregationTemporality?: AggregationTemporality) { + this._aggregationTemporality = aggregationTemporality ?? AggregationTemporality.CUMULATIVE; } export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void) { @@ -50,8 +52,8 @@ export class ConsoleMetricExporter implements PushMetricExporter { }); } - getAggregationTemporality(instrumentType: InstrumentType) { - return this._aggregationTemporalitySelector(instrumentType); + selectAggregationTemporality(_instrumentType: InstrumentType) { + return this._aggregationTemporality; } // nothing to do diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts index c9932c58cd..44581649f0 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts @@ -15,7 +15,7 @@ */ import * as api from '@opentelemetry/api'; -import { AggregationTemporality, AggregationTemporalitySelector } from './AggregationTemporality'; +import { AggregationTemporality } from './AggregationTemporality'; import { MetricProducer } from './MetricProducer'; import { ResourceMetrics } from './MetricData'; import { callWithTimeout, Maybe } from '../utils'; @@ -35,9 +35,6 @@ export abstract class MetricReader { // MetricProducer used by this instance. private _metricProducer?: MetricProducer; - constructor(private readonly _aggregationTemporalitySelector: AggregationTemporalitySelector) { - } - /** * Set the {@link MetricProducer} used by this instance. * @@ -52,11 +49,9 @@ export abstract class MetricReader { } /** - * Get the {@link AggregationTemporality} preferred by this {@link MetricReader} + * Get the default {@link AggregationTemporality} for the given {@link InstrumentType} */ - getAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality { - return this._aggregationTemporalitySelector(instrumentType); - } + abstract selectAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality; /** * Handle once the SDK has initialized this {@link MetricReader} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts index 5af051a7cc..0db14825ff 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts @@ -17,6 +17,8 @@ import * as api from '@opentelemetry/api'; import { ExportResultCode, globalErrorHandler } from '@opentelemetry/core'; import { MetricReader } from './MetricReader'; +import { AggregationTemporality } from './AggregationTemporality'; +import { InstrumentType } from '../InstrumentDescriptor'; import { PushMetricExporter } from './MetricExporter'; import { callWithTimeout, TimeoutError } from '../utils'; @@ -40,7 +42,7 @@ export class PeriodicExportingMetricReader extends MetricReader { private readonly _exportTimeout: number; constructor(options: PeriodicExportingMetricReaderOptions) { - super(options.exporter.getAggregationTemporality); + super(); if (options.exportIntervalMillis !== undefined && options.exportIntervalMillis <= 0) { throw Error('exportIntervalMillis must be greater than 0'); @@ -111,4 +113,8 @@ export class PeriodicExportingMetricReader extends MetricReader { await this._exporter.shutdown(); } + + selectAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality { + return this._exporter.selectAggregationTemporality(instrumentType); + } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts index df1d63417d..fb1d7885a9 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts @@ -58,8 +58,8 @@ export class MetricCollector implements MetricProducer { await this._metricReader.shutdown(options); } - getAggregationTemporality(instrumentType: InstrumentType) { - return this._metricReader.getAggregationTemporality(instrumentType); + selectAggregationTemporality(instrumentType: InstrumentType) { + return this._metricReader.selectAggregationTemporality(instrumentType); } } @@ -68,5 +68,5 @@ export class MetricCollector implements MetricProducer { * information for metric collection. */ export interface MetricCollectorHandle { - getAggregationTemporality: AggregationTemporalitySelector; + selectAggregationTemporality: AggregationTemporalitySelector; } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts index 0fe5da40b0..c9131e2252 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts @@ -69,7 +69,7 @@ export class TemporalMetricProcessor { sdkStartTime: HrTime, collectionTime: HrTime, ): Maybe { - const aggregationTemporality = collector.getAggregationTemporality(instrumentDescriptor.type); + const aggregationTemporality = collector.selectAggregationTemporality(instrumentDescriptor.type); // In case it's our first collection, default to start timestamp (see below for explanation). let lastCollectionTime = sdkStartTime; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts index 64538f984f..bba89f8c32 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts @@ -77,13 +77,13 @@ class TestMetricExporter implements PushMetricExporter { return this._batches.slice(0, numberOfExports); } - getAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality { + selectAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality { return AggregationTemporality.CUMULATIVE; } } class TestDeltaMetricExporter extends TestMetricExporter { - override getAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality { + override selectAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality { return AggregationTemporality.DELTA; } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts index 55b04a71dd..67f33a898d 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts @@ -27,13 +27,13 @@ export class TestMetricExporter implements PushMetricExporter { async forceFlush(): Promise {} async shutdown(): Promise {} - getAggregationTemporality(): AggregationTemporality { + selectAggregationTemporality(): AggregationTemporality { return AggregationTemporality.CUMULATIVE; } } export class TestDeltaMetricExporter extends TestMetricExporter { - override getAggregationTemporality(): AggregationTemporality { + override selectAggregationTemporality(): AggregationTemporality { return AggregationTemporality.DELTA; } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts index 4607bf575c..4b02562c94 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts @@ -14,15 +14,23 @@ * limitations under the License. */ -import { AggregationTemporalitySelector, CumulativeTemporalitySelector, MetricReader } from '../../src'; +import { + AggregationTemporality, + AggregationTemporalitySelector, + InstrumentType, + MetricReader, +} from '../../src'; import { MetricCollector } from '../../src/state/MetricCollector'; /** * A test metric reader that implements no-op onForceFlush() and onShutdown() handlers. */ export class TestMetricReader extends MetricReader { + private _aggregationTemporalitySelector: AggregationTemporalitySelector; + constructor(aggregationTemporalitySelector?: AggregationTemporalitySelector) { - super(aggregationTemporalitySelector ?? CumulativeTemporalitySelector); + super(); + this._aggregationTemporalitySelector = aggregationTemporalitySelector ?? (() => AggregationTemporality.CUMULATIVE); } protected onForceFlush(): Promise { @@ -33,6 +41,10 @@ export class TestMetricReader extends MetricReader { return Promise.resolve(undefined); } + selectAggregationTemporality(instrumentType: InstrumentType) { + return this._aggregationTemporalitySelector(instrumentType); + } + getMetricCollector(): MetricCollector { return this['_metricProducer'] as MetricCollector; } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/AsyncMetricStorage.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/AsyncMetricStorage.test.ts index 12914ee1b8..8bd5f7d5e3 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/AsyncMetricStorage.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/AsyncMetricStorage.test.ts @@ -27,11 +27,11 @@ import { assertMetricData, assertDataPoint, defaultInstrumentDescriptor } from ' import { ObservableCallback } from '@opentelemetry/api-metrics'; const deltaCollector: MetricCollectorHandle = { - getAggregationTemporality: () => AggregationTemporality.DELTA, + selectAggregationTemporality: () => AggregationTemporality.DELTA, }; const cumulativeCollector: MetricCollectorHandle = { - getAggregationTemporality: () => AggregationTemporality.CUMULATIVE, + selectAggregationTemporality: () => AggregationTemporality.CUMULATIVE, }; const sdkStartTime = hrTime(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts index 941c33103b..7ae11d66a5 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts @@ -16,7 +16,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; -import { Meter, MeterProvider, DataPointType, CumulativeTemporalitySelector, DeltaTemporalitySelector } from '../../src'; +import { AggregationTemporality, Meter, MeterProvider, DataPointType } from '../../src'; import { assertMetricData, defaultInstrumentationLibrary, defaultResource } from '../util'; import { TestMetricReader } from '../export/TestMetricReader'; import { MeterSharedState } from '../../src/state/MeterSharedState'; @@ -30,11 +30,11 @@ describe('MeterSharedState', () => { function setupInstruments() { const meterProvider = new MeterProvider({ resource: defaultResource }); - const cumulativeReader = new TestMetricReader(CumulativeTemporalitySelector); + const cumulativeReader = new TestMetricReader(() => AggregationTemporality.CUMULATIVE); meterProvider.addMetricReader(cumulativeReader); const cumulativeCollector = cumulativeReader.getMetricCollector(); - const deltaReader = new TestMetricReader(DeltaTemporalitySelector); + const deltaReader = new TestMetricReader(() => AggregationTemporality.DELTA); meterProvider.addMetricReader(deltaReader); const deltaCollector = deltaReader.getMetricCollector(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts index 709c3d3de0..dd5fac5127 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts @@ -35,7 +35,7 @@ describe('MetricCollector', () => { const meterProviderSharedState = new MeterProviderSharedState(defaultResource); const exporters = [ new TestMetricExporter(), new TestDeltaMetricExporter() ]; for (const exporter of exporters) { - const reader = new TestMetricReader(exporter.getAggregationTemporality); + const reader = new TestMetricReader(exporter.selectAggregationTemporality); assert.doesNotThrow(() => new MetricCollector(meterProviderSharedState, reader)); } }); @@ -46,7 +46,7 @@ describe('MetricCollector', () => { function setupInstruments(exporter: PushMetricExporter) { const meterProvider = new MeterProvider({ resource: defaultResource }); - const reader = new TestMetricReader(exporter.getAggregationTemporality); + const reader = new TestMetricReader(exporter.selectAggregationTemporality); meterProvider.addMetricReader(reader); const metricCollector = reader.getMetricCollector(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/SyncMetricStorage.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/SyncMetricStorage.test.ts index 935dadacfa..c7896f0a19 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/SyncMetricStorage.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/SyncMetricStorage.test.ts @@ -27,11 +27,11 @@ import { NoopAttributesProcessor } from '../../src/view/AttributesProcessor'; import { assertMetricData, assertDataPoint, commonAttributes, commonValues, defaultInstrumentDescriptor } from '../util'; const deltaCollector: MetricCollectorHandle = { - getAggregationTemporality: () => AggregationTemporality.DELTA, + selectAggregationTemporality: () => AggregationTemporality.DELTA, }; const cumulativeCollector: MetricCollectorHandle = { - getAggregationTemporality: () => AggregationTemporality.CUMULATIVE, + selectAggregationTemporality: () => AggregationTemporality.CUMULATIVE, }; const sdkStartTime = hrTime(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/TemporalMetricProcessor.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/TemporalMetricProcessor.test.ts index dc8160be75..7a9710db15 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/TemporalMetricProcessor.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/TemporalMetricProcessor.test.ts @@ -26,15 +26,15 @@ import { TemporalMetricProcessor } from '../../src/state/TemporalMetricProcessor import { assertMetricData, assertDataPoint, defaultInstrumentDescriptor } from '../util'; const deltaCollector1: MetricCollectorHandle = { - getAggregationTemporality: () => AggregationTemporality.DELTA, + selectAggregationTemporality: () => AggregationTemporality.DELTA, }; const deltaCollector2: MetricCollectorHandle = { - getAggregationTemporality: () => AggregationTemporality.DELTA, + selectAggregationTemporality: () => AggregationTemporality.DELTA, }; const cumulativeCollector1: MetricCollectorHandle = { - getAggregationTemporality: () => AggregationTemporality.CUMULATIVE, + selectAggregationTemporality: () => AggregationTemporality.CUMULATIVE, }; const sdkStartTime = hrTime(); From 39b765ce41a11e3d4fa01deab149bdfc8973349c Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 22 Apr 2022 17:45:26 +0300 Subject: [PATCH 4/7] fix: revert accidental protos change --- experimental/packages/otlp-grpc-exporter-base/protos | 2 +- experimental/packages/otlp-proto-exporter-base/protos | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/experimental/packages/otlp-grpc-exporter-base/protos b/experimental/packages/otlp-grpc-exporter-base/protos index cc4ed55c08..59c488bfb8 160000 --- a/experimental/packages/otlp-grpc-exporter-base/protos +++ b/experimental/packages/otlp-grpc-exporter-base/protos @@ -1 +1 @@ -Subproject commit cc4ed55c082cb75e084d40b4ddf3805eda099f97 +Subproject commit 59c488bfb8fb6d0458ad6425758b70259ff4a2bd diff --git a/experimental/packages/otlp-proto-exporter-base/protos b/experimental/packages/otlp-proto-exporter-base/protos index cc4ed55c08..59c488bfb8 160000 --- a/experimental/packages/otlp-proto-exporter-base/protos +++ b/experimental/packages/otlp-proto-exporter-base/protos @@ -1 +1 @@ -Subproject commit cc4ed55c082cb75e084d40b4ddf3805eda099f97 +Subproject commit 59c488bfb8fb6d0458ad6425758b70259ff4a2bd From eff2b39e5c8a4e5fb70d58044f1670077644665a Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Tue, 26 Apr 2022 12:11:56 +0300 Subject: [PATCH 5/7] refactor: rename preferredAggregationTemporality to temporalityPreference --- .../test/OTLPMetricExporter.test.ts | 10 +++++----- .../src/OTLPMetricExporterBase.ts | 6 +++--- .../src/OTLPMetricExporterOptions.ts | 4 ++-- .../test/browser/CollectorMetricExporter.test.ts | 10 +++++----- .../test/node/CollectorMetricExporter.test.ts | 4 ++-- .../test/OTLPMetricExporter.test.ts | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts index 9d6ce26de2..f93134087f 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts @@ -128,7 +128,7 @@ const testOTLPMetricExporter = (params: TestParams) => url: 'grpcs://' + address, credentials, metadata: params.metadata, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); setUp(); @@ -182,7 +182,7 @@ const testOTLPMetricExporter = (params: TestParams) => headers: { foo: 'bar', }, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); const args = warnStub.args[0]; assert.strictEqual(args[0], 'Headers cannot be set when using grpc'); @@ -190,7 +190,7 @@ const testOTLPMetricExporter = (params: TestParams) => it('should warn about path in url', () => { collectorExporter = new OTLPMetricExporter({ url: `http://${address}/v1/metrics`, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); const args = warnStub.args[0]; assert.strictEqual( @@ -264,7 +264,7 @@ describe('OTLPMetricExporter - node (getDefaultUrl)', () => { const url = 'http://foo.bar.com'; const collectorExporter = new OTLPMetricExporter({ url, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); setTimeout(() => { assert.strictEqual(collectorExporter._otlpExporter.url, 'foo.bar.com'); @@ -309,7 +309,7 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const collectorExporter = new OTLPMetricExporter({ metadata, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); assert.deepStrictEqual(collectorExporter._otlpExporter.metadata?.get('foo'), ['boo']); assert.deepStrictEqual(collectorExporter._otlpExporter.metadata?.get('bar'), ['foo']); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts index 518a054c8e..1fa040e6a6 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts @@ -41,8 +41,8 @@ export const DeltaTemporalitySelector: AggregationTemporalitySelector = (instrum } }; -function chooseTemporalitySelector(preferredAggregationTemporality?: AggregationTemporality): AggregationTemporalitySelector { - if (preferredAggregationTemporality === AggregationTemporality.DELTA) { +function chooseTemporalitySelector(temporalityPreference?: AggregationTemporality): AggregationTemporalitySelector { + if (temporalityPreference === AggregationTemporality.DELTA) { return DeltaTemporalitySelector; } @@ -59,7 +59,7 @@ implements PushMetricExporter { constructor(exporter: T, config: OTLPMetricExporterOptions = defaultOptions) { this._otlpExporter = exporter; - this._aggregationTemporalitySelector = chooseTemporalitySelector(config.preferredAggregationTemporality); + this._aggregationTemporalitySelector = chooseTemporalitySelector(config.temporalityPreference); } export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void): void { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts index 5636f2f273..a9f601992f 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts @@ -18,7 +18,7 @@ import { AggregationTemporality } from '@opentelemetry/sdk-metrics-base'; import { OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base'; export interface OTLPMetricExporterOptions extends OTLPExporterConfigBase { - preferredAggregationTemporality?: AggregationTemporality + temporalityPreference?: AggregationTemporality } export const defaultExporterTemporality = AggregationTemporality.CUMULATIVE; -export const defaultOptions = {preferredAggregationTemporality: defaultExporterTemporality}; +export const defaultOptions = {temporalityPreference: defaultExporterTemporality}; diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts index 0f95ebfba8..b57fabff5a 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts @@ -94,7 +94,7 @@ describe('OTLPMetricExporter - web', () => { beforeEach(() => { collectorExporter = new OTLPMetricExporter({ url: 'http://foo.bar.com', - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); }); it('should successfully send metrics using sendBeacon', done => { @@ -202,7 +202,7 @@ describe('OTLPMetricExporter - web', () => { (window.navigator as any).sendBeacon = false; collectorExporter = new OTLPMetricExporter({ url: 'http://foo.bar.com', - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); // Overwrites the start time to make tests consistent Object.defineProperty(collectorExporter, '_startTime', { @@ -338,7 +338,7 @@ describe('OTLPMetricExporter - web', () => { beforeEach(() => { collectorExporterConfig = { headers: customHeaders, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }; server = sinon.fakeServer.create(); }); @@ -430,7 +430,7 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const collectorExporter = new OTLPMetricExporter({ headers: {}, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].foo, 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; @@ -440,7 +440,7 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const collectorExporter = new OTLPMetricExporter({ headers: {}, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].foo, 'boo'); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].bar, 'foo'); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index 1187f34204..70e0f5a447 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -163,7 +163,7 @@ describe('OTLPMetricExporter - node with json over http', () => { url: 'http://foo.bar.com', keepAlive: true, httpAgentOptions: { keepAliveMsecs: 2000 }, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }; collectorExporter = new OTLPMetricExporter(collectorExporterConfig); @@ -334,7 +334,7 @@ describe('OTLPMetricExporter - node with json over http', () => { const url = 'http://foo.bar.com'; const collectorExporter = new OTLPMetricExporter({ url, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }); setTimeout(() => { assert.strictEqual(collectorExporter._otlpExporter.url, url); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index 2148319a88..c146340ec1 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -112,7 +112,7 @@ describe('OTLPMetricExporter - node with proto over http', () => { url: 'http://foo.bar.com', keepAlive: true, httpAgentOptions: { keepAliveMsecs: 2000 }, - preferredAggregationTemporality: AggregationTemporality.CUMULATIVE + temporalityPreference: AggregationTemporality.CUMULATIVE }; collectorExporter = new OTLPMetricExporter(collectorExporterConfig); setUp(); From c9cfb82952b0d45923eeaa58783e48bdd3eda394 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Tue, 26 Apr 2022 12:51:06 +0300 Subject: [PATCH 6/7] doc: better changelog wording --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 114cc1c132..cf26fe5f20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file. ### :boom: Breaking Change -* feat: metric aggregation temporality controls [#2902](https://github.com/open-telemetry/opentelemetry-js/pull/2902) @seemk +* feat(metrics): metric readers and exporters now select aggregation temporality based on instrument type [#2902](https://github.com/open-telemetry/opentelemetry-js/pull/2902) @seemk ### :rocket: (Enhancement) From a4b1e72e39b043624805d8b88a4ceecfcc53b9b8 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Tue, 10 May 2022 12:25:10 +0300 Subject: [PATCH 7/7] fix: remove unnecessary files --- .../src/transformMetrics.ts | 225 ------------------ .../test/common/transformMetrics.test.ts | 182 -------------- 2 files changed, 407 deletions(-) delete mode 100644 experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/transformMetrics.ts delete mode 100644 experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/transformMetrics.test.ts diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/transformMetrics.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/transformMetrics.ts deleted file mode 100644 index 1cf0f0f1cb..0000000000 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/transformMetrics.ts +++ /dev/null @@ -1,225 +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 { SpanAttributes } from '@opentelemetry/api'; -import { ValueType } from '@opentelemetry/api-metrics'; -import * as core from '@opentelemetry/core'; -import { otlpTypes, toCollectorResource } from '@opentelemetry/exporter-trace-otlp-http'; -import { OTLPExporterBase, OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base'; -import { - AggregationTemporality, - DataPointType, - Histogram, - InstrumentType, - MetricData, - ResourceMetrics -} from '@opentelemetry/sdk-metrics-base'; - -/** - * Converts {@link SpanAttributes} to a collector-compatible format. - * @param attributes - */ -export function toCollectorAttributes( - attributes: SpanAttributes -): otlpTypes.opentelemetryProto.common.v1.StringKeyValue[] { - return Object.entries(attributes).map(([key, value]) => { - return { key, value: String(value) }; - }); -} - -/** - * Convert {@link AggregationTemporality} to a collector-compatible format. - * @param aggregationTemporality - */ -export function toAggregationTemporality( - aggregationTemporality: AggregationTemporality -): otlpTypes.opentelemetryProto.metrics.v1.AggregationTemporality { - if (aggregationTemporality === AggregationTemporality.CUMULATIVE) { - return otlpTypes.opentelemetryProto.metrics.v1.AggregationTemporality - .AGGREGATION_TEMPORALITY_CUMULATIVE; - } - if (aggregationTemporality === AggregationTemporality.DELTA) { - return otlpTypes.opentelemetryProto.metrics.v1.AggregationTemporality - .AGGREGATION_TEMPORALITY_DELTA; - } - - return otlpTypes.opentelemetryProto.metrics.v1.AggregationTemporality - .AGGREGATION_TEMPORALITY_UNSPECIFIED; -} - -/** - * Convert {@link MetricData} of {@link DataPointType.SINGULAR} to a collector-compatible format. - * @param metricData - */ -export function toSingularDataPoints( - metricData: MetricData -): otlpTypes.opentelemetryProto.metrics.v1.DataPoint[] { - return Array.from(metricData.dataPoints.map(dataPoint => { - return { - labels: toCollectorAttributes(dataPoint.attributes), - value: dataPoint.value as number, - startTimeUnixNano: core.hrTimeToNanoseconds( - dataPoint.startTime - ), - timeUnixNano: core.hrTimeToNanoseconds( - dataPoint.endTime - ), - }; - })); -} - -/** - * Convert {@link MetricData} of {@link DataPointType.HISTOGRAM} to a collector-compatible format. - * @param metricData - */ -export function toHistogramDataPoints( - metricData: MetricData -): otlpTypes.opentelemetryProto.metrics.v1.HistogramDataPoint[] { - return Array.from(metricData.dataPoints.map(dataPoints => { - const histogram = dataPoints.value as Histogram; - return { - labels: toCollectorAttributes(dataPoints.attributes), - sum: histogram.sum, - count: histogram.count, - startTimeUnixNano: core.hrTimeToNanoseconds( - dataPoints.startTime - ), - timeUnixNano: core.hrTimeToNanoseconds( - dataPoints.endTime - ), - bucketCounts: histogram.buckets.counts, - explicitBounds: histogram.buckets.boundaries, - }; - })); -} - -/** - * Converts {@link MetricData} to a collector-compatible format. - * @param metricData - * @param aggregationTemporality - */ -export function toCollectorMetric( - metricData: MetricData, -): otlpTypes.opentelemetryProto.metrics.v1.Metric { - const metricCollector: otlpTypes.opentelemetryProto.metrics.v1.Metric = { - name: metricData.descriptor.name, - description: metricData.descriptor.description, - unit: metricData.descriptor.unit, - }; - if (metricData.dataPointType === DataPointType.SINGULAR) { - const result = { - dataPoints: toSingularDataPoints(metricData), - isMonotonic: - metricData.descriptor.type === InstrumentType.COUNTER || - metricData.descriptor.type === InstrumentType.OBSERVABLE_COUNTER, - aggregationTemporality: toAggregationTemporality(metricData.aggregationTemporality), - }; - - if ( - metricData.descriptor.type === InstrumentType.COUNTER || - metricData.descriptor.type === InstrumentType.OBSERVABLE_COUNTER || - metricData.descriptor.type === InstrumentType.UP_DOWN_COUNTER || - metricData.descriptor.type === InstrumentType.OBSERVABLE_UP_DOWN_COUNTER - ) { - if (metricData.descriptor.valueType === ValueType.INT) { - metricCollector.intSum = result; - } else { - metricCollector.doubleSum = result; - } - } else{ - // Instrument is a gauge. - if (metricData.descriptor.valueType === ValueType.INT) { - metricCollector.intGauge = result; - } else { - metricCollector.doubleGauge = result; - } - } - } else if (metricData.dataPointType === DataPointType.HISTOGRAM) { - const result = { - dataPoints: toHistogramDataPoints(metricData), - aggregationTemporality: toAggregationTemporality(metricData.aggregationTemporality) - }; - if (metricData.descriptor.valueType === ValueType.INT) { - metricCollector.intHistogram = result; - } else { - metricCollector.doubleHistogram = result; - } - } - - // TODO: Add support for exponential histograms when they're ready. - - return metricCollector; -} - -/** - * Prepares metric service request to be sent to collector - * @param metrics metrics - * @param aggregationTemporality - * @param collectorExporterBase - */ -export function toOTLPExportMetricServiceRequest( - metrics: ResourceMetrics, - collectorExporterBase: OTLPExporterBase -): otlpTypes.opentelemetryProto.collector.metrics.v1.ExportMetricsServiceRequest { - const additionalAttributes = Object.assign( - {}, - collectorExporterBase.attributes - ); - return { - resourceMetrics: toCollectorResourceMetrics( - metrics, - additionalAttributes, - ), - }; -} - -/** - * Convert to InstrumentationLibraryMetrics - * @param instrumentationLibrary - * @param metrics - * @param temporalitySelector - */ -function toCollectorInstrumentationLibraryMetrics( - instrumentationLibrary: core.InstrumentationLibrary, - metrics: MetricData[], -): otlpTypes.opentelemetryProto.metrics.v1.InstrumentationLibraryMetrics { - return { - metrics: metrics.map(metric => toCollectorMetric(metric)), - instrumentationLibrary, - }; -} - -/** - * Returns a list of resource metrics which will be exported to the collector - * @param resourceMetrics - * @param baseAttributes - * @param aggregationTemporality - */ -function toCollectorResourceMetrics( - resourceMetrics: ResourceMetrics, - baseAttributes: SpanAttributes, -): otlpTypes.opentelemetryProto.metrics.v1.ResourceMetrics[] { - return [{ - resource: toCollectorResource(resourceMetrics.resource, baseAttributes), - instrumentationLibraryMetrics: Array.from(resourceMetrics.instrumentationLibraryMetrics.map( - instrumentationLibraryMetrics => toCollectorInstrumentationLibraryMetrics( - instrumentationLibraryMetrics.instrumentationLibrary, - instrumentationLibraryMetrics.metrics, - ))) - }]; -} diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/transformMetrics.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/transformMetrics.test.ts deleted file mode 100644 index 4b8fec5e77..0000000000 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/transformMetrics.test.ts +++ /dev/null @@ -1,182 +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 { hrTimeToNanoseconds, hrTime } from '@opentelemetry/core'; -import { AggregationTemporality, DataPointType, InstrumentType } from '@opentelemetry/sdk-metrics-base'; -import * as assert from 'assert'; -import * as transform from '../../src/transformMetrics'; -import { - collect, - ensureCounterIsCorrect, - ensureDoubleCounterIsCorrect, - ensureHistogramIsCorrect, - ensureObservableCounterIsCorrect, - ensureObservableGaugeIsCorrect, - ensureObservableUpDownCounterIsCorrect, - mockCounter, - mockDoubleCounter, - mockHistogram, - mockObservableCounter, - mockObservableGauge, - mockObservableUpDownCounter, - setUp, -} from '../metricsHelper'; - -describe('transformMetrics', () => { - describe('toCollectorMetric', async () => { - - function getValue(count: number) { - if (count % 2 === 0) { - return 3; - } - return -1; - } - - beforeEach(() => { - setUp(); - }); - - it('should convert counter', async () => { - const counter = mockCounter(); - counter.add(1); - const metrics = (await collect()); - - const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; - ensureCounterIsCorrect( - transform.toCollectorMetric(metric), - hrTimeToNanoseconds(metric.dataPoints[0].endTime), - hrTimeToNanoseconds(metric.dataPoints[0].startTime) - ); - }); - - it('should convert double counter', async () => { - const doubleCounter = mockDoubleCounter(); - doubleCounter.add(8); - const metrics = (await collect()); - - const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; - ensureDoubleCounterIsCorrect( - transform.toCollectorMetric(metric), - hrTimeToNanoseconds(metric.dataPoints[0].endTime), - hrTimeToNanoseconds(metric.dataPoints[0].startTime), - ); - }); - - it('should convert observable gauge', async () => { - let count = 0; - mockObservableGauge(observableResult => { - count++; - observableResult.observe(getValue(count), {}); - }); - - // collect three times. - await collect(); - await collect(); - const metrics = (await collect()); - - const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; - ensureObservableGaugeIsCorrect( - transform.toCollectorMetric(metric), - hrTimeToNanoseconds(metric.dataPoints[0].endTime), - hrTimeToNanoseconds(metric.dataPoints[0].startTime), - -1, - ); - }); - - it('should convert observable counter', async () => { - mockObservableCounter(observableResult => { - observableResult.observe(1, {}); - }); - - // collect three times. - await collect(); - await collect(); - const metrics = (await collect()); - // TODO: Collect seems to not deliver the last observation -> why? - - const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; - ensureObservableCounterIsCorrect( - transform.toCollectorMetric(metric), - hrTimeToNanoseconds(metric.dataPoints[0].endTime), - hrTimeToNanoseconds(metric.dataPoints[0].startTime), - 2, - ); - }); - - it('should convert observable up-down counter', async () => { - mockObservableUpDownCounter(observableResult => { - observableResult.observe(1, {}); - }); - - // collect three times. - await collect(); - await collect(); - const metrics = (await collect()); - // TODO: Collect seems to not deliver the last observation -> why? - - const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; - ensureObservableUpDownCounterIsCorrect( - transform.toCollectorMetric(metric), - hrTimeToNanoseconds(metric.dataPoints[0].endTime), - hrTimeToNanoseconds(metric.dataPoints[0].startTime), - 2, - ); - }); - - it('should convert observable histogram', async () => { - const histogram = mockHistogram(); - histogram.record(7); - histogram.record(14); - - const metrics = (await collect()); - - const metric = metrics.instrumentationLibraryMetrics[0].metrics[0]; - ensureHistogramIsCorrect( - transform.toCollectorMetric(metric), - hrTimeToNanoseconds(metric.dataPoints[0].endTime), - hrTimeToNanoseconds(metric.dataPoints[0].startTime), - [0, 100], - [0, 2, 0] - ); - }); - - it('should convert metric attributes value to string', () => { - const metric = transform.toCollectorMetric( - { - descriptor: { - name: 'name', - description: 'description', - unit: 'unit', - type: InstrumentType.COUNTER, - valueType: 0, - }, - aggregationTemporality: AggregationTemporality.CUMULATIVE, - dataPoints: [ - { - value: 1, - attributes: { foo: (1 as unknown) as string }, - startTime: hrTime(), - endTime: hrTime(), - } - ], - dataPointType: DataPointType.SINGULAR, - }, - ); - const collectorMetric = metric.intSum?.dataPoints[0]; - assert.strictEqual(collectorMetric?.labels[0].value, '1'); - }); - }); -});