Skip to content

Commit

Permalink
fix: emit warn when labels and values are out of sync
Browse files Browse the repository at this point in the history
  • Loading branch information
leftieFriele committed Jun 21, 2024
1 parent 9149e4e commit 35cbdbf
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 1 deletion.
83 changes: 83 additions & 0 deletions .tap/test-results/test/guard.js.tap
Original file line number Diff line number Diff line change
@@ -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
22 changes: 21 additions & 1 deletion lib/guard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
});
}
Expand Down Expand Up @@ -131,18 +134,35 @@ 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) {
this.emit("drop", metric);
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);
}
Expand Down
21 changes: 21 additions & 0 deletions test/guard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});

0 comments on commit 35cbdbf

Please sign in to comment.