General purpose rule/alert testing tool

[tamsky]

https://groups.google.com/d/msg/prometheus-developers/h6Vp2pf619A/AJsKJsJTHQAJ
DL;DR (very edited) summary:

Q on (8/7/15) by @swsnider : https://github.com/prometheus/prometheus/promql/test.go -- Is it appropriate to write some wrapper code to turn this into a general-purpose rule/alert testing tool?

A by @fabxc :
It is absolutely on our roadmap (should probably be added to the actual one on the website).
What's blocking it is not so much implementation but specification.

I would like to help get the specification ball rolling here.

@fabxc Has there been any work done to spec this out since that post?

[fabxc]

Yes, getting the discussion started would help a lot and make it easier for people willing to contribute to start working on something. So far this area has not progressed.

For these kinds of things an RFC in a shared Google doc generally works best.

[tamsky]

Let me get the ball rolling with a few ideas here, a google doc can follow.

(in the same groups thread) @fabxc wrote:
the syntax for evaluation is something that strongly bothers me. Specifying time (ranges) is fine for the current testing but not suitable for a more general purpose language.

Perhaps you can put into a few more words what bothers you regarding the current time (range) stuff, and alternatives that might help?

The current test syntax (to me) appears to be very simple, which is good:
load <step:duration>)
eval[_fail|_ordered] instant [at <offset:duration>] <query>

Concepts I think are missing:

• ability to include ../*.rules
• NOW <date:dateTime>|<[-+]offset:duration>
• evaluation_interval (allows rule testing tool to interact with the global config param)
• eval_vector (accepts an instant-vector, and drops all labels)
• eval range
• a way to express missed scrapes in load which should affect metric staleness

NOW would provide a way to set the date+time for all actions that follow, simplifying eval commands. Example ::

clear
evaluation_interval 5m
include "../rules/*.rules"

NOW 2016-06-02T00:00:00
load 5m
    http_errors{result="404"} 0+10x12 120x12   # steady error rate for 1h, zero additional for 1h

    # eval without an 'at' clause implies clock == NOW command above
    eval instant ALERT{alertname="http_errors", alertstate!~"pending", alertstate!~"firing"}
         # no data here == alert is not active

NOW 2016-06-02T00:30:00
    eval_vector range ALERT{alertname="http_errors", alertstate="pending"}[30m]   
    {}  1 1 1 1 0        # pending -> firing transition

NOW +30m
    eval_vector range ALERT{alertname="http_errors", alertstate="firing"}[10m]
    {}  1 0        # firing -> cleared

Especially as promtool should eventually replace prometheus_cli and the should use the same language for that.

Are there any query syntax changes proposed yet (in #1605 or elsewhere)?

[fabxc]

Unfortunately we didn't follow up on the topic for a while even though we strongly want it.

One main concern I have in hindsight is the DSL we currently use for testing PromQL. While it's brief and precise, it's yet another language we have to parse, and more importantly, specify ourselves.
Especially as we want to extend it as discussed here, this seems to be asking for trouble. We should discuss switching to a bit more noisy but easier to handle YAML.

Especially as we want to have the following, partially overlapping, features:

• easy to define PromQL tests
• easy to define PromQL benchmarks
• alerting rule unit testing for users (which can probably be reduced to test data + PromQL expression)

I also don't see the DSL incorporate loading of arbitrary test/benchmarking sample data well, which won't be specifiable in 20 characters.

[brian-brazil]

alerting rule unit testing for users (which can probably be reduced to test data + PromQL expression)

We need rule file parsing and evaluation for that too, as there can be several layers of rules between data and alerts.

[gouthamve]

For alerts, this should also have the ability to test that the label and annotation templates expand to the expected values.

[juliusv]

@gouthamve That part is already mentioned/covered in #1220, although it could be additionally relevant here too.

[juliusv]

And also #1219.

[fabxc]

If we want to actually test not just whether the expression triggers but also whether it generates the right annotations and labels, we need to consider the full alert.
Copying an alert into a unit test file works but will with high certainty drift over time. So practically, it would be good to just reference it by <rulfile>/<alertname>. This won't work because alert names don't have to be unique, even on a per-file basis. The only other good option then seems to be inlining of unit tests directly into the rule files. That might be a bit noisy though.

Would be open to enforcing alertname uniqueness (I don't think many people use duplicates anyway) or does anyone have other ideas to address this?

[juliusv]

I think using the same alertname with different severity labels (expression with different thresholds) would be a common use case.

[brian-brazil]

Yes, duplicate alertnames with different labels is common. There could also be duplicates across rules from different teams.

[fabxc]

Forgot about warn/page level alerts with the same name – haven't written alerts in a while and it shows :)

Anyway, so how would we tackle this then? Inline unit tests directly in rules files or rely on users to keep their alerts and their copies in unit tests in sync?

[brian-brazil]

I would have it that unit tests can pull in the rule files/groups that are relevant.

[fabxc]

Yea sure, but how is this supposed to happen if there's no unique identifier to reference a rule by?

[brian-brazil]

This is why I've been saying that rule groups should usually be named.

[juliusv]

Or just specify a (list of) rule files to load in whole, a list of series and their samples as input (similarly to how we load in data for testing expressions), and the entirety of expected received alerts / notifications as output. That still allows you to test only one alert per test if you want (simply reference the same rule files from multiple test cases, but with different expected inputs / outputs). It also handles the case nicely where an alerting rule depends on other recording rules, etc.

[fabxc]

Mh, that might actually work – makes it easy to define cases triggering warn and severe at once too. Would still constrain that to <file>:<group_name> as references though.

@gouthamve

[aweiteka]

I have a high interest in rules testing. I've tried m-lab/prometheus-support#30 and like the general pattern.

I'll be at KubeCon 2017 in Austin this week if folks want to discuss.

[gouthamve]

Sure, we would love to discuss this. Please catch us at any of the events: https://prometheus.io/blog/2017/11/30/prometheus-at-cloudnativecon/ Or hop onto IRC (#prometheus on freenode) to say Hi!

…

On Tue, Dec 5, 2017 at 10:00 AM Aaron Weitekamp ***@***.***> wrote: I have a high interest in rules testing. I've tried m-lab/prometheus-support#30 <m-lab/prometheus-support#30> and like the general pattern. I'll be at KubeCon 2017 in Austin this week if folks want to discuss. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1695 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHA3HxRVPbuvKYKUKIP2nj8HucsNSzCsks5s9WiEgaJpZM4Iq39t> .

[serathius]

Is anyone working on this issue? I would be interested in pushing it further with some help.

[brian-brazil]

Noone is working on this that I'm aware of.

[gouthamve]

/cc @codesome who wants to put this in his GSoC proposal

[codesome]

Yes, I plan to add this in my GSoC proposal

[codesome]

https://groups.google.com/forum/#!topic/prometheus-developers/l92C6R1jzRI

[kevinjqiu]

Ahh didn't know it's a feature being actively worked on. At my work, we came up with this https://github.com/kevinjqiu/pat and hopefully this can be of some use to someone.

[DeathBorn]

for some reson this pat tool does not work with record rules included in alerts.

[kevinjqiu]

@DeathBorn Could you open an issue @ https://github.com/kevinjqiu/pat/issues ?

[codesome]

Got added in #4350.
@brian-brazil I think we can close this.

[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.