From 125f11e721a49119cd5cae437213cf394ae96ea1 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 15 Nov 2022 16:43:41 +0100 Subject: [PATCH] fix(sdk-metrics): use default Resource to comply with semconv (#3411) * 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 --- CHANGELOG.md | 7 ++++ .../test/metricsHelper.ts | 4 +- .../test/metricsHelper.ts | 4 +- .../package.json | 1 + .../test/PrometheusExporter.test.ts | 42 +++++++++++-------- .../test/PrometheusSerializer.test.ts | 14 ++++--- .../test/util.ts | 11 +++++ .../tsconfig.json | 3 ++ packages/sdk-metrics/src/MeterProvider.ts | 3 +- packages/sdk-metrics/test/util.ts | 4 +- 10 files changed, 63 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 545e002cd8..d605eff3e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts index e7f2baa351..9a11c8b6f2 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts @@ -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 }); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts index ce3a70ff24..d86775d332 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts @@ -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 }); diff --git a/experimental/packages/opentelemetry-exporter-prometheus/package.json b/experimental/packages/opentelemetry-exporter-prometheus/package.json index 8d6c32a868..0a36b920e4 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/package.json +++ b/experimental/packages/opentelemetry-exporter-prometheus/package.json @@ -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", diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts index 3104348300..31137bd156 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts @@ -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', () => { @@ -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}`, @@ -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}`, @@ -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}`, @@ -351,7 +357,7 @@ describe('PrometheusExporter', () => { const lines = body.split('\n'); assert.deepStrictEqual(lines, [ - ...serializedEmptyResourceLines, + ...serializedDefaultResourceLines, '# no registered metrics' ]); }); @@ -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}`, @@ -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}`, @@ -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}`, @@ -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}`, @@ -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}`, @@ -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}`, @@ -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}`, @@ -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}`, @@ -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}`, @@ -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', diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index 2694c96f83..a4792eef54 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -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'; @@ -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() { @@ -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' + @@ -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` diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/util.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/util.ts index 49536a5a35..8caa7795cd 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/util.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/util.ts @@ -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; @@ -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'); diff --git a/experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json b/experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json index f0e9f90992..cd21ec8274 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json +++ b/experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json @@ -18,6 +18,9 @@ { "path": "../../../packages/opentelemetry-resources" }, + { + "path": "../../../packages/opentelemetry-semantic-conventions" + }, { "path": "../../../packages/sdk-metrics" } diff --git a/packages/sdk-metrics/src/MeterProvider.ts b/packages/sdk-metrics/src/MeterProvider.ts index 01c3f54a1b..13b17fb564 100644 --- a/packages/sdk-metrics/src/MeterProvider.ts +++ b/packages/sdk-metrics/src/MeterProvider.ts @@ -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); diff --git a/packages/sdk-metrics/test/util.ts b/packages/sdk-metrics/test/util.ts index 80f9ab1c49..7227698a93 100644 --- a/packages/sdk-metrics/test/util.ts +++ b/packages/sdk-metrics/test/util.ts @@ -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',