Skip to content

Commit

Permalink
Remove label set from metrics API (#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 Apr 1, 2020
1 parent 802882d commit 3d26f82
Show file tree
Hide file tree
Showing 26 changed files with 186 additions and 274 deletions.
8 changes: 4 additions & 4 deletions examples/metrics/metrics/observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ function getCpuUsage() {
}

otelCpuUsage.setCallback((observerResult) => {
observerResult.observe(getCpuUsage, meter.labels({ pid: process.pid, core: '1' }));
observerResult.observe(getCpuUsage, meter.labels({ pid: process.pid, core: '2' }));
observerResult.observe(getCpuUsage, meter.labels({ pid: process.pid, core: '3' }));
observerResult.observe(getCpuUsage, meter.labels({ pid: process.pid, core: '4' }));
observerResult.observe(getCpuUsage, { pid: process.pid, core: '1' });
observerResult.observe(getCpuUsage, { pid: process.pid, core: '2' });
observerResult.observe(getCpuUsage, { pid: process.pid, core: '3' });
observerResult.observe(getCpuUsage, { pid: process.pid, core: '4' });
});
4 changes: 2 additions & 2 deletions examples/prometheus/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ const nonMonotonicCounter = meter.createCounter('non_monotonic_counter', {
description: 'Example of a non-monotonic counter',
});

setInterval(() => {
const labels = meter.labels({ pid: process.pid });
const labels = { pid: process.pid };

setInterval(() => {
monotonicCounter.bind(labels).add(1);
nonMonotonicCounter.bind(labels).add(Math.random() > 0.5 ? 1 : -1);
}, 1000);
8 changes: 4 additions & 4 deletions getting-started/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ const boundInstruments = new Map();
module.exports.countAllRequests = () => {
return (req, res, next) => {
if (!boundInstruments.has(req.path)) {
const labelSet = meter.labels({ route: req.path });
const boundCounter = requestCount.bind(labelSet);
const labels = { route: req.path };
const boundCounter = requestCount.bind(labels);
boundInstruments.set(req.path, boundCounter);
}
Expand Down Expand Up @@ -328,8 +328,8 @@ const boundInstruments = new Map();
module.exports.countAllRequests = () => {
return (req, res, next) => {
if (!boundInstruments.has(req.path)) {
const labelSet = meter.labels({ route: req.path });
const boundCounter = requestCount.bind(labelSet);
const labels = { route: req.path };
const boundCounter = requestCount.bind(labels);
boundInstruments.set(req.path, boundCounter);
}
Expand Down
4 changes: 2 additions & 2 deletions getting-started/monitored-example/monitoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ const boundInstruments = new Map();
module.exports.countAllRequests = () => {
return (req, res, next) => {
if (!boundInstruments.has(req.path)) {
const labelSet = meter.labels({ route: req.path });
const boundCounter = requestCount.bind(labelSet);
const labels = { route: req.path };
const boundCounter = requestCount.bind(labels);
boundInstruments.set(req.path, boundCounter);
}

Expand Down
8 changes: 4 additions & 4 deletions getting-started/ts-example/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@ const handles = new Map();
export const countAllRequests = () => {
return (req, res, next) => {
if (!handles.has(req.path)) {
const labelSet = meter.labels({ route: req.path });
const handle = requestCount.bind(labelSet);
const labels = { route: req.path };
const handle = requestCount.bind(labels);
handles.set(req.path, handle);
}
Expand Down Expand Up @@ -326,8 +326,8 @@ const handles = new Map();
export const countAllRequests = () => {
return (req, res, next) => {
if (!handles.has(req.path)) {
const labelSet = meter.labels({ route: req.path });
const handle = requestCount.bind(labelSet);
const labels = { route: req.path };
const handle = requestCount.bind(labels);
handles.set(req.path, handle);
}
Expand Down
4 changes: 2 additions & 2 deletions getting-started/ts-example/monitoring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ const handles = new Map();
export const countAllRequests = () => {
return (req, res, next) => {
if (!handles.has(req.path)) {
const labelSet = meter.labels({ route: req.path });
const handle = requestCount.bind(labelSet);
const labels = { route: req.path };
const handle = requestCount.bind(labels);
handles.set(req.path, handle);
}

Expand Down
10 changes: 1 addition & 9 deletions packages/opentelemetry-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 packages/opentelemetry-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 packages/opentelemetry-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 packages/opentelemetry-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;
}
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
4 changes: 2 additions & 2 deletions packages/opentelemetry-exporter-prometheus/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ const meter = new MeterProvider({

// Now, start recording data
const counter = meter.createCounter('metric_name');
counter.add(10, meter.labels({ [key]: 'value' }));
counter.add(10, { [key]: 'value' });

// Record data using Instrument: It is recommended to keep a reference to the Bound Instrument instead of
// always calling `bind()` method for every operations.
const boundCounter = counter.bind(meter.labels({ [key]: 'value' }));
const boundCounter = counter.bind({ [key]: 'value' });
boundCounter.add(10);

// .. some other work
Expand Down
3 changes: 1 addition & 2 deletions packages/opentelemetry-exporter-prometheus/src/prometheus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,8 @@ export class PrometheusExporter implements MetricExporter {
// TODO: only counter and gauge are implemented in metrics so far
}

private _getLabelValues(keys: string[], values: types.LabelSet) {
private _getLabelValues(keys: string[], labels: types.Labels) {
const labelValues: labelValues = {};
const labels = values.labels;
for (let i = 0; i < keys.length; i++) {
if (labels[keys[i]] !== null) {
labelValues[keys[i]] = labels[keys[i]];
Expand Down
Loading

0 comments on commit 3d26f82

Please sign in to comment.