From 3cf95a04c676976cd7bb2a97b9669747d62e8c4b Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Mon, 28 Oct 2019 14:25:59 -0700 Subject: [PATCH 01/13] feat: implement labelset --- .../src/metrics/NoopMeter.ts | 9 ++-- .../test/metrics/NoopMeter.test.ts | 13 +++--- packages/opentelemetry-metrics/src/Handle.ts | 8 ++-- packages/opentelemetry-metrics/src/Metric.ts | 24 +++++----- .../opentelemetry-metrics/src/export/types.ts | 6 +-- packages/opentelemetry-metrics/src/types.ts | 4 -- .../opentelemetry-metrics/test/Meter.test.ts | 46 +++++++++---------- .../opentelemetry-types/src/metrics/Metric.ts | 13 +++--- 8 files changed, 60 insertions(+), 63 deletions(-) diff --git a/packages/opentelemetry-core/src/metrics/NoopMeter.ts b/packages/opentelemetry-core/src/metrics/NoopMeter.ts index ccff9f1358..5cd17e93dd 100644 --- a/packages/opentelemetry-core/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-core/src/metrics/NoopMeter.ts @@ -23,6 +23,7 @@ import { MetricOptions, MeasureHandle, SpanContext, + LabelSet } from '@opentelemetry/types'; /** @@ -70,9 +71,9 @@ export class NoopMetric implements Metric { * Returns a Handle associated with specified label values. * It is recommended to keep a reference to the Handle instead of always * calling this method for every operations. - * @param labelValues the list of label values. + * @param labels the object of label set. */ - getHandle(labelValues: string[]): T { + getHandle(labels: LabelSet): T { return this._handle; } @@ -85,9 +86,9 @@ export class NoopMetric implements Metric { /** * Removes the Handle from the metric, if it is present. - * @param labelValues the list of label values. + * @param labels the object of label set. */ - removeHandle(labelValues: string[]): void { + removeHandle(labels: LabelSet): void { return; } diff --git a/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts b/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts index b37762bc7c..4999027e54 100644 --- a/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts +++ b/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts @@ -24,23 +24,25 @@ import { NOOP_MEASURE_HANDLE, NOOP_MEASURE_METRIC, } from '../../src/metrics/NoopMeter'; +import { LabelSet } from '@opentelemetry/types'; describe('NoopMeter', () => { it('should not crash', () => { const meter = new NoopMeter(); const counter = meter.createCounter('some-name'); + const labels: LabelSet = { 'key1': 'val1', 'key2': 'val2'}; // ensure NoopMetric does not crash. counter.setCallback(() => { assert.fail('callback occurred'); }); - counter.getHandle(['val1', 'val2']).add(1); + counter.getHandle(labels).add(1); counter.getDefaultHandle().add(1); - counter.removeHandle(['val1', 'val2']); + counter.removeHandle(labels); // ensure the correct noop const is returned assert.strictEqual(counter, NOOP_COUNTER_METRIC); assert.strictEqual( - counter.getHandle(['val1', 'val2']), + counter.getHandle(labels), NOOP_COUNTER_HANDLE ); assert.strictEqual(counter.getDefaultHandle(), NOOP_COUNTER_HANDLE); @@ -62,7 +64,7 @@ describe('NoopMeter', () => { assert.strictEqual(measure, NOOP_MEASURE_METRIC); assert.strictEqual(measure.getDefaultHandle(), NOOP_MEASURE_HANDLE); assert.strictEqual( - measure.getHandle(['val1', 'val2']), + measure.getHandle(labels), NOOP_MEASURE_HANDLE ); @@ -72,12 +74,11 @@ describe('NoopMeter', () => { // ensure the correct noop const is returned assert.strictEqual(gauge, NOOP_GAUGE_METRIC); assert.strictEqual(gauge.getDefaultHandle(), NOOP_GAUGE_HANDLE); - assert.strictEqual(gauge.getHandle(['val1', 'val2']), NOOP_GAUGE_HANDLE); + assert.strictEqual(gauge.getHandle(labels), NOOP_GAUGE_HANDLE); const options = { component: 'tests', description: 'the testing package', - labelKeys: ['key1', 'key2'], }; const measureWithOptions = meter.createMeasure('some-name', options); diff --git a/packages/opentelemetry-metrics/src/Handle.ts b/packages/opentelemetry-metrics/src/Handle.ts index 45bb34e390..85ef3b1e98 100644 --- a/packages/opentelemetry-metrics/src/Handle.ts +++ b/packages/opentelemetry-metrics/src/Handle.ts @@ -28,7 +28,7 @@ export class CounterHandle implements types.CounterHandle { constructor( private readonly _disabled: boolean, private readonly _monotonic: boolean, - private readonly _labelValues: string[], + private readonly _labels: types.LabelSet, private readonly _logger: types.Logger ) {} @@ -50,7 +50,7 @@ export class CounterHandle implements types.CounterHandle { */ getTimeSeries(timestamp: types.HrTime): TimeSeries { return { - labelValues: this._labelValues.map(value => ({ value })), + labelValues: Object.values(this._labels).map(value => ({ value })), points: [{ value: this._data, timestamp }], }; } @@ -66,7 +66,7 @@ export class GaugeHandle implements types.GaugeHandle { constructor( private readonly _disabled: boolean, private readonly _monotonic: boolean, - private readonly _labelValues: string[], + private readonly _labels: types.LabelSet, private readonly _logger: types.Logger ) {} @@ -88,7 +88,7 @@ export class GaugeHandle implements types.GaugeHandle { */ getTimeSeries(timestamp: types.HrTime): TimeSeries { return { - labelValues: this._labelValues.map(value => ({ value })), + labelValues: Object.values(this._labels).map(value => ({ value })), points: [{ value: this._data, timestamp }], }; } diff --git a/packages/opentelemetry-metrics/src/Metric.ts b/packages/opentelemetry-metrics/src/Metric.ts index 1daecfd88c..9befe98cb7 100644 --- a/packages/opentelemetry-metrics/src/Metric.ts +++ b/packages/opentelemetry-metrics/src/Metric.ts @@ -36,13 +36,13 @@ export abstract class Metric implements types.Metric { * Returns a Handle associated with specified label values. * It is recommended to keep a reference to the Handle instead of always * calling this method for each operation. - * @param labelValues the list of label values. + * @param labels the object of label set. */ - getHandle(labelValues: string[]): T { - const hash = hashLabelValues(labelValues); + getHandle(labels: types.LabelSet): T { + const hash = hashLabelValues(Object.values(labels)); if (this._handles.has(hash)) return this._handles.get(hash)!; - const handle = this._makeHandle(labelValues); + const handle = this._makeHandle(labels); this._handles.set(hash, handle); return handle; } @@ -58,10 +58,10 @@ export abstract class Metric implements types.Metric { /** * Removes the Handle from the metric, if it is present. - * @param labelValues the list of label values. + * @param labels the object of label set. */ - removeHandle(labelValues: string[]): void { - this._handles.delete(hashLabelValues(labelValues)); + removeHandle(labels: types.LabelSet): void { + this._handles.delete(hashLabelValues(Object.values(labels))); } /** @@ -77,7 +77,7 @@ export abstract class Metric implements types.Metric { return; } - protected abstract _makeHandle(labelValues: string[]): T; + protected abstract _makeHandle(labels: types.LabelSet): T; } /** This is a SDK implementation of Counter Metric. */ @@ -85,11 +85,11 @@ export class CounterMetric extends Metric { constructor(name: string, options: MetricOptions) { super(name, options); } - protected _makeHandle(labelValues: string[]): CounterHandle { + protected _makeHandle(labels: types.LabelSet): CounterHandle { return new CounterHandle( this._disabled, this._monotonic, - labelValues, + labels, this._logger ); } @@ -100,11 +100,11 @@ export class GaugeMetric extends Metric { constructor(name: string, options: MetricOptions) { super(name, options); } - protected _makeHandle(labelValues: string[]): GaugeHandle { + protected _makeHandle(labels: types.LabelSet): GaugeHandle { return new GaugeHandle( this._disabled, this._monotonic, - labelValues, + labels, this._logger ); } diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts index 582d3b85a4..12d31a7c0f 100644 --- a/packages/opentelemetry-metrics/src/export/types.ts +++ b/packages/opentelemetry-metrics/src/export/types.ts @@ -22,7 +22,7 @@ * opentelemetry-proto/opentelemetry/proto/metrics/v1/metrics.proto */ -import { HrTime } from '@opentelemetry/types'; +import { HrTime, LabelSet } from '@opentelemetry/types'; import { Resource, ExportResult } from '@opentelemetry/base'; export interface ReadableMetric { @@ -61,8 +61,8 @@ export interface MetricDescriptor { readonly unit: string; /** MetricDescriptor type */ readonly type: MetricDescriptorType; - /** The label keys associated with the metric descriptor. */ - readonly labelKeys: string[]; + /** The label set associated with the metric descriptor. */ + readonly labels: LabelSet; } /** diff --git a/packages/opentelemetry-metrics/src/types.ts b/packages/opentelemetry-metrics/src/types.ts index d4078bbfcf..b1e262bd5f 100644 --- a/packages/opentelemetry-metrics/src/types.ts +++ b/packages/opentelemetry-metrics/src/types.ts @@ -28,9 +28,6 @@ export interface MetricOptions { /** The unit of the Metric values. */ unit: string; - /** The list of label keys for the Metric. */ - labelKeys: string[]; - /** The map of constant labels for the Metric. */ constantLabels?: Map; @@ -63,5 +60,4 @@ export const DEFAULT_METRIC_OPTIONS = { disabled: false, description: '', unit: '1', - labelKeys: [], }; diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 5567cc2338..4a0e80f7b3 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -21,7 +21,7 @@ import { NoopLogger } from '@opentelemetry/core'; describe('Meter', () => { let meter: Meter; - const labelValues = ['value']; + const labels: types.LabelSet = {'key': 'value'}; const hrTime: types.HrTime = [22, 400000000]; beforeEach(() => { @@ -49,7 +49,7 @@ describe('Meter', () => { describe('.getHandle()', () => { it('should create a counter handle', () => { const counter = meter.createCounter('name'); - const handle = counter.getHandle(labelValues); + const handle = counter.getHandle(labels); handle.add(10); assert.strictEqual(handle['_data'], 10); handle.add(10); @@ -58,7 +58,7 @@ describe('Meter', () => { it('should return the timeseries', () => { const counter = meter.createCounter('name'); - const handle = counter.getHandle(['value1', 'value2']); + const handle = counter.getHandle({'key1': 'value1', 'key2': 'value2'}); handle.add(20); assert.deepStrictEqual(handle.getTimeSeries(hrTime), { labelValues: [{ value: 'value1' }, { value: 'value2' }], @@ -68,7 +68,7 @@ describe('Meter', () => { it('should add positive values by default', () => { const counter = meter.createCounter('name'); - const handle = counter.getHandle(labelValues); + const handle = counter.getHandle(labels); handle.add(10); assert.strictEqual(handle['_data'], 10); handle.add(-100); @@ -79,7 +79,7 @@ describe('Meter', () => { const counter = meter.createCounter('name', { disabled: true, }); - const handle = counter.getHandle(labelValues); + const handle = counter.getHandle(labels); handle.add(10); assert.strictEqual(handle['_data'], 0); }); @@ -88,16 +88,16 @@ describe('Meter', () => { const counter = meter.createCounter('name', { monotonic: false, }); - const handle = counter.getHandle(labelValues); + const handle = counter.getHandle(labels); handle.add(-10); assert.strictEqual(handle['_data'], -10); }); it('should return same handle on same label values', () => { const counter = meter.createCounter('name'); - const handle = counter.getHandle(labelValues); + const handle = counter.getHandle(labels); handle.add(10); - const handle1 = counter.getHandle(labelValues); + const handle1 = counter.getHandle(labels); handle1.add(10); assert.strictEqual(handle['_data'], 20); assert.strictEqual(handle, handle1); @@ -107,11 +107,11 @@ describe('Meter', () => { describe('.removeHandle()', () => { it('should remove a counter handle', () => { const counter = meter.createCounter('name'); - const handle = counter.getHandle(labelValues); + const handle = counter.getHandle(labels); assert.strictEqual(counter['_handles'].size, 1); - counter.removeHandle(labelValues); + counter.removeHandle(labels); assert.strictEqual(counter['_handles'].size, 0); - const handle1 = counter.getHandle(labelValues); + const handle1 = counter.getHandle(labels); assert.strictEqual(counter['_handles'].size, 1); assert.notStrictEqual(handle, handle1); }); @@ -123,7 +123,7 @@ describe('Meter', () => { it('should clear all handles', () => { const counter = meter.createCounter('name'); - counter.getHandle(labelValues); + counter.getHandle(labels); assert.strictEqual(counter['_handles'].size, 1); counter.clear(); assert.strictEqual(counter['_handles'].size, 0); @@ -150,7 +150,7 @@ describe('Meter', () => { describe('.getHandle()', () => { it('should create a gauge handle', () => { const gauge = meter.createGauge('name'); - const handle = gauge.getHandle(labelValues); + const handle = gauge.getHandle(labels); handle.set(10); assert.strictEqual(handle['_data'], 10); handle.set(250); @@ -159,7 +159,7 @@ describe('Meter', () => { it('should return the timeseries', () => { const gauge = meter.createGauge('name'); - const handle = gauge.getHandle(['v1', 'v2']); + const handle = gauge.getHandle({'k1': 'v1', 'k2': 'v2'}); handle.set(150); assert.deepStrictEqual(handle.getTimeSeries(hrTime), { labelValues: [{ value: 'v1' }, { value: 'v2' }], @@ -169,7 +169,7 @@ describe('Meter', () => { it('should go up and down by default', () => { const gauge = meter.createGauge('name'); - const handle = gauge.getHandle(labelValues); + const handle = gauge.getHandle(labels); handle.set(10); assert.strictEqual(handle['_data'], 10); handle.set(-100); @@ -180,7 +180,7 @@ describe('Meter', () => { const gauge = meter.createGauge('name', { disabled: true, }); - const handle = gauge.getHandle(labelValues); + const handle = gauge.getHandle(labels); handle.set(10); assert.strictEqual(handle['_data'], 0); }); @@ -189,16 +189,16 @@ describe('Meter', () => { const gauge = meter.createGauge('name', { monotonic: true, }); - const handle = gauge.getHandle(labelValues); + const handle = gauge.getHandle(labels); handle.set(-10); assert.strictEqual(handle['_data'], 0); }); it('should return same handle on same label values', () => { const gauge = meter.createGauge('name'); - const handle = gauge.getHandle(labelValues); + const handle = gauge.getHandle(labels); handle.set(10); - const handle1 = gauge.getHandle(labelValues); + const handle1 = gauge.getHandle(labels); handle1.set(10); assert.strictEqual(handle['_data'], 10); assert.strictEqual(handle, handle1); @@ -208,11 +208,11 @@ describe('Meter', () => { describe('.removeHandle()', () => { it('should remove the gauge handle', () => { const gauge = meter.createGauge('name'); - const handle = gauge.getHandle(labelValues); + const handle = gauge.getHandle(labels); assert.strictEqual(gauge['_handles'].size, 1); - gauge.removeHandle(labelValues); + gauge.removeHandle(labels); assert.strictEqual(gauge['_handles'].size, 0); - const handle1 = gauge.getHandle(labelValues); + const handle1 = gauge.getHandle(labels); assert.strictEqual(gauge['_handles'].size, 1); assert.notStrictEqual(handle, handle1); }); @@ -224,7 +224,7 @@ describe('Meter', () => { it('should clear all handles', () => { const gauge = meter.createGauge('name'); - gauge.getHandle(labelValues); + gauge.getHandle(labels); assert.strictEqual(gauge['_handles'].size, 1); gauge.clear(); assert.strictEqual(gauge['_handles'].size, 0); diff --git a/packages/opentelemetry-types/src/metrics/Metric.ts b/packages/opentelemetry-types/src/metrics/Metric.ts index a34bd2d2f2..d26c9340b0 100644 --- a/packages/opentelemetry-types/src/metrics/Metric.ts +++ b/packages/opentelemetry-types/src/metrics/Metric.ts @@ -33,9 +33,6 @@ export interface MetricOptions { */ unit?: string; - /** The list of label keys for the Metric. */ - labelKeys?: string[]; - /** The map of constant labels for the Metric. */ constantLabels?: Map; @@ -61,9 +58,9 @@ export interface Metric { * Returns a Handle associated with specified label values. * It is recommended to keep a reference to the Handle instead of always * calling this method for every operations. - * @param labelValues the list of label values. + * @param labels the object of label set. */ - getHandle(labelValues: string[]): T; + getHandle(labels: LabelSet): T; /** * Returns a Handle for a metric with all labels not set. @@ -72,9 +69,9 @@ export interface Metric { /** * Removes the Handle from the metric, if it is present. - * @param labelValues the list of label values. + * @param labels the object of label set. */ - removeHandle(labelValues: string[]): void; + removeHandle(labels: LabelSet): void; /** * Clears all timeseries from the Metric. @@ -86,3 +83,5 @@ export interface Metric { */ setCallback(fn: () => void): void; } + +export interface LabelSet {} From 860fe6d525af67e11082e8bb6325cb1d95775994 Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Tue, 29 Oct 2019 10:02:05 -0700 Subject: [PATCH 02/13] fix: linting --- .../opentelemetry-core/src/metrics/NoopMeter.ts | 2 +- .../test/metrics/NoopMeter.test.ts | 14 +++++--------- packages/opentelemetry-metrics/test/Meter.test.ts | 14 +++++++++++--- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/opentelemetry-core/src/metrics/NoopMeter.ts b/packages/opentelemetry-core/src/metrics/NoopMeter.ts index 5cd17e93dd..b154b8396a 100644 --- a/packages/opentelemetry-core/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-core/src/metrics/NoopMeter.ts @@ -23,7 +23,7 @@ import { MetricOptions, MeasureHandle, SpanContext, - LabelSet + LabelSet, } from '@opentelemetry/types'; /** diff --git a/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts b/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts index 4999027e54..72d6064437 100644 --- a/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts +++ b/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts @@ -30,7 +30,9 @@ describe('NoopMeter', () => { it('should not crash', () => { const meter = new NoopMeter(); const counter = meter.createCounter('some-name'); - const labels: LabelSet = { 'key1': 'val1', 'key2': 'val2'}; + const key1 = 'key1'; + const key2 = 'key2'; + const labels: LabelSet = { [key1]: 'val1', [key2]: 'val2' }; // ensure NoopMetric does not crash. counter.setCallback(() => { assert.fail('callback occurred'); @@ -41,10 +43,7 @@ describe('NoopMeter', () => { // ensure the correct noop const is returned assert.strictEqual(counter, NOOP_COUNTER_METRIC); - assert.strictEqual( - counter.getHandle(labels), - NOOP_COUNTER_HANDLE - ); + assert.strictEqual(counter.getHandle(labels), NOOP_COUNTER_HANDLE); assert.strictEqual(counter.getDefaultHandle(), NOOP_COUNTER_HANDLE); counter.clear(); @@ -63,10 +62,7 @@ describe('NoopMeter', () => { // ensure the correct noop const is returned assert.strictEqual(measure, NOOP_MEASURE_METRIC); assert.strictEqual(measure.getDefaultHandle(), NOOP_MEASURE_HANDLE); - assert.strictEqual( - measure.getHandle(labels), - NOOP_MEASURE_HANDLE - ); + assert.strictEqual(measure.getHandle(labels), NOOP_MEASURE_HANDLE); const gauge = meter.createGauge('some-name'); gauge.getDefaultHandle().set(1); diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 4a0e80f7b3..059cc397a9 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -21,7 +21,8 @@ import { NoopLogger } from '@opentelemetry/core'; describe('Meter', () => { let meter: Meter; - const labels: types.LabelSet = {'key': 'value'}; + const key = 'key'; + const labels: types.LabelSet = { [key]: 'value' }; const hrTime: types.HrTime = [22, 400000000]; beforeEach(() => { @@ -58,7 +59,12 @@ describe('Meter', () => { it('should return the timeseries', () => { const counter = meter.createCounter('name'); - const handle = counter.getHandle({'key1': 'value1', 'key2': 'value2'}); + const key1 = 'key1'; + const key2 = 'key2'; + const handle = counter.getHandle({ + [key1]: 'value1', + [key2]: 'value2', + }); handle.add(20); assert.deepStrictEqual(handle.getTimeSeries(hrTime), { labelValues: [{ value: 'value1' }, { value: 'value2' }], @@ -159,7 +165,9 @@ describe('Meter', () => { it('should return the timeseries', () => { const gauge = meter.createGauge('name'); - const handle = gauge.getHandle({'k1': 'v1', 'k2': 'v2'}); + const k1 = 'k1'; + const k2 = 'k2'; + const handle = gauge.getHandle({ [k1]: 'v1', [k2]: 'v2' }); handle.set(150); assert.deepStrictEqual(handle.getTimeSeries(hrTime), { labelValues: [{ value: 'v1' }, { value: 'v2' }], From 51ef29e876059637b58c8bc9f4b19140367dbf07 Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Tue, 29 Oct 2019 14:06:44 -0700 Subject: [PATCH 03/13] fix: doc strings --- packages/opentelemetry-core/src/metrics/NoopMeter.ts | 7 ++++--- packages/opentelemetry-metrics/src/Handle.ts | 5 ++--- packages/opentelemetry-metrics/src/Metric.ts | 6 +++--- packages/opentelemetry-metrics/src/export/types.ts | 2 +- packages/opentelemetry-types/src/metrics/Metric.ts | 6 +++--- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/opentelemetry-core/src/metrics/NoopMeter.ts b/packages/opentelemetry-core/src/metrics/NoopMeter.ts index b154b8396a..3f32107b56 100644 --- a/packages/opentelemetry-core/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-core/src/metrics/NoopMeter.ts @@ -68,10 +68,10 @@ export class NoopMetric implements Metric { this._handle = handle; } /** - * Returns a Handle associated with specified label values. + * Returns a Handle associated with specified LabelSet. * It is recommended to keep a reference to the Handle instead of always * calling this method for every operations. - * @param labels the object of label set. + * @param labels the LabelSet used to associate with this metric handle. */ getHandle(labels: LabelSet): T { return this._handle; @@ -86,9 +86,10 @@ export class NoopMetric implements Metric { /** * Removes the Handle from the metric, if it is present. - * @param labels the object of label set. + * @param labels the LabelSet used to associate with this metric handle. */ removeHandle(labels: LabelSet): void { + // @todo: implement this method return; } diff --git a/packages/opentelemetry-metrics/src/Handle.ts b/packages/opentelemetry-metrics/src/Handle.ts index 85ef3b1e98..04159630e2 100644 --- a/packages/opentelemetry-metrics/src/Handle.ts +++ b/packages/opentelemetry-metrics/src/Handle.ts @@ -19,8 +19,7 @@ import { TimeSeries } from './export/types'; /** * CounterHandle allows the SDK to observe/record a single metric event. The - * value of single handle in the `Counter` associated with specified label - * values. + * value of single handle in the `Counter` associated with specified LabelSet. */ export class CounterHandle implements types.CounterHandle { private _data = 0; @@ -58,7 +57,7 @@ export class CounterHandle implements types.CounterHandle { /** * GaugeHandle allows the SDK to observe/record a single metric event. The - * value of single handle in the `Gauge` associated with specified label values. + * value of single handle in the `Gauge` associated with specified LabelSet. */ export class GaugeHandle implements types.GaugeHandle { private _data = 0; diff --git a/packages/opentelemetry-metrics/src/Metric.ts b/packages/opentelemetry-metrics/src/Metric.ts index 9befe98cb7..adff4f56de 100644 --- a/packages/opentelemetry-metrics/src/Metric.ts +++ b/packages/opentelemetry-metrics/src/Metric.ts @@ -33,10 +33,10 @@ export abstract class Metric implements types.Metric { } /** - * Returns a Handle associated with specified label values. + * Returns a Handle associated with specified LabelSet. * It is recommended to keep a reference to the Handle instead of always * calling this method for each operation. - * @param labels the object of label set. + * @param labels the LabelSet used to associate with this metric handle. */ getHandle(labels: types.LabelSet): T { const hash = hashLabelValues(Object.values(labels)); @@ -58,7 +58,7 @@ export abstract class Metric implements types.Metric { /** * Removes the Handle from the metric, if it is present. - * @param labels the object of label set. + * @param labels the LabelSet used to associate with this metric handle. */ removeHandle(labels: types.LabelSet): void { this._handles.delete(hashLabelValues(Object.values(labels))); diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts index 12d31a7c0f..6b9eb6158c 100644 --- a/packages/opentelemetry-metrics/src/export/types.ts +++ b/packages/opentelemetry-metrics/src/export/types.ts @@ -124,7 +124,7 @@ export enum MetricDescriptorType { export interface TimeSeries { /** * The set of label values that uniquely identify this timeseries. Applies to - * all points. The order of label values must match that of label keys in the + * all points. The order of label values must match that of LabelSet keys in the * metric descriptor. */ readonly labelValues: LabelValue[]; diff --git a/packages/opentelemetry-types/src/metrics/Metric.ts b/packages/opentelemetry-types/src/metrics/Metric.ts index d26c9340b0..4bc523c0c6 100644 --- a/packages/opentelemetry-types/src/metrics/Metric.ts +++ b/packages/opentelemetry-types/src/metrics/Metric.ts @@ -55,10 +55,10 @@ export interface MetricOptions { */ export interface Metric { /** - * Returns a Handle associated with specified label values. + * Returns a Handle associated with specified LabelSet. * It is recommended to keep a reference to the Handle instead of always * calling this method for every operations. - * @param labels the object of label set. + * @param labels the LabelSet used to associate with this metric handle. */ getHandle(labels: LabelSet): T; @@ -69,7 +69,7 @@ export interface Metric { /** * Removes the Handle from the metric, if it is present. - * @param labels the object of label set. + * @param labels the LabelSet used to associate with this metric handle. */ removeHandle(labels: LabelSet): void; From 3599b53cdebabc9367163a56bd38c07c47702daa Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Wed, 30 Oct 2019 14:37:49 -0700 Subject: [PATCH 04/13] fix: expose labels function in Meter --- .../src/metrics/NoopMeter.ts | 10 ++++++++-- packages/opentelemetry-metrics/src/Meter.ts | 13 ++++++++++++ packages/opentelemetry-metrics/src/Metric.ts | 4 ++-- packages/opentelemetry-metrics/src/Utils.ts | 2 +- .../opentelemetry-metrics/src/export/types.ts | 2 ++ packages/opentelemetry-metrics/src/types.ts | 4 ++++ .../opentelemetry-metrics/test/Meter.test.ts | 20 +++++++++++++++---- .../opentelemetry-types/src/metrics/Meter.ts | 10 +++++++++- .../opentelemetry-types/src/metrics/Metric.ts | 6 +++--- 9 files changed, 58 insertions(+), 13 deletions(-) diff --git a/packages/opentelemetry-core/src/metrics/NoopMeter.ts b/packages/opentelemetry-core/src/metrics/NoopMeter.ts index 3f32107b56..34d8de0481 100644 --- a/packages/opentelemetry-core/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-core/src/metrics/NoopMeter.ts @@ -59,6 +59,10 @@ export class NoopMeter implements Meter { createGauge(name: string, options?: MetricOptions): Metric { return NOOP_GAUGE_METRIC; } + + labels(labels: LabelSet): LabelSet { + return NOOP_METER; + } } export class NoopMetric implements Metric { @@ -71,7 +75,7 @@ export class NoopMetric implements Metric { * Returns a Handle associated with specified LabelSet. * It is recommended to keep a reference to the Handle instead of always * calling this method for every operations. - * @param labels the LabelSet used to associate with this metric handle. + * @param labels the canonicalized LabelSet used to associate with this metric handle. */ getHandle(labels: LabelSet): T { return this._handle; @@ -86,7 +90,7 @@ export class NoopMetric implements Metric { /** * Removes the Handle from the metric, if it is present. - * @param labels the LabelSet used to associate with this metric handle. + * @param labels the canonicalized LabelSet used to associate with this metric handle. */ removeHandle(labels: LabelSet): void { // @todo: implement this method @@ -139,3 +143,5 @@ export const NOOP_MEASURE_HANDLE = new NoopMeasureHandle(); export const NOOP_MEASURE_METRIC = new NoopMetric( NOOP_MEASURE_HANDLE ); + +export const NOOP_METER = {} as LabelSet; diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index a882b5e048..896232ac4f 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -93,4 +93,17 @@ export class Meter implements types.Meter { }; return new GaugeMetric(name, opt); } + + /** + * Provide a pre-computed re-useable LabelSet by + * converting the unordered LabelSet into a canonicalized + * set of lables, useful for pre-aggregation. + * @param labels user provided unordered LabelSet. + */ + labels(labels: types.LabelSet): types.LabelSet { + return Object.keys(labels).sort().reduce((result, key) => { + result[key] = labels[key]; + return result; + }, {} as types.LabelSet); + } } diff --git a/packages/opentelemetry-metrics/src/Metric.ts b/packages/opentelemetry-metrics/src/Metric.ts index adff4f56de..602a6c029b 100644 --- a/packages/opentelemetry-metrics/src/Metric.ts +++ b/packages/opentelemetry-metrics/src/Metric.ts @@ -36,7 +36,7 @@ export abstract class Metric implements types.Metric { * Returns a Handle associated with specified LabelSet. * It is recommended to keep a reference to the Handle instead of always * calling this method for each operation. - * @param labels the LabelSet used to associate with this metric handle. + * @param labels the canonicalized LabelSet used to associate with this metric handle. */ getHandle(labels: types.LabelSet): T { const hash = hashLabelValues(Object.values(labels)); @@ -58,7 +58,7 @@ export abstract class Metric implements types.Metric { /** * Removes the Handle from the metric, if it is present. - * @param labels the LabelSet used to associate with this metric handle. + * @param labels the canonicalized LabelSet used to associate with this metric handle. */ removeHandle(labels: types.LabelSet): void { this._handles.delete(hashLabelValues(Object.values(labels))); diff --git a/packages/opentelemetry-metrics/src/Utils.ts b/packages/opentelemetry-metrics/src/Utils.ts index 4fa4242cb7..71565a8d97 100644 --- a/packages/opentelemetry-metrics/src/Utils.ts +++ b/packages/opentelemetry-metrics/src/Utils.ts @@ -23,5 +23,5 @@ const COMMA_SEPARATOR = ','; * @returns The hashed label values string. */ export function hashLabelValues(labelValues: string[]): string { - return labelValues.sort().join(COMMA_SEPARATOR); + return labelValues.join(COMMA_SEPARATOR); } diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts index 6b9eb6158c..b9a3e3e424 100644 --- a/packages/opentelemetry-metrics/src/export/types.ts +++ b/packages/opentelemetry-metrics/src/export/types.ts @@ -61,6 +61,8 @@ export interface MetricDescriptor { readonly unit: string; /** MetricDescriptor type */ readonly type: MetricDescriptorType; + /** The label keys associated with the metric descriptor. */ + readonly labelKeys: string[]; /** The label set associated with the metric descriptor. */ readonly labels: LabelSet; } diff --git a/packages/opentelemetry-metrics/src/types.ts b/packages/opentelemetry-metrics/src/types.ts index b1e262bd5f..d4078bbfcf 100644 --- a/packages/opentelemetry-metrics/src/types.ts +++ b/packages/opentelemetry-metrics/src/types.ts @@ -28,6 +28,9 @@ export interface MetricOptions { /** The unit of the Metric values. */ unit: string; + /** The list of label keys for the Metric. */ + labelKeys: string[]; + /** The map of constant labels for the Metric. */ constantLabels?: Map; @@ -60,4 +63,5 @@ export const DEFAULT_METRIC_OPTIONS = { disabled: false, description: '', unit: '1', + labelKeys: [], }; diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 059cc397a9..1ff73490a9 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -21,16 +21,28 @@ import { NoopLogger } from '@opentelemetry/core'; describe('Meter', () => { let meter: Meter; - const key = 'key'; - const labels: types.LabelSet = { [key]: 'value' }; + const keya = 'keya'; + const keyb = 'keyb'; + let labels: types.LabelSet = { [keyb]: 'value2', [keya]: 'value1' }; const hrTime: types.HrTime = [22, 400000000]; beforeEach(() => { meter = new Meter({ logger: new NoopLogger(), }); + labels = meter.labels(labels); }); + describe('#meter', () => { + it('should re-order labels to a canonicalized set', () => { + const orderedLabels: types.LabelSet = { + [keya]: 'value1', + [keyb]: 'value2' + } + assert.deepEqual(labels, orderedLabels); + }); + }); + describe('#counter', () => { it('should create a counter', () => { const counter = meter.createCounter('name'); @@ -124,7 +136,7 @@ describe('Meter', () => { it('should not fail when removing non existing handle', () => { const counter = meter.createCounter('name'); - counter.removeHandle([]); + counter.removeHandle({}); }); it('should clear all handles', () => { @@ -227,7 +239,7 @@ describe('Meter', () => { it('should not fail when removing non existing handle', () => { const gauge = meter.createGauge('name'); - gauge.removeHandle([]); + gauge.removeHandle({}); }); it('should clear all handles', () => { diff --git a/packages/opentelemetry-types/src/metrics/Meter.ts b/packages/opentelemetry-types/src/metrics/Meter.ts index 15f03688b6..7d71b43940 100644 --- a/packages/opentelemetry-types/src/metrics/Meter.ts +++ b/packages/opentelemetry-types/src/metrics/Meter.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Metric, MetricOptions } from './Metric'; +import { Metric, MetricOptions, LabelSet } from './Metric'; import { CounterHandle, GaugeHandle, MeasureHandle } from './Handle'; /** @@ -58,4 +58,12 @@ export interface Meter { * @param [options] the metric options. */ createGauge(name: string, options?: MetricOptions): Metric; + + /** + * Provide a pre-computed re-useable LabelSet by + * converting the unordered LabelSet into a canonicalized + * set of lables, useful for pre-aggregation. + * @param labels user provided unordered LabelSet. + */ + labels(labels: LabelSet): LabelSet; } diff --git a/packages/opentelemetry-types/src/metrics/Metric.ts b/packages/opentelemetry-types/src/metrics/Metric.ts index 4bc523c0c6..7ab9fc75fc 100644 --- a/packages/opentelemetry-types/src/metrics/Metric.ts +++ b/packages/opentelemetry-types/src/metrics/Metric.ts @@ -58,7 +58,7 @@ export interface Metric { * Returns a Handle associated with specified LabelSet. * It is recommended to keep a reference to the Handle instead of always * calling this method for every operations. - * @param labels the LabelSet used to associate with this metric handle. + * @param labels the canonicalized LabelSet used to associate with this metric handle. */ getHandle(labels: LabelSet): T; @@ -69,7 +69,7 @@ export interface Metric { /** * Removes the Handle from the metric, if it is present. - * @param labels the LabelSet used to associate with this metric handle. + * @param labels the canonicalized LabelSet used to associate with this metric handle. */ removeHandle(labels: LabelSet): void; @@ -84,4 +84,4 @@ export interface Metric { setCallback(fn: () => void): void; } -export interface LabelSet {} +export type LabelSet = Record; From 05d1c6312970ce0d3a9c2cea4a2e7fdd5ba821e5 Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Wed, 30 Oct 2019 15:00:15 -0700 Subject: [PATCH 05/13] fix: linting --- packages/opentelemetry-metrics/src/Meter.ts | 15 ++++++++++----- packages/opentelemetry-metrics/test/Meter.test.ts | 6 +++--- packages/opentelemetry-types/src/metrics/Meter.ts | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 896232ac4f..29ff7ca256 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -96,14 +96,19 @@ export class Meter implements types.Meter { /** * Provide a pre-computed re-useable LabelSet by - * converting the unordered LabelSet into a canonicalized + * converting the unordered LabelSet into a canonicalized * set of lables, useful for pre-aggregation. * @param labels user provided unordered LabelSet. */ labels(labels: types.LabelSet): types.LabelSet { - return Object.keys(labels).sort().reduce((result, key) => { - result[key] = labels[key]; - return result; - }, {} as types.LabelSet); + return Object.keys(labels) + .sort() + .reduce( + (result, key) => { + result[key] = labels[key]; + return result; + }, + {} as types.LabelSet + ); } } diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 1ff73490a9..da406a5cb4 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -37,11 +37,11 @@ describe('Meter', () => { it('should re-order labels to a canonicalized set', () => { const orderedLabels: types.LabelSet = { [keya]: 'value1', - [keyb]: 'value2' - } + [keyb]: 'value2', + }; assert.deepEqual(labels, orderedLabels); }); - }); + }); describe('#counter', () => { it('should create a counter', () => { diff --git a/packages/opentelemetry-types/src/metrics/Meter.ts b/packages/opentelemetry-types/src/metrics/Meter.ts index 7d71b43940..56c870dd3b 100644 --- a/packages/opentelemetry-types/src/metrics/Meter.ts +++ b/packages/opentelemetry-types/src/metrics/Meter.ts @@ -61,7 +61,7 @@ export interface Meter { /** * Provide a pre-computed re-useable LabelSet by - * converting the unordered LabelSet into a canonicalized + * converting the unordered LabelSet into a canonicalized * set of lables, useful for pre-aggregation. * @param labels user provided unordered LabelSet. */ From 821ad672b10e5c55a33714bd974175913d13cb7d Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Thu, 31 Oct 2019 11:29:42 -0700 Subject: [PATCH 06/13] fix: address comments --- packages/opentelemetry-core/src/metrics/NoopMeter.ts | 4 ++-- packages/opentelemetry-metrics/src/export/types.ts | 4 +--- packages/opentelemetry-types/src/metrics/Metric.ts | 3 +++ 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-core/src/metrics/NoopMeter.ts b/packages/opentelemetry-core/src/metrics/NoopMeter.ts index 34d8de0481..6c56e29863 100644 --- a/packages/opentelemetry-core/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-core/src/metrics/NoopMeter.ts @@ -61,7 +61,7 @@ export class NoopMeter implements Meter { } labels(labels: LabelSet): LabelSet { - return NOOP_METER; + return NOOP_LABEL_SET; } } @@ -144,4 +144,4 @@ export const NOOP_MEASURE_METRIC = new NoopMetric( NOOP_MEASURE_HANDLE ); -export const NOOP_METER = {} as LabelSet; +export const NOOP_LABEL_SET = {} as LabelSet; diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts index b9a3e3e424..c6aa9c08c8 100644 --- a/packages/opentelemetry-metrics/src/export/types.ts +++ b/packages/opentelemetry-metrics/src/export/types.ts @@ -22,7 +22,7 @@ * opentelemetry-proto/opentelemetry/proto/metrics/v1/metrics.proto */ -import { HrTime, LabelSet } from '@opentelemetry/types'; +import { HrTime } from '@opentelemetry/types'; import { Resource, ExportResult } from '@opentelemetry/base'; export interface ReadableMetric { @@ -63,8 +63,6 @@ export interface MetricDescriptor { readonly type: MetricDescriptorType; /** The label keys associated with the metric descriptor. */ readonly labelKeys: string[]; - /** The label set associated with the metric descriptor. */ - readonly labels: LabelSet; } /** diff --git a/packages/opentelemetry-types/src/metrics/Metric.ts b/packages/opentelemetry-types/src/metrics/Metric.ts index 7ab9fc75fc..5e13b81d93 100644 --- a/packages/opentelemetry-types/src/metrics/Metric.ts +++ b/packages/opentelemetry-types/src/metrics/Metric.ts @@ -33,6 +33,9 @@ export interface MetricOptions { */ unit?: string; + /** The list of label keys for the Metric. */ + labelKeys?: string[]; + /** The map of constant labels for the Metric. */ constantLabels?: Map; From dea892cdfce287e0eaedd277f3fce12547370538 Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Fri, 1 Nov 2019 17:30:55 -0700 Subject: [PATCH 07/13] fix: udpate LabelSet logic --- .../src/metrics/NoopMeter.ts | 3 +- .../test/metrics/NoopMeter.test.ts | 18 ++--- packages/opentelemetry-metrics/src/Handle.ts | 12 ++-- packages/opentelemetry-metrics/src/Meter.ts | 31 +++++---- packages/opentelemetry-metrics/src/Metric.ts | 29 ++++---- packages/opentelemetry-metrics/src/Utils.ts | 12 ---- packages/opentelemetry-metrics/src/types.ts | 12 +++- .../opentelemetry-metrics/test/Meter.test.ts | 66 +++++++++---------- .../opentelemetry-types/src/metrics/Meter.ts | 11 ++-- .../opentelemetry-types/src/metrics/Metric.ts | 13 +++- 10 files changed, 113 insertions(+), 94 deletions(-) diff --git a/packages/opentelemetry-core/src/metrics/NoopMeter.ts b/packages/opentelemetry-core/src/metrics/NoopMeter.ts index 6c56e29863..9f354cbaa0 100644 --- a/packages/opentelemetry-core/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-core/src/metrics/NoopMeter.ts @@ -24,6 +24,7 @@ import { MeasureHandle, SpanContext, LabelSet, + Labels, } from '@opentelemetry/types'; /** @@ -60,7 +61,7 @@ export class NoopMeter implements Meter { return NOOP_GAUGE_METRIC; } - labels(labels: LabelSet): LabelSet { + labels(labels: Labels): LabelSet { return NOOP_LABEL_SET; } } diff --git a/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts b/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts index 72d6064437..be6324d1b2 100644 --- a/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts +++ b/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts @@ -24,26 +24,26 @@ import { NOOP_MEASURE_HANDLE, NOOP_MEASURE_METRIC, } from '../../src/metrics/NoopMeter'; -import { LabelSet } from '@opentelemetry/types'; +import { Labels } from '@opentelemetry/types'; describe('NoopMeter', () => { it('should not crash', () => { const meter = new NoopMeter(); const counter = meter.createCounter('some-name'); - const key1 = 'key1'; - const key2 = 'key2'; - const labels: LabelSet = { [key1]: 'val1', [key2]: 'val2' }; + const labels = {} as Labels; + const labelSet = meter.labels(labels); + // ensure NoopMetric does not crash. counter.setCallback(() => { assert.fail('callback occurred'); }); - counter.getHandle(labels).add(1); + counter.getHandle(labelSet).add(1); counter.getDefaultHandle().add(1); - counter.removeHandle(labels); + counter.removeHandle(labelSet); // ensure the correct noop const is returned assert.strictEqual(counter, NOOP_COUNTER_METRIC); - assert.strictEqual(counter.getHandle(labels), NOOP_COUNTER_HANDLE); + assert.strictEqual(counter.getHandle(labelSet), NOOP_COUNTER_HANDLE); assert.strictEqual(counter.getDefaultHandle(), NOOP_COUNTER_HANDLE); counter.clear(); @@ -62,7 +62,7 @@ describe('NoopMeter', () => { // ensure the correct noop const is returned assert.strictEqual(measure, NOOP_MEASURE_METRIC); assert.strictEqual(measure.getDefaultHandle(), NOOP_MEASURE_HANDLE); - assert.strictEqual(measure.getHandle(labels), NOOP_MEASURE_HANDLE); + assert.strictEqual(measure.getHandle(labelSet), NOOP_MEASURE_HANDLE); const gauge = meter.createGauge('some-name'); gauge.getDefaultHandle().set(1); @@ -70,7 +70,7 @@ describe('NoopMeter', () => { // ensure the correct noop const is returned assert.strictEqual(gauge, NOOP_GAUGE_METRIC); assert.strictEqual(gauge.getDefaultHandle(), NOOP_GAUGE_HANDLE); - assert.strictEqual(gauge.getHandle(labels), NOOP_GAUGE_HANDLE); + assert.strictEqual(gauge.getHandle(labelSet), NOOP_GAUGE_HANDLE); const options = { component: 'tests', diff --git a/packages/opentelemetry-metrics/src/Handle.ts b/packages/opentelemetry-metrics/src/Handle.ts index 04159630e2..0c0760d13e 100644 --- a/packages/opentelemetry-metrics/src/Handle.ts +++ b/packages/opentelemetry-metrics/src/Handle.ts @@ -27,7 +27,7 @@ export class CounterHandle implements types.CounterHandle { constructor( private readonly _disabled: boolean, private readonly _monotonic: boolean, - private readonly _labels: types.LabelSet, + private readonly _labelSet: types.LabelSet, private readonly _logger: types.Logger ) {} @@ -49,7 +49,9 @@ export class CounterHandle implements types.CounterHandle { */ getTimeSeries(timestamp: types.HrTime): TimeSeries { return { - labelValues: Object.values(this._labels).map(value => ({ value })), + labelValues: Object.values(this._labelSet.labels).map(value => ({ + value, + })), points: [{ value: this._data, timestamp }], }; } @@ -65,7 +67,7 @@ export class GaugeHandle implements types.GaugeHandle { constructor( private readonly _disabled: boolean, private readonly _monotonic: boolean, - private readonly _labels: types.LabelSet, + private readonly _labelSet: types.LabelSet, private readonly _logger: types.Logger ) {} @@ -87,7 +89,9 @@ export class GaugeHandle implements types.GaugeHandle { */ getTimeSeries(timestamp: types.HrTime): TimeSeries { return { - labelValues: Object.values(this._labels).map(value => ({ value })), + labelValues: Object.values(this._labelSet.labels).map(value => ({ + value, + })), points: [{ value: this._data, timestamp }], }; } diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 46203d58fa..7c45799585 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -27,6 +27,7 @@ import { DEFAULT_METRIC_OPTIONS, DEFAULT_CONFIG, MeterConfig, + LabelSet, } from './types'; /** @@ -118,20 +119,24 @@ export class Meter implements types.Meter { /** * Provide a pre-computed re-useable LabelSet by - * converting the unordered LabelSet into a canonicalized - * set of lables, useful for pre-aggregation. - * @param labels user provided unordered LabelSet. + * converting the unordered labels into a canonicalized + * set of lables with a unique labelSetKey, useful for pre-aggregation. + * @param labels user provided unordered Labels. */ - labels(labels: types.LabelSet): types.LabelSet { - return Object.keys(labels) - .sort() - .reduce( - (result, key) => { - result[key] = labels[key]; - return result; - }, - {} as types.LabelSet - ); + labels(labels: types.Labels): types.LabelSet { + let keys = Object.keys(labels).sort(); + let labelSetKey = keys.reduce((result, key) => { + if (result.length > 2) { + result += ','; + } + result = result + key + ':' + labels[key]; + return result; + }, '|#'); + let sortedLabels = {} as types.Labels; + keys.forEach(key => { + sortedLabels[key] = labels[key]; + }); + return new LabelSet(labelSetKey, sortedLabels); } /** diff --git a/packages/opentelemetry-metrics/src/Metric.ts b/packages/opentelemetry-metrics/src/Metric.ts index 602a6c029b..f6f4291409 100644 --- a/packages/opentelemetry-metrics/src/Metric.ts +++ b/packages/opentelemetry-metrics/src/Metric.ts @@ -15,7 +15,6 @@ */ import * as types from '@opentelemetry/types'; -import { hashLabelValues } from './Utils'; import { CounterHandle, GaugeHandle } from './Handle'; import { MetricOptions } from './types'; @@ -36,14 +35,14 @@ export abstract class Metric implements types.Metric { * Returns a Handle associated with specified LabelSet. * It is recommended to keep a reference to the Handle instead of always * calling this method for each operation. - * @param labels the canonicalized LabelSet used to associate with this metric handle. + * @param labelSet the canonicalized LabelSet used to associate with this metric handle. */ - getHandle(labels: types.LabelSet): T { - const hash = hashLabelValues(Object.values(labels)); - if (this._handles.has(hash)) return this._handles.get(hash)!; + getHandle(labelSet: types.LabelSet): T { + if (this._handles.has(labelSet.labelSetKey)) + return this._handles.get(labelSet.labelSetKey)!; - const handle = this._makeHandle(labels); - this._handles.set(hash, handle); + const handle = this._makeHandle(labelSet); + this._handles.set(labelSet.labelSetKey, handle); return handle; } @@ -58,10 +57,10 @@ export abstract class Metric implements types.Metric { /** * Removes the Handle from the metric, if it is present. - * @param labels the canonicalized LabelSet used to associate with this metric handle. + * @param labelSet the canonicalized LabelSet used to associate with this metric handle. */ - removeHandle(labels: types.LabelSet): void { - this._handles.delete(hashLabelValues(Object.values(labels))); + removeHandle(labelSet: types.LabelSet): void { + this._handles.delete(labelSet.labelSetKey); } /** @@ -77,7 +76,7 @@ export abstract class Metric implements types.Metric { return; } - protected abstract _makeHandle(labels: types.LabelSet): T; + protected abstract _makeHandle(labelSet: types.LabelSet): T; } /** This is a SDK implementation of Counter Metric. */ @@ -85,11 +84,11 @@ export class CounterMetric extends Metric { constructor(name: string, options: MetricOptions) { super(name, options); } - protected _makeHandle(labels: types.LabelSet): CounterHandle { + protected _makeHandle(labelSet: types.LabelSet): CounterHandle { return new CounterHandle( this._disabled, this._monotonic, - labels, + labelSet, this._logger ); } @@ -100,11 +99,11 @@ export class GaugeMetric extends Metric { constructor(name: string, options: MetricOptions) { super(name, options); } - protected _makeHandle(labels: types.LabelSet): GaugeHandle { + protected _makeHandle(labelSet: types.LabelSet): GaugeHandle { return new GaugeHandle( this._disabled, this._monotonic, - labels, + labelSet, this._logger ); } diff --git a/packages/opentelemetry-metrics/src/Utils.ts b/packages/opentelemetry-metrics/src/Utils.ts index 71565a8d97..ae225f6b52 100644 --- a/packages/opentelemetry-metrics/src/Utils.ts +++ b/packages/opentelemetry-metrics/src/Utils.ts @@ -13,15 +13,3 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -const COMMA_SEPARATOR = ','; - -/** - * Returns a string(comma separated) from the list of label values. - * - * @param labelValues The list of the label values. - * @returns The hashed label values string. - */ -export function hashLabelValues(labelValues: string[]): string { - return labelValues.join(COMMA_SEPARATOR); -} diff --git a/packages/opentelemetry-metrics/src/types.ts b/packages/opentelemetry-metrics/src/types.ts index d4078bbfcf..5f637d31fd 100644 --- a/packages/opentelemetry-metrics/src/types.ts +++ b/packages/opentelemetry-metrics/src/types.ts @@ -15,7 +15,7 @@ */ import { LogLevel } from '@opentelemetry/core'; -import { Logger } from '@opentelemetry/types'; +import { Logger, Labels } from '@opentelemetry/types'; /** Options needed for SDK metric creation. */ export interface MetricOptions { @@ -65,3 +65,13 @@ export const DEFAULT_METRIC_OPTIONS = { unit: '1', labelKeys: [], }; + +export class LabelSet implements LabelSet { + labelSetKey: string; + labels: Labels; + + constructor(labelSetKey: string, labels: Labels) { + this.labelSetKey = labelSetKey; + this.labels = labels; + } +} diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index e3a740259a..4ceb97e4e2 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -18,28 +18,31 @@ import * as assert from 'assert'; import { Meter, Metric, CounterMetric, GaugeMetric } from '../src'; import * as types from '@opentelemetry/types'; import { NoopLogger, NoopMetric } from '@opentelemetry/core'; +import { LabelSet } from '../src/types'; describe('Meter', () => { let meter: Meter; const keya = 'keya'; const keyb = 'keyb'; - let labels: types.LabelSet = { [keyb]: 'value2', [keya]: 'value1' }; + let labels: types.Labels = { [keyb]: 'value2', [keya]: 'value1' }; + let labelSet: types.LabelSet; const hrTime: types.HrTime = [22, 400000000]; beforeEach(() => { meter = new Meter({ logger: new NoopLogger(), }); - labels = meter.labels(labels); + labelSet = meter.labels(labels); }); describe('#meter', () => { it('should re-order labels to a canonicalized set', () => { - const orderedLabels: types.LabelSet = { + const orderedLabels: types.Labels = { [keya]: 'value1', [keyb]: 'value2', }; - assert.deepEqual(labels, orderedLabels); + const labelSetKey = '|#keya:value1,keyb:value2'; + assert.deepEqual(labelSet, new LabelSet(labelSetKey, orderedLabels)); }); }); @@ -62,7 +65,7 @@ describe('Meter', () => { describe('.getHandle()', () => { it('should create a counter handle', () => { const counter = meter.createCounter('name') as CounterMetric; - const handle = counter.getHandle(labels); + const handle = counter.getHandle(labelSet); handle.add(10); assert.strictEqual(handle['_data'], 10); handle.add(10); @@ -71,12 +74,7 @@ describe('Meter', () => { it('should return the timeseries', () => { const counter = meter.createCounter('name') as CounterMetric; - const key1 = 'key1'; - const key2 = 'key2'; - const handle = counter.getHandle({ - [key1]: 'value1', - [key2]: 'value2', - }); + const handle = counter.getHandle(labelSet); handle.add(20); assert.deepStrictEqual(handle.getTimeSeries(hrTime), { labelValues: [{ value: 'value1' }, { value: 'value2' }], @@ -86,7 +84,7 @@ describe('Meter', () => { it('should add positive values by default', () => { const counter = meter.createCounter('name') as CounterMetric; - const handle = counter.getHandle(labels); + const handle = counter.getHandle(labelSet); handle.add(10); assert.strictEqual(handle['_data'], 10); handle.add(-100); @@ -97,7 +95,7 @@ describe('Meter', () => { const counter = meter.createCounter('name', { disabled: true, }) as CounterMetric; - const handle = counter.getHandle(labels); + const handle = counter.getHandle(labelSet); handle.add(10); assert.strictEqual(handle['_data'], 0); }); @@ -106,16 +104,16 @@ describe('Meter', () => { const counter = meter.createCounter('name', { monotonic: false, }) as CounterMetric; - const handle = counter.getHandle(labels); + const handle = counter.getHandle(labelSet); handle.add(-10); assert.strictEqual(handle['_data'], -10); }); it('should return same handle on same label values', () => { const counter = meter.createCounter('name') as CounterMetric; - const handle = counter.getHandle(labels); + const handle = counter.getHandle(labelSet); handle.add(10); - const handle1 = counter.getHandle(labels); + const handle1 = counter.getHandle(labelSet); handle1.add(10); assert.strictEqual(handle['_data'], 20); assert.strictEqual(handle, handle1); @@ -125,23 +123,23 @@ describe('Meter', () => { describe('.removeHandle()', () => { it('should remove a counter handle', () => { const counter = meter.createCounter('name') as CounterMetric; - const handle = counter.getHandle(labels); + const handle = counter.getHandle(labelSet); assert.strictEqual(counter['_handles'].size, 1); - counter.removeHandle(labels); + counter.removeHandle(labelSet); assert.strictEqual(counter['_handles'].size, 0); - const handle1 = counter.getHandle(labels); + const handle1 = counter.getHandle(labelSet); assert.strictEqual(counter['_handles'].size, 1); assert.notStrictEqual(handle, handle1); }); it('should not fail when removing non existing handle', () => { const counter = meter.createCounter('name'); - counter.removeHandle({}); + counter.removeHandle(new LabelSet('', {})); }); it('should clear all handles', () => { const counter = meter.createCounter('name') as CounterMetric; - counter.getHandle(labels); + counter.getHandle(labelSet); assert.strictEqual(counter['_handles'].size, 1); counter.clear(); assert.strictEqual(counter['_handles'].size, 0); @@ -196,7 +194,7 @@ describe('Meter', () => { describe('.getHandle()', () => { it('should create a gauge handle', () => { const gauge = meter.createGauge('name') as GaugeMetric; - const handle = gauge.getHandle(labels); + const handle = gauge.getHandle(labelSet); handle.set(10); assert.strictEqual(handle['_data'], 10); handle.set(250); @@ -207,7 +205,9 @@ describe('Meter', () => { const gauge = meter.createGauge('name') as GaugeMetric; const k1 = 'k1'; const k2 = 'k2'; - const handle = gauge.getHandle({ [k1]: 'v1', [k2]: 'v2' }); + const labels = { [k1]: 'v1', [k2]: 'v2' }; + const LabelSet2 = new LabelSet('|#k1:v1,k2:v2', labels); + const handle = gauge.getHandle(LabelSet2); handle.set(150); assert.deepStrictEqual(handle.getTimeSeries(hrTime), { labelValues: [{ value: 'v1' }, { value: 'v2' }], @@ -217,7 +217,7 @@ describe('Meter', () => { it('should go up and down by default', () => { const gauge = meter.createGauge('name') as GaugeMetric; - const handle = gauge.getHandle(labels); + const handle = gauge.getHandle(labelSet); handle.set(10); assert.strictEqual(handle['_data'], 10); handle.set(-100); @@ -228,7 +228,7 @@ describe('Meter', () => { const gauge = meter.createGauge('name', { disabled: true, }) as GaugeMetric; - const handle = gauge.getHandle(labels); + const handle = gauge.getHandle(labelSet); handle.set(10); assert.strictEqual(handle['_data'], 0); }); @@ -237,16 +237,16 @@ describe('Meter', () => { const gauge = meter.createGauge('name', { monotonic: true, }) as GaugeMetric; - const handle = gauge.getHandle(labels); + const handle = gauge.getHandle(labelSet); handle.set(-10); assert.strictEqual(handle['_data'], 0); }); it('should return same handle on same label values', () => { const gauge = meter.createGauge('name') as GaugeMetric; - const handle = gauge.getHandle(labels); + const handle = gauge.getHandle(labelSet); handle.set(10); - const handle1 = gauge.getHandle(labels); + const handle1 = gauge.getHandle(labelSet); handle1.set(10); assert.strictEqual(handle['_data'], 10); assert.strictEqual(handle, handle1); @@ -256,23 +256,23 @@ describe('Meter', () => { describe('.removeHandle()', () => { it('should remove the gauge handle', () => { const gauge = meter.createGauge('name') as GaugeMetric; - const handle = gauge.getHandle(labels); + const handle = gauge.getHandle(labelSet); assert.strictEqual(gauge['_handles'].size, 1); - gauge.removeHandle(labels); + gauge.removeHandle(labelSet); assert.strictEqual(gauge['_handles'].size, 0); - const handle1 = gauge.getHandle(labels); + const handle1 = gauge.getHandle(labelSet); assert.strictEqual(gauge['_handles'].size, 1); assert.notStrictEqual(handle, handle1); }); it('should not fail when removing non existing handle', () => { const gauge = meter.createGauge('name'); - gauge.removeHandle({}); + gauge.removeHandle(new LabelSet('', {})); }); it('should clear all handles', () => { const gauge = meter.createGauge('name') as GaugeMetric; - gauge.getHandle(labels); + gauge.getHandle(labelSet); assert.strictEqual(gauge['_handles'].size, 1); gauge.clear(); assert.strictEqual(gauge['_handles'].size, 0); diff --git a/packages/opentelemetry-types/src/metrics/Meter.ts b/packages/opentelemetry-types/src/metrics/Meter.ts index 56c870dd3b..cb0827b4dd 100644 --- a/packages/opentelemetry-types/src/metrics/Meter.ts +++ b/packages/opentelemetry-types/src/metrics/Meter.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Metric, MetricOptions, LabelSet } from './Metric'; +import { Metric, MetricOptions, Labels, LabelSet } from './Metric'; import { CounterHandle, GaugeHandle, MeasureHandle } from './Handle'; /** @@ -61,9 +61,10 @@ export interface Meter { /** * Provide a pre-computed re-useable LabelSet by - * converting the unordered LabelSet into a canonicalized - * set of lables, useful for pre-aggregation. - * @param labels user provided unordered LabelSet. + * converting the unordered labels into a canonicalized + * set of lables with a unique labelSetKey, useful for pre-aggregation. + * @param labels user provided unordered Labels. */ - labels(labels: LabelSet): LabelSet; + labels(labels: Labels): LabelSet; } + diff --git a/packages/opentelemetry-types/src/metrics/Metric.ts b/packages/opentelemetry-types/src/metrics/Metric.ts index 5e13b81d93..272f5705d1 100644 --- a/packages/opentelemetry-types/src/metrics/Metric.ts +++ b/packages/opentelemetry-types/src/metrics/Metric.ts @@ -87,4 +87,15 @@ export interface Metric { setCallback(fn: () => void): void; } -export type LabelSet = Record; +/** + * key-value pairs passed by the user. + */ +export type Labels = Record; + +/** + * Canonicalized labels with an unique string labelSetKey. + */ +export interface LabelSet { + labelSetKey: string; + labels: Labels; +} From 2a7e0935da614f4b4b3a19d6957af8afd7d1d76e Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Fri, 1 Nov 2019 17:39:43 -0700 Subject: [PATCH 08/13] fix: linting --- packages/opentelemetry-core/test/metrics/NoopMeter.test.ts | 2 +- packages/opentelemetry-types/src/metrics/Meter.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts b/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts index be6324d1b2..38294975f4 100644 --- a/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts +++ b/packages/opentelemetry-core/test/metrics/NoopMeter.test.ts @@ -32,7 +32,7 @@ describe('NoopMeter', () => { const counter = meter.createCounter('some-name'); const labels = {} as Labels; const labelSet = meter.labels(labels); - + // ensure NoopMetric does not crash. counter.setCallback(() => { assert.fail('callback occurred'); diff --git a/packages/opentelemetry-types/src/metrics/Meter.ts b/packages/opentelemetry-types/src/metrics/Meter.ts index cb0827b4dd..633b0d3645 100644 --- a/packages/opentelemetry-types/src/metrics/Meter.ts +++ b/packages/opentelemetry-types/src/metrics/Meter.ts @@ -67,4 +67,3 @@ export interface Meter { */ labels(labels: Labels): LabelSet; } - From 9e9fc2768eb75b1474f8227932716418e130ab88 Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Mon, 4 Nov 2019 14:35:21 -0800 Subject: [PATCH 09/13] fix: address comments --- .../src/{Utils.ts => LabelSet.ts} | 15 +++++++++++++++ packages/opentelemetry-metrics/src/Meter.ts | 2 +- packages/opentelemetry-metrics/src/types.ts | 12 +----------- packages/opentelemetry-metrics/test/Meter.test.ts | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) rename packages/opentelemetry-metrics/src/{Utils.ts => LabelSet.ts} (65%) diff --git a/packages/opentelemetry-metrics/src/Utils.ts b/packages/opentelemetry-metrics/src/LabelSet.ts similarity index 65% rename from packages/opentelemetry-metrics/src/Utils.ts rename to packages/opentelemetry-metrics/src/LabelSet.ts index ae225f6b52..67d6aa7ab1 100644 --- a/packages/opentelemetry-metrics/src/Utils.ts +++ b/packages/opentelemetry-metrics/src/LabelSet.ts @@ -13,3 +13,18 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +import * as types from '@opentelemetry/types'; + +/** + * Canonicalized labels with an unique string encoded. + */ +export class LabelSet implements types.LabelSet { + encoded: string; + labels: types.Labels; + + constructor(encoded: string, labels: types.Labels) { + this.encoded = encoded; + this.labels = labels; + } +} diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 579b8edbb8..adbd9221d7 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -28,9 +28,9 @@ import { DEFAULT_METRIC_OPTIONS, DEFAULT_CONFIG, MeterConfig, - LabelSet, } from './types'; import { ReadableMetric } from './export/types'; +import { LabelSet } from './LabelSet'; /** * Meter is an implementation of the {@link Meter} interface. diff --git a/packages/opentelemetry-metrics/src/types.ts b/packages/opentelemetry-metrics/src/types.ts index 06692b8a50..a20ef0afd9 100644 --- a/packages/opentelemetry-metrics/src/types.ts +++ b/packages/opentelemetry-metrics/src/types.ts @@ -15,7 +15,7 @@ */ import { LogLevel } from '@opentelemetry/core'; -import { Logger, ValueType, Labels } from '@opentelemetry/types'; +import { Logger, ValueType } from '@opentelemetry/types'; /** Options needed for SDK metric creation. */ export interface MetricOptions { @@ -69,13 +69,3 @@ export const DEFAULT_METRIC_OPTIONS = { labelKeys: [], valueType: ValueType.DOUBLE, }; - -export class LabelSet implements LabelSet { - encoded: string; - labels: Labels; - - constructor(encoded: string, labels: Labels) { - this.encoded = encoded; - this.labels = labels; - } -} diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index e078090e59..231c697273 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -23,7 +23,7 @@ import { MetricDescriptorType, } from '../src'; import * as types from '@opentelemetry/types'; -import { LabelSet } from '../src/types'; +import { LabelSet } from '../src/LabelSet'; import { NoopLogger, NoopMetric, From 88e598011b4adfab11dd699b64401374c8230f48 Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Mon, 4 Nov 2019 14:44:13 -0800 Subject: [PATCH 10/13] fix: update "encoded" to "identifier" --- packages/opentelemetry-metrics/src/LabelSet.ts | 8 ++++---- packages/opentelemetry-metrics/src/Meter.ts | 6 +++--- packages/opentelemetry-metrics/src/Metric.ts | 8 ++++---- packages/opentelemetry-metrics/test/Meter.test.ts | 4 ++-- packages/opentelemetry-types/src/metrics/Meter.ts | 2 +- packages/opentelemetry-types/src/metrics/Metric.ts | 4 ++-- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/opentelemetry-metrics/src/LabelSet.ts b/packages/opentelemetry-metrics/src/LabelSet.ts index 67d6aa7ab1..fb038384bf 100644 --- a/packages/opentelemetry-metrics/src/LabelSet.ts +++ b/packages/opentelemetry-metrics/src/LabelSet.ts @@ -17,14 +17,14 @@ import * as types from '@opentelemetry/types'; /** - * Canonicalized labels with an unique string encoded. + * Canonicalized labels with an unique string identifier. */ export class LabelSet implements types.LabelSet { - encoded: string; + identifier: string; labels: types.Labels; - constructor(encoded: string, labels: types.Labels) { - this.encoded = encoded; + constructor(identifier: string, labels: types.Labels) { + this.identifier = identifier; this.labels = labels; } } diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index adbd9221d7..5f55205192 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -156,12 +156,12 @@ export class Meter implements types.Meter { /** * Provide a pre-computed re-useable LabelSet by * converting the unordered labels into a canonicalized - * set of lables with an unique encoded, useful for pre-aggregation. + * set of lables with an unique identifier, useful for pre-aggregation. * @param labels user provided unordered Labels. */ labels(labels: types.Labels): types.LabelSet { let keys = Object.keys(labels).sort(); - let encoded = keys.reduce((result, key) => { + let identifier = keys.reduce((result, key) => { if (result.length > 2) { result += ','; } @@ -171,7 +171,7 @@ export class Meter implements types.Meter { keys.forEach(key => { sortedLabels[key] = labels[key]; }); - return new LabelSet(encoded, sortedLabels); + return new LabelSet(identifier, sortedLabels); } /** diff --git a/packages/opentelemetry-metrics/src/Metric.ts b/packages/opentelemetry-metrics/src/Metric.ts index a8e6caea5e..4c54313a5b 100644 --- a/packages/opentelemetry-metrics/src/Metric.ts +++ b/packages/opentelemetry-metrics/src/Metric.ts @@ -52,11 +52,11 @@ export abstract class Metric implements types.Metric { * @param labelSet the canonicalized LabelSet used to associate with this metric handle. */ getHandle(labelSet: types.LabelSet): T { - if (this._handles.has(labelSet.encoded)) - return this._handles.get(labelSet.encoded)!; + if (this._handles.has(labelSet.identifier)) + return this._handles.get(labelSet.identifier)!; const handle = this._makeHandle(labelSet); - this._handles.set(labelSet.encoded, handle); + this._handles.set(labelSet.identifier, handle); return handle; } @@ -74,7 +74,7 @@ export abstract class Metric implements types.Metric { * @param labelSet the canonicalized LabelSet used to associate with this metric handle. */ removeHandle(labelSet: types.LabelSet): void { - this._handles.delete(labelSet.encoded); + this._handles.delete(labelSet.identifier); } /** diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 231c697273..1f715ede13 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -54,8 +54,8 @@ describe('Meter', () => { [keya]: 'value1', [keyb]: 'value2', }; - const encoded = '|#keya:value1,keyb:value2'; - assert.deepEqual(labelSet, new LabelSet(encoded, orderedLabels)); + const identifier = '|#keya:value1,keyb:value2'; + assert.deepEqual(labelSet, new LabelSet(identifier, orderedLabels)); }); }); diff --git a/packages/opentelemetry-types/src/metrics/Meter.ts b/packages/opentelemetry-types/src/metrics/Meter.ts index 5913f5e21a..f9edeaabdd 100644 --- a/packages/opentelemetry-types/src/metrics/Meter.ts +++ b/packages/opentelemetry-types/src/metrics/Meter.ts @@ -62,7 +62,7 @@ export interface Meter { /** * Provide a pre-computed re-useable LabelSet by * converting the unordered labels into a canonicalized - * set of lables with an unique encoded, useful for pre-aggregation. + * set of lables with an unique identifier, useful for pre-aggregation. * @param labels user provided unordered Labels. */ labels(labels: Labels): LabelSet; diff --git a/packages/opentelemetry-types/src/metrics/Metric.ts b/packages/opentelemetry-types/src/metrics/Metric.ts index d86fda6cba..0c011d3811 100644 --- a/packages/opentelemetry-types/src/metrics/Metric.ts +++ b/packages/opentelemetry-types/src/metrics/Metric.ts @@ -105,9 +105,9 @@ export interface Metric { export type Labels = Record; /** - * Canonicalized labels with an unique string encoded. + * Canonicalized labels with an unique string identifier. */ export interface LabelSet { - encoded: string; + identifier: string; labels: Labels; } From 167394e6973380c48d01c8f1c12f59d5adfa7ffb Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Wed, 6 Nov 2019 13:43:05 -0800 Subject: [PATCH 11/13] chore: update naming --- packages/opentelemetry-metrics/src/Handle.ts | 12 ++++++------ packages/opentelemetry-metrics/src/Meter.ts | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-metrics/src/Handle.ts b/packages/opentelemetry-metrics/src/Handle.ts index f38eb2f869..14dd66addc 100644 --- a/packages/opentelemetry-metrics/src/Handle.ts +++ b/packages/opentelemetry-metrics/src/Handle.ts @@ -25,8 +25,8 @@ export class BaseHandle { protected _data = 0; protected _labelSet: types.LabelSet; - constructor(_labelSet: types.LabelSet) { - this._labelSet = _labelSet; + constructor(labelSet: types.LabelSet) { + this._labelSet = labelSet; } /** @@ -51,13 +51,13 @@ export class BaseHandle { */ export class CounterHandle extends BaseHandle implements types.CounterHandle { constructor( - _labelSet: types.LabelSet, + labelSet: types.LabelSet, private readonly _disabled: boolean, private readonly _monotonic: boolean, private readonly _valueType: types.ValueType, private readonly _logger: types.Logger ) { - super(_labelSet); + super(labelSet); } add(value: number): void { @@ -89,13 +89,13 @@ export class CounterHandle extends BaseHandle implements types.CounterHandle { */ export class GaugeHandle extends BaseHandle implements types.GaugeHandle { constructor( - _labelSet: types.LabelSet, + labelSet: types.LabelSet, private readonly _disabled: boolean, private readonly _monotonic: boolean, private readonly _valueType: types.ValueType, private readonly _logger: types.Logger ) { - super(_labelSet); + super(labelSet); } set(value: number): void { diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 5f55205192..b602445f11 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -167,7 +167,7 @@ export class Meter implements types.Meter { } return (result += key + ':' + labels[key]); }, '|#'); - let sortedLabels = {} as types.Labels; + let sortedLabels: types.Labels = {}; keys.forEach(key => { sortedLabels[key] = labels[key]; }); From eca223683d6e44b4d25465edb7c7e72cf9eec89f Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Wed, 6 Nov 2019 17:09:52 -0800 Subject: [PATCH 12/13] fix: conflicts --- packages/opentelemetry-metrics/src/Utils.ts | 24 +++++++++++++++++++ .../opentelemetry-metrics/test/Meter.test.ts | 8 +++---- 2 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 packages/opentelemetry-metrics/src/Utils.ts diff --git a/packages/opentelemetry-metrics/src/Utils.ts b/packages/opentelemetry-metrics/src/Utils.ts new file mode 100644 index 0000000000..e2c8fba463 --- /dev/null +++ b/packages/opentelemetry-metrics/src/Utils.ts @@ -0,0 +1,24 @@ +/*! + * Copyright 2019, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Type guard to remove nulls from arrays + * + * @param value value to be checked for null equality + */ +export function notNull(value: T | null): value is T { + return value !== null; +} diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 7c3eedec5f..06fd599abc 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -538,8 +538,8 @@ describe('Meter', () => { meter.addExporter(exporter); const gauge = meter.createGauge('name') as GaugeMetric; - const handle = gauge.getHandle(['value1', 'value2']); - handle.set(20); + const labelSet = meter.labels({ value: 'value1', value2: 'value2' }); + gauge.getHandle(labelSet).set(20); }); it('should export a counter when it is updated', done => { @@ -560,8 +560,8 @@ describe('Meter', () => { }); meter.addExporter(exporter); - const handle = counter.getHandle(['value1', 'value2']); - handle.add(20); + const labelSet = meter.labels({ value: 'value1', value2: 'value2' }); + counter.getHandle(labelSet).add(20); }); }); }); From 58717cb58c725d95bbe0906726d4e002a30aabc7 Mon Sep 17 00:00:00 2001 From: xiao-lix Date: Thu, 7 Nov 2019 09:49:31 -0800 Subject: [PATCH 13/13] chore: update let to const --- packages/opentelemetry-metrics/src/Meter.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index da167aebd5..d1cb29fce7 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -195,14 +195,14 @@ export class Meter implements types.Meter { * @param labels user provided unordered Labels. */ labels(labels: types.Labels): types.LabelSet { - let keys = Object.keys(labels).sort(); - let identifier = keys.reduce((result, key) => { + const keys = Object.keys(labels).sort(); + const identifier = keys.reduce((result, key) => { if (result.length > 2) { result += ','; } return (result += key + ':' + labels[key]); }, '|#'); - let sortedLabels: types.Labels = {}; + const sortedLabels: types.Labels = {}; keys.forEach(key => { sortedLabels[key] = labels[key]; });