New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Alerting: Elide requests to Loki if nothing should be recorded #65011
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but please see my comment.
@@ -62,6 +62,11 @@ func (h *AnnotationBackend) Record(ctx context.Context, rule history_model.RuleM | |||
panel := parsePanelKey(rule, logger) | |||
|
|||
errCh := make(chan error, 1) | |||
if len(annotations) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only concern is that there's no distinction between recording and not recording to the caller of this function - but maybe this is captures by metrics and therefore is fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's how it already works to begin with. Right now the scheduler and state packages call Record on every evaluation, and they pass in in tons of states that we don't actually care about or actually want to record, like Alerting->Alerting. Regardless of this change, it relies on getting called a ton and nothing in the caller cares if it recorded or didn't.
Exit early if no log streams or annotations (cherry picked from commit e39d7f4)
Exit early if no log streams or annotations
What is this feature?
State history does filtering on state transitions and chooses not to write when nothing has changed.
When this happens, we'd have an empty slice of streams - we'd still attempt to write to Loki in this case, resulting in a pointless empty push request.
Now we exit early when this happens. This PR cuts down on a lot of outgoing loki traffic.
There was a similar change in Annotations to do this same thing, but for the database. Refactored this to exit even earlier. Now we don't even start any goroutines if there's nothing to write.
Which issue(s) does this PR fix?:
n/a
Special notes for your reviewer: