open-telemetry / open-telemetry/opentelemetry-cpp
[Metrics SDK] Enforce MetricReader-level cardinality limit as a fallback during collection
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 1.4k
- Forks
- 632
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 75
Description
Background
#4188 added MetricReader::SetCardinalityLimits() / GetCardinalityLimit(), storing per-instrument-type reader-level cardinality limits, but explicitly left them unenforced (see the TODO in MetricReader::SetCardinalityLimits()). The initial attempt to enforce them by resolving the reader's limit directly into the shared per-view metric storage was reverted during review, since it broke per-reader semantics: with two readers sharing one instrument's storage (limit 10 and limit 1000), using the reader's limit for shared storage capacity let the low-limit reader see far more series than configured (see https://github.com/open-telemetry/opentelemetry-cpp/pull/4188#discussion_r3506330000 area).
Per the spec, cardinality limit precedence is View > Reader > SDK default. Today only View-level (AggregationConfig::cardinality_limit_) and the SDK default are enforced.
Proposal
- Shared recording storage (the
AttributesHashMapinsideSyncMetricStorage/AsyncMetricStorage) should be sized at the max cardinality limit across allMetricReaders currently attached, when the view has no explicit limit of its own, so no reader loses data purely because storage was capped too low for it. - Each reader's own (possibly stricter) limit should then be re-applied to just its own output during collection (
TemporalMetricStorage::buildMetrics), so a stricter reader is still capped to what it configured, without affecting other readers sharing the same storage.
I'd like to work on this — will follow up with a PR shortly.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the TODO in MetricReader::SetCardinalityLimits(), then trace AttributesHashMap in SyncMetricStorage and AsyncMetricStorage through TemporalMetricStorage::buildMetrics. Verify how view-level and SDK-default limits are currently applied. Done means shared storage accommodates the maximum attached-reader limit while each reader's collection output honors its own stricter limit.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- observability-sre
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100