mudhorn/prometheus
1
0
Fork 0
mirror of https://github.com/prometheus/prometheus.git synced 2025-02-21 03:16:00 -08:00
Code Issues Actions 27 Packages Projects Releases Wiki Activity

rulefmt: support YAML aliases for Alert/Record/Expr (#14957)

Browse source 
* rulefmt: add tests with YAML aliases for Alert/Record/Expr

Altough somewhat discouraged in favour of using proper configuration
management tools to generate full YAML, it can still be useful in some
situations to use YAML anchors/aliases in rules.

The current implementation is however confusing: aliases will work
everywhere except on the alert/record name and expr

This first commit adds (failing) tests to illustrate the issue, the next
one fixes it. The YAML test file is intentionally filled with anchors
and aliases. Although this is probably not representative of a real-world
use case (which would have less of them), it errs on the safer side.

Signed-off-by: François HORTA <fhorta@scaleway.com>

* rulefmt: support YAML aliases for Alert/Record/Expr

This fixes the use of YAML aliases in alert/recording rule names and
expressions. A side effect of this change is that the RuleNode YAML type is
no longer propagated deeper in the codebase, instead the generic Rule type
can now be used.

Signed-off-by: François HORTA <fhorta@scaleway.com>

* rulefmt: Add test for YAML merge combined with aliases

Currently this does work, but adding a test for the related
functionally here makes sense.

Signed-off-by: David Leadbeater <dgl@dgl.cx>

* rulefmt: Rebase to latest changes

Signed-off-by: David Leadbeater <dgl@dgl.cx>

---------

Signed-off-by: François HORTA <fhorta@scaleway.com>
Signed-off-by: David Leadbeater <dgl@dgl.cx>
Co-authored-by: David Leadbeater <dgl@dgl.cx>
This commit is contained in:
frazou 2025-02-13 10:48:33 +01:00 committed by GitHub
parent 898af6102d
commit 9b4c8f6be2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 139 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
cmd/promtool/main.go
View file

@ -992,11 +992,11 @@ func checkDuplicates(groups []rulefmt.RuleGroup) []compareRuleType {
return duplicates
}
func ruleMetric(rule rulefmt.RuleNode) string {
if rule.Alert.Value != "" {
return rule.Alert.Value
func ruleMetric(rule rulefmt.Rule) string {
if rule.Alert != "" {
return rule.Alert
}
return rule.Record.Value
return rule.Record
}
var checkMetricsUsage = strings.TrimSpace(`

75
model/rulefmt/rulefmt.go
View file

@ -92,7 +92,7 @@ type RuleGroups struct {
}
type ruleGroups struct {
Groups []yaml.Node `yaml:"groups"`
Groups []ruleGroupNode `yaml:"groups"`
}
// Validate validates all rules in the rule groups.
@ -128,9 +128,9 @@ func (g *RuleGroups) Validate(node ruleGroups) (errs []error) {
set[g.Name] = struct{}{}
for i, r := range g.Rules {
for _, node := range g.Rules[i].Validate() {
var ruleName yaml.Node
if r.Alert.Value != "" {
for _, node := range r.Validate(node.Groups[j].Rules[i]) {
var ruleName string
if r.Alert != "" {
ruleName = r.Alert
} else {
ruleName = r.Record
@ -138,7 +138,7 @@ func (g *RuleGroups) Validate(node ruleGroups) (errs []error) {
errs = append(errs, &Error{
Group: g.Name,
Rule: i + 1,
RuleName: ruleName.Value,
RuleName: ruleName,
Err: node,
})
}
@ -154,7 +154,18 @@ type RuleGroup struct {
Interval model.Duration `yaml:"interval,omitempty"`
QueryOffset *model.Duration `yaml:"query_offset,omitempty"`
Limit int `yaml:"limit,omitempty"`
Rules []RuleNode `yaml:"rules"`
Rules []Rule `yaml:"rules"`
Labels map[string]string `yaml:"labels,omitempty"`
}
// ruleGroupNode adds yaml.v3 layer to support line and columns outputs for invalid rule groups.
type ruleGroupNode struct {
yaml.Node
Name string `yaml:"name"`
Interval model.Duration `yaml:"interval,omitempty"`
QueryOffset *model.Duration `yaml:"query_offset,omitempty"`
Limit int `yaml:"limit,omitempty"`
Rules []ruleNode `yaml:"rules"`
Labels map[string]string `yaml:"labels,omitempty"`
}
@ -169,8 +180,8 @@ type Rule struct {
Annotations map[string]string `yaml:"annotations,omitempty"`
}
// RuleNode adds yaml.v3 layer to support line and column outputs for invalid rules.
type RuleNode struct {
// ruleNode adds yaml.v3 layer to support line and column outputs for invalid rules.
type ruleNode struct {
Record yaml.Node `yaml:"record,omitempty"`
Alert yaml.Node `yaml:"alert,omitempty"`
Expr yaml.Node `yaml:"expr"`
@ -181,64 +192,64 @@ type RuleNode struct {
}
// Validate the rule and return a list of encountered errors.
func (r *RuleNode) Validate() (nodes []WrappedError) {
if r.Record.Value != "" && r.Alert.Value != "" {
func (r *Rule) Validate(node ruleNode) (nodes []WrappedError) {
if r.Record != "" && r.Alert != "" {
nodes = append(nodes, WrappedError{
err: errors.New("only one of 'record' and 'alert' must be set"),
node: &r.Record,
nodeAlt: &r.Alert,
node: &node.Record,
nodeAlt: &node.Alert,
})
}
if r.Record.Value == "" && r.Alert.Value == "" {
if r.Record == "" && r.Alert == "" {
nodes = append(nodes, WrappedError{
err: errors.New("one of 'record' or 'alert' must be set"),
node: &r.Record,
nodeAlt: &r.Alert,
node: &node.Record,
nodeAlt: &node.Alert,
})
}
if r.Expr.Value == "" {
if r.Expr == "" {
nodes = append(nodes, WrappedError{
err: errors.New("field 'expr' must be set in rule"),
node: &r.Expr,
node: &node.Expr,
})
} else if _, err := parser.ParseExpr(r.Expr.Value); err != nil {
} else if _, err := parser.ParseExpr(r.Expr); err != nil {
nodes = append(nodes, WrappedError{
err: fmt.Errorf("could not parse expression: %w", err),
node: &r.Expr,
node: &node.Expr,
})
}
if r.Record.Value != "" {
if r.Record != "" {
if len(r.Annotations) > 0 {
nodes = append(nodes, WrappedError{
err: errors.New("invalid field 'annotations' in recording rule"),
node: &r.Record,
node: &node.Record,
})
}
if r.For != 0 {
nodes = append(nodes, WrappedError{
err: errors.New("invalid field 'for' in recording rule"),
node: &r.Record,
node: &node.Record,
})
}
if r.KeepFiringFor != 0 {
nodes = append(nodes, WrappedError{
err: errors.New("invalid field 'keep_firing_for' in recording rule"),
node: &r.Record,
node: &node.Record,
})
}
if !model.IsValidMetricName(model.LabelValue(r.Record.Value)) {
if !model.IsValidMetricName(model.LabelValue(r.Record)) {
nodes = append(nodes, WrappedError{
err: fmt.Errorf("invalid recording rule name: %s", r.Record.Value),
node: &r.Record,
err: fmt.Errorf("invalid recording rule name: %s", r.Record),
node: &node.Record,
})
}
// While record is a valid UTF-8 it's common mistake to put PromQL expression in the record name.
// Disallow "{}" chars.
if strings.Contains(r.Record.Value, "{") || strings.Contains(r.Record.Value, "}") {
if strings.Contains(r.Record, "{") || strings.Contains(r.Record, "}") {
nodes = append(nodes, WrappedError{
err: fmt.Errorf("braces present in the recording rule name; should it be in expr?: %s", r.Record.Value),
node: &r.Record,
err: fmt.Errorf("braces present in the recording rule name; should it be in expr?: %s", r.Record),
node: &node.Record,
})
}
}
@ -274,8 +285,8 @@ func (r *RuleNode) Validate() (nodes []WrappedError) {
// testTemplateParsing checks if the templates used in labels and annotations
// of the alerting rules are parsed correctly.
func testTemplateParsing(rl *RuleNode) (errs []error) {
if rl.Alert.Value == "" {
func testTemplateParsing(rl *Rule) (errs []error) {
if rl.Alert == "" {
// Not an alerting rule.
return errs
}
@ -292,7 +303,7 @@ func testTemplateParsing(rl *RuleNode) (errs []error) {
tmpl := template.NewTemplateExpander(
context.TODO(),
strings.Join(append(defs, text), ""),
"__alert_"+rl.Alert.Value,
"__alert_"+rl.Alert,
tmplData,
model.Time(timestamp.FromTime(time.Now())),
nil,

27
model/rulefmt/rulefmt_test.go
View file

@ -33,6 +33,33 @@ func TestParseFileSuccess(t *testing.T) {
require.Empty(t, errs, "unexpected errors parsing file")
}
func TestParseFileSuccessWithAliases(t *testing.T) {
exprString := `sum without(instance) (rate(errors_total[5m]))
/
sum without(instance) (rate(requests_total[5m]))
`
rgs, errs := ParseFile("testdata/test_aliases.yaml", false)
require.Empty(t, errs, "unexpected errors parsing file")
for _, rg := range rgs.Groups {
require.Equal(t, "HighAlert", rg.Rules[0].Alert)
require.Equal(t, "critical", rg.Rules[0].Labels["severity"])
require.Equal(t, "stuff's happening with {{ $.labels.service }}", rg.Rules[0].Annotations["description"])
require.Equal(t, "new_metric", rg.Rules[1].Record)
require.Equal(t, "HighAlert", rg.Rules[2].Alert)
require.Equal(t, "critical", rg.Rules[2].Labels["severity"])
require.Equal(t, "stuff's happening with {{ $.labels.service }}", rg.Rules[0].Annotations["description"])
require.Equal(t, "HighAlert2", rg.Rules[3].Alert)
require.Equal(t, "critical", rg.Rules[3].Labels["severity"])
for _, rule := range rg.Rules {
require.Equal(t, exprString, rule.Expr)
}
}
}
func TestParseFileFailure(t *testing.T) {
for _, c := range []struct {
filename string

57
model/rulefmt/testdata/test_aliases.yaml vendored Normal file
View file

@ -0,0 +1,57 @@
groups:
- name: my-group-name
interval: 30s # defaults to global interval
rules:
- &highalert
alert: &alertname HighAlert
expr: &expr |
sum without(instance) (rate(errors_total[5m]))
/
sum without(instance) (rate(requests_total[5m]))
for: 5m
labels:
severity: &severity critical
annotations:
description: &description "stuff's happening with {{ $.labels.service }}"
# Mix recording rules in the same list
- record: &recordname "new_metric"
expr: *expr
labels:
abc: edf
uvw: xyz
- alert: *alertname
expr: *expr
for: 5m
labels:
severity: *severity
annotations:
description: *description
- <<: *highalert
alert: HighAlert2
- name: my-another-name
interval: 30s # defaults to global interval
rules:
- alert: *alertname
expr: *expr
for: 5m
labels:
severity: *severity
annotations:
description: *description
- record: *recordname
expr: *expr
- alert: *alertname
expr: *expr
labels:
severity: *severity
annotations:
description: *description
- <<: *highalert
alert: HighAlert2

8
rules/manager.go
View file

@ -324,16 +324,16 @@ func (m *Manager) LoadGroups(
rules := make([]Rule, 0, len(rg.Rules))
for _, r := range rg.Rules {
expr, err := m.opts.GroupLoader.Parse(r.Expr.Value)
expr, err := m.opts.GroupLoader.Parse(r.Expr)
if err != nil {
return nil, []error{fmt.Errorf("%s: %w", fn, err)}
}
mLabels := FromMaps(rg.Labels, r.Labels)
if r.Alert.Value != "" {
if r.Alert != "" {
rules = append(rules, NewAlertingRule(
r.Alert.Value,
r.Alert,
expr,
time.Duration(r.For),
time.Duration(r.KeepFiringFor),
@ -347,7 +347,7 @@ func (m *Manager) LoadGroups(
continue
}
rules = append(rules, NewRecordingRule(
r.Record.Value,
r.Record,
expr,
mLabels,
))

8
rules/manager_test.go
View file

@ -842,7 +842,7 @@ func TestUpdate(t *testing.T) {
// Change group rules and reload.
for i, g := range rgs.Groups {
for j, r := range g.Rules {
rgs.Groups[i].Rules[j].Expr.SetString(fmt.Sprintf("%s * 0", r.Expr.Value))
rgs.Groups[i].Rules[j].Expr = fmt.Sprintf("%s * 0", r.Expr)
}
}
reloadAndValidate(rgs, t, tmpFile, ruleManager, ogs)
@ -869,9 +869,9 @@ func formatRules(r *rulefmt.RuleGroups) ruleGroupsTest {
rtmp := []rulefmt.Rule{}
for _, r := range g.Rules {
rtmp = append(rtmp, rulefmt.Rule{
Record: r.Record.Value,
Alert: r.Alert.Value,
Expr: r.Expr.Value,
Record: r.Record,
Alert: r.Alert,
Expr: r.Expr,
For: r.For,
Labels: r.Labels,
Annotations: r.Annotations,