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 <cs14btech11014@iith.ac.in>

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

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)

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 != "" {