From 3f69a7279f1815c29ff2908217224233b5587970 Mon Sep 17 00:00:00 2001 From: "marc.pichler" Date: Mon, 14 Nov 2022 11:18:31 +0100 Subject: [PATCH 1/3] fix(sdk-metrics): use default Resource to comply with semantic conventions --- CHANGELOG.md | 7 ++++ .../test/metricsHelper.ts | 4 +- .../test/metricsHelper.ts | 4 +- .../package.json | 1 + .../test/PrometheusExporter.test.ts | 38 ++++++++++--------- .../test/PrometheusSerializer.test.ts | 9 +++-- .../tsconfig.json | 3 ++ packages/sdk-metrics/src/MeterProvider.ts | 3 +- packages/sdk-metrics/test/util.ts | 4 +- 9 files changed, 45 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 545e002cd8..20eb2bcf9c 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 [#3410](https://github.com/open-telemetry/opentelemetry-js/pull/3410) @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..2d08b5e117 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts @@ -29,11 +29,15 @@ import { mockHrTime } from './util'; import { SinonStubbedInstance } from 'sinon'; +import { Resource } from '@opentelemetry/resources'; +import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; -const serializedEmptyResourceLines = [ +const infoLine = `target_info{service_name="${Resource.default().attributes[SemanticResourceAttributes.SERVICE_NAME]}",telemetry_sdk_language="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_LANGUAGE]}",telemetry_sdk_name="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_NAME]}",telemetry_sdk_version="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_VERSION]}"} 1`; + +const serializedDefaultResourceLines = [ '# HELP target_info Target metadata', '# TYPE target_info gauge', - 'target_info 1' + infoLine ]; describe('PrometheusExporter', () => { @@ -261,12 +265,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 +300,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 +320,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 +355,7 @@ describe('PrometheusExporter', () => { const lines = body.split('\n'); assert.deepStrictEqual(lines, [ - ...serializedEmptyResourceLines, + ...serializedDefaultResourceLines, '# no registered metrics' ]); }); @@ -365,7 +369,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 +386,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 +404,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 +433,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 +462,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 +481,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 +535,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 +564,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 +593,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 +622,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..e66a503f4e 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -39,16 +39,17 @@ import { mockHrTime } from './util'; import { Resource } from '@opentelemetry/resources'; +import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; const attributes = { foo1: 'bar1', 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="${Resource.default().attributes[SemanticResourceAttributes.SERVICE_NAME]}",telemetry_sdk_language="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_LANGUAGE]}",telemetry_sdk_name="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_NAME]}",telemetry_sdk_version="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_VERSION]}"} 1\n`; class TestMetricReader extends MetricReader { constructor() { @@ -477,7 +478,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 +492,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/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', From 74b2303e1d1746d9bd84840dcf7e239c230d6e8a Mon Sep 17 00:00:00 2001 From: "marc.pichler" Date: Mon, 14 Nov 2022 14:25:46 +0100 Subject: [PATCH 2/3] fix(prometheus-exporter): escape default resource attribute values --- .../test/PrometheusExporter.test.ts | 10 ++++++---- .../test/PrometheusSerializer.test.ts | 9 ++++++--- .../opentelemetry-exporter-prometheus/test/util.ts | 11 +++++++++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts index 2d08b5e117..31137bd156 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts @@ -26,13 +26,15 @@ import * as http from 'http'; import { PrometheusExporter } from '../src'; import { mockedHrTimeMs, - mockHrTime + mockHrTime, + sdkLanguage, + sdkName, + sdkVersion, + serviceName } from './util'; import { SinonStubbedInstance } from 'sinon'; -import { Resource } from '@opentelemetry/resources'; -import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; -const infoLine = `target_info{service_name="${Resource.default().attributes[SemanticResourceAttributes.SERVICE_NAME]}",telemetry_sdk_language="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_LANGUAGE]}",telemetry_sdk_name="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_NAME]}",telemetry_sdk_version="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_VERSION]}"} 1`; +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', diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index e66a503f4e..a4792eef54 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -36,10 +36,13 @@ import * as sinon from 'sinon'; import { PrometheusSerializer } from '../src'; import { mockedHrTimeMs, - mockHrTime + mockHrTime, + sdkLanguage, + sdkName, + sdkVersion, + serviceName } from './util'; import { Resource } from '@opentelemetry/resources'; -import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; const attributes = { foo1: 'bar1', @@ -49,7 +52,7 @@ const attributes = { const serializedDefaultResource = '# HELP target_info Target metadata\n' + '# TYPE target_info gauge\n' + - `target_info{service_name="${Resource.default().attributes[SemanticResourceAttributes.SERVICE_NAME]}",telemetry_sdk_language="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_LANGUAGE]}",telemetry_sdk_name="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_NAME]}",telemetry_sdk_version="${Resource.default().attributes[SemanticResourceAttributes.TELEMETRY_SDK_VERSION]}"} 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() { 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'); From 53070dbdd067f5fee0cd88462c41a9fa4ad3ba1e Mon Sep 17 00:00:00 2001 From: "marc.pichler" Date: Tue, 15 Nov 2022 10:07:39 +0100 Subject: [PATCH 3/3] fix(changelog): use correct pr number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20eb2bcf9c..d605eff3e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/ ### :bug: (Bug Fix) -* fix(sdk-metrics): use default Resource to comply with semantic conventions [#3410](https://github.com/open-telemetry/opentelemetry-js/pull/3410) @pichlermarc +* 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`