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

Implement cardinality limits for metric streams #1066

Merged
merged 14 commits into from
May 25, 2023

Conversation

lzchen
Copy link
Contributor

@lzchen lzchen commented May 19, 2023

Part of #1065

This PR implements a fixed cardinality limit for metric data points of the same stream to 2000. If an attempt to use an instrument to create a new metric data point for the same stream is made which would exceed the limit, the data point is added/recorded to the data point representing the overflow metric stream which is denoted by an AttributeSet containing only {otel.metric.overflow: True}.

Note there is a bit of duplication of code done on purpose to optimize for the most common scenarios, which is usually when an entry exists already within a hashmap.

@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Patch coverage: 35.5% and project coverage change: -0.1 ⚠️

Comparison is base (e9adef6) 50.9% compared to head (6339e41) 50.8%.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1066     +/-   ##
=======================================
- Coverage   50.9%   50.8%   -0.1%     
=======================================
  Files        165     165             
  Lines      19689   19751     +62     
=======================================
+ Hits       10025   10037     +12     
- Misses      9664    9714     +50     
Impacted Files Coverage Δ
...entelemetry-sdk/src/metrics/internal/last_value.rs 0.0% <0.0%> (ø)
opentelemetry-sdk/src/metrics/internal/sum.rs 21.0% <26.3%> (-0.3%) ⬇️
...entelemetry-sdk/src/metrics/internal/aggregator.rs 30.0% <42.8%> (+30.0%) ⬆️
...pentelemetry-sdk/src/metrics/internal/histogram.rs 60.1% <63.3%> (-2.9%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@lzchen lzchen marked this pull request as ready for review May 19, 2023 22:02
@lzchen lzchen requested a review from a team May 19, 2023 22:02
@lzchen lzchen changed the title [WIP] Implement cardinality limits for metric streams Implement cardinality limits for metric streams May 19, 2023
opentelemetry-sdk/Cargo.toml Outdated Show resolved Hide resolved
opentelemetry-sdk/src/metrics/internal/aggregator.rs Outdated Show resolved Hide resolved
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.

3 participants