Skip to content

Commit

Permalink
fix: observers should not expose bind/unbind method (open-telemetry#1001
Browse files Browse the repository at this point in the history
)
  • Loading branch information
legendecas authored and dyladan committed Feb 18, 2021
1 parent 69357a8 commit ea6564f
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 52 deletions.
12 changes: 0 additions & 12 deletions api/src/metrics/BoundInstrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
9 changes: 4 additions & 5 deletions api/src/metrics/Meter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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;
}
46 changes: 27 additions & 19 deletions api/src/metrics/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
*/
Expand All @@ -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.
*/
Expand Down
33 changes: 17 additions & 16 deletions api/src/metrics/NoopMeter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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 {}
}

Expand All @@ -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();

0 comments on commit ea6564f

Please sign in to comment.