Refactor comment parsing

cloudflare · Nov 28, 2023 · 4c14f46 · 4c14f46
1 parent 06b336b
commit 4c14f46
Show file tree

Hide file tree

Showing 23 changed files with 1,066 additions and 415 deletions.
diff --git a/cmd/pint/tests/0111_snooze.txt b/cmd/pint/tests/0111_snooze.txt
@@ -8,7 +8,7 @@ level=INFO msg="Finding all rules to check" paths=["rules"]
 level=DEBUG msg="File parsed" path=rules/0001.yml rules=1
 level=DEBUG msg="Generated all Prometheus servers" count=0
 level=DEBUG msg="Found recording rule" path=rules/0001.yml record=sum-job lines=2-3
-level=DEBUG msg="Check snoozed by comment" check=promql/aggregate(job:true) comment="snooze 2099-11-28T10:24:18Z promql/aggregate" until="2099-11-28T10:24:18Z" snooze=promql/aggregate
+level=DEBUG msg="Check snoozed by comment" check=promql/aggregate(job:true) match=promql/aggregate until="2099-11-28T10:24:18Z"
 level=DEBUG msg="Configured checks for rule" enabled=["promql/syntax","alerts/for","alerts/comparison","alerts/template","promql/fragile","promql/regexp"] path=rules/0001.yml rule=sum-job
 -- rules/0001.yml --
 # pint snooze 2099-11-28T10:24:18Z promql/aggregate

diff --git a/cmd/pint/tests/0116_file_snooze.txt b/cmd/pint/tests/0116_file_snooze.txt
@@ -5,8 +5,8 @@ cmp stderr stderr.txt
 -- stderr.txt --
 level=INFO msg="Loading configuration file" path=.pint.hcl
 level=INFO msg="Finding all rules to check" paths=["rules"]
-level=DEBUG msg="Check snoozed by comment" check=promql/aggregate(job:true) comment="file/snooze 2099-11-28T10:24:18Z promql/aggregate(job:true)" until="2099-11-28T10:24:18Z" snooze=promql/aggregate(job:true)
-level=DEBUG msg="Check snoozed by comment" check=alerts/for comment="file/snooze 2099-11-28T10:24:18Z alerts/for" until="2099-11-28T10:24:18Z" snooze=alerts/for
+level=DEBUG msg="Check snoozed by comment" check=promql/aggregate(job:true) match=promql/aggregate(job:true) until="2099-11-28T10:24:18Z"
+level=DEBUG msg="Check snoozed by comment" check=alerts/for match=alerts/for until="2099-11-28T10:24:18Z"
 level=DEBUG msg="File parsed" path=rules/0001.yml rules=2
 level=DEBUG msg="Generated all Prometheus servers" count=0
 level=DEBUG msg="Found recording rule" path=rules/0001.yml record=sum-job lines=4-5

diff --git a/internal/checks/base_test.go b/internal/checks/base_test.go
@@ -173,7 +173,7 @@ func runTests(t *testing.T, testCases []checkTest) {
 
 func parseContent(content string) (entries []discovery.Entry, err error) {
 	p := parser.NewParser()
-	rules, err := p.Parse([]byte(content))
+	rules, _, err := p.Parse([]byte(content))
 	if err != nil {
 		return nil, err
 	}

diff --git a/internal/checks/promql_series.go b/internal/checks/promql_series.go
@@ -10,6 +10,7 @@ import (
 
 	"golang.org/x/exp/slices"
 
+	"github.com/cloudflare/pint/internal/comments"
 	"github.com/cloudflare/pint/internal/discovery"
 	"github.com/cloudflare/pint/internal/output"
 	"github.com/cloudflare/pint/internal/parser"
@@ -615,18 +616,22 @@ func (c SeriesCheck) instantSeriesCount(ctx context.Context, query string) (int,
 
 func (c SeriesCheck) getMinAge(rule parser.Rule, selector promParser.VectorSelector) (minAge time.Duration, problems []Problem) {
 	minAge = time.Hour * 2
-
 	bareSelector := stripLabels(selector)
-	for _, s := range [][]string{
-		{"rule/set", c.Reporter(), "min-age"},
-		{"rule/set", fmt.Sprintf("%s(%s)", c.Reporter(), bareSelector.String()), "min-age"},
-		{"rule/set", fmt.Sprintf("%s(%s)", c.Reporter(), selector.String()), "min-age"},
-	} {
-		if cmt, ok := rule.GetComment(s...); ok {
-			dur, err := model.ParseDuration(cmt.Value)
+	prefixes := []string{
+		fmt.Sprintf("%s min-age ", c.Reporter()),
+		fmt.Sprintf("%s(%s) min-age ", c.Reporter(), bareSelector.String()),
+		fmt.Sprintf("%s(%s) min-age ", c.Reporter(), selector.String()),
+	}
+	for _, ruleSet := range comments.Only[comments.RuleSet](rule.Comments, comments.RuleSetType) {
+		for _, prefix := range prefixes {
+			if !strings.HasPrefix(ruleSet.Value, prefix) {
+				continue
+			}
+			val := strings.TrimPrefix(ruleSet.Value, prefix)
+			dur, err := model.ParseDuration(val)
 			if err != nil {
 				problems = append(problems, Problem{
-					Fragment: cmt.String(),
+					Fragment: fmt.Sprintf("%s %s", comments.RuleSetComment, ruleSet.Value),
 					Lines:    rule.LineRange(),
 					Reporter: c.Reporter(),
 					Text:     fmt.Sprintf("Failed to parse pint comment as duration: %s", err),
@@ -644,13 +649,16 @@ func (c SeriesCheck) getMinAge(rule parser.Rule, selector promParser.VectorSelec
 
 func (c SeriesCheck) isLabelValueIgnored(rule parser.Rule, selector promParser.VectorSelector, labelName string) bool {
 	bareSelector := stripLabels(selector)
-	for _, s := range []string{
-		fmt.Sprintf("rule/set %s ignore/label-value %s", c.Reporter(), labelName),
-		fmt.Sprintf("rule/set %s(%s) ignore/label-value %s", c.Reporter(), bareSelector.String(), labelName),
-		fmt.Sprintf("rule/set %s(%s) ignore/label-value %s", c.Reporter(), selector.String(), labelName),
-	} {
-		if rule.HasComment(s) {
-			return true
+	values := []string{
+		fmt.Sprintf("%s ignore/label-value %s", c.Reporter(), labelName),
+		fmt.Sprintf("%s(%s) ignore/label-value %s", c.Reporter(), bareSelector.String(), labelName),
+		fmt.Sprintf("%s(%s) ignore/label-value %s", c.Reporter(), selector.String(), labelName),
+	}
+	for _, ruleSet := range comments.Only[comments.RuleSet](rule.Comments, comments.RuleSetType) {
+		for _, val := range values {
+			if ruleSet.Value == val {
+				return true
+			}
 		}
 	}
 	return false
@@ -704,9 +712,9 @@ func stripLabels(selector promParser.VectorSelector) promParser.VectorSelector {
 }
 
 func isDisabled(rule parser.Rule, selector promParser.VectorSelector) bool {
-	for _, c := range rule.GetComments("disable") {
-		if strings.HasPrefix(c.Value, SeriesCheckName+"(") && strings.HasSuffix(c.Value, ")") {
-			cs := strings.TrimSuffix(strings.TrimPrefix(c.Value, SeriesCheckName+"("), ")")
+	for _, disable := range comments.Only[comments.Disable](rule.Comments, comments.DisableType) {
+		if strings.HasPrefix(disable.Match, SeriesCheckName+"(") && strings.HasSuffix(disable.Match, ")") {
+			cs := strings.TrimSuffix(strings.TrimPrefix(disable.Match, SeriesCheckName+"("), ")")
 			// try full string or name match first
 			if cs == selector.String() || cs == selector.Name {
 				return true

diff --git a/internal/checks/template_test.go b/internal/checks/template_test.go
@@ -106,7 +106,7 @@ func TestTemplatedRegexpExpand(t *testing.T) {
 
 func newMustRule(content string) parser.Rule {
 	p := parser.NewParser()
-	rules, err := p.Parse([]byte(content))
+	rules, _, err := p.Parse([]byte(content))
 	if err != nil {
 		panic(err)
 	}