Skip to content

Commit f494181

Browse files
prymitiveprymitive
committed
Delay parsing of PromQL until it's needed
1 parent f715ce3 commit f494181

28 files changed

+406
-190
lines changed

‎internal/checks/alerts_absent.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"github.com/cloudflare/pint/internal/diags"
1010
"github.com/cloudflare/pint/internal/discovery"
1111
"github.com/cloudflare/pint/internal/output"
12-
"github.com/cloudflare/pint/internal/parser/utils"
12+
"github.com/cloudflare/pint/internal/parser/source"
1313
"github.com/cloudflare/pint/internal/promapi"
1414

1515
"github.com/prometheus/common/model"
@@ -59,12 +59,12 @@ func (c AlertsAbsentCheck) Check(ctx context.Context, entry *discovery.Entry, _
5959
return problems
6060
}
6161

62-
if entry.Rule.AlertingRule.Expr.SyntaxError != nil {
62+
if entry.Rule.AlertingRule.Expr.SyntaxError() != nil {
6363
return problems
6464
}
6565

66-
src := utils.LabelsSource(entry.Rule.AlertingRule.Expr.Value.Value, entry.Rule.AlertingRule.Expr.Query.Expr)
67-
absentSources := make([]utils.Source, 0, len(src))
66+
src := entry.Rule.AlertingRule.Expr.Source()
67+
absentSources := make([]source.Source, 0, len(src))
6868
for _, s := range src {
6969
if s.Operation() != "absent" {
7070
continue
@@ -101,7 +101,7 @@ func (c AlertsAbsentCheck) Check(ctx context.Context, entry *discovery.Entry, _
101101
for _, s := range absentSources {
102102
var summary string
103103

104-
call, _ := utils.MostOuterOperation[*promParser.Call](s)
104+
call, _ := source.MostOuterOperation[*promParser.Call](s)
105105
dgs := []diags.Diagnostic{
106106
{
107107
Message: "Using `absent()` might cause false positive alerts when Prometheus restarts.",

‎internal/checks/alerts_comparison.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55

66
"github.com/cloudflare/pint/internal/diags"
77
"github.com/cloudflare/pint/internal/discovery"
8-
"github.com/cloudflare/pint/internal/parser/utils"
98
)
109

1110
const (
@@ -49,12 +48,12 @@ func (c ComparisonCheck) Check(_ context.Context, entry *discovery.Entry, _ []*d
4948
return problems
5049
}
5150

52-
if entry.Rule.AlertingRule.Expr.SyntaxError != nil {
51+
if entry.Rule.AlertingRule.Expr.SyntaxError() != nil {
5352
return problems
5453
}
5554

5655
expr := entry.Rule.Expr()
57-
srcs := utils.LabelsSource(expr.Value.Value, expr.Query.Expr)
56+
srcs := expr.Source()
5857
var msg string
5958
for _, src := range srcs {
6059
if src.DeadInfo != nil {

‎internal/checks/alerts_count.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func (c AlertsCheck) Check(ctx context.Context, entry *discovery.Entry, _ []*dis
6969
return problems
7070
}
7171

72-
if entry.Rule.AlertingRule.Expr.SyntaxError != nil {
72+
if entry.Rule.AlertingRule.Expr.SyntaxError() != nil {
7373
return problems
7474
}
7575

‎internal/checks/alerts_external_labels.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (c AlertsExternalLabelsCheck) Check(ctx context.Context, entry *discovery.E
5252
return problems
5353
}
5454

55-
if entry.Rule.AlertingRule.Expr.SyntaxError != nil {
55+
if entry.Rule.AlertingRule.Expr.SyntaxError() != nil {
5656
return problems
5757
}
5858

‎internal/checks/alerts_template.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"github.com/cloudflare/pint/internal/diags"
2121
"github.com/cloudflare/pint/internal/discovery"
2222
"github.com/cloudflare/pint/internal/parser"
23-
"github.com/cloudflare/pint/internal/parser/utils"
23+
"github.com/cloudflare/pint/internal/parser/source"
2424
)
2525

2626
const (
@@ -117,11 +117,11 @@ func (c TemplateCheck) Check(ctx context.Context, entry *discovery.Entry, _ []*d
117117
return nil
118118
}
119119

120-
if entry.Rule.AlertingRule.Expr.SyntaxError != nil {
120+
if entry.Rule.AlertingRule.Expr.SyntaxError() != nil {
121121
return nil
122122
}
123123

124-
src := utils.LabelsSource(entry.Rule.AlertingRule.Expr.Value.Value, entry.Rule.AlertingRule.Expr.Query.Expr)
124+
src := entry.Rule.AlertingRule.Expr.Source()
125125
data := promTemplate.AlertTemplateData(map[string]string{}, map[string]string{}, "", promql.Sample{})
126126

127127
for _, label := range entry.Labels().Items {
@@ -216,7 +216,7 @@ func (c TemplateCheck) checkHumanizeIsNeeded(expr parser.PromQLExpr, ann *parser
216216
if !ok {
217217
return problems
218218
}
219-
for _, src := range utils.LabelsSource(expr.Value.Value, expr.Query.Expr) {
219+
for _, src := range expr.Source() {
220220
call := isRateResult(src)
221221
if call != nil {
222222
dgs := []diags.Diagnostic{
@@ -260,15 +260,15 @@ func (c TemplateCheck) checkHumanizeIsNeeded(expr parser.PromQLExpr, ann *parser
260260
return problems
261261
}
262262

263-
func isRateResult(src utils.Source) *promParser.Call {
264-
if src.Type == utils.AggregateSource {
263+
func isRateResult(src source.Source) *promParser.Call {
264+
if src.Type == source.AggregateSource {
265265
switch src.Operation() {
266266
case "count", "count_values", "group":
267267
return nil
268268
}
269269
}
270270

271-
call, ok := utils.MostOuterOperation[*promParser.Call](src)
271+
call, ok := source.MostOuterOperation[*promParser.Call](src)
272272
if !ok {
273273
return nil
274274
}
@@ -502,7 +502,7 @@ func findTemplateVariables(_, text string) (vars []tmplVar, aliases aliasMap, ok
502502
return vars, aliases, true
503503
}
504504

505-
func (c TemplateCheck) checkQueryLabels(group *parser.Group, rule parser.Rule, label *parser.YamlKeyValue, src []utils.Source) (problems []Problem) {
505+
func (c TemplateCheck) checkQueryLabels(group *parser.Group, rule parser.Rule, label *parser.YamlKeyValue, src []source.Source) (problems []Problem) {
506506
vars, aliases, ok := findTemplateVariables(label.Key.Value, label.Value.Value)
507507
if !ok {
508508
return nil

‎internal/checks/labels_conflict.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@ func (c LabelsConflictCheck) Reporter() string {
5050

5151
func (c LabelsConflictCheck) Check(ctx context.Context, entry *discovery.Entry, _ []*discovery.Entry) (problems []Problem) {
5252
var labels *parser.YamlMap
53-
if entry.Rule.AlertingRule != nil && entry.Rule.AlertingRule.Expr.SyntaxError == nil && entry.Rule.AlertingRule.Labels != nil {
53+
if entry.Rule.AlertingRule != nil && entry.Rule.AlertingRule.Expr.SyntaxError() == nil && entry.Rule.AlertingRule.Labels != nil {
5454
labels = entry.Rule.AlertingRule.Labels
5555
}
56-
if entry.Rule.RecordingRule != nil && entry.Rule.RecordingRule.Expr.SyntaxError == nil && entry.Rule.RecordingRule.Labels != nil {
56+
if entry.Rule.RecordingRule != nil && entry.Rule.RecordingRule.Expr.SyntaxError() == nil && entry.Rule.RecordingRule.Labels != nil {
5757
labels = entry.Rule.RecordingRule.Labels
5858
}
5959
if labels == nil {

‎internal/checks/promql_aggregation.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88

99
"github.com/cloudflare/pint/internal/diags"
1010
"github.com/cloudflare/pint/internal/discovery"
11-
"github.com/cloudflare/pint/internal/parser/utils"
11+
"github.com/cloudflare/pint/internal/parser/source"
1212
)
1313

1414
const (
@@ -58,7 +58,7 @@ func (c AggregationCheck) Reporter() string {
5858

5959
func (c AggregationCheck) Check(_ context.Context, entry *discovery.Entry, _ []*discovery.Entry) (problems []Problem) {
6060
expr := entry.Rule.Expr()
61-
if expr.SyntaxError != nil {
61+
if expr.SyntaxError() != nil {
6262
return nil
6363
}
6464

@@ -88,8 +88,8 @@ func (c AggregationCheck) Check(_ context.Context, entry *discovery.Entry, _ []*
8888
nameDesc = "all"
8989
}
9090

91-
for _, src := range utils.LabelsSource(expr.Value.Value, expr.Query.Expr) {
92-
if src.Type != utils.AggregateSource {
91+
for _, src := range expr.Source() {
92+
if src.Type != source.AggregateSource {
9393
continue
9494
}
9595
if c.keep && !src.CanHaveLabel(c.label) {
@@ -122,13 +122,13 @@ func (c AggregationCheck) Check(_ context.Context, entry *discovery.Entry, _ []*
122122
}
123123
if !c.keep && src.CanHaveLabel(c.label) {
124124
posrange := src.Position
125-
if aggr, ok := utils.MostOuterOperation[*promParser.AggregateExpr](src); ok {
125+
if aggr, ok := source.MostOuterOperation[*promParser.AggregateExpr](src); ok {
126126
posrange = aggr.PosRange
127127
if len(aggr.Grouping) != 0 {
128128
if aggr.Without {
129-
posrange = utils.FindFuncNamePosition(expr.Value.Value, aggr.PosRange, "without")
129+
posrange = source.FindFuncNamePosition(expr.Value.Value, aggr.PosRange, "without")
130130
} else {
131-
posrange = utils.FindFuncNamePosition(expr.Value.Value, aggr.PosRange, "by")
131+
posrange = source.FindFuncNamePosition(expr.Value.Value, aggr.PosRange, "by")
132132
}
133133
}
134134
}

‎internal/checks/promql_counter.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ func (c CounterCheck) Reporter() string {
5858
func (c CounterCheck) Check(ctx context.Context, entry *discovery.Entry, _ []*discovery.Entry) (problems []Problem) {
5959
expr := entry.Rule.Expr()
6060

61-
if expr.SyntaxError != nil {
61+
if expr.SyntaxError() != nil {
6262
return problems
6363
}
6464

6565
done := map[string]struct{}{}
6666

6767
LOOP:
68-
for _, vs := range parser.WalkDownExpr[*promParser.VectorSelector](expr.Query) {
68+
for _, vs := range parser.WalkDownExpr[*promParser.VectorSelector](expr.Query()) {
6969
if vs.Parent == nil {
7070
// This might be a counter but there's no parent so we have something like `expr: foo`.
7171
// We're only testing for the existence of foo in alerts OR copying it via recording rules.

‎internal/checks/promql_fragile.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"github.com/cloudflare/pint/internal/diags"
1212
"github.com/cloudflare/pint/internal/discovery"
1313
"github.com/cloudflare/pint/internal/parser"
14-
"github.com/cloudflare/pint/internal/parser/utils"
14+
"github.com/cloudflare/pint/internal/parser/source"
1515
)
1616

1717
const (
@@ -57,12 +57,12 @@ func (c FragileCheck) Reporter() string {
5757

5858
func (c FragileCheck) Check(_ context.Context, entry *discovery.Entry, _ []*discovery.Entry) (problems []Problem) {
5959
expr := entry.Rule.Expr()
60-
if expr.SyntaxError != nil {
60+
if expr.SyntaxError() != nil {
6161
return nil
6262
}
6363

6464
if entry.Rule.AlertingRule != nil {
65-
for _, src := range utils.LabelsSource(expr.Value.Value, expr.Query.Expr) {
65+
for _, src := range expr.Source() {
6666
problems = append(problems, c.checkTopK(expr, src)...)
6767
problems = append(problems, c.checkPartialData(expr, src, entry.Rule.AlertingRule.For)...)
6868
}
@@ -71,11 +71,11 @@ func (c FragileCheck) Check(_ context.Context, entry *discovery.Entry, _ []*disc
7171
return problems
7272
}
7373

74-
func (c FragileCheck) checkTopK(expr parser.PromQLExpr, src utils.Source) (problems []Problem) {
75-
if src.Type != utils.AggregateSource {
74+
func (c FragileCheck) checkTopK(expr *parser.PromQLExpr, src source.Source) (problems []Problem) {
75+
if src.Type != source.AggregateSource {
7676
return problems
7777
}
78-
if src.FixedLabels && len(src.TransformedLabels(utils.PossibleLabel)) == 0 {
78+
if src.FixedLabels && len(src.TransformedLabels(source.PossibleLabel)) == 0 {
7979
return problems
8080
}
8181
if !slices.Contains([]string{"topk", "bottomk", "limit", "limit_ratio"}, src.Operation()) {
@@ -101,8 +101,8 @@ func (c FragileCheck) checkTopK(expr parser.PromQLExpr, src utils.Source) (probl
101101
return problems
102102
}
103103

104-
func (c FragileCheck) checkPartialData(expr parser.PromQLExpr, src utils.Source, forVal *parser.YamlNode) (problems []Problem) {
105-
if src.Type != utils.AggregateSource {
104+
func (c FragileCheck) checkPartialData(expr *parser.PromQLExpr, src source.Source, forVal *parser.YamlNode) (problems []Problem) {
105+
if src.Type != source.AggregateSource {
106106
return problems
107107
}
108108
if !src.IsConditional {
@@ -118,7 +118,7 @@ func (c FragileCheck) checkPartialData(expr parser.PromQLExpr, src utils.Source,
118118

119119
for _, j := range src.Joins {
120120
// Only look for joins that are aggregations.
121-
if j.Src.Type != utils.AggregateSource {
121+
if j.Src.Type != source.AggregateSource {
122122
continue
123123
}
124124
// Ignore joins that are not conditional and instead are used to add labels.

‎internal/checks/promql_impossible.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"github.com/cloudflare/pint/internal/diags"
77
"github.com/cloudflare/pint/internal/discovery"
88
"github.com/cloudflare/pint/internal/parser"
9-
"github.com/cloudflare/pint/internal/parser/utils"
9+
"github.com/cloudflare/pint/internal/parser/source"
1010
)
1111

1212
const (
@@ -42,20 +42,20 @@ func (c ImpossibleCheck) Reporter() string {
4242

4343
func (c ImpossibleCheck) Check(_ context.Context, entry *discovery.Entry, _ []*discovery.Entry) (problems []Problem) {
4444
expr := entry.Rule.Expr()
45-
if expr.SyntaxError != nil {
45+
if expr.SyntaxError() != nil {
4646
return problems
4747
}
4848

49-
for _, src := range utils.LabelsSource(expr.Value.Value, expr.Query.Expr) {
50-
src.WalkSources(func(s utils.Source, _ *utils.Join, _ *utils.Unless) {
49+
for _, src := range expr.Source() {
50+
src.WalkSources(func(s source.Source, _ *source.Join, _ *source.Unless) {
5151
problems = append(problems, c.checkSource(expr, s)...)
5252
})
5353
}
5454

5555
return problems
5656
}
5757

58-
func (c ImpossibleCheck) checkSource(expr parser.PromQLExpr, s utils.Source) (problems []Problem) {
58+
func (c ImpossibleCheck) checkSource(expr *parser.PromQLExpr, s source.Source) (problems []Problem) {
5959
if s.DeadInfo != nil {
6060
problems = append(problems, Problem{
6161
Anchor: AnchorAfter,

0 commit comments

Comments
 (0)