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

feat!: remove label keys as they are no longer part of the spec #1126

Merged
merged 9 commits into from
Jun 9, 2020
1 change: 0 additions & 1 deletion examples/metrics/metrics/observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const meter = new MeterProvider({

const otelCpuUsage = meter.createObserver('metric_observer', {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, how am I able to create a labels at this moment after this change. What would be a correct way of doing it now, if for any reason I want to have labels ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't. Labels are now applied only by binding them to an instrument, or calling the instrument method with labels as the second argument like add(10, { core: 1 })

monotonic: false,
labelKeys: ['pid', 'core'],
description: 'Example of a observer',
});

Expand Down
2 changes: 0 additions & 2 deletions examples/prometheus/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@ const meter = new MeterProvider({
// Monotonic counters can only be increased.
const monotonicCounter = meter.createCounter('monotonic_counter', {
monotonic: true,
labelKeys: ['pid'],
description: 'Example of a monotonic counter',
});

// Non-monotonic counters can be increased or decreased.
const nonMonotonicCounter = meter.createCounter('non_monotonic_counter', {
monotonic: false,
labelKeys: ['pid'],
description: 'Example of a non-monotonic counter',
});

Expand Down
2 changes: 0 additions & 2 deletions getting-started/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ const meter = new MeterProvider().getMeter('your-meter-name');

const requestCount = meter.createCounter("requests", {
monotonic: true,
labelKeys: ["route"],
description: "Count all incoming requests"
});

Expand Down Expand Up @@ -319,7 +318,6 @@ const meter = new MeterProvider({

const requestCount = meter.createCounter("requests", {
monotonic: true,
labelKeys: ["route"],
description: "Count all incoming requests"
});

Expand Down
1 change: 0 additions & 1 deletion getting-started/monitored-example/monitoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const meter = new MeterProvider({

const requestCount = meter.createCounter("requests", {
monotonic: true,
labelKeys: ["route"],
description: "Count all incoming requests"
});

Expand Down
2 changes: 0 additions & 2 deletions getting-started/ts-example/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ const meter = new MeterProvider().getMeter('your-meter-name');

const requestCount: Metric<BoundCounter> = meter.createCounter("requests", {
monotonic: true,
labelKeys: ["route"],
description: "Count all incoming requests"
});

Expand Down Expand Up @@ -317,7 +316,6 @@ const meter = new MeterProvider({

const requestCount: Metric<BoundCounter> = meter.createCounter("requests", {
monotonic: true,
labelKeys: ["route"],
description: "Count all incoming requests"
});

Expand Down
1 change: 0 additions & 1 deletion getting-started/ts-example/monitoring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const meter = new MeterProvider({

const requestCount: Metric<BoundCounter> = meter.createCounter("requests", {
monotonic: true,
labelKeys: ["route"],
description: "Count all incoming requests"
});

Expand Down
3 changes: 0 additions & 3 deletions packages/opentelemetry-api/src/metrics/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,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<string, string>;

Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-exporter-prometheus/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const meter = new MeterProvider({

// Now, start recording data
const counter = meter.createCounter('metric_name', {
labelKeys: ['pid'],
description: 'Example of a counter'
});
counter.add(10, { pid: process.pid });
Expand Down
30 changes: 8 additions & 22 deletions packages/opentelemetry-exporter-prometheus/src/prometheus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
} from '@opentelemetry/metrics';
import * as api from '@opentelemetry/api';
import { createServer, IncomingMessage, Server, ServerResponse } from 'http';
import { Counter, Gauge, labelValues, Metric, Registry } from 'prom-client';
import { Counter, Gauge, Metric, Registry } from 'prom-client';
import * as url from 'url';
import { ExporterConfig } from './export/types';

Expand Down Expand Up @@ -126,30 +126,28 @@ export class PrometheusExporter implements MetricExporter {
const metric = this._registerMetric(record);
if (!metric) return;

const labelValues = this._getLabelValues(
record.descriptor.labelKeys,
record.labels
);
const point = record.aggregator.toPoint();

const labels = record.labels;

if (metric instanceof Counter) {
// Prometheus counter saves internal state and increments by given value.
// MetricRecord value is the current state, not the delta to be incremented by.
// Currently, _registerMetric creates a new counter every time the value changes,
// so the increment here behaves as a set value (increment from 0)
metric.inc(
labelValues,
labels,
point.value as Sum,
hrTimeToMilliseconds(point.timestamp)
);
}

if (metric instanceof Gauge) {
if (record.aggregator instanceof CounterSumAggregator) {
metric.set(labelValues, point.value as Sum);
metric.set(labels, point.value as Sum);
} else if (record.aggregator instanceof ObserverAggregator) {
metric.set(
labelValues,
labels,
point.value as LastValue,
hrTimeToMilliseconds(point.timestamp)
);
Expand All @@ -159,16 +157,6 @@ export class PrometheusExporter implements MetricExporter {
// TODO: only counter and gauge are implemented in metrics so far
}

private _getLabelValues(keys: string[], labels: api.Labels) {
const labelValues: labelValues = {};
for (let i = 0; i < keys.length; i++) {
if (labels[keys[i]] !== null) {
labelValues[keys[i]] = labels[keys[i]];
}
}
return labelValues;
}

private _registerMetric(record: MetricRecord): Metric | undefined {
const metricName = this._getPrometheusMetricName(record.descriptor);
const metric = this._registry.getSingleMetric(metricName);
Expand All @@ -183,9 +171,7 @@ export class PrometheusExporter implements MetricExporter {
* https://prometheus.io/docs/instrumenting/exposition_formats/
*/
if (metric instanceof Counter) {
metric.remove(
...record.descriptor.labelKeys.map(k => record.labels[k].toString())
);
metric.remove(...Object.values(record.labels));
}

if (metric) return metric;
Expand All @@ -198,7 +184,7 @@ export class PrometheusExporter implements MetricExporter {
name,
// prom-client throws with empty description which is our default
help: record.descriptor.description || 'description missing',
labelNames: record.descriptor.labelKeys,
labelNames: Object.keys(record.labels),
// list of registries to register the newly created metric
registers: [this._registry],
};
Expand Down
19 changes: 9 additions & 10 deletions packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ describe('PrometheusExporter', () => {
it('should export a count aggregation', done => {
const counter = meter.createCounter('counter', {
description: 'a test description',
labelKeys: ['key1'],
});

const boundCounter = counter.bind({ key1: 'labelValue1' });
Expand Down Expand Up @@ -245,7 +244,6 @@ describe('PrometheusExporter', () => {

const observer = meter.createObserver('metric_observer', {
description: 'a test description',
labelKeys: ['pid'],
}) as ObserverMetric;

observer.setCallback((observerResult: ObserverResult) => {
Expand All @@ -269,7 +267,10 @@ describe('PrometheusExporter', () => {
assert.strictEqual(lines[1], '# TYPE metric_observer gauge');

const line3 = lines[2].split(' ');
assert.strictEqual(line3[0], 'metric_observer{pid="123"}');
assert.strictEqual(
line3[0],
'metric_observer{pid="123",core="1"}'
);
assert.ok(
parseFloat(line3[1]) >= 0 && parseFloat(line3[1]) <= 1
);
Expand All @@ -286,7 +287,6 @@ describe('PrometheusExporter', () => {
it('should export multiple labels', done => {
const counter = meter.createCounter('counter', {
description: 'a test description',
labelKeys: ['counterKey1'],
}) as CounterMetric;

counter.bind({ counterKey1: 'labelValue1' }).add(10);
Expand Down Expand Up @@ -347,7 +347,7 @@ describe('PrometheusExporter', () => {
assert.deepStrictEqual(lines, [
'# HELP counter description missing',
'# TYPE counter counter',
`counter 10 ${mockedTimeMS}`,
`counter{key1="labelValue1"} 10 ${mockedTimeMS}`,
'',
]);

Expand All @@ -373,7 +373,7 @@ describe('PrometheusExporter', () => {
assert.deepStrictEqual(lines, [
'# HELP counter_bad_name description missing',
'# TYPE counter_bad_name counter',
`counter_bad_name 10 ${mockedTimeMS}`,
`counter_bad_name{key1="labelValue1"} 10 ${mockedTimeMS}`,
'',
]);

Expand All @@ -388,7 +388,6 @@ describe('PrometheusExporter', () => {
const counter = meter.createCounter('counter', {
description: 'a test description',
monotonic: false,
labelKeys: ['key1'],
});

counter.bind({ key1: 'labelValue1' }).add(20);
Expand Down Expand Up @@ -449,7 +448,7 @@ describe('PrometheusExporter', () => {
assert.deepStrictEqual(lines, [
'# HELP test_prefix_counter description missing',
'# TYPE test_prefix_counter counter',
`test_prefix_counter 10 ${mockedTimeMS}`,
`test_prefix_counter{key1="labelValue1"} 10 ${mockedTimeMS}`,
'',
]);

Expand Down Expand Up @@ -478,7 +477,7 @@ describe('PrometheusExporter', () => {
assert.deepStrictEqual(lines, [
'# HELP counter description missing',
'# TYPE counter counter',
`counter 10 ${mockedTimeMS}`,
`counter{key1="labelValue1"} 10 ${mockedTimeMS}`,
'',
]);

Expand Down Expand Up @@ -507,7 +506,7 @@ describe('PrometheusExporter', () => {
assert.deepStrictEqual(lines, [
'# HELP counter description missing',
'# TYPE counter counter',
`counter 10 ${mockedTimeMS}`,
`counter{key1="labelValue1"} 10 ${mockedTimeMS}`,
'',
]);

Expand Down
2 changes: 0 additions & 2 deletions packages/opentelemetry-metrics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const { MeterProvider } = require('@opentelemetry/metrics');
const meter = new MeterProvider().getMeter('your-meter-name');

const counter = meter.createCounter('metric_name', {
labelKeys: ['pid'],
description: 'Example of a counter'
});

Expand All @@ -47,7 +46,6 @@ const { MeterProvider, MetricObservable } = require('@opentelemetry/metrics');
const meter = new MeterProvider().getMeter('your-meter-name');

const observer = meter.createObserver('metric_name', {
labelKeys: ['pid', 'core'],
description: 'Example of a observer'
});

Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-metrics/src/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ export abstract class Metric<T extends BaseBoundInstrument>
unit: this._options.unit,
metricKind: this._kind,
valueType: this._valueType,
labelKeys: this._options.labelKeys,
monotonic: this._monotonic,
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-metrics/src/export/Batcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class UngroupedBatcher extends Batcher {
}

process(record: MetricRecord): void {
const labels = record.descriptor.labelKeys
const labels = Object.keys(record.labels)
.map(k => `${k}=${record.labels[k]}`)
.join(',');
this._batchMap.set(record.descriptor.name + labels, record);
Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-metrics/src/export/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ export interface MetricDescriptor {
readonly unit: string;
readonly metricKind: MetricKind;
readonly valueType: ValueType;
readonly labelKeys: string[];
readonly monotonic: boolean;
}

Expand Down
4 changes: 0 additions & 4 deletions packages/opentelemetry-metrics/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,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<string, string>;

Expand Down Expand Up @@ -84,6 +81,5 @@ export const DEFAULT_METRIC_OPTIONS = {
disabled: false,
description: '',
unit: '1',
labelKeys: [],
valueType: ValueType.DOUBLE,
};
4 changes: 1 addition & 3 deletions packages/opentelemetry-metrics/test/Batcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ describe('Batcher', () => {
logger: new NoopLogger(),
interval: 10000,
}).getMeter('test-meter');
counter = meter.createCounter('ungrouped-batcher-test', {
labelKeys: ['key'],
});
counter = meter.createCounter('ungrouped-batcher-test');
fooCounter = counter.bind({ key: 'foo' });
barCounter = counter.bind({ key: 'bar' });
});
Expand Down
6 changes: 0 additions & 6 deletions packages/opentelemetry-metrics/test/Meter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ describe('Meter', () => {
assert.strictEqual(record.length, 1);
assert.deepStrictEqual(record[0].descriptor, {
description: '',
labelKeys: [],
metricKind: MetricKind.COUNTER,
monotonic: true,
name: 'name1',
Expand Down Expand Up @@ -487,7 +486,6 @@ describe('Meter', () => {
it('should set callback and observe value ', () => {
const valueRecorder = meter.createObserver('name', {
description: 'desc',
labelKeys: ['pid', 'core'],
}) as ObserverMetric;

function getCpuUsage() {
Expand Down Expand Up @@ -546,7 +544,6 @@ describe('Meter', () => {
const key = 'key';
const counter = meter.createCounter('counter', {
description: 'test',
labelKeys: [key],
});
const labels = { [key]: 'counter-value' };
const boundCounter = counter.bind(labels);
Expand All @@ -563,7 +560,6 @@ describe('Meter', () => {
monotonic: true,
unit: '1',
valueType: ValueType.DOUBLE,
labelKeys: ['key'],
});
assert.strictEqual(record[0].labels, labels);
const value = record[0].aggregator.toPoint().value as Sum;
Expand All @@ -574,7 +570,6 @@ describe('Meter', () => {
const key = 'key';
const counter = meter.createCounter('counter', {
description: 'test',
labelKeys: [key],
valueType: api.ValueType.INT,
});
const labels = { [key]: 'counter-value' };
Expand All @@ -592,7 +587,6 @@ describe('Meter', () => {
monotonic: true,
unit: '1',
valueType: ValueType.INT,
labelKeys: ['key'],
});
assert.strictEqual(record[0].labels, labels);
const value = record[0].aggregator.toPoint().value as Sum;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ describe('ConsoleMetricExporter', () => {
);
const counter = meter.createCounter('counter', {
description: 'a test description',
labelKeys: ['key1', 'key2'],
});
const boundCounter = counter.bind({
key1: 'labelValue1',
Expand All @@ -57,7 +56,6 @@ describe('ConsoleMetricExporter', () => {
assert.deepStrictEqual(descriptor, [
{
description: 'a test description',
labelKeys: ['key1', 'key2'],
metricKind: MetricKind.COUNTER,
monotonic: true,
name: 'counter',
Expand Down