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

change: replace cortex_discarded_samples_total label to sample-timestamp-too-old #9885

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

NickAnge
Copy link
Contributor

@NickAnge NickAnge commented Nov 12, 2024

This change was made in order to match err-mimir-sample-timestamp-too-old event logs

What this PR does

This PR replaces cortex_discarded_samples_total label from sample-out-of-bounds to sample-timestamp-too-old.

Which issue(s) this PR fixes or relates to

Fixes #5970

Checklist

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

@NickAnge NickAnge changed the title fix: change cortex_discarded_samples_total label to sample-timestamp-… fix: change cortex_discarded_samples_total label to sample-timestamp-too-old Nov 12, 2024
…too-old

This change was made in order to match err-mimir-sample-timestamp-too-old event logs
@NickAnge NickAnge force-pushed the nickange/ingester/fix-metric-label-sample-out-of-bounds branch from 8b52dc9 to 27b7229 Compare November 12, 2024 17:18
@NickAnge NickAnge changed the title fix: change cortex_discarded_samples_total label to sample-timestamp-too-old change: replace cortex_discarded_samples_total label to sample-timestamp-too-old Nov 12, 2024
@NickAnge NickAnge marked this pull request as ready for review November 12, 2024 17:24
@NickAnge NickAnge requested a review from a team as a code owner November 12, 2024 17:24
@NickAnge NickAnge marked this pull request as draft November 12, 2024 17:32
Signed-off-by: Nikos Angelopoulos <[email protected]>
@NickAnge NickAnge force-pushed the nickange/ingester/fix-metric-label-sample-out-of-bounds branch from a44f512 to b3f2a61 Compare November 12, 2024 17:56
@NickAnge NickAnge marked this pull request as ready for review November 13, 2024 08:31
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM, but would be nice to have someone else give it a look too (maybe someone on the ingest squad?). I found a reference to this label in the GEM docs. Can you fix it there too?

@pr00se pr00se self-requested a review November 15, 2024 01:32
@NickAnge
Copy link
Contributor Author

Thanks @dimitarvdimitrov . I updated it here https://github.com/grafana/backend-enterprise/pull/7628. I assume I need to merge it after this one.

Also I see that @pr00se , has self-requested a review which is part of the ingest team if I am not mistaken

Copy link
Contributor

@pr00se pr00se left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a [CHANGE] entry to the CHANGELOG, but otherwise LGTM, thank you!

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.

err-mimir-sample-timestamp-too-old event logs don't match cortex_discarded_samples_total label
3 participants