Skip to content

Commit

Permalink
[requested-changes] Removed alert_on_related field
Browse files Browse the repository at this point in the history 
- Removed alert_on_related feature from this PR.
  • Loading branch information
@Aryamanz29
Aryamanz29 committed Aug 9, 2022
1 parent b12d9b5 commit e0e23d9
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 199 deletions.
7 changes: 0 additions & 7 deletions README.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1949,13 +1949,6 @@ The ``AlertSettings`` of ``ping`` metric will by default use ``threshold`` and `
defined in the ``alert_settings`` key.
You can always override them and define your own custom values via the *admin*.

You can also use ``alert_on_related_field`` key in metric configuration
which allows ``AlertSettings`` to use ``related_field`` as value to check ``threshold`` instead
of default ``field_name`` key. A real world example of this can be seen in
`Iperf metric configuration
<https://github.com/openwisp/openwisp-monitoring/blob/master/openwisp_monitoring/monitoring/configuration.py#L546-L700>`_,
Where we used ``jitter`` (related_field) for alerts.

**Note**: It will raise ``ImproperlyConfigured`` exception if a metric configuration
is already registered with same name (not to be confused with verbose_name).

Expand Down
9 changes: 0 additions & 9 deletions openwisp_monitoring/check/classes/iperf.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ def _get_metric(self):
"""
metric, created = self._get_or_create_metric()
if created:
self._create_alert_settings(metric)
self._create_charts(metric)
return metric

Expand All @@ -361,11 +360,3 @@ def _create_charts(self, metric):
chart = Chart(metric=metric, configuration=chart)
chart.full_clean()
chart.save()

def _create_alert_settings(self, metric):
"""
Creates iperf alertsettings
"""
alert_settings = AlertSettings(metric=metric)
alert_settings.full_clean()
alert_settings.save()
31 changes: 1 addition & 30 deletions openwisp_monitoring/monitoring/base/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,21 +355,6 @@ def write(
'send_alert': send_alert,
}
options['metric_pk'] = self.pk

# check if alert_on_related_field is present in metric configuration
if 'alert_on_related_field' in self.config_dict:
related_field = self.config_dict['alert_on_related_field']
if not extra_values:
raise ValueError(
'write() missing positional argument: "extra_values" required for alert on related field'
)
if related_field not in extra_values.keys():
raise ValueError(
f'"{key}" is not defined for alert_on_related_field in metric configuration'
)
options['check_threshold_kwargs'].update(
{'value': extra_values[related_field]}
)
timeseries_write.delay(name=self.key, values=values, **options)

def read(self, **kwargs):
Expand Down Expand Up @@ -768,19 +753,13 @@ def _is_crossed_by(self, current_value, time=None, retention_policy=None):
return value_crossed
# tolerance is set, we must go back in time
# to ensure the threshold is trepassed for enough time
# check if alert_on_related_field is present in metric configuration
if 'alert_on_related_field' in self.metric.config_dict:
alert_on_related_field = [self.metric.config_dict['alert_on_related_field']]
else:
alert_on_related_field = []
if time is None:
# retrieves latest measurements, ordered by most recent first
points = self.metric.read(
since=f'{self._tolerance_search_range}m',
limit=None,
order='-time',
retention_policy=retention_policy,
extra_fields=alert_on_related_field,
)
# store a list with the results
results = [value_crossed]
Expand All @@ -792,15 +771,7 @@ def _is_crossed_by(self, current_value, time=None, retention_policy=None):
continue
utc_time = utc.localize(datetime.utcfromtimestamp(point['time']))
# did this point cross the threshold? Append to result list
# check if alert_on_related_field is present in metric configuration
if 'alert_on_related_field' in self.metric.config_dict:
results.append(
self._value_crossed(
point[self.metric.config_dict['alert_on_related_field']]
)
)
else:
results.append(self._value_crossed(point[self.metric.field_name]))
results.append(self._value_crossed(point[self.metric.field_name]))
# tolerance is trepassed
if self._time_crossed(utc_time):
# if the latest results are consistent, the metric being
Expand Down
32 changes: 0 additions & 32 deletions openwisp_monitoring/monitoring/configuration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,6 @@ def _get_access_tech():
'lost_packets',
'lost_percent',
],
'alert_on_related_field': 'jitter',
'charts': {
'bandwidth': {
'type': 'scatter',
Expand Down Expand Up @@ -658,33 +657,6 @@ def _get_access_tech():
'colors': [DEFAULT_COLORS[3]],
},
},
'alert_settings': {'operator': '>', 'threshold': 5, 'tolerance': 0},
'notification': {
'problem': {
'verbose_name': 'Iperf PROBLEM',
'verb': _('iperf test jitter is greater than normal value'),
'level': 'warning',
'email_subject': _(
'[{site.name}] PROBLEM: {notification.target} {notification.verb}'
),
'message': _(
'The device [{notification.target}]({notification.target_link}) '
'{notification.verb}.'
),
},
'recovery': {
'verbose_name': 'Iperf RECOVERY',
'verb': _('iperf test jitter now back to normal'),
'level': 'info',
'email_subject': _(
'[{site.name}] RECOVERY: {notification.target} {notification.verb}'
),
'message': _(
'The device [{notification.target}]({notification.target_link}) '
'{notification.verb}.'
),
},
},
},
}

Expand All @@ -698,10 +670,6 @@ def _validate_metric_configuration(metric_config):
assert 'name' in metric_config
assert 'key' in metric_config
assert 'field_name' in metric_config
if 'alert_on_related_field' in metric_config:
assert (
metric_config['alert_on_related_field'] in metric_config['related_fields']
)


def _validate_chart_configuration(chart_config):
Expand Down
9 changes: 0 additions & 9 deletions openwisp_monitoring/monitoring/tests/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,6 @@
'label': 'Test Metric',
'notification': test_notification,
},
'test_alert_on_rf': {
'name': 'test_alert_related',
'key': '{key}',
'field_name': '{field_name}',
'label': 'Test alert related',
'notification': test_notification,
'related_fields': ['test_related_1', 'test_related_2', 'test_related_3'],
'alert_on_related_field': 'test_related_2',
},
'top_fields_mean': {
'name': 'top_fields_mean_test',
'key': '{key}',
Expand Down
112 changes: 0 additions & 112 deletions openwisp_monitoring/monitoring/tests/test_monitoring_notifications.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,118 +402,6 @@ def test_alerts_disabled(self):
self.assertEqual(d.monitoring.status, 'problem')
self.assertEqual(Notification.objects.count(), 0)

def test_alert_on_related_field(self):
admin = self._create_admin()
m = self._create_general_metric(configuration='test_alert_on_rf')
self._create_alert_settings(
metric=m, custom_operator='>', custom_threshold=30, custom_tolerance=0
)

with self.subTest('Test notification for metric without related field'):
with self.assertRaises(ValueError) as err:
m.write(1)
self.assertEqual(
str(err.exception),
'write() missing positional argument: "extra_values" required for alert on related field',
)
m.refresh_from_db()
self.assertEqual(m.is_healthy, True)
self.assertEqual(m.is_healthy_tolerant, True)
self.assertEqual(Notification.objects.count(), 0)

with self.subTest('Test notification for metric on different related field'):
with self.assertRaises(ValueError) as err:
m.write(10, extra_values={'test_related_3': 40})
self.assertEqual(
str(err.exception),
'"test_related_3" is not defined for alert_on_related_field in metric configuration',
)
m.refresh_from_db()
self.assertEqual(m.is_healthy, True)
self.assertEqual(m.is_healthy_tolerant, True)
self.assertEqual(Notification.objects.count(), 0)

with self.subTest('Test notification for metric with multiple related fields'):
m.write(10, extra_values={'test_related_2': 40, 'test_related_3': 20})
m.refresh_from_db()
self.assertEqual(m.is_healthy, False)
self.assertEqual(m.is_healthy_tolerant, False)
self.assertEqual(Notification.objects.count(), 1)
n = notification_queryset.first()
self.assertEqual(n.recipient, admin)
self.assertEqual(n.actor, m)
self.assertEqual(n.action_object, m.alertsettings)
self.assertEqual(n.level, 'warning')

with self.subTest(
'Test notification for metric exceeding related field alert settings'
):
m.write(10, extra_values={'test_related_2': 40})
m.refresh_from_db()
self.assertEqual(m.is_healthy, False)
self.assertEqual(m.is_healthy_tolerant, False)
self.assertEqual(Notification.objects.count(), 1)
n = notification_queryset.first()
self.assertEqual(n.recipient, admin)
self.assertEqual(n.actor, m)
self.assertEqual(n.action_object, m.alertsettings)
self.assertEqual(n.level, 'warning')

with self.subTest(
'Test no double alarm for metric exceeding related field alert settings'
):
m.write(20, extra_values={'test_related_2': 35})
m.refresh_from_db()
self.assertEqual(m.is_healthy, False)
self.assertEqual(m.is_healthy_tolerant, False)
self.assertEqual(Notification.objects.count(), 1)

with self.subTest(
'Test notification for metric falling behind related field alert settings'
):
m.write(30, extra_values={'test_related_2': 25})
m.refresh_from_db()
self.assertEqual(m.is_healthy, True)
self.assertEqual(m.is_healthy_tolerant, True)
self.assertEqual(Notification.objects.count(), 2)
n = notification_queryset.last()
self.assertEqual(n.recipient, admin)
self.assertEqual(n.actor, m)
self.assertEqual(n.action_object, m.alertsettings)
self.assertEqual(n.level, 'info')

def test_general_check_threshold_with_alert_on_rf_crossed_deferred(self):
admin = self._create_admin()
m = self._create_general_metric(configuration='test_alert_on_rf')
self._create_alert_settings(
metric=m, custom_operator='>', custom_threshold=30, custom_tolerance=1
)
m.write(10, time=ten_minutes_ago, extra_values={'test_related_2': 35})
m.refresh_from_db()
self.assertEqual(m.is_healthy, False)
self.assertEqual(m.is_healthy_tolerant, False)
self.assertEqual(Notification.objects.count(), 1)
n = notification_queryset.first()
self.assertEqual(n.recipient, admin)
self.assertEqual(n.actor, m)
self.assertEqual(n.action_object, m.alertsettings)
self.assertEqual(n.level, 'warning')

def test_general_check_threshold_with_alert_on_rf_deferred_not_crossed(self):
self._create_admin()
m = self._create_general_metric(configuration='test_alert_on_rf')
self._create_alert_settings(
metric=m, custom_operator='>', custom_threshold=30, custom_tolerance=1
)
m.write(10, extra_values={'test_related_2': 32})
self.assertEqual(m.is_healthy, True)
self.assertEqual(m.is_healthy_tolerant, True)
self.assertEqual(Notification.objects.count(), 0)
m.write(20, extra_values={'test_related_2': 35})
self.assertEqual(m.is_healthy, True)
self.assertEqual(m.is_healthy_tolerant, True)
self.assertEqual(Notification.objects.count(), 0)


class TestTransactionMonitoringNotifications(DeviceMonitoringTransactionTestcase):
device_model = Device
Expand Down

0 comments on commit e0e23d9

Please sign in to comment.