Skip to content

Commit 23d5cf9

Browse files
committed
Adding parser for xfunctions in rule validation
Signed-off-by: Paurush Garg <[email protected]>
1 parent 216697b commit 23d5cf9

File tree

2 files changed

+86
-11
lines changed

2 files changed

+86
-11
lines changed

pkg/ruler/manager.go

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"io"
77
"net/http"
88
"os"
9+
"strings"
910
"sync"
1011
"time"
1112

@@ -24,6 +25,7 @@ import (
2425
"github.com/weaveworks/common/user"
2526
"golang.org/x/net/context/ctxhttp"
2627

28+
"github.com/cortexproject/cortex/pkg/parser"
2729
"github.com/cortexproject/cortex/pkg/ring/client"
2830
"github.com/cortexproject/cortex/pkg/ruler/rulespb"
2931
)
@@ -457,19 +459,41 @@ func (*DefaultMultiTenantManager) ValidateRuleGroup(g rulefmt.RuleGroup) []error
457459
}
458460

459461
for i, r := range g.Rules {
462+
// Use Cortex parser for expression validation (supports XFunctions)
463+
if r.Expr != "" {
464+
if _, err := parser.ParseExpr(r.Expr); err != nil {
465+
var ruleName string
466+
if r.Alert != "" {
467+
ruleName = r.Alert
468+
} else {
469+
ruleName = r.Record
470+
}
471+
errs = append(errs, &rulefmt.Error{
472+
Group: g.Name,
473+
Rule: i,
474+
RuleName: ruleName,
475+
Err: rulefmt.WrappedError{},
476+
})
477+
}
478+
}
479+
480+
// Validate other rule fields using Prometheus validation
460481
for _, err := range r.Validate(rulefmt.RuleNode{}) {
461-
var ruleName string
462-
if r.Alert != "" {
463-
ruleName = r.Alert
464-
} else {
465-
ruleName = r.Record
482+
// Skip expression validation errors since we handle them above
483+
if !strings.Contains(err.Error(), "could not parse expression") {
484+
var ruleName string
485+
if r.Alert != "" {
486+
ruleName = r.Alert
487+
} else {
488+
ruleName = r.Record
489+
}
490+
errs = append(errs, &rulefmt.Error{
491+
Group: g.Name,
492+
Rule: i,
493+
RuleName: ruleName,
494+
Err: err,
495+
})
466496
}
467-
errs = append(errs, &rulefmt.Error{
468-
Group: g.Name,
469-
Rule: i,
470-
RuleName: ruleName,
471-
Err: err,
472-
})
473497
}
474498
}
475499

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package ruler
2+
3+
import (
4+
"testing"
5+
6+
"github.com/prometheus/prometheus/model/rulefmt"
7+
)
8+
9+
func TestValidateRuleGroup_AcceptsXFunctions(t *testing.T) {
10+
manager := &DefaultMultiTenantManager{}
11+
12+
// Test rule with XFunction (should now pass)
13+
ruleGroupWithXFunc := rulefmt.RuleGroup{
14+
Name: "test_group",
15+
Rules: []rulefmt.Rule{
16+
{
17+
Alert: "TestAlert",
18+
Expr: "xrate(cpu_usage[5m]) > 0.8", // XFunction
19+
},
20+
},
21+
}
22+
23+
errs := manager.ValidateRuleGroup(ruleGroupWithXFunc)
24+
25+
// Should have no validation errors now
26+
if len(errs) != 0 {
27+
t.Fatalf("Expected no validation errors for XFunction after fix, got: %v", errs)
28+
}
29+
}
30+
31+
func TestValidateRuleGroup_AcceptsStandardFunctions(t *testing.T) {
32+
manager := &DefaultMultiTenantManager{}
33+
34+
// Test rule with standard function (should pass)
35+
ruleGroupStandard := rulefmt.RuleGroup{
36+
Name: "test_group",
37+
Rules: []rulefmt.Rule{
38+
{
39+
Alert: "TestAlert",
40+
Expr: "rate(cpu_usage[5m]) > 0.8", // Standard function
41+
},
42+
},
43+
}
44+
45+
errs := manager.ValidateRuleGroup(ruleGroupStandard)
46+
47+
// Should have no validation errors
48+
if len(errs) != 0 {
49+
t.Fatalf("Expected no validation errors for standard function, got: %v", errs)
50+
}
51+
}

0 commit comments

Comments
 (0)