diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d2af8ed12..83ae8f07a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se ### :bug: (Bug Fix) +* fix(sdk-metrics): do not export from `PeriodicExportingMetricReader` when there are no metrics to export. [#5288](https://github.com/open-telemetry/opentelemetry-js/pull/5288) @jacksonweber + ### :books: (Refine Doc) ### :house: (Internal) diff --git a/packages/sdk-metrics/src/export/PeriodicExportingMetricReader.ts b/packages/sdk-metrics/src/export/PeriodicExportingMetricReader.ts index 6a9096652d..567dde78db 100644 --- a/packages/sdk-metrics/src/export/PeriodicExportingMetricReader.ts +++ b/packages/sdk-metrics/src/export/PeriodicExportingMetricReader.ts @@ -135,11 +135,13 @@ export class PeriodicExportingMetricReader extends MetricReader { } } - const result = await internal._export(this._exporter, resourceMetrics); - if (result.code !== ExportResultCode.SUCCESS) { - throw new Error( - `PeriodicExportingMetricReader: metrics export failed (error ${result.error})` - ); + if (resourceMetrics.scopeMetrics.length > 0) { + const result = await internal._export(this._exporter, resourceMetrics); + if (result.code !== ExportResultCode.SUCCESS) { + throw new Error( + `PeriodicExportingMetricReader: metrics export failed (error ${result.error})` + ); + } } } diff --git a/packages/sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts b/packages/sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts index 3294697874..e3b622810e 100644 --- a/packages/sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts +++ b/packages/sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts @@ -24,7 +24,11 @@ import { MetricProducer, PushMetricExporter, } from '../../src'; -import { ResourceMetrics } from '../../src/export/MetricData'; +import { + DataPointType, + ResourceMetrics, + ScopeMetrics, +} from '../../src/export/MetricData'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { TimeoutError } from '../../src/utils'; @@ -34,7 +38,7 @@ import { setGlobalErrorHandler, } from '@opentelemetry/core'; import { assertRejects } from '../test-utils'; -import { emptyResourceMetrics, TestMetricProducer } from './TestMetricProducer'; +import { TestMetricProducer } from './TestMetricProducer'; import { assertAggregationSelector, assertAggregationTemporalitySelector, @@ -43,6 +47,7 @@ import { DEFAULT_AGGREGATION_SELECTOR, DEFAULT_AGGREGATION_TEMPORALITY_SELECTOR, } from '../../src/export/AggregationSelector'; +import { ValueType } from '@opentelemetry/api'; const MAX_32_BIT_INT = 2 ** 31 - 1; @@ -126,6 +131,52 @@ describe('PeriodicExportingMetricReader', () => { sinon.restore(); }); + const waitForAsyncAttributesStub = sinon.stub().returns( + new Promise(resolve => + setTimeout(() => { + resolve(); + }, 10) + ) + ); + const scopeMetrics: ScopeMetrics[] = [ + { + scope: { + name: 'test', + }, + metrics: [ + { + dataPointType: DataPointType.GAUGE, + dataPoints: [ + { + // Sample hr time datapoints. + startTime: [12345, 678901234], + endTime: [12345, 678901234], + attributes: {}, + value: 1, + }, + ], + descriptor: { + name: '', + description: '', + unit: '', + type: InstrumentType.COUNTER, + valueType: ValueType.INT, + }, + aggregationTemporality: AggregationTemporality.CUMULATIVE, + }, + ], + }, + ]; + const resourceMetrics: ResourceMetrics = { + resource: { + attributes: {}, + merge: sinon.stub(), + asyncAttributesPending: true, // ensure we try to await async attributes + waitForAsyncAttributes: waitForAsyncAttributesStub, // resolve when awaited + }, + scopeMetrics: scopeMetrics, + }; + describe('constructor', () => { it('should construct PeriodicExportingMetricReader without exceptions', () => { const exporter = new TestDeltaMetricExporter(); @@ -203,17 +254,31 @@ describe('PeriodicExportingMetricReader', () => { exportTimeoutMillis: 20, }); - reader.setMetricProducer(new TestMetricProducer()); + reader.setMetricProducer( + new TestMetricProducer({ resourceMetrics: resourceMetrics, errors: [] }) + ); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [ - emptyResourceMetrics, - emptyResourceMetrics, - ]); + assert.deepStrictEqual(result, [resourceMetrics, resourceMetrics]); await reader.shutdown(); }); }); + it('should not export without populated scope metrics', async () => { + const exporter = new TestMetricExporter(); + const reader = new PeriodicExportingMetricReader({ + exporter: exporter, + exportIntervalMillis: 30, + exportTimeoutMillis: 20, + }); + + reader.setMetricProducer(new TestMetricProducer()); + const result = await exporter.forceFlush(); + + assert.deepStrictEqual(result, undefined); + await reader.shutdown(); + }); + describe('periodic export', () => { it('should keep running on export errors', async () => { const exporter = new TestMetricExporter(); @@ -224,13 +289,12 @@ describe('PeriodicExportingMetricReader', () => { exportTimeoutMillis: 20, }); - reader.setMetricProducer(new TestMetricProducer()); + reader.setMetricProducer( + new TestMetricProducer({ resourceMetrics: resourceMetrics, errors: [] }) + ); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [ - emptyResourceMetrics, - emptyResourceMetrics, - ]); + assert.deepStrictEqual(result, [resourceMetrics, resourceMetrics]); exporter.throwExport = false; await reader.shutdown(); @@ -245,13 +309,12 @@ describe('PeriodicExportingMetricReader', () => { exportTimeoutMillis: 20, }); - reader.setMetricProducer(new TestMetricProducer()); + reader.setMetricProducer( + new TestMetricProducer({ resourceMetrics: resourceMetrics, errors: [] }) + ); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [ - emptyResourceMetrics, - emptyResourceMetrics, - ]); + assert.deepStrictEqual(result, [resourceMetrics, resourceMetrics]); exporter.rejectExport = false; await reader.shutdown(); @@ -267,13 +330,12 @@ describe('PeriodicExportingMetricReader', () => { exportTimeoutMillis: 20, }); - reader.setMetricProducer(new TestMetricProducer()); + reader.setMetricProducer( + new TestMetricProducer({ resourceMetrics: resourceMetrics, errors: [] }) + ); const result = await exporter.waitForNumberOfExports(2); - assert.deepStrictEqual(result, [ - emptyResourceMetrics, - emptyResourceMetrics, - ]); + assert.deepStrictEqual(result, [resourceMetrics, resourceMetrics]); exporter.throwExport = false; await reader.shutdown(); @@ -295,7 +357,9 @@ describe('PeriodicExportingMetricReader', () => { exportTimeoutMillis: 80, }); - reader.setMetricProducer(new TestMetricProducer()); + reader.setMetricProducer( + new TestMetricProducer({ resourceMetrics: resourceMetrics, errors: [] }) + ); await reader.forceFlush(); exporterMock.verify(); @@ -321,7 +385,7 @@ describe('PeriodicExportingMetricReader', () => { asyncAttributesPending: true, // ensure we try to await async attributes waitForAsyncAttributes: waitForAsyncAttributesStub, // resolve when awaited }, - scopeMetrics: [], + scopeMetrics: scopeMetrics, }; const mockCollectionResult: CollectionResult = {