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

Refactor config rule by taking inspiration on Prometheus alerting rule #136

Open
fraxken opened this issue Oct 29, 2023 · 3 comments
Open
Labels
config Related to config enhancement New feature or request

Comments

@fraxken
Copy link
Member

fraxken commented Oct 29, 2023

I think we need to analyze Prometheus Alerting rule template and take inspiration.

The goal is be a bit closer to what the ecosystem already provide (so people are not lost if they use Loki Ruler or Prometheus alerting).

  • expr instead of logql
  • for for duration interval?
  • annotations with summary and description
@fraxken fraxken added enhancement New feature or request config Related to config labels Oct 29, 2023
@PierreDemailly
Copy link
Member

The optional for clause causes Prometheus to wait for a certain duration between first encountering a new expression output vector element and counting an alert as firing for this element. In this case, Prometheus will check that the alert continues to be active during each evaluation for 10 minutes before firing the alert. Elements that are active, but not firing yet, are in the pending state. Alerting rules without the for clause will become active on the first evaluation.

It looks confusing to use for as duration/cron interval since the behavior is different. WDYT?

annotations.summary could replace name and we could add a annotations.description so it can be used in the template, but the behavior is slightly different because with Prometheus alerting rule these fields can be templated with logs labels (i.e app) while on our side this is not possible as we use the name/annotation.summary as an identifier in the database and for logging.

Even the expr is not very accurate to me because Prometheus (PromQL) is indeed based on expressions:

Alerting rules allow you to define alert conditions based on Prometheus expression language expressions and to send notifications about firing alerts to an external service.

But Loki (LogQL) is based on queries, it can be confusing as expr could mean "label filter expression" and/or "line filter expression" (etc): we don't want an expression but a full LogQL query based on multiple expressions instead. It look like if we set expr instead of logql we become closer to Prometheus but farest to Loki (at least this is my feeling).

So, i'm not sure about theses but if it's ok for you, it's ok for me 😃


Templating can also be inspired:

{{ $labels.<labelname> }}

labels instead of label + double brace instead of simple brace ? So we are ISO with Prometheus.

There are some fields we could get inspired but the problem is that they use snake_case while we use camelCase:

group_wait: 0s
group_interval: 1m
repeat_interval: 1h

Maybe there is something to do getting inspiration from theses fields

inhibit_rule is also interesting

@PierreDemailly
Copy link
Member

errorFilters should be errorIgnorePatterns (or something like that)

@PierreDemailly
Copy link
Member

Throttle, use releaseThreshold instead of count ?

CompositeRules, use threshold.rules & threshold.alerts objects ?

{
  "compositeRules": {
    "threshold": {
      "rules": {
        "trigerredCount": 5,
        ...
      },
      "alerts": {
        "count": 20
      }
    }
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Related to config enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants