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

Move error samplers from Limiter to Ingester #6014

Merged
merged 4 commits into from
Sep 13, 2023
Merged

Conversation

duricanikolic
Copy link
Contributor

@duricanikolic duricanikolic commented Sep 12, 2023

What this PR does

In PR #5584 we introduced sampled logging of errors. The logic of sampling is implented in the Sampler struct. The samplers are currently stored in the samplers field of Limiter.

This PR moves the samplers field from Limiter to Ingester (and renames it to errorSamplers), which is a more appropriate place for storing them.

Moreover, it improves the already existing tests TestIngester_SampledUserLimitExceeded and TestIngester_SampledMetricLimitExceeded by ensuring that samples errors get tracked by the cortex_discarded_samples_total counter.

Which issue(s) this PR fixes or relates to

Relates to #1900, #5894

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@duricanikolic duricanikolic requested a review from a team as a code owner September 12, 2023 21:40
@duricanikolic duricanikolic self-assigned this Sep 12, 2023
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
…dMetricLimitExceeded

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>
@colega colega enabled auto-merge (squash) September 13, 2023 07:27
@colega colega merged commit e1d79ee into main Sep 13, 2023
28 checks passed
@colega colega deleted the yuri/sampled-error-logging branch September 13, 2023 07:44
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.

2 participants