diff --git a/.github/workflows/dashboards.yml b/.github/workflows/dashboards.yml index 6f8ff7d7813..ed6b169c352 100644 --- a/.github/workflows/dashboards.yml +++ b/.github/workflows/dashboards.yml @@ -91,6 +91,9 @@ jobs: with: fetch-depth: 0 + - name: Test the cleanup script + run: python3 -m unittest discover -s dashboards/misc -p 'test_*.py' -v + - name: Get changed dashboard files id: changed run: | diff --git a/.gitignore b/.gitignore index 026903dd185..dc21e256a82 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,10 @@ fuzzing/ *-fuzz.zip *.bench +# Python (dashboards/misc scripts and their tests) +__pycache__/ +*.pyc + # QAN /qan-api2/logs/ diff --git a/dashboards/dashboards/Insight/Home_Dashboard.json b/dashboards/dashboards/Insight/Home_Dashboard.json index b11bcd0731a..c9110649415 100644 --- a/dashboards/dashboards/Insight/Home_Dashboard.json +++ b/dashboards/dashboards/Insight/Home_Dashboard.json @@ -2417,7 +2417,7 @@ } ], "timeFrom": "1h", - "title": "CPU Anomaly ", + "title": "CPU Anomaly", "type": "timeseries" }, { diff --git a/dashboards/dashboards/Kubernetes (experimental)/Databases_on_Kubernetes.json b/dashboards/dashboards/Kubernetes (experimental)/Databases_on_Kubernetes.json index 489b788767e..491d8417b43 100644 --- a/dashboards/dashboards/Kubernetes (experimental)/Databases_on_Kubernetes.json +++ b/dashboards/dashboards/Kubernetes (experimental)/Databases_on_Kubernetes.json @@ -832,7 +832,7 @@ "refId": "A" } ], - "title": "Storage Utilization ", + "title": "Storage Utilization", "type": "timeseries" }, { diff --git a/dashboards/dashboards/MongoDB/MongoDB_Instances_Compare.json b/dashboards/dashboards/MongoDB/MongoDB_Instances_Compare.json index f94de173f17..b83dff8a6ad 100644 --- a/dashboards/dashboards/MongoDB/MongoDB_Instances_Compare.json +++ b/dashboards/dashboards/MongoDB/MongoDB_Instances_Compare.json @@ -1069,7 +1069,7 @@ "step": 20 } ], - "title": "$service_name - Cursors ", + "title": "$service_name - Cursors", "type": "timeseries" }, { diff --git a/dashboards/dashboards/MySQL/HAProxy_Instance_Summary.json b/dashboards/dashboards/MySQL/HAProxy_Instance_Summary.json index ec67c1312e8..ff41fa5bc51 100644 --- a/dashboards/dashboards/MySQL/HAProxy_Instance_Summary.json +++ b/dashboards/dashboards/MySQL/HAProxy_Instance_Summary.json @@ -2095,7 +2095,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "Aborted Data Transfers ", + "title": "Aborted Data Transfers", "tooltip": { "msResolution": false, "shared": true, diff --git a/dashboards/dashboards/MySQL/MySQL_Group_Replication_Summary.json b/dashboards/dashboards/MySQL/MySQL_Group_Replication_Summary.json index 4fafea34eb0..05e7e4d58ea 100644 --- a/dashboards/dashboards/MySQL/MySQL_Group_Replication_Summary.json +++ b/dashboards/dashboards/MySQL/MySQL_Group_Replication_Summary.json @@ -1143,7 +1143,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "Applied Transactions ", + "title": "Applied Transactions", "tooltip": { "msResolution": false, "shared": true, @@ -1248,7 +1248,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "Sent Transactions ", + "title": "Sent Transactions", "tooltip": { "msResolution": false, "shared": true, @@ -1458,7 +1458,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "Rolled Back Transactions ", + "title": "Rolled Back Transactions", "tooltip": { "msResolution": false, "shared": true, diff --git a/dashboards/dashboards/MySQL/MySQL_InnoDB_Compression_Details.json b/dashboards/dashboards/MySQL/MySQL_InnoDB_Compression_Details.json index 81ed8aa4816..450755ced98 100644 --- a/dashboards/dashboards/MySQL/MySQL_InnoDB_Compression_Details.json +++ b/dashboards/dashboards/MySQL/MySQL_InnoDB_Compression_Details.json @@ -1485,7 +1485,7 @@ "step": 300 } ], - "title": "Buffer Pool Size ", + "title": "Buffer Pool Size", "type": "stat" }, { diff --git a/dashboards/dashboards/MySQL/MySQL_InnoDB_Details.json b/dashboards/dashboards/MySQL/MySQL_InnoDB_Details.json index 1c95e5c67ed..2b5caa62b3a 100644 --- a/dashboards/dashboards/MySQL/MySQL_InnoDB_Details.json +++ b/dashboards/dashboards/MySQL/MySQL_InnoDB_Details.json @@ -170,7 +170,7 @@ "step": 300 } ], - "title": "Buffer Pool Size ", + "title": "Buffer Pool Size", "type": "stat" }, { @@ -1728,7 +1728,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "InnoDB Read-Only Transactions ", + "title": "InnoDB Read-Only Transactions", "tooltip": { "msResolution": false, "shared": true, @@ -1864,7 +1864,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "InnoDB Read-Write Transactions ", + "title": "InnoDB Read-Write Transactions", "tooltip": { "msResolution": false, "shared": true, @@ -2138,7 +2138,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "Misc InnoDB Transactions Information ", + "title": "Misc InnoDB Transactions Information", "tooltip": { "msResolution": false, "shared": true, @@ -2808,7 +2808,7 @@ "step": 300 } ], - "title": "Space Used ", + "title": "Space Used", "type": "stat" }, { @@ -5513,7 +5513,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "InnoDB IO Targets Read ", + "title": "InnoDB IO Targets Read", "tooltip": { "msResolution": false, "shared": true, @@ -6359,7 +6359,7 @@ "step": 300 } ], - "title": "Buffer Pool Size ", + "title": "Buffer Pool Size", "type": "stat" }, { @@ -6700,7 +6700,7 @@ "step": 300 } ], - "title": "BP Data ", + "title": "BP Data", "type": "stat" }, { @@ -7161,7 +7161,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "InnoDB Buffer Pool Data ", + "title": "InnoDB Buffer Pool Data", "tooltip": { "msResolution": false, "shared": true, @@ -8485,7 +8485,7 @@ "step": 300 } ], - "title": "InnoDB Old Blocks ", + "title": "InnoDB Old Blocks", "type": "stat" }, { @@ -9571,7 +9571,7 @@ "step": 300 } ], - "title": "Pages scanned per LRU Scan ", + "title": "Pages scanned per LRU Scan", "type": "stat" }, { @@ -9655,7 +9655,7 @@ "step": 300 } ], - "title": "LRU Get Free Waits ", + "title": "LRU Get Free Waits", "type": "stat" } ], @@ -13345,7 +13345,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "InnoDB Log Buffer Usage ", + "title": "InnoDB Log Buffer Usage", "tooltip": { "msResolution": false, "shared": true, @@ -14970,7 +14970,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "InnoDB Row Lock Wait Time ", + "title": "InnoDB Row Lock Wait Time", "tooltip": { "msResolution": false, "shared": true, @@ -17376,7 +17376,7 @@ "step": 300 } ], - "title": "Purge Batch Size ", + "title": "Purge Batch Size", "type": "stat" }, { @@ -18923,7 +18923,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "InnoDB AHI Churn - Rows ", + "title": "InnoDB AHI Churn - Rows", "tooltip": { "msResolution": false, "shared": true, diff --git a/dashboards/dashboards/MySQL/MySQL_User_Details.json b/dashboards/dashboards/MySQL/MySQL_User_Details.json index 12a920b7721..6de2202eee2 100644 --- a/dashboards/dashboards/MySQL/MySQL_User_Details.json +++ b/dashboards/dashboards/MySQL/MySQL_User_Details.json @@ -89,7 +89,7 @@ "refId": "A" } ], - "title": " ", + "title": "", "type": "text" }, { diff --git a/dashboards/dashboards/OS/Node_Temperature_Details.json b/dashboards/dashboards/OS/Node_Temperature_Details.json index 264e5dbe62a..a48135b408f 100644 --- a/dashboards/dashboards/OS/Node_Temperature_Details.json +++ b/dashboards/dashboards/OS/Node_Temperature_Details.json @@ -522,7 +522,7 @@ ], "thresholds": [], "timeRegions": [], - "title": " Fan Power Usage", + "title": "Fan Power Usage", "tooltip": { "msResolution": false, "shared": true, diff --git a/dashboards/dashboards/OS/Nodes_Overview.json b/dashboards/dashboards/OS/Nodes_Overview.json index f5d7aeada74..15db6e0f24b 100644 --- a/dashboards/dashboards/OS/Nodes_Overview.json +++ b/dashboards/dashboards/OS/Nodes_Overview.json @@ -4598,7 +4598,7 @@ "refId": "A" } ], - "title": "Min Virtual Memory Available ", + "title": "Min Virtual Memory Available", "type": "stat" }, { @@ -6724,7 +6724,7 @@ "refId": "A" } ], - "title": " Top Disk Latency", + "title": "Top Disk Latency", "type": "stat" }, { @@ -6804,7 +6804,7 @@ "refId": "A" } ], - "title": " Top Disk Operations", + "title": "Top Disk Operations", "type": "stat" }, { @@ -6884,7 +6884,7 @@ "refId": "A" } ], - "title": " Top Disk Bandwidth", + "title": "Top Disk Bandwidth", "type": "stat" }, { @@ -6964,7 +6964,7 @@ "refId": "A" } ], - "title": " Top I/O Activity", + "title": "Top I/O Activity", "type": "stat" }, { @@ -7838,7 +7838,7 @@ "refId": "A" } ], - "title": " Disk Latency", + "title": "Disk Latency", "type": "grafana-polystat-panel" }, { @@ -8274,7 +8274,7 @@ "refId": "A" } ], - "title": " Disk Bandwidth", + "title": "Disk Bandwidth", "type": "grafana-polystat-panel" }, { @@ -8807,7 +8807,7 @@ "refId": "A" } ], - "title": " Top Receive Network Traffic", + "title": "Top Receive Network Traffic", "type": "stat" }, { @@ -8887,7 +8887,7 @@ "refId": "A" } ], - "title": " Top Transmit Network Traffic", + "title": "Top Transmit Network Traffic", "type": "stat" }, { diff --git a/dashboards/dashboards/OS/Processes_Details.json b/dashboards/dashboards/OS/Processes_Details.json index 3e73d464d6a..e11c23a4c91 100644 --- a/dashboards/dashboards/OS/Processes_Details.json +++ b/dashboards/dashboards/OS/Processes_Details.json @@ -738,7 +738,7 @@ ], "thresholds": [], "timeRegions": [], - "title": "Blocked Processes Waiting for I/O ", + "title": "Blocked Processes Waiting for I/O", "tooltip": { "msResolution": false, "shared": true, diff --git a/dashboards/dashboards/PMM Health/PMM_Health.json b/dashboards/dashboards/PMM Health/PMM_Health.json index ce9a1903e4b..178d2023593 100644 --- a/dashboards/dashboards/PMM Health/PMM_Health.json +++ b/dashboards/dashboards/PMM Health/PMM_Health.json @@ -2388,7 +2388,7 @@ "refId": "A" } ], - "title": "Ingestions cap ", + "title": "Ingestions cap", "type": "stat" }, { diff --git a/dashboards/dashboards/PostgreSQL/PostgreSQL_Overview_Extended.json b/dashboards/dashboards/PostgreSQL/PostgreSQL_Overview_Extended.json index aa6c4b4e520..67478df2026 100644 --- a/dashboards/dashboards/PostgreSQL/PostgreSQL_Overview_Extended.json +++ b/dashboards/dashboards/PostgreSQL/PostgreSQL_Overview_Extended.json @@ -4327,7 +4327,7 @@ "refId": "A" } ], - "title": " Duration of Active Transactions", + "title": "Duration of Active Transactions", "type": "grafana-polystat-panel" }, { diff --git a/dashboards/dashboards/Valkey/Valkey_PersistenceDetails.json b/dashboards/dashboards/Valkey/Valkey_PersistenceDetails.json index a826ff74357..71a388b41cb 100644 --- a/dashboards/dashboards/Valkey/Valkey_PersistenceDetails.json +++ b/dashboards/dashboards/Valkey/Valkey_PersistenceDetails.json @@ -66,7 +66,7 @@ }, "id": 60, "panels": [], - "title": "Primary AOF Persistence ", + "title": "Primary AOF Persistence", "type": "row" }, { diff --git a/dashboards/misc/cleanup-dash.py b/dashboards/misc/cleanup-dash.py index d27081be4a0..3b2f44b1c66 100755 --- a/dashboards/misc/cleanup-dash.py +++ b/dashboards/misc/cleanup-dash.py @@ -44,6 +44,36 @@ def set_time(dashboard): dashboard['time']['to'] = "now" return dashboard +def walk_titles(node, callback): + """Apply callback to every 'title' value in the dashboard tree.""" + if isinstance(node, dict): + for key, value in node.items(): + if key == 'title' and isinstance(value, str): + node[key] = callback(value) + else: + walk_titles(value, callback) + elif isinstance(node, list): + for item in node: + walk_titles(item, callback) + + return node + +def trim_titles(dashboard): + """Strip leading/trailing whitespace from every title.""" + + return walk_titles(dashboard, str.strip) + +def collect_titles(dashboard): + """Return every title in the dashboard, in traversal order.""" + titles = [] + + def record(title): + titles.append(title) + return title + + walk_titles(dashboard, record) + return titles + def main(): parser = argparse.ArgumentParser(description='Dashboard cleaner') parser.add_argument('dashboard_file', type=str, help='dashboard file to cleanup') @@ -54,7 +84,7 @@ def main(): dashboard = json.loads(dashboard_file.read()) raw_dashboard = copy.deepcopy(dashboard) - CLEANUPERS = [set_editable, set_time, set_timezone, set_refresh, set_dashboard_id_to_null] + CLEANUPERS = [set_editable, set_time, set_timezone, set_refresh, set_dashboard_id_to_null, trim_titles] for func in CLEANUPERS: dashboard = func(dashboard) @@ -88,6 +118,9 @@ def jv(v): issues.append(f" time.to: {jv(raw_dashboard.get('time', {}).get('to'))} -> {jv(dashboard.get('time', {}).get('to'))}") if raw_dashboard.get('id') != dashboard.get('id'): issues.append(f" id: {jv(raw_dashboard.get('id'))} -> {jv(dashboard.get('id'))}") + for raw_title, title in zip(collect_titles(raw_dashboard), collect_titles(dashboard)): + if raw_title != title: + issues.append(f" title: {jv(raw_title)} -> {jv(title)}") print(f'Dashboard: {args.dashboard_file}') for issue in issues: print(issue) diff --git a/dashboards/misc/test_cleanup_dash.py b/dashboards/misc/test_cleanup_dash.py new file mode 100644 index 00000000000..5401ef4fd20 --- /dev/null +++ b/dashboards/misc/test_cleanup_dash.py @@ -0,0 +1,192 @@ +#!/usr/bin/env python3 +"""Tests for dashboards/misc/cleanup-dash.py (PMM-15308). + +Run from the repo root: python3 -m unittest discover -s -p 'test_*.py' -v +""" + +import copy +import importlib.util +import json +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +REPO = os.environ.get('PMM_REPO', os.getcwd()) +SCRIPT = os.path.join(REPO, 'dashboards', 'misc', 'cleanup-dash.py') +DASH_DIR = os.path.join(REPO, 'dashboards', 'dashboards') + + +def load_module(): + spec = importlib.util.spec_from_file_location('cleanup_dash', SCRIPT) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +cd = load_module() + + +def run_cli(*args): + return subprocess.run([sys.executable, SCRIPT, *args], + capture_output=True, text=True) + + +MINIMAL = { + 'editable': True, + 'refresh': '1m', + 'timezone': 'browser', + 'time': {'from': 'now-6h', 'to': 'now'}, + 'id': 42, + 'title': 'Dash', + 'panels': [], +} + + +class TestTrimTitles(unittest.TestCase): + def test_trims_root_nested_and_templating(self): + d = { + 'title': ' Root ', + 'panels': [ + {'title': 'Panel A ', 'panels': [{'title': ' Nested B '}]}, + ], + 'templating': {'list': [{'title': ' Var '}]}, + } + out = cd.trim_titles(copy.deepcopy(d)) + self.assertEqual(out['title'], 'Root') + self.assertEqual(out['panels'][0]['title'], 'Panel A') + self.assertEqual(out['panels'][0]['panels'][0]['title'], 'Nested B') + self.assertEqual(out['templating']['list'][0]['title'], 'Var') + + def test_whitespace_only_title_becomes_empty(self): + # PMM-15308: MySQL_User_Details spacer panel, title is literally " ". + out = cd.trim_titles({'panels': [{'title': ' ', 'type': 'text'}]}) + self.assertEqual(out['panels'][0]['title'], '') + + def test_strips_tabs_and_newlines(self): + out = cd.trim_titles({'title': '\tSpaced\n'}) + self.assertEqual(out['title'], 'Spaced') + + def test_non_string_titles_untouched(self): + # Must not raise: a null title is valid JSON. + out = cd.trim_titles({'panels': [{'title': None}, {'title': 7}]}) + self.assertIsNone(out['panels'][0]['title']) + self.assertEqual(out['panels'][1]['title'], 7) + + def test_nested_dict_under_title_key_is_traversed(self): + out = cd.trim_titles({'title': {'title': ' inner '}}) + self.assertEqual(out['title']['title'], 'inner') + + def test_non_title_keys_untouched(self): + out = cd.trim_titles({'legendFormat': ' keep me ', 'expr': ' x '}) + self.assertEqual(out['legendFormat'], ' keep me ') + self.assertEqual(out['expr'], ' x ') + + def test_idempotent(self): + d = {'title': ' A ', 'panels': [{'title': ' B '}]} + once = cd.trim_titles(copy.deepcopy(d)) + twice = cd.trim_titles(copy.deepcopy(once)) + self.assertEqual(once, twice) + + def test_unicode_preserved(self): + out = cd.trim_titles({'title': ' Répliqué ☠ '}) + self.assertEqual(out['title'], 'Répliqué ☠') + + def test_returns_dashboard_like_other_cleanupers(self): + d = {'title': ' A '} + self.assertIs(cd.trim_titles(d), d) + + +class TestCollectTitles(unittest.TestCase): + def test_collects_in_traversal_order_without_mutating(self): + d = {'title': 'root', 'panels': [{'title': 'a'}, {'title': 'b'}]} + before = copy.deepcopy(d) + self.assertEqual(cd.collect_titles(d), ['root', 'a', 'b']) + self.assertEqual(d, before, 'collect_titles must not mutate its input') + + def test_counts_match_between_raw_and_trimmed(self): + d = {'title': ' a ', 'panels': [{'title': ' b '}, {'title': None}]} + trimmed = cd.trim_titles(copy.deepcopy(d)) + self.assertEqual(len(cd.collect_titles(d)), len(cd.collect_titles(trimmed))) + + +class TestCheckOnlyCLI(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp) + + def write(self, dashboard): + path = os.path.join(self.tmp, 'dash.json') + with open(path, 'w', encoding='utf-8') as fh: + json.dump(dashboard, fh, sort_keys=True, indent=4, ensure_ascii=False) + fh.write('\n') + return path + + def clean_dashboard(self, **over): + d = copy.deepcopy(MINIMAL) + d.update({'editable': False, 'refresh': False, 'timezone': '', + 'time': {'from': 'now-12h', 'to': 'now'}, 'id': None}) + d.update(over) + return d + + def test_clean_dashboard_passes(self): + r = run_cli('--check-only', self.write(self.clean_dashboard())) + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + + def test_untrimmed_title_fails_and_explains_why(self): + # The regression this guards: exit 1 must PRINT the reason, not just fail. + path = self.write(self.clean_dashboard(panels=[{'title': 'Buffer Pool Data '}])) + r = run_cli('--check-only', path) + self.assertEqual(r.returncode, 1) + self.assertIn('title:', r.stdout) + self.assertIn('"Buffer Pool Data " -> "Buffer Pool Data"', r.stdout) + + def test_write_mode_fixes_then_check_passes(self): + path = self.write(self.clean_dashboard(panels=[{'title': 'Space Used '}])) + self.assertEqual(run_cli(path).returncode, 0) + with open(path, encoding='utf-8') as fh: + self.assertEqual(json.load(fh)['panels'][0]['title'], 'Space Used') + self.assertEqual(run_cli('--check-only', path).returncode, 0) + + def test_other_cleanupers_still_reported(self): + # trim_titles must not mask the pre-existing checks. + path = self.write(self.clean_dashboard(editable=True, timezone='browser')) + r = run_cli('--check-only', path) + self.assertEqual(r.returncode, 1) + self.assertIn('editable:', r.stdout) + self.assertIn('timezone:', r.stdout) + + +class TestRepoDashboards(unittest.TestCase): + """Replays the dashboards.yml `check` job over every committed dashboard.""" + + def dashboards(self): + for dp, _, fns in os.walk(DASH_DIR): + for fn in sorted(fns): + if fn.endswith('.json'): + yield os.path.join(dp, fn) + + def test_all_dashboards_pass_check_only(self): + failures = [p for p in self.dashboards() + if run_cli('--check-only', p).returncode != 0] + self.assertEqual(failures, [], f'{len(failures)} dashboard(s) fail the gate') + + def test_no_untrimmed_titles_remain(self): + offenders = [] + for path in self.dashboards(): + with open(path, encoding='utf-8') as fh: + for title in cd.collect_titles(json.load(fh)): + if isinstance(title, str) and title != title.strip(): + offenders.append((path, title)) + self.assertEqual(offenders, []) + + def test_all_dashboards_are_valid_json(self): + for path in self.dashboards(): + with open(path, encoding='utf-8') as fh: + json.load(fh) + + +if __name__ == '__main__': + unittest.main(verbosity=2)