Add support for Prometheus rule query offset

[mustafain117]

What this PR does:

This PR adds support for query_offset field on RuleGroup and global rule_query_offset configuration in Cortex Ruler.
This feature was added in Prometheus 2.53.0

Which issue(s) this PR fixes:

Fixes #6055

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[yeya24]

There is already a per tenant limit called -ruler.evaluation-delay-duration. Should we deprecate that in this PR?

[mustafain117]

There is already a per tenant limit called -ruler.evaluation-delay-duration. Should we deprecate that in this PR?

Yes that will be a duplicate after this PR. Can I simply remove the usages of that limit and mention it as deprecated in the changelog?

[yeya24]

Yes I think that works. Please also mark the flag as deprecated. You can find some examples in the codebase.

[klingerf]

I'm trying to use this branch to set a global rule query offset, and that setting isn't being honored in my local setup. In my cortex config file, I'm using:

limits:
  ruler_query_offset: 1m

I think the issue is that this queryOffset field needs to be nullable. Since it's non-nullable, it's being initialized asDuration(0) in compat.go, and the ruler only falls back to the global offset if the field is actually nil, which you can see here:

https://github.com/prometheus/prometheus/blob/b75e6353742e32c7e18997ebd6a75b77563e33b5/rules/group.go#L644-L654

This change seems to fix it:

diff --git a/pkg/ruler/ruler.go b/pkg/ruler/ruler.go
index d53f78506..8fec45acb 100644
--- a/pkg/ruler/ruler.go
+++ b/pkg/ruler/ruler.go
@@ -910,6 +910,10 @@ func (r *Ruler) getLocalRules(userID string, rulesRequest RulesRequest, includeB
 			}
 		}
 		interval := group.Interval()
+		var queryOffset *time.Duration
+		if offset := group.QueryOffset(); offset != 0 {
+			queryOffset = &offset
+		}
 
 		groupDesc := &GroupStateDesc{
 			Group: &rulespb.RuleGroupDesc{
@@ -918,7 +922,7 @@ func (r *Ruler) getLocalRules(userID string, rulesRequest RulesRequest, includeB
 				Interval:    interval,
 				User:        userID,
 				Limit:       int64(group.Limit()),
-				QueryOffset: group.QueryOffset(),
+				QueryOffset: queryOffset,
 			},
 
 			EvaluationTimestamp: group.GetLastEvaluation(),
diff --git a/pkg/ruler/rulespb/compat.go b/pkg/ruler/rulespb/compat.go
index 4f916d09c..7526062ad 100644
--- a/pkg/ruler/rulespb/compat.go
+++ b/pkg/ruler/rulespb/compat.go
@@ -13,9 +13,10 @@ import (
 
 // ToProto transforms a formatted prometheus rulegroup to a rule group protobuf
 func ToProto(user string, namespace string, rl rulefmt.RuleGroup) *RuleGroupDesc {
-	queryOffset := time.Duration(0)
+	var queryOffset *time.Duration
 	if rl.QueryOffset != nil {
-		queryOffset = time.Duration(*rl.QueryOffset)
+		offset := time.Duration(*rl.QueryOffset)
+		queryOffset = &offset
 	}
 	rg := RuleGroupDesc{
 		Name:        rl.Name,
@@ -48,13 +49,17 @@ func formattedRuleToProto(rls []rulefmt.RuleNode) []*RuleDesc {
 
 // FromProto generates a rulefmt RuleGroup
 func FromProto(rg *RuleGroupDesc) rulefmt.RuleGroup {
-	queryOffset := model.Duration(rg.QueryOffset)
+	var queryOffset *model.Duration
+	if rg.QueryOffset != nil {
+		offset := model.Duration(*rg.QueryOffset)
+		queryOffset = &offset
+	}
 	formattedRuleGroup := rulefmt.RuleGroup{
 		Name:        rg.GetName(),
 		Interval:    model.Duration(rg.Interval),
 		Rules:       make([]rulefmt.RuleNode, len(rg.GetRules())),
 		Limit:       int(rg.Limit),
-		QueryOffset: &queryOffset,
+		QueryOffset: queryOffset,
 	}
 
 	for i, rl := range rg.GetRules() {
diff --git a/pkg/ruler/rulespb/rules.proto b/pkg/ruler/rulespb/rules.proto
index 189c050eb..ed4e98a76 100644
--- a/pkg/ruler/rulespb/rules.proto
+++ b/pkg/ruler/rulespb/rules.proto
@@ -29,7 +29,7 @@ message RuleGroupDesc {
   repeated google.protobuf.Any options = 9;
   int64 limit =10;
   google.protobuf.Duration queryOffset = 11
-      [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
+      [(gogoproto.nullable) = true, (gogoproto.stdduration) = true];
 }
 
 // RuleDesc is a proto representation of a Prometheus Rule

[klingerf]

There's a related discussion of this in the original prometheus PR, here: prometheus/prometheus#14061 (comment)

[yeya24]

Thanks. This LGTM!

[alanprot]

LGTM!

[yeya24]

@mustafain117 Let's make sure that we create a follow up PR to deprecate the old configuration.

[klingerf]

Hey folks, thanks for putting this change together. I'm still seeing the issue that I mentioned here #6085 (comment) on the version of this PR that merged to master. As far as I can tell, all rules are being initialized with a hardcoded query offset of 0, and that prevents them from falling back to the global rule query offset.

I can dig into it and try to submit a follow-up PR, but at least wanted to give you all a heads up.

[klingerf]

I created #6131 with a fix.