-
Notifications
You must be signed in to change notification settings - Fork 60
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 multiple conditions with logic for AlertPolicy #218
Labels
enhancement
New feature or request
v2alpha1
Incompatible changes proposed for the next major release
Comments
I think this is a reasonable improvements |
nieomylnieja
added
the
v2alpha1
Incompatible changes proposed for the next major release
label
Nov 17, 2023
@Simon-Boyer, I think we can proceed with your proposal, before we make anything official, I'd be grateful if you could describe the proposed changes here: https://github.com/OpenSLO/OpenSLO/blob/main/enhancements/v2alpha1.md#datasource :) |
Sounds good, I'll try to open a PR this week |
Last weeks were a bit chaotic, finally had some time to write the PR: #240 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
New feature or request
v2alpha1
Incompatible changes proposed for the next major release
Problem to solve
Currently, the specifications only allow for 1 condition to be specified in an AlertPolicy. This can be limiting, particularly when setting up multi-window alerts. There should be some ways to setup AND/OR logics for conditions.
Proposal
I think the easiest way to do it is to have an AND logic at the top level, like slogen is doing here: https://github.com/OpenSLO/slogen/blob/main/samples/sumologic/v1/alert-policy/burn-rate.yaml. For me that is the most "natural" assumption and it would also keep slogen in the spec without any change.
To support OR logic (and even more complex AND), we could have and or/and keyword. So it could look something like that:
which is roughly equivalent to the expression for the "page" level in the SRE workbook I just linked to:
Or you could simply have multiple conditions which all needs to be met, in which case you have to option to stay at the top level:
This will allow it to be backward compatible with the current v1 spec, will be compatible with what slogen does and will allow for more complex and/or logic expressions.
Further details
Briefly discussed previously here: #54 (comment)
Links / references
https://sre.google/workbook/alerting-on-slos/#6-multiwindow-multi-burn-rate-alerts
https://github.com/OpenSLO/slogen/blob/main/samples/sumologic/v1/alert-policy/burn-rate.yaml
The text was updated successfully, but these errors were encountered: