From 036da8184dc366db06d28a7fd24df2ac6312327d Mon Sep 17 00:00:00 2001 From: Mayur Kale Date: Wed, 1 Apr 2020 12:36:20 -0700 Subject: [PATCH] Remove label set from metrics API (#915) * metrics: remove LabeSet API * fix: tests * fix: lint --- api/src/metrics/Meter.ts | 10 +----- api/src/metrics/Metric.ts | 33 +++++++------------ api/src/metrics/NoopMeter.ts | 33 ++++++++----------- api/src/metrics/ObserverResult.ts | 6 ++-- .../noop-implementations/noop-meter.test.ts | 14 ++++---- 5 files changed, 35 insertions(+), 61 deletions(-) diff --git a/api/src/metrics/Meter.ts b/api/src/metrics/Meter.ts index 4348370fd08..5231629c910 100644 --- a/api/src/metrics/Meter.ts +++ b/api/src/metrics/Meter.ts @@ -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'; /** @@ -47,12 +47,4 @@ export interface Meter { * @param [options] the metric options. */ createObserver(name: string, options?: MetricOptions): Metric; - - /** - * 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; } diff --git a/api/src/metrics/Metric.ts b/api/src/metrics/Metric.ts index 23f19f6af2a..d57896a1d32 100644 --- a/api/src/metrics/Metric.ts +++ b/api/src/metrics/Metric.ts @@ -79,20 +79,19 @@ export enum ValueType { */ export interface Metric { /** - * 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. @@ -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 @@ -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; @@ -140,12 +139,4 @@ export interface MetricUtils { /** * key-value pairs passed by the user. */ -export type Labels = Record; - -/** - * Canonicalized labels with an unique string identifier. - */ -export interface LabelSet { - identifier: string; - labels: Labels; -} +export type Labels = { [key: string]: string }; diff --git a/api/src/metrics/NoopMeter.ts b/api/src/metrics/NoopMeter.ts index 02d4cb2bdcd..7825863818c 100644 --- a/api/src/metrics/NoopMeter.ts +++ b/api/src/metrics/NoopMeter.ts @@ -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'; @@ -54,10 +54,6 @@ export class NoopMeter implements Meter { createObserver(name: string, options?: MetricOptions): Metric { return NOOP_OBSERVER_METRIC; } - - labels(labels: Labels): LabelSet { - return NOOP_LABEL_SET; - } } export class NoopMetric implements Metric { @@ -67,22 +63,21 @@ export class NoopMetric implements Metric { 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; } @@ -96,8 +91,8 @@ export class NoopMetric implements Metric { export class NoopCounterMetric extends NoopMetric implements Pick { - add(value: number, labelSet: LabelSet) { - this.bind(labelSet).add(value); + add(value: number, labels: Labels) { + this.bind(labels).add(value); } } @@ -105,16 +100,16 @@ export class NoopMeasureMetric extends NoopMetric implements Pick { 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); } } } @@ -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; diff --git a/api/src/metrics/ObserverResult.ts b/api/src/metrics/ObserverResult.ts index f3eb082fd45..6fb4edba70f 100644 --- a/api/src/metrics/ObserverResult.ts +++ b/api/src/metrics/ObserverResult.ts @@ -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; - observe(callback: Function, labelSet: LabelSet): void; + observers: Map; + observe(callback: Function, labels: Labels): void; } diff --git a/api/test/noop-implementations/noop-meter.test.ts b/api/test/noop-implementations/noop-meter.test.ts index c3d03884364..7b86f5d59d5 100644 --- a/api/test/noop-implementations/noop-meter.test.ts +++ b/api/test/noop-implementations/noop-meter.test.ts @@ -16,7 +16,6 @@ import * as assert from 'assert'; import { - Labels, NoopMeterProvider, NOOP_BOUND_COUNTER, NOOP_BOUND_MEASURE, @@ -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',