Skip to content
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

Promtool unit test rules support values as a range #5352

Open
bosschaert opened this Issue Mar 13, 2019 · 10 comments

Comments

Projects
None yet
3 participants
@bosschaert
Copy link

bosschaert commented Mar 13, 2019

Proposal

Promtool test cases don't always provide exact values, hence expected values with an allowable delta should be supported.

Bug Report

I am using the promtool unit testing support to test my alerts and recording rules. However, probably due to inaccuracies in floating point math, the test result of my rule is not precise.

- record: avg_read_latency_5m
  expr: rate(duration_sum[5m])/rate(duration_count[5m])

And then I have the following test

tests:
  - interval: 10s 
    input_series: 
    - series: 'duration_sum'
      values: '0+0.4x120'
    - series: 'duration_count'
      values: '0+1x120'
    promql_expr_test:
    - expr: avg_read_latency_5m
      eval_time: 10m
      exp_samples:
        - labels: 'avg_read_latency_5m'
          value: 0.4

However, the above test fails with:

  FAILED:
    expr:'avg_read_latency_5m', time:10m0s,
        exp:"{__name__=\"avg_read_latency_5m\"} 4E-01, {__name__=\"avg_read_latency_5m\"} 4E-01",
        got:"{__name__=\"avg_read_latency_5m\"} 3.999999999999991E-01, {__name__=\"avg_read_latency_5m\"} 3.999999999999991E-01"

In that case it would be good to be able to, instead of using 0.4 as expected value, be able to specify something like 0.4 ±3% or something like this.

  • System information:

    Darwin 18.2.0 x86_64

  • Prometheus version:

    promtool, version 2.7.1 (branch: HEAD, revision: 62e591f)
    build user: root@f9f82868fc43
    build date: 20190131-11:16:59
    go version: go1.11.5

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 13, 2019

In prometheus/docs#1305 the potential bug is that the answer is .9991 rather than .999. This behaviour here is as expected.

@bosschaert

This comment has been minimized.

Copy link
Author

bosschaert commented Mar 13, 2019

If the behaviour here is as expected how can I write a test that checks for 0.4, if it fails with 0.3999999999999991 ? Isn't this standard floating point inaccuracy?

Other testing frameworks support deltas for situations like this, e.g. Junit: https://junit.org/junit4/javadoc/latest/org/junit/Assert.html#assertEquals(java.lang.String,%20double,%20double,%20double)

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 13, 2019

You need to use 0.3999999999999991 in that case.

@bosschaert

This comment has been minimized.

Copy link
Author

bosschaert commented Mar 13, 2019

Well, 2.4/6 equals to 0.4, so it seems wrong to specify 0.3999999999999991 even though that is in practice an acceptable value...

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 13, 2019

To be honest, I wouldn't even test this simple a rule. It's more for alerts, and non-trivial PromQL.

@bosschaert

This comment has been minimized.

Copy link
Author

bosschaert commented Mar 13, 2019

But if I can't even declare a clear test for such a simple rule, where does that leave me for complex ones?

@codesome

This comment has been minimized.

Copy link
Member

codesome commented Mar 14, 2019

Quoting prometheus/docs#1305 (comment): as PromQL is meant to be deterministic, if we want to check within a delta, I think it should be internal to promtool and not specified in the yaml.

@bosschaert

This comment has been minimized.

Copy link
Author

bosschaert commented Mar 14, 2019

@codesome that would be fine with me too, as long as I can state 0.4 and don't have to state 0.3999999999999991.
BTW note that this is just an example, for illustration purposes. It obviously also applies to more complex rules and alerts.

@codesome

This comment has been minimized.

Copy link
Member

codesome commented Mar 14, 2019

@brian-brazil would check within a delta make sense for unit testing? Or the current way is the way it should be?

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 14, 2019

It wouldn't make sense, as you'd silently be letting changes happen without the tests failing. We depend on this for some of our floating-point accuracy related tests for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.