From da020e3a77717d8a25bc8610a1a0f57085ce18fe Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Thu, 11 Jul 2024 15:58:23 +0100 Subject: [PATCH] Improve comment warnings --- cmd/pint/tests/0160_ci_comment_edit.txt | 2 +- cmd/pint/tests/0176_comments.txt | 45 +++++ docs/changelog.md | 5 + docs/checks/promql/series.md | 2 +- internal/checks/promql_regexp.go | 3 +- internal/checks/promql_series.go | 198 +++++++++---------- internal/checks/promql_series_test.go | 243 +++++++++++++++++++++++- internal/parser/read.go | 12 +- internal/parser/read_test.go | 6 +- internal/parser/utils/vectorselector.go | 4 +- 10 files changed, 411 insertions(+), 109 deletions(-) create mode 100644 cmd/pint/tests/0176_comments.txt diff --git a/cmd/pint/tests/0160_ci_comment_edit.txt b/cmd/pint/tests/0160_ci_comment_edit.txt index 614fab9b..5d9b48a6 100644 --- a/cmd/pint/tests/0160_ci_comment_edit.txt +++ b/cmd/pint/tests/0160_ci_comment_edit.txt @@ -50,7 +50,7 @@ level=INFO msg="Problems found" Bug=2 Warning=1 rules.yml:8 Bug: `prom` Prometheus server at http://127.0.0.1:7160 didn't have any series for `up` metric in the last 1w. (promql/series) 8 | expr: up == 0 -rules.yml:8 Warning: pint disable comment `promql/series(xxx)` check doesn't match any selector in this query (promql/series) +rules.yml:8 Warning: pint disable comment `promql/series(xxx)` doesn't match any selector in this query (promql/series) 8 | expr: up == 0 rules.yml:16 Bug: `prom` Prometheus server at http://127.0.0.1:7160 didn't have any series for `up` metric in the last 1w. (promql/series) diff --git a/cmd/pint/tests/0176_comments.txt b/cmd/pint/tests/0176_comments.txt new file mode 100644 index 00000000..d88ac79f --- /dev/null +++ b/cmd/pint/tests/0176_comments.txt @@ -0,0 +1,45 @@ +http response prometheus /api/v1/status/flags 200 {"status":"success","data":{"storage.tsdb.retention.time": "1d"}} +http response prometheus /api/v1/metadata 200 {"status":"success","data":{}} +http response prometheus /api/v1/status/config 200 {"status":"success","data":{"yaml":"global:\n scrape_interval: 1m\n"}} +http response prometheus /api/v1/query_range 200 {"status":"success","data":{"resultType":"matrix","result":[]}} +http response prometheus /api/v1/query 200 {"status":"success","data":{"resultType":"vector","result":[]}} +http start prometheus 127.0.0.1:7176 + +pint.error --no-color lint rules +! stdout . +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=INFO msg="Configured new Prometheus server" name=prom uris=1 uptime=up tags=[] include=[] exclude=[] +level=WARN msg="No results for Prometheus uptime metric, you might have set uptime config option to a missing metric, please check your config" name=prom metric=up +level=WARN msg="Using dummy Prometheus uptime metric results with no gaps" name=prom metric=up +rules/1.yml:15 Bug: `prom` Prometheus server at http://127.0.0.1:7176 didn't have any series for `up` metric in the last 1w. (promql/series) + 15 | expr: up == 0 + +level=INFO msg="Problems found" Bug=1 +level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher" +-- rules/1.yml -- +--- +groups: +- name: "{{ source }}" + rules: +# pint ignore/begin + - alert: Ignored + # pint rule/set promql/series(colo_metadata) ignore/label-value brand + # pint rule/set promql/series ignore/label-value colo_status + expr: count(colo_metadata{colo_status="v", brand="b1"}) > 0 +# pint ignore/end + + # dummy comment 1 + - alert: Active + # dummy comment 2 + expr: up == 0 + +-- .pint.hcl -- +prometheus "prom" { + uri = "http://127.0.0.1:7176" + failover = [] + timeout = "5s" +} diff --git a/docs/changelog.md b/docs/changelog.md index 160a7ddd..c158413f 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -7,6 +7,11 @@ - [promql/series](checks/promql/series.md) check will now generate warnings if there are `# pint disable` or `# pint rule/set` comments that are not matching any valid query selector or Prometheus server. +### Fixed + +- [promql/series](checks/promql/series.md) will now parse `rule/set` comments that target specific + time series selectors in PromQL the same way as `# pint disable` comments do. + ## v0.61.2 ### Fixed diff --git a/docs/checks/promql/series.md b/docs/checks/promql/series.md index 13a1c1e7..671ceae5 100644 --- a/docs/checks/promql/series.md +++ b/docs/checks/promql/series.md @@ -193,7 +193,7 @@ Duration must follow syntax documented [here](https://prometheus.io/docs/prometh To set `min-age` for specific metric: ```yaml -# pint rule/set promql/series($metric_name) min-age $duration +# pint rule/set promql/series($selector)) min-age $duration ``` Example: diff --git a/internal/checks/promql_regexp.go b/internal/checks/promql_regexp.go index d7c9e61b..1a257198 100644 --- a/internal/checks/promql_regexp.go +++ b/internal/checks/promql_regexp.go @@ -11,6 +11,7 @@ import ( "github.com/cloudflare/pint/internal/discovery" "github.com/cloudflare/pint/internal/parser" + "github.com/cloudflare/pint/internal/parser/utils" ) const ( @@ -52,7 +53,7 @@ func (c RegexpCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule } done := map[string]struct{}{} - for _, selector := range getSelectors(expr.Query) { + for _, selector := range utils.HasVectorSelector(expr.Query) { if _, ok := done[selector.String()]; ok { continue } diff --git a/internal/checks/promql_series.go b/internal/checks/promql_series.go index 68ab3ad7..4fd315da 100644 --- a/internal/checks/promql_series.go +++ b/internal/checks/promql_series.go @@ -14,6 +14,7 @@ import ( "github.com/cloudflare/pint/internal/discovery" "github.com/cloudflare/pint/internal/output" "github.com/cloudflare/pint/internal/parser" + "github.com/cloudflare/pint/internal/parser/utils" "github.com/cloudflare/pint/internal/promapi" "github.com/prometheus/common/model" @@ -120,10 +121,10 @@ func (c SeriesCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Ru params := promapi.NewRelativeRange(settings.lookbackRangeDuration, settings.lookbackStepDuration) - selectors := getSelectors(expr.Query) + selectors := utils.HasVectorSelector(expr.Query) done := map[string]bool{} - for _, selector := range selectors { + for _, selector := range getNonFallbackSelectors(expr.Query) { if _, ok := done[selector.String()]; ok { continue } @@ -539,12 +540,12 @@ func (c SeriesCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Ru problems = append(problems, Problem{ Lines: expr.Value.Lines, Reporter: c.Reporter(), - Text: fmt.Sprintf("pint %s comment `%s` check doesn't match any selector in this query", comments.DisableComment, disable.Match), + Text: fmt.Sprintf("pint %s comment `%s` doesn't match any selector in this query", comments.DisableComment, disable.Match), Details: SeriesCheckUnusedDisableComment, Severity: Warning, }) } - for _, ruleSet := range orphanedRuleSetComments(c.Reporter(), rule, selectors) { + for _, ruleSet := range orphanedRuleSetComments(rule, selectors) { problems = append(problems, Problem{ Lines: expr.Value.Lines, Reporter: c.Reporter(), @@ -635,25 +636,28 @@ 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 - prefixes := ruleSetMinAgePrefixes(c.Reporter(), selector) for _, ruleSet := range comments.Only[comments.RuleSet](rule.Comments, comments.RuleSetType) { - for _, prefix := range prefixes { - if !strings.HasPrefix(ruleSet.Value, prefix) { + matcher, key, value := parseRuleSet(ruleSet.Value) + if key != "min-age" { + continue + } + if matcher != "" { + isMatch, _ := matchSelectorToMetric(selector, matcher) + if !isMatch { continue } - val := strings.TrimPrefix(ruleSet.Value, prefix) - dur, err := model.ParseDuration(val) - if err != nil { - problems = append(problems, Problem{ - Lines: rule.Lines, - Reporter: c.Reporter(), - Text: fmt.Sprintf("Failed to parse pint comment as duration: %s", err), - Details: SeriesCheckMinAgeDetails, - Severity: Warning, - }) - } else { - minAge = time.Duration(dur) - } + } + dur, err := model.ParseDuration(value) + if err != nil { + problems = append(problems, Problem{ + Lines: rule.Lines, + Reporter: c.Reporter(), + Text: fmt.Sprintf("Failed to parse pint comment as duration: %s", err), + Details: SeriesCheckMinAgeDetails, + Severity: Warning, + }) + } else { + minAge = time.Duration(dur) } } @@ -661,13 +665,20 @@ func (c SeriesCheck) getMinAge(rule parser.Rule, selector promParser.VectorSelec } func (c SeriesCheck) isLabelValueIgnored(rule parser.Rule, selector promParser.VectorSelector, labelName string) bool { - values := ruleSetIgnoreLabelValValues(c.Reporter(), labelName, selector) for _, ruleSet := range comments.Only[comments.RuleSet](rule.Comments, comments.RuleSetType) { - for _, val := range values { - if ruleSet.Value == val { - return true + matcher, key, value := parseRuleSet(ruleSet.Value) + if key != "ignore/label-value" { + continue + } + if matcher != "" { + isMatch, _ := matchSelectorToMetric(selector, matcher) + if !isMatch { + continue } } + if labelName == value { + return true + } } return false } @@ -686,7 +697,7 @@ func (c SeriesCheck) textAndSeverity(settings *PromqlSeriesSettings, name, text return text, s } -func getSelectors(n *parser.PromQLNode) (selectors []promParser.VectorSelector) { +func getNonFallbackSelectors(n *parser.PromQLNode) (selectors []promParser.VectorSelector) { LOOP: for _, vs := range parser.WalkDownExpr[*promParser.VectorSelector](n) { for _, bin := range parser.WalkUpExpr[*promParser.BinaryExpr](vs.Parent) { @@ -735,26 +746,12 @@ func isDisabled(rule parser.Rule, selector promParser.VectorSelector) bool { 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 - } - // then try matchers - m, err := promParser.ParseMetricSelector(cs) - if err != nil { + isMatch, ok := matchSelectorToMetric(selector, cs) + if !ok { continue } - for _, l := range m { - var isMatch bool - for _, s := range selector.LabelMatchers { - if s.Type == l.Type && s.Name == l.Name && s.Value == l.Value { - isMatch = true - break - } - } - if !isMatch { - goto NEXT - } + if !isMatch { + goto NEXT } return true } @@ -763,6 +760,48 @@ func isDisabled(rule parser.Rule, selector promParser.VectorSelector) bool { return false } +func matchSelectorToMetric(selector promParser.VectorSelector, metric string) (bool, bool) { + // Try full string or name match first. + if metric == selector.String() || metric == selector.Name { + return true, true + } + // Then try matchers. + m, err := promParser.ParseMetricSelector(metric) + if err != nil { + // Ignore errors + return false, false + } + for _, l := range m { + var isMatch bool + for _, s := range selector.LabelMatchers { + if s.Type == l.Type && s.Name == l.Name && s.Value == l.Value { + return true, true + } + } + if !isMatch { + return false, true + } + } + return false, true +} + +func parseRuleSet(s string) (matcher, key, value string) { + if strings.HasPrefix(s, SeriesCheckName+"(") { + matcher = strings.TrimPrefix(s[:strings.LastIndex(s, ")")], SeriesCheckName+"(") + s = s[strings.LastIndex(s, ")")+1:] + } else { + s = strings.TrimPrefix(s, SeriesCheckName) + } + parts := strings.Fields(s) + if len(parts) > 0 { + key = parts[0] + } + if len(parts) > 1 { + value = strings.Join(parts[1:], " ") + } + return matcher, key, value +} + func orphanedDisableComments(ctx context.Context, rule parser.Rule, selectors []promParser.VectorSelector) (orhpaned []comments.Disable) { var promNames, promTags []string if val := ctx.Value(promapi.AllPrometheusServers); val != nil { @@ -787,32 +826,13 @@ func orphanedDisableComments(ctx context.Context, rule parser.Rule, selectors [] continue } for _, selector := range selectors { - // try full string or name match first - if match == selector.String() || match == selector.Name { - wasUsed = true - goto NEXT - } - // then try matchers - m, err := promParser.ParseMetricSelector(match) - if err != nil { + isMatch, ok := matchSelectorToMetric(selector, match) + if !ok { continue } - var isMismatch bool - for _, l := range m { - var isMatch bool - for _, s := range selector.LabelMatchers { - if s.Type == l.Type && s.Name == l.Name && s.Value == l.Value { - isMatch = true - break - } - } - if !isMatch { - isMismatch = true - break - } - } - if !isMismatch { + if isMatch { wasUsed = true + goto NEXT } } NEXT: @@ -823,45 +843,31 @@ func orphanedDisableComments(ctx context.Context, rule parser.Rule, selectors [] return orhpaned } -func ruleSetIgnoreLabelValValues(reporter, labelName string, selector promParser.VectorSelector) []string { - bareSelector := stripLabels(selector) - return []string{ - fmt.Sprintf("%s ignore/label-value %s", reporter, labelName), - fmt.Sprintf("%s(%s) ignore/label-value %s", reporter, bareSelector.String(), labelName), - fmt.Sprintf("%s(%s) ignore/label-value %s", reporter, selector.String(), labelName), - } -} - -func ruleSetMinAgePrefixes(reporter string, selector promParser.VectorSelector) []string { - bareSelector := stripLabels(selector) - return []string{ - reporter + " min-age ", - fmt.Sprintf("%s(%s) min-age ", reporter, bareSelector.String()), - fmt.Sprintf("%s(%s) min-age ", reporter, selector.String()), - } -} - -func orphanedRuleSetComments(reporter string, rule parser.Rule, selectors []promParser.VectorSelector) (orhpaned []comments.RuleSet) { +func orphanedRuleSetComments(rule parser.Rule, selectors []promParser.VectorSelector) (orhpaned []comments.RuleSet) { for _, ruleSet := range comments.Only[comments.RuleSet](rule.Comments, comments.RuleSetType) { - var used bool + var wasUsed bool + matcher, key, value := parseRuleSet(ruleSet.Value) for _, selector := range selectors { - for _, lm := range selector.LabelMatchers { - for _, val := range ruleSetIgnoreLabelValValues(reporter, lm.Name, selector) { - if ruleSet.Value == val { - used = true - goto NEXT - } + if matcher != "" { + isMatch, _ := matchSelectorToMetric(selector, matcher) + if !isMatch { + continue } - for _, val := range ruleSetMinAgePrefixes(reporter, selector) { - if strings.HasPrefix(ruleSet.Value, val) { - used = true + } + switch key { + case "min-age": + wasUsed = true + case "ignore/label-value": + for _, lm := range selector.LabelMatchers { + if lm.Name == value { + wasUsed = true goto NEXT } } } } NEXT: - if !used { + if !wasUsed { orhpaned = append(orhpaned, ruleSet) } } diff --git a/internal/checks/promql_series_test.go b/internal/checks/promql_series_test.go index 509827b6..ce7844b3 100644 --- a/internal/checks/promql_series_test.go +++ b/internal/checks/promql_series_test.go @@ -63,7 +63,7 @@ func metricIgnored(metric, check, re string) string { } func unusedDisableText(m string) string { - return fmt.Sprintf("pint %s comment `%s` check doesn't match any selector in this query", comments.DisableComment, m) + return fmt.Sprintf("pint %s comment `%s` doesn't match any selector in this query", comments.DisableComment, m) } func unusedRuleSetText(m string) string { @@ -1498,6 +1498,7 @@ func TestSeriesCheck(t *testing.T) { description: "#4 metric was present but disappeared / min-age / ok", content: ` - record: foo + # pint rule/set promql/series ignore/label-value instance # pint rule/set promql/series min-age 5d expr: sum(found{job="foo", instance="bar"}) `, @@ -1557,6 +1558,81 @@ func TestSeriesCheck(t *testing.T) { - record: foo # pint rule/set promql/series(found) min-age 5d expr: sum(found{job="foo", instance="bar"}) +`, + checker: newSeriesCheck, + prometheus: newSimpleProm, + problems: noProblems, + mocks: []*prometheusMock{ + { + conds: []requestCondition{ + requireQueryPath, + formCond{key: "query", value: `count(found{instance="bar",job="foo"})`}, + }, + resp: respondWithEmptyVector(), + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: `count(found)`}, + }, + resp: matrixResponse{ + samples: []*model.SampleStream{ + generateSampleStream( + map[string]string{}, + time.Now().Add(time.Hour*24*-7), + time.Now().Add(time.Hour*24*-4).Add(time.Minute*-5), + time.Minute*5, + ), + }, + }, + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: `absent(found{job=~".+"})`}, + }, + resp: matrixResponse{ + samples: []*model.SampleStream{ + generateSampleStream( + map[string]string{}, + time.Now().Add(time.Hour*24*-4).Add(time.Minute*-5), + time.Now(), + time.Minute*5, + ), + }, + }, + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: `absent(found{instance=~".+"})`}, + }, + resp: matrixResponse{ + samples: []*model.SampleStream{ + generateSampleStream( + map[string]string{}, + time.Now().Add(time.Hour*24*-4).Add(time.Minute*-5), + time.Now(), + time.Minute*5, + ), + }, + }, + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: "count(up)"}, + }, + resp: respondWithSingleRangeVector1W(), + }, + }, + }, + { + description: "#4 metric was present but disappeared / min-age with selector / match", + content: ` +- record: foo + # pint rule/set promql/series(found{instance="bar"}) min-age 5d + expr: sum(found{job="foo", instance="bar"}) `, checker: newSeriesCheck, prometheus: newSimpleProm, @@ -2258,6 +2334,118 @@ func TestSeriesCheck(t *testing.T) { }, }, }, + { + description: "#5 ignored label value / selector match", + content: ` +- record: foo + # pint rule/set promql/series(foo{}) ignore/label-value error + expr: sum(foo{error="notfound"}) +`, + checker: newSeriesCheck, + prometheus: newSimpleProm, + problems: noProblems, + mocks: []*prometheusMock{ + { + conds: []requestCondition{ + requireQueryPath, + formCond{key: "query", value: `count(foo{error="notfound"})`}, + }, + resp: respondWithEmptyVector(), + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: `count(foo)`}, + }, + resp: respondWithSingleRangeVector1W(), + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: `absent(foo{error=~".+"})`}, + }, + resp: respondWithEmptyMatrix(), + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: "count(up)"}, + }, + resp: respondWithSingleRangeVector1W(), + }, + }, + }, + { + description: "#5 ignored label value / selector mismatch", + content: ` +- record: foo + # pint rule/set promql/series(foo{job="bob"}) ignore/label-value error + expr: sum(foo{error="notfound"}) +`, + checker: newSeriesCheck, + prometheus: newSimpleProm, + problems: func(uri string) []checks.Problem { + return []checks.Problem{ + { + Lines: parser.LineRange{ + First: 4, + Last: 4, + }, + Reporter: checks.SeriesCheckName, + Text: noFilterMatchText("prom", uri, "foo", "error", `{error="notfound"}`, "1w"), + Details: checks.SeriesCheckCommonProblemDetails, + Severity: checks.Bug, + }, + { + Lines: parser.LineRange{ + First: 4, + Last: 4, + }, + Reporter: checks.SeriesCheckName, + Text: unusedRuleSetText(`promql/series(foo{job="bob"}) ignore/label-value error`), + Details: checks.SeriesCheckUnusedRuleSetComment, + Severity: checks.Warning, + }, + } + }, + mocks: []*prometheusMock{ + { + conds: []requestCondition{ + requireQueryPath, + formCond{key: "query", value: `count(foo{error="notfound"})`}, + }, + resp: respondWithEmptyVector(), + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: `count(foo{error="notfound"})`}, + }, + resp: respondWithEmptyMatrix(), + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: `count(foo)`}, + }, + resp: respondWithSingleRangeVector1W(), + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: `absent(foo{error=~".+"})`}, + }, + resp: respondWithEmptyMatrix(), + }, + { + conds: []requestCondition{ + requireRangeQueryPath, + formCond{key: "query", value: "count(up)"}, + }, + resp: respondWithSingleRangeVector1W(), + }, + }, + }, { description: "#6 metric was always present but label disappeared", content: "- record: foo\n expr: sum({__name__=\"found\", removed=\"xxx\"})\n", @@ -3505,6 +3693,59 @@ func TestSeriesCheck(t *testing.T) { prometheus: newSimpleProm, problems: noProblems, }, + { + description: "disable comment with partial selector", + content: ` +# pint disable promql/series(foo{job="foo"}) +- record: my_series + expr: count(foo{env="prod", job="foo"}) +`, + checker: newSeriesCheck, + prometheus: newSimpleProm, + problems: noProblems, + }, + { + description: "disable comment with vector() fallback", + content: ` +- alert: Foo + # pint disable promql/series(metric1) + # pint disable promql/series(metric2) + # pint disable promql/series(metric3) + expr: | + (rate(metric2[5m]) or vector(0)) + + (rate(metric1[5m]) or vector(0)) + + (rate(metric3{log_name="samplerd"}[5m]) or vector(0)) + > 0 +`, + checker: newSeriesCheck, + prometheus: newSimpleProm, + problems: noProblems, + }, + { + description: "unused rule/set comment", + content: ` +- alert : Foo + # pint rule/set promql/series(mymetric) ignore/label-value action + # pint rule/set promql/series(mymetric) ignore/label-value type + expr: (rate(mymetric{action="failure"}[2m]) or vector(0)) > 0 +`, + checker: newSeriesCheck, + prometheus: newSimpleProm, + problems: func(_ string) []checks.Problem { + return []checks.Problem{ + { + Lines: parser.LineRange{ + First: 5, + Last: 5, + }, + Reporter: checks.SeriesCheckName, + Text: unusedRuleSetText("promql/series(mymetric) ignore/label-value type"), + Details: checks.SeriesCheckUnusedRuleSetComment, + Severity: checks.Warning, + }, + } + }, + }, { description: "disable comment for a mismatched series", content: ` diff --git a/internal/parser/read.go b/internal/parser/read.go index 373b7cec..13c70986 100644 --- a/internal/parser/read.go +++ b/internal/parser/read.go @@ -20,7 +20,7 @@ const ( skipFile ) -func emptyLine(line string, comments []comments.Comment) string { +func emptyLine(line string, comments []comments.Comment, stripComments bool) string { offset := len(line) for _, c := range comments { offset = c.Offset @@ -32,7 +32,7 @@ func emptyLine(line string, comments []comments.Comment) string { switch { case r == '\n': buf.WriteRune(r) - case i < offset: + case i < offset || stripComments: buf.WriteRune(' ') default: buf.WriteRune(r) @@ -77,7 +77,7 @@ func ReadContent(r io.Reader) (out Content, fileComments []comments.Comment, err lineComments = comments.Parse(lineno, line) if skipAll { - out.Body = append(out.Body, []byte(emptyLine(line, lineComments))...) + out.Body = append(out.Body, []byte(emptyLine(line, lineComments, inBegin))...) } else { skip = skipNone found = false @@ -125,12 +125,12 @@ func ReadContent(r io.Reader) (out Content, fileComments []comments.Comment, err case skipFile: out.Ignored = true out.IgnoreLine = lineno - out.Body = append(out.Body, []byte(emptyLine(line, lineComments))...) + out.Body = append(out.Body, []byte(emptyLine(line, lineComments, inBegin))...) skipNext = true autoReset = false skipAll = true case skipCurrentLine: - out.Body = append(out.Body, []byte(emptyLine(line, lineComments))...) + out.Body = append(out.Body, []byte(emptyLine(line, lineComments, inBegin))...) if !inBegin { skipNext = false autoReset = true @@ -151,7 +151,7 @@ func ReadContent(r io.Reader) (out Content, fileComments []comments.Comment, err inBegin = false } case skipNext: - out.Body = append(out.Body, []byte(emptyLine(line, lineComments))...) + out.Body = append(out.Body, []byte(emptyLine(line, lineComments, inBegin))...) if autoReset { skipNext = false } diff --git a/internal/parser/read_test.go b/internal/parser/read_test.go index bed4c8d0..7cc070c8 100644 --- a/internal/parser/read_test.go +++ b/internal/parser/read_test.go @@ -113,7 +113,7 @@ func TestReadContent(t *testing.T) { }, { input: []byte("# pint ignore/begin\nfoo # pint ignore/line\nbar\n# pint ignore/begin"), - output: []byte("# pint ignore/begin\n # pint ignore/line\n \n# pint ignore/begin"), + output: []byte("# pint ignore/begin\n \n \n# pint ignore/begin"), }, { input: []byte("line1\nline2 # pint ignore/line\n"), @@ -159,6 +159,10 @@ func TestReadContent(t *testing.T) { input: []byte("{#- hide this comment -#} # pint ignore/line\n"), output: []byte(" # pint ignore/line\n"), }, + { + input: []byte("# pint ignore/begin\n - alert: Ignored\n # pint rule/set foo\n # pint rule/set bar\n expr: up\n# pint ignore/end\n"), + output: []byte("# pint ignore/begin\n \n \n \n \n# pint ignore/end\n"), + }, } cmpErrorText := cmp.Comparer(func(x, y interface{}) bool { diff --git a/internal/parser/utils/vectorselector.go b/internal/parser/utils/vectorselector.go index be1a1a40..1813dd67 100644 --- a/internal/parser/utils/vectorselector.go +++ b/internal/parser/utils/vectorselector.go @@ -6,9 +6,9 @@ import ( promParser "github.com/prometheus/prometheus/promql/parser" ) -func HasVectorSelector(node *parser.PromQLNode) (vs []*promParser.VectorSelector) { +func HasVectorSelector(node *parser.PromQLNode) (vs []promParser.VectorSelector) { if n, ok := node.Expr.(*promParser.VectorSelector); ok { - vs = append(vs, n) + vs = append(vs, *n) } for _, child := range node.Children {