From e3ff6e42c00a6cd5cf8138aa20cafeefc1b02752 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 25 Aug 2023 18:58:51 +0530 Subject: [PATCH 01/10] [feature] Added organization filter to timeseries charts #532 - Added option to define "summary_query" for metrics - Added functionality to handle "GROUP by tags" clause to InfluxDB client Related to #532 --- .../db/backends/influxdb/client.py | 48 ++++++++++++++- .../db/backends/influxdb/tests.py | 42 ++++++++++++++ openwisp_monitoring/device/apps.py | 3 + openwisp_monitoring/monitoring/api/views.py | 28 +++++++++ openwisp_monitoring/monitoring/base/models.py | 8 +++ .../static/monitoring/js/chart-utils.js | 56 ++++++++++++++---- .../templates/monitoring/paritals/chart.html | 3 + .../monitoring/tests/__init__.py | 19 ++++++ .../monitoring/tests/test_api.py | 58 ++++++++++++++++--- .../monitoring/tests/test_charts.py | 34 +++++++++++ 10 files changed, 278 insertions(+), 21 deletions(-) diff --git a/openwisp_monitoring/db/backends/influxdb/client.py b/openwisp_monitoring/db/backends/influxdb/client.py index 338258c9d..f9c997536 100644 --- a/openwisp_monitoring/db/backends/influxdb/client.py +++ b/openwisp_monitoring/db/backends/influxdb/client.py @@ -256,7 +256,24 @@ def read(self, key, fields, tags, **kwargs): return list(self.query(q, precision='s').get_points()) def get_list_query(self, query, precision='s'): - return list(self.query(query, precision=precision).get_points()) + result = self.query(query, precision=precision) + if not len(result.keys()) or result.keys()[0][1] is None: + return list(result.get_points()) + # Handles query which contains "GROUP BY TAG" clause + result_points = {} + for (measurement, tag), group_points in result.items(): + tag_suffix = '_'.join(tag.values()) + for point in group_points: + values = {} + for key, value in point.items(): + if key != 'time': + values[f'{tag_suffix}'] = value + values['time'] = point['time'] + try: + result_points[values['time']].update(values) + except KeyError: + result_points[values['time']] = values + return list(result_points.values()) @retry def get_list_retention_policies(self): @@ -329,7 +346,12 @@ def get_query( query = f'{query} LIMIT 1' return f"{query} tz('{timezone}')" - _group_by_regex = re.compile(r'GROUP BY time\(\w+\)', flags=re.IGNORECASE) + _group_by_time_tag_regex = re.compile( + r'GROUP BY ((time\(\w+\))(?:,\s+\w+)?)', flags=re.IGNORECASE + ) + _group_by_time_regex = re.compile(r'GROUP BY time\(\w+\)\s?', flags=re.IGNORECASE) + _time_regex = re.compile(r'time\(\w+\)\s?', flags=re.IGNORECASE) + _time_comma_regex = re.compile(r'time\(\w+\),\s?', flags=re.IGNORECASE) def _group_by(self, query, time, chart_type, group_map, strip=False): if not self.validate_query(query): @@ -343,7 +365,27 @@ def _group_by(self, query, time, chart_type, group_map, strip=False): if 'GROUP BY' not in query.upper(): query = f'{query} {group_by}' else: - query = re.sub(self._group_by_regex, group_by, query) + # The query could have GROUP BY clause for a TAG + if group_by: + # The query already contains "GROUP BY", therefore + # we remove it from the "group_by" to avoid duplicating + # "GROUP BY" + group_by = group_by.replace('GROUP BY ', '') + # We only need to substitute the time function. + # The resulting query would be "GROUP BY time(), " + query = re.sub(self._time_regex, group_by, query) + else: + # The query should not include the "GROUP by time()" + matches = re.search(self._group_by_time_tag_regex, query) + group_by_fields = matches.group(1) + if len(group_by_fields.split(',')) > 1: + # If the query has "GROUP BY time(), tag", + # then return "GROUP BY tag" + query = re.sub(self._time_comma_regex, '', query) + else: + # If the query has only has "GROUP BY time()", + # then remove the "GROUP BY" clause + query = re.sub(self._group_by_time_regex, '', query) return query _fields_regex = re.compile( diff --git a/openwisp_monitoring/db/backends/influxdb/tests.py b/openwisp_monitoring/db/backends/influxdb/tests.py index c87d09c59..508bd3bc8 100644 --- a/openwisp_monitoring/db/backends/influxdb/tests.py +++ b/openwisp_monitoring/db/backends/influxdb/tests.py @@ -190,6 +190,48 @@ def test_get_query_30d(self): self.assertIn(str(last30d)[0:10], q) self.assertIn('group by time(24h)', q.lower()) + def test_group_by_tags(self): + self.assertEqual( + timeseries_db._group_by( + 'SELECT COUNT(item) FROM measurement GROUP BY time(1d)', + time='30d', + chart_type='stackedbar+lines', + group_map={'30d': '30d'}, + strip=False, + ), + 'SELECT COUNT(item) FROM measurement GROUP BY time(30d)', + ) + self.assertEqual( + timeseries_db._group_by( + 'SELECT COUNT(item) FROM measurement GROUP BY time(1d)', + time='30d', + chart_type='stackedbar+lines', + group_map={'30d': '30d'}, + strip=True, + ), + 'SELECT COUNT(item) FROM measurement ', + ) + self.assertEqual( + timeseries_db._group_by( + 'SELECT COUNT(item) FROM measurement GROUP BY time(1d), tag', + time='30d', + chart_type='stackedbar+lines', + group_map={'30d': '30d'}, + strip=False, + ), + 'SELECT COUNT(item) FROM measurement GROUP BY time(30d), tag', + ) + self.assertEqual( + timeseries_db._group_by( + 'SELECT COUNT(item) FROM measurement GROUP BY time(1d), tag', + time='30d', + chart_type='stackedbar+lines', + group_map={'30d': '30d'}, + strip=True, + ), + 'SELECT COUNT(item) FROM measurement GROUP BY tag', + ) + def test_retention_policy(self): manage_short_retention_policy() manage_default_retention_policy() diff --git a/openwisp_monitoring/device/apps.py b/openwisp_monitoring/device/apps.py index 739709878..3fb18463e 100644 --- a/openwisp_monitoring/device/apps.py +++ b/openwisp_monitoring/device/apps.py @@ -415,8 +415,11 @@ def register_dashboard_items(self): 'monitoring/css/percircle.min.css', 'monitoring/css/chart.css', 'monitoring/css/dashboard-chart.css', + 'admin/css/vendor/select2/select2.min.css', + 'admin/css/autocomplete.css', ), 'js': ( + 'admin/js/vendor/select2/select2.full.min.js', 'monitoring/js/lib/moment.min.js', 'monitoring/js/lib/daterangepicker.min.js', 'monitoring/js/lib/percircle.min.js', diff --git a/openwisp_monitoring/monitoring/api/views.py b/openwisp_monitoring/monitoring/api/views.py index 952a796c7..83f66be75 100644 --- a/openwisp_monitoring/monitoring/api/views.py +++ b/openwisp_monitoring/monitoring/api/views.py @@ -166,5 +166,33 @@ def invalidate_cache(cls, instance, *args, **kwargs): return cls._get_charts.invalidate() + def _get_user_managed_orgs(self, request): + """ + Return list of dictionary containing organization name and slug + in select2 compatible format. + """ + orgs = [] + qs = Organization.objects.only('slug', 'name') + if not request.user.is_superuser: + if len(request.user.organizations_managed) > 1: + qs = qs.filter(pk__in=request.user.organizations_managed) + else: + return orgs + for org in qs.iterator(): + orgs.append({'id': org.slug, 'text': org.name}) + if len(orgs) < 2: + # Handles scenarios for superuser when the project has only + # one organization. + return [] + return orgs + + def get(self, request, *args, **kwargs): + response = super().get(request, *args, **kwargs) + if not request.GET.get('csv'): + user_managed_orgs = self._get_user_managed_orgs(request) + if user_managed_orgs: + response.data['organizations'] = user_managed_orgs + return response + dashboard_timeseries = DashboardTimeseriesView.as_view() diff --git a/openwisp_monitoring/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py index f26aec6b7..649a21b6a 100644 --- a/openwisp_monitoring/monitoring/base/models.py +++ b/openwisp_monitoring/monitoring/base/models.py @@ -601,6 +601,12 @@ def query(self): return query[timeseries_db.backend_name] return self._default_query + @property + def summary_query(self): + query = self.config_dict.get('summary_query', None) + if query: + return query[timeseries_db.backend_name] + @property def top_fields(self): return self.config_dict.get('top_fields', None) @@ -659,6 +665,8 @@ def get_query( additional_params=None, ): query = query or self.query + if summary and self.summary_query: + query = self.summary_query additional_params = additional_params or {} params = self._get_query_params(time, start_date, end_date) params.update(additional_params) diff --git a/openwisp_monitoring/monitoring/static/monitoring/js/chart-utils.js b/openwisp_monitoring/monitoring/static/monitoring/js/chart-utils.js index 0781e63c6..058695272 100644 --- a/openwisp_monitoring/monitoring/static/monitoring/js/chart-utils.js +++ b/openwisp_monitoring/monitoring/static/monitoring/js/chart-utils.js @@ -197,6 +197,10 @@ django.jQuery(function ($) { url = `${url}&timezone=${timezone}`; } } + if ($('#org-selector').val()) { + var orgSlug = $('#org-selector').val(); + url = `${url}&organization_slug=${orgSlug}`; + } return url; }, createCharts = function (data){ @@ -213,6 +217,22 @@ django.jQuery(function ($) { createChart(chart, data.x, htmlId, chart.title, chart.type, chartQuickLink); }); }, + addOrganizationSelector = function (data) { + var orgSelector = $('#org-selector'); + if (data.organizations === undefined) { + orgSelector.hide(); + return; + } + if (orgSelector.data('select2-id') === 'org-selector') { + return; + } + orgSelector.select2({ + data: data.organizations, + allowClear: true, + placeholder: gettext('Organization Filter') + }); + orgSelector.show(); + }, loadCharts = function (time, showLoading) { $.ajax(getChartFetchUrl(time), { dataType: 'json', @@ -233,6 +253,7 @@ django.jQuery(function ($) { fallback.show(); } createCharts(data); + addOrganizationSelector(data); }, error: function (response) { var errorMessage = gettext('Something went wrong while loading the charts'); @@ -324,24 +345,30 @@ django.jQuery(function ($) { }); // bind export button $('#ow-chart-time a.export').click(function () { - var time = localStorage.getItem(timeRangeKey); - location.href = baseUrl + time + '&csv=1'; + var queryString, + queryParams = {'csv': 1}; + queryParams.time = localStorage.getItem(timeRangeKey); // If custom or pickerChosenLabelKey is 'Custom Range', pass pickerEndDate and pickerStartDate to csv url if (localStorage.getItem(isCustomDateRange) === 'true' || localStorage.getItem(pickerChosenLabelKey) === customDateRangeLabel) { - var startDate = localStorage.getItem(startDateTimeKey); - var endDate = localStorage.getItem(endDateTimeKey); + queryParams.start = localStorage.getItem(startDateTimeKey); + queryParams.end = localStorage.getItem(endDateTimeKey); if (localStorage.getItem(isChartZoomed) === 'true') { - time = localStorage.getItem(zoomtimeRangeKey); - endDate = localStorage.getItem(zoomEndDateTimeKey); - startDate = localStorage.getItem(zoomStartDateTimeKey); + queryParams.time = localStorage.getItem(zoomtimeRangeKey); + queryParams.end = localStorage.getItem(zoomEndDateTimeKey); + queryParams.start = localStorage.getItem(zoomStartDateTimeKey); } - var url = `${apiUrl}?start=${startDate}&end=${endDate}&csv=1`; - var timezone = Intl.DateTimeFormat().resolvedOptions().timeZone; + timezone = Intl.DateTimeFormat().resolvedOptions().timeZone; if (timezone) { - url = `${url}&timezone=${timezone}`; + queryParams.timezone = timezone; } - location.href = url; } + if ($('#org-selector').val()) { + queryParams.organization_slug = $('#org-selector').val(); + } + queryString = Object.keys(queryParams) + .map(key => `${encodeURIComponent(key)}=${encodeURIComponent(queryParams[key])}`) + .join('&'); + location.href = `${apiUrl}?${queryString}`; }); // fetch chart data and replace the old charts with the new ones function loadFetchedCharts(time){ @@ -358,5 +385,12 @@ django.jQuery(function ($) { }, }); } + + $('#org-selector').change(function(){ + loadCharts( + localStorage.getItem(timeRangeKey) || defaultTimeRange, + true + ); + }); }); }(django.jQuery)); diff --git a/openwisp_monitoring/monitoring/templates/monitoring/paritals/chart.html b/openwisp_monitoring/monitoring/templates/monitoring/paritals/chart.html index c2ee44764..5f3127df9 100644 --- a/openwisp_monitoring/monitoring/templates/monitoring/paritals/chart.html +++ b/openwisp_monitoring/monitoring/templates/monitoring/paritals/chart.html @@ -12,6 +12,9 @@ +
diff --git a/openwisp_monitoring/monitoring/tests/__init__.py b/openwisp_monitoring/monitoring/tests/__init__.py index 87b8836c5..8f50774e1 100644 --- a/openwisp_monitoring/monitoring/tests/__init__.py +++ b/openwisp_monitoring/monitoring/tests/__init__.py @@ -136,6 +136,25 @@ ) }, }, + 'group_by_tag': { + 'type': 'stackedbars', + 'title': 'Group by tag', + 'description': 'Query is groupped by tag along with time', + 'unit': 'n.', + 'order': 999, + 'query': { + 'influxdb': ( + "SELECT CUMULATIVE_SUM(SUM({field_name})) FROM {key} WHERE time >= '{time}'" + " GROUP BY time(1d), metric_num" + ) + }, + 'summary_query': { + 'influxdb': ( + "SELECT SUM({field_name}) FROM {key} WHERE time >= '{time}'" + " GROUP BY time(30d), metric_num" + ) + }, + }, 'mean_test': { 'type': 'line', 'title': 'Mean test', diff --git a/openwisp_monitoring/monitoring/tests/test_api.py b/openwisp_monitoring/monitoring/tests/test_api.py index 07d374721..3c89d07db 100644 --- a/openwisp_monitoring/monitoring/tests/test_api.py +++ b/openwisp_monitoring/monitoring/tests/test_api.py @@ -68,14 +68,12 @@ def _create_org_traffic_metric(self, org, interface_name): def _create_test_users(self, org): admin = self._create_admin() - org2_administrator = self._create_user() - OrganizationUser.objects.create( - user=org2_administrator, organization=org, is_admin=True - ) + org_admin = self._create_user() + OrganizationUser.objects.create(user=org_admin, organization=org, is_admin=True) groups = Group.objects.filter(name='Administrator') - org2_administrator.groups.set(groups) + org_admin.groups.set(groups) operator = self._create_operator() - return admin, org2_administrator, operator + return admin, org_admin, operator def test_wifi_client_chart(self): def _test_chart_properties(chart): @@ -242,7 +240,7 @@ def _test_chart_properties(chart): self.client.force_login(admin) with self.subTest('Test superuser retrieves metric for all organizations'): - with self.assertNumQueries(2): + with self.assertNumQueries(3): response = self.client.get(path) self.assertEqual(response.status_code, 200) self._test_response_data(response) @@ -675,3 +673,49 @@ def test_group_by_time(self): with self.subTest('Test with invalid group time'): response = self.client.get(path, {'time': '3w'}) self.assertEqual(response.status_code, 400) + + def test_organizations_list(self): + path = reverse('monitoring_general:api_dashboard_timeseries') + Organization.objects.all().delete() + org1 = self._create_org(name='org1', slug='org1') + admin, org_admin, _ = self._create_test_users(org1) + + self.client.force_login(admin) + with self.subTest('Superuser: Only one organization is present'): + response = self.client.get(path) + self.assertEqual(response.status_code, 200) + self.assertNotIn('organizations', response.data) + + org2 = self._create_org(name='org2', slug='org2', id=self.org2_id) + + with self.subTest('Superuser: Multiple organizations are present'): + response = self.client.get(path) + self.assertEqual(response.status_code, 200) + self.assertIn('organizations', response.data) + self.assertEqual(len(response.data['organizations']), 2) + self.assertEqual( + response.data['organizations'], + [{'id': org.slug, 'text': org.name} for org in [org1, org2]], + ) + + self.client.logout() + self.client.force_login(org_admin) + + with self.subTest('Non-superuser: Administrator of one organization'): + response = self.client.get(path) + self.assertEqual(response.status_code, 200) + self.assertNotIn('organizations', response.data) + + OrganizationUser.objects.create( + user=org_admin, organization=org2, is_admin=True + ) + + with self.subTest('Non-superuser: Administrator of multiple organizations'): + response = self.client.get(path) + self.assertEqual(response.status_code, 200) + self.assertIn('organizations', response.data) + self.assertEqual(len(response.data['organizations']), 2) + self.assertEqual( + response.data['organizations'], + [{'id': org.slug, 'text': org.name} for org in [org1, org2]], + ) diff --git a/openwisp_monitoring/monitoring/tests/test_charts.py b/openwisp_monitoring/monitoring/tests/test_charts.py index a823c46bc..23680ee4f 100644 --- a/openwisp_monitoring/monitoring/tests/test_charts.py +++ b/openwisp_monitoring/monitoring/tests/test_charts.py @@ -164,6 +164,40 @@ def test_read_multiple(self): self.assertEqual(charts[0][1], [3, 2, 1]) self.assertEqual(charts[1][1], [4, 3, 2]) + def test_read_group_by_tag(self): + m1 = self._create_object_metric( + name='test metric 1', + key='test_metric', + field_name='item', + extra_tags={'metric_num': 'first'}, + ) + m2 = self._create_object_metric( + name='test metric 2', + key='test_metric', + field_name='item', + content_object=self._create_org(), + extra_tags={'metric_num': 'second'}, + ) + c = self._create_chart(metric=m1, test_data=False, configuration='group_by_tag') + now_ = now() + for n in range(0, 3): + time = now_ - timedelta(days=n) + m1.write(n + 1, time=time) + m2.write(n + 2, time=time) + data = self._read_chart(c, time='30d', start_date=time) + self.assertIn('x', data) + self.assertIn('traces', data) + self.assertEqual(len(data['x']), 3) + charts = data['traces'] + self.assertIn('first', charts[0][0]) + self.assertIn('second', charts[1][0]) + self.assertEqual(len(charts[0][1]), 3) + self.assertEqual(len(charts[1][1]), 3) + self.assertEqual(charts[0][1], [3, 5, 6]) + self.assertEqual(charts[1][1], [4, 7, 9]) + self.assertEqual(data['summary']['first'], 6) + self.assertEqual(data['summary']['second'], 9) + def test_json(self): c = self._create_chart() data = self._read_chart(c) From 5b48eb5246703ea48b69cd89e9bdbcdc26ecd1ce Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 29 Aug 2023 17:46:15 +0530 Subject: [PATCH 02/10] [fix] Fixed chart utils on narrow screens --- .../static/monitoring/css/chart.css | 9 ++++++++ .../static/monitoring/css/dashboard-chart.css | 16 +++++++++++--- .../static/monitoring/js/chart-utils.js | 4 ++-- .../templates/monitoring/paritals/chart.html | 21 +++++++++++-------- openwisp_monitoring/tests/test_selenium.py | 2 +- 5 files changed, 37 insertions(+), 15 deletions(-) diff --git a/openwisp_monitoring/monitoring/static/monitoring/css/chart.css b/openwisp_monitoring/monitoring/static/monitoring/css/chart.css index 7aee17bdc..fb6f430f0 100644 --- a/openwisp_monitoring/monitoring/static/monitoring/css/chart.css +++ b/openwisp_monitoring/monitoring/static/monitoring/css/chart.css @@ -147,3 +147,12 @@ display: none; font-size: 1.2em; } +@media (max-width: 768px) { + #ow-chart-utils { + display: block; + } + #ow-chart-utils > span + span { + margin: 5px 0px; + justify-content: end; + } +} diff --git a/openwisp_monitoring/monitoring/static/monitoring/css/dashboard-chart.css b/openwisp_monitoring/monitoring/static/monitoring/css/dashboard-chart.css index 4a341c119..a9385e9ee 100644 --- a/openwisp_monitoring/monitoring/static/monitoring/css/dashboard-chart.css +++ b/openwisp_monitoring/monitoring/static/monitoring/css/dashboard-chart.css @@ -7,9 +7,6 @@ #ow-chart-inner-container .quick-link-container { margin-top: 10px; } -#ow-chart-time { - margin-top: 9px; -} .ow-chart:first-child { margin-top: 45px; } @@ -17,3 +14,16 @@ margin-top: 23px; padding-top: 23px; } +.select2-dropdown { + min-width: 200px !important; +} +#ow-chart-utils > span { + display: inline; + width: auto; +} +@media (max-width: 768px) { + #ow-chart-utils > span { + display: flex; + width: 100%; + } +} diff --git a/openwisp_monitoring/monitoring/static/monitoring/js/chart-utils.js b/openwisp_monitoring/monitoring/static/monitoring/js/chart-utils.js index 058695272..3584854b7 100644 --- a/openwisp_monitoring/monitoring/static/monitoring/js/chart-utils.js +++ b/openwisp_monitoring/monitoring/static/monitoring/js/chart-utils.js @@ -220,12 +220,12 @@ django.jQuery(function ($) { addOrganizationSelector = function (data) { var orgSelector = $('#org-selector'); if (data.organizations === undefined) { - orgSelector.hide(); return; } if (orgSelector.data('select2-id') === 'org-selector') { return; } + orgSelector.parent().show(); orgSelector.select2({ data: data.organizations, allowClear: true, @@ -344,7 +344,7 @@ django.jQuery(function ($) { ); }); // bind export button - $('#ow-chart-time a.export').click(function () { + $('#ow-chart-export').click(function () { var queryString, queryParams = {'csv': 1}; queryParams.time = localStorage.getItem(timeRangeKey); diff --git a/openwisp_monitoring/monitoring/templates/monitoring/paritals/chart.html b/openwisp_monitoring/monitoring/templates/monitoring/paritals/chart.html index 5f3127df9..9049258b0 100644 --- a/openwisp_monitoring/monitoring/templates/monitoring/paritals/chart.html +++ b/openwisp_monitoring/monitoring/templates/monitoring/paritals/chart.html @@ -5,16 +5,19 @@ -
- {% trans 'export data' %} - + - - - -
diff --git a/openwisp_monitoring/tests/test_selenium.py b/openwisp_monitoring/tests/test_selenium.py index 6e797187a..8bab42117 100644 --- a/openwisp_monitoring/tests/test_selenium.py +++ b/openwisp_monitoring/tests/test_selenium.py @@ -165,7 +165,7 @@ def test_dashboard_timeseries_charts(self): self.fail('Timeseries chart container not found on dashboard') try: WebDriverWait(self.web_driver, 5).until( - EC.visibility_of_element_located((By.CSS_SELECTOR, '#ow-chart-time')) + EC.visibility_of_element_located((By.CSS_SELECTOR, '#ow-chart-utils')) ) except TimeoutException: self.fail('Timeseries chart time filter not found on dashboard') From 111d6f0113fb5c289104d022213ce37da24b00e5 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 29 Aug 2023 18:24:29 +0530 Subject: [PATCH 03/10] [feature] Added setting to set the DEFAULT_CHART_TIME --- README.rst | 14 ++++++++++++++ openwisp_monitoring/monitoring/base/models.py | 4 ++-- openwisp_monitoring/settings.py | 1 + 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 1e7442d90..33ee7161c 100644 --- a/README.rst +++ b/README.rst @@ -2088,6 +2088,20 @@ In case you just want to change the colors used in a chart here's how to do it: } } +``OPENWISP_MONITORING_DEFAULT_CHART_TIME`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++---------------------+---------------------------------------------+ +| **type**: | ``str`` | ++---------------------+---------------------------------------------+ +| **default**: | ``7d`` | ++---------------------+---------------------------------------------+ +| **possible values** | ``1d``, ``3d``, ``7d``, ``30d`` or ``365d`` | ++---------------------+---------------------------------------------+ + +This setting allows you to set the default time period for showing +charts. + ``OPENWISP_MONITORING_AUTO_CLEAR_MANAGEMENT_IP`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/openwisp_monitoring/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py index 649a21b6a..b2ead2a71 100644 --- a/openwisp_monitoring/monitoring/base/models.py +++ b/openwisp_monitoring/monitoring/base/models.py @@ -25,7 +25,7 @@ from openwisp_utils.base import TimeStampedEditableModel from ...db import default_chart_query, timeseries_db -from ...settings import CACHE_TIMEOUT +from ...settings import CACHE_TIMEOUT, DEFAULT_CHART_TIME from ..configuration import ( CHART_CONFIGURATION_CHOICES, DEFAULT_COLORS, @@ -495,7 +495,7 @@ class AbstractChart(TimeStampedEditableModel): max_length=16, null=True, choices=CHART_CONFIGURATION_CHOICES ) GROUP_MAP = {'1d': '10m', '3d': '20m', '7d': '1h', '30d': '24h', '365d': '7d'} - DEFAULT_TIME = '7d' + DEFAULT_TIME = DEFAULT_CHART_TIME class Meta: abstract = True diff --git a/openwisp_monitoring/settings.py b/openwisp_monitoring/settings.py index e47e0b9c7..e44c6ef22 100644 --- a/openwisp_monitoring/settings.py +++ b/openwisp_monitoring/settings.py @@ -28,3 +28,4 @@ def get_settings_value(option, default=None): 'CACHE_TIMEOUT', 24 * 60 * 60, # 24 hours in seconds ) +DEFAULT_CHART_TIME = get_settings_value('DEFAULT_CHART_TIME', '7d') From 045b6c985dfd127c45b18d50e03418e8a4b3dfe2 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 13 Nov 2023 07:05:13 +0530 Subject: [PATCH 04/10] [req-changes] Updated README --- README.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 33ee7161c..223c4b980 100644 --- a/README.rst +++ b/README.rst @@ -2099,8 +2099,7 @@ In case you just want to change the colors used in a chart here's how to do it: | **possible values** | ``1d``, ``3d``, ``7d``, ``30d`` or ``365d`` | +---------------------+---------------------------------------------+ -This setting allows you to set the default time period for showing -charts. +Allows to set the default time period of the time series charts. ``OPENWISP_MONITORING_AUTO_CLEAR_MANAGEMENT_IP`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 845d1dcad988aea11d70f5ecf3031de3cef2e254 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 8 Mar 2024 21:57:57 +0530 Subject: [PATCH 05/10] [req-changes] Added space between pie charts and date filter --- .../static/monitoring/css/chart.css | 62 +++++++++---------- .../static/monitoring/css/dashboard-chart.css | 1 + 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/openwisp_monitoring/monitoring/static/monitoring/css/chart.css b/openwisp_monitoring/monitoring/static/monitoring/css/chart.css index fb6f430f0..29e09f73d 100644 --- a/openwisp_monitoring/monitoring/static/monitoring/css/chart.css +++ b/openwisp_monitoring/monitoring/static/monitoring/css/chart.css @@ -1,40 +1,38 @@ #ow-chart-fallback { text-align: center; } -#ow-chart-time { - margin-top: 15px; - text-align: center; - padding: 4px; -} -#ow-chart-time a { - display: inline-block; - margin: 0 2px; - color: #fff; - background-color: #333; - padding: 5px 15px; - text-decoration: none !important; - border-radius: 3px; - cursor: pointer; -} -#ow-chart-time a:hover { - background-color: #777 !important; -} -#ow-chart-time a.active { - background: transparent; - color: #666; - cursor: default; - border: 1px solid #666; -} -#ow-chart-time a.export { - float: right; - background: #333; +#ow-chart-utils { + display: flex; + justify-content: space-between; +} +#ow-chart-utils .select2-container { + min-width: 150px !important; + width: unset !important; +} +#ow-chart-utils span.select2-selection { + min-height: 27px; + min-width: 200px; + height: 27px; +} +#ow-chart-utils .select2-selection__rendered { + line-height: 27px; + height: 27px; +} +#ow-chart-utils .select2-selection__arrow { + top: 0; + height: 27px; +} +#ow-chart-utils>span { + display: flex; + justify-content: space-between; + width: 100%; } -#ow-chart-time a#daterangepicker-widget { - float: left; - margin-right: 100px; +#ow-chart-utils .button { + font-size: 14px; + padding: 5px 10px; } -#ow-chart-time a.export:hover { - background: #777; +#ow-chart-export { + margin-left: 10px; } #ow-chart-fallback { display: none; diff --git a/openwisp_monitoring/monitoring/static/monitoring/css/dashboard-chart.css b/openwisp_monitoring/monitoring/static/monitoring/css/dashboard-chart.css index a9385e9ee..2dd1f9d82 100644 --- a/openwisp_monitoring/monitoring/static/monitoring/css/dashboard-chart.css +++ b/openwisp_monitoring/monitoring/static/monitoring/css/dashboard-chart.css @@ -1,4 +1,5 @@ #ow-chart-inner-container { + margin-top: 25px; padding: 0px 40px; } #ow-chart-inner-container .js-plotly-plot { From 2f4463b96299b661ba5de5d2dc77efd416057f39 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Thu, 14 Mar 2024 22:54:16 +0530 Subject: [PATCH 06/10] [feature] Added option to filter metric with __all__ organization_id by default --- openwisp_monitoring/db/backends/influxdb/client.py | 4 ++-- openwisp_monitoring/monitoring/base/models.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/openwisp_monitoring/db/backends/influxdb/client.py b/openwisp_monitoring/db/backends/influxdb/client.py index f9c997536..38786d75e 100644 --- a/openwisp_monitoring/db/backends/influxdb/client.py +++ b/openwisp_monitoring/db/backends/influxdb/client.py @@ -260,14 +260,14 @@ def get_list_query(self, query, precision='s'): if not len(result.keys()) or result.keys()[0][1] is None: return list(result.get_points()) # Handles query which contains "GROUP BY TAG" clause - result_points = {} + result_points = OrderedDict() for (measurement, tag), group_points in result.items(): tag_suffix = '_'.join(tag.values()) for point in group_points: values = {} for key, value in point.items(): if key != 'time': - values[f'{tag_suffix}'] = value + values[tag_suffix] = value values['time'] = point['time'] try: result_points[values['time']].update(values) diff --git a/openwisp_monitoring/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py index b2ead2a71..7e4f8a25d 100644 --- a/openwisp_monitoring/monitoring/base/models.py +++ b/openwisp_monitoring/monitoring/base/models.py @@ -671,6 +671,8 @@ def get_query( params = self._get_query_params(time, start_date, end_date) params.update(additional_params) params.update({'start_date': start_date, 'end_date': end_date}) + if not params.get('organization_id') and self.config_dict.get('filter__all__'): + params['organization_id'] = ['__all__'] return timeseries_db.get_query( self.type, params, From fc3691906b2feaa78dfa0a3d0cca726b10c5a839 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 15 Mar 2024 13:57:56 +0530 Subject: [PATCH 07/10] [req-changes] Changed filter__all__ to __all__ --- openwisp_monitoring/db/backends/influxdb/client.py | 2 +- openwisp_monitoring/monitoring/base/models.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/openwisp_monitoring/db/backends/influxdb/client.py b/openwisp_monitoring/db/backends/influxdb/client.py index 38786d75e..906769a00 100644 --- a/openwisp_monitoring/db/backends/influxdb/client.py +++ b/openwisp_monitoring/db/backends/influxdb/client.py @@ -260,7 +260,7 @@ def get_list_query(self, query, precision='s'): if not len(result.keys()) or result.keys()[0][1] is None: return list(result.get_points()) # Handles query which contains "GROUP BY TAG" clause - result_points = OrderedDict() + result_points = {} for (measurement, tag), group_points in result.items(): tag_suffix = '_'.join(tag.values()) for point in group_points: diff --git a/openwisp_monitoring/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py index 7e4f8a25d..72ad45e6c 100644 --- a/openwisp_monitoring/monitoring/base/models.py +++ b/openwisp_monitoring/monitoring/base/models.py @@ -671,7 +671,7 @@ def get_query( params = self._get_query_params(time, start_date, end_date) params.update(additional_params) params.update({'start_date': start_date, 'end_date': end_date}) - if not params.get('organization_id') and self.config_dict.get('filter__all__'): + if not params.get('organization_id') and self.config_dict.get('__all__', False): params['organization_id'] = ['__all__'] return timeseries_db.get_query( self.type, From 1dd6bec49c219813e0b0ecc79f9410db9f8dc678 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 18 Mar 2024 21:48:14 +0530 Subject: [PATCH 08/10] [feature] Added support to override metrics added with register_metric method --- .../monitoring/configuration.py | 26 ++++++- .../monitoring/tests/test_configuration.py | 78 +++++++++++++------ 2 files changed, 81 insertions(+), 23 deletions(-) diff --git a/openwisp_monitoring/monitoring/configuration.py b/openwisp_monitoring/monitoring/configuration.py index 448771222..9f0de2ef9 100644 --- a/openwisp_monitoring/monitoring/configuration.py +++ b/openwisp_monitoring/monitoring/configuration.py @@ -1,4 +1,5 @@ from collections import OrderedDict +from copy import deepcopy from django.core.exceptions import ImproperlyConfigured from django.utils.translation import gettext_lazy as _ @@ -713,7 +714,19 @@ def unregister_metric_notifications(metric_name): def get_metric_configuration(): - metrics = deep_merge_dicts(DEFAULT_METRICS, app_settings.ADDITIONAL_METRICS) + additional_metrics = deepcopy(app_settings.ADDITIONAL_METRICS) + for metric_name in list(additional_metrics.keys()): + if additional_metrics[metric_name].get('partial', False): + # A partial configuration can be defined in the settings.py + # with OPENWISP_MONITORING_METRICS setting to override + # metrics that are added with register_metric method in + # other django apps. + # Since, the partial configuration could be defined to + # override limited fields in the configuration, hence + # we don't validate the configuration here. Instead. + # configuration is validated in the register_metric method. + del additional_metrics[metric_name] + metrics = deep_merge_dicts(DEFAULT_METRICS, additional_metrics) # ensure configuration is not broken for metric_config in metrics.values(): _validate_metric_configuration(metric_config) @@ -741,6 +754,17 @@ def register_metric(metric_name, metric_config): raise ImproperlyConfigured( f'{metric_name} is an already registered Metric Configuration.' ) + if metric_name in app_settings.ADDITIONAL_METRICS: + # There is partial configuration present for this "metric_name" in + # ADDITIONAL_METRICS. We need to merge the partial configuration with + # the registered metric before validating. Otherwise, users won't be + # able to override registered metrics using OPENWISP_MONITORING_METRICS + # setting. + metric_config = deep_merge_dicts( + metric_config, + app_settings.ADDITIONAL_METRICS[metric_name], + ) + metric_config.pop('partial', None) _validate_metric_configuration(metric_config) for chart in metric_config.get('charts', {}).values(): _validate_chart_configuration(chart_config=chart) diff --git a/openwisp_monitoring/monitoring/tests/test_configuration.py b/openwisp_monitoring/monitoring/tests/test_configuration.py index 1df932734..4ee11b669 100644 --- a/openwisp_monitoring/monitoring/tests/test_configuration.py +++ b/openwisp_monitoring/monitoring/tests/test_configuration.py @@ -1,8 +1,12 @@ +from unittest.mock import patch + from django.core.exceptions import ImproperlyConfigured from django.test import TestCase from django.utils.translation import gettext_lazy as _ +from .. import settings as app_settings from ..configuration import ( + DEFAULT_METRICS, get_metric_configuration, get_metric_configuration_choices, register_metric, @@ -12,6 +16,30 @@ class TestConfiguration(TestMonitoringMixin, TestCase): + def _get_new_metric(self): + return { + 'label': _('Histogram'), + 'name': 'Histogram', + 'key': 'histogram', + 'field_name': 'value', + 'charts': { + 'histogram': { + 'type': 'histogram', + 'title': 'Histogram', + 'description': 'Histogram', + 'top_fields': 2, + 'order': 999, + 'query': { + 'influxdb': ( + "SELECT {fields|SUM|/ 1} FROM {key} " + "WHERE time >= '{time}' AND content_type = " + "'{content_type}' AND object_id = '{object_id}'" + ) + }, + } + }, + } + def test_register_invalid_metric_config(self): with self.subTest('Test Invalid Metric name'): with self.assertRaisesMessage( @@ -45,30 +73,36 @@ def test_register_duplicate_metric(self): ): register_metric(m.configuration, metric_config) + @patch.dict(DEFAULT_METRICS) def test_register_metric_configuration(self): - metric_config = { - 'label': _('Histogram'), - 'name': 'Histogram', - 'key': 'histogram', - 'field_name': 'value', - 'charts': { - 'histogram': { - 'type': 'histogram', - 'title': 'Histogram', - 'description': 'Histogram', - 'top_fields': 2, - 'order': 999, - 'query': { - 'influxdb': ( - "SELECT {fields|SUM|/ 1} FROM {key} " - "WHERE time >= '{time}' AND content_type = " - "'{content_type}' AND object_id = '{object_id}'" - ) - }, - } - }, - } + metric_config = self._get_new_metric() register_metric('histogram', metric_config) self.assertIn(metric_config, get_metric_configuration().values()) self.assertIn(('histogram', 'Histogram'), get_metric_configuration_choices()) unregister_metric('histogram') + + @patch.dict(DEFAULT_METRICS) + @patch.object( + app_settings, + 'ADDITIONAL_METRICS', + { + 'histogram': { + 'partial': True, + 'name': 'Partial Updated', + 'charts': { + 'histogram': { + 'title': 'Partial Updated', + } + }, + } + }, + ) + def test_overriding_registered_metric_configuration(self): + metric_config = self._get_new_metric() + register_metric('histogram', metric_config) + metrics = get_metric_configuration() + self.assertIn('histogram', metrics.keys()) + self.assertEqual(metrics['histogram']['name'], 'Partial Updated') + self.assertEqual( + metrics['histogram']['charts']['histogram']['title'], 'Partial Updated' + ) From c9be866e95dfa440b9290082ae1aea459e52345d Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Thu, 21 Mar 2024 20:00:05 +0530 Subject: [PATCH 09/10] [req-changes] Improved CSS --- .../device/static/monitoring/css/wifi-sessions.css | 4 ---- .../device/static/monitoring/js/wifi-session-inline.js | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/openwisp_monitoring/device/static/monitoring/css/wifi-sessions.css b/openwisp_monitoring/device/static/monitoring/css/wifi-sessions.css index 42b06130c..d0c57ebae 100644 --- a/openwisp_monitoring/device/static/monitoring/css/wifi-sessions.css +++ b/openwisp_monitoring/device/static/monitoring/css/wifi-sessions.css @@ -1,10 +1,6 @@ td.original p { display: none; } -#inline-wifisession-quick-link-container { - text-align: center; - margin: 20px 0 40px; -} #wifisession_set-group { margin-bottom: 0px; } diff --git a/openwisp_monitoring/device/static/monitoring/js/wifi-session-inline.js b/openwisp_monitoring/device/static/monitoring/js/wifi-session-inline.js index af51281a4..add627ed5 100644 --- a/openwisp_monitoring/device/static/monitoring/js/wifi-session-inline.js +++ b/openwisp_monitoring/device/static/monitoring/js/wifi-session-inline.js @@ -27,7 +27,7 @@ if (typeof gettext === 'undefined') { wifiSessionLinkElement; wifiSessionUrl = `${wifiSessionUrl}?device=${getObjectIdFromUrl()}`; wifiSessionLinkElement = ` -