fix: observers should not expose bind/unbind method

Author: legendecas

packages/opentelemetry-api/src/metrics/BoundInstrument.ts

@@ -16,7 +16,6 @@
 
 import { CorrelationContext } from '../correlation_context/CorrelationContext';
 import { SpanContext } from '../trace/span_context';
-import { ObserverResult } from './ObserverResult';
 
 /** An Instrument for Counter Metric. */
 export interface BoundCounter {
@@ -45,14 +44,3 @@ export interface BoundMeasure {
     spanContext: SpanContext
   ): void;
 }
-
-/** Base interface for the Observer metrics. */
-export interface BoundObserver {
-  /**
-   * Sets callback for the observer. The callback is called once and then it
-   * sets observers for values. The observers are called periodically to
-   * retrieve the value.
-   * @param callback
-   */
-  setCallback(callback: (observerResult: ObserverResult) => void): void;
-}

packages/opentelemetry-api/src/metrics/Meter.ts

@@ -14,8 +14,7 @@
  * limitations under the License.
  */
 
-import { Metric, MetricOptions } from './Metric';
-import { BoundCounter, BoundMeasure, BoundObserver } from './BoundInstrument';
+import { MetricOptions, Counter, Measure, Observer } from './Metric';
 
 /**
  * An interface to allow the recording metrics.
@@ -30,7 +29,7 @@ export interface Meter {
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createMeasure(name: string, options?: MetricOptions): Metric<BoundMeasure>;
+  createMeasure(name: string, options?: MetricOptions): Measure;
 
   /**
    * Creates a new `Counter` metric. Generally, this kind of metric when the
@@ -39,12 +38,12 @@ export interface Meter {
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createCounter(name: string, options?: MetricOptions): Metric<BoundCounter>;
+  createCounter(name: string, options?: MetricOptions): Counter;
 
   /**
    * Creates a new `Observer` metric.
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createObserver(name: string, options?: MetricOptions): Metric<BoundObserver>;
+  createObserver(name: string, options?: MetricOptions): Observer;
 }

packages/opentelemetry-api/src/metrics/Metric.ts

@@ -17,6 +17,7 @@
 import { CorrelationContext } from '../correlation_context/CorrelationContext';
 import { SpanContext } from '../trace/span_context';
 import { ObserverResult } from './ObserverResult';
+import { BoundCounter, BoundMeasure } from './BoundInstrument';
 
 /**
  * Options needed for metric creation
@@ -77,7 +78,18 @@ export enum ValueType {
  * Metric represents a base class for different types of metric
  * pre aggregations.
  */
-export interface Metric<T> {
+export interface Metric {
+  /**
+   * Clears all bound instruments from the Metric.
+   */
+  clear(): void;
+}
+
+/**
+ * UnboundMetric represents a base class for different types of metric
+ * pre aggregations without label value bound yet.
+ */
+export interface UnboundMetric<T> extends Metric {
   /**
    * Returns a Instrument associated with specified Labels.
    * It is recommended to keep a reference to the Instrument instead of always
@@ -92,31 +104,16 @@ export interface Metric<T> {
    * @param labels key-values pairs that are associated with a specific metric.
    */
   unbind(labels: Labels): void;
-
-  /**
-   * Clears all timeseries from the Metric.
-   */
-  clear(): void;
 }
 
-export interface MetricUtils {
+export interface Counter extends UnboundMetric<BoundCounter> {
   /**
    * Adds the given value to the current value. Values cannot be negative.
    */
   add(value: number, labels: Labels): void;
+}
 
-  /**
-   * Sets a callback where user can observe value for certain labels
-   * @param callback a function that will be called once to set observers
-   *     for values
-   */
-  setCallback(callback: (observerResult: ObserverResult) => void): void;
-
-  /**
-   * Sets the given value. Values can be negative.
-   */
-  set(value: number, labels: Labels): void;
-
+export interface Measure extends UnboundMetric<BoundMeasure> {
   /**
    * Records the given value to this measure.
    */
@@ -136,6 +133,17 @@ export interface MetricUtils {
   ): void;
 }
 
+/** Base interface for the Observer metrics. */
+export interface Observer extends Metric {
+  /**
+   * Sets a callback where user can observe value for certain labels. The
+   * observers are called periodically to retrieve the value.
+   * @param callback a function that will be called once to set observers
+   *     for values
+   */
+  setCallback(callback: (observerResult: ObserverResult) => void): void;
+}
+
 /**
  * key-value pairs passed by the user.
  */

packages/opentelemetry-api/src/metrics/NoopMeter.ts

@@ -15,8 +15,15 @@
  */
 
 import { Meter } from './Meter';
-import { MetricOptions, Metric, Labels, MetricUtils } from './Metric';
-import { BoundMeasure, BoundCounter, BoundObserver } from './BoundInstrument';
+import {
+  MetricOptions,
+  UnboundMetric,
+  Labels,
+  Counter,
+  Measure,
+  Observer,
+} from './Metric';
+import { BoundMeasure, BoundCounter } from './BoundInstrument';
 import { CorrelationContext } from '../correlation_context/CorrelationContext';
 import { SpanContext } from '../trace/span_context';
 import { ObserverResult } from './ObserverResult';
@@ -33,7 +40,7 @@ export class NoopMeter implements Meter {
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createMeasure(name: string, options?: MetricOptions): Metric<BoundMeasure> {
+  createMeasure(name: string, options?: MetricOptions): Measure {
     return NOOP_MEASURE_METRIC;
   }
 
@@ -42,7 +49,7 @@ export class NoopMeter implements Meter {
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createCounter(name: string, options?: MetricOptions): Metric<BoundCounter> {
+  createCounter(name: string, options?: MetricOptions): Counter {
     return NOOP_COUNTER_METRIC;
   }
 
@@ -51,12 +58,12 @@ export class NoopMeter implements Meter {
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createObserver(name: string, options?: MetricOptions): Metric<BoundObserver> {
+  createObserver(name: string, options?: MetricOptions): Observer {
     return NOOP_OBSERVER_METRIC;
   }
 }
 
-export class NoopMetric<T> implements Metric<T> {
+export class NoopMetric<T> implements UnboundMetric<T> {
   private readonly _instrument: T;
 
   constructor(instrument: T) {
@@ -90,14 +97,14 @@ export class NoopMetric<T> implements Metric<T> {
 }
 
 export class NoopCounterMetric extends NoopMetric<BoundCounter>
-  implements Pick<MetricUtils, 'add'> {
+  implements Counter {
   add(value: number, labels: Labels) {
     this.bind(labels).add(value);
   }
 }
 
 export class NoopMeasureMetric extends NoopMetric<BoundMeasure>
-  implements Pick<MetricUtils, 'record'> {
+  implements Measure {
   record(
     value: number,
     labels: Labels,
@@ -114,8 +121,7 @@ export class NoopMeasureMetric extends NoopMetric<BoundMeasure>
   }
 }
 
-export class NoopObserverMetric extends NoopMetric<BoundObserver>
-  implements Pick<MetricUtils, 'setCallback'> {
+export class NoopObserverMetric extends NoopMetric<void> implements Observer {
   setCallback(callback: (observerResult: ObserverResult) => void): void {}
 }
 
@@ -135,16 +141,11 @@ export class NoopBoundMeasure implements BoundMeasure {
   }
 }
 
-export class NoopBoundObserver implements BoundObserver {
-  setCallback(callback: (observerResult: ObserverResult) => void): void {}
-}
-
 export const NOOP_METER = new NoopMeter();
 export const NOOP_BOUND_COUNTER = new NoopBoundCounter();
 export const NOOP_COUNTER_METRIC = new NoopCounterMetric(NOOP_BOUND_COUNTER);
 
 export const NOOP_BOUND_MEASURE = new NoopBoundMeasure();
 export const NOOP_MEASURE_METRIC = new NoopMeasureMetric(NOOP_BOUND_MEASURE);
 
-export const NOOP_BOUND_OBSERVER = new NoopBoundObserver();
-export const NOOP_OBSERVER_METRIC = new NoopObserverMetric(NOOP_BOUND_OBSERVER);
+export const NOOP_OBSERVER_METRIC = new NoopObserverMetric();

packages/opentelemetry-metrics/src/BoundInstrument.ts

@@ -16,7 +16,6 @@
 
 import * as types from '@opentelemetry/api';
 import { Aggregator } from './export/types';
-import { ObserverResult } from './ObserverResult';
 
 /**
  * This class represent the base to BoundInstrument, which is responsible for generating
@@ -134,8 +133,7 @@ export class BoundMeasure extends BaseBoundInstrument
 /**
  * BoundObserver is an implementation of the {@link BoundObserver} interface.
  */
-export class BoundObserver extends BaseBoundInstrument
-  implements types.BoundObserver {
+export class BoundObserver extends BaseBoundInstrument {
   constructor(
     labels: types.Labels,
     disabled: boolean,
@@ -146,9 +144,4 @@ export class BoundObserver extends BaseBoundInstrument
   ) {
     super(labels, logger, monotonic, disabled, valueType, aggregator);
   }
-
-  setCallback(callback: (observerResult: types.ObserverResult) => void): void {
-    const observerResult = new ObserverResult();
-    callback(observerResult);
-  }
 }

packages/opentelemetry-metrics/src/Meter.ts

@@ -56,10 +56,7 @@ export class Meter implements types.Meter {
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createMeasure(
-    name: string,
-    options?: types.MetricOptions
-  ): types.Metric<types.BoundMeasure> {
+  createMeasure(name: string, options?: types.MetricOptions): types.Measure {
     if (!this._isValidName(name)) {
       this._logger.warn(
         `Invalid metric name ${name}. Defaulting to noop metric implementation.`
@@ -86,10 +83,7 @@ export class Meter implements types.Meter {
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createCounter(
-    name: string,
-    options?: types.MetricOptions
-  ): types.Metric<types.BoundCounter> {
+  createCounter(name: string, options?: types.MetricOptions): types.Counter {
     if (!this._isValidName(name)) {
       this._logger.warn(
         `Invalid metric name ${name}. Defaulting to noop metric implementation.`
@@ -113,10 +107,7 @@ export class Meter implements types.Meter {
    * @param name the name of the metric.
    * @param [options] the metric options.
    */
-  createObserver(
-    name: string,
-    options?: types.MetricOptions
-  ): types.Metric<types.BoundObserver> {
+  createObserver(name: string, options?: types.MetricOptions): types.Observer {
     if (!this._isValidName(name)) {
       this._logger.warn(
         `Invalid metric name ${name}. Defaulting to noop metric implementation.`

packages/opentelemetry-metrics/src/Metric.ts

@@ -30,7 +30,7 @@ import { hashLabels } from './Utils';
 
 /** This is a SDK implementation of {@link Metric} interface. */
 export abstract class Metric<T extends BaseBoundInstrument>
-  implements api.Metric<T> {
+  implements api.UnboundMetric<T> {
   protected readonly _monotonic: boolean;
   protected readonly _disabled: boolean;
   protected readonly _valueType: api.ValueType;
@@ -106,8 +106,7 @@ export abstract class Metric<T extends BaseBoundInstrument>
 }
 
 /** This is a SDK implementation of Counter Metric. */
-export class CounterMetric extends Metric<BoundCounter>
-  implements Pick<api.MetricUtils, 'add'> {
+export class CounterMetric extends Metric<BoundCounter> implements api.Counter {
   constructor(
     name: string,
     options: MetricOptions,
@@ -139,8 +138,7 @@ export class CounterMetric extends Metric<BoundCounter>
   }
 }
 
-export class MeasureMetric extends Metric<BoundMeasure>
-  implements Pick<api.MetricUtils, 'record'> {
+export class MeasureMetric extends Metric<BoundMeasure> implements api.Measure {
   protected readonly _absolute: boolean;
 
   constructor(
@@ -172,7 +170,7 @@ export class MeasureMetric extends Metric<BoundMeasure>
 
 /** This is a SDK implementation of Observer Metric. */
 export class ObserverMetric extends Metric<BoundObserver>
-  implements Pick<api.MetricUtils, 'setCallback'> {
+  implements api.Observer {
   private _observerResult = new ObserverResult();
 
   constructor(

packages/opentelemetry-metrics/test/Batcher.test.ts

@@ -24,7 +24,7 @@ describe('Batcher', () => {
     let meter: Meter;
     let fooCounter: types.BoundCounter;
     let barCounter: types.BoundCounter;
-    let counter: types.Metric<types.BoundCounter>;
+    let counter: types.Counter;
     beforeEach(() => {
       meter = new MeterProvider({
         logger: new NoopLogger(),