Skip to content

Commit

Permalink
Remove label set from metrics API (open-telemetry#915)
Browse files Browse the repository at this point in the history
* metrics: remove LabeSet API

* fix: tests

* fix: lint
  • Loading branch information
mayurkale22 authored and dyladan committed Feb 18, 2021
1 parent bdb4125 commit 036da81
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 61 deletions.
10 changes: 1 addition & 9 deletions api/src/metrics/Meter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { Metric, MetricOptions, Labels, LabelSet } from './Metric';
import { Metric, MetricOptions } from './Metric';
import { BoundCounter, BoundMeasure, BoundObserver } from './BoundInstrument';

/**
Expand Down Expand Up @@ -47,12 +47,4 @@ export interface Meter {
* @param [options] the metric options.
*/
createObserver(name: string, options?: MetricOptions): Metric<BoundObserver>;

/**
* Provide a pre-computed re-useable LabelSet by
* converting the unordered labels into a canonicalized
* set of labels with an unique identifier, useful for pre-aggregation.
* @param labels user provided unordered Labels.
*/
labels(labels: Labels): LabelSet;
}
33 changes: 12 additions & 21 deletions api/src/metrics/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,19 @@ export enum ValueType {
*/
export interface Metric<T> {
/**
* Returns a Instrument associated with specified LabelSet.
* Returns a Instrument associated with specified Labels.
* It is recommended to keep a reference to the Instrument instead of always
* calling this method for every operations.
* @param labels the canonicalized LabelSet used to associate with this
* metric instrument.
* @param labels key-values pairs that are associated with a specific metric
* that you want to record.
*/
bind(labels: LabelSet): T;
bind(labels: Labels): T;

/**
* Removes the Instrument from the metric, if it is present.
* @param labels the canonicalized LabelSet used to associate with this
* metric instrument.
* @param labels key-values pairs that are associated with a specific metric.
*/
unbind(labels: LabelSet): void;
unbind(labels: Labels): void;

/**
* Clears all timeseries from the Metric.
Expand All @@ -104,7 +103,7 @@ export interface MetricUtils {
/**
* Adds the given value to the current value. Values cannot be negative.
*/
add(value: number, labelSet: LabelSet): void;
add(value: number, labels: Labels): void;

/**
* Sets a callback where user can observe value for certain labels
Expand All @@ -116,22 +115,22 @@ export interface MetricUtils {
/**
* Sets the given value. Values can be negative.
*/
set(value: number, labelSet: LabelSet): void;
set(value: number, labels: Labels): void;

/**
* Records the given value to this measure.
*/
record(value: number, labelSet: LabelSet): void;
record(value: number, labels: Labels): void;

record(
value: number,
labelSet: LabelSet,
labels: Labels,
correlationContext: CorrelationContext
): void;

record(
value: number,
labelSet: LabelSet,
labels: Labels,
correlationContext: CorrelationContext,
spanContext: SpanContext
): void;
Expand All @@ -140,12 +139,4 @@ export interface MetricUtils {
/**
* key-value pairs passed by the user.
*/
export type Labels = Record<string, string>;

/**
* Canonicalized labels with an unique string identifier.
*/
export interface LabelSet {
identifier: string;
labels: Labels;
}
export type Labels = { [key: string]: string };
33 changes: 13 additions & 20 deletions api/src/metrics/NoopMeter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { Meter } from './Meter';
import { MetricOptions, Metric, Labels, LabelSet, MetricUtils } from './Metric';
import { MetricOptions, Metric, Labels, MetricUtils } from './Metric';
import { BoundMeasure, BoundCounter, BoundObserver } from './BoundInstrument';
import { CorrelationContext } from '../correlation_context/CorrelationContext';
import { SpanContext } from '../trace/span_context';
Expand Down Expand Up @@ -54,10 +54,6 @@ export class NoopMeter implements Meter {
createObserver(name: string, options?: MetricOptions): Metric<BoundObserver> {
return NOOP_OBSERVER_METRIC;
}

labels(labels: Labels): LabelSet {
return NOOP_LABEL_SET;
}
}

export class NoopMetric<T> implements Metric<T> {
Expand All @@ -67,22 +63,21 @@ export class NoopMetric<T> implements Metric<T> {
this._instrument = instrument;
}
/**
* Returns a Bound Instrument associated with specified LabelSet.
* Returns a Bound Instrument associated with specified Labels.
* It is recommended to keep a reference to the Bound Instrument instead of
* always calling this method for every operations.
* @param labels the canonicalized LabelSet used to associate with this
* metric instrument.
* @param labels key-values pairs that are associated with a specific metric
* that you want to record.
*/
bind(labels: LabelSet): T {
bind(labels: Labels): T {
return this._instrument;
}

/**
* Removes the Binding from the metric, if it is present.
* @param labels the canonicalized LabelSet used to associate with this
* metric instrument.
* @param labels key-values pairs that are associated with a specific metric.
*/
unbind(labels: LabelSet): void {
unbind(labels: Labels): void {
return;
}

Expand All @@ -96,25 +91,25 @@ export class NoopMetric<T> implements Metric<T> {

export class NoopCounterMetric extends NoopMetric<BoundCounter>
implements Pick<MetricUtils, 'add'> {
add(value: number, labelSet: LabelSet) {
this.bind(labelSet).add(value);
add(value: number, labels: Labels) {
this.bind(labels).add(value);
}
}

export class NoopMeasureMetric extends NoopMetric<BoundMeasure>
implements Pick<MetricUtils, 'record'> {
record(
value: number,
labelSet: LabelSet,
labels: Labels,
correlationContext?: CorrelationContext,
spanContext?: SpanContext
) {
if (typeof correlationContext === 'undefined') {
this.bind(labelSet).record(value);
this.bind(labels).record(value);
} else if (typeof spanContext === 'undefined') {
this.bind(labelSet).record(value, correlationContext);
this.bind(labels).record(value, correlationContext);
} else {
this.bind(labelSet).record(value, correlationContext, spanContext);
this.bind(labels).record(value, correlationContext, spanContext);
}
}
}
Expand Down Expand Up @@ -153,5 +148,3 @@ 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_LABEL_SET = {} as LabelSet;
6 changes: 3 additions & 3 deletions api/src/metrics/ObserverResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
* limitations under the License.
*/

import { LabelSet } from './Metric';
import { Labels } from './Metric';

/**
* Interface that is being used in function setCallback for Observer Metric
*/
export interface ObserverResult {
observers: Map<LabelSet, Function>;
observe(callback: Function, labelSet: LabelSet): void;
observers: Map<Labels, Function>;
observe(callback: Function, labels: Labels): void;
}
14 changes: 6 additions & 8 deletions api/test/noop-implementations/noop-meter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import * as assert from 'assert';
import {
Labels,
NoopMeterProvider,
NOOP_BOUND_COUNTER,
NOOP_BOUND_MEASURE,
Expand All @@ -28,24 +27,23 @@ describe('NoopMeter', () => {
it('should not crash', () => {
const meter = new NoopMeterProvider().getMeter('test-noop');
const counter = meter.createCounter('some-name');
const labels = {} as Labels;
const labelSet = meter.labels(labels);
const labels = {};

// ensure NoopMetric does not crash.
counter.bind(labelSet).add(1);
counter.unbind(labelSet);
counter.bind(labels).add(1);
counter.unbind(labels);

// ensure the correct noop const is returned
assert.strictEqual(counter, NOOP_COUNTER_METRIC);
assert.strictEqual(counter.bind(labelSet), NOOP_BOUND_COUNTER);
assert.strictEqual(counter.bind(labels), NOOP_BOUND_COUNTER);
counter.clear();

const measure = meter.createMeasure('some-name');
measure.bind(labelSet).record(1);
measure.bind(labels).record(1);

// ensure the correct noop const is returned
assert.strictEqual(measure, NOOP_MEASURE_METRIC);
assert.strictEqual(measure.bind(labelSet), NOOP_BOUND_MEASURE);
assert.strictEqual(measure.bind(labels), NOOP_BOUND_MEASURE);

const options = {
component: 'tests',
Expand Down

0 comments on commit 036da81

Please sign in to comment.