Fix query offset issue on individual rule groups (cortexproject#6131)

* Fix rule group query offset issue This is a follow-up to cortexproject#6085, which added support for setting the `query_offset` field on individual recording rule groups, as well as a per-tenant `ruler_query_offset` limit that should be used when no individual recording rule group offset is set. It turns out that compatibility code to convert from a protobuf RuleGroup to a prometheus RuleGroup was coercing null-value query offsets to explicit 0s, which meant that no rule groups would ever fall back to the per-tenant offset. This PR fixes that issue, and it cleans up handling of the query offset in a few other ruler files. Signed-off-by: Kevin Ingelman <[email protected]> * Revert change to rules API response Signed-off-by: Kevin Ingelman <[email protected]> --------- Signed-off-by: Kevin Ingelman <[email protected]>
danielblando · Aug 6, 2024 · df270ee · df270ee
1 parent ce9c4ba
commit df270ee
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 10 deletions.
diff --git a/pkg/ruler/api_test.go b/pkg/ruler/api_test.go
@@ -279,7 +279,7 @@ rules:
   labels:
     test: test
 `,
-			output: "name: test\ninterval: 15s\nquery_offset: 0s\nrules:\n    - record: up_rule\n      expr: up{}\n    - alert: up_alert\n      expr: sum(up{}) > 1\n      for: 30s\n      labels:\n        test: test\n      annotations:\n        test: test\n",
+			output: "name: test\ninterval: 15s\nrules:\n    - record: up_rule\n      expr: up{}\n    - alert: up_alert\n      expr: sum(up{}) > 1\n      for: 30s\n      labels:\n        test: test\n      annotations:\n        test: test\n",
 		},
 		{
 			name:   "with a valid rule query offset",
@@ -342,7 +342,7 @@ func TestRuler_DeleteNamespace(t *testing.T) {
 
 	router.ServeHTTP(w, req)
 	require.Equal(t, http.StatusOK, w.Code)
-	require.Equal(t, "name: group1\ninterval: 1m\nquery_offset: 0s\nrules:\n    - record: UP_RULE\n      expr: up\n    - alert: UP_ALERT\n      expr: up < 1\n", w.Body.String())
+	require.Equal(t, "name: group1\ninterval: 1m\nrules:\n    - record: UP_RULE\n      expr: up\n    - alert: UP_ALERT\n      expr: up < 1\n", w.Body.String())
 
 	// Delete namespace1
 	req = requestFor(t, http.MethodDelete, "https://localhost:8080/api/v1/rules/namespace1", nil, "user1")

diff --git a/pkg/ruler/ruler.go b/pkg/ruler/ruler.go
@@ -1061,11 +1061,12 @@ func (r *Ruler) ruleGroupListToGroupStateDesc(userID string, backupGroups rulesp
 
 		groupDesc := &GroupStateDesc{
 			Group: &rulespb.RuleGroupDesc{
-				Name:      group.GetName(),
-				Namespace: group.GetNamespace(),
-				Interval:  interval,
-				User:      userID,
-				Limit:     group.Limit,
+				Name:        group.GetName(),
+				Namespace:   group.GetNamespace(),
+				Interval:    interval,
+				User:        userID,
+				Limit:       group.Limit,
+				QueryOffset: group.QueryOffset,
 			},
 			// We are keeping default value for EvaluationTimestamp and EvaluationDuration since the backup is not evaluating
 		}

diff --git a/pkg/ruler/rulespb/compat.go b/pkg/ruler/rulespb/compat.go
@@ -49,16 +49,17 @@ func formattedRuleToProto(rls []rulefmt.RuleNode) []*RuleDesc {
 
 // FromProto generates a rulefmt RuleGroup
 func FromProto(rg *RuleGroupDesc) rulefmt.RuleGroup {
-	var queryOffset model.Duration
+	var queryOffset *model.Duration
 	if rg.QueryOffset != nil {
-		queryOffset = model.Duration(*rg.QueryOffset)
+		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() {