-
Notifications
You must be signed in to change notification settings - Fork 0
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
Range rules #154
Range rules #154
Conversation
Allows matching on when the alert appeared
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.
A few points for discussion, but overall it LGTM.
pkg/aggregation/aggregator.go
Outdated
// meaning the rules are combined via `AND`. | ||
matchCount := 0 | ||
for a, b := range res { | ||
if b.MatchString(a) { | ||
for val, matcher := range matchers { |
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.
A bit of nitpicking, val is used as a keyword in some programming languages, which did confuse me as well while skipping over the changes at first.
Would you consider renaming the variable?
pkg/config/rule_matching.go
Outdated
} | ||
|
||
func ParseRuleMatcher(label, s string) RuleMatcher { | ||
if strings.HasPrefix(s, "~= ") { |
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.
Depending on how strict you want to be here, it might be nice to use whitespaces as delimiters, instead of static prefix/startAtSymbolX. This would allow the user to i.e. align their config values.
Nice to have, being strict is fine as well.
pkg/config/rule_matching.go
Outdated
} else if strings.HasPrefix(s, "> ") { | ||
return newNumberMatcher(gt, s[2:]) | ||
} else if strings.HasPrefix(s, "= ") { | ||
if _, err := strconv.ParseFloat(s[2:], 64); err == nil { |
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.
I think this warrants at least a note in the documentation, that "=" on numbers is not an exact equals, but, due to the nature of floats, is an approximation.
readme, fix typo in matching rules Co-authored-by: Oliver Schmitt <DerGeras@users.noreply.github.com>
* Make it possible to align configuration
when
rule to be able to match on when the alert happened.greater than
on numerical values. SeeREADME
for details.what
rules are now combined viaAND
with label rules.This streamlines the behaviour, making it behave like the label
rules themselves. It also makes it possible e.g. to express that
a rule matcher only applies when the alert is old.
Rule changes:
what
rules will combine asAND
with label rules.when
rules will combine withAND
with label andwhat
rules.The default is to match the value in the configuration as a regular expression.
However, this can be changed by specifying an operator.
~= string
: Explicitly require a regular expression to be matched= string
: Require the string to exactly match> number
: Require both configuration and the value in the alert to be anumerical value and that the value in the alert to be bigger than the
configured number.
This also applies to the
<
,>=
,<=
operators.