fix(ruler): fix critical correctness issues in select merging

1. Group by expression structure — rules with different functions
   (rate vs sum) on the same metric are no longer incorrectly merged.

2. Only include labels present in ALL entries — a label missing from
   any rule means that rule matches all values; including it in the
   merged set would make the pre-fetch too restrictive.

3. Skip when no prefetchExpr found — if no rule's matchers equal the
   merged matchers (no superset rule exists), skip merging for that
   metric rather than falling back to a bare selector that returns
   raw series instead of computed values.

4. Disable select merging when ConcurrentEvalsEnabled is true —
   SetQueryFunc is not thread-safe.

Author: alanprot

pkg/ruler/manager.go

@@ -134,7 +134,7 @@ func NewDefaultMultiTenantManager(cfg Config, limits RulesLimits, managerFactory
 		logger:                 logger,
 		ruleGroupIterationFunc: defaultRuleGroupIterationFunc,
 	}
-	if cfg.SelectMergerEnabled {
+	if cfg.SelectMergerEnabled && !cfg.ConcurrentEvalsEnabled {
 		m.ruleGroupIterationFunc = mergedSelectIterationFunc(cfg.SelectMergerMinRules)
 	}
 	if cfg.RulesBackupEnabled() {

pkg/ruler/prefetch_queryable.go

@@ -70,11 +70,7 @@ func cachedQueryFunc(inner rules.QueryFunc, cache *prefetchCache) rules.QueryFun
 func executePrefetch(ctx context.Context, plan []mergedSelect, queryFunc rules.QueryFunc, ts time.Time) *prefetchCache {
 	cache := &prefetchCache{}
 	for _, ms := range plan {
-		expr := ms.prefetchExpr
-		if expr == "" {
-			expr = buildSelectorExpr(ms.mergedMatchers)
-		}
-		vec, err := queryFunc(ctx, expr, ts)
+		vec, err := queryFunc(ctx, ms.prefetchExpr, ts)
 		if err != nil {
 			continue
 		}

pkg/ruler/select_merger.go

@@ -1,6 +1,8 @@
 package ruler
 
 import (
+	"strings"
+
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/promql/parser"
 	"github.com/prometheus/prometheus/rules"
@@ -14,11 +16,15 @@ type mergedSelect struct {
 }
 
 func planMergedSelects(rls []rules.Rule, minRules int) []mergedSelect {
-	// Group selectors by metric name.
+	// Group selectors by metric name AND expression structure.
+	// The "structure key" is the expression with matchers blanked out,
+	// so rate(m{a="1"}[5m]) and rate(m{a="2"}[5m]) share a key but
+	// sum(m{a="1"}) does not.
 	type entry struct {
 		matchers []*labels.Matcher
 		exprStr  string
 	}
+	// Key: metricName + "\x00" + expression structure
 	groups := map[string][]entry{}
 
 	for _, r := range rls {
@@ -28,12 +34,13 @@ func planMergedSelects(rls []rules.Rule, minRules int) []mergedSelect {
 			if name == "" {
 				return
 			}
-			groups[name] = append(groups[name], entry{matchers: vs.LabelMatchers, exprStr: exprStr})
+			key := name + "\x00" + exprStructureKey(r.Query(), vs)
+			groups[key] = append(groups[key], entry{matchers: vs.LabelMatchers, exprStr: exprStr})
 		})
 	}
 
 	var result []mergedSelect
-	for name, entries := range groups {
+	for _, entries := range groups {
 		if len(entries) < minRules {
 			continue
 		}
@@ -44,14 +51,19 @@ func planMergedSelects(rls []rules.Rule, minRules int) []mergedSelect {
 		merged := computeMergedMatchers(originals)
 
 		// Find the expression whose matchers equal the merged set (the broadest rule).
+		// If none found, skip — we can't safely pre-fetch without a full expression.
 		prefetchExpr := ""
 		for _, e := range entries {
 			if matchersEqual(e.matchers, merged) {
 				prefetchExpr = e.exprStr
 				break
 			}
 		}
+		if prefetchExpr == "" {
+			continue
+		}
 
+		name := metricNameFromMatchers(merged)
 		result = append(result, mergedSelect{
 			metricName:      name,
 			mergedMatchers:  merged,
@@ -62,6 +74,16 @@ func planMergedSelects(rls []rules.Rule, minRules int) []mergedSelect {
 	return result
 }
 
+// exprStructureKey returns a string representing the expression structure
+// with the VectorSelector's matchers replaced by a placeholder.
+// Two expressions with the same structure key differ only in their matchers.
+func exprStructureKey(expr parser.Expr, vs *parser.VectorSelector) string {
+	full := expr.String()
+	selectorStr := vs.String()
+	// Replace the specific selector with a placeholder.
+	return strings.Replace(full, selectorStr, "{__PLACEHOLDER__}", 1)
+}
+
 // matchersEqual returns true if two matcher slices contain the same matchers (order-independent).
 func matchersEqual(a, b []*labels.Matcher) bool {
 	if len(a) != len(b) {
@@ -101,7 +123,10 @@ func metricNameFromMatchers(ms []*labels.Matcher) string {
 }
 
 func computeMergedMatchers(entries [][]*labels.Matcher) []*labels.Matcher {
-	// Collect all label names across entries.
+	// Collect all label names across entries, but only keep labels
+	// that appear in ALL entries. A label missing from any entry means
+	// that entry matches all values for that label — including it in the
+	// merged set would make the pre-fetch too restrictive.
 	labelMatchers := map[string][]*labels.Matcher{}
 	for _, ms := range entries {
 		for _, m := range ms {
@@ -111,6 +136,10 @@ func computeMergedMatchers(entries [][]*labels.Matcher) []*labels.Matcher {
 
 	var result []*labels.Matcher
 	for _, ms := range labelMatchers {
+		// Only include if present in ALL entries.
+		if len(ms) != len(entries) {
+			continue
+		}
 		if sup := findSuperset(ms); sup != nil {
 			result = append(result, sup)
 		}

pkg/ruler/select_merger_bench_test.go

@@ -24,10 +24,18 @@ func BenchmarkSelectMerger(b *testing.B) {
 	colors := []string{"blue", "green", "canary"}
 
 	// Build 114 rules: 38 metrics × 3 deploy_color variants.
+	// One variant uses =~".*" (superset) to enable merging.
 	rls := make([]rules.Rule, 0, numMetrics*numColors)
 	for i := range numMetrics {
-		for _, c := range colors {
-			expr := fmt.Sprintf(`sum(metric_%d{deploy_color="%s",job="svc"})`, i, c)
+		for ci, c := range colors {
+			var expr string
+			if ci == numColors-1 {
+				// Last color variant uses regex superset — enables merging.
+				expr = fmt.Sprintf(`sum(metric_%d{deploy_color=~".*",job="svc"})`, i)
+			} else {
+				expr = fmt.Sprintf(`sum(metric_%d{deploy_color="%s",job="svc"})`, i, c)
+			}
+			_ = c
 			e, err := parser.ParseExpr(expr)
 			if err != nil {
 				b.Fatal(err)

pkg/ruler/select_merger_test.go

@@ -26,10 +26,14 @@ func mustParseRule(t *testing.T, expr string) rules.Rule {
 
 func TestSelectMerger_Plan_GroupsByMetric(t *testing.T) {
 	rls := []rules.Rule{
-		mustParseRule(t, `sum(http_requests_total{job="api"})`),
-		mustParseRule(t, `sum(http_requests_total{job="web"})`),
+		// http_requests_total: 3 rules with same structure, deploy_color varies, one has =~".*" superset
+		mustParseRule(t, `sum(http_requests_total{job="api",deploy_color="blue"})`),
+		mustParseRule(t, `sum(http_requests_total{job="api",deploy_color="green"})`),
+		mustParseRule(t, `sum(http_requests_total{job="api",deploy_color=~".*"})`),
+		// cpu_usage: 3 rules with same structure, host varies, one has =~".*" superset
 		mustParseRule(t, `avg(cpu_usage{host="a"})`),
 		mustParseRule(t, `avg(cpu_usage{host="b"})`),
+		mustParseRule(t, `avg(cpu_usage{host=~".*"})`),
 	}
 
 	result := planMergedSelects(rls, 2)
@@ -92,17 +96,46 @@ func TestSelectMerger_SupersetDetection(t *testing.T) {
 
 func TestSelectMerger_MinRulesThreshold(t *testing.T) {
 	rls := []rules.Rule{
-		mustParseRule(t, `sum(http_requests_total{job="api"})`),
-		mustParseRule(t, `sum(http_requests_total{job="web"})`),
+		mustParseRule(t, `sum(http_requests_total{job="api",color="blue"})`),
+		mustParseRule(t, `sum(http_requests_total{job="api",color="green"})`),
+		mustParseRule(t, `sum(http_requests_total{job="api",color=~".*"})`),
 		mustParseRule(t, `avg(cpu_usage{host="a"})`),
 	}
 
-	// minRules=2: http_requests_total has 2 rules, cpu_usage has 1
+	// minRules=2: http_requests_total has 3 rules with superset, cpu_usage has only 1
+	result := planMergedSelects(rls, 2)
+	require.Len(t, result, 1)
+	assert.Equal(t, "http_requests_total", result[0].metricName)
+
+	// minRules=4: nothing qualifies
+	result = planMergedSelects(rls, 4)
+	assert.Len(t, result, 0)
+}
+
+func TestSelectMerger_DifferentExprStructures_NotMerged(t *testing.T) {
+	// Same metric but different expression structures should NOT be merged.
+	rls := []rules.Rule{
+		mustParseRule(t, `rate(http_requests_total{job="api",color="blue"}[5m])`),
+		mustParseRule(t, `rate(http_requests_total{job="api",color=~".*"}[5m])`),
+		mustParseRule(t, `sum(http_requests_total{job="api",color="red"})`),
+	}
+
 	result := planMergedSelects(rls, 2)
-	assert.Len(t, result, 1)
+	// Only the rate() pair should merge (2 rules with same structure).
+	// The sum() rule has a different structure.
+	require.Len(t, result, 1)
 	assert.Equal(t, "http_requests_total", result[0].metricName)
+	assert.Contains(t, result[0].prefetchExpr, "rate")
+}
+
+func TestSelectMerger_NoSuperset_NotMerged(t *testing.T) {
+	// When no rule has a superset matcher, merging is skipped.
+	rls := []rules.Rule{
+		mustParseRule(t, `sum(http_requests_total{job="api"})`),
+		mustParseRule(t, `sum(http_requests_total{job="web"})`),
+	}
 
-	// minRules=3: nothing qualifies
-	result = planMergedSelects(rls, 3)
+	result := planMergedSelects(rls, 2)
+	// job="api" and job="web" have no superset, so no prefetchExpr can be found.
 	assert.Len(t, result, 0)
 }