From 35cbdbf1ae0b7d14761b2e52877aab18fa8cdffe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?espen=20dall=C3=B8kken?= Date: Fri, 21 Jun 2024 13:09:29 +0200 Subject: [PATCH] fix: emit warn when labels and values are out of sync --- .tap/test-results/test/guard.js.tap | 83 +++++++++++++++++++++++++++++ lib/guard.js | 22 +++++++- test/guard.js | 21 ++++++++ 3 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 .tap/test-results/test/guard.js.tap diff --git a/.tap/test-results/test/guard.js.tap b/.tap/test-results/test/guard.js.tap new file mode 100644 index 0000000..1f606d3 --- /dev/null +++ b/.tap/test-results/test/guard.js.tap @@ -0,0 +1,83 @@ +TAP version 14 +# Subtest: Guard() - object type - should be MetricsGuard + ok 1 - should be equal + 1..1 +ok 1 - Guard() - object type - should be MetricsGuard # time=1.514ms + +# Subtest: Guard() - "metricsThreshold" argument is not valid - should throw + 1..1 + ok 1 - expected to throw +ok 2 - Guard() - "metricsThreshold" argument is not valid - should throw # time=2.47ms + +# Subtest: Guard() - "permutationThreshold" argument is not valid - should throw + 1..1 + ok 1 - expected to throw +ok 3 - Guard() - "permutationThreshold" argument is not valid - should throw # time=0.346ms + +# Subtest: Guard() - "enabled" argument is not valid - should throw + 1..1 + ok 1 - expected to throw +ok 4 - Guard() - "enabled" argument is not valid - should throw # time=0.418ms + +# Subtest: Guard() - "permutationThreshold" is at 6 - one metric exceed this - should drop the metric from the stream when exceeding the threshold + ok 1 - should be equal + ok 2 - should be equal + ok 3 - should be equal + ok 4 - should be equal + ok 5 - should be equal + ok 6 - should be equal + 1..6 +ok 5 - Guard() - "permutationThreshold" is at 6 - one metric exceed this - should drop the metric from the stream when exceeding the threshold # time=3.59ms + +# Subtest: Guard() - "metricsThreshold" is at 4 - one metric exceed this - should emit warn event + ok 1 - should be equal + ok 2 - should be equal + 1..2 +ok 6 - Guard() - "metricsThreshold" is at 4 - one metric exceed this - should emit warn event # time=0.445ms + +# Subtest: Guard() - "enabled" is "false" - metrics exceeds limits - should not interfere and should not store internal metrics + ok 1 - should be equal + ok 2 - should be equal + ok 3 - should be equal + 1..3 +ok 7 - Guard() - "enabled" is "false" - metrics exceeds limits - should not interfere and should not store internal metrics # time=0.595ms + +# Subtest: .getMetrics() - set metrics - call method - should return an Array with metric names without duplicates + ok 1 - should be equal + ok 2 - should be equal + ok 3 - should be equal + 1..3 +ok 8 - .getMetrics() - set metrics - call method - should return an Array with metric names without duplicates # time=0.496ms + +# Subtest: .getLabels() - set metrics - call method - should return an Array with metric names without duplicates + ok 1 - should be equal + ok 2 - should be equal + ok 3 - should be equal + ok 4 - should be equal + ok 5 - should be equal + ok 6 - should be equal + 1..6 +ok 9 - .getLabels() - set metrics - call method - should return an Array with metric names without duplicates # time=1.978ms + +# Subtest: .reset() - set metrics - call method - should empty the guard registry + ok 1 - should be equal + ok 2 - should be equal + ok 3 - should be equal + ok 4 - should be equal + ok 5 - should be equal + 1..5 +ok 10 - .reset() - set metrics - call method - should empty the guard registry # time=0.64ms + +# Subtest: .pipe() - exceed the default, 10, number of max event listeners - should not cause the process to emit a MaxListenersExceededWarning + ok 1 - should be equal + 1..1 +ok 11 - .pipe() - exceed the default, 10, number of max event listeners - should not cause the process to emit a MaxListenersExceededWarning # time=0.924ms + +# Subtest: Guard() - do not add metric where label name / values are null and emit warning + ok 1 - should be equal + ok 2 - should match pattern + ok 3 - should be equal + 1..3 +ok 12 - Guard() - do not add metric where label name / values are null and emit warning # time=0.402ms + +1..12 diff --git a/lib/guard.js b/lib/guard.js index 55c17cc..3d0a84d 100644 --- a/lib/guard.js +++ b/lib/guard.js @@ -85,6 +85,9 @@ const MetricsGuard = class MetricsGuard extends stream.Transform { if (labels) { return Array.from(labels.keys()).map((label) => { const [name, value] = label.split(SEPARATOR); + if (!name || !value) { + this.emit("warn", "metrics", `is null ${name} ${value}`); + } return { name, value }; }); } @@ -131,11 +134,22 @@ const MetricsGuard = class MetricsGuard extends stream.Transform { this.registry.set(metric.name, entry); } + let hasInvalidLabelValue = []; // push all label permutations on the metric into the // Set for the metric in the registry. the size of // this Set is compared with the threshold metric.labels.forEach((label) => { - entry.add(`${label.name}${SEPARATOR}${label.value}`); + if ( + label && + (label.name === null || + typeof label.name === "undefined" || + label.value === null || + typeof label.value === "undefined") + ) { + hasInvalidLabelValue.push(`${metric.name} label name="${label.name}", value=${label.value}`); + } else { + entry.add(`${label.name}${SEPARATOR}${label.value}`); + } }); if (metric.source === this.id) { @@ -143,6 +157,12 @@ const MetricsGuard = class MetricsGuard extends stream.Transform { next(null); return; } + if (hasInvalidLabelValue.length > 0) { + // Omit a warning when name or value is null / undefined + this.emit("warn", "labels", hasInvalidLabelValue.toString()); + next(null); + return; + } next(null, metric); } diff --git a/test/guard.js b/test/guard.js index beaa24a..5c5451f 100644 --- a/test/guard.js +++ b/test/guard.js @@ -425,3 +425,24 @@ tap.test( }); }, ); + +tap.test("Guard() - do not add metric where label name / values are null and emit warning", (t) => { + const guard = new Guard(); + const metrics = [new Metric({ name: "foo", description: "foo", labels: [{ name: undefined, value: null }] })]; + + const src = srcObjectStream(metrics); + const dest = destObjectStream((arr) => { + t.equal(arr.length, 0); + t.end(); + }); + + guard.on("warn", (type, message) => { + t.equal(type, "labels"); + t.match(message, /undefined/); + }); + src.pipe(guard).pipe(dest); + + setImmediate(() => { + dest.end(); + }); +});