-
Notifications
You must be signed in to change notification settings - Fork 598
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
fix(alerts): add instance label to KubeAggregatedAPIErrors #991
Conversation
Signed-off-by: Sebastian Gaiser <[email protected]>
Another idea would be to add a |
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.
Thanks for contributing and drawing my attention to this!
I ended up doing a bit of a deep dive to try and understand the metric properly, left you a suggestion on improving the PromQL query in the review.
Please could you add the 'for': '10m'
to the alert?
And update the description to something like:
description: 'Kubernetes aggregated API {{ $labels.instance }}/{{ $labels.name }} has reported {{ $labels.reason }} errors%s.' % [
(utils.ifShowMultiCluster($._config, ' on cluster {{ $labels.%(clusterLabel)s }}' % $._config),
],
I'm hoping all of that would resolve the underlying issue, wdyt?
Co-authored-by: Stephen Lang <[email protected]>
…adjust description Signed-off-by: Sebastian Gaiser <[email protected]>
Sounds good. Thank you @skl for investing time here, I added all your suggestions. |
Signed-off-by: Stephen Lang <[email protected]>
Fixed a syntax error for you in 21c0b87 |
Lint failure inherited from main, this can be ignored as I'm dealing with that separately
|
Supersede of #774
@skl please let me know what you think about this ;)