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]>

Author: klingerf

pkg/ruler/api.go

@@ -62,6 +62,7 @@ type RuleGroup struct {
 	// same array.
 	Rules          []rule    `json:"rules"`
 	Interval       float64   `json:"interval"`
+	QueryOffset    *float64  `json:"queryOffset,omitempty"`
 	LastEvaluation time.Time `json:"lastEvaluation"`
 	EvaluationTime float64   `json:"evaluationTime"`
 	Limit          int64     `json:"limit"`
@@ -182,11 +183,17 @@ func (a *API) PrometheusRules(w http.ResponseWriter, req *http.Request) {
 	groups := make([]*RuleGroup, 0, len(rgs))
 
 	for _, g := range rgs {
+		var queryOffset *float64
+		if g.Group.QueryOffset != nil {
+			offset := g.Group.QueryOffset.Seconds()
+			queryOffset = &offset
+		}
 		grp := RuleGroup{
 			Name:           g.Group.Name,
 			File:           g.Group.Namespace,
 			Rules:          make([]rule, len(g.ActiveRules)),
 			Interval:       g.Group.Interval.Seconds(),
+			QueryOffset:    queryOffset,
 			LastEvaluation: g.GetEvaluationTimestamp(),
 			EvaluationTime: g.GetEvaluationDuration().Seconds(),
 			Limit:          g.Group.Limit,

pkg/ruler/api_test.go

@@ -44,6 +44,7 @@ func TestRuler_rules(t *testing.T) {
 	require.Equal(t, responseJSON.Status, "success")
 
 	// Testing the running rules for user1 in the mock store
+	queryOffset := float64(0)
 	expectedResponse, _ := json.Marshal(util_api.Response{
 		Status: "success",
 		Data: &RuleDiscovery{
@@ -67,7 +68,8 @@ func TestRuler_rules(t *testing.T) {
 							Alerts: []*Alert{},
 						},
 					},
-					Interval: 60,
+					Interval:    60,
+					QueryOffset: &queryOffset,
 				},
 			},
 		},
@@ -100,6 +102,7 @@ func TestRuler_rules_special_characters(t *testing.T) {
 	require.Equal(t, responseJSON.Status, "success")
 
 	// Testing the running rules for user1 in the mock store
+	queryOffset := float64(0)
 	expectedResponse, _ := json.Marshal(util_api.Response{
 		Status: "success",
 		Data: &RuleDiscovery{
@@ -123,7 +126,8 @@ func TestRuler_rules_special_characters(t *testing.T) {
 							Alerts: []*Alert{},
 						},
 					},
-					Interval: 60,
+					Interval:    60,
+					QueryOffset: &queryOffset,
 				},
 			},
 		},
@@ -154,6 +158,7 @@ func TestRuler_rules_limit(t *testing.T) {
 	require.Equal(t, responseJSON.Status, "success")
 
 	// Testing the running rules for user1 in the mock store
+	queryOffset := float64(0)
 	expectedResponse, _ := json.Marshal(util_api.Response{
 		Status: "success",
 		Data: &RuleDiscovery{
@@ -178,7 +183,8 @@ func TestRuler_rules_limit(t *testing.T) {
 							Alerts: []*Alert{},
 						},
 					},
-					Interval: 60,
+					Interval:    60,
+					QueryOffset: &queryOffset,
 				},
 			},
 		},
@@ -279,7 +285,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 +348,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")

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
 		}

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() {