From aace56961199b00accdb560490db7ab4bc023daa Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Mon, 28 Feb 2022 22:10:29 +0200 Subject: [PATCH 1/3] refactor: match metrics export model with proto --- .../src/Meter.ts | 17 ++- .../src/aggregator/Drop.ts | 4 - .../src/aggregator/Histogram.ts | 6 - .../src/aggregator/LastValue.ts | 7 +- .../src/aggregator/Sum.ts | 6 - .../src/aggregator/types.ts | 6 +- .../src/export/MetricData.ts | 93 +++++++++++--- .../src/export/MetricExporter.ts | 6 +- .../src/export/MetricProducer.ts | 4 +- .../src/export/MetricReader.ts | 6 +- .../src/state/AsyncMetricStorage.ts | 6 - .../src/state/MetricCollector.ts | 6 +- .../src/state/MetricStorage.ts | 4 - .../src/state/SyncMetricStorage.ts | 6 - .../src/state/TemporalMetricProcessor.ts | 6 - .../test/Instruments.test.ts | 13 +- .../test/aggregator/Drop.test.ts | 4 +- .../test/aggregator/Histogram.test.ts | 6 +- .../test/aggregator/LastValue.test.ts | 6 +- .../test/aggregator/Sum.test.ts | 6 +- .../test/export/MetricData.test.ts | 118 ++++++++++++++++++ .../PeriodicExportingMetricReader.test.ts | 20 +-- .../test/state/AsyncMetricStorage.test.ts | 14 +-- .../test/state/MetricCollector.test.ts | 22 ++-- .../test/state/SyncMetricStorage.test.ts | 14 +-- .../state/TemporalMetricProcessor.test.ts | 22 +--- .../test/util.ts | 8 -- 27 files changed, 256 insertions(+), 180 deletions(-) create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricData.test.ts diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts index cc4c54545c..d6ab0106c9 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts @@ -22,7 +22,7 @@ import { MeterProviderSharedState } from './state/MeterProviderSharedState'; import { MultiMetricStorage } from './state/MultiWritableMetricStorage'; import { SyncMetricStorage } from './state/SyncMetricStorage'; import { MetricStorage } from './state/MetricStorage'; -import { MetricData } from './export/MetricData'; +import { MetricsData } from './export/MetricData'; import { isNotNullish } from './utils'; import { MetricCollectorHandle } from './state/MetricCollector'; import { HrTime } from '@opentelemetry/api'; @@ -130,16 +130,21 @@ export class Meter implements metrics.Meter { * @param collectionTime the HrTime at which the collection was initiated. * @returns the list of {@link MetricData} collected. */ - async collect(collector: MetricCollectorHandle, collectionTime: HrTime): Promise { - const result = await Promise.all(Array.from(this._metricStorageRegistry.values()).map(metricStorage => { + async collect(collector: MetricCollectorHandle, collectionTime: HrTime): Promise { + const metricData = await Promise.all(Array.from(this._metricStorageRegistry.values()).map(metricStorage => { return metricStorage.collect( collector, this._meterProviderSharedState.metricCollectors, - this._meterProviderSharedState.resource, - this._instrumentationLibrary, this._meterProviderSharedState.sdkStartTime, collectionTime); })); - return result.filter(isNotNullish); + + return new MetricsData({ + resource: this._meterProviderSharedState.resource, + instrumentationLibraryMetrics: [{ + instrumentationLibrary: this._instrumentationLibrary, + metrics: metricData.filter(isNotNullish), + }] + }); } } 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 54ad0ce7ed..ba244b415f 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Drop.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Drop.ts @@ -15,8 +15,6 @@ */ import { HrTime } from '@opentelemetry/api'; -import { InstrumentationLibrary } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; import { MetricData } from '../export/MetricData'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; @@ -43,8 +41,6 @@ export class DropAggregator implements Aggregator { } toMetricData( - _resource: Resource, - _instrumentationLibrary: InstrumentationLibrary, _instrumentDescriptor: InstrumentDescriptor, _accumulationByAttributes: AccumulationRecord[], _startTime: HrTime, 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 4caa575c3c..fafc404009 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Histogram.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Histogram.ts @@ -22,8 +22,6 @@ import { Histogram, } from './types'; import { HistogramMetricData, PointDataType } from '../export/MetricData'; -import { Resource } from '@opentelemetry/resources'; -import { InstrumentationLibrary } from '@opentelemetry/core'; import { HrTime } from '@opentelemetry/api'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; @@ -138,15 +136,11 @@ export class HistogramAggregator implements Aggregator { } toMetricData( - resource: Resource, - instrumentationLibrary: InstrumentationLibrary, metricDescriptor: InstrumentDescriptor, accumulationByAttributes: AccumulationRecord[], startTime: HrTime, endTime: HrTime): Maybe { return { - resource, - instrumentationLibrary, instrumentDescriptor: metricDescriptor, pointDataType: PointDataType.HISTOGRAM, pointData: accumulationByAttributes.map(([attributes, accumulation]) => { 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 ddfc5a024b..ea581eb7db 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/LastValue.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/LastValue.ts @@ -16,8 +16,7 @@ import { LastValue, AggregatorKind, Aggregator, Accumulation, AccumulationRecord } from './types'; import { HrTime } from '@opentelemetry/api'; -import { hrTime, hrTimeToMicroseconds, InstrumentationLibrary } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; +import { hrTime, hrTimeToMicroseconds } from '@opentelemetry/core'; import { PointDataType, SingularMetricData } from '../export/MetricData'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; @@ -67,15 +66,11 @@ export class LastValueAggregator implements Aggregator { } toMetricData( - resource: Resource, - instrumentationLibrary: InstrumentationLibrary, instrumentDescriptor: InstrumentDescriptor, accumulationByAttributes: AccumulationRecord[], startTime: HrTime, endTime: HrTime): Maybe { return { - resource, - instrumentationLibrary, instrumentDescriptor, pointDataType: PointDataType.SINGULAR, pointData: accumulationByAttributes.map(([attributes, accumulation]) => { 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 5a9aaac8d1..eec26e4724 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Sum.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Sum.ts @@ -16,8 +16,6 @@ import { Sum, AggregatorKind, Aggregator, Accumulation, AccumulationRecord } from './types'; import { HrTime } from '@opentelemetry/api'; -import { InstrumentationLibrary } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; import { PointDataType, SingularMetricData } from '../export/MetricData'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; @@ -57,15 +55,11 @@ export class SumAggregator implements Aggregator { } toMetricData( - resource: Resource, - instrumentationLibrary: InstrumentationLibrary, instrumentDescriptor: InstrumentDescriptor, accumulationByAttributes: AccumulationRecord[], startTime: HrTime, endTime: HrTime): Maybe { return { - resource, - instrumentationLibrary, instrumentDescriptor, pointDataType: PointDataType.SINGULAR, pointData: accumulationByAttributes.map(([attributes, accumulation]) => { 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 28d20a634e..2f49ef2996 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/types.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/types.ts @@ -16,8 +16,6 @@ import { HrTime } from '@opentelemetry/api'; import { Attributes } from '@opentelemetry/api-metrics'; -import { InstrumentationLibrary } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; import { MetricData } from '../export/MetricData'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; @@ -116,9 +114,7 @@ export interface Aggregator { * @param endTime the end time of the metric data. * @return the {@link MetricData} that this {@link Aggregator} will produce. */ - toMetricData(resource: Resource, - instrumentationLibrary: InstrumentationLibrary, - instrumentDescriptor: InstrumentDescriptor, + toMetricData(instrumentDescriptor: InstrumentDescriptor, accumulationByAttributes: AccumulationRecord[], startTime: HrTime, endTime: HrTime): Maybe; 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 736c83034a..ccf30ba108 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts @@ -25,22 +25,11 @@ import { Histogram } from '../aggregator/types'; * Basic metric data fields. */ export interface BaseMetricData { - /** - * Resource associated with metric telemetry. - */ - readonly resource: Resource; - /** - * InstrumentationLibrary which created the metric instrument. - */ - readonly instrumentationLibrary: InstrumentationLibrary; - /** - * InstrumentDescriptor which describes the metric instrument. - */ readonly instrumentDescriptor: InstrumentDescriptor; /** * PointDataType of the metric instrument. */ - readonly pointDataType: PointDataType, + readonly pointDataType: PointDataType; } /** @@ -48,16 +37,16 @@ export interface BaseMetricData { * SumAggregation. */ export interface SingularMetricData extends BaseMetricData { - readonly pointDataType: PointDataType.SINGULAR, - readonly pointData: PointData[], + readonly pointDataType: PointDataType.SINGULAR; + readonly pointData: PointData[]; } /** * Represents a metric data aggregated by a HistogramAggregation. */ export interface HistogramMetricData extends BaseMetricData { - readonly pointDataType: PointDataType.HISTOGRAM, - readonly pointData: PointData[], + readonly pointDataType: PointDataType.HISTOGRAM; + readonly pointData: PointData[]; } /** @@ -65,6 +54,78 @@ export interface HistogramMetricData extends BaseMetricData { */ export type MetricData = SingularMetricData | HistogramMetricData; +export interface InstrumentationLibraryMetrics { + instrumentationLibrary: InstrumentationLibrary; + metrics: MetricData[]; +} + +export interface ResourceMetrics { + resource: Resource; + instrumentationLibraryMetrics: InstrumentationLibraryMetrics[]; +} + +type InstrumentationLibraryMetricsMap = Map; + +export class MetricsData { + private _metricsData: Map = new Map(); + + constructor(resourceMetrics?: ResourceMetrics) { + if (resourceMetrics === undefined) { + return; + } + + const metricsByInstrLib = new Map(); + + for (const ilMetrics of resourceMetrics.instrumentationLibraryMetrics) { + metricsByInstrLib.set(ilMetrics.instrumentationLibrary, ilMetrics.metrics); + } + + this._metricsData.set(resourceMetrics.resource, metricsByInstrLib); + } + + resourceMetrics(): ResourceMetrics[] { + return Array.from(this._metricsData, ([resource, metricsByInstrLib]) => { + const instrumentationLibraryMetrics = Array.from(metricsByInstrLib, ([instrumentationLibrary, metrics]) => ({ + instrumentationLibrary, + metrics + })); + + return { + resource, + instrumentationLibraryMetrics, + }; + }); + } + + merge(source: MetricsData): MetricsData { + function mergeIlMetrics(into: InstrumentationLibraryMetricsMap, from: InstrumentationLibraryMetricsMap): InstrumentationLibraryMetricsMap { + from.forEach((metrics, instrumentationLib) => { + const targetMetrics = into.get(instrumentationLib); + + if (targetMetrics === undefined) { + into.set(instrumentationLib, metrics); + } else { + into.set(instrumentationLib, targetMetrics.concat(metrics)); + } + }); + + return into; + } + + source._metricsData.forEach((sourceIlMetrics, resource) => { + const targetIlMetrics = this._metricsData.get(resource); + + if (targetIlMetrics === undefined) { + this._metricsData.set(resource, sourceIlMetrics); + } else { + this._metricsData.set(resource, mergeIlMetrics(targetIlMetrics, sourceIlMetrics)); + } + }); + + return this; + } +} + /** * The aggregated point data type. */ 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 697f020b28..c8d62070ec 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts @@ -15,7 +15,7 @@ */ import { AggregationTemporality } from './AggregationTemporality'; -import { MetricData } from './MetricData'; +import { MetricsData } from './MetricData'; // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricexporter @@ -24,7 +24,7 @@ import { MetricData } from './MetricData'; export abstract class MetricExporter { protected _shutdown = false; - abstract export(batch: MetricData[]): Promise; + abstract export(batch: MetricsData): Promise; abstract forceFlush(): Promise; @@ -48,7 +48,7 @@ export abstract class MetricExporter { } export class ConsoleMetricExporter extends MetricExporter { - async export(_batch: MetricData[]) { + async export(_batch: MetricsData) { throw new Error('Method not implemented'); } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricProducer.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricProducer.ts index 7afb9eb4ea..347e8a2d62 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricProducer.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricProducer.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { MetricData } from './MetricData'; +import { MetricsData } from './MetricData'; /** * This is a public interface that represent an export state of a MetricReader. */ export interface MetricProducer { - collect(): Promise; + collect(): Promise; } 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 db0bc301b7..43f9a8309c 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts @@ -17,7 +17,7 @@ import * as api from '@opentelemetry/api'; import { AggregationTemporality } from './AggregationTemporality'; import { MetricProducer } from './MetricProducer'; -import { MetricData } from './MetricData'; +import { MetricsData } from './MetricData'; import { callWithTimeout } from '../utils'; export type ReaderOptions = { @@ -91,7 +91,7 @@ export abstract class MetricReader { /** * Collect all metrics from the associated {@link MetricProducer} */ - async collect(options?: ReaderCollectionOptions): Promise { + async collect(options?: ReaderCollectionOptions): Promise { if (this._metricProducer === undefined) { throw new Error('MetricReader is not bound to a MetricProducer'); } @@ -99,7 +99,7 @@ export abstract class MetricReader { // Subsequent invocations to collect are not allowed. SDKs SHOULD return some failure for these calls. if (this._shutdown) { api.diag.warn('Collection is not allowed after shutdown'); - return []; + return new MetricsData(); } // No timeout if timeoutMillis is undefined or null. diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/AsyncMetricStorage.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/AsyncMetricStorage.ts index d98ececf7f..07cf13fcc3 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/AsyncMetricStorage.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/AsyncMetricStorage.ts @@ -21,8 +21,6 @@ import { View } from '../view/View'; import { createInstrumentDescriptorWithView, InstrumentDescriptor } from '../InstrumentDescriptor'; import { AttributesProcessor } from '../view/AttributesProcessor'; import { MetricStorage } from './MetricStorage'; -import { InstrumentationLibrary } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; import { MetricData } from '../export/MetricData'; import { DeltaMetricProcessor } from './DeltaMetricProcessor'; import { TemporalMetricProcessor } from './TemporalMetricProcessor'; @@ -68,8 +66,6 @@ export class AsyncMetricStorage> implements Metric async collect( collector: MetricCollectorHandle, collectors: MetricCollectorHandle[], - resource: Resource, - instrumentationLibrary: InstrumentationLibrary, sdkStartTime: HrTime, collectionTime: HrTime, ): Promise> { @@ -83,8 +79,6 @@ export class AsyncMetricStorage> implements Metric return this._temporalMetricStorage.buildMetrics( collector, collectors, - resource, - instrumentationLibrary, this._instrumentDescriptor, accumulations, sdkStartTime, 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 46fc576f50..52adaac428 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts @@ -16,7 +16,7 @@ import { hrTime } from '@opentelemetry/core'; import { AggregationTemporality } from '../export/AggregationTemporality'; -import { MetricData } from '../export/MetricData'; +import { MetricsData } from '../export/MetricData'; import { MetricProducer } from '../export/MetricProducer'; import { MetricReader } from '../export/MetricReader'; import { MeterProviderSharedState } from './MeterProviderSharedState'; @@ -32,12 +32,12 @@ export class MetricCollector implements MetricProducer { this.aggregatorTemporality = this._metricReader.getPreferredAggregationTemporality(); } - async collect(): Promise { + async collect(): Promise { const collectionTime = hrTime(); const results = await Promise.all(this._sharedState.meters .map(meter => meter.collect(this, collectionTime))); - return results.reduce((cumulation, current) => cumulation.concat(current), []); + return results.reduce((cumulation, current) => cumulation.merge(current), new MetricsData()); } /** diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorage.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorage.ts index 7369800bd0..820c6a5a35 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorage.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorage.ts @@ -15,8 +15,6 @@ */ import { HrTime } from '@opentelemetry/api'; -import { InstrumentationLibrary } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; import { MetricData } from '../export/MetricData'; import { Maybe } from '../utils'; import { MetricCollectorHandle } from './MetricCollector'; @@ -36,8 +34,6 @@ export interface MetricStorage { collect( collector: MetricCollectorHandle, collectors: MetricCollectorHandle[], - resource: Resource, - instrumentationLibrary: InstrumentationLibrary, sdkStartTime: HrTime, collectionTime: HrTime, ): Promise>; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/SyncMetricStorage.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/SyncMetricStorage.ts index 609f0fc585..ddcebb85d8 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/SyncMetricStorage.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/SyncMetricStorage.ts @@ -22,8 +22,6 @@ import { View } from '../view/View'; import { createInstrumentDescriptorWithView, InstrumentDescriptor } from '../InstrumentDescriptor'; import { AttributesProcessor } from '../view/AttributesProcessor'; import { MetricStorage } from './MetricStorage'; -import { InstrumentationLibrary } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; import { MetricData } from '../export/MetricData'; import { DeltaMetricProcessor } from './DeltaMetricProcessor'; import { TemporalMetricProcessor } from './TemporalMetricProcessor'; @@ -62,8 +60,6 @@ export class SyncMetricStorage> implements Writabl async collect( collector: MetricCollectorHandle, collectors: MetricCollectorHandle[], - resource: Resource, - instrumentationLibrary: InstrumentationLibrary, sdkStartTime: HrTime, collectionTime: HrTime, ): Promise> { @@ -72,8 +68,6 @@ export class SyncMetricStorage> implements Writabl return this._temporalMetricStorage.buildMetrics( collector, collectors, - resource, - instrumentationLibrary, this._instrumentDescriptor, accumulations, sdkStartTime, 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 9c4936de39..c916523a6a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/TemporalMetricProcessor.ts @@ -17,8 +17,6 @@ import { HrTime } from '@opentelemetry/api'; import { AccumulationRecord, Aggregator } from '../aggregator/types'; import { MetricData } from '../export/MetricData'; -import { Resource } from '@opentelemetry/resources'; -import { InstrumentationLibrary } from '@opentelemetry/core'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { AggregationTemporality } from '../export/AggregationTemporality'; import { Maybe } from '../utils'; @@ -66,8 +64,6 @@ export class TemporalMetricProcessor { buildMetrics( collector: MetricCollectorHandle, collectors: MetricCollectorHandle[], - resource: Resource, - instrumentationLibrary: InstrumentationLibrary, instrumentDescriptor: InstrumentDescriptor, currentAccumulations: AttributeHashMap, sdkStartTime: HrTime, @@ -111,8 +107,6 @@ export class TemporalMetricProcessor { // 1. Cumulative Aggregation time span: (sdkStartTime, collectionTime] // 2. Delta Aggregation time span: (lastCollectionTime, collectionTime] return this._aggregator.toMetricData( - resource, - instrumentationLibrary, instrumentDescriptor, AttributesMapToAccumulationRecords(result), /* startTime */ aggregationTemporality === AggregationTemporality.CUMULATIVE ? sdkStartTime : lastCollectionTime, 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 97151c5e42..c454a638e9 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts @@ -543,15 +543,20 @@ interface ValidateMetricData { } async function validateExport(reader: MetricReader, expected: ValidateMetricData) { - const metricData = await reader.collect(); - const metric = metricData[0]; + const metricsData = await reader.collect(); + + const { resource, instrumentationLibraryMetrics } = metricsData.resourceMetrics()[0]; + const { instrumentationLibrary, metrics } = instrumentationLibraryMetrics[0]; + + const metric = metrics[0]; + + assert.deepStrictEqual(resource, defaultResource); + assert.deepStrictEqual(instrumentationLibrary, defaultInstrumentationLibrary); assertMetricData( metric, expected.pointDataType, expected.instrumentDescriptor ?? null, - expected.instrumentationLibrary, - expected.resource ); if (expected.pointData == null) { 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 4d12e38511..05242cc192 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 @@ -17,7 +17,7 @@ import { HrTime } from '@opentelemetry/api'; import * as assert from 'assert'; import { DropAggregator } from '../../src/aggregator'; -import { defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util'; +import { defaultInstrumentDescriptor } from '../util'; describe('DropAggregator', () => { describe('createAccumulation', () => { @@ -54,8 +54,6 @@ describe('DropAggregator', () => { const endTime: HrTime = [1, 1]; assert.strictEqual(aggregator.toMetricData( - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, [[{}, undefined]], startTime, 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 2af44dc804..d873a846dc 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 @@ -18,7 +18,7 @@ import { HrTime } from '@opentelemetry/api'; import * as assert from 'assert'; import { HistogramAccumulation, HistogramAggregator } from '../../src/aggregator'; import { MetricData, PointDataType } from '../../src/export/MetricData'; -import { commonValues, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util'; +import { commonValues, defaultInstrumentDescriptor } from '../util'; describe('HistogramAggregator', () => { describe('createAccumulation', () => { @@ -92,8 +92,6 @@ describe('HistogramAggregator', () => { const endTime: HrTime = [1, 1]; const expected: MetricData = { - resource: defaultResource, - instrumentationLibrary: defaultInstrumentationLibrary, instrumentDescriptor: defaultInstrumentDescriptor, pointDataType: PointDataType.HISTOGRAM, pointData: [ @@ -113,8 +111,6 @@ describe('HistogramAggregator', () => { ], }; assert.deepStrictEqual(aggregator.toMetricData( - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, [[{}, accumulation]], startTime, 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 5b7ec19e91..6362d65fa8 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 @@ -18,7 +18,7 @@ import { HrTime } from '@opentelemetry/api'; import * as assert from 'assert'; import { LastValueAccumulation, LastValueAggregator } from '../../src/aggregator'; import { MetricData, PointDataType } from '../../src/export/MetricData'; -import { commonValues, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource, sleep } from '../util'; +import { commonValues, defaultInstrumentDescriptor, sleep } from '../util'; describe('LastValueAggregator', () => { describe('createAccumulation', () => { @@ -100,8 +100,6 @@ describe('LastValueAggregator', () => { const endTime: HrTime = [1, 1]; const expected: MetricData = { - resource: defaultResource, - instrumentationLibrary: defaultInstrumentationLibrary, instrumentDescriptor: defaultInstrumentDescriptor, pointDataType: PointDataType.SINGULAR, pointData: [ @@ -114,8 +112,6 @@ describe('LastValueAggregator', () => { ], }; assert.deepStrictEqual(aggregator.toMetricData( - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, [[{}, accumulation]], startTime, 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 26c322ebdc..92ffec89a3 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 @@ -18,7 +18,7 @@ import { HrTime } from '@opentelemetry/api'; import * as assert from 'assert'; import { SumAccumulation, SumAggregator } from '../../src/aggregator'; import { MetricData, PointDataType } from '../../src/export/MetricData'; -import { commonValues, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util'; +import { commonValues, defaultInstrumentDescriptor } from '../util'; describe('SumAggregator', () => { describe('createAccumulation', () => { @@ -78,8 +78,6 @@ describe('SumAggregator', () => { const endTime: HrTime = [1, 1]; const expected: MetricData = { - resource: defaultResource, - instrumentationLibrary: defaultInstrumentationLibrary, instrumentDescriptor: defaultInstrumentDescriptor, pointDataType: PointDataType.SINGULAR, pointData: [ @@ -92,8 +90,6 @@ describe('SumAggregator', () => { ], }; assert.deepStrictEqual(aggregator.toMetricData( - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, [[{}, accumulation]], startTime, diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricData.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricData.test.ts new file mode 100644 index 0000000000..f0486d8279 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricData.test.ts @@ -0,0 +1,118 @@ +/* + * 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 { MetricData, MetricsData, PointDataType } from '../../src/export/MetricData'; +import { InstrumentType } from '../../src/InstrumentDescriptor'; +import { ValueType } from '@opentelemetry/api-metrics-wip'; +import * as assert from 'assert'; +import { defaultInstrumentationLibrary, defaultResource } from '../util'; +import { hrTime, InstrumentationLibrary } from '@opentelemetry/core'; + +const defaultInstrumentDescriptor = { + name: 'test', + description: 'abc', + unit: 's', + type: InstrumentType.COUNTER, + valueType: ValueType.DOUBLE, +}; + +const defaultMetrics: MetricData[] = [{ + instrumentDescriptor: defaultInstrumentDescriptor, + pointDataType: PointDataType.SINGULAR, + pointData: [ + { point: 0, attributes: {}, startTime: hrTime(), endTime: hrTime() } + ] +}]; + +describe('MetricsData', () => { + describe('merge', () => { + it('should produce empty metrics data when merging empty data', () => { + const merged = new MetricsData().merge(new MetricsData()); + assert.deepStrictEqual(new MetricsData(), merged); + }); + + it('should retain target values when merging with empty data', () => { + const target = new MetricsData({ + resource: defaultResource, + instrumentationLibraryMetrics: [{ + instrumentationLibrary: defaultInstrumentationLibrary, + metrics: defaultMetrics, + }] + }); + + const merged = target.merge(new MetricsData()); + assert.deepStrictEqual(target, merged); + }); + + it('should concatenate metrics list for matchin resource and instrumentation library', () => { + const target = new MetricsData({ + resource: defaultResource, + instrumentationLibraryMetrics: [{ + instrumentationLibrary: defaultInstrumentationLibrary, + metrics: defaultMetrics, + }] + }); + + const merged = target.merge(target); + + const expected = new MetricsData({ + resource: defaultResource, + instrumentationLibraryMetrics: [{ + instrumentationLibrary: defaultInstrumentationLibrary, + metrics: defaultMetrics.concat(defaultMetrics), + }] + }); + assert.deepStrictEqual(expected, merged); + }); + }); + + it('should add new instrumentation library for matching resource', () => { + const target = new MetricsData({ + resource: defaultResource, + instrumentationLibraryMetrics: [{ + instrumentationLibrary: defaultInstrumentationLibrary, + metrics: defaultMetrics, + }] + }); + + const instrumentationLibrary: InstrumentationLibrary = { + name: 'other', + version: '1.0.0', + schemaUrl: 'https://opentelemetry.io/schemas/1.7.0' + }; + + const merged = target.merge(new MetricsData({ + resource: defaultResource, + instrumentationLibraryMetrics: [{ + instrumentationLibrary, + metrics: defaultMetrics, + }] + })); + + const expected = new MetricsData({ + resource: defaultResource, + instrumentationLibraryMetrics: [{ + instrumentationLibrary: defaultInstrumentationLibrary, + metrics: defaultMetrics, + }, { + instrumentationLibrary, + metrics: defaultMetrics, + }] + }); + + assert.deepStrictEqual(expected, merged); + }); +}); 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 4950f1cadc..fe49f3e0b4 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 @@ -17,7 +17,7 @@ import { PeriodicExportingMetricReader } from '../../src/export/PeriodicExportingMetricReader'; import { AggregationTemporality } from '../../src/export/AggregationTemporality'; import { MetricExporter } from '../../src'; -import { MetricData } from '../../src/export/MetricData'; +import { MetricsData } from '../../src/export/MetricData'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { MetricProducer } from '../../src/export/MetricProducer'; @@ -30,9 +30,9 @@ class TestMetricExporter extends MetricExporter { public exportTime = 0; public forceFlushTime = 0; public throwException = false; - private _batches: MetricData[][] = []; + private _batches: MetricsData[] = []; - async export(batch: MetricData[]): Promise { + async export(batch: MetricsData): Promise { this._batches.push(batch); if (this.throwException) { @@ -49,7 +49,7 @@ class TestMetricExporter extends MetricExporter { await new Promise(resolve => setTimeout(resolve, this.forceFlushTime)); } - async waitForNumberOfExports(numberOfExports: number): Promise { + async waitForNumberOfExports(numberOfExports: number): Promise { if (numberOfExports <= 0) { throw new Error('numberOfExports must be greater than or equal to 0'); } @@ -74,9 +74,9 @@ class TestDeltaMetricExporter extends TestMetricExporter { class TestMetricProducer implements MetricProducer { public collectionTime = 0; - async collect(): Promise { + async collect(): Promise { await new Promise(resolve => setTimeout(resolve, this.collectionTime)); - return []; + return new MetricsData(); } } @@ -152,7 +152,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [[], []]); + assert.deepStrictEqual(result, [new MetricsData(), new MetricsData()]); await reader.shutdown(); }); }); @@ -170,7 +170,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [[], []]); + assert.deepStrictEqual(result, [new MetricsData(), new MetricsData()]); exporter.throwException = false; await reader.shutdown(); @@ -189,7 +189,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [[], []]); + assert.deepStrictEqual(result, [new MetricsData(), new MetricsData()]); exporter.throwException = false; await reader.shutdown(); @@ -355,7 +355,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); await reader.shutdown(); - assert.deepStrictEqual([], await reader.collect()); + assert.deepStrictEqual(await reader.collect(), new MetricsData()); }); it('should time out when timeoutMillis is set', async () => { 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 555d1f5049..11a70d6fca 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 @@ -23,7 +23,7 @@ import { PointDataType } from '../../src/export/MetricData'; import { MetricCollectorHandle } from '../../src/state/MetricCollector'; import { AsyncMetricStorage } from '../../src/state/AsyncMetricStorage'; import { NoopAttributesProcessor } from '../../src/view/AttributesProcessor'; -import { assertMetricData, assertPointData, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util'; +import { assertMetricData, assertPointData, defaultInstrumentDescriptor } from '../util'; import { ObservableCallback } from '@opentelemetry/api-metrics-wip'; const deltaCollector: MetricCollectorHandle = { @@ -71,8 +71,6 @@ describe('AsyncMetricStorage', () => { const metric = await metricStorage.collect( deltaCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -89,8 +87,6 @@ describe('AsyncMetricStorage', () => { const metric = await metricStorage.collect( deltaCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -107,8 +103,6 @@ describe('AsyncMetricStorage', () => { const metric = await metricStorage.collect( deltaCollector, [deltaCollector], - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -142,8 +136,6 @@ describe('AsyncMetricStorage', () => { const metric = await metricStorage.collect( cumulativeCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -160,8 +152,6 @@ describe('AsyncMetricStorage', () => { const metric = await metricStorage.collect( cumulativeCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -181,8 +171,6 @@ describe('AsyncMetricStorage', () => { const metric = await metricStorage.collect( cumulativeCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); 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 ea263bdd25..0343f9c945 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 @@ -18,7 +18,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { MeterProvider } from '../../src'; import { AggregationTemporality } from '../../src/export/AggregationTemporality'; -import { MetricData, PointDataType } from '../../src/export/MetricData'; +import { MetricsData, PointDataType } from '../../src/export/MetricData'; import { MetricExporter } from '../../src/export/MetricExporter'; import { MeterProviderSharedState } from '../../src/state/MeterProviderSharedState'; import { MetricCollector } from '../../src/state/MetricCollector'; @@ -26,9 +26,9 @@ import { defaultInstrumentationLibrary, defaultResource, assertMetricData, asser import { TestMetricReader } from '../export/TestMetricReader'; class TestMetricExporter extends MetricExporter { - metricDataList: MetricData[] = []; - async export(batch: MetricData[]): Promise { - this.metricDataList.push(...batch); + metricDataList: MetricsData[] = []; + async export(batch: MetricsData): Promise { + this.metricDataList.push(batch); } async forceFlush(): Promise {} @@ -91,24 +91,24 @@ describe('MetricCollector', () => { counter2.add(3); /** collect metrics */ - const batch = await metricCollector.collect(); - assert(Array.isArray(batch)); - assert.strictEqual(batch.length, 2); + const metricsData = await metricCollector.collect(); + const { metrics } = metricsData.resourceMetrics()[0].instrumentationLibraryMetrics[0]; + assert.strictEqual(metrics.length, 2); /** checking batch[0] */ - const metricData1 = batch[0]; + const metricData1 = metrics[0]; assertMetricData(metricData1, PointDataType.SINGULAR, { name: 'counter1' - }, defaultInstrumentationLibrary); + }); assert.strictEqual(metricData1.pointData.length, 2); assertPointData(metricData1.pointData[0], {}, 1); assertPointData(metricData1.pointData[1], { foo: 'bar' }, 2); /** checking batch[1] */ - const metricData2 = batch[1]; + const metricData2 = metrics[1]; assertMetricData(metricData2, PointDataType.SINGULAR, { name: 'counter2' - }, defaultInstrumentationLibrary); + }); assert.strictEqual(metricData2.pointData.length, 1); assertPointData(metricData2.pointData[0], {}, 3); }); 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 e782c14876..52e6eb2493 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 @@ -24,7 +24,7 @@ import { PointDataType } from '../../src/export/MetricData'; import { MetricCollectorHandle } from '../../src/state/MetricCollector'; import { SyncMetricStorage } from '../../src/state/SyncMetricStorage'; import { NoopAttributesProcessor } from '../../src/view/AttributesProcessor'; -import { assertMetricData, assertPointData, commonAttributes, commonValues, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util'; +import { assertMetricData, assertPointData, commonAttributes, commonValues, defaultInstrumentDescriptor } from '../util'; const deltaCollector: MetricCollectorHandle = { aggregatorTemporality: AggregationTemporality.DELTA, @@ -61,8 +61,6 @@ describe('SyncMetricStorage', () => { const metric = await metricStorage.collect( deltaCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -76,8 +74,6 @@ describe('SyncMetricStorage', () => { const metric = await metricStorage.collect( deltaCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -90,8 +86,6 @@ describe('SyncMetricStorage', () => { const metric = await metricStorage.collect( deltaCollector, [deltaCollector], - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -113,8 +107,6 @@ describe('SyncMetricStorage', () => { const metric = await metricStorage.collect( cumulativeCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -128,8 +120,6 @@ describe('SyncMetricStorage', () => { const metric = await metricStorage.collect( cumulativeCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, sdkStartTime, hrTime()); @@ -143,8 +133,6 @@ describe('SyncMetricStorage', () => { const metric = await metricStorage.collect( cumulativeCollector, collectors, - defaultResource, - defaultInstrumentationLibrary, 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 081ff0d035..569bb464ef 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 @@ -23,7 +23,7 @@ import { PointDataType } from '../../src/export/MetricData'; import { DeltaMetricProcessor } from '../../src/state/DeltaMetricProcessor'; import { MetricCollectorHandle } from '../../src/state/MetricCollector'; import { TemporalMetricProcessor } from '../../src/state/TemporalMetricProcessor'; -import { assertMetricData, assertPointData, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util'; +import { assertMetricData, assertPointData, defaultInstrumentDescriptor } from '../util'; const deltaCollector1: MetricCollectorHandle = { aggregatorTemporality: AggregationTemporality.DELTA, @@ -54,8 +54,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( deltaCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -71,8 +69,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( deltaCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -87,8 +83,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( deltaCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -113,8 +107,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( deltaCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -129,8 +121,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( deltaCollector2, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -155,8 +145,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( cumulativeCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -172,8 +160,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( cumulativeCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -198,8 +184,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( cumulativeCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -215,8 +199,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( deltaCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, @@ -230,8 +212,6 @@ describe('TemporalMetricProcessor', () => { const metric = temporalMetricStorage.buildMetrics( cumulativeCollector1, collectors, - defaultResource, - defaultInstrumentationLibrary, defaultInstrumentDescriptor, deltaMetricStorage.collect(), sdkStartTime, diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts index b50f6ac928..89eef5e65b 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts @@ -57,19 +57,11 @@ export function assertMetricData( actual: unknown, pointDataType?: PointDataType, instrumentDescriptor: Partial | null = defaultInstrumentDescriptor, - instrumentationLibrary: Partial | null = defaultInstrumentationLibrary, - resource: Resource | null = defaultResource, ): asserts actual is MetricData { const it = actual as MetricData; - if (resource != null) { - assert.deepStrictEqual(it.resource, resource); - } if (instrumentDescriptor != null) { assertPartialDeepStrictEqual(it.instrumentDescriptor, instrumentDescriptor); } - if (instrumentationLibrary != null) { - assertPartialDeepStrictEqual(it.instrumentationLibrary, instrumentationLibrary); - } if (isNotNullish(pointDataType)) { assert.strictEqual(it.pointDataType, pointDataType); } else { From a8be4f9b6a0e488b9ca94ebfc8c2fe2f8a248c07 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 4 Mar 2022 17:01:08 +0200 Subject: [PATCH 2/3] refactor: get rid of MetricsData --- .../src/Meter.ts | 15 +-- .../src/export/MetricData.ts | 62 --------- .../src/export/MetricExporter.ts | 6 +- .../src/export/MetricProducer.ts | 4 +- .../src/export/MetricReader.ts | 9 +- .../export/PeriodicExportingMetricReader.ts | 6 + .../src/state/MetricCollector.ts | 13 +- .../test/Instruments.test.ts | 11 +- .../test/export/MetricData.test.ts | 118 ------------------ .../PeriodicExportingMetricReader.test.ts | 27 ++-- .../test/state/MetricCollector.test.ts | 10 +- 11 files changed, 56 insertions(+), 225 deletions(-) delete mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricData.test.ts diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts index d6ab0106c9..ed5416e7f1 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts @@ -22,7 +22,7 @@ import { MeterProviderSharedState } from './state/MeterProviderSharedState'; import { MultiMetricStorage } from './state/MultiWritableMetricStorage'; import { SyncMetricStorage } from './state/SyncMetricStorage'; import { MetricStorage } from './state/MetricStorage'; -import { MetricsData } from './export/MetricData'; +import { InstrumentationLibraryMetrics } from './export/MetricData'; import { isNotNullish } from './utils'; import { MetricCollectorHandle } from './state/MetricCollector'; import { HrTime } from '@opentelemetry/api'; @@ -130,7 +130,7 @@ export class Meter implements metrics.Meter { * @param collectionTime the HrTime at which the collection was initiated. * @returns the list of {@link MetricData} collected. */ - async collect(collector: MetricCollectorHandle, collectionTime: HrTime): Promise { + async collect(collector: MetricCollectorHandle, collectionTime: HrTime): Promise { const metricData = await Promise.all(Array.from(this._metricStorageRegistry.values()).map(metricStorage => { return metricStorage.collect( collector, @@ -139,12 +139,9 @@ export class Meter implements metrics.Meter { collectionTime); })); - return new MetricsData({ - resource: this._meterProviderSharedState.resource, - instrumentationLibraryMetrics: [{ - instrumentationLibrary: this._instrumentationLibrary, - metrics: metricData.filter(isNotNullish), - }] - }); + return { + instrumentationLibrary: this._instrumentationLibrary, + metrics: metricData.filter(isNotNullish), + }; } } 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 ccf30ba108..2d9f5bcf34 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts @@ -64,68 +64,6 @@ export interface ResourceMetrics { instrumentationLibraryMetrics: InstrumentationLibraryMetrics[]; } -type InstrumentationLibraryMetricsMap = Map; - -export class MetricsData { - private _metricsData: Map = new Map(); - - constructor(resourceMetrics?: ResourceMetrics) { - if (resourceMetrics === undefined) { - return; - } - - const metricsByInstrLib = new Map(); - - for (const ilMetrics of resourceMetrics.instrumentationLibraryMetrics) { - metricsByInstrLib.set(ilMetrics.instrumentationLibrary, ilMetrics.metrics); - } - - this._metricsData.set(resourceMetrics.resource, metricsByInstrLib); - } - - resourceMetrics(): ResourceMetrics[] { - return Array.from(this._metricsData, ([resource, metricsByInstrLib]) => { - const instrumentationLibraryMetrics = Array.from(metricsByInstrLib, ([instrumentationLibrary, metrics]) => ({ - instrumentationLibrary, - metrics - })); - - return { - resource, - instrumentationLibraryMetrics, - }; - }); - } - - merge(source: MetricsData): MetricsData { - function mergeIlMetrics(into: InstrumentationLibraryMetricsMap, from: InstrumentationLibraryMetricsMap): InstrumentationLibraryMetricsMap { - from.forEach((metrics, instrumentationLib) => { - const targetMetrics = into.get(instrumentationLib); - - if (targetMetrics === undefined) { - into.set(instrumentationLib, metrics); - } else { - into.set(instrumentationLib, targetMetrics.concat(metrics)); - } - }); - - return into; - } - - source._metricsData.forEach((sourceIlMetrics, resource) => { - const targetIlMetrics = this._metricsData.get(resource); - - if (targetIlMetrics === undefined) { - this._metricsData.set(resource, sourceIlMetrics); - } else { - this._metricsData.set(resource, mergeIlMetrics(targetIlMetrics, sourceIlMetrics)); - } - }); - - return this; - } -} - /** * The aggregated point data type. */ 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 40530e338a..2d84d94eac 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts @@ -15,7 +15,7 @@ */ import { AggregationTemporality } from './AggregationTemporality'; -import { MetricsData } from './MetricData'; +import { ResourceMetrics } from './MetricData'; import { ExportResult, ExportResultCode, @@ -26,7 +26,7 @@ import { export interface PushMetricExporter { - export(batch: MetricsData, resultCallback: (result: ExportResult) => void): void; + export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void): void; forceFlush(): Promise; @@ -39,7 +39,7 @@ export interface PushMetricExporter { export class ConsoleMetricExporter implements PushMetricExporter { protected _shutdown = true; - export(_batch: MetricsData, resultCallback: (result: ExportResult) => void) { + export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void) { return resultCallback({ code: ExportResultCode.FAILED, error: new Error('Method not implemented') diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricProducer.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricProducer.ts index 347e8a2d62..6174286b0c 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricProducer.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricProducer.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { MetricsData } from './MetricData'; +import { ResourceMetrics } from './MetricData'; /** * This is a public interface that represent an export state of a MetricReader. */ export interface MetricProducer { - collect(): Promise; + collect(): Promise; } 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 43f9a8309c..5086172628 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts @@ -17,8 +17,9 @@ import * as api from '@opentelemetry/api'; import { AggregationTemporality } from './AggregationTemporality'; import { MetricProducer } from './MetricProducer'; -import { MetricsData } from './MetricData'; -import { callWithTimeout } from '../utils'; +import { ResourceMetrics } from './MetricData'; +import { callWithTimeout, Maybe } from '../utils'; + export type ReaderOptions = { timeoutMillis?: number @@ -91,7 +92,7 @@ export abstract class MetricReader { /** * Collect all metrics from the associated {@link MetricProducer} */ - async collect(options?: ReaderCollectionOptions): Promise { + async collect(options?: ReaderCollectionOptions): Promise> { if (this._metricProducer === undefined) { throw new Error('MetricReader is not bound to a MetricProducer'); } @@ -99,7 +100,7 @@ export abstract class MetricReader { // Subsequent invocations to collect are not allowed. SDKs SHOULD return some failure for these calls. if (this._shutdown) { api.diag.warn('Collection is not allowed after shutdown'); - return new MetricsData(); + return undefined; } // No timeout if timeoutMillis is undefined or null. 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 f5595a743d..088eba88ff 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts @@ -63,7 +63,13 @@ export class PeriodicExportingMetricReader extends MetricReader { private async _runOnce(): Promise { const metrics = await this.collect({}); + return new Promise((resolve, reject) => { + if (metrics === undefined) { + resolve(); + return; + } + this._exporter.export(metrics, result => { if (result.code !== ExportResultCode.SUCCESS) { reject( 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 52adaac428..7364d3548b 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts @@ -16,7 +16,7 @@ import { hrTime } from '@opentelemetry/core'; import { AggregationTemporality } from '../export/AggregationTemporality'; -import { MetricsData } from '../export/MetricData'; +import { ResourceMetrics } from '../export/MetricData'; import { MetricProducer } from '../export/MetricProducer'; import { MetricReader } from '../export/MetricReader'; import { MeterProviderSharedState } from './MeterProviderSharedState'; @@ -32,12 +32,15 @@ export class MetricCollector implements MetricProducer { this.aggregatorTemporality = this._metricReader.getPreferredAggregationTemporality(); } - async collect(): Promise { + async collect(): Promise { const collectionTime = hrTime(); - const results = await Promise.all(this._sharedState.meters - .map(meter => meter.collect(this, collectionTime))); + const instrumentationLibraryMetrics = (await Promise.all(this._sharedState.meters + .map(meter => meter.collect(this, collectionTime)))).filter(({ metrics }) => metrics.length > 0); - return results.reduce((cumulation, current) => cumulation.merge(current), new MetricsData()); + return { + resource: this._sharedState.resource, + instrumentationLibraryMetrics, + }; } /** 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 c454a638e9..0c03d1aaed 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts @@ -543,16 +543,19 @@ interface ValidateMetricData { } async function validateExport(reader: MetricReader, expected: ValidateMetricData) { - const metricsData = await reader.collect(); + const resourceMetrics = await reader.collect(); - const { resource, instrumentationLibraryMetrics } = metricsData.resourceMetrics()[0]; - const { instrumentationLibrary, metrics } = instrumentationLibraryMetrics[0]; + assert.notStrictEqual(resourceMetrics, undefined); - const metric = metrics[0]; + const { resource, instrumentationLibraryMetrics } = resourceMetrics!; + + const { instrumentationLibrary, metrics } = instrumentationLibraryMetrics[0]; assert.deepStrictEqual(resource, defaultResource); assert.deepStrictEqual(instrumentationLibrary, defaultInstrumentationLibrary); + const metric = metrics[0]; + assertMetricData( metric, expected.pointDataType, diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricData.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricData.test.ts deleted file mode 100644 index f0486d8279..0000000000 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricData.test.ts +++ /dev/null @@ -1,118 +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 { MetricData, MetricsData, PointDataType } from '../../src/export/MetricData'; -import { InstrumentType } from '../../src/InstrumentDescriptor'; -import { ValueType } from '@opentelemetry/api-metrics-wip'; -import * as assert from 'assert'; -import { defaultInstrumentationLibrary, defaultResource } from '../util'; -import { hrTime, InstrumentationLibrary } from '@opentelemetry/core'; - -const defaultInstrumentDescriptor = { - name: 'test', - description: 'abc', - unit: 's', - type: InstrumentType.COUNTER, - valueType: ValueType.DOUBLE, -}; - -const defaultMetrics: MetricData[] = [{ - instrumentDescriptor: defaultInstrumentDescriptor, - pointDataType: PointDataType.SINGULAR, - pointData: [ - { point: 0, attributes: {}, startTime: hrTime(), endTime: hrTime() } - ] -}]; - -describe('MetricsData', () => { - describe('merge', () => { - it('should produce empty metrics data when merging empty data', () => { - const merged = new MetricsData().merge(new MetricsData()); - assert.deepStrictEqual(new MetricsData(), merged); - }); - - it('should retain target values when merging with empty data', () => { - const target = new MetricsData({ - resource: defaultResource, - instrumentationLibraryMetrics: [{ - instrumentationLibrary: defaultInstrumentationLibrary, - metrics: defaultMetrics, - }] - }); - - const merged = target.merge(new MetricsData()); - assert.deepStrictEqual(target, merged); - }); - - it('should concatenate metrics list for matchin resource and instrumentation library', () => { - const target = new MetricsData({ - resource: defaultResource, - instrumentationLibraryMetrics: [{ - instrumentationLibrary: defaultInstrumentationLibrary, - metrics: defaultMetrics, - }] - }); - - const merged = target.merge(target); - - const expected = new MetricsData({ - resource: defaultResource, - instrumentationLibraryMetrics: [{ - instrumentationLibrary: defaultInstrumentationLibrary, - metrics: defaultMetrics.concat(defaultMetrics), - }] - }); - assert.deepStrictEqual(expected, merged); - }); - }); - - it('should add new instrumentation library for matching resource', () => { - const target = new MetricsData({ - resource: defaultResource, - instrumentationLibraryMetrics: [{ - instrumentationLibrary: defaultInstrumentationLibrary, - metrics: defaultMetrics, - }] - }); - - const instrumentationLibrary: InstrumentationLibrary = { - name: 'other', - version: '1.0.0', - schemaUrl: 'https://opentelemetry.io/schemas/1.7.0' - }; - - const merged = target.merge(new MetricsData({ - resource: defaultResource, - instrumentationLibraryMetrics: [{ - instrumentationLibrary, - metrics: defaultMetrics, - }] - })); - - const expected = new MetricsData({ - resource: defaultResource, - instrumentationLibraryMetrics: [{ - instrumentationLibrary: defaultInstrumentationLibrary, - metrics: defaultMetrics, - }, { - instrumentationLibrary, - metrics: defaultMetrics, - }] - }); - - assert.deepStrictEqual(expected, merged); - }); -}); 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 3be1a22894..1cde6184c2 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 @@ -17,13 +17,14 @@ import { PeriodicExportingMetricReader } from '../../src/export/PeriodicExportingMetricReader'; import { AggregationTemporality } from '../../src/export/AggregationTemporality'; import { PushMetricExporter } from '../../src'; -import { MetricsData } from '../../src/export/MetricData'; +import { ResourceMetrics } from '../../src/export/MetricData'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { MetricProducer } from '../../src/export/MetricProducer'; import { TimeoutError } from '../../src/utils'; import { ExportResult, ExportResultCode } from '@opentelemetry/core'; import { assertRejects } from '../test-utils'; +import { defaultResource } from '../util'; const MAX_32_BIT_INT = 2 ** 31 - 1; @@ -32,11 +33,11 @@ class TestMetricExporter implements PushMetricExporter { public forceFlushTime = 0; public throwException = false; public failureResult = false; - private _batches: MetricsData[] = []; + private _batches: ResourceMetrics[] = []; private _shutdown: boolean = false; - export(batch: MetricsData, resultCallback: (result: ExportResult) => void): void { - this._batches.push(batch); + export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void): void { + this._batches.push(metrics); if (this.throwException) { throw new Error('Error during export'); @@ -65,7 +66,7 @@ class TestMetricExporter implements PushMetricExporter { await new Promise(resolve => setTimeout(resolve, this.forceFlushTime)); } - async waitForNumberOfExports(numberOfExports: number): Promise { + async waitForNumberOfExports(numberOfExports: number): Promise { if (numberOfExports <= 0) { throw new Error('numberOfExports must be greater than or equal to 0'); } @@ -87,12 +88,14 @@ class TestDeltaMetricExporter extends TestMetricExporter { } } +const emptyResourceMetrics = { resource: defaultResource, instrumentationLibraryMetrics: [] }; + class TestMetricProducer implements MetricProducer { public collectionTime = 0; - async collect(): Promise { + async collect(): Promise { await new Promise(resolve => setTimeout(resolve, this.collectionTime)); - return new MetricsData(); + return { resource: defaultResource, instrumentationLibraryMetrics: [] }; } } @@ -168,7 +171,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [new MetricsData(), new MetricsData()]); + assert.deepStrictEqual(result, [emptyResourceMetrics, emptyResourceMetrics]); await reader.shutdown(); }); }); @@ -186,7 +189,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [new MetricsData(), new MetricsData()]); + assert.deepStrictEqual(result, [emptyResourceMetrics, emptyResourceMetrics]); exporter.throwException = false; await reader.shutdown(); @@ -204,7 +207,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [new MetricsData(), new MetricsData()]); + assert.deepStrictEqual(result, [emptyResourceMetrics, emptyResourceMetrics]); exporter.failureResult = false; await reader.shutdown(); @@ -223,7 +226,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [new MetricsData(), new MetricsData()]); + assert.deepStrictEqual(result, [emptyResourceMetrics, emptyResourceMetrics]); exporter.throwException = false; await reader.shutdown(); @@ -389,7 +392,7 @@ describe('PeriodicExportingMetricReader', () => { reader.setMetricProducer(new TestMetricProducer()); await reader.shutdown(); - assert.deepStrictEqual(await reader.collect(), new MetricsData()); + assert.deepStrictEqual(await reader.collect(), undefined); }); it('should time out when timeoutMillis is set', async () => { 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 a4cd6e86ed..56ac74a701 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 @@ -18,7 +18,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { MeterProvider } from '../../src'; import { AggregationTemporality } from '../../src/export/AggregationTemporality'; -import { MetricsData, PointDataType } from '../../src/export/MetricData'; +import { PointDataType, ResourceMetrics } from '../../src/export/MetricData'; import { PushMetricExporter } from '../../src/export/MetricExporter'; import { MeterProviderSharedState } from '../../src/state/MeterProviderSharedState'; import { MetricCollector } from '../../src/state/MetricCollector'; @@ -27,9 +27,7 @@ import { TestMetricReader } from '../export/TestMetricReader'; import { ExportResult, ExportResultCode } from '@opentelemetry/core'; class TestMetricExporter implements PushMetricExporter { - metricDataList: MetricsData[] = []; - async export(batch: MetricsData, resultCallback: (result: ExportResult) => void): Promise { - this.metricDataList.push(batch); + async export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void): Promise { resultCallback({code: ExportResultCode.SUCCESS}); } @@ -96,8 +94,8 @@ describe('MetricCollector', () => { counter2.add(3); /** collect metrics */ - const metricsData = await metricCollector.collect(); - const { metrics } = metricsData.resourceMetrics()[0].instrumentationLibraryMetrics[0]; + const { instrumentationLibraryMetrics } = await metricCollector.collect(); + const { metrics } = instrumentationLibraryMetrics[0]; assert.strictEqual(metrics.length, 2); /** checking batch[0] */ From a6b1bea85941dd5b249fd19c69b9bd143a142e6e Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 4 Mar 2022 20:33:46 +0200 Subject: [PATCH 3/3] fix: don't filter empty instrumentation library metrics --- .../src/export/PeriodicExportingMetricReader.ts | 9 ++++----- .../src/state/MetricCollector.ts | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) 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 088eba88ff..bd647f4029 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts @@ -64,12 +64,11 @@ export class PeriodicExportingMetricReader extends MetricReader { private async _runOnce(): Promise { const metrics = await this.collect({}); - return new Promise((resolve, reject) => { - if (metrics === undefined) { - resolve(); - return; - } + if (metrics === undefined) { + return; + } + return new Promise((resolve, reject) => { this._exporter.export(metrics, result => { if (result.code !== ExportResultCode.SUCCESS) { reject( 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 7364d3548b..6be9022b99 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts @@ -35,7 +35,7 @@ export class MetricCollector implements MetricProducer { async collect(): Promise { const collectionTime = hrTime(); const instrumentationLibraryMetrics = (await Promise.all(this._sharedState.meters - .map(meter => meter.collect(this, collectionTime)))).filter(({ metrics }) => metrics.length > 0); + .map(meter => meter.collect(this, collectionTime)))); return { resource: this._sharedState.resource,