canonical / prometheus-k8s-operator

https://charmhub.io/prometheus-k8s
Apache License 2.0
21 stars 35 forks source link

Add explicit support for recording rules #428

Open lucabello opened 1 year ago

lucabello commented 1 year ago

Enhancement Proposal

Issue

Currently, Prometheus recording rules are implicitly supported; if they are put in the prometheus_alert_folder, the library will pick them up and import them correctly.

However, following our discussion, there should be an explicit support for recording rules in a separate folder.

Solution

I will sum up the points made in the discussion to serve as guidelines for the PR.

Ideally, we would refactor all mentions of alert rules to the more general rules, so that all the functions that currently handle alert rules would be suited to handle recording rules as well. This includes quite some documentation changes to reflect the new code.

The library should take two configurable paths, one for alert rules (i.e., prometheus_alert_folder) and one for recording rules (i.e., prometheus_record_folder). There's no need for explicit checking on the rule type; the goal is to encourage the splitting of different rule types in different folder, not to enforce their separation. However, we could emit a warning when a recording rule is found in the alert rule folder, and vice versa.

The documentation should only describe the suggested behavior of using the two different folders.

As per the AlertRule class itself, it was suggested to implement the following structure:

class Rules(...):  # the old AlertRules class, after refactoring

class AlertRules(Rules):
  """Some docstring, but no unique methods needed yet."""

class RecordingRules(Rules):
  """Some docstring, but no unique methods needed yet."""

This class separation would allow us to implement specific behaviors for different rule types in the future, if we ever need to do so.

The library should then import rules from both folders and process them as it does now.