diff --git a/schemas/performance-artifact.json b/schemas/performance-artifact.json index f79a0419b..891ab96f9 100644 --- a/schemas/performance-artifact.json +++ b/schemas/performance-artifact.json @@ -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", diff --git a/tests/etl/test_perf_data_adapters.py b/tests/etl/test_perf_data_adapters.py index 70d3eb094..31cb571e9 100644 --- a/tests/etl/test_perf_data_adapters.py +++ b/tests/etl/test_perf_data_adapters.py @@ -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')) diff --git a/tests/perfalert/test_create_alerts.py b/tests/perfalert/test_create_alerts.py index 575821f07..195d07f85 100644 --- a/tests/perfalert/test_create_alerts.py +++ b/tests/perfalert/test_create_alerts.py @@ -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 diff --git a/treeherder/etl/perf.py b/treeherder/etl/perf.py index 982e20359..7709f7026 100644 --- a/treeherder/etl/perf.py +++ b/treeherder/etl/perf.py @@ -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'), diff --git a/treeherder/perf/alerts.py b/treeherder/perf/alerts.py index 958a07405..d6365ca5a 100644 --- a/treeherder/perf/alerts.py +++ b/treeherder/perf/alerts.py @@ -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( diff --git a/treeherder/perf/migrations/0003_add_performance_signature_alert_change_type.py b/treeherder/perf/migrations/0003_add_performance_signature_alert_change_type.py new file mode 100644 index 000000000..6701e5b08 --- /dev/null +++ b/treeherder/perf/migrations/0003_add_performance_signature_alert_change_type.py @@ -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), + ), + ] diff --git a/treeherder/perf/models.py b/treeherder/perf/models.py index 42fdcb7fc..86772374f 100644 --- a/treeherder/perf/models.py +++ b/treeherder/perf/models.py @@ -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