From c843a0cd29e8b4c42b5201659d25dc2a76185410 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Wed, 7 Jun 2017 16:58:15 +0200 Subject: [PATCH 01/14] pkg/rulefmt: Add rule group parsing --- pkg/rulefmt/rulefmt.go | 98 ++++++++++++++++++++++++++++++++++ pkg/rulefmt/rulefmt_test.go | 12 +++++ pkg/rulefmt/testdata/test.yaml | 59 ++++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 pkg/rulefmt/rulefmt.go create mode 100644 pkg/rulefmt/rulefmt_test.go create mode 100644 pkg/rulefmt/testdata/test.yaml diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go new file mode 100644 index 0000000000..0d1796d9e5 --- /dev/null +++ b/pkg/rulefmt/rulefmt.go @@ -0,0 +1,98 @@ +package rulefmt + +import ( + "io/ioutil" + + "github.com/ghodss/yaml" + "github.com/pkg/errors" + "github.com/prometheus/prometheus/promql" +) + +// Error represents semantical errors on parsing rule groups. +type Error struct { + Group string + Rule int + Err error +} + +func (err *Error) Error() string { + return errors.Wrapf(err, "group %q, rule %d", err.Group, err.Rule).Error() +} + +// RuleGroups is a set of rule groups that are typically exposed in a file. +type RuleGroups struct { + Version int `json:"version"` + Groups []RuleGroup `json:"groups"` +} + +// Validate validates all rules in the rule groups. +func (g *RuleGroups) Validate() (errs []error) { + if g.Version != 1 { + errs = append(errs, errors.Errorf("invalid rule group version %d", g.Version)) + } + for _, g := range g.Groups { + for i, r := range g.Rules { + for _, err := range r.Validate() { + errs = append(errs, &Error{ + Group: g.Name, + Rule: i, + Err: err, + }) + } + } + } + return errs +} + +// RuleGroup is a list of sequentially evaluated recording and alerting rules. +type RuleGroup struct { + Name string `json:"name"` + Rules []Rule `json:"rules"` +} + +// Rule describes an alerting or recording rule. +type Rule struct { + Record string `json:"record"` + Alert string `json:"alert"` + Expr string `json:"expr"` + For string `json:"for"` + Labels map[string]string `json:"labels"` + Annotations map[string]string `json:"annotations"` +} + +// Validate the rule and return a list of encountered errors. +func (r *Rule) Validate() (errs []error) { + if r.Record != "" && r.Alert != "" { + errs = append(errs, errors.Errorf("only one of 'record' and 'alert' must be set")) + } + if r.Record == "" && r.Alert == "" { + errs = append(errs, errors.Errorf("one of 'record' or 'alert' must be set")) + } + if r.Expr == "" { + errs = append(errs, errors.Errorf("field 'expr' must be set in rule")) + } else if _, err := promql.ParseExpr(r.Expr); err != nil { + errs = append(errs, errors.Errorf("could not parse expression: %s", err)) + } + if r.Record != "" { + if len(r.Annotations) > 0 { + errs = append(errs, errors.Errorf("invalid field 'annotations' in recording rule")) + } + if r.For != "" { + errs = append(errs, errors.Errorf("invalid field 'for' in recording rule")) + } + } + return errs +} + +// ParseFile parses the rule file and validates it. +func ParseFile(file string) (*RuleGroups, []error) { + b, err := ioutil.ReadFile(file) + if err != nil { + return nil, []error{err} + } + var groups RuleGroups + if err := yaml.Unmarshal(b, &groups); err != nil { + return nil, []error{err} + } + return &groups, groups.Validate() +} diff --git a/pkg/rulefmt/rulefmt_test.go b/pkg/rulefmt/rulefmt_test.go new file mode 100644 index 0000000000..494a8ee504 --- /dev/null +++ b/pkg/rulefmt/rulefmt_test.go @@ -0,0 +1,12 @@ +package rulefmt + +import "testing" + +func TestParseFileSuccess(t *testing.T) { + if _, errs := ParseFile("testdata/test.yaml"); len(errs) > 0 { + t.Errorf("unexpected errors parsing file") + for _, err := range errs { + t.Error(err) + } + } +} diff --git a/pkg/rulefmt/testdata/test.yaml b/pkg/rulefmt/testdata/test.yaml new file mode 100644 index 0000000000..321b0268fb --- /dev/null +++ b/pkg/rulefmt/testdata/test.yaml @@ -0,0 +1,59 @@ +version: 1 +groups: +- name: my-group-name + interval: 30s # defaults to global interval + rules: + - alert: HighErrors + expr: | + sum without(instance) (rate(errors_total[5m])) + / sum without(instance) (rate(requests_total[5m])) + for: 5m + labels: + severity: critical + annotations: + description: "stuff's happening with {{ $.labels.service }}" + + # Mix recording rules in the same list + - record: "new_metric" + expr: | + sum without(instance) (rate(errors_total[5m])) + / sum without(instance) (rate(requests_total[5m])) + labels: + abc: edf + uvw: xyz + + - alert: HighErrors + expr: | + sum without(instance) (rate(errors_total[5m])) + / sum without(instance) (rate(requests_total[5m])) + for: 5m + labels: + severity: critical + annotations: + description: "stuff's happening with {{ $.labels.service }}" + +- name: my-group-name + interval: 30s # defaults to global interval + rules: + - alert: HighErrors + expr: | + sum without(instance) (rate(errors_total[5m])) + / sum without(instance) (rate(requests_total[5m])) + for: 5m + labels: + severity: critical + + - record: "new_metric" + expr: | + sum without(instance) (rate(errors_total[5m])) + / sum without(instance) (rate(requests_total[5m])) + + - alert: HighErrors + expr: | + sum without(instance) (rate(errors_total[5m])) + / sum without(instance) (rate(requests_total[5m])) + for: 5m + labels: + severity: critical + annotations: + description: "stuff's happening with {{ $.labels.service }}" From e8f55669ea92c8e173127c8427914d70e6960835 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Mon, 12 Jun 2017 18:14:39 +0530 Subject: [PATCH 02/14] Move rules to new format Signed-off-by: Goutham Veeramachaneni --- pkg/rulefmt/rulefmt.go | 5 +-- rules/manager.go | 80 ++++++++++++++++++++++++------------------ 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index 0d1796d9e5..ee0e6df6cb 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -46,8 +46,9 @@ func (g *RuleGroups) Validate() (errs []error) { // RuleGroup is a list of sequentially evaluated recording and alerting rules. type RuleGroup struct { - Name string `json:"name"` - Rules []Rule `json:"rules"` + Name string `json:"name"` + Interval string `json:"interval"` + Rules []Rule `json:"rules"` } // Rule describes an alerting or recording rule. diff --git a/rules/manager.go b/rules/manager.go index 0b4661668f..74322877cc 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -15,7 +15,6 @@ package rules import ( "fmt" - "io/ioutil" "math" "net/url" "path/filepath" @@ -32,6 +31,7 @@ import ( "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/notifier" "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/pkg/rulefmt" "github.com/prometheus/prometheus/pkg/timestamp" "github.com/prometheus/prometheus/pkg/value" "github.com/prometheus/prometheus/promql" @@ -270,18 +270,10 @@ func typeForRule(r Rule) ruleType { // In the future a single group will be evaluated sequentially to properly handle // rule dependency. func (g *Group) Eval(ts time.Time) { - var ( - wg sync.WaitGroup - ) - for i, rule := range g.rules { rtyp := string(typeForRule(rule)) - wg.Add(1) - // BUG(julius): Look at fixing thundering herd. - go func(i int, rule Rule) { - defer wg.Done() - + func(i int, rule Rule) { defer func(t time.Time) { evalDuration.WithLabelValues(rtyp).Observe(time.Since(t).Seconds()) }(time.Now()) @@ -358,7 +350,6 @@ func (g *Group) Eval(ts time.Time) { } }(i, rule) } - wg.Wait() } // sendAlerts sends alert notifications for the given rule. @@ -506,38 +497,59 @@ func (m *Manager) ApplyConfig(conf *config.Config) error { // As there's currently no group syntax a single group named "default" containing // all rules will be returned. func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[string]*Group, error) { - rules := []Rule{} + groups := make(map[string]*Group) + for _, fn := range filenames { - content, err := ioutil.ReadFile(fn) + rgs, err := rulefmt.ParseFile(fn) if err != nil { - return nil, err - } - stmts, err := promql.ParseStmts(string(content)) - if err != nil { - return nil, fmt.Errorf("error parsing %s: %s", fn, err) + // TODO(gouthamve): Use multi-error? + return nil, err[0] } - for _, stmt := range stmts { - var rule Rule + for _, rg := range rgs.Groups { + itv := interval + if rg.Interval != "" { + dur, err := model.ParseDuration(rg.Interval) + if err != nil { + return nil, err + } - switch r := stmt.(type) { - case *promql.AlertStmt: - rule = NewAlertingRule(r.Name, r.Expr, r.Duration, r.Labels, r.Annotations) - - case *promql.RecordStmt: - rule = NewRecordingRule(r.Name, r.Expr, r.Labels) - - default: - panic("retrieval.Manager.LoadRuleFiles: unknown statement type") + itv = time.Duration(dur) } - rules = append(rules, rule) + + rules := make([]Rule, len(rg.Rules), 0) + for _, r := range rg.Rules { + expr, err := promql.ParseExpr(r.Expr) + if err != nil { + return nil, err + } + + if r.Alert != "" { + fordur, err := model.ParseDuration(r.For) + if err != nil { + return nil, err + } + + rules = append(rules, NewAlertingRule( + r.Alert, + expr, + time.Duration(fordur), + labels.FromMap(r.Labels), + labels.FromMap(r.Annotations), + )) + continue + } + rules = append(rules, NewRecordingRule( + r.Record, + expr, + labels.FromMap(r.Labels), + )) + } + + groups[rg.Name] = NewGroup(rg.Name, itv, rules, m.opts) } } - // Currently there is no group syntax implemented. Thus all rules - // are read into a single default group. - g := NewGroup("default", interval, rules, m.opts) - groups := map[string]*Group{g.name: g} return groups, nil } From cea1e99f78c739fe2261406313bfc673a5b77ddb Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 14 Jun 2017 11:13:00 +0530 Subject: [PATCH 03/14] Add update-rules command to promtool Signed-off-by: Goutham Veeramachaneni --- cmd/promtool/main.go | 88 ++++++++++++++++++++++++++++++++++++++++++ pkg/rulefmt/rulefmt.go | 12 +++--- rules/manager.go | 3 +- 3 files changed, 96 insertions(+), 7 deletions(-) diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index 431d381357..cf2debf8fa 100644 --- a/cmd/promtool/main.go +++ b/cmd/promtool/main.go @@ -20,8 +20,12 @@ import ( "path/filepath" "strings" + yaml "github.com/ghodss/yaml" + + "github.com/prometheus/common/model" "github.com/prometheus/common/version" "github.com/prometheus/prometheus/config" + "github.com/prometheus/prometheus/pkg/rulefmt" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/util/cli" "github.com/prometheus/prometheus/util/promlint" @@ -183,6 +187,85 @@ func checkRules(t cli.Term, filename string) (int, error) { return len(rules), nil } +// UpdateRulesCmd updates the rule files. +func UpdateRulesCmd(t cli.Term, args ...string) int { + if len(args) == 0 { + t.Infof("usage: promtool update-rules ") + return 2 + } + failed := false + + for _, arg := range args { + if err := updateRules(t, arg); err != nil { + t.Errorf(" FAILED: %s", err) + failed = true + } + } + + if failed { + return 1 + } + return 0 +} + +func updateRules(t cli.Term, filename string) error { + t.Infof("Updating %s", filename) + + if stat, err := os.Stat(filename); err != nil { + return fmt.Errorf("cannot get file info") + } else if stat.IsDir() { + return fmt.Errorf("is a directory") + } + + content, err := ioutil.ReadFile(filename) + if err != nil { + return err + } + + rules, err := promql.ParseStmts(string(content)) + if err != nil { + return err + } + + yamlRG := &rulefmt.RuleGroups{ + Version: 1, + Groups: []rulefmt.RuleGroup{{ + Name: filename, + }}, + } + + yamlRules := make([]rulefmt.Rule, 0, len(rules)) + + for _, rule := range rules { + switch r := rule.(type) { + case *promql.AlertStmt: + yamlRules = append(yamlRules, rulefmt.Rule{ + Alert: r.Name, + Expr: r.Expr.String(), + For: model.Duration(r.Duration).String(), + Labels: r.Labels.Map(), + Annotations: r.Annotations.Map(), + }) + case *promql.RecordStmt: + yamlRules = append(yamlRules, rulefmt.Rule{ + Record: r.Name, + Expr: r.Expr.String(), + Labels: r.Labels.Map(), + }) + default: + panic("unknown statement type") + } + } + + yamlRG.Groups[0].Rules = yamlRules + y, err := yaml.Marshal(yamlRG) + if err != nil { + return err + } + + return ioutil.WriteFile(filename+".yaml", y, 0777) +} + var checkMetricsUsage = strings.TrimSpace(` usage: promtool check-metrics @@ -243,6 +326,11 @@ func main() { Run: CheckMetricsCmd, }) + app.Register("update-rules", &cli.Command{ + Desc: "update the rules to the new YAML format", + Run: UpdateRulesCmd, + }) + app.Register("version", &cli.Command{ Desc: "print the version of this binary", Run: VersionCmd, diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index ee0e6df6cb..3453ce5d6d 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -47,18 +47,18 @@ func (g *RuleGroups) Validate() (errs []error) { // RuleGroup is a list of sequentially evaluated recording and alerting rules. type RuleGroup struct { Name string `json:"name"` - Interval string `json:"interval"` + Interval string `json:"interval,omitempty"` Rules []Rule `json:"rules"` } // Rule describes an alerting or recording rule. type Rule struct { - Record string `json:"record"` - Alert string `json:"alert"` + Record string `json:"record,omitempty"` + Alert string `json:"alert,omitempty"` Expr string `json:"expr"` - For string `json:"for"` - Labels map[string]string `json:"labels"` - Annotations map[string]string `json:"annotations"` + For string `json:"for,omitempty"` + Labels map[string]string `json:"labels,omitempty"` + Annotations map[string]string `json:"annotations,omitempty"` } // Validate the rule and return a list of encountered errors. diff --git a/rules/manager.go b/rules/manager.go index 74322877cc..fcfcba606b 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -390,6 +390,7 @@ type Manager struct { block chan struct{} } +// Appendable returns an Appender. type Appendable interface { Appender() (storage.Appender, error) } @@ -517,7 +518,7 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s itv = time.Duration(dur) } - rules := make([]Rule, len(rg.Rules), 0) + rules := make([]Rule, 0, len(rg.Rules)) for _, r := range rg.Rules { expr, err := promql.ParseExpr(r.Expr) if err != nil { From a48a01836810055029145881b7777fddd00c233a Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 14 Jun 2017 12:19:21 +0530 Subject: [PATCH 04/14] Make sure groups are unique in a single file Signed-off-by: Goutham Veeramachaneni --- pkg/rulefmt/rulefmt.go | 13 ++++++++++++- rules/manager.go | 11 ++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index 3453ce5d6d..777da84c27 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -16,7 +16,7 @@ type Error struct { } func (err *Error) Error() string { - return errors.Wrapf(err, "group %q, rule %d", err.Group, err.Rule).Error() + return errors.Wrapf(err.Err, "group %q, rule %d", err.Group, err.Rule).Error() } // RuleGroups is a set of rule groups that are typically exposed in a file. @@ -30,7 +30,18 @@ func (g *RuleGroups) Validate() (errs []error) { if g.Version != 1 { errs = append(errs, errors.Errorf("invalid rule group version %d", g.Version)) } + set := map[string]struct{}{} + for _, g := range g.Groups { + if _, ok := set[g.Name]; ok { + errs = append( + errs, + errors.Errorf("groupname: \"%s\" is repeated in the same file", g.Name), + ) + } + + set[g.Name] = struct{}{} + for i, r := range g.Rules { for _, err := range r.Validate() { errs = append(errs, &Error{ diff --git a/rules/manager.go b/rules/manager.go index fcfcba606b..9720c3829c 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -26,6 +26,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/log" "github.com/prometheus/common/model" + "github.com/prometheus/tsdb" "golang.org/x/net/context" "github.com/prometheus/prometheus/config" @@ -501,10 +502,9 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s groups := make(map[string]*Group) for _, fn := range filenames { - rgs, err := rulefmt.ParseFile(fn) - if err != nil { - // TODO(gouthamve): Use multi-error? - return nil, err[0] + rgs, errs := rulefmt.ParseFile(fn) + if errs != nil { + return nil, tsdb.MultiError(errs) } for _, rg := range rgs.Groups { @@ -547,7 +547,8 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s )) } - groups[rg.Name] = NewGroup(rg.Name, itv, rules, m.opts) + // Groups need not be unique across filenames. + groups[rg.Name+";"+fn] = NewGroup(rg.Name, itv, rules, m.opts) } } From 2d1e92513b4b66df2619b4cc02add379cbe29f06 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 14 Jun 2017 13:21:32 +0530 Subject: [PATCH 05/14] Add tests Signed-off-by: Goutham Veeramachaneni --- pkg/rulefmt/rulefmt.go | 5 ++ pkg/rulefmt/rulefmt_test.go | 50 ++++++++++++++++++- pkg/rulefmt/testdata/bad_expr.bad.yaml | 6 +++ pkg/rulefmt/testdata/duplicate_grp.bad.yaml | 4 ++ pkg/rulefmt/testdata/no_rec_alert.bad.yaml | 5 ++ pkg/rulefmt/testdata/noexpr.bad.yaml | 5 ++ pkg/rulefmt/testdata/noversion.bad.yaml | 2 + .../testdata/record_and_alert.bad.yaml | 7 +++ pkg/rulefmt/testdata/test.yaml | 2 +- 9 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 pkg/rulefmt/testdata/bad_expr.bad.yaml create mode 100644 pkg/rulefmt/testdata/duplicate_grp.bad.yaml create mode 100644 pkg/rulefmt/testdata/no_rec_alert.bad.yaml create mode 100644 pkg/rulefmt/testdata/noexpr.bad.yaml create mode 100644 pkg/rulefmt/testdata/noversion.bad.yaml create mode 100644 pkg/rulefmt/testdata/record_and_alert.bad.yaml diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index 777da84c27..97e7bd8e90 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -33,6 +33,10 @@ func (g *RuleGroups) Validate() (errs []error) { set := map[string]struct{}{} for _, g := range g.Groups { + if g.Name == "" { + errs = append(errs, errors.Errorf("Groupname should not be empty")) + } + if _, ok := set[g.Name]; ok { errs = append( errs, @@ -80,6 +84,7 @@ func (r *Rule) Validate() (errs []error) { if r.Record == "" && r.Alert == "" { errs = append(errs, errors.Errorf("one of 'record' or 'alert' must be set")) } + if r.Expr == "" { errs = append(errs, errors.Errorf("field 'expr' must be set in rule")) } else if _, err := promql.ParseExpr(r.Expr); err != nil { diff --git a/pkg/rulefmt/rulefmt_test.go b/pkg/rulefmt/rulefmt_test.go index 494a8ee504..6939565998 100644 --- a/pkg/rulefmt/rulefmt_test.go +++ b/pkg/rulefmt/rulefmt_test.go @@ -1,6 +1,10 @@ package rulefmt -import "testing" +import ( + "path/filepath" + "strings" + "testing" +) func TestParseFileSuccess(t *testing.T) { if _, errs := ParseFile("testdata/test.yaml"); len(errs) > 0 { @@ -10,3 +14,47 @@ func TestParseFileSuccess(t *testing.T) { } } } + +func TestParseFileFailure(t *testing.T) { + table := []struct { + filename string + errMsg string + }{ + { + filename: "duplicate_grp.bad.yaml", + errMsg: "groupname: \"yolo\" is repeated in the same file", + }, + { + filename: "noversion.bad.yaml", + errMsg: "invalid rule group version 0", + }, + { + filename: "bad_expr.bad.yaml", + errMsg: "parse error", + }, + { + filename: "record_and_alert.bad.yaml", + errMsg: "only one of 'record' and 'alert' must be set", + }, + { + filename: "no_rec_alert.bad.yaml", + errMsg: "one of 'record' or 'alert' must be set", + }, + { + filename: "noexpr.bad.yaml", + errMsg: "field 'expr' must be set in rule", + }, + } + + for _, c := range table { + _, errs := ParseFile(filepath.Join("testdata", c.filename)) + if errs == nil { + t.Errorf("Expected error parsing %s but got none", c.filename) + continue + } + if !strings.Contains(errs[0].Error(), c.errMsg) { + t.Errorf("Expected error for %s to contain %q but got: %s", c.filename, c.errMsg, errs) + } + } + +} diff --git a/pkg/rulefmt/testdata/bad_expr.bad.yaml b/pkg/rulefmt/testdata/bad_expr.bad.yaml new file mode 100644 index 0000000000..df9a8cac1f --- /dev/null +++ b/pkg/rulefmt/testdata/bad_expr.bad.yaml @@ -0,0 +1,6 @@ +Version: 1 +Groups: +- name: yolo + rules: + - record: yolo + expr: rate(hi) diff --git a/pkg/rulefmt/testdata/duplicate_grp.bad.yaml b/pkg/rulefmt/testdata/duplicate_grp.bad.yaml new file mode 100644 index 0000000000..dc22391217 --- /dev/null +++ b/pkg/rulefmt/testdata/duplicate_grp.bad.yaml @@ -0,0 +1,4 @@ +Version: 1 +Groups: +- name: yolo +- name: yolo diff --git a/pkg/rulefmt/testdata/no_rec_alert.bad.yaml b/pkg/rulefmt/testdata/no_rec_alert.bad.yaml new file mode 100644 index 0000000000..5806275aa9 --- /dev/null +++ b/pkg/rulefmt/testdata/no_rec_alert.bad.yaml @@ -0,0 +1,5 @@ +Version: 1 +Groups: + - name: yolo + rules: + - expr: 1 diff --git a/pkg/rulefmt/testdata/noexpr.bad.yaml b/pkg/rulefmt/testdata/noexpr.bad.yaml new file mode 100644 index 0000000000..99c5027965 --- /dev/null +++ b/pkg/rulefmt/testdata/noexpr.bad.yaml @@ -0,0 +1,5 @@ +Version: 1 +Groups: + - name: yolo + rules: + - record: ylo diff --git a/pkg/rulefmt/testdata/noversion.bad.yaml b/pkg/rulefmt/testdata/noversion.bad.yaml new file mode 100644 index 0000000000..8b4ed9ce7c --- /dev/null +++ b/pkg/rulefmt/testdata/noversion.bad.yaml @@ -0,0 +1,2 @@ +Groups: +- name: yolo diff --git a/pkg/rulefmt/testdata/record_and_alert.bad.yaml b/pkg/rulefmt/testdata/record_and_alert.bad.yaml new file mode 100644 index 0000000000..98063960c0 --- /dev/null +++ b/pkg/rulefmt/testdata/record_and_alert.bad.yaml @@ -0,0 +1,7 @@ +Version: 1 +Groups: +- name: yolo + rules: + - record: Hi + alert: Hello + expr: 1 diff --git a/pkg/rulefmt/testdata/test.yaml b/pkg/rulefmt/testdata/test.yaml index 321b0268fb..70bb267eee 100644 --- a/pkg/rulefmt/testdata/test.yaml +++ b/pkg/rulefmt/testdata/test.yaml @@ -32,7 +32,7 @@ groups: annotations: description: "stuff's happening with {{ $.labels.service }}" -- name: my-group-name +- name: my-another-name interval: 30s # defaults to global interval rules: - alert: HighErrors From 1c0874372169d75c6765c89097863750ff490f21 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 14 Jun 2017 13:32:26 +0530 Subject: [PATCH 06/14] Update check-rules to new format. Signed-off-by: Goutham Veeramachaneni --- cmd/promtool/main.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index cf2debf8fa..75f6061e9d 100644 --- a/cmd/promtool/main.go +++ b/cmd/promtool/main.go @@ -29,6 +29,7 @@ import ( "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/util/cli" "github.com/prometheus/prometheus/util/promlint" + "github.com/prometheus/tsdb" ) // CheckConfigCmd validates configuration files. @@ -175,16 +176,17 @@ func checkRules(t cli.Term, filename string) (int, error) { return 0, fmt.Errorf("is a directory") } - content, err := ioutil.ReadFile(filename) - if err != nil { - return 0, err + rgs, errs := rulefmt.ParseFile(filename) + if errs != nil { + return 0, tsdb.MultiError(errs) } - rules, err := promql.ParseStmts(string(content)) - if err != nil { - return 0, err + numRules := 0 + for _, rg := range rgs.Groups { + numRules += len(rg.Rules) } - return len(rules), nil + + return numRules, nil } // UpdateRulesCmd updates the rule files. From c884881f7d4b6254669da09918444208f7bac7d4 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 14 Jun 2017 13:34:13 +0530 Subject: [PATCH 07/14] Add License Header Signed-off-by: Goutham Veeramachaneni --- pkg/rulefmt/rulefmt.go | 13 +++++++++++++ pkg/rulefmt/rulefmt_test.go | 13 +++++++++++++ 2 files changed, 26 insertions(+) diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index 97e7bd8e90..cd922fd4f0 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -1,3 +1,16 @@ +// Copyright 2017 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package rulefmt import ( diff --git a/pkg/rulefmt/rulefmt_test.go b/pkg/rulefmt/rulefmt_test.go index 6939565998..fd914e839b 100644 --- a/pkg/rulefmt/rulefmt_test.go +++ b/pkg/rulefmt/rulefmt_test.go @@ -1,3 +1,16 @@ +// Copyright 2017 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package rulefmt import ( From e893c89333d250e4061b857d1876d2f2d6caee71 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 14 Jun 2017 15:07:54 +0530 Subject: [PATCH 08/14] Validate labels and annotations Signed-off-by: Goutham Veeramachaneni --- pkg/rulefmt/rulefmt.go | 18 ++++++++++++++++++ pkg/rulefmt/rulefmt_test.go | 8 ++++++++ pkg/rulefmt/testdata/bad_annotation.bad.yaml | 8 ++++++++ pkg/rulefmt/testdata/bad_lname.bad.yaml | 8 ++++++++ pkg/rulefmt/testdata/test.yaml | 18 ++++++++++++------ rules/manager.go | 4 +--- 6 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 pkg/rulefmt/testdata/bad_annotation.bad.yaml create mode 100644 pkg/rulefmt/testdata/bad_lname.bad.yaml diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index cd922fd4f0..8fef3a1c8d 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -18,6 +18,7 @@ import ( "github.com/ghodss/yaml" "github.com/pkg/errors" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/promql" ) @@ -111,6 +112,23 @@ func (r *Rule) Validate() (errs []error) { errs = append(errs, errors.Errorf("invalid field 'for' in recording rule")) } } + + for k, v := range r.Labels { + if !model.LabelName(k).IsValid() { + errs = append(errs, errors.Errorf("invalid label name: %s", k)) + } + + if !model.LabelValue(v).IsValid() { + errs = append(errs, errors.Errorf("invalid label value: %s", v)) + } + } + + for k := range r.Annotations { + if !model.LabelName(k).IsValid() { + errs = append(errs, errors.Errorf("invalid annotation name: %s", k)) + } + } + return errs } diff --git a/pkg/rulefmt/rulefmt_test.go b/pkg/rulefmt/rulefmt_test.go index fd914e839b..95d088f18e 100644 --- a/pkg/rulefmt/rulefmt_test.go +++ b/pkg/rulefmt/rulefmt_test.go @@ -57,6 +57,14 @@ func TestParseFileFailure(t *testing.T) { filename: "noexpr.bad.yaml", errMsg: "field 'expr' must be set in rule", }, + { + filename: "bad_lname.bad.yaml", + errMsg: "invalid label name", + }, + { + filename: "bad_annotation.bad.yaml", + errMsg: "invalid annotation name", + }, } for _, c := range table { diff --git a/pkg/rulefmt/testdata/bad_annotation.bad.yaml b/pkg/rulefmt/testdata/bad_annotation.bad.yaml new file mode 100644 index 0000000000..523da2322b --- /dev/null +++ b/pkg/rulefmt/testdata/bad_annotation.bad.yaml @@ -0,0 +1,8 @@ +Version: 1 +Groups: + - name: yolo + rules: + - alert: hola + expr: 1 + annotations: + ins-tance: localhost diff --git a/pkg/rulefmt/testdata/bad_lname.bad.yaml b/pkg/rulefmt/testdata/bad_lname.bad.yaml new file mode 100644 index 0000000000..3ece3cf44e --- /dev/null +++ b/pkg/rulefmt/testdata/bad_lname.bad.yaml @@ -0,0 +1,8 @@ +Version: 1 +Groups: + - name: yolo + rules: + - record: hola + expr: 1 + labels: + ins-tance: localhost diff --git a/pkg/rulefmt/testdata/test.yaml b/pkg/rulefmt/testdata/test.yaml index 70bb267eee..d7cf526b78 100644 --- a/pkg/rulefmt/testdata/test.yaml +++ b/pkg/rulefmt/testdata/test.yaml @@ -6,7 +6,8 @@ groups: - alert: HighErrors expr: | sum without(instance) (rate(errors_total[5m])) - / sum without(instance) (rate(requests_total[5m])) + / + sum without(instance) (rate(requests_total[5m])) for: 5m labels: severity: critical @@ -17,7 +18,8 @@ groups: - record: "new_metric" expr: | sum without(instance) (rate(errors_total[5m])) - / sum without(instance) (rate(requests_total[5m])) + / + sum without(instance) (rate(requests_total[5m])) labels: abc: edf uvw: xyz @@ -25,7 +27,8 @@ groups: - alert: HighErrors expr: | sum without(instance) (rate(errors_total[5m])) - / sum without(instance) (rate(requests_total[5m])) + / + sum without(instance) (rate(requests_total[5m])) for: 5m labels: severity: critical @@ -38,7 +41,8 @@ groups: - alert: HighErrors expr: | sum without(instance) (rate(errors_total[5m])) - / sum without(instance) (rate(requests_total[5m])) + / + sum without(instance) (rate(requests_total[5m])) for: 5m labels: severity: critical @@ -46,12 +50,14 @@ groups: - record: "new_metric" expr: | sum without(instance) (rate(errors_total[5m])) - / sum without(instance) (rate(requests_total[5m])) + / + sum without(instance) (rate(requests_total[5m])) - alert: HighErrors expr: | sum without(instance) (rate(errors_total[5m])) - / sum without(instance) (rate(requests_total[5m])) + / + sum without(instance) (rate(requests_total[5m])) for: 5m labels: severity: critical diff --git a/rules/manager.go b/rules/manager.go index 9720c3829c..ec8812c801 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -267,9 +267,7 @@ func typeForRule(r Rule) ruleType { panic(fmt.Errorf("unknown rule type: %T", r)) } -// Eval runs a single evaluation cycle in which all rules are evaluated in parallel. -// In the future a single group will be evaluated sequentially to properly handle -// rule dependency. +// Eval runs a single evaluation cycle in which all rules are evaluated sequentially. func (g *Group) Eval(ts time.Time) { for i, rule := range g.rules { rtyp := string(typeForRule(rule)) From 8cca666cf2132919c213385a31ee057b527cae60 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 14 Jun 2017 15:18:39 +0530 Subject: [PATCH 09/14] Add file name to group. Signed-off-by: Goutham Veeramachaneni --- rules/manager.go | 12 ++++++++---- rules/manager_test.go | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/rules/manager.go b/rules/manager.go index ec8812c801..229d97f9e7 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -128,6 +128,7 @@ type Rule interface { // Group is a set of rules that have a logical relation. type Group struct { name string + file string interval time.Duration rules []Rule seriesInPreviousEval []map[string]labels.Labels // One per Rule. @@ -138,7 +139,7 @@ type Group struct { } // NewGroup makes a new Group with the given name, options, and rules. -func NewGroup(name string, interval time.Duration, rules []Rule, opts *ManagerOptions) *Group { +func NewGroup(name, file string, interval time.Duration, rules []Rule, opts *ManagerOptions) *Group { return &Group{ name: name, interval: interval, @@ -201,7 +202,10 @@ func (g *Group) stop() { } func (g *Group) fingerprint() model.Fingerprint { - l := model.LabelSet{"name": model.LabelValue(g.name)} + l := model.LabelSet{ + "name": model.LabelValue(g.name), + "filename": model.LabelValue(g.file), + } return l.Fingerprint() } @@ -545,8 +549,8 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s )) } - // Groups need not be unique across filenames. - groups[rg.Name+";"+fn] = NewGroup(rg.Name, itv, rules, m.opts) + // Group names need not be unique across filenames. + groups[rg.Name+";"+fn] = NewGroup(rg.Name, fn, itv, rules, m.opts) } } diff --git a/rules/manager_test.go b/rules/manager_test.go index 716095b03d..ff85ae4629 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -172,7 +172,7 @@ func TestStaleness(t *testing.T) { t.Fatal(err) } rule := NewRecordingRule("a_plus_one", expr, labels.Labels{}) - group := NewGroup("default", time.Second, []Rule{rule}, opts) + group := NewGroup("default", "", time.Second, []Rule{rule}, opts) // A time series that has two samples and then goes stale. app, _ := storage.Appender() From 5ff283a7b7f1aaa3d564b46f3aa02d3746ab0f45 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 14 Jun 2017 16:09:14 +0530 Subject: [PATCH 10/14] Reflect the grouping in the UI Signed-off-by: Goutham Veeramachaneni --- rules/manager.go | 28 +++++++++++++ web/ui/bindata.go | 80 ++++++++++++++++++------------------- web/ui/templates/rules.html | 3 +- 3 files changed, 70 insertions(+), 41 deletions(-) diff --git a/rules/manager.go b/rules/manager.go index 229d97f9e7..af532829ca 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -18,6 +18,7 @@ import ( "math" "net/url" "path/filepath" + "sort" "sync" "time" @@ -142,6 +143,7 @@ type Group struct { func NewGroup(name, file string, interval time.Duration, rules []Rule, opts *ManagerOptions) *Group { return &Group{ name: name, + file: file, interval: interval, rules: rules, opts: opts, @@ -151,6 +153,15 @@ func NewGroup(name, file string, interval time.Duration, rules []Rule, opts *Man } } +// Name returns the group name. +func (g *Group) Name() string { return g.name } + +// File returns the group's file. +func (g *Group) File() string { return g.file } + +// Rules returns the group's rules. +func (g *Group) Rules() []Rule { return g.rules } + func (g *Group) run() { defer close(g.terminated) @@ -557,6 +568,23 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s return groups, nil } +// RuleGroups returns the list of manager's rule groups. +func (m *Manager) RuleGroups() []*Group { + m.mtx.RLock() + defer m.mtx.RUnlock() + + rgs := make([]*Group, 0, len(m.groups)) + for _, g := range m.groups { + rgs = append(rgs, g) + } + + sort.Slice(rgs, func(i, j int) bool { + return rgs[i].file < rgs[j].file && rgs[i].name < rgs[j].name + }) + + return rgs +} + // Rules returns the list of the manager's rules. func (m *Manager) Rules() []Rule { m.mtx.RLock() diff --git a/web/ui/bindata.go b/web/ui/bindata.go index 7ea35f6d1c..7a9619bbd0 100644 --- a/web/ui/bindata.go +++ b/web/ui/bindata.go @@ -121,7 +121,7 @@ func webUiTemplates_baseHtml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/templates/_base.html", size: 2858, mode: os.FileMode(420), modTime: time.Unix(1495630073, 0)} + info := bindataFileInfo{name: "web/ui/templates/_base.html", size: 2858, mode: os.FileMode(420), modTime: time.Unix(1491024130, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -141,7 +141,7 @@ func webUiTemplatesAlertsHtml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/templates/alerts.html", size: 1832, mode: os.FileMode(420), modTime: time.Unix(1496733728, 0)} + info := bindataFileInfo{name: "web/ui/templates/alerts.html", size: 1832, mode: os.FileMode(420), modTime: time.Unix(1497004733, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -161,7 +161,7 @@ func webUiTemplatesConfigHtml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/templates/config.html", size: 175, mode: os.FileMode(420), modTime: time.Unix(1464223038, 0)} + info := bindataFileInfo{name: "web/ui/templates/config.html", size: 175, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -181,7 +181,7 @@ func webUiTemplatesFlagsHtml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/templates/flags.html", size: 433, mode: os.FileMode(420), modTime: time.Unix(1464223038, 0)} + info := bindataFileInfo{name: "web/ui/templates/flags.html", size: 433, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -201,12 +201,12 @@ func webUiTemplatesGraphHtml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/templates/graph.html", size: 1941, mode: os.FileMode(420), modTime: time.Unix(1495630073, 0)} + info := bindataFileInfo{name: "web/ui/templates/graph.html", size: 1941, mode: os.FileMode(420), modTime: time.Unix(1491024130, 0)} a := &asset{bytes: bytes, info: info} return a, nil } -var _webUiTemplatesRulesHtml = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x44\xce\xc1\x4a\xc7\x30\x0c\xc7\xf1\x7b\x9f\x22\xf6\xde\x7f\x61\xe7\xae\x67\x0f\x0a\xa2\xbe\x40\x5d\x33\x1b\x18\xb1\x74\xdd\x18\x84\xbc\xbb\x6c\x38\x3c\xe5\xf0\xcd\x0f\x3e\x22\x19\x67\x62\x04\x5b\x30\x65\xab\x1a\x9e\x9c\x03\xa6\x03\x9c\x8b\x22\xc8\x59\xd5\x98\xff\xaf\xe9\x87\x3b\x72\xb7\xaa\x06\x20\x64\xda\x61\x5a\xd2\xba\x8e\x57\x48\xc4\xd8\xdc\xbc\x6c\x94\x6d\x34\x00\x00\xa1\x0c\x40\x79\xb4\x6d\x5b\x70\xb5\xf1\xfd\x3c\xc1\x97\xe1\xaf\xd6\x86\x51\xa4\x25\xfe\x46\x78\x5c\x51\x55\xe4\xf1\xfc\xf9\xfa\xf2\xc1\x54\x2b\x76\xa8\xa9\x97\xb7\x86\x33\x1d\xaa\xe1\xab\xf9\x1b\x15\xfc\x39\x3e\x11\x3e\xd3\x1e\xcd\x6d\xfd\x0d\x00\x00\xff\xff\x04\x2c\xc2\x57\xd1\x00\x00\x00") +var _webUiTemplatesRulesHtml = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x54\x8f\xc1\x4a\x03\x31\x10\x86\xef\x79\x8a\x31\xf7\xec\x42\x8f\x9a\xe6\xa8\x1e\x54\x44\x7d\x81\xd8\xcc\xba\x03\xdb\x69\x48\xb2\xa5\x30\xcc\xbb\x4b\x6a\x8b\xf4\xf4\x0f\xf3\xcd\xc0\xff\x89\x24\x9c\x88\x11\xec\x8c\x31\x59\x55\x7f\xe7\x1c\x30\x9d\xc0\xb9\x20\x82\x9c\x54\x8d\xf9\xbf\xda\x1d\xb8\x21\x37\xab\x6a\x00\x7c\xa2\x23\xec\x96\x58\xeb\xf6\x0c\x22\x31\x16\x37\x2d\x2b\x25\x1b\x0c\x00\x80\x9f\x37\x40\x69\x6b\xcb\xba\x60\xb5\xe1\xa3\x87\x1f\xe7\xcd\x85\xe6\x82\x41\xa4\x44\xfe\x41\x18\x3a\x7c\x2a\x87\x35\x57\xd5\x47\x5a\xf0\x1e\x44\x86\x3e\xa8\x3e\xc0\x19\x00\xc7\xfd\xdf\xfa\x2d\xee\x51\xd5\xdc\xfc\x56\x55\x91\xe1\xf9\xeb\xf5\xe5\x93\x29\x67\x6c\x90\x63\x9b\xdf\x0b\x4e\x74\x52\x05\x3f\x7e\x97\xab\xd1\x25\xfc\xd8\x0b\x74\x91\x31\xd1\x31\x98\xab\xef\x6f\x00\x00\x00\xff\xff\xb9\xb8\xbf\x79\x15\x01\x00\x00") func webUiTemplatesRulesHtmlBytes() ([]byte, error) { return bindataRead( @@ -221,7 +221,7 @@ func webUiTemplatesRulesHtml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/templates/rules.html", size: 209, mode: os.FileMode(420), modTime: time.Unix(1464223038, 0)} + info := bindataFileInfo{name: "web/ui/templates/rules.html", size: 277, mode: os.FileMode(420), modTime: time.Unix(1497436569, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -241,7 +241,7 @@ func webUiTemplatesStatusHtml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/templates/status.html", size: 1756, mode: os.FileMode(420), modTime: time.Unix(1495630073, 0)} + info := bindataFileInfo{name: "web/ui/templates/status.html", size: 1756, mode: os.FileMode(420), modTime: time.Unix(1494943211, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -261,7 +261,7 @@ func webUiTemplatesTargetsHtml() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/templates/targets.html", size: 2275, mode: os.FileMode(420), modTime: time.Unix(1496733728, 0)} + info := bindataFileInfo{name: "web/ui/templates/targets.html", size: 2275, mode: os.FileMode(420), modTime: time.Unix(1497004733, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -281,7 +281,7 @@ func webUiStaticCssAlertsCss() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/css/alerts.css", size: 74, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/css/alerts.css", size: 74, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -301,7 +301,7 @@ func webUiStaticCssGraphCss() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/css/graph.css", size: 2710, mode: os.FileMode(420), modTime: time.Unix(1495630073, 0)} + info := bindataFileInfo{name: "web/ui/static/css/graph.css", size: 2710, mode: os.FileMode(420), modTime: time.Unix(1494943211, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -321,7 +321,7 @@ func webUiStaticCssProm_consoleCss() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/css/prom_console.css", size: 2883, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/css/prom_console.css", size: 2883, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -341,7 +341,7 @@ func webUiStaticCssPrometheusCss() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/css/prometheus.css", size: 405, mode: os.FileMode(420), modTime: time.Unix(1462274250, 0)} + info := bindataFileInfo{name: "web/ui/static/css/prometheus.css", size: 405, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -361,7 +361,7 @@ func webUiStaticImgAjaxLoaderGif() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/img/ajax-loader.gif", size: 847, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/img/ajax-loader.gif", size: 847, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -381,7 +381,7 @@ func webUiStaticImgFaviconIco() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/img/favicon.ico", size: 15086, mode: os.FileMode(420), modTime: time.Unix(1485793847, 0)} + info := bindataFileInfo{name: "web/ui/static/img/favicon.ico", size: 15086, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -401,7 +401,7 @@ func webUiStaticJsAlertsJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/js/alerts.js", size: 445, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/js/alerts.js", size: 445, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -421,7 +421,7 @@ func webUiStaticJsGraphJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/js/graph.js", size: 27415, mode: os.FileMode(420), modTime: time.Unix(1496733731, 0)} + info := bindataFileInfo{name: "web/ui/static/js/graph.js", size: 27415, mode: os.FileMode(420), modTime: time.Unix(1497004733, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -441,7 +441,7 @@ func webUiStaticJsGraph_templateHandlebar() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/js/graph_template.handlebar", size: 6337, mode: os.FileMode(420), modTime: time.Unix(1495630073, 0)} + info := bindataFileInfo{name: "web/ui/static/js/graph_template.handlebar", size: 6337, mode: os.FileMode(420), modTime: time.Unix(1491024130, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -461,7 +461,7 @@ func webUiStaticJsProm_consoleJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/js/prom_console.js", size: 22279, mode: os.FileMode(420), modTime: time.Unix(1496733731, 0)} + info := bindataFileInfo{name: "web/ui/static/js/prom_console.js", size: 22279, mode: os.FileMode(420), modTime: time.Unix(1497004733, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -481,7 +481,7 @@ func webUiStaticVendorBootstrap331CssBootstrapThemeMinCss() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/css/bootstrap-theme.min.css", size: 19835, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/css/bootstrap-theme.min.css", size: 19835, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -501,7 +501,7 @@ func webUiStaticVendorBootstrap331CssBootstrapMinCss() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/css/bootstrap.min.css", size: 113498, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/css/bootstrap.min.css", size: 113498, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -521,7 +521,7 @@ func webUiStaticVendorBootstrap331FontsGlyphiconsHalflingsRegularEot() (*asset, return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/fonts/glyphicons-halflings-regular.eot", size: 20335, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/fonts/glyphicons-halflings-regular.eot", size: 20335, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -541,7 +541,7 @@ func webUiStaticVendorBootstrap331FontsGlyphiconsHalflingsRegularSvg() (*asset, return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/fonts/glyphicons-halflings-regular.svg", size: 62926, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/fonts/glyphicons-halflings-regular.svg", size: 62926, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -561,7 +561,7 @@ func webUiStaticVendorBootstrap331FontsGlyphiconsHalflingsRegularTtf() (*asset, return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/fonts/glyphicons-halflings-regular.ttf", size: 41280, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/fonts/glyphicons-halflings-regular.ttf", size: 41280, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -581,7 +581,7 @@ func webUiStaticVendorBootstrap331FontsGlyphiconsHalflingsRegularWoff() (*asset, return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/fonts/glyphicons-halflings-regular.woff", size: 23320, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/fonts/glyphicons-halflings-regular.woff", size: 23320, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -601,7 +601,7 @@ func webUiStaticVendorBootstrap331JsBootstrapMinJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/js/bootstrap.min.js", size: 35601, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/js/bootstrap.min.js", size: 35601, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -621,7 +621,7 @@ func webUiStaticVendorBootstrap331JsNpmJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/js/npm.js", size: 484, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap-3.3.1/js/npm.js", size: 484, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -641,7 +641,7 @@ func webUiStaticVendorBootstrap3TypeaheadBootstrap3TypeaheadMinJs() (*asset, err return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap3-typeahead/bootstrap3-typeahead.min.js", size: 7856, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/bootstrap3-typeahead/bootstrap3-typeahead.min.js", size: 7856, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -661,7 +661,7 @@ func webUiStaticVendorEonasdanBootstrapDatetimepickerBootstrapDatetimepickerMinC return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/eonasdan-bootstrap-datetimepicker/bootstrap-datetimepicker.min.css", size: 7771, mode: os.FileMode(420), modTime: time.Unix(1481727016, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/eonasdan-bootstrap-datetimepicker/bootstrap-datetimepicker.min.css", size: 7771, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -681,7 +681,7 @@ func webUiStaticVendorEonasdanBootstrapDatetimepickerBootstrapDatetimepickerMinJ return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/eonasdan-bootstrap-datetimepicker/bootstrap-datetimepicker.min.js", size: 48881, mode: os.FileMode(420), modTime: time.Unix(1481727016, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/eonasdan-bootstrap-datetimepicker/bootstrap-datetimepicker.min.js", size: 48881, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -701,7 +701,7 @@ func webUiStaticVendorFuzzyFuzzyJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/fuzzy/fuzzy.js", size: 5669, mode: os.FileMode(420), modTime: time.Unix(1495630073, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/fuzzy/fuzzy.js", size: 5669, mode: os.FileMode(420), modTime: time.Unix(1489836724, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -721,7 +721,7 @@ func webUiStaticVendorJsJqueryHotkeysJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/js/jquery.hotkeys.js", size: 3283, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/js/jquery.hotkeys.js", size: 3283, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -741,7 +741,7 @@ func webUiStaticVendorJsJqueryMinJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/js/jquery.min.js", size: 95935, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/js/jquery.min.js", size: 95935, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -761,7 +761,7 @@ func webUiStaticVendorJsJquerySelectionJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/js/jquery.selection.js", size: 13320, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/js/jquery.selection.js", size: 13320, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -781,7 +781,7 @@ func webUiStaticVendorMomentMomentMinJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/moment/moment.min.js", size: 61281, mode: os.FileMode(420), modTime: time.Unix(1481727016, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/moment/moment.min.js", size: 61281, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -801,7 +801,7 @@ func webUiStaticVendorMustacheMustacheMinJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/mustache/mustache.min.js", size: 9528, mode: os.FileMode(420), modTime: time.Unix(1481727016, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/mustache/mustache.min.js", size: 9528, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -821,7 +821,7 @@ func webUiStaticVendorRickshawRickshawMinCss() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/rickshaw/rickshaw.min.css", size: 6102, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/rickshaw/rickshaw.min.css", size: 6102, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -841,7 +841,7 @@ func webUiStaticVendorRickshawRickshawMinJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/rickshaw/rickshaw.min.js", size: 76322, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/rickshaw/rickshaw.min.js", size: 76322, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -861,7 +861,7 @@ func webUiStaticVendorRickshawVendorD3LayoutMinJs() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/rickshaw/vendor/d3.layout.min.js", size: 17514, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/rickshaw/vendor/d3.layout.min.js", size: 17514, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -881,7 +881,7 @@ func webUiStaticVendorRickshawVendorD3V3Js() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "web/ui/static/vendor/rickshaw/vendor/d3.v3.js", size: 144718, mode: os.FileMode(420), modTime: time.Unix(1461244866, 0)} + info := bindataFileInfo{name: "web/ui/static/vendor/rickshaw/vendor/d3.v3.js", size: 144718, mode: os.FileMode(420), modTime: time.Unix(1488310986, 0)} a := &asset{bytes: bytes, info: info} return a, nil } diff --git a/web/ui/templates/rules.html b/web/ui/templates/rules.html index 41e0d5e0cb..db0c1dfe2e 100644 --- a/web/ui/templates/rules.html +++ b/web/ui/templates/rules.html @@ -3,6 +3,7 @@ {{define "content"}}

Rules

- 
{{range .Rules}}{{.HTMLSnippet pathPrefix}}
{{end}}
+ 
{{range .RuleGroups}}File: {{.File}}; Group name: {{.Name}}
+{{range .Rules}}{{.HTMLSnippet pathPrefix}} 
{{end}}{{end}}
{{end}} From dc69645e92024c092bf729907b40ded21e086a93 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Fri, 16 Jun 2017 10:46:21 +0530 Subject: [PATCH 11/14] Move back to go-yaml Signed-off-by: Goutham Veeramachaneni --- cmd/promtool/main.go | 4 +-- pkg/rulefmt/rulefmt.go | 26 +++++++++---------- pkg/rulefmt/testdata/bad_annotation.bad.yaml | 4 +-- pkg/rulefmt/testdata/bad_expr.bad.yaml | 4 +-- pkg/rulefmt/testdata/bad_lname.bad.yaml | 4 +-- pkg/rulefmt/testdata/duplicate_grp.bad.yaml | 4 +-- pkg/rulefmt/testdata/no_rec_alert.bad.yaml | 4 +-- pkg/rulefmt/testdata/noexpr.bad.yaml | 4 +-- pkg/rulefmt/testdata/noversion.bad.yaml | 2 +- .../testdata/record_and_alert.bad.yaml | 4 +-- rules/manager.go | 16 +++--------- 11 files changed, 33 insertions(+), 43 deletions(-) diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index 75f6061e9d..73e2698a3f 100644 --- a/cmd/promtool/main.go +++ b/cmd/promtool/main.go @@ -20,7 +20,7 @@ import ( "path/filepath" "strings" - yaml "github.com/ghodss/yaml" + yaml "gopkg.in/yaml.v2" "github.com/prometheus/common/model" "github.com/prometheus/common/version" @@ -244,7 +244,7 @@ func updateRules(t cli.Term, filename string) error { yamlRules = append(yamlRules, rulefmt.Rule{ Alert: r.Name, Expr: r.Expr.String(), - For: model.Duration(r.Duration).String(), + For: model.Duration(r.Duration), Labels: r.Labels.Map(), Annotations: r.Annotations.Map(), }) diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index 8fef3a1c8d..3afb1bbfd5 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -16,10 +16,10 @@ package rulefmt import ( "io/ioutil" - "github.com/ghodss/yaml" "github.com/pkg/errors" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/promql" + yaml "gopkg.in/yaml.v2" ) // Error represents semantical errors on parsing rule groups. @@ -35,8 +35,8 @@ func (err *Error) Error() string { // RuleGroups is a set of rule groups that are typically exposed in a file. type RuleGroups struct { - Version int `json:"version"` - Groups []RuleGroup `json:"groups"` + Version int `yaml:"version"` + Groups []RuleGroup `yaml:"groups"` } // Validate validates all rules in the rule groups. @@ -75,19 +75,19 @@ func (g *RuleGroups) Validate() (errs []error) { // RuleGroup is a list of sequentially evaluated recording and alerting rules. type RuleGroup struct { - Name string `json:"name"` - Interval string `json:"interval,omitempty"` - Rules []Rule `json:"rules"` + Name string `yaml:"name"` + Interval model.Duration `yaml:"interval,omitempty"` + Rules []Rule `yaml:"rules"` } // Rule describes an alerting or recording rule. type Rule struct { - Record string `json:"record,omitempty"` - Alert string `json:"alert,omitempty"` - Expr string `json:"expr"` - For string `json:"for,omitempty"` - Labels map[string]string `json:"labels,omitempty"` - Annotations map[string]string `json:"annotations,omitempty"` + Record string `yaml:"record,omitempty"` + Alert string `yaml:"alert,omitempty"` + Expr string `yaml:"expr"` + For model.Duration `yaml:"for,omitempty"` + Labels map[string]string `yaml:"labels,omitempty"` + Annotations map[string]string `yaml:"annotations,omitempty"` } // Validate the rule and return a list of encountered errors. @@ -108,7 +108,7 @@ func (r *Rule) Validate() (errs []error) { if len(r.Annotations) > 0 { errs = append(errs, errors.Errorf("invalid field 'annotations' in recording rule")) } - if r.For != "" { + if r.For != 0 { errs = append(errs, errors.Errorf("invalid field 'for' in recording rule")) } } diff --git a/pkg/rulefmt/testdata/bad_annotation.bad.yaml b/pkg/rulefmt/testdata/bad_annotation.bad.yaml index 523da2322b..6d18637659 100644 --- a/pkg/rulefmt/testdata/bad_annotation.bad.yaml +++ b/pkg/rulefmt/testdata/bad_annotation.bad.yaml @@ -1,5 +1,5 @@ -Version: 1 -Groups: +version: 1 +groups: - name: yolo rules: - alert: hola diff --git a/pkg/rulefmt/testdata/bad_expr.bad.yaml b/pkg/rulefmt/testdata/bad_expr.bad.yaml index df9a8cac1f..4fe1a6a0d1 100644 --- a/pkg/rulefmt/testdata/bad_expr.bad.yaml +++ b/pkg/rulefmt/testdata/bad_expr.bad.yaml @@ -1,5 +1,5 @@ -Version: 1 -Groups: +version: 1 +groups: - name: yolo rules: - record: yolo diff --git a/pkg/rulefmt/testdata/bad_lname.bad.yaml b/pkg/rulefmt/testdata/bad_lname.bad.yaml index 3ece3cf44e..2de03beeb5 100644 --- a/pkg/rulefmt/testdata/bad_lname.bad.yaml +++ b/pkg/rulefmt/testdata/bad_lname.bad.yaml @@ -1,5 +1,5 @@ -Version: 1 -Groups: +version: 1 +groups: - name: yolo rules: - record: hola diff --git a/pkg/rulefmt/testdata/duplicate_grp.bad.yaml b/pkg/rulefmt/testdata/duplicate_grp.bad.yaml index dc22391217..9eb14c263a 100644 --- a/pkg/rulefmt/testdata/duplicate_grp.bad.yaml +++ b/pkg/rulefmt/testdata/duplicate_grp.bad.yaml @@ -1,4 +1,4 @@ -Version: 1 -Groups: +version: 1 +groups: - name: yolo - name: yolo diff --git a/pkg/rulefmt/testdata/no_rec_alert.bad.yaml b/pkg/rulefmt/testdata/no_rec_alert.bad.yaml index 5806275aa9..cd1fdf02fc 100644 --- a/pkg/rulefmt/testdata/no_rec_alert.bad.yaml +++ b/pkg/rulefmt/testdata/no_rec_alert.bad.yaml @@ -1,5 +1,5 @@ -Version: 1 -Groups: +version: 1 +groups: - name: yolo rules: - expr: 1 diff --git a/pkg/rulefmt/testdata/noexpr.bad.yaml b/pkg/rulefmt/testdata/noexpr.bad.yaml index 99c5027965..6e71550648 100644 --- a/pkg/rulefmt/testdata/noexpr.bad.yaml +++ b/pkg/rulefmt/testdata/noexpr.bad.yaml @@ -1,5 +1,5 @@ -Version: 1 -Groups: +version: 1 +groups: - name: yolo rules: - record: ylo diff --git a/pkg/rulefmt/testdata/noversion.bad.yaml b/pkg/rulefmt/testdata/noversion.bad.yaml index 8b4ed9ce7c..ba7c2cd05e 100644 --- a/pkg/rulefmt/testdata/noversion.bad.yaml +++ b/pkg/rulefmt/testdata/noversion.bad.yaml @@ -1,2 +1,2 @@ -Groups: +groups: - name: yolo diff --git a/pkg/rulefmt/testdata/record_and_alert.bad.yaml b/pkg/rulefmt/testdata/record_and_alert.bad.yaml index 98063960c0..0cb0b48a29 100644 --- a/pkg/rulefmt/testdata/record_and_alert.bad.yaml +++ b/pkg/rulefmt/testdata/record_and_alert.bad.yaml @@ -1,5 +1,5 @@ -Version: 1 -Groups: +version: 1 +groups: - name: yolo rules: - record: Hi diff --git a/rules/manager.go b/rules/manager.go index af532829ca..42928ef6ad 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -522,13 +522,8 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s for _, rg := range rgs.Groups { itv := interval - if rg.Interval != "" { - dur, err := model.ParseDuration(rg.Interval) - if err != nil { - return nil, err - } - - itv = time.Duration(dur) + if rg.Interval != 0 { + itv = time.Duration(rg.Interval) } rules := make([]Rule, 0, len(rg.Rules)) @@ -539,15 +534,10 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s } if r.Alert != "" { - fordur, err := model.ParseDuration(r.For) - if err != nil { - return nil, err - } - rules = append(rules, NewAlertingRule( r.Alert, expr, - time.Duration(fordur), + time.Duration(r.For), labels.FromMap(r.Labels), labels.FromMap(r.Annotations), )) From 6b70a4d85031ecfb1404c3702237ebc0e7d3f2c8 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Fri, 16 Jun 2017 16:44:33 +0530 Subject: [PATCH 12/14] Incorporate PR feedback * Move fingerprint to Hash() * Move away from tsdb.MultiError * 0777 -> 0666 for files * checkOverflow of extra fields Signed-off-by: Goutham Veeramachaneni --- cmd/promtool/main.go | 20 +++++++++++--------- pkg/rulefmt/rulefmt.go | 35 +++++++++++++++++++++++++++++++++++ rules/manager.go | 34 +++++++++++++++++++--------------- 3 files changed, 65 insertions(+), 24 deletions(-) diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index 73e2698a3f..6e5dbf029d 100644 --- a/cmd/promtool/main.go +++ b/cmd/promtool/main.go @@ -29,7 +29,6 @@ import ( "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/util/cli" "github.com/prometheus/prometheus/util/promlint" - "github.com/prometheus/tsdb" ) // CheckConfigCmd validates configuration files. @@ -153,8 +152,11 @@ func CheckRulesCmd(t cli.Term, args ...string) int { failed := false for _, arg := range args { - if n, err := checkRules(t, arg); err != nil { - t.Errorf(" FAILED: %s", err) + if n, errs := checkRules(t, arg); errs != nil { + t.Errorf(" FAILED:") + for _, e := range errs { + t.Errorf(e.Error()) + } failed = true } else { t.Infof(" SUCCESS: %d rules found", n) @@ -167,18 +169,18 @@ func CheckRulesCmd(t cli.Term, args ...string) int { return 0 } -func checkRules(t cli.Term, filename string) (int, error) { +func checkRules(t cli.Term, filename string) (int, []error) { t.Infof("Checking %s", filename) if stat, err := os.Stat(filename); err != nil { - return 0, fmt.Errorf("cannot get file info") + return 0, []error{fmt.Errorf("cannot get file info")} } else if stat.IsDir() { - return 0, fmt.Errorf("is a directory") + return 0, []error{fmt.Errorf("is a directory")} } rgs, errs := rulefmt.ParseFile(filename) if errs != nil { - return 0, tsdb.MultiError(errs) + return 0, errs } numRules := 0 @@ -265,7 +267,7 @@ func updateRules(t cli.Term, filename string) error { return err } - return ioutil.WriteFile(filename+".yaml", y, 0777) + return ioutil.WriteFile(filename+".yaml", y, 0666) } var checkMetricsUsage = strings.TrimSpace(` @@ -329,7 +331,7 @@ func main() { }) app.Register("update-rules", &cli.Command{ - Desc: "update the rules to the new YAML format", + Desc: "update rules to the new YAML format", Run: UpdateRulesCmd, }) diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index 3afb1bbfd5..81a5962c01 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -14,7 +14,9 @@ package rulefmt import ( + "fmt" "io/ioutil" + "strings" "github.com/pkg/errors" "github.com/prometheus/common/model" @@ -37,6 +39,9 @@ func (err *Error) Error() string { type RuleGroups struct { Version int `yaml:"version"` Groups []RuleGroup `yaml:"groups"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // Validate validates all rules in the rule groups. @@ -58,6 +63,10 @@ func (g *RuleGroups) Validate() (errs []error) { ) } + if err := checkOverflow(g.XXX, "rule_group"); err != nil { + errs = append(errs, errors.Wrapf(err, "Group: %s", g.Name)) + } + set[g.Name] = struct{}{} for i, r := range g.Rules { @@ -70,6 +79,11 @@ func (g *RuleGroups) Validate() (errs []error) { } } } + + if err := checkOverflow(g.XXX, "config_file"); err != nil { + errs = append(errs, err) + } + return errs } @@ -78,6 +92,9 @@ type RuleGroup struct { Name string `yaml:"name"` Interval model.Duration `yaml:"interval,omitempty"` Rules []Rule `yaml:"rules"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // Rule describes an alerting or recording rule. @@ -88,6 +105,9 @@ type Rule struct { For model.Duration `yaml:"for,omitempty"` Labels map[string]string `yaml:"labels,omitempty"` Annotations map[string]string `yaml:"annotations,omitempty"` + + // Catches all undefined fields and must be empty after parsing. + XXX map[string]interface{} `yaml:",inline"` } // Validate the rule and return a list of encountered errors. @@ -129,9 +149,24 @@ func (r *Rule) Validate() (errs []error) { } } + if err := checkOverflow(r.XXX, "rule"); err != nil { + errs = append(errs, err) + } + return errs } +func checkOverflow(m map[string]interface{}, ctx string) error { + if len(m) > 0 { + var keys []string + for k := range m { + keys = append(keys, k) + } + return fmt.Errorf("unknown fields in %s: %s", ctx, strings.Join(keys, ", ")) + } + return nil +} + // ParseFile parses the rule file and validates it. func ParseFile(file string) (*RuleGroups, []error) { b, err := ioutil.ReadFile(file) diff --git a/rules/manager.go b/rules/manager.go index 42928ef6ad..a72cfe777f 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -14,6 +14,7 @@ package rules import ( + "errors" "fmt" "math" "net/url" @@ -26,8 +27,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/log" - "github.com/prometheus/common/model" - "github.com/prometheus/tsdb" "golang.org/x/net/context" "github.com/prometheus/prometheus/config" @@ -212,12 +211,13 @@ func (g *Group) stop() { <-g.terminated } -func (g *Group) fingerprint() model.Fingerprint { - l := model.LabelSet{ - "name": model.LabelValue(g.name), - "filename": model.LabelValue(g.file), - } - return l.Fingerprint() +func (g *Group) hash() uint64 { + l := labels.New( + labels.Label{"name", g.name}, + labels.Label{"file", g.file}, + ) + + return l.Hash() } // offset returns until the next consistently slotted evaluation interval. @@ -226,7 +226,7 @@ func (g *Group) offset() time.Duration { var ( base = now - (now % int64(g.interval)) - offset = uint64(g.fingerprint()) % uint64(g.interval) + offset = g.hash() % uint64(g.interval) next = base + int64(offset) ) @@ -466,9 +466,13 @@ func (m *Manager) ApplyConfig(conf *config.Config) error { } // To be replaced with a configurable per-group interval. - groups, err := m.loadGroups(time.Duration(conf.GlobalConfig.EvaluationInterval), files...) - if err != nil { - return fmt.Errorf("error loading rules, previous rule set restored: %s", err) + groups, errs := m.loadGroups(time.Duration(conf.GlobalConfig.EvaluationInterval), files...) + if errs != nil { + for _, e := range errs { + // TODO(gouthamve): Move to non-global logger. + log.Errorln(e) + } + return errors.New("error loading rules, previous rule set restored") } var wg sync.WaitGroup @@ -511,13 +515,13 @@ func (m *Manager) ApplyConfig(conf *config.Config) error { // loadGroups reads groups from a list of files. // As there's currently no group syntax a single group named "default" containing // all rules will be returned. -func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[string]*Group, error) { +func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[string]*Group, []error) { groups := make(map[string]*Group) for _, fn := range filenames { rgs, errs := rulefmt.ParseFile(fn) if errs != nil { - return nil, tsdb.MultiError(errs) + return nil, errs } for _, rg := range rgs.Groups { @@ -530,7 +534,7 @@ func (m *Manager) loadGroups(interval time.Duration, filenames ...string) (map[s for _, r := range rg.Rules { expr, err := promql.ParseExpr(r.Expr) if err != nil { - return nil, err + return nil, []error{err} } if r.Alert != "" { From c472316fb387bcaa5cd08a65665355936f70a5c1 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Fri, 16 Jun 2017 16:57:22 +0530 Subject: [PATCH 13/14] Check done before every rule evaluation. Signed-off-by: Goutham Veeramachaneni --- rules/manager.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rules/manager.go b/rules/manager.go index a72cfe777f..b9f22b6d57 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -285,6 +285,12 @@ func typeForRule(r Rule) ruleType { // Eval runs a single evaluation cycle in which all rules are evaluated sequentially. func (g *Group) Eval(ts time.Time) { for i, rule := range g.rules { + select { + case <-g.done: + return + default: + } + rtyp := string(typeForRule(rule)) func(i int, rule Rule) { From 592cb00c2f3a322eee4dae711a3afbc1c7ef85ee Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Mon, 19 Jun 2017 16:38:46 +0530 Subject: [PATCH 14/14] Remove version from RuleGroups Signed-off-by: Goutham Veeramachaneni --- cmd/promtool/main.go | 1 - pkg/rulefmt/rulefmt.go | 6 +----- pkg/rulefmt/rulefmt_test.go | 4 ---- pkg/rulefmt/testdata/bad_annotation.bad.yaml | 1 - pkg/rulefmt/testdata/bad_expr.bad.yaml | 1 - pkg/rulefmt/testdata/bad_lname.bad.yaml | 1 - pkg/rulefmt/testdata/duplicate_grp.bad.yaml | 1 - pkg/rulefmt/testdata/no_rec_alert.bad.yaml | 1 - pkg/rulefmt/testdata/noexpr.bad.yaml | 1 - pkg/rulefmt/testdata/noversion.bad.yaml | 2 -- pkg/rulefmt/testdata/record_and_alert.bad.yaml | 1 - pkg/rulefmt/testdata/test.yaml | 1 - 12 files changed, 1 insertion(+), 20 deletions(-) delete mode 100644 pkg/rulefmt/testdata/noversion.bad.yaml diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index 6e5dbf029d..2af10249cc 100644 --- a/cmd/promtool/main.go +++ b/cmd/promtool/main.go @@ -232,7 +232,6 @@ func updateRules(t cli.Term, filename string) error { } yamlRG := &rulefmt.RuleGroups{ - Version: 1, Groups: []rulefmt.RuleGroup{{ Name: filename, }}, diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index 81a5962c01..bc08bc0319 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -37,8 +37,7 @@ func (err *Error) Error() string { // RuleGroups is a set of rule groups that are typically exposed in a file. type RuleGroups struct { - Version int `yaml:"version"` - Groups []RuleGroup `yaml:"groups"` + Groups []RuleGroup `yaml:"groups"` // Catches all undefined fields and must be empty after parsing. XXX map[string]interface{} `yaml:",inline"` @@ -46,9 +45,6 @@ type RuleGroups struct { // Validate validates all rules in the rule groups. func (g *RuleGroups) Validate() (errs []error) { - if g.Version != 1 { - errs = append(errs, errors.Errorf("invalid rule group version %d", g.Version)) - } set := map[string]struct{}{} for _, g := range g.Groups { diff --git a/pkg/rulefmt/rulefmt_test.go b/pkg/rulefmt/rulefmt_test.go index 95d088f18e..358a98377a 100644 --- a/pkg/rulefmt/rulefmt_test.go +++ b/pkg/rulefmt/rulefmt_test.go @@ -37,10 +37,6 @@ func TestParseFileFailure(t *testing.T) { filename: "duplicate_grp.bad.yaml", errMsg: "groupname: \"yolo\" is repeated in the same file", }, - { - filename: "noversion.bad.yaml", - errMsg: "invalid rule group version 0", - }, { filename: "bad_expr.bad.yaml", errMsg: "parse error", diff --git a/pkg/rulefmt/testdata/bad_annotation.bad.yaml b/pkg/rulefmt/testdata/bad_annotation.bad.yaml index 6d18637659..b59c41a632 100644 --- a/pkg/rulefmt/testdata/bad_annotation.bad.yaml +++ b/pkg/rulefmt/testdata/bad_annotation.bad.yaml @@ -1,4 +1,3 @@ -version: 1 groups: - name: yolo rules: diff --git a/pkg/rulefmt/testdata/bad_expr.bad.yaml b/pkg/rulefmt/testdata/bad_expr.bad.yaml index 4fe1a6a0d1..f9a029ccfd 100644 --- a/pkg/rulefmt/testdata/bad_expr.bad.yaml +++ b/pkg/rulefmt/testdata/bad_expr.bad.yaml @@ -1,4 +1,3 @@ -version: 1 groups: - name: yolo rules: diff --git a/pkg/rulefmt/testdata/bad_lname.bad.yaml b/pkg/rulefmt/testdata/bad_lname.bad.yaml index 2de03beeb5..7153f3ba50 100644 --- a/pkg/rulefmt/testdata/bad_lname.bad.yaml +++ b/pkg/rulefmt/testdata/bad_lname.bad.yaml @@ -1,4 +1,3 @@ -version: 1 groups: - name: yolo rules: diff --git a/pkg/rulefmt/testdata/duplicate_grp.bad.yaml b/pkg/rulefmt/testdata/duplicate_grp.bad.yaml index 9eb14c263a..97d453429e 100644 --- a/pkg/rulefmt/testdata/duplicate_grp.bad.yaml +++ b/pkg/rulefmt/testdata/duplicate_grp.bad.yaml @@ -1,4 +1,3 @@ -version: 1 groups: - name: yolo - name: yolo diff --git a/pkg/rulefmt/testdata/no_rec_alert.bad.yaml b/pkg/rulefmt/testdata/no_rec_alert.bad.yaml index cd1fdf02fc..64d2e8f20d 100644 --- a/pkg/rulefmt/testdata/no_rec_alert.bad.yaml +++ b/pkg/rulefmt/testdata/no_rec_alert.bad.yaml @@ -1,4 +1,3 @@ -version: 1 groups: - name: yolo rules: diff --git a/pkg/rulefmt/testdata/noexpr.bad.yaml b/pkg/rulefmt/testdata/noexpr.bad.yaml index 6e71550648..ad0c29e4cd 100644 --- a/pkg/rulefmt/testdata/noexpr.bad.yaml +++ b/pkg/rulefmt/testdata/noexpr.bad.yaml @@ -1,4 +1,3 @@ -version: 1 groups: - name: yolo rules: diff --git a/pkg/rulefmt/testdata/noversion.bad.yaml b/pkg/rulefmt/testdata/noversion.bad.yaml deleted file mode 100644 index ba7c2cd05e..0000000000 --- a/pkg/rulefmt/testdata/noversion.bad.yaml +++ /dev/null @@ -1,2 +0,0 @@ -groups: -- name: yolo diff --git a/pkg/rulefmt/testdata/record_and_alert.bad.yaml b/pkg/rulefmt/testdata/record_and_alert.bad.yaml index 0cb0b48a29..0ba81b7423 100644 --- a/pkg/rulefmt/testdata/record_and_alert.bad.yaml +++ b/pkg/rulefmt/testdata/record_and_alert.bad.yaml @@ -1,4 +1,3 @@ -version: 1 groups: - name: yolo rules: diff --git a/pkg/rulefmt/testdata/test.yaml b/pkg/rulefmt/testdata/test.yaml index d7cf526b78..a3127426d8 100644 --- a/pkg/rulefmt/testdata/test.yaml +++ b/pkg/rulefmt/testdata/test.yaml @@ -1,4 +1,3 @@ -version: 1 groups: - name: my-group-name interval: 30s # defaults to global interval