Speed up label escaping

[jdmarshall]

On the back of #798, there is now an opportunity to escape label values once at insert time rather than on every metrics call.

The metrics call is a high priority target for both time and space improvements. When I first came to the project, it was the most common complaint in the issue backlog. We have managed to speed up everything but this part. Collection is ~20% faster, aggregation 2.4x faster, but metrics are only a couple % faster.

With the introduction of #insert, we should be able to move the escapeString() call to insert. This could be done two ways:

1. change the labels to only be escaped labels
2. store the escaped form of the labels beside the original form

Number 2 may be more compatible, and since most strings do not need to be escaped in the first place, represent only a small increase in memory usage.

This work also may reduce or remove the need for the label cache in the Registry. Despite the use of a WeakMap for storage, this does represent a memory cost, and if the label concatenation is cheaper, then it's worth also considering whether the added overhead of the lookups is really worth the extra space and code, or whether the whole thing can be deleted.

[milcho0604]

Author of #798 here. Thanks for updating that branch — and I would be glad to take this one on if it is still free.

Before starting, could I check the direction with you? Reading the render path on the current head:

• formatLabels() escapes label values on every line (escapeLabelValue(value), lib/registry.js:288); label names are interpolated raw there.
• Metric name and help are escaped once per metric (lib/registry.js:58-59), not per series.

So the per-call cost that #insert could absorb looks like the label-value escaping. Options 1 and 2 read that way to me as well, but the opening line says "label names", and I would rather ask than solve the wrong half.

Assuming label values, I would go with your option 2. Beyond the compatibility point you make, the stored labels are round-tripped by remove() and by the Summary pruning path, so replacing them in place changes observable behaviour — #798 ran into exactly that.

One gap I would want to cover either way: formatLabels() is also called for exemplar label sets (lib/registry.js:100), and those do not go through LabelMap#insert. They would still need escaping at render time unless handled separately.

If you would rather keep this one yourself, no problem at all — and if option 1 is what you actually want, I am happy to do it that way instead.

[jdmarshall]

I was hoping to look at this one but I am six hours into #803 and all I've managed to do is refactor the async and promise code to something more amenable to adding new features.

Yes, I meant the label values. I had forgotten how convoluted the registry code was there. I do recall tamping it down a little bit but 'a little bit' seems to leave a lot to be desired.

I'm not sure it does affect remove(), because that's running keyFrom() and that would presumably be using the originally input data. What code path are you seeing that would result in a miss? If there's a path there then unfortunately that means we cannot pre-escape the values, yeah. I think we don't actually want to escape the values within keyFrom as that's another very hot path. So it's down to a matter of whether lookups can ever fail.

_defaultLabels in registry.js is another point of interest.

I think I'm okay if formatExemplars() needs to be added to keep things sane. I don't know if the code reuse there is actually strong or ducky. I think it depends on whether you consider exemplars as actually being labels, or being label-like.

Looking at the code, there's a much simpler optimization before getting to the escaping logic. formatLabels() takes an optional parameter that's only used in one call site. A filter() at the caller simplifies the loop logic for everyone else. And that makes the two formatters pretty simple.

[milcho0604]

Yes, there is one — Summary's pruning.

lib/summary.js:73 calls this.store.remove(entry.labels) with the stored labels rather than caller input, and keyFrom() concatenates raw values (lib/util.js:431) while #insert is handed a key computed before the labels are stored (lib/util.js:267). Pre-escape inside #insert and the two diverge.

Measured on this branch — patched #insert to escape string values, aged a summary out, then counted what was left in the store:

stored value	unpatched	pre-escaped at insert
a"b	pruned	still in the map
plain	pruned	pruned

It is also invisible to the suite. Instead of touching escaping I made remove() early-return whenever a label value matches /["\\\n]/, leaving rendering alone: all 574 jest tests still pass. The one pruning test (test/summaryTest.js:571) has no labels at all, so keyFrom() returns '' either way and it cannot reach this; and it asserts on exported values, while the prune branch skips the export whether or not the delete lands. The entry just stays in the map with nothing showing at the exposition.

That said, I do not think this alone forces "we cannot pre-escape". The mismatch exists only because the entry does not remember the key it was filed under, and #insert already has that key in hand. Storing it on the entry and having the prune path delete by it would remove the round-trip without adding anything to keyFrom().

What looks like the harder constraint is the public surface:

• getMetricsAsJSON() returns metric.get() results directly (lib/registry.js:145), and get() hands back the stored labels by reference — (await counter.get()).values[0].labels === the object in the store, checked on this branch. Under option 1 those come back escaped (a"b → a\"b, a newline as a literal \n). index.d.ts still types them as string, so nothing would warn a consumer.
• The same objects round-trip today: metric.remove((await metric.get()).values[i].labels) removes the series, and would silently stop matching for exactly the values that need escaping.
• Three label sources never reach #insert yet are escaped at render today: registry default labels merged in at lib/registry.js:83, exemplar label sets at :100, and — the one I would worry about most — any custom collector's labels, which the registry consumes directly at :71. If render-time escaping goes away, those emit malformed exposition.

Option 2 keeps all of that intact, which is probably the stronger reason for it, ahead of the pruning path.

On whether the win justifies it, I measured a ceiling: escapeLabelValue replaced by the identity function, which no real implementation can beat. One build per process, node v25.9.0, median of repeated metrics():

workload	with escaping	without	ceiling
benchmarks/registry.js fixture, 2 labels × 64 series	0.242 ms	0.224 ms	7%
same fixture, 6 labels × 64 series	0.305 ms	0.264 ms	13%
same fixture, 4 labels × 1600 series	7.90 ms	7.22 ms	9%
custom collector, 6 labels × 30k series	35.9 ms	23.6 ms	34%
same, values actually containing " \ \n	63.2 ms	28.4 ms	55%

The share tracks how many label values get rendered per scrape rather than how many series, and it roughly doubles once values genuinely need escaping, since the .replace then allocates. On a small registry it is single digits; on a large one it is a third of the scrape, so the target looks right to me even if the modest end is what most users see.

Happy to take option 2. And if you would rather land the formatLabels cleanup on its own first, I can do that separately — it does have three call sites with only :89 passing exclude.

[jdmarshall]

I would recommend the formatLabels change first because you don't need the other PR to be merged in order to file the PR, and we can see how much if any that improves the metrics on its own.