Skip to content

Commit

Permalink
Merge pull request #1212 from cloudflare/thanos
Browse files Browse the repository at this point in the history
Fix thanos schema
  • Loading branch information
prymitive authored Dec 4, 2024
2 parents cfc56f9 + 64c3b3e commit 9f4ebbd
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 141 deletions.
10 changes: 5 additions & 5 deletions cmd/pint/tests/0203_parser_schema_prom_err.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,23 @@ 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=WARN msg="Failed to parse file content" err="error at line 7: partial_response_strategy is only valid when parser is configured to use the Thanos rule schema" path=rules/1.yml lines=1-9
rules/1.yml:7 Fatal: partial_response_strategy is only valid when parser is configured to use the Thanos rule schema (yaml/parse)
7 | partial_response_strategy: warn
level=WARN msg="Failed to parse file content" err="error at line 3: partial_response_strategy is only valid when parser is configured to use the Thanos rule schema" path=rules/1.yml lines=1-9
rules/1.yml:3 Fatal: partial_response_strategy is only valid when parser is configured to use the Thanos rule schema (yaml/parse)
3 | partial_response_strategy: warn

level=INFO msg="Problems found" Fatal=1
level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher"
-- rules/1.yml --
groups:
- name: foo
partial_response_strategy: warn
rules:
- alert: foo
expr: up == 0
- record: bar
partial_response_strategy: warn
expr: sum(up)

-- .pint.hcl --
parser {
schema = "prometheus"
}
}
7 changes: 4 additions & 3 deletions cmd/pint/tests/0204_parser_schema_thanos_err.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,20 @@ 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"]
rules/1.yml:7 Fatal: This rule is not a valid Prometheus rule: `invalid partial_response_strategy value: bob`. (yaml/parse)
7 | partial_response_strategy: bob
level=WARN msg="Failed to parse file content" err="error at line 3: invalid partial_response_strategy value: bob" path=rules/1.yml lines=1-9
rules/1.yml:3 Fatal: invalid partial_response_strategy value: bob (yaml/parse)
3 | partial_response_strategy: bob

level=INFO msg="Problems found" Fatal=1
level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher"
-- rules/1.yml --
groups:
- name: foo
partial_response_strategy: bob
rules:
- alert: foo
expr: up == 0
- record: bar
partial_response_strategy: bob
expr: sum(up)

-- .pint.hcl --
Expand Down
9 changes: 7 additions & 2 deletions cmd/pint/tests/0205_parser_schema_thanos_ok.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@ level=INFO msg="Finding all rules to check" paths=["rules"]
-- rules/1.yml --
groups:
- name: foo
partial_response_strategy: warn
rules:
- alert: foo
partial_response_strategy: warn
expr: up == 0

-- rules/2.yml --
groups:
- name: foo
partial_response_strategy: abort
rules:
- record: bar
partial_response_strategy: abort
expr: sum(up)

-- .pint.hcl --
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0206_parser_schema_err.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ level=ERROR msg="Fatal error" err="failed to load config file \".pint.hcl\": uns
-- rules/1.yml --
groups:
- name: foo
partial_response_strategy: bob
rules:
- alert: foo
expr: up == 0
- record: bar
partial_response_strategy: bob
expr: sum(up)

-- .pint.hcl --
Expand Down
43 changes: 3 additions & 40 deletions internal/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,13 @@ import (
)

const (
// Standard Prometheus fields.
recordKey = "record"
exprKey = "expr"
labelsKey = "labels"
alertKey = "alert"
forKey = "for"
keepFiringForKey = "keep_firing_for"
annotationsKey = "annotations"

// Thanos Rule specific fields.
partialResponseStrategyKey = "partial_response_strategy"
)

var ErrRuleCommentOnFile = errors.New("this comment is only valid when attached to a rule")
Expand Down Expand Up @@ -100,7 +96,7 @@ To allow for multi-document YAML files set parser->relaxed option in pint config
}

func parseNode(content []byte, node *yaml.Node, offset int, schema Schema) (rules []Rule) {
ret, isEmpty := parseRule(content, node, offset, schema)
ret, isEmpty := parseRule(content, node, offset)
if !isEmpty {
rules = append(rules, ret)
return rules
Expand All @@ -115,7 +111,7 @@ func parseNode(content []byte, node *yaml.Node, offset int, schema Schema) (rule
rules = append(rules, parseNode(content, n, offset, schema)...)
}
case yaml.MappingNode:
rule, isEmpty = parseRule(content, root, offset, schema)
rule, isEmpty = parseRule(content, root, offset)
if !isEmpty {
rules = append(rules, rule)
} else {
Expand All @@ -136,7 +132,7 @@ func parseNode(content []byte, node *yaml.Node, offset int, schema Schema) (rule
return rules
}

func parseRule(content []byte, node *yaml.Node, offset int, schema Schema) (rule Rule, _ bool) {
func parseRule(content []byte, node *yaml.Node, offset int) (rule Rule, _ bool) {
if node.Kind != yaml.MappingNode {
return rule, true
}
Expand All @@ -149,7 +145,6 @@ func parseRule(content []byte, node *yaml.Node, offset int, schema Schema) (rule
var forPart *YamlNode
var keepFiringForPart *YamlNode
var annotationsPart *YamlMap
var partialResponseStrategyPart *YamlNode

var recordNode *yaml.Node
var alertNode *yaml.Node
Expand All @@ -158,7 +153,6 @@ func parseRule(content []byte, node *yaml.Node, offset int, schema Schema) (rule
var keepFiringForNode *yaml.Node
var labelsNode *yaml.Node
var annotationsNode *yaml.Node
var partialResponseStrategyNode *yaml.Node

labelsNodes := []yamlMap{}
annotationsNodes := []yamlMap{}
Expand Down Expand Up @@ -248,18 +242,6 @@ func parseRule(content []byte, node *yaml.Node, offset int, schema Schema) (rule
annotationsNodes = mappingNodes(part)
annotationsPart = newYamlMap(key, part, offset)
lines.Last = max(lines.Last, annotationsPart.Lines.Last)
case partialResponseStrategyKey:
if schema != ThanosSchema {
unknownKeys = append(unknownKeys, key)
continue
}

if partialResponseStrategyPart != nil {
return duplicatedKeyError(lines, part.Line+offset, partialResponseStrategyKey)
}
partialResponseStrategyNode = part
partialResponseStrategyPart = newYamlNodeWithKey(key, part, offset)
lines.Last = max(lines.Last, partialResponseStrategyPart.Lines.Last)
default:
unknownKeys = append(unknownKeys, key)
}
Expand Down Expand Up @@ -458,25 +440,6 @@ func parseRule(content []byte, node *yaml.Node, offset int, schema Schema) (rule
}
}

if partialResponseStrategyPart != nil && partialResponseStrategyNode != nil {
val := nodeValue(partialResponseStrategyNode)
if !isTag(partialResponseStrategyNode.ShortTag(), strTag) {
return invalidValueError(lines, partialResponseStrategyNode.Line+offset, partialResponseStrategyKey, describeTag(strTag), describeTag(partialResponseStrategyNode.ShortTag()))
}
switch val {
case "warn":
case "abort":
default:
return Rule{
Lines: lines,
Error: ParseError{
Line: partialResponseStrategyPart.Lines.First,
Err: fmt.Errorf("invalid %s value: %s", partialResponseStrategyKey, val),
},
}, false
}
}

if recordPart != nil && exprPart != nil {
rule = Rule{
Lines: lines,
Expand Down
104 changes: 24 additions & 80 deletions internal/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2885,36 +2885,36 @@ groups:
content: []byte(`
groups:
- name: mygroup
partial_response_strategy: bob
rules:
- record: up:count
expr: count(up)
partial_response_strategy: bob
`),
strict: true,
err: "error at line 7: partial_response_strategy is only valid when parser is configured to use the Thanos rule schema",
err: "error at line 4: partial_response_strategy is only valid when parser is configured to use the Thanos rule schema",
},
{
content: []byte(`
groups:
- name: mygroup
partial_response_strategy: warn
rules:
- record: up:count
expr: count(up)
partial_response_strategy: warn
`),
strict: true,
schema: parser.ThanosSchema,
output: []parser.Rule{
{
Lines: parser.LineRange{First: 5, Last: 7},
Lines: parser.LineRange{First: 6, Last: 7},
RecordingRule: &parser.RecordingRule{
Record: parser.YamlNode{
Lines: parser.LineRange{First: 5, Last: 5},
Lines: parser.LineRange{First: 6, Last: 6},
Value: "up:count",
},
Expr: parser.PromQLExpr{
Value: &parser.YamlNode{
Lines: parser.LineRange{First: 6, Last: 6},
Lines: parser.LineRange{First: 7, Last: 7},
Value: "count(up)",
},
},
Expand All @@ -2926,49 +2926,24 @@ groups:
content: []byte(`
groups:
- name: mygroup
partial_response_strategy: abort
rules:
- record: up:count
expr: count(up)
partial_response_strategy: abort
`),
strict: true,
schema: parser.ThanosSchema,
output: []parser.Rule{
{
Lines: parser.LineRange{First: 5, Last: 7},
RecordingRule: &parser.RecordingRule{
Record: parser.YamlNode{
Lines: parser.LineRange{First: 5, Last: 5},
Value: "up:count",
},
Expr: parser.PromQLExpr{
Value: &parser.YamlNode{
Lines: parser.LineRange{First: 6, Last: 6},
Value: "count(up)",
},
},
},
},
},
},
{
content: []byte(`
- record: up:count
expr: count(up)
partial_response_strategy: warn
`),
schema: parser.ThanosSchema,
output: []parser.Rule{
{
Lines: parser.LineRange{First: 2, Last: 4},
Lines: parser.LineRange{First: 6, Last: 7},
RecordingRule: &parser.RecordingRule{
Record: parser.YamlNode{
Lines: parser.LineRange{First: 2, Last: 2},
Lines: parser.LineRange{First: 6, Last: 6},
Value: "up:count",
},
Expr: parser.PromQLExpr{
Value: &parser.YamlNode{
Lines: parser.LineRange{First: 3, Last: 3},
Lines: parser.LineRange{First: 7, Last: 7},
Value: "count(up)",
},
},
Expand All @@ -2978,22 +2953,26 @@ groups:
},
{
content: []byte(`
- record: up:count
expr: count(up)
groups:
- name: mygroup
partial_response_strategy: abort
rules:
- record: up:count
expr: count(up)
`),
schema: parser.ThanosSchema,
strict: false,
schema: parser.PrometheusSchema,
output: []parser.Rule{
{
Lines: parser.LineRange{First: 2, Last: 4},
Lines: parser.LineRange{First: 6, Last: 7},
RecordingRule: &parser.RecordingRule{
Record: parser.YamlNode{
Lines: parser.LineRange{First: 2, Last: 2},
Lines: parser.LineRange{First: 6, Last: 6},
Value: "up:count",
},
Expr: parser.PromQLExpr{
Value: &parser.YamlNode{
Lines: parser.LineRange{First: 3, Last: 3},
Lines: parser.LineRange{First: 7, Last: 7},
Value: "count(up)",
},
},
Expand All @@ -3005,62 +2984,27 @@ groups:
content: []byte(`
groups:
- name: mygroup
partial_response_strategy: bob
rules:
- record: up:count
expr: count(up)
partial_response_strategy: bob
`),
strict: true,
schema: parser.ThanosSchema,
output: []parser.Rule{
{
Lines: parser.LineRange{First: 5, Last: 7},
Error: parser.ParseError{
Line: 7,
Err: errors.New("invalid partial_response_strategy value: bob"),
},
},
},
err: "error at line 4: invalid partial_response_strategy value: bob",
},
{
content: []byte(`
groups:
- name: mygroup
partial_response_strategy: 1
rules:
- record: up:count
expr: count(up)
partial_response_strategy: 1
`),
strict: true,
schema: parser.ThanosSchema,
output: []parser.Rule{
{
Lines: parser.LineRange{First: 5, Last: 7},
Error: parser.ParseError{
Line: 7,
Err: errors.New("partial_response_strategy value must be a string, got integer instead"),
},
},
},
},
{
content: []byte("- record: foo\n expr: foo\n partial_response_strategy: true\n"),
output: []parser.Rule{
{
Lines: parser.LineRange{First: 1, Last: 3},
Error: parser.ParseError{Err: errors.New("invalid key(s) found: partial_response_strategy"), Line: 3},
},
},
},
{
content: []byte("- record: foo\n expr: foo\n partial_response_strategy: true\n partial_response_strategy: false\n"),
schema: parser.ThanosSchema,
output: []parser.Rule{
{
Lines: parser.LineRange{First: 1, Last: 4},
Error: parser.ParseError{Err: errors.New("duplicated partial_response_strategy key"), Line: 4},
},
},
err: "error at line 4: partial_response_strategy must be a string, got integer",
},
}

Expand Down
Loading

0 comments on commit 9f4ebbd

Please sign in to comment.