fix: parse expression error message based on record or alert (#10065)

CHANGELOG.md

@@ -50,6 +50,7 @@
 
 ##### Fixes
 
+* [10065](https://github.com/grafana/loki/pull/10065) **fgouteroux**: Fix the syntax error message when parsing expression rule.
 * [8979](https://github.com/grafana/loki/pull/8979) **slim-bean**: Fix the case where a logs query with start time == end time was returning logs when none should be returned.
 * [9099](https://github.com/grafana/loki/pull/9099) **salvacorts**: Fix the estimated size of chunks when writing a new TSDB file during compaction.
 * [9130](https://github.com/grafana/loki/pull/9130) **salvacorts**: Pass LogQL engine options down to the _split by range_, _sharding_, and _query size limiter_ middlewares.

pkg/ruler/compat.go

@@ -249,7 +249,10 @@ func validateRuleNode(r *rulefmt.RuleNode, groupName string) error {
 	if r.Expr.Value == "" {
 		return errors.Errorf("field 'expr' must be set in rule")
 	} else if _, err := syntax.ParseExpr(r.Expr.Value); err != nil {
-		return errors.Wrapf(err, fmt.Sprintf("could not parse expression for record '%s' in group '%s'", r.Record.Value, groupName))
+		if r.Record.Value != "" {
+			return errors.Wrapf(err, fmt.Sprintf("could not parse expression for record '%s' in group '%s'", r.Record.Value, groupName))
+		}
+		return errors.Wrapf(err, fmt.Sprintf("could not parse expression for alert '%s' in group '%s'", r.Alert.Value, groupName))
 	}
 
 	if r.Record.Value != "" {

pkg/ruler/compat_test.go

@@ -6,8 +6,12 @@ import (
 	"time"
 
 	"github.com/prometheus/prometheus/config"
+	"github.com/prometheus/prometheus/model/rulefmt"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"gopkg.in/yaml.v3"
+
 	"github.com/grafana/loki/pkg/iter"
 	"github.com/grafana/loki/pkg/logql"
 	rulerbase "github.com/grafana/loki/pkg/ruler/base"
@@ -15,6 +19,61 @@ import (
 	"github.com/grafana/loki/pkg/validation"
 )
 
+// TestInvalidRuleGroup tests that a validation error is raised when rule group is invalid
+func TestInvalidRuleGroup(t *testing.T) {
+	ruleGroupValid := rulefmt.RuleGroup{
+		Name: "test",
+		Rules: []rulefmt.RuleNode{
+			{
+				Alert: yaml.Node{Value: "alert-1-name"},
+				Expr:  yaml.Node{Value: "sum by (job) (rate({namespace=~\"test\"} [5m]) > 0)"},
+			},
+			{
+				Alert: yaml.Node{Value: "record-1-name"},
+				Expr:  yaml.Node{Value: "sum by (job) (rate({namespace=~\"test\"} [5m]) > 0)"},
+			},
+		},
+	}
+	require.Nil(t, ValidateGroups(ruleGroupValid))
+
+	ruleGroupInValid := rulefmt.RuleGroup{
+		Name: "test",
+		Rules: []rulefmt.RuleNode{
+			{
+				Alert: yaml.Node{Value: "alert-1-name"},
+				Expr:  yaml.Node{Value: "bad_value"},
+			},
+			{
+				Record: yaml.Node{Value: "record-1-name"},
+				Expr:   yaml.Node{Value: "bad_value"},
+			},
+		},
+	}
+	require.Error(t, ValidateGroups(ruleGroupInValid)[0])
+	require.Error(t, ValidateGroups(ruleGroupInValid)[1])
+}
+
+// TestInvalidRuleExprParsing tests that a validation error is raised when rule expression is invalid
+func TestInvalidRuleExprParsing(t *testing.T) {
+	expectedAlertErrorMsg := "could not parse expression for alert 'alert-1-name' in group 'test': parse error"
+	alertRuleExprInvalid := &rulefmt.RuleNode{
+		Alert: yaml.Node{Value: "alert-1-name"},
+		Expr:  yaml.Node{Value: "bad_value"},
+	}
+
+	alertErr := validateRuleNode(alertRuleExprInvalid, "test")
+	assert.Containsf(t, alertErr.Error(), expectedAlertErrorMsg, "expected error containing '%s', got '%s'", expectedAlertErrorMsg, alertErr)
+
+	expectedRecordErrorMsg := "could not parse expression for record 'record-1-name' in group 'test': parse error"
+	recordRuleExprInvalid := &rulefmt.RuleNode{
+		Record: yaml.Node{Value: "record-1-name"},
+		Expr:   yaml.Node{Value: "bad_value"},
+	}
+
+	recordErr := validateRuleNode(recordRuleExprInvalid, "test")
+	assert.Containsf(t, recordErr.Error(), expectedRecordErrorMsg, "expected error containing '%s', got '%s'", expectedRecordErrorMsg, recordErr)
+}
+
 // TestInvalidRemoteWriteConfig tests that a validation error is raised when config is invalid
 func TestInvalidRemoteWriteConfig(t *testing.T) {
 	// if remote-write is not enabled, validation fails