From d3adc22c4715d77e3a6913d0e14950fd68d470f9 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Mon, 5 Sep 2022 10:05:43 +0200 Subject: [PATCH] chore(api-metrics): clean up exports (#3198) * chore(api-metrics): clean up exports * fix(changelog): add changelog entry. * fix(changelog): add changelog entry. * fix: formatting. * fix(changelog): move entry to unreleased. * fix(instrumentation): remove references to NOOP_METER_PROVIDER * fix(instrumentation): also stub setMeterProvider. * sort changelog export list * feat(api-metrics): add createNoopMeter function. * feat(api-metrics): document createNoopMeter. * fix(whitespace): reformat NoopMeter.ts * chore: add test for createNoopMeter() Co-authored-by: Daniel Dyla --- experimental/CHANGELOG.md | 19 +++++++++ .../src/NoopMeter.ts | 10 +++++ .../opentelemetry-api-metrics/src/index.ts | 40 ++++++++++++++++--- .../test/api/api.test.ts | 5 ++- .../test/api/global.test.ts | 2 +- .../noop-implementations/noop-meter.test.ts | 11 ++++- .../test/common/autoLoader.test.ts | 26 ++++++++---- .../src/MeterProvider.ts | 2 +- .../test/MeterProvider.test.ts | 4 +- .../opentelemetry-sdk-node/test/sdk.test.ts | 24 ++++++++--- 10 files changed, 117 insertions(+), 26 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 98efacaa7e..a489427839 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -19,6 +19,25 @@ All notable changes to experimental packages in this project will be documented * `createInstrumentDescriptor` * `createInstrumentDescriptorWithView` * `isDescriptorCompatibleWith` +* chore(api-metrics): clean up exports [#3198](https://github.com/open-telemetry/opentelemetry-js/pull/3198) @pichlermarc + * removes export for: + * `NOOP_COUNTER_METRIC` + * `NOOP_HISTOGRAM_METRIC` + * `NOOP_METER_PROVIDER` + * `NOOP_OBSERVABLE_COUNTER_METRIC` + * `NOOP_OBSERVABLE_GAUGE_METRIC` + * `NOOP_OBSERVABLE_UP_DOWN_COUNTER_METRIC` + * `NOOP_UP_DOWN_COUNTER_METRIC` + * `NoopCounterMetric` + * `NoopHistogramMetric` + * `NoopMeter` + * `NoopMeterProvider` + * `NoopMetric` + * `NoopObservableCounterMetric` + * `NoopObservableGaugeMetric` + * `NoopObservableMetric` + * `NoopObservableUpDownCounterMetric` + * `NoopUpDownCounterMetric` ### :rocket: (Enhancement) diff --git a/experimental/packages/opentelemetry-api-metrics/src/NoopMeter.ts b/experimental/packages/opentelemetry-api-metrics/src/NoopMeter.ts index 317431096f..69c2513449 100644 --- a/experimental/packages/opentelemetry-api-metrics/src/NoopMeter.ts +++ b/experimental/packages/opentelemetry-api-metrics/src/NoopMeter.ts @@ -114,11 +114,14 @@ export class NoopHistogramMetric extends NoopMetric implements Histogram { export class NoopObservableMetric { addCallback(_callback: ObservableCallback) {} + removeCallback(_callback: ObservableCallback) {} } export class NoopObservableCounterMetric extends NoopObservableMetric implements ObservableCounter {} + export class NoopObservableGaugeMetric extends NoopObservableMetric implements ObservableGauge {} + export class NoopObservableUpDownCounterMetric extends NoopObservableMetric implements ObservableUpDownCounter {} export const NOOP_METER = new NoopMeter(); @@ -132,3 +135,10 @@ export const NOOP_UP_DOWN_COUNTER_METRIC = new NoopUpDownCounterMetric(); export const NOOP_OBSERVABLE_COUNTER_METRIC = new NoopObservableCounterMetric(); export const NOOP_OBSERVABLE_GAUGE_METRIC = new NoopObservableGaugeMetric(); export const NOOP_OBSERVABLE_UP_DOWN_COUNTER_METRIC = new NoopObservableUpDownCounterMetric(); + +/** + * Create a no-op Meter + */ +export function createNoopMeter(): Meter { + return NOOP_METER; +} diff --git a/experimental/packages/opentelemetry-api-metrics/src/index.ts b/experimental/packages/opentelemetry-api-metrics/src/index.ts index e306e6c1ab..5a59fe2d96 100644 --- a/experimental/packages/opentelemetry-api-metrics/src/index.ts +++ b/experimental/packages/opentelemetry-api-metrics/src/index.ts @@ -14,13 +14,41 @@ * limitations under the License. */ -export * from './NoopMeter'; -export * from './NoopMeterProvider'; -export * from './types/Meter'; -export * from './types/MeterProvider'; -export * from './types/Metric'; -export * from './types/ObservableResult'; +export { + createNoopMeter, +} from './NoopMeter'; + +export { + MeterOptions, + Meter, +} from './types/Meter'; + +export { + MeterProvider, +} from './types/MeterProvider'; + +export { + ValueType, + Counter, + Histogram, + MetricOptions, + Observable, + ObservableCounter, + ObservableGauge, + ObservableUpDownCounter, + UpDownCounter, + BatchObservableCallback, + MetricAttributes, + MetricAttributeValue, + ObservableCallback, +} from './types/Metric'; + +export { + BatchObservableResult, + ObservableResult, +} from './types/ObservableResult'; import { MetricsAPI } from './api/metrics'; + /** Entrypoint for metrics API */ export const metrics = MetricsAPI.getInstance(); diff --git a/experimental/packages/opentelemetry-api-metrics/test/api/api.test.ts b/experimental/packages/opentelemetry-api-metrics/test/api/api.test.ts index e9a837230e..66278d6bc7 100644 --- a/experimental/packages/opentelemetry-api-metrics/test/api/api.test.ts +++ b/experimental/packages/opentelemetry-api-metrics/test/api/api.test.ts @@ -15,7 +15,10 @@ */ import * as assert from 'assert'; -import { metrics, NoopMeter, NoopMeterProvider } from '../../src'; +import { metrics } from '../../src'; +import { NoopMeter } from '../../src/NoopMeter'; +import { NoopMeterProvider } from '../../src/NoopMeterProvider'; + describe('API', () => { it('should expose a meter provider via getMeterProvider', () => { diff --git a/experimental/packages/opentelemetry-api-metrics/test/api/global.test.ts b/experimental/packages/opentelemetry-api-metrics/test/api/global.test.ts index 485c68c41a..c48949a3e5 100644 --- a/experimental/packages/opentelemetry-api-metrics/test/api/global.test.ts +++ b/experimental/packages/opentelemetry-api-metrics/test/api/global.test.ts @@ -16,7 +16,7 @@ import * as assert from 'assert'; import { _global, GLOBAL_METRICS_API_KEY } from '../../src/api/global-utils'; -import { NoopMeterProvider } from '../../src'; +import { NoopMeterProvider } from '../../src/NoopMeterProvider'; const api1 = require('../../src') as typeof import('../../src'); diff --git a/experimental/packages/opentelemetry-api-metrics/test/noop-implementations/noop-meter.test.ts b/experimental/packages/opentelemetry-api-metrics/test/noop-implementations/noop-meter.test.ts index 82f0e8b6be..07395a925b 100644 --- a/experimental/packages/opentelemetry-api-metrics/test/noop-implementations/noop-meter.test.ts +++ b/experimental/packages/opentelemetry-api-metrics/test/noop-implementations/noop-meter.test.ts @@ -17,14 +17,15 @@ import * as assert from 'assert'; import { NoopMeter, - NoopMeterProvider, NOOP_COUNTER_METRIC, NOOP_HISTOGRAM_METRIC, NOOP_OBSERVABLE_COUNTER_METRIC, NOOP_OBSERVABLE_GAUGE_METRIC, NOOP_OBSERVABLE_UP_DOWN_COUNTER_METRIC, NOOP_UP_DOWN_COUNTER_METRIC, -} from '../../src'; + createNoopMeter, +} from '../../src/NoopMeter'; +import { NoopMeterProvider } from '../../src/NoopMeterProvider'; const attributes = {}; const options = { @@ -133,3 +134,9 @@ describe('NoopMeter', () => { meter.removeBatchObservableCallback(() => {}, []); }); }); + +describe('createNoopMeter', () => { + it('should return NoopMeter', () => { + assert.ok(createNoopMeter() instanceof NoopMeter); + }); +}); diff --git a/experimental/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts b/experimental/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts index ff7971f66a..c7bf2f6967 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts @@ -15,16 +15,27 @@ */ import { Tracer, TracerProvider } from '@opentelemetry/api'; -import { NOOP_METER_PROVIDER } from '@opentelemetry/api-metrics'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { InstrumentationBase, registerInstrumentations } from '../../src'; +import { + Meter, + MeterOptions, + MeterProvider, +} from '@opentelemetry/api-metrics'; class DummyTracerProvider implements TracerProvider { getTracer(name: string, version?: string): Tracer { throw new Error('not implemented'); } } + +class DummyMeterProvider implements MeterProvider { + getMeter(name: string, version?: string, options?: MeterOptions): Meter { + throw new Error('not implemented'); + } +} + class FooInstrumentation extends InstrumentationBase { init() { return []; @@ -50,14 +61,14 @@ describe('autoLoader', () => { let instrumentation: InstrumentationBase; let enableSpy: sinon.SinonSpy; let setTracerProviderSpy: sinon.SinonSpy; - let setsetMeterProvider: sinon.SinonSpy; + let setMeterProviderSpy: sinon.SinonSpy; const tracerProvider = new DummyTracerProvider(); - const meterProvider = NOOP_METER_PROVIDER; + const meterProvider = new DummyMeterProvider(); beforeEach(() => { instrumentation = new FooInstrumentation('foo', '1', {}); enableSpy = sinon.spy(instrumentation, 'enable'); setTracerProviderSpy = sinon.stub(instrumentation, 'setTracerProvider'); - setsetMeterProvider = sinon.stub(instrumentation, 'setMeterProvider'); + setMeterProviderSpy = sinon.stub(instrumentation, 'setMeterProvider'); unload = registerInstrumentations({ instrumentations: [instrumentation], tracerProvider, @@ -85,6 +96,7 @@ describe('autoLoader', () => { ); enableSpy = sinon.spy(instrumentation, 'enable'); setTracerProviderSpy = sinon.stub(instrumentation, 'setTracerProvider'); + setMeterProviderSpy = sinon.stub(instrumentation, 'setMeterProvider'); unload = registerInstrumentations({ instrumentations: [instrumentation], tracerProvider, @@ -104,9 +116,9 @@ describe('autoLoader', () => { }); it('should set MeterProvider', () => { - assert.strictEqual(setsetMeterProvider.callCount, 1); - assert.ok(setsetMeterProvider.lastCall.args[0] === meterProvider); - assert.strictEqual(setsetMeterProvider.lastCall.args.length, 1); + assert.strictEqual(setMeterProviderSpy.callCount, 1); + assert.ok(setMeterProviderSpy.lastCall.args[0] === meterProvider); + assert.strictEqual(setMeterProviderSpy.lastCall.args.length, 1); }); }); }); diff --git a/experimental/packages/opentelemetry-sdk-metrics/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics/src/MeterProvider.ts index 003bb41a6b..f8008d1c4b 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/src/MeterProvider.ts @@ -55,7 +55,7 @@ export class MeterProvider implements metrics.MeterProvider { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#meter-creation if (this._shutdown) { api.diag.warn('A shutdown MeterProvider cannot provide a Meter'); - return metrics.NOOP_METER; + return metrics.createNoopMeter(); } return this._sharedState diff --git a/experimental/packages/opentelemetry-sdk-metrics/test/MeterProvider.test.ts b/experimental/packages/opentelemetry-sdk-metrics/test/MeterProvider.test.ts index 3d7dc68c6a..e8c42bf798 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/MeterProvider.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/test/MeterProvider.test.ts @@ -15,7 +15,6 @@ */ import * as assert from 'assert'; -import { NOOP_METER } from '@opentelemetry/api-metrics'; import { Meter, MeterProvider, InstrumentType, DataPointType } from '../src'; import { assertScopeMetrics, @@ -62,7 +61,8 @@ describe('MeterProvider', () => { const meterProvider = new MeterProvider(); meterProvider.shutdown(); const meter = meterProvider.getMeter('meter1', '1.0.0'); - assert.strictEqual(meter, NOOP_METER); + // returned tracer should be no-op, not instance of Meter (from SDK) + assert.ok(!(meter instanceof Meter)); }); it('get meter with same identity', async () => { diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index ebc07dc2bc..c1b0b90d56 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -22,13 +22,21 @@ import { diag, DiagLogLevel, } from '@opentelemetry/api'; -import { metrics, NoopMeterProvider } from '@opentelemetry/api-metrics'; +import { metrics } from '@opentelemetry/api-metrics'; import { AsyncHooksContextManager, AsyncLocalStorageContextManager, } from '@opentelemetry/context-async-hooks'; import { CompositePropagator } from '@opentelemetry/core'; -import { AggregationTemporality, ConsoleMetricExporter, InMemoryMetricExporter, InstrumentType, MeterProvider, PeriodicExportingMetricReader, View } from '@opentelemetry/sdk-metrics'; +import { + AggregationTemporality, + ConsoleMetricExporter, + InMemoryMetricExporter, + InstrumentType, + MeterProvider, + PeriodicExportingMetricReader, + View, +} from '@opentelemetry/sdk-metrics'; import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; import { assertServiceResource, @@ -41,7 +49,11 @@ import * as assert from 'assert'; import * as semver from 'semver'; import * as Sinon from 'sinon'; import { NodeSDK } from '../src'; -import {envDetector, processDetector, Resource} from '@opentelemetry/resources'; +import { + envDetector, + processDetector, + Resource +} from '@opentelemetry/resources'; const DefaultContextManager = semver.gte(process.version, '14.8.0') @@ -76,7 +88,7 @@ describe('Node SDK', () => { assert.strictEqual(propagation['_getGlobalPropagator'](), propagator, 'propagator should not change'); assert.strictEqual((trace.getTracerProvider() as ProxyTracerProvider).getDelegate(), delegate, 'tracer provider should not have changed'); - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider)); }); it('should register a tracer provider if an exporter is provided', async () => { @@ -87,7 +99,7 @@ describe('Node SDK', () => { await sdk.start(); - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider)); assert.ok( context['_getContextManager']().constructor.name === DefaultContextManager.name @@ -110,7 +122,7 @@ describe('Node SDK', () => { await sdk.start(); - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider)); assert.ok( context['_getContextManager']().constructor.name === DefaultContextManager.name