1342495 - allow perfherder to alert on absolute change, rather than just percent changed

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect)

Description

[William Lachance (:wlach)]

For some metrics, such as number of constructors or build size, it would be nice to be able to alert based on an absolute value, rather than absolute percentage change (e.g. for number of constructors we would like to alert on changes of 3 or greater, irrespective of what the value currently is).

I believe there are three steps to this:

1. Modify the schema to support a "alertChangeType" property for suite (an enumeration with types "percent" and "absolute", default should be percent). Also modify the description of alertThreshold to remove the reference to %.

https://github.com/mozilla/treeherder/blob/1ef2306/schemas/performance-artifact.json

2. Ingest the alertChangeType in the etl code

https://github.com/mozilla/treeherder/blob/1ef2306/treeherder/etl/perf.py#L87

3. Use the threshold in the code which actually generates alerts:

https://github.com/mozilla/treeherder/blob/1ef2306/treeherder/perf/alerts.py#L85

4. Add a test or two to make sure it works as expected:

https://github.com/mozilla/treeherder/blob/1ef2306/tests/perfalert/test_create_alerts.py

Comment 1

[William Lachance (:wlach)]

Attachment: PR

Comment 2

[Robert Wood [:rwood]]

Please see the question I left in the PR re: having an alertChangeType for suites as well as subtests, I'm a bit confused about that.

Also, are you going to change some existing tests from the current percentage-based alert change type to absolute value? If so, just concerned about new alerts popping up because of this. It may be hard when sheriffing alerts to track new alerts inadvertently caused when the change is made from percentage to absolute vs new real regressions. Although I guess if the abosolute value is based on historical data then it's pretty safe that invalid alerts won't be generated with the switch to abosolute alertChangeType...

Comment 3

[William Lachance (:wlach)]

(In reply to Robert Wood [:rwood] from comment #2)
> Please see the question I left in the PR re: having an alertChangeType for
> suites as well as subtests, I'm a bit confused about that.

Yes, the logic was incorrect. I'll fix it.

> Also, are you going to change some existing tests from the current
> percentage-based alert change type to absolute value? If so, just concerned
> about new alerts popping up because of this. It may be hard when sheriffing
> alerts to track new alerts inadvertently caused when the change is made from
> percentage to absolute vs new real regressions. Although I guess if the
> abosolute value is based on historical data then it's pretty safe that
> invalid alerts won't be generated with the switch to abosolute
> alertChangeType...

Alert generation only goes back to the last alert or the last two weeks, whichever is less.

The only test I plan on changing for now is the num constructors one (bug 1338210), which alerts on an almost daily basis. So the only affect should be a reduction in incoming alerts.

Comment 4

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/e702a309d906ed6379ea995115a0df6cdccf1149
Bug 1342495 - Allow perfherder to alert on abs change instead of percentage (#2242)