open-telemetry / open-telemetry/opentelemetry-cpp

[Metrics SDK] Enforce MetricReader-level cardinality limit as a fallback during collection

Open
#4,387 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

triage/accepted
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 AttributesHashMap inside SyncMetricStorage/AsyncMetricStorage) should be sized at the max cardinality limit across all MetricReaders 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.