From dba2d0b06e69d173ab3c07e616ac3b0568696b91 Mon Sep 17 00:00:00 2001 From: Weyert de Boer Date: Mon, 19 Apr 2021 17:43:11 +0100 Subject: [PATCH 1/6] feat: ensure the label names are sanitised Sanitises the attribute names to match the naming convention enforced by Prometheus --- .../src/PrometheusSerializer.ts | 10 +++++--- .../test/PrometheusSerializer.test.ts | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts index 45b1abdad48..d15c842ea35 100644 --- a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts +++ b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts @@ -111,17 +111,19 @@ function stringify( let labelsStr = ''; for (const [key, val] of Object.entries(labels)) { + const santisedLabelName = sanitizePrometheusMetricName(key); hasLabel = true; - labelsStr += `${labelsStr.length > 0 ? ',' : ''}${key}="${escapeLabelValue( - val - )}"`; + labelsStr += `${ + labelsStr.length > 0 ? ',' : '' + }${santisedLabelName}="${escapeLabelValue(val)}"`; } if (additionalLabels) { for (const [key, val] of Object.entries(additionalLabels)) { + const santisedLabelName = sanitizePrometheusMetricName(key); hasLabel = true; labelsStr += `${ labelsStr.length > 0 ? ',' : '' - }${key}="${escapeLabelValue(val)}"`; + }${santisedLabelName}="${escapeLabelValue(val)}"`; } } diff --git a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index 4495154f921..54d39be6198 100644 --- a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -488,5 +488,30 @@ describe('PrometheusSerializer', () => { ); }); }); + + it('should sanitise label names', async () => { + const serializer = new PrometheusSerializer(); + + const meter = new MeterProvider({ + processor: new ExactProcessor(SumAggregator), + }).getMeter('test'); + const counter = meter.createCounter('test') as CounterMetric; + // if you try to use a label name like account-id prometheus will complain + // with an error like: + // error while linting: text format parsing error in line 282: expected '=' after label name, found '-' + counter + .bind(({ + 'account-id': '123456', + } as unknown) as Labels) + .add(1); + const records = await counter.getMetricRecord(); + const record = records[0]; + + const result = serializer.serializeRecord(record.descriptor.name, record); + assert.strictEqual( + result, + `test{account_id="123456"} 1 ${mockedHrTimeMs}\n` + ); + }); }); }); From 713b62adf7fbed232120706b3502b1edd7c1fb03 Mon Sep 17 00:00:00 2001 From: Weyert de Boer Date: Mon, 19 Apr 2021 18:08:33 +0100 Subject: [PATCH 2/6] chore: make new test prt of the `with SumAggregator`-group --- .../test/PrometheusSerializer.test.ts | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index 54d39be6198..8a3ee5b47e4 100644 --- a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -487,31 +487,34 @@ describe('PrometheusSerializer', () => { `} 1 ${mockedHrTimeMs}\n` ); }); - }); - it('should sanitise label names', async () => { - const serializer = new PrometheusSerializer(); + it('should sanitise label names', async () => { + const serializer = new PrometheusSerializer(); + + const meter = new MeterProvider({ + processor: new ExactProcessor(SumAggregator), + }).getMeter('test'); + const counter = meter.createCounter('test') as CounterMetric; + // if you try to use a label name like account-id prometheus will complain + // with an error like: + // error while linting: text format parsing error in line 282: expected '=' after label name, found '-' + counter + .bind(({ + 'account-id': '123456', + } as unknown) as Labels) + .add(1); + const records = await counter.getMetricRecord(); + const record = records[0]; - const meter = new MeterProvider({ - processor: new ExactProcessor(SumAggregator), - }).getMeter('test'); - const counter = meter.createCounter('test') as CounterMetric; - // if you try to use a label name like account-id prometheus will complain - // with an error like: - // error while linting: text format parsing error in line 282: expected '=' after label name, found '-' - counter - .bind(({ - 'account-id': '123456', - } as unknown) as Labels) - .add(1); - const records = await counter.getMetricRecord(); - const record = records[0]; - - const result = serializer.serializeRecord(record.descriptor.name, record); - assert.strictEqual( - result, - `test{account_id="123456"} 1 ${mockedHrTimeMs}\n` - ); + const result = serializer.serializeRecord( + record.descriptor.name, + record + ); + assert.strictEqual( + result, + `test{account_id="123456"} 1 ${mockedHrTimeMs}\n` + ); + }); }); }); }); From caa86e90822fbd9d2ca67db068e87aa62f200972 Mon Sep 17 00:00:00 2001 From: Weyert de Boer Date: Mon, 19 Apr 2021 23:22:42 +0100 Subject: [PATCH 3/6] style: rename sanitize to sanitise in variable name Co-authored-by: Bartlomiej Obecny --- .../src/PrometheusSerializer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts index d15c842ea35..b5a10913e43 100644 --- a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts +++ b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts @@ -111,7 +111,7 @@ function stringify( let labelsStr = ''; for (const [key, val] of Object.entries(labels)) { - const santisedLabelName = sanitizePrometheusMetricName(key); + const sanitisedLabelName = sanitisePrometheusMetricName(key); hasLabel = true; labelsStr += `${ labelsStr.length > 0 ? ',' : '' From cec15b26e5613a928262ebc17baa8452f85e7976 Mon Sep 17 00:00:00 2001 From: Weyert de Boer Date: Mon, 19 Apr 2021 23:44:46 +0100 Subject: [PATCH 4/6] chore: run the `lint:fix`-task to fix formatting --- .../src/PrometheusSerializer.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts index b5a10913e43..2518f719e93 100644 --- a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts +++ b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts @@ -111,19 +111,19 @@ function stringify( let labelsStr = ''; for (const [key, val] of Object.entries(labels)) { - const sanitisedLabelName = sanitisePrometheusMetricName(key); + const santizedLabelName = sanitizePrometheusMetricName(key); hasLabel = true; labelsStr += `${ labelsStr.length > 0 ? ',' : '' - }${santisedLabelName}="${escapeLabelValue(val)}"`; + }${santizedLabelName}="${escapeLabelValue(val)}"`; } if (additionalLabels) { for (const [key, val] of Object.entries(additionalLabels)) { - const santisedLabelName = sanitizePrometheusMetricName(key); + const santizedLabelName = sanitizePrometheusMetricName(key); hasLabel = true; labelsStr += `${ labelsStr.length > 0 ? ',' : '' - }${santisedLabelName}="${escapeLabelValue(val)}"`; + }${santizedLabelName}="${escapeLabelValue(val)}"`; } } From 920c9788b763bfce313f206357486e67c16ec84f Mon Sep 17 00:00:00 2001 From: Weyert de Boer Date: Tue, 20 Apr 2021 15:31:58 +0100 Subject: [PATCH 5/6] style: fixing American spelling continued Co-authored-by: Bartlomiej Obecny --- .../test/PrometheusSerializer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index 8a3ee5b47e4..7a05e52334c 100644 --- a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -488,7 +488,7 @@ describe('PrometheusSerializer', () => { ); }); - it('should sanitise label names', async () => { + it('should sanitize label names', async () => { const serializer = new PrometheusSerializer(); const meter = new MeterProvider({ From e6c81dc611f32b4f4876721bec0cbb58ce0da8a7 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 21 Apr 2021 09:56:49 -0400 Subject: [PATCH 6/6] Fix typo --- .../src/PrometheusSerializer.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts index 2518f719e93..d9b4b0f05cd 100644 --- a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts +++ b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts @@ -111,19 +111,19 @@ function stringify( let labelsStr = ''; for (const [key, val] of Object.entries(labels)) { - const santizedLabelName = sanitizePrometheusMetricName(key); + const sanitizedLabelName = sanitizePrometheusMetricName(key); hasLabel = true; labelsStr += `${ labelsStr.length > 0 ? ',' : '' - }${santizedLabelName}="${escapeLabelValue(val)}"`; + }${sanitizedLabelName}="${escapeLabelValue(val)}"`; } if (additionalLabels) { for (const [key, val] of Object.entries(additionalLabels)) { - const santizedLabelName = sanitizePrometheusMetricName(key); + const sanitizedLabelName = sanitizePrometheusMetricName(key); hasLabel = true; labelsStr += `${ labelsStr.length > 0 ? ',' : '' - }${santizedLabelName}="${escapeLabelValue(val)}"`; + }${sanitizedLabelName}="${escapeLabelValue(val)}"`; } }