Skip to content

Commit

Permalink
fix(sdk-metrics): use default Resource to comply with semconv (#3411)
Browse files Browse the repository at this point in the history 
* fix(sdk-metrics): use default Resource to comply with semantic conventions

* fix(prometheus-exporter): escape default resource attribute values

* fix(changelog): use correct pr number
  • Loading branch information
@pichlermarc
pichlermarc authored Nov 15, 2022
1 parent 1b7246a commit 125f11e
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 30 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :bug: (Bug Fix)

* fix(sdk-metrics): use default Resource to comply with semantic conventions [#3411](https://github.com/open-telemetry/opentelemetry-js/pull/3411) @pichlermarc
* Metrics exported by the SDK now contain the following resource attributes by default:
* `service.name`
* `telemetry.sdk.name`
* `telemetry.sdk.language`
* `telemetry.sdk.version`

### :books: (Refine Doc)

### :house: (Internal)
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ class TestMetricReader extends MetricReader {
}
}

const testResource = Resource.default().merge(new Resource({
const testResource = new Resource({
service: 'ui',
version: 1,
cost: 112.12,
}));
});

let meterProvider = new MeterProvider({ resource: testResource });

Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ export class TestMetricReader extends MetricReader {
}
}

const testResource = Resource.default().merge(new Resource({
const testResource = new Resource({
service: 'ui',
version: 1,
cost: 112.12,
}));
});

let meterProvider = new MeterProvider({ resource: testResource });

Expand Down
1 change: 1 addition & 0 deletions experimental/packages/opentelemetry-exporter-prometheus/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
},
"devDependencies": {
"@opentelemetry/api": "^1.3.0",
"@opentelemetry/semantic-conventions": "1.8.0",
"@types/mocha": "10.0.0",
"@types/node": "18.6.5",
"@types/sinon": "10.0.13",
Expand Down
42 changes: 24 additions & 18 deletions experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,20 @@ import * as http from 'http';
import { PrometheusExporter } from '../src';
import {
mockedHrTimeMs,
mockHrTime
mockHrTime,
sdkLanguage,
sdkName,
sdkVersion,
serviceName
} from './util';
import { SinonStubbedInstance } from 'sinon';

const serializedEmptyResourceLines = [
const infoLine = `target_info{service_name="${serviceName}",telemetry_sdk_language="${sdkLanguage}",telemetry_sdk_name="${sdkName}",telemetry_sdk_version="${sdkVersion}"} 1`;

const serializedDefaultResourceLines = [
'# HELP target_info Target metadata',
'# TYPE target_info gauge',
'target_info 1'
infoLine
];

describe('PrometheusExporter', () => {
Expand Down Expand Up @@ -261,12 +267,12 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.strictEqual(
lines[serializedEmptyResourceLines.length],
lines[serializedDefaultResourceLines.length],
'# HELP counter_total a test description'
);

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP counter_total a test description',
'# TYPE counter_total counter',
`counter_total{key1="attributeValue1"} 10 ${mockedHrTimeMs}`,
Expand Down Expand Up @@ -296,7 +302,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP metric_observable_gauge a test description',
'# TYPE metric_observable_gauge gauge',
`metric_observable_gauge{pid="123",core="1"} 0.999 ${mockedHrTimeMs}`,
Expand All @@ -316,7 +322,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP counter_total a test description',
'# TYPE counter_total counter',
`counter_total{counterKey1="attributeValue1"} 10 ${mockedHrTimeMs}`,
Expand Down Expand Up @@ -351,7 +357,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# no registered metrics'
]);
});
Expand All @@ -365,7 +371,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP counter_total description missing',
'# TYPE counter_total counter',
`counter_total{key1="attributeValue1"} 10 ${mockedHrTimeMs}`,
Expand All @@ -382,7 +388,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP counter_bad_name_total description missing',
'# TYPE counter_bad_name_total counter',
`counter_bad_name_total{key1="attributeValue1"} 10 ${mockedHrTimeMs}`,
Expand All @@ -400,7 +406,7 @@ describe('PrometheusExporter', () => {
const body = await request('http://localhost:9464/metrics');
const lines = body.split('\n');
assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP counter a test description',
'# TYPE counter gauge',
`counter{key1="attributeValue1"} 20 ${mockedHrTimeMs}`,
Expand Down Expand Up @@ -429,7 +435,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP metric_observable_counter a test description',
'# TYPE metric_observable_counter counter',
`metric_observable_counter{key1="attributeValue1"} 20 ${mockedHrTimeMs}`,
Expand Down Expand Up @@ -458,7 +464,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP metric_observable_up_down_counter a test description',
'# TYPE metric_observable_up_down_counter gauge',
`metric_observable_up_down_counter{key1="attributeValue1"} 20 ${mockedHrTimeMs}`,
Expand All @@ -477,7 +483,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP test_histogram a test description',
'# TYPE test_histogram histogram',
`test_histogram_count{key1="attributeValue1"} 1 ${mockedHrTimeMs}`,
Expand Down Expand Up @@ -531,7 +537,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP test_prefix_counter_total description missing',
'# TYPE test_prefix_counter_total counter',
`test_prefix_counter_total{key1="attributeValue1"} 10 ${mockedHrTimeMs}`,
Expand Down Expand Up @@ -560,7 +566,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP counter_total description missing',
'# TYPE counter_total counter',
`counter_total{key1="attributeValue1"} 10 ${mockedHrTimeMs}`,
Expand Down Expand Up @@ -589,7 +595,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP counter_total description missing',
'# TYPE counter_total counter',
`counter_total{key1="attributeValue1"} 10 ${mockedHrTimeMs}`,
Expand Down Expand Up @@ -618,7 +624,7 @@ describe('PrometheusExporter', () => {
const lines = body.split('\n');

assert.deepStrictEqual(lines, [
...serializedEmptyResourceLines,
...serializedDefaultResourceLines,
'# HELP counter_total description missing',
'# TYPE counter_total counter',
'counter_total{key1="attributeValue1"} 10',
Expand Down
14 changes: 9 additions & 5 deletions experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ import * as sinon from 'sinon';
import { PrometheusSerializer } from '../src';
import {
mockedHrTimeMs,
mockHrTime
mockHrTime,
sdkLanguage,
sdkName,
sdkVersion,
serviceName
} from './util';
import { Resource } from '@opentelemetry/resources';

Expand All @@ -45,10 +49,10 @@ const attributes = {
foo2: 'bar2',
};

const serializedEmptyResource =
const serializedDefaultResource =
'# HELP target_info Target metadata\n' +
'# TYPE target_info gauge\n' +
'target_info 1\n';
`target_info{service_name="${serviceName}",telemetry_sdk_language="${sdkLanguage}",telemetry_sdk_name="${sdkName}",telemetry_sdk_version="${sdkVersion}"} 1\n`;

class TestMetricReader extends MetricReader {
constructor() {
Expand Down Expand Up @@ -477,7 +481,7 @@ describe('PrometheusSerializer', () => {
const result = await getCounterResult('test', serializer, { unit: unitOfMetric, exportAll: true });
assert.strictEqual(
result,
serializedEmptyResource +
serializedDefaultResource +
'# HELP test_total description missing\n' +
`# UNIT test_total ${unitOfMetric}\n` +
'# TYPE test_total counter\n' +
Expand All @@ -491,7 +495,7 @@ describe('PrometheusSerializer', () => {
const result = await getCounterResult('test', serializer, { exportAll: true });
assert.strictEqual(
result,
serializedEmptyResource +
serializedDefaultResource +
'# HELP test_total description missing\n' +
'# TYPE test_total counter\n' +
`test_total 1 ${mockedHrTimeMs}\n`
Expand Down
11 changes: 11 additions & 0 deletions experimental/packages/opentelemetry-exporter-prometheus/test/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

import * as sinon from 'sinon';
import * as perf_hooks from 'perf_hooks';
import { Resource } from '@opentelemetry/resources';
import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';

export const mockedHrTimeMs = 1586347902211;

Expand All @@ -25,3 +27,12 @@ export function mockHrTime() {
sinon.stub(perf_hooks.performance, 'timeOrigin').value(0);
sinon.stub(perf_hooks.performance, 'now').returns(mockedHrTimeMs);
}

export const serviceName = Resource.default().attributes[SemanticResourceAttributes.SERVICE_NAME]?.toString()
.replace(/\\/g, '\\\\').replace(/\n/g, '\\n');
export const sdkLanguage = Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_LANGUAGE]?.toString()
.replace(/\\/g, '\\\\').replace(/\n/g, '\\n');
export const sdkName = Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_NAME]?.toString()
.replace(/\\/g, '\\\\').replace(/\n/g, '\\n');
export const sdkVersion = Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_VERSION]?.toString()
.replace(/\\/g, '\\\\').replace(/\n/g, '\\n');
3 changes: 3 additions & 0 deletions experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
{
"path": "../../../packages/opentelemetry-resources"
},
{
"path": "../../../packages/opentelemetry-semantic-conventions"
},
{
"path": "../../../packages/sdk-metrics"
}
Expand Down
3 changes: 2 additions & 1 deletion packages/sdk-metrics/src/MeterProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ export class MeterProvider implements IMeterProvider {
private _shutdown = false;

constructor(options?: MeterProviderOptions) {
this._sharedState = new MeterProviderSharedState(options?.resource ?? Resource.empty());
const resource = Resource.default().merge(options?.resource ?? Resource.empty());
this._sharedState = new MeterProviderSharedState(resource);
if(options?.views != null && options.views.length > 0){
for(const view of options.views){
this._sharedState.viewRegistry.addView(view);
Expand Down
4 changes: 2 additions & 2 deletions packages/sdk-metrics/test/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ export type Measurement = {
context?: Context;
};

export const defaultResource = new Resource({
export const defaultResource = Resource.default().merge(new Resource({
resourceKey: 'my-resource',
});
}));

export const defaultInstrumentDescriptor: InstrumentDescriptor = {
name: 'default_metric',
Expand Down

0 comments on commit 125f11e

Please sign in to comment.