perf(metrics): partition ValueMap into per-thread shards to reduce lock contention

[souyang-dev]

Metrics ValueMap is protected by a RwLock. It can be a source of contention under high concurrency. What's more, the cost to update a histogram is even higher because you need to grab a mutex lock after the read lock before you can update a bucket.
There is a previous attempt to shard the ValueMap (#1564), but that PR partitions metrics updates based on attribute-sets, therefore updates to the same attribute-set still land on the same shard, so it's not effective in reducing locking contention when most updates are focused on a small set of attributes.

Fixes #
This PR partitions the single HashMap into shards. Each thread update different shards based on thread id to minimize locking contention. If the number of shards >= number of CPUs you won't see locking contentions when updating metrics.

Here are Benchcmark results (metrics_histogram, metrics) with AMD EPYC 9555 CPUs (255 CPU threads).
The numbers in the tables are updates/second in millions. "shards=16" is this PR with 16 shards, "shards=32" is this PR with 32 shards.

Counter update throughput (cargo run --release --package stress --bin metrics -- N)

number of threads	1	2	4	8	16
upstream	10	5.7	6.8	6	5.5
shards=16	10	22	32	40	43
shards=32	10	22	31	48	100

Histogram update throughput (cargo run --release --package stress --bin metrics_histogram -- N)

number of threads	1	2	4	8	16
upstream	9.5	4.8	5.2	6	5.5
shards=16	9.5	19	28	40	42
shards=32	9.5	19	28	45	84

Changes

Two changes are made:
1, Partition ValueMap into shards and let different threads update different shards to minimize locking contention.
2, Implement merge function for Aggregators, so collector can merge the shards.

Merge requirement checklist

• [* ] CONTRIBUTING guidelines followed
• [* ] Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: souyang-dev / name: Shawn (4aa7aa9, 6dfac19, 9676b42, ff20752)

[codecov]

Codecov Report

❌ Patch coverage is 94.90196% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.0%. Comparing base (8db5a12) to head (ee38917).
⚠️ Report is 18 commits behind head on main.

Files with missing lines	Patch %	Lines
...-sdk/src/metrics/internal/exponential_histogram.rs	93.7%	16 Missing ⚠️
opentelemetry-sdk/src/metrics/internal/mod.rs	95.4%	10 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #3473     +/-   ##
=======================================
+ Coverage   82.8%   83.0%   +0.2%     
=======================================
  Files        130     130             
  Lines      27289   27701    +412     
=======================================
+ Hits       22622   23019    +397     
- Misses      4667    4682     +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[scottgerring]

Pinging @cijothomas because metrics is really his area :) !

I'm not sure about this part but it might be a gap in my mental model. The metrics SDK spec defines aggregation_cardinality_limit as "the maximum number of data points allowed to be emitted in a collection cycle by a single instrument" and says that, for a given metric, the cardinality limit is a "hard limit" on the number of metric points collected during a collection cycle:

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#cardinality-limits

It looks like with this change, the effective limit becomes roughly configured_limit * num_shards where num_shards is the smaller of CPU count or 64, so a configured limit of 2000 can produce many more than 2000 points.

I think sharding needs to remain an internal implementation detail and preserve the configured stream cardinality limit. The user-visible impact is that a limit chosen to cap SDK memory usage and downstream time-series cardinality would no longer provide that cap; the same configuration could produce very different numbers of exported
series depending on CPU count.

[cijothomas]

Previous shard attempts were also blocked on this same problem. It is non-trivial to solve with sharding approach, unfortunately. We'd need to solve contention issue differently where consciously opts in to this model. (I'll write what I have in my mind soon)

[souyang-dev]

Thank you @scottgerring for the comments.
I have updated the patch to enforce cardinality limit across all shards.

[cijothomas]

Will hold off to finish the current release. and Review when the current release is out (this week)

[cijothomas]

Trying to understand the reason why we minimum use 64? Aggregators pre-allocate memory for each Metric upto its cardinality limit. So this would cause it to be done 64 times. If a user has a simple MeterProvider/low volume, we'd be taking up a lot of memory?

[cijothomas]

previously, we were taking read lock only and its upgraded to write lock only if we have stale entries. This allows updates to continue to work on existing attribute set.
it looks like we now have a write lock here, and it held while iterating entire hashmap, blocking updates to even existing attribute set. This would cause jitter around collection intervals.

[cijothomas]

@souyang-dev
Thanks for the work here — the contention problem and the numbers are both real! As discussed earlier/offline, I'd like to really make sure if this is the right direction for us to pursue.

I think we should revisit the thread-local instrument idea from #1386 (likely as an evolution of bound instruments, added recently): an opt-in !Send handle that owns its own aggregate, merged centrally at collect time. The merge_to and expo-histogram merge logic from this PR would carry over directly, so this exploration helps. (OTel Arrow project already implemented a version of this for its internal metrics! They cannot use OTel Rust sdk today due to this exact problem!)

Related — @martintmk did some benchmarks in #1386 It showed ~92M/s with a per-thread MeterProvider vs ~8M/s shared, with no SDK changes at all. That's comparable to the gains in this PR, achievable today by end users. I think we should also document this pattern for users who need the throughput now, while we work on the proper local-instrument API.

Would you be open to redirecting the effort in that direction?

(Apologies, I haven't fully review this PR yet. These are quite complex PRs and need lot of time to thoroughly review. Which is why I want to do a quick glance, and confirm our direction before moving too deep. Hope that makes sense)