Persist alert 'for' state across restarts

[brian-brazil]

Currently if Prometheus restarts, we lose the 'for' state for firing alerts. While this isn't an issue for short for clauses, it presents a problem for clauses that are in the hours to days range.

It'd be good to persist this state in some way, so that alerts don't have to start again from scratch. We probably don't want to count the time the prometheus server is down against the 'for' clause.

[gouthamve]

How would this work? Do we evaluate from the point we terminated on startup again as a way of catching up?

For example take an alert A1 with FOR 12h that has started life at 0000. Now it is pending and we shut down the server at 0600 and we persist A1 pending for 6h at 0600. Suppose we restart at 01500, do we start evaluating from 0600-01500 and then say catched up, continue normal evaluation? Isn't this going to be costly?

Are we going to have a threshold? Something like you lose alert state if Prometheus is down for more than 1h? We can make this a flag to support use-cases where the server was down for a long time.

[brian-brazil]

Suppose we restart at 01500, do we start evaluating from 0600-01500 and then say catched up, continue normal evaluation?

No, and the alert probably wouldn't be firing during that period anyway as there'll be no data from the Prometheus going down.

What we want is that we remember that 6h of the FOR has already been satisfied, and when it's satisfied for another 6h then we alert. We need to be a little careful here, for safety if the remaining time is <10m we should treat that to min(10m, for).

To implement I was thinking an analouge to ALERTs that'd track when an alert started, and when Promethus restarts it'd check that metric (adjusting if needed for the 10m safety) and initialise internal state accordingly.

One other thing is that it can take a while for a Prometheus to warm up and have done enough scrapes for alerts to start firing properly. Thus we should delay rule&alert evaluation for 2 scrape_intervals at starttup to mitigate this.

[gouthamve]

Okay, biggest facepalm moment ever! How could I not see that the server is not running! Sorry about that!

Coming to when to start evaluation, will 2 global scrape_interval be okay? Because we might be alerting on series that are scraped much less frequently. And what about alerts on range queries?

[brian-brazil]

global scrape interval is all that makes sense.

Range queries will usually be rate(), and two scrapes+slack should be enough for that to work.

[jacksontj]

Do we want to persist state? If state is persisted then it means that the alerts require a timeseries store locally to enforce the alerting. This is fine for prometheus itself (although it is storing data we might not care about) but does pose a problem for things that use the alerting libraries to enforce promql alerts to remote storage systems. I had hacked up an approach for backfilling on startup, but that doesn't cover the case that you mention here that you don't want to count "prom downtime" against the alert FOR interval. Maybe it'd be helpful to make some interface for "load alert state" that can be passed in (prometheus itself can store/load from a timeseries, others can re-query etc.)?

[brian-brazil]

Do we want to persist state?

That's kind of the entire point of this PR. If you're running some non-standard system then you won't get the benefit of this feature, but alerting will continue to function as it does today.

[jacksontj]

I assume then that this functionality would be optional (through some config)?

[brian-brazil]

I see no reason why this functionality would not be always on in the Prometheus server. The Prometheus server always has a local tsdb.

[jacksontj]

Quoting from myself:

If state is persisted then it means that the alerts require a timeseries store locally to enforce the alerting. This is fine for prometheus itself (although it is storing data we might not care about) but does pose a problem for things that use the alerting libraries to enforce promql alerts to remote storage systems.

TLDR; What about others using the library?

[brian-brazil]

It's an internal API so we offer no guarantees there, but you can pass in whatever storage objects you like.

[gouthamve]

Closed by #4061

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.