Skip to content

Commit

Permalink
BUGFIX: Mark the rule's restoration process as completed always
Browse files Browse the repository at this point in the history
In #13980 I introduced a change to reduce the number of queries executed when we restore alert statuses.

With this, the querying semantics changed as we now need to go through all series before we enter the alert restoration loop and I missed the fact that exiting early when there are no rules to restore would lead to an incomplete restoration.

An alert being restored is used as a proxy for "we're now ready to write `ALERTS/ALERTS_FOR_SERIES` metrics" so as a result we weren't writing the series if we didn't restore anything the first time around.

Signed-off-by: gotjosh <josue.abreu@gmail.com>
  • Loading branch information
gotjosh committed May 3, 2024
1 parent 94c81bb commit 0cff3bc
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 0 deletions.
2 changes: 2 additions & 0 deletions rules/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,7 @@ func (g *Group) RestoreForState(ts time.Time) {
"stage", "Select",
"err", err,
)
alertRule.SetRestored(true)
continue
}

Expand All @@ -684,6 +685,7 @@ func (g *Group) RestoreForState(ts time.Time) {
// No results for this alert rule.
if len(seriesByLabels) == 0 {
level.Debug(g.logger).Log("msg", "Failed to find a series to restore the 'for' state", labels.AlertName, alertRule.Name())
alertRule.SetRestored(true)
continue
}

Expand Down
3 changes: 3 additions & 0 deletions rules/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,9 @@ func TestForStateRestore(t *testing.T) {
return labels.Compare(got[i].Labels, got[j].Labels) < 0
})

// In all cases, we expect the restoration process to have completed.
require.Truef(t, newRule.Restored(), "expected the rule restoration proccess to have completed")

Check failure on line 486 in rules/manager_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

`proccess` is a misspelling of `process` (misspell)

Check failure on line 486 in rules/manager_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

`proccess` is a misspelling of `process` (misspell)

// Checking if we have restored it correctly.
switch {
case tt.noRestore:
Expand Down

0 comments on commit 0cff3bc

Please sign in to comment.