v1.12.0 exposes some high cardinality metrics

[prymitive]

It seems that v1.12.0 includes some extra high cardinality metrics, most likely autogenerated by Go itself thanks to #955.

Here's a full diff of metrics between 1.11 and 1.12 - https://gist.github.com/prymitive/3d16b79a8187448bf6e0d61db7336ca0

Having so many extra metrics exposed by default might cause problems for people with lots of Go services.

[kakkoyun]

Thanks for reporting. It seems like we need to think more about default bucket configuration.
@mknyszek what do you think?

[kakkoyun]

cc @bwplotka

[beorn7]

Yeah, that's rather extreme for default metrics. Perhaps those could be opt-in at that granularity.

I would also like to change that before releasing the final Prometheus v2.33.

[mknyszek]

Yeah, the histograms in runtime/metrics are large by some standards. The time-related ones have pretty high granularity, but it's fairly straightforward to downsample and make them exponentially bucketed. Maybe some code to check a threshold on bucket granularity and then downsample if necessary?

FWIW I think it would be nice keep the allocs-by-size and frees-by-size as-is -- they're bounded by the number of size classes (currently; that can change but likely won't change drastically), so that's 136 total. Is that too much? Perhaps that's why MemStats.BySize wasn't included initially.

[mknyszek]

Having so many extra metrics exposed by default might cause problems for people with lots of Go services.

Out of curiosity, what kind of problems do you expect?

[beorn7]

Having so many extra metrics exposed by default might cause problems for people with lots of Go services.

Out of curiosity, what kind of problems do you expect?

If these metrics came with a newly introduced package, it might be fine. Users could do the resource planning for it or decide to drop them on ingestion. However, the problem is that these default Go metrics are in a lot of existing binaries, and all of them will suddenly expose a significantly increased number of metrics after merely bumping the dependencies to the current minor release. This might easily push many users beyond their cardinality budget.

[mknyszek]

This might easily push many users beyond their cardinality budget.

Not to derail this too hard, but just for my understanding: what does this cardinality budget look like? Memory? Throughput?

[prymitive]

Each unique time series eats ~4KB on average in our production environments. The diff I posted has more than 1k lines. Adding 1k extra metrics adds ~4MB of Prometheus memory per instance for every Go service scraped by Prometheus (plus some memory for exposing those metrics in those services). 4MB might not be much but it adds up quickly.

[mknyszek]

Thanks, that's good to know. I meant to say this earlier, but I'll send a PR to reduce the bucket granularity.

I'm thinking something like a function that iterates over the buckets and merges buckets within either a factor of 2 or a factor of 10 (depending on the metric unit), making the buckets exponential. Does this sound reasonable?

Opt-in to higher granularity can be future work; an exponential distribution is already pretty useful.

[prymitive]

FWIW I think it would be nice keep the allocs-by-size and frees-by-size as-is -- they're bounded by the number of size classes (currently; that can change but likely won't change drastically), so that's 136 total. Is that too much? Perhaps that's why MemStats.BySize wasn't included initially.

Is 136 too much?
I see that an example Go service I run with only a few custom metrics exposes ~54 time series on average across all instances. If you add 136 time series to that you're more than doubling metrics footprint of client_golang and that's a big jump. Nowhere near as huge as adding over a 1k time series but still relatively large.
It's probably gonna be fine for some people, might be too much for others. It can be dropped via relabel rules if needed, but that requires everyone to update scrape configs.

[mknyszek]

@prymitive Sure. I can also reduce the impact of this further if there's a way to alias metrics (i.e. just say "here's metric name, but it's really just this other metric, so use that data instead") since some existing metrics are re-exported under new names. I can also drop the new names for just those metrics altogether, but that would make for a somewhat worse user experience. Currently, the names of the metrics are generated from the runtime/metrics keys, so there's a clear mapping and it's easy to find a metric from knowing its runtime/metrics key and vice versa.

[beorn7]

I leave it to @bwplotka and @kakkoyun to make the call what increase in metric cardinality is fine for a minor release. As I might have said before, this library is used a lot, so we have to navigate with care and in doubt make potentially disruptive features opt-in.

[mknyszek]

For sure. The current state of things is not great. I didn't anticipate that each histogram bucket would be treated as an individual metric, and I also forgot quite how big I made these histograms. 😅

Going back over the list, it looks like at a bare minimum (without removing "duplicate" metrics -- I'd rather not because the new forms of them are more orthogonal and clearer; the divisions and groupings of memory in runtime.MemStats are pretty arbitrary), that's 25 new individual metrics and 4 new histograms.

2 of those new histograms are time histograms. They're high dynamic range histograms which are a combination of exponentially and linearly bucketed. Collapsing those down to be exponentially bucketed, we get 45 buckets each. We can then square the base of the exponential series and cut that down to 23, and then cut out a lot of the higher buckets (the range goes all the way up to O(days), we can just catch everything above 1 second into its own bucket for all of the existing ones). That gets us down to about 15. For the other two histograms, we can just do power-of-two exponential bucketing which collapses the 67 or so buckets down to 13 or 14.

To be clear, this code would be generic, so any new runtime/metrics histograms metrics with unit "bytes" or "seconds" would immediately be minimized. I can add a test that ensures new metrics to be audited via the generator do not accidentally create a whole bunch of bloat.

Another alternative is a Summary but I have a slight preference against that, just because it implicitly assumes a normal distribution of the underlying data where it may not exist, resulting in potentially misleading information.

So, in total, that's 83 additional metrics assuming we just shrink the histograms as I suggested above. According to @prymitive's back-of-envelope math that's an added on-server bloat of about 400 KiB for this release. Like I said earlier we can cut out metrics that already exist in their old form (in some fashion) (EDIT: cut out new metrics that represent the same information as old metrics with a new name and a slightly different form) and get back an additional 23. Personally I'd prefer being more aggressive about scaling down the histograms than getting rid of the "duplicate" metrics, but if this isn't acceptable then so be it. Earlier I asked about an aliasing mechanism because some of these are a 1:1 mapping, and so we could avoid the storage of both by just having one point to the other's data in the backend.

Side-note: the reasoning behind these huge histograms is it doesn't cost much (< 10 KiB I'll guess?) to actually manage these in the runtime, and it's better for the runtime to expose more granular data than less granular data. It's totally reasonable for downstream users to adjust that granularity.

[kakkoyun]

So, in total, that's 83 additional metrics assuming we just shrink the histograms as I suggested above. According to @prymitive's back-of-envelope math that's an added on-server bloat of about 400 KiB for this release. Like I said earlier we can cut out metrics that already exist in their old form (in some fashion) and get back an additional 23. Personally I'd prefer being more aggressive about scaling down the histograms than getting rid of the "duplicate" metrics, but if this isn't acceptable then so be it.

First of all, let's not remove old metrics. There are lots of dashboards out there would be broken.

And @mknyszek as you suggested let's try to aggressively reduce the number of the buckets. With the a reason number of cardinality increase and better communication, this should be doable.

To be clear, this code would be generic, so any new runtime/metrics histograms metrics with unit "bytes" or "seconds" would immediately be minimized. I can add a test that ensures new metrics to be audited via the generator do not accidentally create a whole bunch of bloat.

Yes, tests are essential for this case.

And also as @beorn7 suggested, maybe we should go in a direction to make these additional costly metrics opt-in rather than enabling them implicitly until the major version bump.

Or as it already discussed we can start with really conservative bucket distribution for all of them and increase the granularity with configuration rather than having an additional collector.

Personally I would prefer this approach more. Otherwise, it would take longer times to discover these new useful metrics.

What do you think? @bwplotka

[mknyszek]

So, in total, that's 83 additional metrics assuming we just shrink the histograms as I suggested above. According to @prymitive's back-of-envelope math that's an added on-server bloat of about 400 KiB for this release. Like I said earlier we can cut out metrics that already exist in their old form (in some fashion) and get back an additional 23. Personally I'd prefer being more aggressive about scaling down the histograms than getting rid of the "duplicate" metrics, but if this isn't acceptable then so be it.

First of all, let's not remove old metrics. There are lots of dashboards out there would be broken.

Er, very sorry. I was definitely not suggesting that. I recognize how important that is. And I can see how what I wrote that reads that way, but I meant that we can cut out new metrics that already exist in some form.

And @mknyszek as you suggested let's try to aggressively reduce the number of the buckets. With the a reason number of cardinality increase and better communication, this should be doable.

I have a WIP implementation of the reduced bucketing that means 10 exponential buckets for 2 of the histograms, and 13 for the other 2. That makes for a total of 25+10+10+13+13=71 new metrics. I can have this up for review with tests tomorrow.