kube-logging / logging-operator

Logging operator for Kubernetes
https://kube-logging.dev
Apache License 2.0
1.52k stars 326 forks source link

Make severity configurable in PrometheusRules #1749

Open sebastiangaiser opened 2 months ago

sebastiangaiser commented 2 months ago

Is your feature request related to a problem? Please describe. Currently the severity is hardcoded in the PrometheusRules like here

Describe the solution you'd like It would be great to have them configurable via the Logging CR e.g.:

spec:
  fluentd:
    metrics:
      prometheusRules: true
      severity: my-custom-severity
  fluentbit:
    metrics:
      prometheusRules: true
      severity: my-custom-severity
  syslogNG:
    metrics:
      prometheusRules: true
      severity: my-custom-severity

Describe alternatives you've considered My current workaround is to disable the PrometheusRules and deploy them in the way I want

Additional context Logging Operator docs: https://kube-logging.dev/docs/operation/alerting/

sebastiangaiser commented 1 month ago

@pepov sorry to ping you but could you please mind have a look :D

pepov commented 1 month ago

Good idea, but I don't have any free cycles currently. We are looking for contributors to take on "good first issues" like this!

aditya7302 commented 1 month ago

@pepov I would like to work on this issue. Can you assign it to me ?

pepov commented 3 weeks ago

@aditya7302 sure, thanks!

aditya7302 commented 3 weeks ago

@pepov is this crd is where severity metric should be added to make it configurable ?

pepov commented 3 weeks ago

@aditya7302 that is generated from the go code. I recommend to check the Metrics type that could be extended with a severity field and applied everywhere where a prometheusrule is defined, for example here

By looking at the code, my problem is, that using this method we would have to use the same severity for all the prometheusrules that are defined for a single metrics endpoint. For example in case of fluentbit there are two rules defined, one with warning and one with critical severity. How would the proposed severity setting work in this case? 🤔

sebastiangaiser commented 3 weeks ago

@pepov @aditya7302 maybe my comment now is a bit controversial to remove the PrometheusRules completely or making the whole Rules block configurable.

pepov commented 3 weeks ago

I would add a list based override option that would allow setting overrides for the rules based on the list index. Also we could define a type that would allow overriding other fields as well for the rule.

For example the below metrics config would keep the first rule unmodified but would change a few properties of the second rule.

    metrics:
      prometheusRules: true
      prometheusRuleOverrides: 
        - {}
        - for: 1m
          labels:
             severity: warning
             additionalLabel: b

I think we could also allow a list item to be removed if we set the corresponding item to nil.

sebastiangaiser commented 3 weeks ago

Sounds great 🚀

aditya7302 commented 2 weeks ago

@pepov I have made some changes to make severity configurable. Please review the changes.