fix: normalize label values once, when a label combination is first stored

[milcho0604]

Fixes #791, along the direction proposed there: pay the coercion cost once, when a new label combination is first stored, instead of on every metrics() call (#792/#793 were declined for adding cost to rendering, which is linear in total cardinality).

What

• LabelMap gains a single insertion point (#insert) used by set, setDelta, getOrAdd and merge. It coerces label values with template interpolation — the same ToString the exposition applies — via a copy-on-write normalizeLabels() that returns all-string label sets untouched (no allocation in the common case).
• Nullish values stay untouched. keyFrom() treats them as absent, so coercing them to "null"/"undefined" would make stored labels compute a different key than the one they are stored under — breaking remove(entry.labels) round-trips (Summary's pruning does exactly that) and collapsing {a: null} with {a: 'null'} after worker serialization. Their rendered form contains nothing that needs escaping.
• merge() keeps the stored (normalized) labels on update instead of overwriting them with the caller's raw object — the update path that could previously replace stored labels without going through insertion.
• getOrAdd() passes the normalized labels to init(), so values that keep their own copy of the labels store the same normalized object. Summary needs this: its exported labels come from the stored value, not the map entry.
• LabelGrouper deliberately does not normalize, per the discussion in Non-string label values bypass escaping and can produce malformed exposition #791: aggregation input comes from registry.getMetricsAsJSON(), whose store-backed labels were already normalized on first insertion, so re-checking every value would tax aggregate() for work the stores already did — aggregate() keeps its current cost, byte-for-byte. I verified the assumption: worker payloads are exactly getMetricsAsJSON() output, where metric values carry stored (normalized) labels. The inputs that reach aggregation without normalization are unchanged from main: custom collector results, registry default labels (merged in raw by getMetricsAsJSON()), and the synthesized le/quantile labels, which are attached numerically at export time — all reserved or user-controlled values that pass through exactly as today (promtool accepts the aggregated output, see below). On Cluster fixes #789: its diff touches lib/cluster.js/lib/worker.js lifecycle only — payloads are still getMetricsAsJSON() output fed to Registry.aggregate(), so the two changes stay orthogonal.

With the stores normalized, the existing render-time escaping (which already handles strings correctly) produces well-formed exposition for the #791 reproduction:

g{x="say \"hi\""} 1

Cost

The recording path for existing combinations is unchanged — lookup only, no new code. First insertion of a combination pays one scan of its labels, plus a copy and coercion when something is non-string (getOrAdd scans once more inside #insert; normalization is idempotent). Interleaved 5-round medians (Node 25, arm64, lower is better; ranges in parentheses):

path	main	this PR
hot: inc() on existing combination × 5M	135.2 ms (135.0–137.9)	132.4 ms (131.9–137.4) — within noise
metrics() with 10k series × 50	199.8 ms (195.7–210.6)	199.1 ms (184.2–202.0) — within noise
300k first insertions	48.3 ms (47.7–49.2)	51.0 ms (50.2–53.1) — +5.6%, ~9 ns per new combination

aggregate() is untouched — no code change on that path.

Benchmark script (run against two checkouts, interleaved)

'use strict';
// usage: node bench.js <prom-client dir> <hot|insert|render>
const path = process.argv[2];
const which = process.argv[3];
const client = require(require('node:path').resolve(path, 'index.js'));

function hrms(fn) {
	const t0 = process.hrtime.bigint();
	fn();
	return Number(process.hrtime.bigint() - t0) / 1e6;
}

if (which === 'hot') {
	const r = new client.Registry();
	const c = new client.Counter({ name: 'c_total', help: 'h', labelNames: ['x'], registers: [r] });
	c.inc({ x: 'warm' }, 1);
	const N = 5_000_000;
	console.log(hrms(() => {
		for (let i = 0; i < N; i++) c.inc({ x: 'warm' }, 1);
	}).toFixed(1));
} else if (which === 'insert') {
	const { LabelMap } = require(require('node:path').resolve(path, 'lib/util.js'));
	const N = 300_000;
	const maps = [];
	for (let i = 0; i < 10; i++) maps.push(new LabelMap(['a', 'b']));
	console.log(hrms(() => {
		for (const m of maps) {
			for (let i = 0; i < N / 10; i++) m.set({ a: `v${i}`, b: 'k' }, i);
		}
	}).toFixed(1));
} else if (which === 'render') {
	const r = new client.Registry();
	const c = new client.Counter({ name: 'c_total', help: 'h', labelNames: ['x'], registers: [r] });
	for (let i = 0; i < 10_000; i++) c.inc({ x: `v${i}` }, 1);
	(async () => {
		await r.metrics();
		const t0 = process.hrtime.bigint();
		for (let i = 0; i < 50; i++) await r.metrics();
		console.log((Number(process.hrtime.bigint() - t0) / 1e6).toFixed(1));
	})();
}

Observable changes

• Label values that pass through the built-in stores are reported as strings (3 becomes "3") wherever stored labels surface: getMetricsAsJSON(), each metric's public get(), worker payloads, and — transitively — aggregation output. Registry.aggregate() itself passes labels through untouched. Noted in the changelog.
• No index.d.ts change: label output types are already string | number, and le/quantile/default/custom-collector labels can still be numeric.
• keyFrom() already coerces ordinary non-nullish values while building keys, so get()/set() lookups accept either representation — pinned by new tests, including null vs 'null' staying distinct and remove(entry.labels) round-tripping.

Scope

Covered: everything that goes through the built-in metric stores (Counter, Gauge, Histogram, Summary); cluster aggregation benefits transitively because worker payloads carry stored labels. Not covered (unchanged, documented): custom collector results rendered directly from get(), registry default labels, and exemplar labels — none of these pass through the stores. Relocating the quote/backslash/newline escaping itself into the stores is deliberately left out: rendering currently escapes backslashes, so it would have to move atomically across every label source or double-escape; I can scope and benchmark that separately.

Test

• New label normalization unit block for LabelMap: coercion at insertion, caller's object never mutated, all-string sets stored without copying, lookups by either representation, nullish round-trips (remove(entry.labels) works; null vs 'null' stay distinct), getOrAdd init receiving normalized labels, merge keeping normalized labels across updates. LabelGrouper tests pin the pass-through behaviour (raw labels preserved).
• End-to-end regressions through real metrics (both content types): Gauge.set with array/number labels renders escaped; Summary.observe covers the stored-value labels path.
• Existing expectations that pinned numeric stored labels updated to the normalized form (versionTest, defaultMetricsTest, utilTest); aggregation expectations stay raw, pinning the pass-through.
• Applying the full test patch to main without the lib changes yields 23 failures; the full suite passes with them: 556/556, lint + prettier + tsc clean.
• promtool check metrics accepts the rendered output (rc=0) for a registry mixing quote/newline/backslash/boolean/number labels across all four metric types, and for a simulated cluster round-trip (two workers' getMetricsAsJSON() through JSON serialization into AggregatorRegistry.aggregate()). The same direct-render check on main fails with the error from Non-string label values bypass escaping and can produce malformed exposition #791 (unexpected end of label value, rc=1).

[jdmarshall]

👍

[jdmarshall]

This is pretty close to ready to go, except that the nested double loop is giving me some pause.

I can't land this yet though because we are working on a dot release to get a huge pile of existing changes out. But this should be able to make the 1.0 cutoff for sure.

[jdmarshall]

I don't like the nested loops here, but there are several ways we could go here.

Do you believe the guard loop is necessary to reduce the need to copy the labels, or would it be simpler to defensively copy them on every #insert() call? This is a spot where we accept a value from the user as if we now own it and perhaps that is not the best idea.

If you think so, then we could still set up these loops as two sequential loops, which means the eye won't stick on this code in perpetuity every time someone goes hunting around for a bug. For instance by setting a

    needsNormalization = true;
    break;

in the first loop and then returning if the boolean is false.

[milcho0604]

Good question — I measured it, and the guard loop isn't worth keeping, so I took the defensive-copy route you suggested. normalizeLabels() is flat now — one loop, no nesting — and always copies.

The structural half of the answer first: #insert() runs once per stored combination, not once per record. All four call sites look the key up and fall through only on a miss, so a new combination's first record always pays for the copy, and every record after it pays nothing.

Numbers are on Node 24.11. (I first measured on Node 25, which is outside engines, and the deltas there were both larger and less consistent — please disregard that if you saw it.)

The repo's own suite, with this branch's previous revision as baseline: no significant regressions across all 46 cases, from 1.03x slower to 1.08x faster.

Focused A/B on the storage path — one implementation per process, since loading both into one makes the shared call sites polymorphic and taxes both; median of 9 samples, order alternated between runs, ns/op:

	guard loop	defensive copy
insert, 1 label (per new combination)	55.0	59.7	+8.6%
insert, 3 labels	125.5	148.7	+18.5%
insert, 6 labels	200.0	244.8	+22.4%
record on an existing combination	51.5	51.9	+0.7%
create + 10 records, per record	125.0	131.6	+5.3%
create + 100 records, per record	106.4	105.9	−0.5%

So the cost is real but confined to a once-per-series path: still visible at ten records of a series (+5.3%), down in the noise by a hundred, which is why none of the suite's workloads can see it. Retained memory at 250k unique series is unchanged (64.6 MiB against 64.7 MiB) — the store simply retains an object of its own instead of retaining the caller's.

Taking ownership did mean pinning down what a "copy" means, and two things in that loop are load-bearing rather than decorative:

• It iterates the source with for...in, so inherited enumerable labels are materialized. keyFrom() reads labels by name and sees those too, so an own-properties-only copy could not reproduce the key its entry is filed under, and remove(entry.labels) — which Summary's pruning depends on — would quietly miss.
• __proto__ gets Object.defineProperty. It passes the label-name regexp, and plain assignment invokes the prototype setter instead of defining a property, so the label would vanish from the stored copy with the same consequence.

Both are covered: removing the defineProperty branch fails one test, and switching the loop to iterate the copy fails two. The spread that seeds the copy is there for the packed object shape — building the copy key by key instead costs about 24 bytes per stored series — so nothing asserts it; it does also carry over symbol-keyed properties, which the loop ignores.

Two divergences I did not chase, in case you'd rather I did. Both come from the copy being an ordinary object built by enumeration, and both were previously masked by the store keeping the caller's object:

• A labels object created with Object.create(null) becomes an ordinary one. That is observable only if a declared label name is also an Object.prototype member (constructor, toString, …) and absent from the object — keyFrom() then reads the inherited function for the copy but undefined for the original.
• A non-enumerable label property is invisible to any enumeration, so it is dropped, while keyFrom() still reads it.

Preserving the source prototype gives up the packed-shape win above, and keyFrom()'s unguarded property access is the underlying sharp edge in both cases, so I left them rather than widen this PR.

One knock-on that I think is a net simplification: once the store owns its labels, Summary's stored value no longer needs its own copy of them. getOrAdd() is back to calling init() with no arguments, as on main, and the summary export helpers take entry.labels — summary.js ends up back at main's length. The one externally visible effect is that a label-less summary now returns labels: {} rather than labels: undefined from getMetricsAsJSON() (serialized JSON previously omitted the property), matching what counters, gauges and histograms already do. Text exposition for ordinary label objects is unchanged apart from the intended difference: a caller that mutates its labels object after recording no longer changes what the stored series reports. Happy to split that part out if you'd rather keep this PR to util.js.

I also shortened the changelog entries, per your note on #790.