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 <[email protected]>
open-telemetry · Sep 5, 2022 · d3adc22 · d3adc22
1 parent 4e123b5
commit d3adc22
Show file tree

Hide file tree

Showing 10 changed files with 117 additions and 26 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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