Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cardinality): Track exceeded CardinalityLimit for each bucket #3825

Merged
merged 13 commits into from
Jul 18, 2024

Conversation

loewenheim
Copy link
Contributor

Fixes #3806.

Outcome::CardinalityLimited now contains the IDs of the CardinalityLimits that were exceeded. We use a SmallVec<[String; 4]> for this (4 chosen essentially at random—please advise on what a sensible number would be here).

We track which cardinality limits are exceeded by a bucket in DefaultReporter. This tracking propagates through CardinalityLimits to CardinalityLimitsSplit, which gains a new field exceeded_limits. Finally, in cardinality_limit_buckets, we now call track with each bucket individually—each of them has a different outcome with different exceeded limits, so it's not possible to do this in bulk.

@loewenheim loewenheim requested a review from a team as a code owner July 17, 2024 12:56
@loewenheim loewenheim self-assigned this Jul 17, 2024
@loewenheim loewenheim changed the title Feat/cardinality limited reason feat: Track exceeded CardinalityLimits for each bucket Jul 17, 2024
relay-cardinality/src/limiter.rs Outdated Show resolved Hide resolved
relay-cardinality/src/limiter.rs Outdated Show resolved Hide resolved
relay-cardinality/src/limiter.rs Outdated Show resolved Hide resolved
relay-cardinality/src/limiter.rs Outdated Show resolved Hide resolved
relay-server/src/services/outcome.rs Outdated Show resolved Hide resolved
relay-server/src/services/outcome.rs Outdated Show resolved Hide resolved
relay-server/src/services/outcome.rs Outdated Show resolved Hide resolved
@loewenheim
Copy link
Contributor Author

Update: we now only store one limit per CardinalityLimit outcome, for implementation simplicity. It's expected to be the most common case anyway.

relay-cardinality/src/limiter.rs Outdated Show resolved Hide resolved
That impl doesn't really make much sense for a single limit.
@loewenheim loewenheim changed the title feat: Track exceeded CardinalityLimits for each bucket feat(cardinality): Track exceeded CardinalityLimit for each bucket Jul 18, 2024
@loewenheim loewenheim enabled auto-merge (squash) July 18, 2024 12:16
@loewenheim loewenheim merged commit e0a9c13 into master Jul 18, 2024
23 checks passed
@loewenheim loewenheim deleted the feat/cardinality-limited-reason branch July 18, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metric Stats: Include which cardinality limit dropped the metrics in the reason
2 participants