Skip to content

Commit

Permalink
feat!: remove label keys as they are no longer part of the spec (#1126)
Browse files Browse the repository at this point in the history
Co-authored-by: Mayur Kale <mayurkale@google.com>
  • Loading branch information
Naseem and mayurkale22 committed Jun 9, 2020
1 parent e93a866 commit c24d992
Show file tree
Hide file tree
Showing 18 changed files with 19 additions and 65 deletions.
1 change: 0 additions & 1 deletion examples/metrics/metrics/observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const meter = new MeterProvider({

const otelCpuUsage = meter.createObserver('metric_observer', {
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 @@ -22,14 +22,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 @@ -254,7 +254,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 @@ -325,7 +324,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 @@ -21,7 +21,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 @@ -253,7 +253,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 @@ -323,7 +322,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 @@ -20,7 +20,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 @@ -37,7 +37,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 @@ -27,7 +27,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 @@ -50,7 +49,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 @@ -82,7 +82,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

0 comments on commit c24d992

Please sign in to comment.