From f289eee9b2ea8f0c441210fe6196f3e7e85c0be0 Mon Sep 17 00:00:00 2001 From: Prapti Sharma Date: Thu, 7 Nov 2024 19:24:08 +0530 Subject: [PATCH] [fix] Fixed data collection if tx/rx stats missing #595 Fixes #595 --------- Co-authored-by: Federico Capoano --- openwisp_monitoring/device/tests/__init__.py | 7 +++-- openwisp_monitoring/device/tests/test_api.py | 30 ++++++++++++++++++++ openwisp_monitoring/device/writer.py | 6 ++-- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/openwisp_monitoring/device/tests/__init__.py b/openwisp_monitoring/device/tests/__init__.py index 98af8830..512a6ac4 100644 --- a/openwisp_monitoring/device/tests/__init__.py +++ b/openwisp_monitoring/device/tests/__init__.py @@ -86,10 +86,11 @@ def assertDataDict(self, dd_data, data): data = self._transform_wireless_interface_test_data(data) self.assertDictEqual(dd_data, data) - def create_test_data(self, no_resources=False): + def create_test_data(self, no_resources=False, data=None, assertions=True): o = self._create_org() d = self._create_device(organization=o) - data = self._data() + if not data: + data = self._data() # creation of resources metrics can be avoided in tests not involving them # this speeds up those tests by reducing requests made if no_resources: @@ -97,6 +98,8 @@ def create_test_data(self, no_resources=False): r = self._post_data(d.id, d.key, data) self.assertEqual(r.status_code, 200) dd = DeviceData(pk=d.pk) + if not assertions: + return dd self.assertDataDict(dd.data, data) if no_resources: metric_count, chart_count = 4, 4 diff --git a/openwisp_monitoring/device/tests/test_api.py b/openwisp_monitoring/device/tests/test_api.py index 4bcec5ef..03a7c621 100644 --- a/openwisp_monitoring/device/tests/test_api.py +++ b/openwisp_monitoring/device/tests/test_api.py @@ -1378,6 +1378,36 @@ def _assert_chart_group(url, status_code, expected): self.assertEqual(rows[-2].strip().split(','), ['ssh', '100.0']) self.assertEqual(rows[-1].strip().split(','), ['http2', '90.0']) + def test_missing_rx_bytes(self): + """ + Regression test for: + https://github.com/openwisp/openwisp-monitoring/issues/595 + """ + data = self._data() + del data['interfaces'][0]['statistics']['rx_bytes'] + dd = self.create_test_data(no_resources=True, data=data, assertions=False) + qs = Metric.objects.filter( + object_id=dd.pk, key='traffic', main_tags={'ifname': 'wlan0'} + ) + self.assertEqual(qs.count(), 1) + m = qs.first() + points = self._read_metric(m, limit=1, extra_fields=['tx_bytes']) + self.assertEqual(points[0].get('tx_bytes'), 145) + self.assertEqual(points[0].get('rx_bytes'), 0) + + def test_missing_tx_bytes(self): + data = self._data() + del data['interfaces'][0]['statistics']['tx_bytes'] + dd = self.create_test_data(no_resources=True, data=data, assertions=False) + qs = Metric.objects.filter( + object_id=dd.pk, key='traffic', main_tags={'ifname': 'wlan0'} + ) + self.assertEqual(qs.count(), 1) + m = qs.first() + points = self._read_metric(m, limit=1, extra_fields=['tx_bytes']) + self.assertEqual(points[0].get('rx_bytes'), 324) + self.assertEqual(points[0].get('tx_bytes'), 0) + class TestGeoApi(TestGeoMixin, AuthenticationMixin, DeviceMonitoringTestCase): location_model = Location diff --git a/openwisp_monitoring/device/writer.py b/openwisp_monitoring/device/writer.py index f09e4824..0d7950dc 100644 --- a/openwisp_monitoring/device/writer.py +++ b/openwisp_monitoring/device/writer.py @@ -79,14 +79,14 @@ def write(self, data, time=None, current=False): # Explicitly stated None to avoid skipping in case the stats are zero if ( ifstats.get('rx_bytes') is not None - and ifstats.get('rx_bytes') is not None + or ifstats.get('tx_bytes') is not None ): field_value = self._calculate_increment( - ifname, 'rx_bytes', ifstats['rx_bytes'] + ifname, 'rx_bytes', ifstats.get('rx_bytes', 0) ) extra_values = { 'tx_bytes': self._calculate_increment( - ifname, 'tx_bytes', ifstats['tx_bytes'] + ifname, 'tx_bytes', ifstats.get('tx_bytes', 0) ) } name = f'{ifname} traffic'