feat(sdk-metrics)!: extract IMetricReader interface and use it over abstract class

CHANGELOG.md

@@ -26,6 +26,9 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
 * refactor(sdk-trace-base)!: remove `new Span` constructor in favor of `Tracer.startSpan` API [#5048](https://github.com/open-telemetry/opentelemetry-js/pull/5048) @david-luna
 * refactor(sdk-trace-base)!: remove `BasicTracerProvider.addSpanProcessor` API in favor of constructor options. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna
 * refactor(sdk-trace-base)!: make `resource` property private in `BasicTracerProvider` and remove `getActiveSpanProcessor` API. [#5192](https://github.com/open-telemetry/opentelemetry-js/pull/5192) @david-luna
+* feat(sdk-metrics)!: extract `IMetricReader` interface and use it over abstract class [#5311](https://github.com/open-telemetry/opentelemetry-js/pull/5311)
+  * (user-facing): `MeterProviderOptions` now provides the more general `IMetricReader` type over `MetricReader`
+  * If you accept `MetricReader` in your public interface, consider accepting the more general `IMetricReader` instead to avoid unintentional breaking changes
 * feat(sdk-trace)!: remove ability to have BasicTracerProvider instantiate exporters [#5239](https://github.com/open-telemetry/opentelemetry-js/pull/5239) @pichlermarc
   * When extending `BasicTracerProvider`, the class offered multiple methods to facilitate the creation of exporters and auto-pairing with `SpanProcessor`s.
     * This functionality has been removed - users may now pass `SpanProcessor`s to the base class constructor when extending

experimental/CHANGELOG.md

@@ -13,6 +13,8 @@ All notable changes to experimental packages in this project will be documented
   * The internal OpenTelemetry data model dropped the concept of instrument type on exported metrics, therefore mapping it is not necessary anymore.
 * feat(instrumentation-fetch)!: passthrough original response to `applyCustomAttributes` hook [#5281](https://github.com/open-telemetry/opentelemetry-js/pull/5281) @chancancode
   * Previously, the fetch instrumentation code unconditionally clones every `fetch()` response in order to preserve the ability for the `applyCustomAttributes` hook to consume the response body. This is fundamentally unsound, as it forces the browser to buffer and retain the response body until it is fully received and read, which crates unnecessary memory pressure on large or long-running response streams. In extreme cases, this is effectively a memory leak and can cause the browser tab to crash. If your use case for `applyCustomAttributes` requires access to the response body, please chime in on [#5293](https://github.com/open-telemetry/opentelemetry-js/issues/5293).
+* feat(sdk-node)!: use `IMetricReader` over `MetricReader` [#5311](https://github.com/open-telemetry/opentelemetry-js/pull/5311)
+  * (user-facing): `NodeSDKConfiguration` now provides the more general `IMetricReader` type over `MetricReader`
 
 ### :rocket: (Enhancement)
 

experimental/packages/opentelemetry-sdk-node/src/sdk.ts

@@ -54,7 +54,7 @@ import { OTLPMetricExporter as OTLPHttpMetricExporter } from '@opentelemetry/exp
 import { PrometheusExporter as PrometheusMetricExporter } from '@opentelemetry/exporter-prometheus';
 import {
   MeterProvider,
-  MetricReader,
+  IMetricReader,
   ViewOptions,
   ConsoleMetricExporter,
   PeriodicExportingMetricReader,
@@ -82,7 +82,7 @@ export type MeterProviderConfig = {
   /**
    * Reference to the MetricReader instance by the NodeSDK
    */
-  reader?: MetricReader;
+  reader?: IMetricReader;
   /**
    * List of {@link ViewOptions}s that should be passed to the MeterProvider
    */
@@ -107,8 +107,8 @@ function getValueInMillis(envName: string, defaultValue: number): number {
  *
  * @returns MetricReader[] if appropriate environment variables are configured
  */
-function configureMetricProviderFromEnv(): MetricReader[] {
-  const metricReaders: MetricReader[] = [];
+function configureMetricProviderFromEnv(): IMetricReader[] {
+  const metricReaders: IMetricReader[] = [];
   const metricsExporterList = process.env.OTEL_METRICS_EXPORTER?.trim();
   if (!metricsExporterList) {
     return metricReaders;
@@ -395,16 +395,16 @@ export class NodeSDK {
       logs.setGlobalLoggerProvider(loggerProvider);
     }
 
-    const metricReadersFromEnv: MetricReader[] =
+    const metricReadersFromEnv: IMetricReader[] =
       configureMetricProviderFromEnv();
     if (this._meterProviderConfig || metricReadersFromEnv.length > 0) {
-      const readers: MetricReader[] = [];
+      const readers: IMetricReader[] = [];
       if (this._meterProviderConfig?.reader) {
         readers.push(this._meterProviderConfig.reader);
       }
 
       if (readers.length === 0) {
-        metricReadersFromEnv.forEach((r: MetricReader) => readers.push(r));
+        metricReadersFromEnv.forEach((r: IMetricReader) => readers.push(r));
       }
 
       const meterProvider = new MeterProvider({

experimental/packages/opentelemetry-sdk-node/src/types.ts

@@ -19,7 +19,7 @@ import { TextMapPropagator } from '@opentelemetry/api';
 import { Instrumentation } from '@opentelemetry/instrumentation';
 import { Detector, DetectorSync, IResource } from '@opentelemetry/resources';
 import { LogRecordProcessor } from '@opentelemetry/sdk-logs';
-import { MetricReader, ViewOptions } from '@opentelemetry/sdk-metrics';
+import { IMetricReader, ViewOptions } from '@opentelemetry/sdk-metrics';
 import {
   Sampler,
   SpanExporter,
@@ -35,7 +35,7 @@ export interface NodeSDKConfiguration {
   /** @deprecated use logRecordProcessors instead*/
   logRecordProcessor: LogRecordProcessor;
   logRecordProcessors?: LogRecordProcessor[];
-  metricReader: MetricReader;
+  metricReader: IMetricReader;
   views: ViewOptions[];
   instrumentations: (Instrumentation | Instrumentation[])[];
   resource: IResource;

packages/sdk-metrics/src/MeterProvider.ts

@@ -22,7 +22,7 @@ import {
   createNoopMeter,
 } from '@opentelemetry/api';
 import { IResource, Resource } from '@opentelemetry/resources';
-import { MetricReader } from './export/MetricReader';
+import { IMetricReader } from './export/MetricReader';
 import { MeterProviderSharedState } from './state/MeterProviderSharedState';
 import { MetricCollector } from './state/MetricCollector';
 import { ForceFlushOptions, ShutdownOptions } from './types';
@@ -35,7 +35,7 @@ export interface MeterProviderOptions {
   /** Resource associated with metric telemetry  */
   resource?: IResource;
   views?: ViewOptions[];
-  readers?: MetricReader[];
+  readers?: IMetricReader[];
 }
 
 /**

packages/sdk-metrics/src/export/MetricProducer.ts

@@ -29,7 +29,7 @@ export interface MetricCollectOptions {
 }
 
 /**
- * This is a public interface that represent an export state of a MetricReader.
+ * This is a public interface that represent an export state of a IMetricReader.
  */
 export interface MetricProducer {
   /**

packages/sdk-metrics/src/export/MetricReader.ts

@@ -18,7 +18,7 @@ import * as api from '@opentelemetry/api';
 import { AggregationTemporality } from './AggregationTemporality';
 import { MetricProducer } from './MetricProducer';
 import { CollectionResult, InstrumentType } from './MetricData';
-import { FlatMap, callWithTimeout } from '../utils';
+import { callWithTimeout, FlatMap } from '../utils';
 import {
   CollectionOptions,
   ForceFlushOptions,
@@ -38,16 +38,22 @@ export interface MetricReaderOptions {
    * Aggregation selector based on metric instrument types. If no views are
    * configured for a metric instrument, a per-metric-reader aggregation is
    * selected with this selector.
+   *
+   * <p> NOTE: the provided function MUST be pure
    */
   aggregationSelector?: AggregationSelector;
   /**
    * Aggregation temporality selector based on metric instrument types. If
    * not configured, cumulative is used for all instruments.
+   *
+   * <p> NOTE: the provided function MUST be pure
    */
   aggregationTemporalitySelector?: AggregationTemporalitySelector;
   /**
    * Cardinality selector based on metric instrument types. If not configured,
    * a default value is used.
+   *
+   * <p> NOTE: the provided function MUST be pure
    */
   cardinalitySelector?: CardinalitySelector;
   /**
@@ -59,11 +65,75 @@ export interface MetricReaderOptions {
   metricProducers?: MetricProducer[];
 }
 
+/**
+ * Reads metrics from the SDK. Implementations MUST follow the Metric Reader Specification as well as the requirements
+ * listed in this interface. Consider extending {@link MetricReader} to get a specification-compliant base implementation
+ * of this interface
+ */
+export interface IMetricReader {
+  /**
+   * Set the {@link MetricProducer} used by this instance. **This should only be called once by the
+   * SDK and should be considered internal.**
+   *
+   * <p> NOTE: implementations MUST throw when called more than once
+   *
+   * @param metricProducer
+   */
+  setMetricProducer(metricProducer: MetricProducer): void;
+
+  /**
+   * Select the {@link AggregationOption} for the given {@link InstrumentType} for this
+   * reader.
+   *
+   * <p> NOTE: implementations MUST be pure
+   */
+  selectAggregation(instrumentType: InstrumentType): AggregationOption;
+
+  /**
+   * Select the {@link AggregationTemporality} for the given
+   * {@link InstrumentType} for this reader.
+   *
+   * <p> NOTE: implementations MUST be pure
+   */
+  selectAggregationTemporality(
+    instrumentType: InstrumentType
+  ): AggregationTemporality;
+
+  /**
+   * Select the cardinality limit for the given {@link InstrumentType} for this
+   * reader.
+   *
+   * <p> NOTE: implementations MUST be pure
+   */
+  selectCardinalityLimit(instrumentType: InstrumentType): number;
+
+  /**
+   * Collect all metrics from the associated {@link MetricProducer}
+   */
+  collect(options?: CollectionOptions): Promise<CollectionResult>;
+
+  /**
+   * Shuts down the metric reader, the promise will reject after the optional timeout or resolve after completion.
+   *
+   * <p> NOTE: this operation MAY continue even after the promise rejects due to a timeout.
+   * @param options options with timeout.
+   */
+  shutdown(options?: ShutdownOptions): Promise<void>;
+
+  /**
+   * Flushes metrics read by this reader, the promise will reject after the optional timeout or resolve after completion.
+   *
+   * <p> NOTE: this operation MAY continue even after the promise rejects due to a timeout.
+   * @param options options with timeout.
+   */
+  forceFlush(options?: ForceFlushOptions): Promise<void>;
+}
+
 /**
  * A registered reader of metrics that, when linked to a {@link MetricProducer}, offers global
  * control over metrics.
  */
-export abstract class MetricReader {
+export abstract class MetricReader implements IMetricReader {
   // Tracks the shutdown state.
   // TODO: use BindOncePromise here once a new version of @opentelemetry/core is available.
   private _shutdown = false;
@@ -85,16 +155,6 @@ export abstract class MetricReader {
     this._cardinalitySelector = options?.cardinalitySelector;
   }
 
-  /**
-   * Set the {@link MetricProducer} used by this instance. **This should only be called by the
-   * SDK and should be considered internal.**
-   *
-   * To add additional {@link MetricProducer}s to a {@link MetricReader}, pass them to the
-   * constructor as {@link MetricReaderOptions.metricProducers}.
-   *
-   * @internal
-   * @param metricProducer
-   */
   setMetricProducer(metricProducer: MetricProducer) {
     if (this._sdkMetricProducer) {
       throw new Error(
@@ -105,28 +165,16 @@ export abstract class MetricReader {
     this.onInitialized();
   }
 
-  /**
-   * Select the {@link AggregationOption} for the given {@link InstrumentType} for this
-   * reader.
-   */
   selectAggregation(instrumentType: InstrumentType): AggregationOption {
     return this._aggregationSelector(instrumentType);
   }
 
-  /**
-   * Select the {@link AggregationTemporality} for the given
-   * {@link InstrumentType} for this reader.
-   */
   selectAggregationTemporality(
     instrumentType: InstrumentType
   ): AggregationTemporality {
     return this._aggregationTemporalitySelector(instrumentType);
   }
 
-  /**
-   * Select the cardinality limit for the given {@link InstrumentType} for this
-   * reader.
-   */
   selectCardinalityLimit(instrumentType: InstrumentType): number {
     return this._cardinalitySelector
       ? this._cardinalitySelector(instrumentType)
@@ -158,9 +206,6 @@ export abstract class MetricReader {
    */
   protected abstract onForceFlush(): Promise<void>;
 
-  /**
-   * Collect all metrics from the associated {@link MetricProducer}
-   */
   async collect(options?: CollectionOptions): Promise<CollectionResult> {
     if (this._sdkMetricProducer === undefined) {
       throw new Error('MetricReader is not bound to a MetricProducer');
@@ -204,12 +249,6 @@ export abstract class MetricReader {
     };
   }
 
-  /**
-   * Shuts down the metric reader, the promise will reject after the optional timeout or resolve after completion.
-   *
-   * <p> NOTE: this operation will continue even after the promise rejects due to a timeout.
-   * @param options options with timeout.
-   */
   async shutdown(options?: ShutdownOptions): Promise<void> {
     // Do not call shutdown again if it has already been called.
     if (this._shutdown) {
@@ -227,12 +266,6 @@ export abstract class MetricReader {
     this._shutdown = true;
   }
 
-  /**
-   * Flushes metrics read by this reader, the promise will reject after the optional timeout or resolve after completion.
-   *
-   * <p> NOTE: this operation will continue even after the promise rejects due to a timeout.
-   * @param options options with timeout.
-   */
   async forceFlush(options?: ForceFlushOptions): Promise<void> {
     if (this._shutdown) {
       api.diag.warn('Cannot forceFlush on already shutdown MetricReader.');

packages/sdk-metrics/src/index.ts

@@ -45,7 +45,11 @@ export {
 
 export { PushMetricExporter } from './export/MetricExporter';
 
-export { MetricReader, MetricReaderOptions } from './export/MetricReader';
+export {
+  IMetricReader,
+  MetricReader,
+  MetricReaderOptions,
+} from './export/MetricReader';
 
 export {
   PeriodicExportingMetricReader,

packages/sdk-metrics/src/state/MetricCollector.ts

@@ -21,8 +21,8 @@ import {
   InstrumentType,
   ScopeMetrics,
 } from '../export/MetricData';
-import { MetricProducer, MetricCollectOptions } from '../export/MetricProducer';
-import { MetricReader } from '../export/MetricReader';
+import { MetricCollectOptions, MetricProducer } from '../export/MetricProducer';
+import { IMetricReader } from '../export/MetricReader';
 import { ForceFlushOptions, ShutdownOptions } from '../types';
 import { MeterProviderSharedState } from './MeterProviderSharedState';
 
@@ -34,7 +34,7 @@ import { MeterProviderSharedState } from './MeterProviderSharedState';
 export class MetricCollector implements MetricProducer {
   constructor(
     private _sharedState: MeterProviderSharedState,
-    private _metricReader: MetricReader
+    private _metricReader: IMetricReader
   ) {}
 
   async collect(options?: MetricCollectOptions): Promise<CollectionResult> {

packages/sdk-metrics/test/Instruments.test.ts

@@ -24,7 +24,6 @@ import {
   Histogram,
   InstrumentType,
   MeterProvider,
-  MetricReader,
 } from '../src';
 import { InstrumentDescriptor } from '../src/InstrumentDescriptor';
 import {
@@ -40,6 +39,7 @@ import {
   defaultResource,
 } from './util';
 import { ObservableResult, ValueType } from '@opentelemetry/api';
+import { IMetricReader } from '../src/export/MetricReader';
 
 describe('Instruments', () => {
   describe('Counter', () => {
@@ -845,7 +845,7 @@ interface ValidateMetricData {
 }
 
 async function validateExport(
-  reader: MetricReader,
+  reader: IMetricReader,
   expected: ValidateMetricData
 ) {
   const { resourceMetrics, errors } = await reader.collect();

packages/sdk-metrics/test/export/utils.ts

@@ -18,7 +18,7 @@ import {
   AggregationSelector,
   AggregationTemporalitySelector,
   InstrumentType,
-  MetricReader,
+  IMetricReader,
   PushMetricExporter,
 } from '../../src';
 import * as assert from 'assert';
@@ -39,7 +39,7 @@ const instrumentTypes = [
  * @param expectedSelector
  */
 export function assertAggregationSelector(
-  reader: MetricReader | PushMetricExporter,
+  reader: IMetricReader | PushMetricExporter,
   expectedSelector: AggregationSelector
 ) {
   for (const instrumentType of instrumentTypes) {
@@ -57,7 +57,7 @@ export function assertAggregationSelector(
  * @param expectedSelector
  */
 export function assertAggregationTemporalitySelector(
-  reader: MetricReader | PushMetricExporter,
+  reader: IMetricReader | PushMetricExporter,
   expectedSelector: AggregationTemporalitySelector
 ) {
   for (const instrumentType of instrumentTypes) {