зеркало из https://github.com/mozilla/treeherder.git
Bug 1342495 - Allow perfherder to alert on abs change instead of percentage (#2242)
This commit is contained in:
William Lachance
GitHub
Родитель
296ecb19c3
Коммит
e702a309d9
|
|
@ -32,12 +32,17 @@
|
|||
"title": "Should alert",
|
||||
"type": "boolean"
|
||||
},
|
||||
"alertChangeType": {
|
||||
"description": "Type of change to alert on (absolute or percentage)",
|
||||
"title": "Type of change to alert on",
|
||||
"enum": ["absolute", "percentage"],
|
||||
"type": "string"
|
||||
},
|
||||
"alertThreshold": {
|
||||
"description": "% change threshold before alerting",
|
||||
"description": "Change threshold before alerting",
|
||||
"title": "Alert threshold",
|
||||
"type": "number",
|
||||
"minimum": 0.0,
|
||||
"maximum": 1000.0
|
||||
"minimum": 0.0
|
||||
},
|
||||
"minBackWindow": {
|
||||
"description": "Minimum back window to use for alerting",
|
||||
|
|
@ -105,11 +110,16 @@
|
|||
"type": "boolean"
|
||||
},
|
||||
"alertThreshold": {
|
||||
"description": "% change threshold before alerting",
|
||||
"description": "Change threshold before alerting",
|
||||
"title": "Alert threshold",
|
||||
"type": "number",
|
||||
"minimum": 0.0,
|
||||
"maximum": 1000.0
|
||||
"minimum": 0.0
|
||||
},
|
||||
"alertChangeType": {
|
||||
"description": "Type of change to alert on (absolute or percentage)",
|
||||
"title": "Type of change to alert on",
|
||||
"enum": ["absolute", "percentage"],
|
||||
"type": "string"
|
||||
},
|
||||
"minBackWindow": {
|
||||
"description": "Minimum back window to use for alerting",
|
||||
|
|
|
|||
|
|
@ -92,6 +92,7 @@ def _verify_signature(repo_name, framework_name, suitename,
|
|||
testname, option_collection_hash, platform,
|
||||
lower_is_better, extra_opts,
|
||||
last_updated=None, alert_threshold=None,
|
||||
alert_change_type=None,
|
||||
min_back_window=None, max_back_window=None,
|
||||
fore_window=None):
|
||||
if not extra_opts:
|
||||
|
|
@ -112,6 +113,11 @@ def _verify_signature(repo_name, framework_name, suitename,
|
|||
assert signature.min_back_window == min_back_window
|
||||
assert signature.max_back_window == max_back_window
|
||||
assert signature.fore_window == fore_window
|
||||
if alert_change_type is not None:
|
||||
assert signature.get_alert_change_type_display() == alert_change_type
|
||||
else:
|
||||
assert signature.alert_change_type is None
|
||||
|
||||
# only verify last updated if explicitly specified
|
||||
if last_updated:
|
||||
assert signature.last_updated == last_updated
|
||||
|
|
@ -341,6 +347,19 @@ def test_same_signature_multiple_performance_frameworks(test_repository,
|
|||
# summary
|
||||
(True, {'shouldAlert': True},
|
||||
{'shouldAlert': True}, True, True),
|
||||
# summary+subtest, alerting override on subtest +
|
||||
# summary, but using absolute change so shouldn't
|
||||
# alert
|
||||
(True,
|
||||
{'shouldAlert': True, 'alertChangeType': 'absolute'},
|
||||
{'shouldAlert': True, 'alertChangeType': 'absolute'},
|
||||
False, False),
|
||||
# summary + subtest, only subtest is absolute so
|
||||
# summary should alert
|
||||
(True,
|
||||
{'shouldAlert': True},
|
||||
{'shouldAlert': True, 'alertChangeType': 'absolute'},
|
||||
False, True),
|
||||
])
|
||||
def test_alert_generation(test_repository,
|
||||
failure_classifications, generic_reference_data,
|
||||
|
|
@ -363,6 +382,7 @@ def test_alert_generation(test_repository,
|
|||
True,
|
||||
None,
|
||||
alert_threshold=extra_subtest_metadata.get('alertThreshold'),
|
||||
alert_change_type=extra_subtest_metadata.get('alertChangeType'),
|
||||
min_back_window=extra_subtest_metadata.get('minBackWindow'),
|
||||
max_back_window=extra_subtest_metadata.get('maxBackWindow'),
|
||||
fore_window=extra_subtest_metadata.get('foreWindow'))
|
||||
|
|
@ -376,6 +396,7 @@ def test_alert_generation(test_repository,
|
|||
True,
|
||||
None,
|
||||
alert_threshold=extra_suite_metadata.get('alertThreshold'),
|
||||
alert_change_type=extra_suite_metadata.get('alertChangeType'),
|
||||
min_back_window=extra_suite_metadata.get('minBackWindow'),
|
||||
max_back_window=extra_suite_metadata.get('maxBackWindow'),
|
||||
fore_window=extra_suite_metadata.get('foreWindow'))
|
||||
|
|
|
|||
|
|
@ -1,11 +1,14 @@
|
|||
import datetime
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
from treeherder.model.models import Push
|
||||
from treeherder.perf.alerts import generate_new_alerts_in_series
|
||||
from treeherder.perf.models import (PerformanceAlert,
|
||||
PerformanceAlertSummary,
|
||||
PerformanceDatum)
|
||||
PerformanceDatum,
|
||||
PerformanceSignature)
|
||||
|
||||
|
||||
def _verify_alert(alertid, expected_push_id, expected_prev_push_id,
|
||||
|
|
@ -166,3 +169,32 @@ def test_custom_alert_threshold(
|
|||
|
||||
assert PerformanceAlert.objects.count() == 1
|
||||
assert PerformanceAlertSummary.objects.count() == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize(('new_value', 'expected_num_alerts'),
|
||||
[(1.0, 1), (0.25, 0)])
|
||||
def test_alert_change_type_absolute(test_repository,
|
||||
failure_classifications,
|
||||
generic_reference_data,
|
||||
test_perf_signature, new_value,
|
||||
expected_num_alerts):
|
||||
# modify the test signature to say that we alert on absolute value
|
||||
# (as opposed to percentage change)
|
||||
test_perf_signature.alert_change_type = PerformanceSignature.ALERT_ABS
|
||||
test_perf_signature.alert_threshold = 0.3
|
||||
test_perf_signature.save()
|
||||
|
||||
base_time = time.time() # generate it based off current time
|
||||
INTERVAL = 30
|
||||
_generate_performance_data(test_repository, test_perf_signature,
|
||||
generic_reference_data,
|
||||
base_time, 1, 0.5, INTERVAL/2)
|
||||
_generate_performance_data(test_repository, test_perf_signature,
|
||||
generic_reference_data,
|
||||
base_time, (INTERVAL/2) + 1, new_value,
|
||||
INTERVAL/2)
|
||||
|
||||
generate_new_alerts_in_series(test_perf_signature)
|
||||
|
||||
assert PerformanceAlert.objects.count() == expected_num_alerts
|
||||
assert PerformanceAlertSummary.objects.count() == expected_num_alerts
|
||||
|
|
|
|||
|
|
@ -99,6 +99,8 @@ def _load_perf_datum(job, perf_datum):
|
|||
# these properties below can be either True, False, or null
|
||||
# (None). Null indicates no preference has been set.
|
||||
'should_alert': suite.get('shouldAlert'),
|
||||
'alert_change_type': PerformanceSignature._get_alert_change_type(
|
||||
suite.get('alertChangeType')),
|
||||
'alert_threshold': suite.get('alertThreshold'),
|
||||
'min_back_window': suite.get('minBackWindow'),
|
||||
'max_back_window': suite.get('maxBackWindow'),
|
||||
|
|
@ -151,6 +153,8 @@ def _load_perf_datum(job, perf_datum):
|
|||
# null (None). Null indicates no preference has been
|
||||
# set.
|
||||
'should_alert': subtest.get('shouldAlert'),
|
||||
'alert_change_type': PerformanceSignature._get_alert_change_type(
|
||||
subtest.get('alertChangeType')),
|
||||
'alert_threshold': subtest.get('alertThreshold'),
|
||||
'min_back_window': subtest.get('minBackWindow'),
|
||||
'max_back_window': subtest.get('maxBackWindow'),
|
||||
|
|
|
|||
|
|
@ -7,7 +7,8 @@ from django.db import transaction
|
|||
|
||||
from treeherder.perf.models import (PerformanceAlert,
|
||||
PerformanceAlertSummary,
|
||||
PerformanceDatum)
|
||||
PerformanceDatum,
|
||||
PerformanceSignature)
|
||||
from treeherder.perfalert import (Datum,
|
||||
detect_changes)
|
||||
|
||||
|
|
@ -82,9 +83,13 @@ def generate_new_alerts_in_series(signature):
|
|||
alert_properties = get_alert_properties(
|
||||
prev_value, new_value, signature.lower_is_better)
|
||||
|
||||
if alert_properties.pct_change < alert_threshold:
|
||||
# ignore regressions below the configured regression
|
||||
# threshold
|
||||
# ignore regressions below the configured regression
|
||||
# threshold
|
||||
if ((signature.alert_change_type is None or
|
||||
signature.alert_change_type == PerformanceSignature.ALERT_PCT) and
|
||||
alert_properties.pct_change < alert_threshold) or \
|
||||
(signature.alert_change_type == PerformanceSignature.ALERT_ABS and
|
||||
alert_properties.delta < alert_threshold):
|
||||
continue
|
||||
|
||||
summary, _ = PerformanceAlertSummary.objects.get_or_create(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,20 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# Generated by Django 1.10.5 on 2017-03-10 18:27
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('perf', '0002_remove_signature_hash_index'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='performancesignature',
|
||||
name='alert_change_type',
|
||||
field=models.IntegerField(choices=[(0, b'percentage'), (1, b'absolute')], null=True),
|
||||
),
|
||||
]
|
||||
|
|
@ -55,12 +55,30 @@ class PerformanceSignature(models.Model):
|
|||
|
||||
# these properties override the default settings for how alert
|
||||
# generation works
|
||||
ALERT_PCT = 0
|
||||
ALERT_ABS = 1
|
||||
ALERT_CHANGE_TYPES = ((ALERT_PCT, 'percentage'),
|
||||
(ALERT_ABS, 'absolute'))
|
||||
|
||||
should_alert = models.NullBooleanField()
|
||||
alert_change_type = models.IntegerField(choices=ALERT_CHANGE_TYPES,
|
||||
null=True)
|
||||
alert_threshold = models.FloatField(null=True)
|
||||
min_back_window = models.IntegerField(null=True)
|
||||
max_back_window = models.IntegerField(null=True)
|
||||
fore_window = models.IntegerField(null=True)
|
||||
|
||||
@staticmethod
|
||||
def _get_alert_change_type(alert_change_type_input):
|
||||
'''
|
||||
Maps a schema-specified alert change type to the internal index
|
||||
value
|
||||
'''
|
||||
for (idx, enum_val) in PerformanceSignature.ALERT_CHANGE_TYPES:
|
||||
if enum_val == alert_change_type_input:
|
||||
return idx
|
||||
return None
|
||||
|
||||
class Meta:
|
||||
db_table = 'performance_signature'
|
||||
# make sure there is only one signature per repository with a
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче