Prom-client high memory usage

[shappir]

I have a service that uses several histograms, some with high cardinality. As a result, the retained memory size just for those histograms is 25-30MB. I'm hoping that this commit, and this PR will significantly improve the situation.

[shappir]

Another cause for the large memory consumption are the long hash key values used: all the label name / value pairs concatenated into a string. Here are possible ways to reduce its size:

• Only use the values, not the names. Since the pairs are sorted by name, that will be unique (label names don't add uniqueness)
• Instead of using the actual string as hash key, compute shorter unique hash from it. For example, using a function like this one: https://stackoverflow.com/questions/7616461/generate-a-hash-from-string-in-javascript/52171480#52171480 (It's Open Source: https://github.com/mbaersch/cyrb53-hasher ) Computing this hash looks cheap, and the lookup might actually be faster.

[jdmarshall]

#658 should reduce the memory pressure quite a bit in practice. Switching to Map lets the metrics avoid switching into dictionary mode.

I made a similar change to an app that made extensive use of structured data lookup tables and caches, I saw about a 40% reduction in memory usage for those caches in extensive heap analysis.

Professional Background:

I also have worked on high cardinality projects, so I can empathize. I did a lift-and shift from statsd to otel, and then had to reduce cardinality of our stats substantially in order to go live. I eventually had to push back on the Ops team demanding more reductions when we represented 15% of the overall metrics (for 90% of user facing traffic, and 90% of cache traffic), and I had already reduced our cardinality by 400x and I was getting real, real tired of staring at telemetry code.

hashObject

The time difference between stats with labels and stats without labels was just a hair below a factor of five hundred. That is very, very bad. I think I have it down to between 400 and 440 with all of my PRs, but this is clearly a pretty uncomfortable state of affairs. I was hoping to find more quick wins but a number of my changes accomplished nothing and were reverted.

I agree with you on the keys (not about only encoding the labels though). The problem though is that util has leaked its
implementation details out across half the project which is a no-no. I'm building a new Map implementation that is self-consistent and we'll have to rework the metrics one at a time to adjust that. On the plus side, I can fix the fact that the functions all have their arguments in the wrong order.

However the assertion that a hash value will result in correct behavior is wrong. You need a perfect hash in order to use this for this purpose, otherwise you'll end up with some poor SOB who has histogram data showing up in one of his counters due to a rare collision.

I've experimented with whether JSON.stringify can produce a key faster. It's almost competitive with what's on master, but not quite. I had a commit which tried to optimize hashObject but I've dropped it from #680 and am exploring a sparse key solution.

And there's a bug in the current implementation - ':' is a legal value in prometheus, so we could get an 'a:b,c', "a,b:c" collision with keys. I may fix that if I run out of other things to do.

Storing metrics forever

This area is one of the biggest limitations of prometheus and OTEL over statsD. Every instance of your app with OTEL and Prometheus has to retain every combination of tags it has seen a stat for in perpetuity. If you're running your application for weeks at a time between reboots, eventually every thread on every machine in your cluster sees nearly every combination of values (hostname and appInstance labels notwithstanding). Neither Prometheus nor OpenTelemetry.js have a way to put stale stats on ice and rehydrate them later if they start showing life again.

With StatsD only the collector has to worry about this. It's fire and forget from the sender, and you are free to decide where and how load shedding should be handled. Of course out of the box statsD does not support tags, and I'm rather fond of them having gotten access to them. There are forks that do support them though.

Default Labels

I believe the code already avoids adding defaultLabels to the keys, only adding them at reporting time. However there were no benchmarks of this code and it's about like what you would expect code with no benchmarks to be #683 is about a 20% reduction on the cost of the entire getMetricsAs call chain if defaultLabels exist. I definitely think it bears creating some documentation on this behavior as an advice section. In the latest release of prom-client, default labels double the processing time again versus just labels. In a PR I've got it down to 11kops/s without versus 7kops/s with, while a single label is 30kops/s

One could beef this up by using different Registries for particularly common clusters of default labels. But that could get ridiculous quickly

Aggregation

I'm also working on a PR to avoid concatenating the metric name to the stats labels. Only about a 3% reduction in aggregate(), and a lot of that seems to be reduced GC pauses (about 1.2% less GC events)

There's a multikey library I discovered today. https://github.com/fregante/many-keys-map Might be worth considering.

The problem though is what order do you use for the keys? Alphabetical? Frequency (ascending or descending)? Cardinality (ascending or descending)? It does have the advantage that we could use a sort-by to create an array to pass in as keys:

//TODO: maybe we should hoist filtering of empty labels up to insertion time?
const key = Object.entries(labels).filter([key, val] => val !== undefined).map([key, val] => `${key}|${val}`);
multiMap.set(key, ...);

I believe there is also a bug in the sharedLabelCache (#684) in registry that is causing a high miss rate due to duplicate key (objects). A high miss rate due to duplicate keys will result in excessive memory consumption on collection operations.

Once again the tests are not representative of real workloads.

> let foo = new WeakMap()
undefined
> foo.set({a: 2}, 2)
WeakMap { <items unknown> }
> 
> foo.get({a: 2})
undefined

String interning?

The multimap does a bit of string interning, since each key appears only once for each collection that owns them. However you will have many sibling collections due to repeats, especially for keys that sort to the end of the ordering.

[jdmarshall]

Only use the values, not the names. Since the pairs are sorted by name, that will be unique (label names don't add uniqueness)

I think I see what you mean now. Yes that would be a good change. I can work on that later this week.