From af3a6661ed56ccf7df5edf937e5f144a6e12c78a Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Fri, 11 Dec 2015 17:02:34 +0100 Subject: [PATCH 1/2] Implement new alerting rule syntax --- promql/ast.go | 4 +-- promql/lex.go | 8 ++---- promql/lex_test.go | 10 ++----- promql/parse.go | 61 ++++++++++++----------------------------- promql/parse_test.go | 64 +++++++++++++++++++++++--------------------- promql/printer.go | 5 ++-- 6 files changed, 59 insertions(+), 93 deletions(-) diff --git a/promql/ast.go b/promql/ast.go index b54a67f2dc..09c6fd44d7 100644 --- a/promql/ast.go +++ b/promql/ast.go @@ -59,9 +59,7 @@ type AlertStmt struct { Expr Expr Duration time.Duration Labels model.LabelSet - Summary string - Description string - Runbook string + Annotations model.LabelSet } // EvalStmt holds an expression and information on the range it should diff --git a/promql/lex.go b/promql/lex.go index c2c850f3ea..5e71c96a53 100644 --- a/promql/lex.go +++ b/promql/lex.go @@ -154,9 +154,7 @@ const ( itemIf itemFor itemWith - itemSummary - itemRunbook - itemDescription + itemAnnotations itemKeepCommon itemOffset itemBy @@ -186,9 +184,7 @@ var key = map[string]itemType{ "if": itemIf, "for": itemFor, "with": itemWith, - "summary": itemSummary, - "runbook": itemRunbook, - "description": itemDescription, + "annotations": itemAnnotations, "offset": itemOffset, "by": itemBy, "keeping_extra": itemKeepCommon, diff --git a/promql/lex_test.go b/promql/lex_test.go index 40c22de1a9..5811b96892 100644 --- a/promql/lex_test.go +++ b/promql/lex_test.go @@ -241,14 +241,8 @@ var tests = []struct { input: "with", expected: []item{{itemWith, 0, "with"}}, }, { - input: "description", - expected: []item{{itemDescription, 0, "description"}}, - }, { - input: "summary", - expected: []item{{itemSummary, 0, "summary"}}, - }, { - input: "runbook", - expected: []item{{itemRunbook, 0, "runbook"}}, + input: "annotations", + expected: []item{{itemAnnotations, 0, "annotations"}}, }, { input: "offset", expected: []item{{itemOffset, 0, "offset"}}, diff --git a/promql/parse.go b/promql/parse.go index c15ee6aed8..7e2be267b2 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -357,9 +357,9 @@ func (p *parser) stmt() Statement { // alertStmt parses an alert rule. // -// ALERT name IF expr [FOR duration] [WITH label_set] -// SUMMARY "summary" -// DESCRIPTION "description" +// ALERT name IF expr [FOR duration] +// [WITH label_set] +// [ANNOTATIONS label_set] // func (p *parser) alertStmt() *AlertStmt { const ctx = "alert statement" @@ -389,44 +389,10 @@ func (p *parser) alertStmt() *AlertStmt { lset = p.labelSet() } - var ( - hasSum, hasDesc, hasRunbook bool - sum, desc, runbook string - ) -Loop: - for { - switch p.next().typ { - case itemSummary: - if hasSum { - p.errorf("summary must not be defined twice") - } - hasSum = true - sum = p.unquoteString(p.expect(itemString, ctx).val) - - case itemDescription: - if hasDesc { - p.errorf("description must not be defined twice") - } - hasDesc = true - desc = p.unquoteString(p.expect(itemString, ctx).val) - - case itemRunbook: - if hasRunbook { - p.errorf("runbook must not be defined twice") - } - hasRunbook = true - runbook = p.unquoteString(p.expect(itemString, ctx).val) - - default: - p.backup() - break Loop - } - } - if sum == "" { - p.errorf("alert summary missing") - } - if desc == "" { - p.errorf("alert description missing") + annotations := model.LabelSet{} + if p.peek().typ == itemAnnotations { + p.expect(itemAnnotations, ctx) + annotations = p.labelSet() } return &AlertStmt{ @@ -434,9 +400,7 @@ Loop: Expr: expr, Duration: duration, Labels: lset, - Summary: sum, - Description: desc, - Runbook: runbook, + Annotations: annotations, } } @@ -874,11 +838,20 @@ func (p *parser) labelMatchers(operators ...itemType) metric.LabelMatchers { matchers = append(matchers, m) + if p.peek().typ == itemIdentifier { + p.errorf("missing comma before next identifier %q", p.peek().val) + } + // Terminate list if last matcher. if p.peek().typ != itemComma { break } p.next() + + // Allow comma after each item in a multi-line listing. + if p.peek().typ == itemRightBrace { + break + } } p.expect(itemRightBrace, ctx) diff --git a/promql/parse_test.go b/promql/parse_test.go index ab9b78d03b..43b62c8a66 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -1153,15 +1153,19 @@ var testStatement = []struct { service = "testservice" # ... more fields here ... } - SUMMARY "Global request rate low" - DESCRIPTION "The global request rate is low" + ANNOTATIONS { + summary = "Global request rate low", + description = "The global request rate is low" + } foo = bar{label1="value1"} ALERT BazAlert IF foo > 10 - DESCRIPTION "BazAlert" - RUNBOOK "http://my.url" - SUMMARY "Baz" + ANNOTATIONS { + description = "BazAlert", + runbook = "http://my.url", + summary = "Baz", + } `, expected: Statements{ &RecordStmt{ @@ -1196,10 +1200,12 @@ var testStatement = []struct { }, RHS: &NumberLiteral{10000}, }}, - Labels: model.LabelSet{"service": "testservice"}, - Duration: 5 * time.Minute, - Summary: "Global request rate low", - Description: "The global request rate is low", + Labels: model.LabelSet{"service": "testservice"}, + Duration: 5 * time.Minute, + Annotations: model.LabelSet{ + "summary": "Global request rate low", + "description": "The global request rate is low", + }, }, &RecordStmt{ Name: "foo", @@ -1224,10 +1230,12 @@ var testStatement = []struct { }, RHS: &NumberLiteral{10}, }, - Labels: model.LabelSet{}, - Summary: "Baz", - Description: "BazAlert", - Runbook: "http://my.url", + Labels: model.LabelSet{}, + Annotations: model.LabelSet{ + "summary": "Baz", + "description": "BazAlert", + "runbook": "http://my.url", + }, }, }, }, { @@ -1248,8 +1256,10 @@ var testStatement = []struct { }, }, { input: `ALERT SomeName IF some_metric > 1 - SUMMARY "Global request rate low" - DESCRIPTION "The global request rate is low" + ANNOTATIONS { + summary = "Global request rate low", + description = "The global request rate is low" + } `, expected: Statements{ &AlertStmt{ @@ -1264,9 +1274,11 @@ var testStatement = []struct { }, RHS: &NumberLiteral{1}, }, - Labels: model.LabelSet{}, - Summary: "Global request rate low", - Description: "The global request rate is low", + Labels: model.LabelSet{}, + Annotations: model.LabelSet{ + "summary": "Global request rate low", + "description": "The global request rate is low", + }, }, }, }, { @@ -1276,8 +1288,10 @@ var testStatement = []struct { service = "testservice" # ... more fields here ... } - SUMMARY "Global request rate low" - DESCRIPTION "The global request rate is low" + ANNOTATIONS { + summary = "Global request rate low" + description = "The global request rate is low" + } `, fail: true, }, { @@ -1323,16 +1337,6 @@ var testStatement = []struct { DESCRIPTION "The global request rate is low" `, fail: true, - }, { - input: `ALERT SomeName IF some_metric > 1 WITH {} - SUMMARY "Global request rate low" - `, - fail: true, - }, { - input: `ALERT SomeName IF some_metric > 1 - DESCRIPTION "The global request rate is low" - `, - fail: true, }, // Fuzzing regression tests. { diff --git a/promql/printer.go b/promql/printer.go index 507518eb3a..5b4d1070e9 100644 --- a/promql/printer.go +++ b/promql/printer.go @@ -109,8 +109,9 @@ func (node *AlertStmt) String() string { if len(node.Labels) > 0 { s += fmt.Sprintf("\n\tWITH %s", node.Labels) } - s += fmt.Sprintf("\n\tSUMMARY %q", node.Summary) - s += fmt.Sprintf("\n\tDESCRIPTION %q", node.Description) + if len(node.Annotations) > 0 { + s += fmt.Sprintf("\n\tANNOTATIONS %s", node.Labels) + } return s } From 7c90db22ed2142d2657515d36e00f9101745adca Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Fri, 11 Dec 2015 17:12:34 +0100 Subject: [PATCH 2/2] Use annotation based alerts in rules/ This commit breaks the previously used alert format. --- rules/alerting.go | 40 +++++++++++++--------------------------- rules/manager.go | 15 +++++++-------- rules/manager_test.go | 2 +- 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/rules/alerting.go b/rules/alerting.go index b1fb721400..12a21ed760 100644 --- a/rules/alerting.go +++ b/rules/alerting.go @@ -105,12 +105,8 @@ type AlertingRule struct { holdDuration time.Duration // Extra labels to attach to the resulting alert sample vectors. labels model.LabelSet - // Short alert summary, suitable for email subjects. - summary string - // More detailed alert description. - description string - // A reference to a runbook for the alert. - runbook string + // Non-identifying key/value pairs. + annotations model.LabelSet // Protects the below. mutex sync.Mutex @@ -120,23 +116,13 @@ type AlertingRule struct { } // NewAlertingRule constructs a new AlertingRule. -func NewAlertingRule( - name string, - vector promql.Expr, - holdDuration time.Duration, - labels model.LabelSet, - summary string, - description string, - runbook string, -) *AlertingRule { +func NewAlertingRule(name string, vec promql.Expr, hold time.Duration, lbls, anns model.LabelSet) *AlertingRule { return &AlertingRule{ name: name, - vector: vector, - holdDuration: holdDuration, - labels: labels, - summary: summary, - description: description, - runbook: runbook, + vector: vec, + holdDuration: hold, + labels: lbls, + annotations: anns, activeAlerts: map[model.Fingerprint]*Alert{}, } @@ -217,9 +203,9 @@ func (rule *AlertingRule) String() string { if len(rule.labels) > 0 { s += fmt.Sprintf("\n\tWITH %s", rule.labels) } - s += fmt.Sprintf("\n\tSUMMARY %q", rule.summary) - s += fmt.Sprintf("\n\tDESCRIPTION %q", rule.description) - s += fmt.Sprintf("\n\tRUNBOOK %q", rule.runbook) + if len(rule.annotations) > 0 { + s += fmt.Sprintf("\n\tANNOTATIONS %s", rule.annotations) + } return s } @@ -239,9 +225,9 @@ func (rule *AlertingRule) HTMLSnippet(pathPrefix string) template.HTML { if len(rule.labels) > 0 { s += fmt.Sprintf("\n WITH %s", rule.labels) } - s += fmt.Sprintf("\n SUMMARY %q", rule.summary) - s += fmt.Sprintf("\n DESCRIPTION %q", rule.description) - s += fmt.Sprintf("\n RUNBOOK %q", rule.runbook) + if len(rule.annotations) > 0 { + s += fmt.Sprintf("\n ANNOTATIONS %s", rule.annotations) + } return template.HTML(s) } diff --git a/rules/manager.go b/rules/manager.go index 18dc62fba9..d226f6b69f 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -208,23 +208,22 @@ func (m *Manager) sendAlertNotifications(rule *AlertingRule, timestamp model.Tim // who are not used to Go's templating system. defs := "{{$labels := .Labels}}{{$value := .Value}}" - expand := func(text string) string { - tmpl := template.NewTemplateExpander(defs+text, "__alert_"+rule.Name(), tmplData, timestamp, m.queryEngine, m.externalURL.Path) + expand := func(text model.LabelValue) model.LabelValue { + tmpl := template.NewTemplateExpander(defs+string(text), "__alert_"+rule.Name(), tmplData, timestamp, m.queryEngine, m.externalURL.Path) result, err := tmpl.Expand() if err != nil { result = err.Error() log.Warnf("Error expanding alert template %v with data '%v': %v", rule.Name(), tmplData, err) } - return result + return model.LabelValue(result) } labels := aa.Labels.Clone() labels[model.AlertNameLabel] = model.LabelValue(rule.Name()) - annotations := model.LabelSet{ - "summary": model.LabelValue(expand(rule.summary)), - "description": model.LabelValue(expand(rule.description)), - "runbook": model.LabelValue(expand(rule.runbook)), + annotations := rule.annotations.Clone() + for an, av := range rule.annotations { + annotations[an] = expand(av) } alerts = append(alerts, &model.Alert{ @@ -359,7 +358,7 @@ func (m *Manager) loadRuleFiles(filenames ...string) error { for _, stmt := range stmts { switch r := stmt.(type) { case *promql.AlertStmt: - rule := NewAlertingRule(r.Name, r.Expr, r.Duration, r.Labels, r.Summary, r.Description, r.Runbook) + rule := NewAlertingRule(r.Name, r.Expr, r.Duration, r.Labels, r.Annotations) m.rules = append(m.rules, rule) case *promql.RecordStmt: rule := NewRecordingRule(r.Name, r.Expr, r.Labels) diff --git a/rules/manager_test.go b/rules/manager_test.go index 45d3ae3c4e..6219868776 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -56,7 +56,7 @@ func TestAlertingRule(t *testing.T) { expr, time.Minute, model.LabelSet{"severity": "critical"}, - "summary", "description", "runbook", + model.LabelSet{}, ) var tests = []struct {