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: Rename Labels to Attributes #2523

Merged
merged 6 commits into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import { Meter } from './types/Meter';
import {
MetricOptions,
Labels,
Attributes,
Counter,
Histogram,
ObservableGauge,
Expand Down Expand Up @@ -109,15 +109,15 @@ export class NoopMeter implements Meter {
export class NoopMetric {}

export class NoopCounterMetric extends NoopMetric implements Counter {
add(_value: number, _labels: Labels): void {}
add(_value: number, _attributes: Attributes): void {}
}

export class NoopUpDownCounterMetric extends NoopMetric implements UpDownCounter {
add(_value: number, _labels: Labels): void {}
add(_value: number, _attributes: Attributes): void {}
}

export class NoopHistogramMetric extends NoopMetric implements Histogram {
record(_value: number, _labels: Labels): void {}
record(_value: number, _attributes: Attributes): void {}
}

export class NoopObservableBaseMetric extends NoopMetric implements ObservableBase {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface MeterOptions {
* An interface to allow the recording metrics.
*
* {@link Metric}s are used for recording pre-defined aggregation (`Counter`),
* or raw values (`Histogram`) in which the aggregation and labels
* or raw values (`Histogram`) in which the aggregation and attributes
* for the exported metric are deferred.
*/
export interface Meter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export interface MetricOptions {
*/
unit?: string;

/** The map of constant labels for the Metric. */
constantLabels?: Map<string, string>;
/** The map of constant attributes for the Metric. */
constantAttributes?: Map<string, string>;

/**
* Indicates the metric is a verbose metric that is disabled by default
Expand Down Expand Up @@ -93,30 +93,30 @@ export enum AggregationTemporality {
*/
export interface Counter {
/**
* Adds the given value to the current value. Values cannot be negative.
* Increment value of counter by the input. Inputs may not be negative.
*/
add(value: number, labels?: Labels): void;
add(value: number, attributes?: Attributes): void;
}

export interface UpDownCounter {
/**
* Adds the given value to the current value. Values can be negative.
* Increment value of counter by the input. Inputs may be negative.
*/
add(value: number, labels?: Labels): void;
add(value: number, attributes?: Attributes): void;
}

export interface Histogram {
/**
* Records the given value to this histogram.
*/
record(value: number, labels?: Labels): void;
record(value: number, attributes?: Attributes): void;
}

/** Base interface for the Observable metrics. */
export interface ObservableBase {
observation: (
value: number,
labels?: Labels,
attributes?: Attributes,
) => Observation;
}

Expand All @@ -132,4 +132,4 @@ export type ObservableCounter = ObservableBase;
/**
* key-value pairs passed by the user.
*/
export type Labels = { [key: string]: string };
export type Attributes = { [key: string]: string };
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
* limitations under the License.
*/

import { Labels } from './Metric';
import { Attributes } from './Metric';

/**
* Interface that is being used in callback function for Observable Metric
*/
export interface ObservableResult {
observe(value: number, labels: Labels): void;
observe(value: number, attributes: Attributes): void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ describe('NoopMeter', () => {
it('should not crash', () => {
const meter = new NoopMeterProvider().getMeter('test-noop');
const counter = meter.createCounter('some-name');
const labels = {};
const attributes = {};

// ensure NoopMetric does not crash.
counter.add(1, labels);
counter.add(1, attributes);

// ensure the correct noop const is returned
assert.strictEqual(counter, NOOP_COUNTER_METRIC);

const histogram = meter.createHistogram('some-name');
histogram.record(1, labels);
histogram.record(1, attributes);

// ensure the correct noop const is returned
assert.strictEqual(histogram, NOOP_HISTOGRAM_METRIC);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { SpanAttributes, HrTime } from '@opentelemetry/api';
import { Labels, ValueType } from '@opentelemetry/api-metrics';
import { Attributes as Labels, ValueType } from '@opentelemetry/api-metrics';
Copy link
Contributor

Choose a reason for hiding this comment

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

Newbie question. On line 30-39 it would not change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I left it as Labels in this case, since its using the types defined for the 0.6.0 proto (until #2586 is done, probably). The field is called labels in the TS representation of the protobuf, so I kept it that way.

import * as core from '@opentelemetry/core';
import {
AggregatorKind,
Expand Down Expand Up @@ -54,7 +54,7 @@ export function toAggregationTemporality(
}

/**
* Returns an DataPoint which can have integers or doublle values
* Returns an DataPoint which can have integers or double values
* @param metric
* @param startTime
*/
Expand All @@ -63,7 +63,7 @@ export function toDataPoint(
startTime: number
): otlpTypes.opentelemetryProto.metrics.v1.DataPoint {
return {
labels: toCollectorLabels(metric.labels),
labels: toCollectorLabels(metric.attributes),
value: metric.aggregator.toPoint().value as number,
startTimeUnixNano: startTime,
timeUnixNano: core.hrTimeToNanoseconds(
Expand All @@ -86,7 +86,7 @@ export function toHistogramPoint(
timestamp: HrTime;
};
return {
labels: toCollectorLabels(metric.labels),
labels: toCollectorLabels(metric.attributes),
sum: value.sum,
count: value.count,
startTimeUnixNano: startTime,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe('transformMetrics', () => {
);
});

it('should convert metric labels value to string', () => {
it('should convert metric attributes value to string', () => {
const metric = transform.toCollectorMetric(
{
descriptor: {
Expand All @@ -168,7 +168,7 @@ describe('transformMetrics', () => {
metricKind: 0,
valueType: 0,
},
labels: { foo: (1 as unknown) as string },
attributes: { foo: (1 as unknown) as string },
aggregator: new SumAggregator(),
resource: new Resource({}),
aggregationTemporality: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ interface BatcherCheckpoint {
records: Map<string, MetricRecord>;
}

export class PrometheusLabelsBatcher {
export class PrometheusAttributesBatcher {
private _batchMap = new Map<string, BatcherCheckpoint>();

get hasMetric(): boolean {
Expand All @@ -45,10 +45,10 @@ export class PrometheusLabelsBatcher {
this._batchMap.set(name, item);
}
const recordMap = item.records;
const labels = Object.keys(record.labels)
.map(k => `${k}=${record.labels[k]}`)
const attributes = Object.keys(record.attributes)
.map(k => `${k}=${record.attributes[k]}`)
.join(',');
recordMap.set(labels, record);
recordMap.set(attributes, record);
}

checkPointSet(): PrometheusCheckpoint[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { createServer, IncomingMessage, Server, ServerResponse } from 'http';
import * as url from 'url';
import { ExporterConfig } from './export/types';
import { PrometheusSerializer } from './PrometheusSerializer';
import { PrometheusLabelsBatcher } from './PrometheusLabelsBatcher';
import { PrometheusAttributesBatcher } from './PrometheusAttributesBatcher';

export class PrometheusExporter implements MetricExporter {
static readonly DEFAULT_OPTIONS = {
Expand All @@ -43,10 +43,10 @@ export class PrometheusExporter implements MetricExporter {
private readonly _prefix?: string;
private readonly _appendTimestamp: boolean;
private _serializer: PrometheusSerializer;
private _batcher = new PrometheusLabelsBatcher();
private _batcher = new PrometheusAttributesBatcher();

// This will be required when histogram is implemented. Leaving here so it is not forgotten
// Histogram cannot have a label named 'le'
// Histogram cannot have a attribute named 'le'
// private static readonly RESERVED_HISTOGRAM_LABEL = 'le';

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
MetricKind,
} from '@opentelemetry/sdk-metrics-base';
import { PrometheusCheckpoint } from './types';
import { Labels } from '@opentelemetry/api-metrics';
import { Attributes } from '@opentelemetry/api-metrics';
import { hrTimeToMilliseconds } from '@opentelemetry/core';

type PrometheusDataTypeLiteral =
Expand All @@ -33,7 +33,7 @@ function escapeString(str: string) {
return str.replace(/\\/g, '\\\\').replace(/\n/g, '\\n');
}

function escapeLabelValue(str: string) {
function escapeAttributeValue(str: string) {
if (typeof str !== 'string') {
str = String(str);
}
Expand All @@ -45,7 +45,7 @@ const invalidCharacterRegex = /[^a-z0-9_]/gi;
* Ensures metric names are valid Prometheus metric names by removing
* characters allowed by OpenTelemetry but disallowed by Prometheus.
*
* https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
* https://prometheus.io/docs/concepts/data_model/#metric-names-and-attributes
*
* 1. Names must match `[a-zA-Z_:][a-zA-Z0-9_:]*`
*
Expand Down Expand Up @@ -123,33 +123,33 @@ function toPrometheusType(

function stringify(
metricName: string,
labels: Labels,
attributes: Attributes,
value: number,
timestamp?: number,
additionalLabels?: Labels
additionalAttributes?: Attributes
) {
let hasLabel = false;
let labelsStr = '';
let hasAttribute = false;
let attributesStr = '';

for (const [key, val] of Object.entries(labels)) {
const sanitizedLabelName = sanitizePrometheusMetricName(key);
hasLabel = true;
labelsStr += `${
labelsStr.length > 0 ? ',' : ''
}${sanitizedLabelName}="${escapeLabelValue(val)}"`;
for (const [key, val] of Object.entries(attributes)) {
const sanitizedAttributeName = sanitizePrometheusMetricName(key);
hasAttribute = true;
attributesStr += `${
attributesStr.length > 0 ? ',' : ''
}${sanitizedAttributeName}="${escapeAttributeValue(val)}"`;
}
if (additionalLabels) {
for (const [key, val] of Object.entries(additionalLabels)) {
const sanitizedLabelName = sanitizePrometheusMetricName(key);
hasLabel = true;
labelsStr += `${
labelsStr.length > 0 ? ',' : ''
}${sanitizedLabelName}="${escapeLabelValue(val)}"`;
if (additionalAttributes) {
for (const [key, val] of Object.entries(additionalAttributes)) {
const sanitizedAttributeName = sanitizePrometheusMetricName(key);
hasAttribute = true;
attributesStr += `${
attributesStr.length > 0 ? ',' : ''
}${sanitizedAttributeName}="${escapeAttributeValue(val)}"`;
}
}

if (hasLabel) {
metricName += `{${labelsStr}}`;
if (hasAttribute) {
metricName += `{${attributesStr}}`;
}

return `${metricName} ${valueString(value)}${
Expand Down Expand Up @@ -219,7 +219,7 @@ export class PrometheusSerializer {
const timestamp = hrTimeToMilliseconds(hrtime);
results += stringify(
name,
record.labels,
record.attributes,
value,
this._appendTimestamp ? timestamp : undefined,
undefined
Expand All @@ -233,7 +233,7 @@ export class PrometheusSerializer {
for (const key of ['count', 'sum'] as ('count' | 'sum')[]) {
results += stringify(
name + '_' + key,
record.labels,
record.attributes,
value[key],
this._appendTimestamp ? timestamp : undefined,
undefined
Expand All @@ -260,7 +260,7 @@ export class PrometheusSerializer {
}
results += stringify(
name + '_bucket',
record.labels,
record.attributes,
cumulativeSum,
this._appendTimestamp ? timestamp : undefined,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ export class ExactProcessor<T, R extends Aggregator> extends Processor {
}

process(record: MetricRecord): void {
const labels = Object.keys(record.labels)
.map(k => `${k}=${record.labels[k]}`)
const attributes = Object.keys(record.attributes)
.map(k => `${k}=${record.attributes[k]}`)
.join(',');
this._batchMap.set(record.descriptor.name + labels, record);
this._batchMap.set(record.descriptor.name + attributes, record);
}
}
Loading