Skip to content

Commit

Permalink
fix: observers should not expose bind/unbind method
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas committed Apr 30, 2020
1 parent 23677a1 commit 0b4c2c6
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 68 deletions.
12 changes: 0 additions & 12 deletions packages/opentelemetry-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;
}
14 changes: 9 additions & 5 deletions packages/opentelemetry-api/src/metrics/Meter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@
* limitations under the License.
*/

import { Metric, MetricOptions } from './Metric';
import { BoundCounter, BoundMeasure, BoundObserver } from './BoundInstrument';
import {
MetricOptions,
UnboundCounterMetric,
UnboundMeasureMetric,
ObserverMetric,
} from './Metric';

/**
* An interface to allow the recording metrics.
Expand All @@ -30,7 +34,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): UnboundMeasureMetric;

/**
* Creates a new `Counter` metric. Generally, this kind of metric when the
Expand All @@ -39,12 +43,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): UnboundCounterMetric;

/**
* 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): ObserverMetric;
}
42 changes: 23 additions & 19 deletions packages/opentelemetry-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,14 @@ export enum ValueType {
* Metric represents a base class for different types of metric
* pre aggregations.
*/
export interface Metric<T> {
export interface Metric {
/**
* Clears all timeseries from the Metric.
*/
clear(): void;
}

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 +100,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 UnboundCounterMetric 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 UnboundMeasureMetric extends UnboundMetric<BoundMeasure> {
/**
* Records the given value to this measure.
*/
Expand All @@ -136,6 +129,17 @@ export interface MetricUtils {
): void;
}

/** Base interface for the Observer metrics. */
export interface ObserverMetric 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
34 changes: 18 additions & 16 deletions packages/opentelemetry-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,
UnboundCounterMetric,
UnboundMeasureMetric,
ObserverMetric,
} 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): UnboundMeasureMetric {
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): UnboundCounterMetric {
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): ObserverMetric {
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 UnboundCounterMetric {
add(value: number, labels: Labels) {
this.bind(labels).add(value);
}
}

export class NoopMeasureMetric extends NoopMetric<BoundMeasure>
implements Pick<MetricUtils, 'record'> {
implements UnboundMeasureMetric {
record(
value: number,
labels: Labels,
Expand All @@ -114,8 +121,8 @@ export class NoopMeasureMetric extends NoopMetric<BoundMeasure>
}
}

export class NoopObserverMetric extends NoopMetric<BoundObserver>
implements Pick<MetricUtils, 'setCallback'> {
export class NoopObserverMetric extends NoopMetric<void>
implements ObserverMetric {
setCallback(callback: (observerResult: ObserverResult) => void): void {}
}

Expand All @@ -135,16 +142,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();
9 changes: 1 addition & 8 deletions packages/opentelemetry-metrics/src/BoundInstrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
}
6 changes: 3 additions & 3 deletions packages/opentelemetry-metrics/src/Meter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class Meter implements types.Meter {
createMeasure(
name: string,
options?: types.MetricOptions
): types.Metric<types.BoundMeasure> {
): types.UnboundMeasureMetric {
if (!this._isValidName(name)) {
this._logger.warn(
`Invalid metric name ${name}. Defaulting to noop metric implementation.`
Expand Down Expand Up @@ -89,7 +89,7 @@ export class Meter implements types.Meter {
createCounter(
name: string,
options?: types.MetricOptions
): types.Metric<types.BoundCounter> {
): types.UnboundCounterMetric {
if (!this._isValidName(name)) {
this._logger.warn(
`Invalid metric name ${name}. Defaulting to noop metric implementation.`
Expand All @@ -116,7 +116,7 @@ export class Meter implements types.Meter {
createObserver(
name: string,
options?: types.MetricOptions
): types.Metric<types.BoundObserver> {
): types.ObserverMetric {
if (!this._isValidName(name)) {
this._logger.warn(
`Invalid metric name ${name}. Defaulting to noop metric implementation.`
Expand Down
8 changes: 4 additions & 4 deletions packages/opentelemetry-metrics/src/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -107,7 +107,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'> {
implements api.UnboundCounterMetric {
constructor(
name: string,
options: MetricOptions,
Expand Down Expand Up @@ -140,7 +140,7 @@ export class CounterMetric extends Metric<BoundCounter>
}

export class MeasureMetric extends Metric<BoundMeasure>
implements Pick<api.MetricUtils, 'record'> {
implements api.UnboundMeasureMetric {
protected readonly _absolute: boolean;

constructor(
Expand Down Expand Up @@ -172,7 +172,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.ObserverMetric {
private _observerResult = new ObserverResult();

constructor(
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-metrics/test/Batcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.UnboundMetric<types.BoundCounter>;
beforeEach(() => {
meter = new MeterProvider({
logger: new NoopLogger(),
Expand Down

0 comments on commit 0b4c2c6

Please sign in to comment.