-
Notifications
You must be signed in to change notification settings - Fork 550
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
Add user label to ingested samples metrics #1533
Conversation
This metric is used in several alerts. Adding a per-user label it will become high cardinality, and so such alerts more complex to evaluate. We should check if for some of such alerts we can use a different metric, or discuss alternative ideas (eg. add recording rule). |
b9ac9c1
to
f5b5d67
Compare
f5b5d67
to
7ee319b
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.
Changes LGTM. Let's look at where these metrics are used and then we can merge.
Note to self: Marco suggests adding recording rule named
|
26be470
to
38187ff
Compare
211aa0e
to
815b548
Compare
@pracucci with the help of @pstibrany I've crafted in total three rules, and converted relevant alerts to use them. PTAL when you have the time :) |
84369c1
to
aa19bcf
Compare
Please rebase this PR on top of |
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.
Good job! The recording and alerting rules LGTM 👏
There's just a thing I would like you to check: why the compiled mixin .yaml
files have been reformatted? I can't explain it. Could you try to change the make build-mixin
target to run in the build-image (needs to be moved in the Makefile) and check if they're still reformatted?
I answered my question (and fixed Makefile) in this PR: Please rebase once merged. Thanks! |
Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
Co-authored-by: Marco Pracucci <[email protected]> Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
69afed2
to
9f7059a
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.
LGTM, thanks!
What this PR does
Add
user
label to ingested samples metrics:The motivation is that we need to be able to differentiate on the tenant when determining the percentage of samples getting dropped by our ingesters.
TODO:
Which issue(s) this PR fixes or relates to
Fixes #1531.
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]