Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove label set from metrics API #915

Merged
merged 4 commits into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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