From 266dcd7416a854da67f8b2ebadc21d0a079fb1f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20F=C3=A4cke?= Date: Sun, 16 Jun 2019 11:28:51 +0200 Subject: [PATCH 1/5] improve performance of registry.metrics() --- lib/counter.js | 8 +- lib/formatter.js | 72 +++++++++++++ lib/gauge.js | 8 +- lib/histogram.js | 5 +- lib/metric.js | 11 +- lib/registry.js | 52 +--------- lib/summary.js | 5 +- lib/util.js | 9 ++ package.json | 1 + test/registerTest.js | 233 +++++++++++++++---------------------------- 10 files changed, 192 insertions(+), 212 deletions(-) create mode 100644 lib/formatter.js diff --git a/lib/counter.js b/lib/counter.js index b7bcc726..75f3f384 100644 --- a/lib/counter.js +++ b/lib/counter.js @@ -4,12 +4,16 @@ 'use strict'; const util = require('util'); -const type = 'counter'; const { hashObject, isObject, getLabels, removeLabels } = require('./util'); const { validateLabel } = require('./validation'); const { Metric } = require('./metric'); class Counter extends Metric { + constructor(...args) { + super(...args); + this.type = 'counter'; + } + /** * Increment counter * @param {object} labels - What label you want to be incremented @@ -37,7 +41,7 @@ class Counter extends Metric { return { help: this.help, name: this.name, - type, + type: this.type, values: Object.values(this.hashMap), aggregator: this.aggregator, }; diff --git a/lib/formatter.js b/lib/formatter.js new file mode 100644 index 00000000..c36a86dc --- /dev/null +++ b/lib/formatter.js @@ -0,0 +1,72 @@ +'use strict'; + +const generateFunction = require('generate-function'); + +function getFormatter(metric, defaultLabels) { + const defaultLabelNames = Object.keys(defaultLabels); + const name = metric.name; + const help = escapeString(metric.help); + const type = metric.type; + const labelsCode = getLabelsCode(metric, defaultLabelNames, defaultLabels); + + const gen = generateFunction(); + gen(` + function format(item, escapeLabelValue, getValueAsString) { + const info = '# HELP ${name} ${help}\\n# TYPE ${name} ${type}\\n'; + let values = ''; + for (const val of item.values || []) { + val.labels = val.labels || {}; + let metricName = val.metricName || '${name}'; + const labels = \`${labelsCode}\`; + const hasLabels = labels.length > 0; + metricName += \`\${hasLabels ? '{' : ''}\${labels}\${hasLabels ? '}' : ''}\`; + let line = \`\${metricName} \${getValueAsString(val.value)}\`; + values += \`\${line}\\n\`; + } + return info + values; + }`); + return gen.toFunction(); +} + +function getLabelsCode(metric, defaultLabelNames, defaultLabels) { + let labelString = ''; + const labelNames = getLabelNames(metric, defaultLabelNames); + for (let index = 0; index < labelNames.length; index++) { + const labelName = labelNames[index]; + if (labelName === 'quantile') { + labelString += `\${val.labels.quantile != null ? \`quantile="\${escapeLabelValue(val.labels.quantile)}"\` : ''}`; + } else if (labelName === 'le') { + labelString += `\${val.labels.le != null ? \`le="\${escapeLabelValue(val.labels.le)}"\` : ''}`; + } else { + labelString += `${labelName}="\${val.labels['${labelName}'] ? escapeLabelValue(val.labels['${labelName}']) : escapeLabelValue('${defaultLabels[labelName]}')}"`; + } + if (index !== labelNames.length - 1 && labelString.length > 0) { + labelString += ','; + } + } + return labelString; +} + +function getLabelNames(metric, defaultLabelNames) { + const labelNames = [...metric.labelNames]; + for (const labelName of defaultLabelNames) { + if (!labelNames.includes(labelName)) { + labelNames.push(labelName); + } + } + if (metric.type === 'summary') { + labelNames.push('quantile'); + } + if (metric.type === 'histogram') { + labelNames.push('le'); + } + return labelNames; +} + +function escapeString(str) { + return str.replace(/\n/g, '\\\\n').replace(/\\(?!n)/g, '\\\\\\\\'); +} + +module.exports = { + getFormatter, +}; diff --git a/lib/gauge.js b/lib/gauge.js index 8eaf19a9..ce3f95b5 100644 --- a/lib/gauge.js +++ b/lib/gauge.js @@ -4,7 +4,6 @@ 'use strict'; const util = require('util'); -const type = 'gauge'; const { setValue, @@ -17,6 +16,11 @@ const { validateLabel } = require('./validation'); const { Metric } = require('./metric'); class Gauge extends Metric { + constructor(...args) { + super(...args); + this.type = 'gauge'; + } + /** * Set a gauge to a value * @param {object} labels - Object with labels and their values @@ -85,7 +89,7 @@ class Gauge extends Metric { return { help: this.help, name: this.name, - type, + type: this.type, values: Object.values(this.hashMap), aggregator: this.aggregator, }; diff --git a/lib/histogram.js b/lib/histogram.js index 24983c84..9a18fb64 100644 --- a/lib/histogram.js +++ b/lib/histogram.js @@ -4,7 +4,6 @@ 'use strict'; const util = require('util'); -const type = 'histogram'; const { getLabels, hashObject, isObject, removeLabels } = require('./util'); const { validateLabel } = require('./validation'); const { Metric } = require('./metric'); @@ -38,6 +37,8 @@ class Histogram extends Metric { ), }; } + + this.type = 'histogram'; } /** @@ -59,7 +60,7 @@ class Histogram extends Metric { return { name: this.name, help: this.help, - type, + type: this.type, values, aggregator: this.aggregator, }; diff --git a/lib/metric.js b/lib/metric.js index 593eee3b..d9ff960c 100644 --- a/lib/metric.js +++ b/lib/metric.js @@ -1,7 +1,8 @@ 'use strict'; const { globalRegistry } = require('./registry'); -const { isObject } = require('./util'); +const { isObject, getValueAsString, escapeLabelValue } = require('./util'); +const { getFormatter } = require('./formatter'); const { validateMetricName, validateLabelName } = require('./validation'); /** @@ -46,6 +47,14 @@ class Metric { } } + getPrometheusString(defaultLabels) { + const item = this.get(); + if (!this.formatter) { + this.formatter = getFormatter(this, defaultLabels); + } + return this.formatter(item, escapeLabelValue, getValueAsString); + } + reset() { /* abstract */ } diff --git a/lib/registry.js b/lib/registry.js index 09a8a1d9..634c6f61 100644 --- a/lib/registry.js +++ b/lib/registry.js @@ -1,15 +1,4 @@ 'use strict'; -const { getValueAsString } = require('./util'); - -function escapeString(str) { - return str.replace(/\n/g, '\\n').replace(/\\(?!n)/g, '\\\\'); -} -function escapeLabelValue(str) { - if (typeof str !== 'string') { - return str; - } - return escapeString(str).replace(/"/g, '\\"'); -} class Registry { constructor() { @@ -23,44 +12,7 @@ class Registry { } getMetricAsPrometheusString(metric) { - const item = metric.get(); - const name = escapeString(item.name); - const help = `# HELP ${name} ${escapeString(item.help)}`; - const type = `# TYPE ${name} ${item.type}`; - const defaultLabelNames = Object.keys(this._defaultLabels); - - let values = ''; - for (const val of item.values || []) { - val.labels = val.labels || {}; - - if (defaultLabelNames.length > 0) { - // Make a copy before mutating - val.labels = Object.assign({}, val.labels); - - for (const labelName of defaultLabelNames) { - val.labels[labelName] = - val.labels[labelName] || this._defaultLabels[labelName]; - } - } - - let metricName = val.metricName || item.name; - - const keys = Object.keys(val.labels); - const size = keys.length; - if (size > 0) { - let labels = ''; - let i = 0; - for (; i < size - 1; i++) { - labels += `${keys[i]}="${escapeLabelValue(val.labels[keys[i]])}",`; - } - labels += `${keys[i]}="${escapeLabelValue(val.labels[keys[i]])}"`; - metricName += `{${labels}}`; - } - - values += `${metricName} ${getValueAsString(val.value)}\n`; - } - - return `${help}\n${type}\n${values}`.trim(); + return metric.getPrometheusString(this._defaultLabels); } metrics() { @@ -69,7 +21,7 @@ class Registry { this.collect(); for (const metric of this.getMetricsAsArray()) { - metrics += `${this.getMetricAsPrometheusString(metric)}\n\n`; + metrics += `${this.getMetricAsPrometheusString(metric)}\n`; } return metrics.substring(0, metrics.length - 1); diff --git a/lib/summary.js b/lib/summary.js index c8033472..466c3474 100644 --- a/lib/summary.js +++ b/lib/summary.js @@ -4,7 +4,6 @@ 'use strict'; const util = require('util'); -const type = 'summary'; const { getLabels, hashObject, removeLabels } = require('./util'); const { validateLabel } = require('./validation'); const { Metric } = require('./metric'); @@ -35,6 +34,8 @@ class Summary extends Metric { }, }; } + + this.type = 'summary'; } /** @@ -61,7 +62,7 @@ class Summary extends Metric { return { name: this.name, help: this.help, - type, + type: this.type, values, aggregator: this.aggregator, }; diff --git a/lib/util.js b/lib/util.js index ca23a19b..3af87d41 100644 --- a/lib/util.js +++ b/lib/util.js @@ -1,5 +1,14 @@ 'use strict'; +const QUOTE_RE = /"/g; + +exports.escapeLabelValue = function escapeLabelValue(str) { + if (typeof str !== 'string') { + return str; + } + return str.replace(QUOTE_RE, '\\"'); +}; + exports.getValueAsString = function getValueString(value) { if (Number.isNaN(value)) { return 'Nan'; diff --git a/package.json b/package.json index ed814896..8a7fbcff 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "typescript": "^3.0.3" }, "dependencies": { + "generate-function": "^2.3.1", "tdigest": "^0.1.1" }, "types": "./index.d.ts", diff --git a/test/registerTest.js b/test/registerTest.js index 9b75eb8d..370b7305 100644 --- a/test/registerTest.js +++ b/test/registerTest.js @@ -5,6 +5,7 @@ describe('register', () => { const Counter = require('../index').Counter; const Gauge = require('../index').Gauge; const Histogram = require('../index').Histogram; + const Registry = require('../index').Registry; const Summary = require('../index').Summary; beforeEach(() => { @@ -43,95 +44,60 @@ describe('register', () => { }); it('should handle and output a metric with a NaN value', () => { - register.registerMetric({ - get() { - return { - name: 'test_metric', - type: 'gauge', - help: 'A test metric', - values: [ - { - value: NaN, - }, - ], - }; - }, + const gauge = new Gauge({ + name: 'test_metric', + help: 'A test metric', }); + gauge.set(NaN); + register.registerMetric(gauge); + const lines = register.metrics().split('\n'); expect(lines).toHaveLength(4); expect(lines[2]).toEqual('test_metric Nan'); }); it('should handle and output a metric with an +Infinity value', () => { - register.registerMetric({ - get() { - return { - name: 'test_metric', - type: 'gauge', - help: 'A test metric', - values: [ - { - value: Infinity, - }, - ], - }; - }, + const gauge = new Gauge({ + name: 'test_metric', + help: 'A test metric', }); + gauge.set(Infinity); + register.registerMetric(gauge); const lines = register.metrics().split('\n'); expect(lines).toHaveLength(4); expect(lines[2]).toEqual('test_metric +Inf'); }); it('should handle and output a metric with an -Infinity value', () => { - register.registerMetric({ - get() { - return { - name: 'test_metric', - type: 'gauge', - help: 'A test metric', - values: [ - { - value: -Infinity, - }, - ], - }; - }, + const gauge = new Gauge({ + name: 'test_metric', + help: 'A test metric', }); + gauge.set(-Infinity); + register.registerMetric(gauge); const lines = register.metrics().split('\n'); expect(lines).toHaveLength(4); expect(lines[2]).toEqual('test_metric -Inf'); }); it('should handle a metric without labels', () => { - register.registerMetric({ - get() { - return { - name: 'test_metric', - type: 'counter', - help: 'A test metric', - values: [ - { - value: 1, - }, - ], - }; - }, + const counter = new Counter({ + name: 'test_metric', + help: 'A test metric', }); + counter.inc(1); + register.registerMetric(counter); expect(register.metrics().split('\n')).toHaveLength(4); }); it('should handle a metric with default labels', () => { register.setDefaultLabels({ testLabel: 'testValue' }); - register.registerMetric({ - get() { - return { - name: 'test_metric', - type: 'counter', - help: 'A test metric', - values: [{ value: 1 }], - }; - }, + const counter = new Counter({ + name: 'test_metric', + help: 'A test metric', }); + counter.inc(1); + register.registerMetric(counter); const output = register.metrics().split('\n')[2]; expect(output).toEqual('test_metric{testLabel="testValue"} 1'); @@ -139,24 +105,13 @@ describe('register', () => { it('labeled metrics should take precidence over defaulted', () => { register.setDefaultLabels({ testLabel: 'testValue' }); - register.registerMetric({ - get() { - return { - name: 'test_metric', - type: 'counter', - help: 'A test metric', - values: [ - { - value: 1, - labels: { - testLabel: 'overlapped', - anotherLabel: 'value123', - }, - }, - ], - }; - }, + const counter = new Counter({ + name: 'test_metric', + help: 'A test metric', + labelNames: ['testLabel', 'anotherLabel'], }); + counter.inc({ testLabel: 'overlapped', anotherLabel: 'value123' }, 1); + register.registerMetric(counter); expect(register.metrics().split('\n')[2]).toEqual( 'test_metric{testLabel="overlapped",anotherLabel="value123"} 1', @@ -181,49 +136,35 @@ describe('register', () => { expect(register.metrics()).toMatchSnapshot(); }); - describe('should escape', () => { - let escapedResult; - beforeEach(() => { - register.registerMetric({ - get() { - return { - name: 'test_"_\\_\n_metric', - help: 'help_help', - type: 'counter', - }; - }, - }); - escapedResult = register.metrics(); - }); - it('backslash to \\\\', () => { - expect(escapedResult).toMatch(/\\\\/); - }); - it('newline to \\\\n', () => { - expect(escapedResult).toMatch(/\n/); + it('should escape " in label values', () => { + const counter = new Counter({ + name: 'test_metric', + help: 'A test metric', + labelNames: ['label', 'code'], }); + counter.inc({ label: 'hello', code: '3"03' }, 12); + register.registerMetric(counter); + const escapedResult = register.metrics(); + expect(escapedResult).toMatch( + 'test_metric{label="hello",code="3\\"03"} 12', + ); }); - it('should escape " in label values', () => { - register.registerMetric({ - get() { - return { - name: 'test_metric', - type: 'counter', - help: 'A test metric', - values: [ - { - value: 12, - labels: { - label: 'hello', - code: '3"03', - }, - }, - ], - }; - }, + it('should escape " in default label values', () => { + const counter = new Counter({ + name: 'test_metric', + help: 'A test metric', + labelNames: ['label', 'code'], + }); + counter.inc({ label: 'hello', code: '3"03' }, 12); + register.setDefaultLabels({ + defaultRegistryLabel: 'test"Value', }); + register.registerMetric(counter); const escapedResult = register.metrics(); - expect(escapedResult).toMatch(/\\"/); + expect(escapedResult).toMatch( + 'test_metric{label="hello",code="3\\"03",defaultRegistryLabel="test\\"Value"} 12', + ); }); describe('should output metrics as JSON', () => { @@ -260,7 +201,7 @@ describe('register', () => { it('should allow removing single metrics', () => { register.registerMetric(getMetric()); - register.registerMetric(getMetric('some other name')); + register.registerMetric(getMetric({ name: 'some_other_name' })); let output = register.getMetricsAsJSON(); expect(output.length).toEqual(2); @@ -270,7 +211,7 @@ describe('register', () => { output = register.getMetricsAsJSON(); expect(output.length).toEqual(1); - expect(output[0].name).toEqual('some other name'); + expect(output[0].name).toEqual('some_other_name'); }); it('should allow getting single metrics', () => { @@ -427,7 +368,7 @@ describe('register', () => { const metrics = r.metrics(); const lines = metrics.split('\n'); expect(lines).toContain( - 'my_histogram_bucket{le="1",type="myType",env="development"} 1', + 'my_histogram_bucket{type="myType",env="development",le="1"} 1', ); myHist.observe(1); @@ -435,7 +376,7 @@ describe('register', () => { const metrics2 = r.metrics(); const lines2 = metrics2.split('\n'); expect(lines2).toContain( - 'my_histogram_bucket{le="1",type="myType",env="development"} 2', + 'my_histogram_bucket{type="myType",env="development",le="1"} 2', ); }); }); @@ -577,7 +518,6 @@ describe('register', () => { }); describe('merging', () => { - const Registry = require('../lib/registry'); let registryOne; let registryTwo; @@ -587,8 +527,12 @@ describe('register', () => { }); it('should merge all provided registers', () => { - registryOne.registerMetric(getMetric('one')); - registryTwo.registerMetric(getMetric('two')); + registryOne.registerMetric( + getMetric({ name: 'one', registers: [registryOne] }), + ); + registryTwo.registerMetric( + getMetric({ name: 'two', registers: [registryTwo] }), + ); const merged = Registry.merge([ registryOne, @@ -598,8 +542,8 @@ describe('register', () => { }); it('should throw if same name exists on both registers', () => { - registryOne.registerMetric(getMetric()); - registryTwo.registerMetric(getMetric()); + registryOne.registerMetric(getMetric({ registers: [registryOne] })); + registryTwo.registerMetric(getMetric({ registers: [registryTwo] })); const fn = function () { Registry.merge([registryOne, registryTwo]); @@ -609,33 +553,16 @@ describe('register', () => { }); }); - function getMetric(name) { - name = name || 'test_metric'; - return { - name, - get() { - return { - name, - type: 'counter', - help: 'A test metric', - values: [ - { - value: 12, - labels: { - label: 'hello', - code: '303', - }, - }, - { - value: 34, - labels: { - label: 'bye', - code: '404', - }, - }, - ], - }; - }, - }; + function getMetric(configuration) { + const counter = new Counter({ + name: 'test_metric', + help: 'A test metric', + labelNames: ['label', 'code'], + ...configuration, + }); + counter.inc({ label: 'hello', code: '303' }, 12); + counter.inc({ label: 'bye', code: '404' }, 34); + + return counter; } }); From 7227147600f39c72f2512ea7a7def285a0d9dc43 Mon Sep 17 00:00:00 2001 From: Aleksei Androsov Date: Sun, 17 May 2020 19:28:49 +0300 Subject: [PATCH 2/5] Move Metric `type` to prototype since it's invariant --- lib/counter.js | 7 ++----- lib/gauge.js | 7 ++----- lib/histogram.js | 4 ++-- lib/summary.js | 4 ++-- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/counter.js b/lib/counter.js index 75f3f384..3f555fe0 100644 --- a/lib/counter.js +++ b/lib/counter.js @@ -9,11 +9,6 @@ const { validateLabel } = require('./validation'); const { Metric } = require('./metric'); class Counter extends Metric { - constructor(...args) { - super(...args); - this.type = 'counter'; - } - /** * Increment counter * @param {object} labels - What label you want to be incremented @@ -62,6 +57,8 @@ class Counter extends Metric { } } +Counter.prototype.type = 'counter'; + const reset = function () { this.hashMap = {}; diff --git a/lib/gauge.js b/lib/gauge.js index ce3f95b5..b44e6f88 100644 --- a/lib/gauge.js +++ b/lib/gauge.js @@ -16,11 +16,6 @@ const { validateLabel } = require('./validation'); const { Metric } = require('./metric'); class Gauge extends Metric { - constructor(...args) { - super(...args); - this.type = 'gauge'; - } - /** * Set a gauge to a value * @param {object} labels - Object with labels and their values @@ -117,6 +112,8 @@ class Gauge extends Metric { } } +Gauge.prototype.type = 'gauge'; + function setToCurrentTime(labels) { return () => { const now = Date.now() / 1000; diff --git a/lib/histogram.js b/lib/histogram.js index 9a18fb64..c4fc21f6 100644 --- a/lib/histogram.js +++ b/lib/histogram.js @@ -37,8 +37,6 @@ class Histogram extends Metric { ), }; } - - this.type = 'histogram'; } /** @@ -101,6 +99,8 @@ class Histogram extends Metric { } } +Histogram.prototype.type = 'histogram'; + function startTimer(startLabels) { return () => { const start = process.hrtime(); diff --git a/lib/summary.js b/lib/summary.js index 466c3474..318c6df4 100644 --- a/lib/summary.js +++ b/lib/summary.js @@ -34,8 +34,6 @@ class Summary extends Metric { }, }; } - - this.type = 'summary'; } /** @@ -105,6 +103,8 @@ class Summary extends Metric { } } +Summary.prototype.type = 'summary'; + function extractSummariesForExport(summaryOfLabels, percentiles) { summaryOfLabels.td.compress(); From 3c4c748028dcbf4ecaf977b36c198e969eba63dd Mon Sep 17 00:00:00 2001 From: Aleksei Androsov Date: Sun, 17 May 2020 19:35:06 +0300 Subject: [PATCH 3/5] Generate function without external dependencies --- lib/formatter.js | 39 ++++++++++++++++++++------------------- package.json | 1 - 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/formatter.js b/lib/formatter.js index c36a86dc..982d9383 100644 --- a/lib/formatter.js +++ b/lib/formatter.js @@ -1,7 +1,5 @@ 'use strict'; -const generateFunction = require('generate-function'); - function getFormatter(metric, defaultLabels) { const defaultLabelNames = Object.keys(defaultLabels); const name = metric.name; @@ -9,23 +7,26 @@ function getFormatter(metric, defaultLabels) { const type = metric.type; const labelsCode = getLabelsCode(metric, defaultLabelNames, defaultLabels); - const gen = generateFunction(); - gen(` - function format(item, escapeLabelValue, getValueAsString) { - const info = '# HELP ${name} ${help}\\n# TYPE ${name} ${type}\\n'; - let values = ''; - for (const val of item.values || []) { - val.labels = val.labels || {}; - let metricName = val.metricName || '${name}'; - const labels = \`${labelsCode}\`; - const hasLabels = labels.length > 0; - metricName += \`\${hasLabels ? '{' : ''}\${labels}\${hasLabels ? '}' : ''}\`; - let line = \`\${metricName} \${getValueAsString(val.value)}\`; - values += \`\${line}\\n\`; - } - return info + values; - }`); - return gen.toFunction(); + // eslint-disable-next-line no-new-func + return new Function( + 'item', + 'escapeLabelValue', + 'getValueAsString', + ` + const info = '# HELP ${name} ${help}\\n# TYPE ${name} ${type}\\n'; + let values = ''; + for (const val of item.values || []) { + val.labels = val.labels || {}; + let metricName = val.metricName || '${name}'; + const labels = \`${labelsCode}\`; + const hasLabels = labels.length > 0; + metricName += \`\${hasLabels ? '{' : ''}\${labels}\${hasLabels ? '}' : ''}\`; + let line = \`\${metricName} \${getValueAsString(val.value)}\`; + values += \`\${line}\\n\`; + } + return info + values; + `, + ); } function getLabelsCode(metric, defaultLabelNames, defaultLabels) { diff --git a/package.json b/package.json index 8a7fbcff..ed814896 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,6 @@ "typescript": "^3.0.3" }, "dependencies": { - "generate-function": "^2.3.1", "tdigest": "^0.1.1" }, "types": "./index.d.ts", From a1a36757842a88c8eb4774b05d7b80a9f52484f9 Mon Sep 17 00:00:00 2001 From: Aleksei Androsov Date: Mon, 18 May 2020 09:20:19 +0300 Subject: [PATCH 4/5] Fix big with label="undefined" --- lib/formatter.js | 27 +++++++++++++++++++-------- test/registerTest.js | 26 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/lib/formatter.js b/lib/formatter.js index 982d9383..f4e5a9db 100644 --- a/lib/formatter.js +++ b/lib/formatter.js @@ -30,22 +30,33 @@ function getFormatter(metric, defaultLabels) { } function getLabelsCode(metric, defaultLabelNames, defaultLabels) { - let labelString = ''; + const labelString = []; const labelNames = getLabelNames(metric, defaultLabelNames); for (let index = 0; index < labelNames.length; index++) { + const comma = index > 0 ? ',' : ''; const labelName = labelNames[index]; if (labelName === 'quantile') { - labelString += `\${val.labels.quantile != null ? \`quantile="\${escapeLabelValue(val.labels.quantile)}"\` : ''}`; + labelString.push( + `\${val.labels.quantile != null ? \`${comma}quantile="\${escapeLabelValue(val.labels.quantile)}"\` : ''}`, + ); } else if (labelName === 'le') { - labelString += `\${val.labels.le != null ? \`le="\${escapeLabelValue(val.labels.le)}"\` : ''}`; + labelString.push( + `\${val.labels.le != null ? \`${comma}le="\${escapeLabelValue(val.labels.le)}"\` : ''}`, + ); } else { - labelString += `${labelName}="\${val.labels['${labelName}'] ? escapeLabelValue(val.labels['${labelName}']) : escapeLabelValue('${defaultLabels[labelName]}')}"`; - } - if (index !== labelNames.length - 1 && labelString.length > 0) { - labelString += ','; + const defaultLabelValue = defaultLabels[labelName]; + if (typeof defaultLabelValue === 'undefined') { + labelString.push( + `\${val.labels['${labelName}'] != null ? \`${comma}${labelName}="\${escapeLabelValue(val.labels['${labelName}'])}"\` : ''}`, + ); + } else { + labelString.push( + `${comma}${labelName}="\${escapeLabelValue(val.labels['${labelName}'] || '${defaultLabelValue}')}"`, + ); + } } } - return labelString; + return labelString.join(''); } function getLabelNames(metric, defaultLabelNames) { diff --git a/test/registerTest.js b/test/registerTest.js index 370b7305..1ecc63a2 100644 --- a/test/registerTest.js +++ b/test/registerTest.js @@ -103,6 +103,32 @@ describe('register', () => { expect(output).toEqual('test_metric{testLabel="testValue"} 1'); }); + it('should handle a metric with default labels with value 0', () => { + register.setDefaultLabels({ testLabel: 0 }); + const counter = new Counter({ + name: 'test_metric', + help: 'A test metric', + }); + counter.inc(1); + register.registerMetric(counter); + + const output = register.metrics().split('\n')[2]; + expect(output).toEqual('test_metric{testLabel="0"} 1'); + }); + + it('should handle a metrics with labels subset', () => { + const gauge = new Gauge({ + name: 'test_metric', + help: 'help', + labelNames: ['code', 'success'], + }); + gauge.inc({ code: '200' }, 10); + register.registerMetric(gauge); + + const output = register.metrics().split('\n')[2]; + expect(output).toEqual('test_metric{code="200"} 10'); + }); + it('labeled metrics should take precidence over defaulted', () => { register.setDefaultLabels({ testLabel: 'testValue' }); const counter = new Counter({ From 3c0ad3ab11d573d3fec98a00c9bb6025a3c438a9 Mon Sep 17 00:00:00 2001 From: Aleksei Androsov Date: Mon, 18 May 2020 09:38:59 +0300 Subject: [PATCH 5/5] CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5533046..f5cb4037 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ project adheres to [Semantic Versioning](http://semver.org/). - chore: refactor metrics to reduce code duplication - chore: replace `utils.getPropertiesFromObj` with `Object.values` - chore: remove unused `catch` bindings +- chore: improve performance of `registry.metrics()` ### Added