-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Limit the number of reported contexts #7084
Conversation
d55105f
to
283b30d
Compare
4329653
to
ae4fff1
Compare
Codecov Report
|
ae4fff1
to
35a4e21
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left a few comments to make the new log messages consistent.
Co-authored-by: Christine Chen <ChristineTChen@users.noreply.github.com>
b79a356
to
a3bb5fa
Compare
a3bb5fa
to
e1e18f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great
The Kafka consumer checks counts the total number of submitted context and displays a warning when the check submits too many metrics.
But the check still collects all metrics, this is a regression introduced here: c4a29b1
On the legacy implementation the limit is applied at collection time. However, as the new implementation uses futures to perform the collection this strategy can only be partially applied and so the limit is applied on submission time.