Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rulegroups #2842

Merged
merged 15 commits into from
Jun 19, 2017
96 changes: 93 additions & 3 deletions cmd/promtool/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@ import (
"path/filepath"
"strings"

yaml "gopkg.in/yaml.v2"

"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"
"github.com/prometheus/tsdb"
)

// CheckConfigCmd validates configuration files.
Expand Down Expand Up @@ -171,16 +176,96 @@ func checkRules(t cli.Term, filename string) (int, error) {
return 0, fmt.Errorf("is a directory")
}

rgs, errs := rulefmt.ParseFile(filename)
if errs != nil {
return 0, tsdb.MultiError(errs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same about import tsdb for this and probably better returning the actual slice so the errors can be logged as one per line. tsdb.MultiError will just concat them with ; which will be impossible to debug with.

}

numRules := 0
for _, rg := range rgs.Groups {
numRules += len(rg.Rules)
}

return numRules, nil
}

// UpdateRulesCmd updates the rule files.
func UpdateRulesCmd(t cli.Term, args ...string) int {
if len(args) == 0 {
t.Infof("usage: promtool update-rules <files>")
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 0, err
return err
}

rules, err := promql.ParseStmts(string(content))
if err != nil {
return 0, err
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),
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)
Copy link
Contributor

@fabxc fabxc Jun 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we are now using the plain YAML lib again this will generate rule files with basically random key ordering right?

@brian-brazil if yes, this is pretty bad UX just because we don't want to use a 100 LOC wrapper lib that's vendored already anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using go-yaml the fields are ordered as we declare them. ghodss/yaml produces the fields in alphabetical order.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's move forward then. Somewhat sure I've seen things wildely mixed – but maybe I recall incorrectly.

if err != nil {
return err
}
return len(rules), nil

return ioutil.WriteFile(filename+".yaml", y, 0777)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0666 for files, 0777 for dirs. That's what we are going with in general I think.

}

var checkMetricsUsage = strings.TrimSpace(`
Expand Down Expand Up @@ -243,6 +328,11 @@ func main() {
Run: CheckMetricsCmd,
})

app.Register("update-rules", &cli.Command{
Desc: "update the rules to the new YAML format",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update rule files to...

to be consistent with the above.

Run: UpdateRulesCmd,
})

app.Register("version", &cli.Command{
Desc: "print the version of this binary",
Run: VersionCmd,
Expand Down
146 changes: 146 additions & 0 deletions pkg/rulefmt/rulefmt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// 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 (
"io/ioutil"

"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.
type Error struct {
Group string
Rule int
Err error
}

func (err *Error) Error() string {
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.
type RuleGroups struct {
Version int `yaml:"version"`
Groups []RuleGroup `yaml:"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))
}
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,
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{
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 `yaml:"name"`
Interval model.Duration `yaml:"interval,omitempty"`
Rules []Rule `yaml:"rules"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have the XXX checkOverflow stuff.

}

// Rule describes an alerting or recording rule.
type Rule struct {
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.
func (r *Rule) Validate() (errs []error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to validate that the labelnames/annotations names are valid, and for rules that label values are valid.

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 != 0 {
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
}

// 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()
}
81 changes: 81 additions & 0 deletions pkg/rulefmt/rulefmt_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// 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 (
"path/filepath"
"strings"
"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)
}
}
}

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",
},
{
filename: "bad_lname.bad.yaml",
errMsg: "invalid label name",
},
{
filename: "bad_annotation.bad.yaml",
errMsg: "invalid annotation name",
},
}

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)
}
}

}
8 changes: 8 additions & 0 deletions pkg/rulefmt/testdata/bad_annotation.bad.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
version: 1
groups:
- name: yolo
rules:
- alert: hola
expr: 1
annotations:
ins-tance: localhost
6 changes: 6 additions & 0 deletions pkg/rulefmt/testdata/bad_expr.bad.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
version: 1
groups:
- name: yolo
rules:
- record: yolo
expr: rate(hi)
8 changes: 8 additions & 0 deletions pkg/rulefmt/testdata/bad_lname.bad.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
version: 1
groups:
- name: yolo
rules:
- record: hola
expr: 1
labels:
ins-tance: localhost
4 changes: 4 additions & 0 deletions pkg/rulefmt/testdata/duplicate_grp.bad.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
version: 1
groups:
- name: yolo
- name: yolo
5 changes: 5 additions & 0 deletions pkg/rulefmt/testdata/no_rec_alert.bad.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
version: 1
groups:
- name: yolo
rules:
- expr: 1
5 changes: 5 additions & 0 deletions pkg/rulefmt/testdata/noexpr.bad.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
version: 1
groups:
- name: yolo
rules:
- record: ylo
Loading