Alerting: Evaluate all imported from Prometheus rules sequentially (#106295)

What is this feature?

Makes all alert rules imported from a Prometheus YAML or Prometheus-compatible data source evaluate sequentially.

Why do we need this feature?

Currently only alert rules [imported via the API](https://grafana.com/docs/grafana-cloud/alerting-and-irm/alerting/alerting-rules/alerting-migration/migration-api/) are evaluated sequentially, because only they have the original alert rule definition in YAML. But alert rules can be imported [in the UI, and from a YAML file](https://grafana.com/docs/grafana-cloud/alerting-and-irm/alerting/alerting-rules/alerting-migration/), and they won't be evaluated sequentially which can lead to issues with recording rules.

pkg/services/ngalert/api/api_convert_prometheus.go

@@ -158,8 +158,8 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusGetRules(c *contextmodel.
 	}
 
 	filterOpts := &provisioning.FilterOptions{
-		ImportedPrometheusRule: util.Pointer(true),
-		NamespaceUIDs:          folderUIDs,
+		HasPrometheusRuleDefinition: util.Pointer(true),
+		NamespaceUIDs:               folderUIDs,
 	}
 	groups, err := srv.alertRuleService.GetAlertGroupsWithFolderFullpath(c.Req.Context(), c.SignedInUser, filterOpts)
 	if err != nil {
@@ -193,8 +193,8 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusDeleteNamespace(c *contex
 
 	provenance := getProvenance(c)
 	filterOpts := &provisioning.FilterOptions{
-		NamespaceUIDs:          []string{namespace.UID},
-		ImportedPrometheusRule: util.Pointer(true),
+		NamespaceUIDs:               []string{namespace.UID},
+		HasPrometheusRuleDefinition: util.Pointer(true),
 	}
 	err = srv.alertRuleService.DeleteRuleGroups(c.Req.Context(), c.SignedInUser, provenance, filterOpts)
 	if errors.Is(err, models.ErrAlertRuleGroupNotFound) {
@@ -251,8 +251,8 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusGetNamespace(c *contextmo
 	}
 
 	filterOpts := &provisioning.FilterOptions{
-		ImportedPrometheusRule: util.Pointer(true),
-		NamespaceUIDs:          []string{namespace.UID},
+		HasPrometheusRuleDefinition: util.Pointer(true),
+		NamespaceUIDs:               []string{namespace.UID},
 	}
 	groups, err := srv.alertRuleService.GetAlertGroupsWithFolderFullpath(c.Req.Context(), c.SignedInUser, filterOpts)
 	if err != nil {
@@ -288,9 +288,9 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusGetRuleGroup(c *contextmo
 	}
 
 	filterOpts := &provisioning.FilterOptions{
-		ImportedPrometheusRule: util.Pointer(true),
-		NamespaceUIDs:          []string{namespace.UID},
-		RuleGroups:             []string{group},
+		HasPrometheusRuleDefinition: util.Pointer(true),
+		NamespaceUIDs:               []string{namespace.UID},
+		RuleGroups:                  []string{group},
 	}
 	groupsWithFolders, err := srv.alertRuleService.GetAlertGroupsWithFolderFullpath(c.Req.Context(), c.SignedInUser, filterOpts)
 	if err != nil {

pkg/services/ngalert/models/alert_rule.go

@@ -406,7 +406,12 @@ func WithoutInternalLabels() LabelOption {
 	}
 }
 
-func (alertRule *AlertRule) ImportedFromPrometheus() bool {
+func (alertRule *AlertRule) ImportedPrometheusRule() bool {
+	_, hasConvertedPrometheusRuleLabel := alertRule.GetLabels()[ConvertedPrometheusRuleLabel]
+	return hasConvertedPrometheusRuleLabel || alertRule.HasPrometheusRuleDefinition()
+}
+
+func (alertRule *AlertRule) HasPrometheusRuleDefinition() bool {
 	_, err := alertRule.PrometheusRuleDefinition()
 	return err == nil
 }
@@ -859,7 +864,7 @@ type ListAlertRulesQuery struct {
 	ReceiverName     string
 	TimeIntervalName string
 
-	ImportedPrometheusRule *bool
+	HasPrometheusRuleDefinition *bool
 }
 
 // CountAlertRulesQuery is the query for counting alert rules

pkg/services/ngalert/models/alert_rule_test.go

@@ -1259,7 +1259,7 @@ func TestAlertRule_PrometheusRuleDefinition(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			result, err := tt.rule.PrometheusRuleDefinition()
-			isPrometheusRule := tt.rule.ImportedFromPrometheus()
+			isPrometheusRule := tt.rule.HasPrometheusRuleDefinition()
 
 			if tt.expectedErrorMsg != "" {
 				require.Error(t, err)
@@ -1273,3 +1273,78 @@ func TestAlertRule_PrometheusRuleDefinition(t *testing.T) {
 		})
 	}
 }
+
+func TestAlertRule_ImportedPrometheusRule(t *testing.T) {
+	tests := []struct {
+		name     string
+		rule     AlertRule
+		expected bool
+	}{
+		{
+			name: "rule with prometheus definition",
+			rule: AlertRule{
+				Metadata: AlertRuleMetadata{
+					PrometheusStyleRule: &PrometheusStyleRule{
+						OriginalRuleDefinition: "some rule definition",
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "rule with converted prometheus rule label",
+			rule: AlertRule{
+				Labels: map[string]string{
+					ConvertedPrometheusRuleLabel: "true",
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "rule with both prometheus definition and converted label",
+			rule: AlertRule{
+				Labels: map[string]string{
+					ConvertedPrometheusRuleLabel: "true",
+				},
+				Metadata: AlertRuleMetadata{
+					PrometheusStyleRule: &PrometheusStyleRule{
+						OriginalRuleDefinition: "some rule definition",
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "rule with empty prometheus definition",
+			rule: AlertRule{
+				Metadata: AlertRuleMetadata{
+					PrometheusStyleRule: &PrometheusStyleRule{
+						OriginalRuleDefinition: "",
+					},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "rule with nil prometheus style rule",
+			rule: AlertRule{
+				Metadata: AlertRuleMetadata{
+					PrometheusStyleRule: nil,
+				},
+			},
+			expected: false,
+		},
+		{
+			name:     "rule with empty metadata",
+			rule:     AlertRule{},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.rule.ImportedPrometheusRule()
+			require.Equal(t, tt.expected, result)
+		})
+	}
+}

pkg/services/ngalert/provisioning/alert_rules.go

@@ -271,9 +271,9 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, user ident
 // FilterOptions provides filtering for alert rule queries.
 // All fields are optional and will be applied as filters if provided.
 type FilterOptions struct {
-	ImportedPrometheusRule *bool
-	RuleGroups             []string
-	NamespaceUIDs          []string
+	HasPrometheusRuleDefinition *bool
+	RuleGroups                  []string
+	NamespaceUIDs               []string
 }
 
 func (opts *FilterOptions) apply(q models.ListAlertRulesQuery) models.ListAlertRulesQuery {
@@ -281,8 +281,8 @@ func (opts *FilterOptions) apply(q models.ListAlertRulesQuery) models.ListAlertR
 		return q
 	}
 
-	if opts.ImportedPrometheusRule != nil {
-		q.ImportedPrometheusRule = opts.ImportedPrometheusRule
+	if opts.HasPrometheusRuleDefinition != nil {
+		q.HasPrometheusRuleDefinition = opts.HasPrometheusRuleDefinition
 	}
 
 	if len(opts.NamespaceUIDs) > 0 {

pkg/services/ngalert/provisioning/alert_rules_test.go

@@ -1924,8 +1924,8 @@ func TestDeleteRuleGroups(t *testing.T) {
 
 	t.Run("when filtering by imported Prometheus rules", func(t *testing.T) {
 		filterOpts := &FilterOptions{
-			ImportedPrometheusRule: util.Pointer(true),
-			NamespaceUIDs:          []string{"namespace1"},
+			HasPrometheusRuleDefinition: util.Pointer(true),
+			NamespaceUIDs:               []string{"namespace1"},
 		}
 
 		t.Run("when the group is not imported", func(t *testing.T) {

pkg/services/ngalert/schedule/metrics.go

@@ -88,8 +88,7 @@ func (sch *schedule) updateRulesMetrics(alertRules []*models.AlertRule) {
 			}
 		}
 
-		_, hasConvertedPrometheusRuleLabel := rule.GetLabels()[models.ConvertedPrometheusRuleLabel]
-		if rule.ImportedFromPrometheus() || hasConvertedPrometheusRuleLabel {
+		if rule.ImportedPrometheusRule() {
 			orgsRulesPrometheusImported[rule.OrgID]++
 		}
 

pkg/services/ngalert/schedule/schedule_unit_test.go

@@ -730,7 +730,7 @@ func TestSchedule_updateRulesMetrics(t *testing.T) {
 		})
 
 		// The metric includes alert rules with either internal ConvertedPrometheusRuleLabel label,
-		// or when AlertRule.ImportedFromPrometheus() returns true.
+		// or when AlertRule.HasPrometheusRuleDefinition() returns true.
 		alertRule1 := models.RuleGen.With(
 			models.RuleGen.WithOrgID(firstOrgID),
 			models.RuleGen.WithPrometheusOriginalRuleDefinition("1"),

pkg/services/ngalert/schedule/sequence.go

@@ -117,7 +117,7 @@ func (sch *schedule) shouldEvaluateSequentially(groupItems []readyToRunItem) boo
 
 	// only evaluate rules in imported groups sequentially
 	for _, item := range groupItems {
-		if item.rule.ImportedFromPrometheus() {
+		if item.rule.ImportedPrometheusRule() {
 			return true
 		}
 	}

pkg/services/ngalert/schedule/sequence_test.go

@@ -90,6 +90,8 @@ func TestSequence(t *testing.T) {
 						models.RuleGen.WithUID("4"),
 						models.RuleGen.WithGroupIndex(2),
 						models.RuleGen.WithGroupName("rg2"),
+						// This rule has the Prometheus rule YAML definition,
+						// indicating it was converted from Prometheus.
 						models.RuleGen.WithPrometheusOriginalRuleDefinition("test"),
 					).GenerateRef(),
 					folderTitle: "folder1",
@@ -102,7 +104,9 @@ func TestSequence(t *testing.T) {
 						models.RuleGen.WithUID("5"),
 						models.RuleGen.WithGroupIndex(3),
 						models.RuleGen.WithGroupName("rg2"),
-						models.RuleGen.WithPrometheusOriginalRuleDefinition("test"),
+						// This rule does not have the YAML definition,
+						// but still has the label indicating it was converted from Prometheus.
+						models.RuleGen.WithLabel(models.ConvertedPrometheusRuleLabel, "true"),
 					).GenerateRef(),
 					folderTitle: "folder1",
 				},

pkg/services/ngalert/store/alert_rule.go

@@ -632,8 +632,8 @@ func (st DBstore) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertR
 			}
 		}
 
-		if query.ImportedPrometheusRule != nil {
-			q, err = st.filterImportedPrometheusRules(*query.ImportedPrometheusRule, q)
+		if query.HasPrometheusRuleDefinition != nil {
+			q, err = st.filterWithPrometheusRuleDefinition(*query.HasPrometheusRuleDefinition, q)
 			if err != nil {
 				return err
 			}
@@ -678,8 +678,8 @@ func (st DBstore) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertR
 					continue
 				}
 			}
-			if query.ImportedPrometheusRule != nil { // remove false-positive hits from the result
-				if *query.ImportedPrometheusRule != converted.ImportedFromPrometheus() {
+			if query.HasPrometheusRuleDefinition != nil { // remove false-positive hits from the result
+				if *query.HasPrometheusRuleDefinition != converted.HasPrometheusRuleDefinition() {
 					continue
 				}
 			}
@@ -1019,7 +1019,7 @@ func (st DBstore) filterByContentInNotificationSettings(value string, sess *xorm
 	return sess.And(sql, param), nil
 }
 
-func (st DBstore) filterImportedPrometheusRules(value bool, sess *xorm.Session) (*xorm.Session, error) {
+func (st DBstore) filterWithPrometheusRuleDefinition(value bool, sess *xorm.Session) (*xorm.Session, error) {
 	if value {
 		// Filter for rules that have both prometheus_style_rule and original_rule_definition in metadata
 		return sess.And(

pkg/services/ngalert/store/alert_rule_test.go

@@ -1743,7 +1743,7 @@ func TestIntegration_ListAlertRules(t *testing.T) {
 		ruleGen.WithIntervalMatching(cfg.UnifiedAlerting.BaseInterval),
 		ruleGen.WithOrgID(orgID),
 	)
-	t.Run("filter by ImportedPrometheusRule", func(t *testing.T) {
+	t.Run("filter by HasPrometheusRuleDefinition", func(t *testing.T) {
 		store := createTestStore(sqlStore, folderService, &logtest.Fake{}, cfg.UnifiedAlerting, b)
 		regularRule := createRule(t, store, ruleGen)
 		importedRule := createRule(t, store, ruleGen.With(
@@ -1773,8 +1773,8 @@ func TestIntegration_ListAlertRules(t *testing.T) {
 		for _, tt := range tc {
 			t.Run(tt.name, func(t *testing.T) {
 				query := &models.ListAlertRulesQuery{
-					OrgID:                  orgID,
-					ImportedPrometheusRule: tt.importedPrometheusRule,
+					OrgID:                       orgID,
+					HasPrometheusRuleDefinition: tt.importedPrometheusRule,
 				}
 				result, err := store.ListAlertRules(context.Background(), query)
 				require.NoError(t, err)

pkg/services/ngalert/tests/fakes/rules.go

@@ -222,8 +222,8 @@ func (f *RuleStore) ListAlertRules(_ context.Context, q *models.ListAlertRulesQu
 		if len(q.RuleUIDs) > 0 && !slices.Contains(q.RuleUIDs, r.UID) {
 			continue
 		}
-		if q.ImportedPrometheusRule != nil {
-			if *q.ImportedPrometheusRule != r.ImportedFromPrometheus() {
+		if q.HasPrometheusRuleDefinition != nil {
+			if *q.HasPrometheusRuleDefinition != r.HasPrometheusRuleDefinition() {
 				continue
 			}
 		}