Skip to content

Commit df270ee

Browse files
authored
Fix query offset issue on individual rule groups (#6131)
* Fix rule group query offset issue This is a follow-up to #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]>
1 parent ce9c4ba commit df270ee

File tree

3 files changed

+12
-10
lines changed

3 files changed

+12
-10
lines changed

pkg/ruler/api_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ rules:
279279
labels:
280280
test: test
281281
`,
282-
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",
282+
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",
283283
},
284284
{
285285
name: "with a valid rule query offset",
@@ -342,7 +342,7 @@ func TestRuler_DeleteNamespace(t *testing.T) {
342342

343343
router.ServeHTTP(w, req)
344344
require.Equal(t, http.StatusOK, w.Code)
345-
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())
345+
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())
346346

347347
// Delete namespace1
348348
req = requestFor(t, http.MethodDelete, "https://localhost:8080/api/v1/rules/namespace1", nil, "user1")

pkg/ruler/ruler.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -1061,11 +1061,12 @@ func (r *Ruler) ruleGroupListToGroupStateDesc(userID string, backupGroups rulesp
10611061

10621062
groupDesc := &GroupStateDesc{
10631063
Group: &rulespb.RuleGroupDesc{
1064-
Name: group.GetName(),
1065-
Namespace: group.GetNamespace(),
1066-
Interval: interval,
1067-
User: userID,
1068-
Limit: group.Limit,
1064+
Name: group.GetName(),
1065+
Namespace: group.GetNamespace(),
1066+
Interval: interval,
1067+
User: userID,
1068+
Limit: group.Limit,
1069+
QueryOffset: group.QueryOffset,
10691070
},
10701071
// We are keeping default value for EvaluationTimestamp and EvaluationDuration since the backup is not evaluating
10711072
}

pkg/ruler/rulespb/compat.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,17 @@ func formattedRuleToProto(rls []rulefmt.RuleNode) []*RuleDesc {
4949

5050
// FromProto generates a rulefmt RuleGroup
5151
func FromProto(rg *RuleGroupDesc) rulefmt.RuleGroup {
52-
var queryOffset model.Duration
52+
var queryOffset *model.Duration
5353
if rg.QueryOffset != nil {
54-
queryOffset = model.Duration(*rg.QueryOffset)
54+
offset := model.Duration(*rg.QueryOffset)
55+
queryOffset = &offset
5556
}
5657
formattedRuleGroup := rulefmt.RuleGroup{
5758
Name: rg.GetName(),
5859
Interval: model.Duration(rg.Interval),
5960
Rules: make([]rulefmt.RuleNode, len(rg.GetRules())),
6061
Limit: int(rg.Limit),
61-
QueryOffset: &queryOffset,
62+
QueryOffset: queryOffset,
6263
}
6364

6465
for i, rl := range rg.GetRules() {

0 commit comments

Comments
 (0)