diff --git a/.gitignore b/.gitignore index 5f8e2366..940ab171 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ *.toml .coverage .bob +.vscode + diff --git a/benchmark/cephtestrados.py b/benchmark/cephtestrados.py index cf110557..942acc45 100644 --- a/benchmark/cephtestrados.py +++ b/benchmark/cephtestrados.py @@ -1,7 +1,7 @@ from .benchmark import Benchmark import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import time import logging @@ -74,7 +74,7 @@ def run(self): self.mkpool() self.dropcaches() self.cluster.dump_config(self.run_dir) - monitoring.start(self.run_dir) + MonitoringFactory.start(self.run_dir) time.sleep(5) # Run the backfill testing thread if requested if 'recovery_test' in self.cluster.config: @@ -92,7 +92,7 @@ def run(self): if 'recovery_test' in self.cluster.config: self.cluster.wait_recovery_done() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) # Finally, get the historic ops self.cluster.dump_historic_ops(self.run_dir) @@ -117,10 +117,10 @@ def mkcmd(self): return ' '.join(cmd) def mkpool(self): - monitoring.start("%s/pool_monitoring" % self.run_dir) + MonitoringFactory.start("%s/pool_monitoring" % self.run_dir) self.cluster.rmpool('ceph_test_rados', self.pool_profile) self.cluster.mkpool('ceph_test_rados', self.pool_profile, 'ceph_test_rados') - monitoring.stop() + MonitoringFactory.stop() def recovery_callback(self): common.pdsh(settings.getnodes('clients'), 'sudo pkill -f ceph_test_rados').communicate() diff --git a/benchmark/cosbench.py b/benchmark/cosbench.py index 46dda0db..a6c623fb 100644 --- a/benchmark/cosbench.py +++ b/benchmark/cosbench.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import sys import time @@ -185,9 +185,9 @@ def initialize(self): self.prerun_check() logger.debug('Pausing for 60s for idle monitoring.') - monitoring.start("%s/idle_monitoring" % self.run_dir) + MonitoringFactory.start("%s/idle_monitoring" % self.run_dir) time.sleep(60) - monitoring.stop() + MonitoringFactory.stop() common.sync_files('%s' % self.run_dir, self.out_dir) @@ -248,7 +248,7 @@ def run(self): super(Cosbench, self).run() self.dropcaches() self.cluster.dump_config(self.run_dir) - monitoring.start(self.run_dir) + MonitoringFactory.start(self.run_dir) # Run cosbench test try: @@ -262,7 +262,7 @@ def run(self): self.check_workload_status() self.check_cosbench_res_dir() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) self.cluster.dump_historic_ops(self.run_dir) common.sync_files('%s/*' % self.run_dir, self.out_dir) diff --git a/benchmark/fio.py b/benchmark/fio.py index 9407dbb1..8ae388f3 100644 --- a/benchmark/fio.py +++ b/benchmark/fio.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import time import logging @@ -228,7 +228,7 @@ def run(self): # Wait for signal to start client IO self.cluster.wait_start_io() - monitoring.start(self.run_dir) + MonitoringFactory.start(self.run_dir) logger.info('Running fio %s test.', self.mode) ps = [] @@ -241,7 +241,7 @@ def run(self): if 'recovery_test' in self.cluster.config: self.cluster.wait_recovery_done() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) # Finally, get the historic ops self.cluster.dump_historic_ops(self.run_dir) diff --git a/benchmark/getput.py b/benchmark/getput.py index f6559e6b..aed81800 100644 --- a/benchmark/getput.py +++ b/benchmark/getput.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import time import logging @@ -58,9 +58,9 @@ def initialize(self): common.make_remote_dir(self.run_dir) logger.info('Pausing for 60s for idle monitoring.') - monitoring.start("%s/idle_monitoring" % self.run_dir) + MonitoringFactory.start("%s/idle_monitoring" % self.run_dir) time.sleep(60) - monitoring.stop() + MonitoringFactory.stop() common.sync_files('%s/*' % self.run_dir, self.out_dir) @@ -125,7 +125,7 @@ def run(self): self.cluster.create_recovery_test(self.run_dir, recovery_callback) # Run getput - monitoring.start(self.run_dir) + MonitoringFactory.start(self.run_dir) logger.info('Running getput %s test.' % self.test) ps = [] @@ -135,7 +135,7 @@ def run(self): ps.append(p) for p in ps: p.wait() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) # If we were doing recovery, wait until it's done. if 'recovery_test' in self.cluster.config: diff --git a/benchmark/hsbench.py b/benchmark/hsbench.py index cdc6ad50..efa9bb3b 100644 --- a/benchmark/hsbench.py +++ b/benchmark/hsbench.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import logging import pathlib @@ -141,7 +141,7 @@ def run(self): recovery_callback = self.recovery_callback self.cluster.create_recovery_test(self.run_dir, recovery_callback) - monitoring.start(self.run_dir) + MonitoringFactory.start(self.run_dir) logger.info('Running hsbench %s test.' % self.modes) ps = [] for i in range(self.endpoints_per_client): @@ -153,7 +153,7 @@ def run(self): if 'recovery_test' in self.cluster.config: self.cluster.wait_recovery_done() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) # If we were doing recovery, wait until it's done. if 'recovery_test' in self.cluster.config: diff --git a/benchmark/kvmrbdfio.py b/benchmark/kvmrbdfio.py index 9c98af3b..225b3770 100644 --- a/benchmark/kvmrbdfio.py +++ b/benchmark/kvmrbdfio.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import time import logging @@ -94,7 +94,7 @@ def run(self): # We'll always drop caches for rados bench self.dropcaches() - monitoring.start(self.run_dir) + MonitoringFactory.start(self.run_dir) time.sleep(5) # Run the backfill testing thread if requested @@ -139,7 +139,7 @@ def run(self): fio_process_list.append(common.pdsh(clnts, fio_cmd, continue_if_error=False)) for p in fio_process_list: p.communicate() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) logger.info('Finished rbd fio test') common.sync_files('%s/*' % self.run_dir, self.out_dir) diff --git a/benchmark/librbdfio.py b/benchmark/librbdfio.py index f7f1d49f..e5166ead 100644 --- a/benchmark/librbdfio.py +++ b/benchmark/librbdfio.py @@ -1,22 +1,25 @@ """ - lirbdfio.py -- module to support the FIO benchmark exercising RBD. +lirbdfio.py -- module to support the FIO benchmark exercising RBD. """ -import os -import time + import logging -import common -import settings -import monitoring +import os import re +import time from pathlib import Path from typing import Union +import common +from monitoring.monitoring_factory import MonitoringFactory +import settings +from post_processing.post_processing_types import ReportType from post_processing.report import Report, ReportOptions from .benchmark import Benchmark logger = logging.getLogger("cbt") + class LibrbdFio(Benchmark): """ Class LibrbdFio @@ -26,32 +29,32 @@ def __init__(self, archive_dir, cluster, config): super(LibrbdFio, self).__init__(archive_dir, cluster, config) # FIXME there are too many permutations, need to put results in SQLITE3 - self.cmd_path = config.get('cmd_path', '/usr/bin/fio') - self.pool_profile = config.get('pool_profile', 'default') - self.recov_pool_profile = config.get('recov_pool_profile', 'default') - self.recov_test_type = config.get('recov_test_type', 'blocking') - self.data_pool_profile = config.get('data_pool_profile', None) - self.time = config.get('time', None) + self.cmd_path = config.get("cmd_path", "/usr/bin/fio") + self.pool_profile = config.get("pool_profile", "default") + self.recov_pool_profile = config.get("recov_pool_profile", "default") + self.recov_test_type = config.get("recov_test_type", "blocking") + self.data_pool_profile = config.get("data_pool_profile", None) + self.time = config.get("time", None) # Global FIO options can be overwritten for specific workload options # would be nice to have them as a separate class -- future PR - self.time_based = bool(config.get('time_based', False)) - self.ramp = config.get('ramp', None) - self.numjobs = config.get('numjobs', 1) - self.end_fsync = config.get('end_fsync', 0) - self.mode = config.get('mode', 'write') - self.rwmixread = config.get('rwmixread', 50) + self.time_based = bool(config.get("time_based", False)) + self.ramp = config.get("ramp", None) + self.numjobs = config.get("numjobs", 1) + self.end_fsync = config.get("end_fsync", 0) + self.mode = config.get("mode", "write") + self.rwmixread = config.get("rwmixread", 50) self.rwmixwrite = 100 - self.rwmixread - self.log_avg_msec = config.get('log_avg_msec', None) - self.op_size = config.get('op_size', 4194304) - - self.pgs = config.get('pgs', 2048) - self.vol_size = config.get('vol_size', 65536) - self.vol_object_size = config.get('vol_object_size', 22) - self.volumes_per_client : int = int(config.get('volumes_per_client', 1)) - self.procs_per_volume = config.get('procs_per_volume', 1) - self.random_distribution = config.get('random_distribution', None) - self.rate_iops = config.get('rate_iops', None) - self.fio_out_format = config.get('fio_out_format', 'json,normal') + self.log_avg_msec = config.get("log_avg_msec", None) + self.op_size = config.get("op_size", 4194304) + + self.pgs = config.get("pgs", 2048) + self.vol_size = config.get("vol_size", 65536) + self.vol_object_size = config.get("vol_object_size", 22) + self.volumes_per_client: int = int(config.get("volumes_per_client", 1)) + self.procs_per_volume = config.get("procs_per_volume", 1) + self.random_distribution = config.get("random_distribution", None) + self.rate_iops = config.get("rate_iops", None) + self.fio_out_format = config.get("fio_out_format", "json,normal") self.data_pool = None iodepth_key: str = self._get_iodepth_key(config.keys()) # type: ignore[arg-type] @@ -61,62 +64,62 @@ def __init__(self, archive_dir, cluster, config): ) # use_existing_volumes needs to be true to set the pool and rbd names - self.use_existing_volumes = bool(config.get('use_existing_volumes', False)) - self.no_sudo = bool(config.get('no_sudo', False)) - self.idle_monitor_sleep = config.get('idle_monitor_sleep', 60) + self.use_existing_volumes = bool(config.get("use_existing_volumes", False)) + self.no_sudo = bool(config.get("no_sudo", False)) + self.idle_monitor_sleep = config.get("idle_monitor_sleep", 60) self.pool_name = config.get("poolname", "cbt-librbdfio") self.recov_pool_name = config.get("recov_pool_name", "cbt-rbdfio-recov") - self.rbdname = config.get('rbdname', '') - self.prefill_vols = config.get('prefill', {'blocksize': '4M', - 'numjobs': '1'}) - self.total_procs = (self.procs_per_volume * self.volumes_per_client * - len(settings.getnodes('clients').split(','))) + self.rbdname = config.get("rbdname", "") + self.prefill_vols = config.get("prefill", {"blocksize": "4M", "numjobs": "1"}) + self.total_procs = ( + self.procs_per_volume * self.volumes_per_client * len(settings.getnodes("clients").split(",")) + ) if not self._workloads.exist(): - self.run_dir += ( f'op_size-{int(self.op_size):08d}/' - f'concurrent_procs-{int(self.total_procs):03d}/' - f'iodepth-{int(self.iodepth):03d}/{self.mode}' ) + self.run_dir += ( + f"op_size-{int(self.op_size):08d}/" + f"concurrent_procs-{int(self.total_procs):03d}/" + f"iodepth-{int(self.iodepth):03d}/{self.mode}" + ) self.out_dir = self.archive_dir self.norandommap = config.get("norandommap", False) self.wait_pgautoscaler_timeout = config.get("wait_pgautoscaler_timeout", -1) # Make the file names string (repeated across volumes) - self.names = '' + self.names = "" for proc_num in range(self.procs_per_volume): - rbd_name = f'cbt-rbdfio-`{common.get_fqdn_cmd()}`-file-{proc_num:d}' - self.names += f'--name={rbd_name} ' + rbd_name = f"cbt-rbdfio-`{common.get_fqdn_cmd()}`-file-{proc_num:d}" + self.names += f"--name={rbd_name} " def exists(self): """ Verify whether the out_dir exists """ if os.path.exists(self.out_dir): - logger.info('Skipping existing test in %s.', self.out_dir) + logger.info("Skipping existing test in %s.", self.out_dir) return True return False - def initialize(self): super(LibrbdFio, self).initialize() # Clean and Create the run directory common.clean_remote_dir(self.run_dir) common.make_remote_dir(self.run_dir) - logger.info('Pausing for %ds for idle monitoring.', self.idle_monitor_sleep) - monitoring.start( f"{self.run_dir}idle_monitoring" ) + logger.info("Pausing for %ds for idle monitoring.", self.idle_monitor_sleep) + MonitoringFactory.start(f"{self.run_dir}idle_monitoring") time.sleep(self.idle_monitor_sleep) - monitoring.stop() - common.sync_files( f'{self.run_dir}/', self.out_dir) + MonitoringFactory.stop() + common.sync_files(f"{self.run_dir}/", self.out_dir) # Create the recovery image based on test type requested - if 'recovery_test' in self.cluster.config and self.recov_test_type == 'background': + if "recovery_test" in self.cluster.config and self.recov_test_type == "background": self.mkrecovimage() if self._workloads.exist(): logger.info("Workloads:\n %s", self._workloads.get_names().replace(" ", "\n")) - logger.info('Creating fio images...') + logger.info("Creating fio images...") self.mkimages() - logger.info('Attempting to prefill fio images...') + logger.info("Attempting to prefill fio images...") self.prefill() - def run(self): super(LibrbdFio, self).run() # We'll always drop caches for rados bench @@ -128,20 +131,18 @@ def run(self): time.sleep(5) # If the pg autoscaler kicks in before starting the test, # wait for it to complete. Otherwise, results may be skewed. - ret = self.cluster.check_pg_autoscaler(self.wait_pgautoscaler_timeout, - f"{self.run_dir}pgautoscaler.log") + ret = self.cluster.check_pg_autoscaler(self.wait_pgautoscaler_timeout, f"{self.run_dir}pgautoscaler.log") if ret == 1: - logger.warn("PG autoscaler taking longer to complete." - "Continuing anyway...results may be skewed.") + logger.warn("PG autoscaler taking longer to complete.Continuing anyway...results may be skewed.") # Start the recovery thread if requested - if 'recovery_test' in self.cluster.config: - if self.recov_test_type == 'blocking': + if "recovery_test" in self.cluster.config: + if self.recov_test_type == "blocking": recovery_callback = self.recovery_callback_blocking - elif self.recov_test_type == 'background': + elif self.recov_test_type == "background": recovery_callback = self.recovery_callback_background self.cluster.create_recovery_test(self.run_dir, recovery_callback, self.recov_test_type) - if 'recovery_test' in self.cluster.config and self.recov_test_type == 'background': + if "recovery_test" in self.cluster.config and self.recov_test_type == "background": # Wait for a signal from the recovery thread to initiate client IO self.cluster.wait_start_io() @@ -151,26 +152,26 @@ def run(self): self._workloads.run() else: # Original style - monitoring.start(self.run_dir) - logger.info('Running rbd fio %s test.', self.mode) + MonitoringFactory.start(self.run_dir) + logger.info("Running rbd fio %s test.", self.mode) ps = [] number_of_volumes: int = len(self._iodepth_per_volume.keys()) for i in range(number_of_volumes): fio_cmd = self.mkfiocmd(i) - p = common.pdsh(settings.getnodes('clients'), fio_cmd) + p = common.pdsh(settings.getnodes("clients"), fio_cmd) ps.append(p) for p in ps: p.wait() - + # If we were doing recovery, wait until it's done. - if 'recovery_test' in self.cluster.config: + if "recovery_test" in self.cluster.config: self.cluster.wait_recovery_done() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) # Finally, get the historic ops self.cluster.dump_historic_ops(self.run_dir) - source_directory: str = f'{self.run_dir}/*' + source_directory: str = f"{self.run_dir}/*" if self._workloads.exist(): source_directory = f"{self._workloads.get_base_run_directory()}/*" common.sync_files(source_directory, self.out_dir) @@ -178,21 +179,20 @@ def run(self): if self._create_report: report_config: dict[str, Union[str, bool]] = settings.report - output_directory: str = report_config.get('output_directory', f"{self.out_dir}/report") + output_directory: str = report_config.get("output_directory", f"{self.out_dir}/report") report_options: ReportOptions = ReportOptions( - archives = [f"{self.archive_dir}"], - output_directory = output_directory, - results_file_root = "json_output", - create_pdf = report_config.get("create_pdf", False), - force_refresh = report_config.get("force_refresh", False), - no_error_bars = report_config.get("no_error_bars", False), - comparison = False, - plot_resources = report_config.get("plot_resource", False) + archives=[f"{self.archive_dir}"], + output_directory=output_directory, + results_file_root="json_output", + create_pdf=report_config.get("create_pdf", False), + force_refresh=report_config.get("force_refresh", False), + no_error_bars=report_config.get("no_error_bars", False), + report_type=ReportType.SIMPLE, + plot_resources=report_config.get("plot_resources", False), ) report: Report = Report(report_options) report.generate() - def mkfiocmd(self, volnum: int) -> str: """ Construct a FIO cmd (note the shell interpolation for the host @@ -201,95 +201,100 @@ def mkfiocmd(self, volnum: int) -> str: if self.use_existing_volumes and len(self.rbdname): rbdname = self.rbdname else: - rbdname = f'cbt-rbdfio-`{common.get_fqdn_cmd()}`-{volnum:d}' + rbdname = f"cbt-rbdfio-`{common.get_fqdn_cmd()}`-{volnum:d}" - logger.debug('Using rbdname %s', rbdname) - out_file = f'{self.run_dir}/output.{volnum:d}' + logger.debug("Using rbdname %s", rbdname) + out_file = f"{self.run_dir}/output.{volnum:d}" - fio_cmd: str = '' + fio_cmd: str = "" if not self.no_sudo: - fio_cmd = 'sudo ' - fio_cmd += '%s --ioengine=rbd --clientname=admin --pool=%s --rbdname=%s --invalidate=0' % (self.cmd_path, self.pool_name, rbdname) - fio_cmd += ' --rw=%s' % self.mode - fio_cmd += ' --output-format=%s' % self.fio_out_format - if (self.mode == 'readwrite' or self.mode == 'randrw'): - fio_cmd += ' --rwmixread=%s --rwmixwrite=%s' % (self.rwmixread, self.rwmixwrite) + fio_cmd = "sudo " + fio_cmd += "%s --ioengine=rbd --clientname=admin --pool=%s --rbdname=%s --invalidate=0" % ( + self.cmd_path, + self.pool_name, + rbdname, + ) + fio_cmd += " --rw=%s" % self.mode + fio_cmd += " --output-format=%s" % self.fio_out_format + if self.mode == "readwrite" or self.mode == "randrw": + fio_cmd += " --rwmixread=%s --rwmixwrite=%s" % (self.rwmixread, self.rwmixwrite) if self.time is not None: - fio_cmd += ' --runtime=%d' % self.time + fio_cmd += " --runtime=%d" % self.time if self.time_based is True: - fio_cmd += ' --time_based' + fio_cmd += " --time_based" if self.ramp is not None: - fio_cmd += ' --ramp_time=%d' % self.ramp - fio_cmd += ' --numjobs=%s' % self.numjobs - fio_cmd += ' --direct=1' - fio_cmd += ' --bs=%dB' % self.op_size + fio_cmd += " --ramp_time=%d" % self.ramp + fio_cmd += " --numjobs=%s" % self.numjobs + fio_cmd += " --direct=1" + fio_cmd += " --bs=%dB" % self.op_size iodepth: str = f"{self._iodepth_per_volume[volnum]}" - - fio_cmd += ' --iodepth=%s' % iodepth - fio_cmd += ' --end_fsync=%d' % self.end_fsync -# if self.vol_size: -# fio_cmd += ' -- size=%dM' % self.vol_size + + fio_cmd += " --iodepth=%s" % iodepth + fio_cmd += " --end_fsync=%d" % self.end_fsync + # if self.vol_size: + # fio_cmd += ' -- size=%dM' % self.vol_size if self.norandommap: - fio_cmd += ' --norandommap' + fio_cmd += " --norandommap" if self.log_iops: - fio_cmd += ' --write_iops_log=%s' % out_file + fio_cmd += " --write_iops_log=%s" % out_file if self.log_bw: - fio_cmd += ' --write_bw_log=%s' % out_file + fio_cmd += " --write_bw_log=%s" % out_file if self.log_lat: - fio_cmd += ' --write_lat_log=%s' % out_file - if 'recovery_test' in self.cluster.config: - fio_cmd += ' --time_based' + fio_cmd += " --write_lat_log=%s" % out_file + if "recovery_test" in self.cluster.config: + fio_cmd += " --time_based" if self.random_distribution is not None: - fio_cmd += ' --random_distribution=%s' % self.random_distribution + fio_cmd += " --random_distribution=%s" % self.random_distribution if self.log_avg_msec is not None: - fio_cmd += ' --log_avg_msec=%s' % self.log_avg_msec + fio_cmd += " --log_avg_msec=%s" % self.log_avg_msec if self.rate_iops is not None: - fio_cmd += ' --rate_iops=%s' % self.rate_iops + fio_cmd += " --rate_iops=%s" % self.rate_iops # End the fio_cmd - fio_cmd += ' %s > %s' % (self.names, out_file) + fio_cmd += " %s > %s" % (self.names, out_file) return fio_cmd - def mkrecovimage(self): """ Create a reecovery image """ - logger.info('Creating recovery image...') - monitoring.start( f"{self.run_dir}/recovery_pool_monitoring" ) + logger.info("Creating recovery image...") + MonitoringFactory.start(f"{self.run_dir}/recovery_pool_monitoring") if self.use_existing_volumes is False: self.cluster.rmpool(self.recov_pool_name, self.recov_pool_profile) - self.cluster.mkpool(self.recov_pool_name, self.recov_pool_profile, 'rbd') - for node in common.get_fqdn_list('clients'): + self.cluster.mkpool(self.recov_pool_name, self.recov_pool_profile, "rbd") + for node in common.get_fqdn_list("clients"): for volnum in range(0, self.volumes_per_client): node = node.rpartition("@")[2] - self.cluster.mkimage( f'cbt-rbdfio-recov-{node}-{volnum:d}', - self.vol_size, self.recov_pool_name, self.data_pool, - self.vol_object_size ) - monitoring.stop() - + self.cluster.mkimage( + f"cbt-rbdfio-recov-{node}-{volnum:d}", + self.vol_size, + self.recov_pool_name, + self.data_pool, + self.vol_object_size, + ) + MonitoringFactory.stop() def mkimages(self): """ Create an RBD pool and a number of volumes per client """ - monitoring.start( f"{self.run_dir}/pool_monitoring" ) + MonitoringFactory.start(f"{self.run_dir}/pool_monitoring") if self.use_existing_volumes is False: self.cluster.rmpool(self.pool_name, self.pool_profile) - self.cluster.mkpool(self.pool_name, self.pool_profile, 'rbd') + self.cluster.mkpool(self.pool_name, self.pool_profile, "rbd") if self.data_pool_profile: self.data_pool = self.pool_name + "-data" self.cluster.rmpool(self.data_pool, self.data_pool_profile) - self.cluster.mkpool(self.data_pool, self.data_pool_profile, 'rbd') - for node in common.get_fqdn_list('clients'): + self.cluster.mkpool(self.data_pool, self.data_pool_profile, "rbd") + for node in common.get_fqdn_list("clients"): for volnum in range(0, self.volumes_per_client): node = node.rpartition("@")[2] - self.cluster.mkimage( f'cbt-rbdfio-{node}-{volnum:d}', - self.vol_size, self.pool_name, self.data_pool, - self.vol_object_size) - monitoring.stop() - + self.cluster.mkimage( + f"cbt-rbdfio-{node}-{volnum:d}", self.vol_size, self.pool_name, self.data_pool, self.vol_object_size + ) + MonitoringFactory.stop() def prefill(self): """ @@ -299,32 +304,31 @@ def prefill(self): if not self.use_existing_volumes: rbd_base_name: str = self.config.get("rbdname", "cbt-rbdfio") for volnum in range(self.volumes_per_client): - rbd_name = f'{rbd_base_name}-`{common.get_fqdn_cmd()}`-{volnum:d}' - pre_cmd = '' + rbd_name = f"{rbd_base_name}-`{common.get_fqdn_cmd()}`-{volnum:d}" + pre_cmd = "" if not self.no_sudo: - pre_cmd += 'sudo ' - numjobs = self.prefill_vols['numjobs'] - bs = self.prefill_vols['blocksize'] - pre_cmd += ( f'{self.cmd_path} --ioengine=rbd --clientname=admin' - f' --pool={self.pool_name}' - f' --rbdname={rbd_name} --invalidate=0 --rw=write' - f' --numjobs={numjobs}' - f' --bs={bs}' - f' --size {self.vol_size:d}M {self.names}' - f' --output-format={self.fio_out_format} > /dev/null' ) - p = common.pdsh(settings.getnodes('clients'), pre_cmd) + pre_cmd += "sudo " + numjobs = self.prefill_vols["numjobs"] + bs = self.prefill_vols["blocksize"] + pre_cmd += ( + f"{self.cmd_path} --ioengine=rbd --clientname=admin" + f" --pool={self.pool_name}" + f" --rbdname={rbd_name} --invalidate=0 --rw=write" + f" --numjobs={numjobs}" + f" --bs={bs}" + f" --size {self.vol_size:d}M {self.names}" + f" --output-format={self.fio_out_format} > /dev/null" + ) + p = common.pdsh(settings.getnodes("clients"), pre_cmd) ps.append(p) for p in ps: p.wait() - def recovery_callback_blocking(self): - common.pdsh(settings.getnodes('clients'), 'sudo killall -2 fio').communicate() - + common.pdsh(settings.getnodes("clients"), "sudo killall -2 fio").communicate() def recovery_callback_background(self): - logger.info('Recovery thread completed!') - + logger.info("Recovery thread completed!") def parse(self, out_dir): """ @@ -337,7 +341,7 @@ def parse(self, out_dir): ] for file in files_to_process: with file.open("r", encoding="utf-8") as input_file: - output_file_name: str = f"{file.parent}/json_output{file.name[file.name.find('.'):]}" + output_file_name: str = f"{file.parent}/json_output{file.name[file.name.find('.') :]}" output_path = Path(output_file_name) found: bool = False with output_path.open("w", encoding="utf-8") as output_file: @@ -349,13 +353,12 @@ def parse(self, out_dir): output_file.write(line) found = False break - + if found: output_file.write(line) - def analyze(self, out_dir): - logger.info('Convert results to json format.') + logger.info("Convert results to json format.") self.parse(out_dir) def _get_iodepth_key(self, configuration_keys: list[str]) -> str: @@ -429,6 +432,6 @@ def _set_iodepth_for_every_volume(self, number_of_volumes: int, iodepth: int) -> queue_depths[volume_id] = iodepth return queue_depths - + def __str__(self): return "%s\n%s\n%s" % (self.run_dir, self.out_dir, super(LibrbdFio, self).__str__()) diff --git a/benchmark/radosbench.py b/benchmark/radosbench.py index eb425077..26f1582f 100644 --- a/benchmark/radosbench.py +++ b/benchmark/radosbench.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import time import logging @@ -60,7 +60,7 @@ def initialize(self): super(Radosbench, self).initialize() logger.info('Pausing for 60s for idle monitoring.') - with monitoring.monitor("%s/idle_monitoring" % self.run_dir): + with MonitoringFactory.monitor("%s/idle_monitoring" % self.run_dir): time.sleep(60) common.sync_files('%s/*' % self.run_dir, self.out_dir) @@ -150,7 +150,7 @@ def _run(self, mode, run_dir, out_dir, max_objects, runtime): self.cluster.create_recovery_test(run_dir, recovery_callback) # Run rados bench - with monitoring.monitor(run_dir) as monitor: + with MonitoringFactory.monitor(run_dir) as monitor: logger.info('Running radosbench %s test.' % mode) ps = [] for i in range(self.concurrent_procs): @@ -204,7 +204,7 @@ def _run(self, mode, run_dir, out_dir, max_objects, runtime): self.analyze(out_dir) def mkpools(self): - with monitoring.monitor("%s/pool_monitoring" % self.run_dir): + with MonitoringFactory.monitor("%s/pool_monitoring" % self.run_dir): if self.pool_per_proc: # allow use of a separate storage pool per process for i in range(self.concurrent_procs): for node in settings.getnodes('clients').split(','): @@ -264,7 +264,7 @@ def get_total_ops(self): return res[0] def get_cpu_cycles(self): - return monitoring.get_cpu_cycles(self.out_dir) + return MonitoringFactory.get_cpu_cycles(self.out_dir) def get_cpu_cycles_per_op(self): num_cpu_cycles = self.get_cpu_cycles() diff --git a/benchmark/rawfio.py b/benchmark/rawfio.py index fad27550..85b818cb 100644 --- a/benchmark/rawfio.py +++ b/benchmark/rawfio.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import time import logging @@ -78,7 +78,7 @@ def run(self): # We'll always drop caches for rados bench self.dropcaches() - monitoring.start(self.run_dir) + MonitoringFactory.start(self.run_dir) time.sleep(5) @@ -119,7 +119,7 @@ def run(self): fio_process_list.append(common.pdsh(clnts, fio_cmd, continue_if_error=False)) for p in fio_process_list: p.communicate() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) logger.info('Finished raw fio test') common.sync_files('%s/*' % self.run_dir, self.out_dir) diff --git a/benchmark/rbdfio.py b/benchmark/rbdfio.py index 80cc2605..8321c641 100644 --- a/benchmark/rbdfio.py +++ b/benchmark/rbdfio.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import time import logging @@ -59,9 +59,9 @@ def initialize(self): super(RbdFio, self).initialize() logger.info('Pausing for 60s for idle monitoring.') - monitoring.start("%s/idle_monitoring" % self.run_dir) + MonitoringFactory.start("%s/idle_monitoring" % self.run_dir) time.sleep(60) - monitoring.stop() + MonitoringFactory.stop() common.sync_files('%s/*' % self.run_dir, self.out_dir) @@ -85,7 +85,7 @@ def run(self): # We'll always drop caches for rados bench self.dropcaches() - monitoring.start(self.run_dir) + MonitoringFactory.start(self.run_dir) # Run the backfill testing thread if requested if 'recovery_test' in self.cluster.config: @@ -129,7 +129,7 @@ def run(self): if 'recovery_test' in self.cluster.config: self.cluster.wait_recovery_done() - monitoring.stop(self.run_dir) + MonitoringFactory.stop(self.run_dir) # Finally, get the historic ops self.cluster.dump_historic_ops(self.run_dir) @@ -145,7 +145,7 @@ def __str__(self): return "%s\n%s\n%s" % (self.run_dir, self.out_dir, super(RbdFio, self).__str__()) def mkimages(self): - monitoring.start("%s/pool_monitoring" % self.run_dir) + MonitoringFactory.start("%s/pool_monitoring" % self.run_dir) self.cluster.rmpool(self.poolname, self.pool_profile) self.cluster.mkpool(self.poolname, self.pool_profile, 'rbd') common.pdsh(settings.getnodes('clients'), '/usr/bin/rbd create cbt-kernelrbdfio-`hostname -s` --size %s --pool %s' % (self.vol_size, self.poolname)).communicate() @@ -153,7 +153,7 @@ def mkimages(self): common.pdsh(settings.getnodes('clients'), 'sudo mkfs.xfs /dev/rbd/cbt-kernelrbdfio/cbt-kernelrbdfio-`hostname -s`').communicate() common.pdsh(settings.getnodes('clients'), 'sudo mkdir -p -m0755 -- %s/cbt-kernelrbdfio-`hostname -s`' % self.cluster.mnt_dir).communicate() common.pdsh(settings.getnodes('clients'), 'sudo mount -t xfs -o noatime,inode64 /dev/rbd/cbt-kernelrbdfio/cbt-kernelrbdfio-`hostname -s` %s/cbt-kernelrbdfio-`hostname -s`' % self.cluster.mnt_dir).communicate() - monitoring.stop() + MonitoringFactory.stop() def recovery_callback(self): common.pdsh(settings.getnodes('clients'), 'sudo killall -9 fio').communicate() diff --git a/cluster/ceph.py b/cluster/ceph.py index 0003d1b0..d631c6f0 100644 --- a/cluster/ceph.py +++ b/cluster/ceph.py @@ -1,6 +1,6 @@ import common import settings -import monitoring +from monitoring.monitoring_factory import MonitoringFactory import os import time import uuid @@ -199,16 +199,16 @@ def initialize(self): self.setup_fs() # Build the cluster - monitoring.start('%s/creation' % self.monitoring_dir) + MonitoringFactory.start('%s/creation' % self.monitoring_dir) self.make_mons() self.start_mgrs() self.make_osds() - monitoring.stop() + MonitoringFactory.stop() # Check Health - monitoring.start('%s/initial_health_check' % self.monitoring_dir) + MonitoringFactory.start('%s/initial_health_check' % self.monitoring_dir) self.check_health() - monitoring.stop() + MonitoringFactory.stop() # Disable scrub and wait for any scrubbing to complete self.disable_scrub() @@ -227,9 +227,9 @@ def initialize(self): self.start_mds() # Peform Idle Monitoring if self.idle_duration > 0: - monitoring.start("%s/idle_monitoring" % self.monitoring_dir) + MonitoringFactory.start("%s/idle_monitoring" % self.monitoring_dir) time.sleep(self.idle_duration) - monitoring.stop() + MonitoringFactory.stop() return True @@ -248,7 +248,7 @@ def shutdown(self): common.pdsh(nodes, 'sudo killall -9 radosgw-admin').communicate() common.pdsh(nodes, 'sudo /etc/init.d/apache2 stop').communicate() common.pdsh(nodes, 'sudo killall -9 pdsh').communicate() - monitoring.stop() + MonitoringFactory.stop() def cleanup(self): nodes = settings.getnodes('clients', 'osds', 'mons', 'rgws', 'mdss', 'mgrs') diff --git a/docs/CollectlIntegration_Phase1_Complete.md b/docs/CollectlIntegration_Phase1_Complete.md new file mode 100644 index 00000000..e69de29b diff --git a/example/rbd_fio_test.yml b/example/rbd_fio_test.yml index cdaf9385..76bfde31 100644 --- a/example/rbd_fio_test.yml +++ b/example/rbd_fio_test.yml @@ -44,11 +44,11 @@ monitoring_profiles: # produce gnuplot file data format (aka csv) in output file (gziped) # args: '-c 30 -sZC -i 5:10 --procopts t --cpufilt 0-3 --procfilt cosd -P -f {collectl_dir}' - perf: + osd_perf: perf_cmd: 'perf' # This collects 10 secs of data to produce flame graphs args: 'record -e cycles:u --call-graph dwarf -i -p {pid} -o {perf_dir}/{pid}_osd_perf.out sleep 10' - top: + osd_top: top_cmd: 'top' # This collects 30 samples, Core and thread CPU utilisation args: '-b -H -1 -p {pid} -n 30 > {top_dir}/{pid}_osd_top.out' @@ -77,7 +77,6 @@ benchmarks: poolname: 'rbd' mode: 'randwrite' iodepth: [1, 4 ,16] - numjobs: [1, 4, 8] op_size: [4096] # block IO size in bytes procs_per_client: [1] volumes_per_client: [1] # volumes per ceph node diff --git a/monitoring.py b/monitoring.py deleted file mode 100644 index 025033e7..00000000 --- a/monitoring.py +++ /dev/null @@ -1,231 +0,0 @@ -from contextlib import contextmanager -import glob -import os.path -import common -import settings -import logging - -logger = logging.getLogger("cbt") - -class Monitoring(object): - def __init__(self, mconfig): - # the initializers should be the very single places interrogating - # settings for the sake of explicitness - nodes_list = mconfig.get('nodes', self._get_default_nodes()) - self.nodes = settings.getnodes(*nodes_list) - - @staticmethod - def _get_all(): - for monitoring, mconfig in sorted(settings.monitoring_profiles.items()): - yield Monitoring._get_object(monitoring, mconfig) - - @staticmethod - def _get_object(monitoring, mconfig): - if monitoring == 'collectl': - return CollectlMonitoring(mconfig) - if monitoring == 'perf': - return PerfMonitoring(mconfig) - if monitoring == 'blktrace': - return BlktraceMonitoring(mconfig) - if monitoring == 'top': - return TopMonitoring(mconfig) - - -class CollectlMonitoring(Monitoring): - def __init__(self, mconfig): - super(CollectlMonitoring, self).__init__(mconfig) - - self.args = mconfig.get('args', '-s+mYZ -i 1:10 -F0 -f {collectl_dir} ' - r'--rawdskfilt \"+cciss/c\d+d\d+ |hd[ab] | sd[a-z]+ |dm-\d+ |xvd[a-z] |fio[a-z]+ | vd[a-z]+ |emcpower[a-z]+ |psv\d+ |nvme[0-9]n[0-9]+p[0-9]+ \"') - - def start(self, directory): - collectl_dir = '%s/collectl' % directory - common.pdsh(self.nodes, 'mkdir -p -m0755 -- %s' % collectl_dir).communicate() - common.pdsh(self.nodes, ['collectl', self.args.format(collectl_dir=collectl_dir)]) - - def stop(self, directory): - common.pdsh(self.nodes, 'pkill -SIGINT -f collectl').communicate() - - @staticmethod - def _get_default_nodes(): - return ['clients', 'osds', 'mons', 'rgws'] - - -class PerfMonitoring(Monitoring): - def __init__(self, mconfig): - super(PerfMonitoring, self).__init__(mconfig) - self.pid_dir = settings.cluster.get('pid_dir') - self.pid_glob = mconfig.get('pid_glob', 'osd.*.pid') - self.user = settings.cluster.get('user') - self.perf_cmd = mconfig.get('perf_cmd', 'sudo perf') - self.args_template = mconfig.get('args') - self.perf_runners = [] - self.perf_dir = '' # we need the output file to extract data - - def start(self, directory): - perf_dir = '%s/perf' % directory - self.perf_dir = perf_dir - common.pdsh(self.nodes, 'mkdir -p -m0755 -- %s' % perf_dir).communicate() - - perf_template = '{} {} &'.format(self.perf_cmd, self.args_template) - local_node = common.get_localnode(self.nodes) - if local_node: - logger.debug("PerfMonitoring: in local_node"); - logger.debug("pid_dir: %s" % self.pid_dir); - for pid_path in glob.glob(os.path.join(self.pid_dir, self.pid_glob)): - logger.debug("PerfMonitoring pid_path: %s" % pid_path); - with open(pid_path) as pidfile: - pid = pidfile.read().strip() - perf_cmd = perf_template.format(perf_dir=perf_dir, pid=pid) - runner = common.sh(local_node, perf_cmd) - self.perf_runners.append(runner) - else: - logger.debug("PerfMonitoring: remote_node"); - # ${pid} will be handled by remote's sh - perf_cmd = perf_template.format(perf_dir=perf_dir, pid='${pid}') - common.pdsh(self.nodes, ['for pid in `cat %s/%s`;' % (self.pid_dir, self.pid_glob), - 'do', perf_cmd, - 'done']) - - def stop(self, directory): - if self.perf_runners: - for runner in self.perf_runners: - runner.kill() - else: - common.pdsh(self.nodes, 'sudo pkill -SIGINT -f perf\ ').communicate() - if directory: - common.pdsh(self.nodes, 'sudo chown {user}.{user} {dir}/perf/perf.data'.format( - user=self.user, dir=directory)) - common.pdsh(self.nodes, 'sudo chown {user}.{user} {dir}/perf/perf_stat.*'.format( - user=self.user, dir=directory)) - - def get_cpu_cycles(self, out_dir): - import re - total_cpu_cycles = 0 - perf_dir_name = str(glob.glob(out_dir + "/perf*")[0]) - perf_stat_fnames = os.listdir(perf_dir_name) - for perf_out_fname in perf_stat_fnames: - perf_output_file = open(perf_dir_name + "/" + perf_out_fname, "rt") - match = re.search(r'(.*) cycles(.*?) .*', perf_output_file.read(), re.M | re.I) - if match: - cpu_cycles = match.group(1).strip() - else: - return None - total_cpu_cycles = total_cpu_cycles + int(cpu_cycles.replace(',', '')) - return total_cpu_cycles - - @staticmethod - def _get_default_nodes(): - return ['osds'] - - -class BlktraceMonitoring(Monitoring): - def __init__(self, mconfig): - super(BlktraceMonitoring, self).__init__(mconfig) - self.osds_per_node = settings.cluster.get('osds_per_node') - self.use_existing = settings.cluster.get('use_existing', True) - self.user = settings.cluster.get('user') - - def start(self, directory): - blktrace_dir = '%s/blktrace' % directory - common.pdsh(self.nodes, 'mkdir -p -m0755 -- %s' % blktrace_dir).communicate() - for device in range(0, self.osds_per_node): - common.pdsh(self.nodes, 'cd %s;sudo blktrace -o device%s -d /dev/disk/by-partlabel/osd-device-%s-data' - % (blktrace_dir, device, device)) - - def stop(self, directory): - common.pdsh(self.nodes, 'sudo pkill -SIGINT -f blktrace').communicate() - if directory and not self.use_existing: - self._make_movies(directory) - - def _make_movies(self, directory): - seekwatcher = '/home/%s/bin/seekwatcher' % self.user - blktrace_dir = '%s/blktrace' % directory - - for device in range(self.osds_per_node): - common.pdsh(self.nodes, 'cd %s;%s -t device%s -o device%s.mpg --movie' % - (blktrace_dir, seekwatcher, device, device)).communicate() - - @staticmethod - def _get_default_nodes(): - return ['osds'] - - -class TopMonitoring(Monitoring): - def __init__(self, mconfig): - super(TopMonitoring, self).__init__(mconfig) - self.pid_dir = settings.cluster.get('pid_dir') - self.pid_glob = mconfig.get('pid_glob', 'osd.*.pid') - self.user = settings.cluster.get('user') - self.top_cmd = mconfig.get('top_cmd', 'top') - self.args = mconfig.get('args', '-b -H -1 -p {pid} -n 30 > {top_dir}/{pid}_osd_top.out') - self.top_runners = [] - - def start(self, directory): - top_dir = '%s/top' % directory - common.pdsh(self.nodes, 'mkdir -p -m0755 -- %s' % top_dir).communicate() - - top_template = '{} {}'.format(self.top_cmd, self.args) - local_node = common.get_localnode(self.nodes) - if local_node: - logger.debug("TopMonitoring: in local_node"); - logger.debug("pid_dir: %s" % self.pid_dir); - for pid_path in glob.glob(os.path.join(self.pid_dir, self.pid_glob)): - logger.debug("TopMonitoring pid_path: %s" % pid_path); - with open(pid_path) as pidfile: - pid = pidfile.read().strip() - top_cmd = top_template.format(top_dir=top_dir, pid=pid) - runner = common.sh(local_node, top_cmd) - self.top_runners.append(runner) - else: - logger.debug("TopMonitoring: remote_node"); - # ${pid} will be handled by remote's sh - top_cmd = top_template.format(top_dir=top_dir, pid='${pid}') - common.pdsh(self.nodes, ['for pid in `cat %s/%s`;' % (self.pid_dir, self.pid_glob), - 'do', top_cmd, - 'done']) - - def stop(self, directory): - #common.pdsh(self.nodes, 'pkill -SIGINT -f top').communicate() - if self.top_runners: - for runner in self.top_runners: - runner.kill() - else: - #ToDO: find the pid of the correct top instance process - common.pdsh(self.nodes, 'sudo pkill -SIGINT -f top\ ').communicate() - if directory: - common.pdsh(self.nodes, 'sudo chown {user}.{user} {dir}/top/*top.out'.format( - user=self.user, dir=directory)) - - @staticmethod - def _get_default_nodes(): - return ['osds'] - - -def start(directory): - for m in Monitoring._get_all(): - m.start(directory) - - -def stop(directory=None): - for m in Monitoring._get_all(): - m.stop(directory) - - -@contextmanager -def monitor(directory): - monitors = [] - for m in Monitoring._get_all(): - m.start(directory) - monitors.append(m) - yield - for m in monitors: - m.stop(directory) - - -def get_cpu_cycles(out_dir): - # check if perf stat is configured - for monitoring_profile in Monitoring._get_all(): - if(isinstance(monitoring_profile, PerfMonitoring)): - return monitoring_profile.get_cpu_cycles(out_dir) # if it is, then return the number of cycle - return None diff --git a/monitoring/__init__.py b/monitoring/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/monitoring/blktrace_monitoring.py b/monitoring/blktrace_monitoring.py new file mode 100644 index 00000000..0abb7de4 --- /dev/null +++ b/monitoring/blktrace_monitoring.py @@ -0,0 +1,50 @@ +"""Blktrace monitoring backend.""" + +import logging +from typing import Any, ClassVar, Optional, cast + +import common +import settings +from monitoring.monitoring import Monitoring + +logger = logging.getLogger("cbt") + + +class BlktraceMonitoring(Monitoring): + """Monitoring backend that captures blktrace output and optionally renders seekwatcher movies.""" + + DEFAULT_NODES: ClassVar[list[str]] = ["osds"] + + def __init__(self, mconfig: dict[str, Any]) -> None: + """Initialize blktrace monitoring configuration.""" + super().__init__(mconfig) + # Remove these casts once settings.cluster.get() is fully typed. + self._osds_per_node = cast(int, settings.cluster.get("osds_per_node")) + self._use_existing = cast(bool, settings.cluster.get("use_existing", True)) + self._user = cast(str, settings.cluster.get("user")) + + def start(self, directory: str) -> None: + """Create the blktrace output directory and start tracing on each OSD device.""" + blktrace_dir = f"{directory}/blktrace" + common.pdsh(self._nodes, f"mkdir -p -m0755 -- {blktrace_dir}").communicate() # type: ignore[no-untyped-call] + for device in range(self._osds_per_node): + common.pdsh( # type: ignore[no-untyped-call] + self._nodes, + f"cd {blktrace_dir};sudo blktrace -o device{device} -d /dev/disk/by-partlabel/osd-device-{device}-data", + ) + + def stop(self, directory: Optional[str]) -> None: + """Stop blktrace and optionally generate seekwatcher movies.""" + common.pdsh(self._nodes, "sudo pkill -SIGINT -f blktrace").communicate() # type: ignore[no-untyped-call] + if directory and not self._use_existing: + self._make_movies(directory) + + def _make_movies(self, directory: str) -> None: + """Generate an mpg movie for each OSD device using seekwatcher.""" + seekwatcher = f"/home/{self._user}/bin/seekwatcher" + blktrace_dir = f"{directory}/blktrace" + for device in range(self._osds_per_node): + common.pdsh( # type: ignore[no-untyped-call] + self._nodes, + f"cd {blktrace_dir};{seekwatcher} -t device{device} -o device{device}.mpg --movie", + ).communicate() diff --git a/monitoring/collectl_monitoring.py b/monitoring/collectl_monitoring.py new file mode 100644 index 00000000..a97a32b2 --- /dev/null +++ b/monitoring/collectl_monitoring.py @@ -0,0 +1,36 @@ +"""Collectl monitoring backend.""" + +from typing import Any, ClassVar, Optional + +import common +from monitoring.monitoring import Monitoring + + +class CollectlMonitoring(Monitoring): + """Monitoring backend that captures collectl output.""" + + DEFAULT_NODES: ClassVar[list[str]] = ["clients", "osds", "mons", "rgws"] + DEFAULT_ARGS: ClassVar[str] = ( + "-s+mYZ -i 1:10 -F0 -f {collectl_dir} " + r"--rawdskfilt \"+cciss/c\d+d\d+ |hd[ab] | sd[a-z]+ |dm-\d+ |" + r"xvd[a-z]+ |fio[a-z]+ | vd[a-z]+ |emcpower[a-z]+ |psv\d+ |" + r"nvme[0-9]n[0-9]+p[0-9]+ \"" + ) + + def __init__(self, mconfig: dict[str, Any]) -> None: + """Initialize collectl monitoring configuration.""" + super().__init__(mconfig) + self._args = mconfig.get("args", self.DEFAULT_ARGS) + + def start(self, directory: str) -> None: + """Create the output directory and start collectl.""" + collectl_dir = f"{directory}/collectl" + common.pdsh(self._nodes, f"mkdir -p -m0755 -- {collectl_dir}").communicate() # type: ignore[no-untyped-call] + common.pdsh( + self._nodes, ["collectl", self._args.format(collectl_dir=collectl_dir)] + ) # type: ignore[no-untyped-call] + + def stop(self, directory: Optional[str]) -> None: + """Stop running collectl processes.""" + del directory + common.pdsh(self._nodes, "pkill -SIGINT -f collectl").communicate() # type: ignore[no-untyped-call] diff --git a/monitoring/monitoring.py b/monitoring/monitoring.py new file mode 100644 index 00000000..33e5814e --- /dev/null +++ b/monitoring/monitoring.py @@ -0,0 +1,26 @@ +"""Base abstractions for monitoring backends.""" + +from abc import ABC, abstractmethod +from typing import Any, ClassVar, Optional, cast + +import settings + + +class Monitoring(ABC): + """Abstract base class for monitoring backends.""" + + DEFAULT_NODES: ClassVar[list[str]] + + def __init__(self, mconfig: dict[str, Any]) -> None: + """Resolve monitoring nodes from configuration or subclass defaults.""" + nodes_list = mconfig.get("nodes", self.DEFAULT_NODES) + # Remove this cast and ignore once settings.getnodes() is fully typed. + self._nodes = cast(str, settings.getnodes(*nodes_list)) # type: ignore[no-untyped-call] + + @abstractmethod + def start(self, directory: str) -> None: + """Start monitoring and write output beneath the given directory.""" + + @abstractmethod + def stop(self, directory: Optional[str]) -> None: + """Stop monitoring and finalize any output files.""" diff --git a/monitoring/monitoring_factory.py b/monitoring/monitoring_factory.py new file mode 100644 index 00000000..e321739c --- /dev/null +++ b/monitoring/monitoring_factory.py @@ -0,0 +1,81 @@ +"""Factory class for creating and managing monitoring backends.""" + +from collections.abc import Generator, Iterator +from contextlib import contextmanager +from typing import Any, ClassVar, Optional + +import settings +from monitoring.blktrace_monitoring import BlktraceMonitoring +from monitoring.collectl_monitoring import CollectlMonitoring +from monitoring.monitoring import Monitoring +from monitoring.perf_monitoring import OsdPerfMonitoring, PerfMonitoring +from monitoring.top_monitoring import OsdTopMonitoring, TopMonitoring + + +class MonitoringFactory: + """Instantiates monitoring backends and owns the lifecycle API.""" + + _REGISTRY: ClassVar[dict[str, type[Monitoring]]] = { + "collectl": CollectlMonitoring, + "perf": PerfMonitoring, + "osd_perf": OsdPerfMonitoring, + "blktrace": BlktraceMonitoring, + "top": TopMonitoring, + "osd_top": OsdTopMonitoring, + } + + @classmethod + def get_object(cls, name: str, mconfig: dict[str, Any]) -> Monitoring: + """Return a new monitoring instance for the given profile name. + + Raises: + ValueError: If *name* is not a known monitoring backend key. + """ + try: + return cls._REGISTRY[name](mconfig) + except KeyError as exc: + raise ValueError(f"Unknown monitoring backend: {name!r}") from exc + + @classmethod + def get_all(cls) -> Iterator[Monitoring]: + """Yield one instance for every entry in ``settings.monitoring_profiles``.""" + for name, mconfig in sorted(settings.monitoring_profiles.items()): + yield cls.get_object(name, mconfig) + + @classmethod + def start(cls, directory: str) -> None: + """Start all configured monitoring backends.""" + for monitor in cls.get_all(): + monitor.start(directory) + + @classmethod + def stop(cls, directory: Optional[str] = None) -> None: + """Stop all configured monitoring backends.""" + for monitor in cls.get_all(): + monitor.stop(directory) + + @classmethod + @contextmanager + def monitor(cls, directory: str) -> Generator[None, None, None]: + """Context manager: start all monitors, yield, then stop all.""" + monitors = list(cls.get_all()) + for monitor in monitors: + monitor.start(directory) + try: + yield + finally: + for monitor in monitors: + monitor.stop(directory) + + @classmethod + def get_cpu_cycles(cls, out_dir: str) -> Optional[int]: + """Return total CPU cycles from perf stat output, if perf is configured. + + Iterates monitoring profiles and delegates to the first + ``PerfMonitoring`` instance found. Returns ``None`` when no perf + profile is configured. + """ + for monitor in cls.get_all(): + if isinstance(monitor, PerfMonitoring): + return monitor.get_cpu_cycles(out_dir) + return None diff --git a/monitoring/perf_monitoring.py b/monitoring/perf_monitoring.py new file mode 100644 index 00000000..bad76ae0 --- /dev/null +++ b/monitoring/perf_monitoring.py @@ -0,0 +1,116 @@ +"""Perf monitoring backend.""" + +import glob +import logging +import os +import re +from typing import Any, ClassVar, Optional, cast + +import common +import settings +from monitoring.monitoring import Monitoring + +logger = logging.getLogger("cbt") + + +class PerfMonitoring(Monitoring): + """Monitoring backend that captures perf output. + + Runs ``perf`` with a caller-supplied ``args`` template. For OSD-specific + PID discovery use :class:`OsdPerfMonitoring` instead. + """ + + DEFAULT_NODES: ClassVar[list[str]] = ["osds"] + + def __init__(self, mconfig: dict[str, Any]) -> None: + """Initialize perf monitoring configuration.""" + super().__init__(mconfig) + # Remove these casts once settings.cluster.get() is fully typed. + self._user = cast(str, settings.cluster.get("user")) + self._perf_cmd = mconfig.get("perf_cmd", "sudo perf") + self._args_template = mconfig.get("args") + self._perf_runners: list[Any] = [] + self._perf_dir = "" + + def start(self, directory: str) -> None: + """Create the perf output directory and start perf collection.""" + perf_dir = f"{directory}/perf" + self._perf_dir = perf_dir + common.pdsh(self._nodes, f"mkdir -p -m0755 -- {perf_dir}").communicate() # type: ignore[no-untyped-call] + + perf_cmd = f"{self._perf_cmd} {self._args_template} &".format(perf_dir=perf_dir) + local_node = common.get_localnode(self._nodes) # type: ignore[no-untyped-call] + if local_node: + runner = common.sh(local_node, perf_cmd) # type: ignore[no-untyped-call] + self._perf_runners.append(runner) + else: + common.pdsh(self._nodes, perf_cmd) # type: ignore[no-untyped-call] + + def stop(self, directory: Optional[str]) -> None: + """Stop perf collection and adjust file ownership when needed.""" + if self._perf_runners: + for runner in self._perf_runners: + runner.kill() + else: + common.pdsh(self._nodes, r"sudo pkill -SIGINT -f perf\ ").communicate() # type: ignore[no-untyped-call] + if directory: + common.pdsh( # type: ignore[no-untyped-call] + self._nodes, f"sudo chown {self._user}.{self._user} {directory}/perf/perf.data" + ) + common.pdsh( # type: ignore[no-untyped-call] + self._nodes, f"sudo chown {self._user}.{self._user} {directory}/perf/perf_stat.*" + ) + + def get_cpu_cycles(self, out_dir: str) -> Optional[int]: + """Return total CPU cycles from perf stat output, if available.""" + total_cpu_cycles = 0 + perf_dir_name = str(glob.glob(out_dir + "/perf*")[0]) + perf_stat_fnames = os.listdir(perf_dir_name) + for perf_out_fname in perf_stat_fnames: + with open(f"{perf_dir_name}/{perf_out_fname}", encoding="utf-8") as perf_output_file: + match = re.search(r"(.*) cycles(.*?) .*", perf_output_file.read(), re.M | re.I) + if match: + cpu_cycles = match.group(1).strip() + else: + return None + total_cpu_cycles = total_cpu_cycles + int(cpu_cycles.replace(",", "")) + return cast(Optional[int], total_cpu_cycles) + + +class OsdPerfMonitoring(PerfMonitoring): + """PerfMonitoring specialised for Ceph OSD processes. + + Discovers the target PIDs by scanning PID files matching ``pid_glob`` + inside ``pid_dir`` (read from ``settings.cluster``), then launches a + separate ``perf`` invocation per OSD PID. + """ + + def __init__(self, mconfig: dict[str, Any]) -> None: + """Initialize OSD perf monitoring configuration.""" + super().__init__(mconfig) + self._pid_dir = cast(str, settings.cluster.get("pid_dir")) + self._pid_glob = mconfig.get("pid_glob", "osd.*.pid") + + def start(self, directory: str) -> None: + """Create the perf output directory and start a perf instance per OSD PID.""" + perf_dir = f"{directory}/perf" + self._perf_dir = perf_dir + common.pdsh(self._nodes, f"mkdir -p -m0755 -- {perf_dir}").communicate() # type: ignore[no-untyped-call] + + perf_template = f"{self._perf_cmd} {self._args_template} &" + local_node = common.get_localnode(self._nodes) # type: ignore[no-untyped-call] + if local_node: + logger.debug("OsdPerfMonitoring: local_node pid_dir=%s", self._pid_dir) + for pid_path in glob.glob(os.path.join(self._pid_dir, self._pid_glob)): + with open(pid_path, encoding="utf-8") as pidfile: + pid = pidfile.read().strip() + perf_cmd = perf_template.format(perf_dir=perf_dir, pid=pid) + runner = common.sh(local_node, perf_cmd) # type: ignore[no-untyped-call] + self._perf_runners.append(runner) + else: + logger.debug("OsdPerfMonitoring: remote_node") + perf_cmd = perf_template.format(perf_dir=perf_dir, pid="${pid}") + common.pdsh( # type: ignore[no-untyped-call] + self._nodes, + [f"for pid in `cat {self._pid_dir}/{self._pid_glob}`;", "do", perf_cmd, ";", "done"], + ) diff --git a/monitoring/top_monitoring.py b/monitoring/top_monitoring.py new file mode 100644 index 00000000..5177b8ad --- /dev/null +++ b/monitoring/top_monitoring.py @@ -0,0 +1,113 @@ +"""Top monitoring backend.""" + +import glob +import logging +import os +from typing import Any, ClassVar, Optional, cast + +import common +import settings +from monitoring.monitoring import Monitoring + +logger = logging.getLogger("cbt") + + +class TopMonitoring(Monitoring): + """Monitoring backend that runs top against an explicit PID list or system-wide. + + When ``args`` contains ``{pid}``, the caller is responsible for supplying + the PID list via ``start()``. For OSD-specific PID discovery use + :class:`OsdTopMonitoring` instead. + """ + + DEFAULT_NODES: ClassVar[list[str]] = ["osds"] + + def __init__(self, mconfig: dict[str, Any]) -> None: + """Initialize top monitoring configuration.""" + super().__init__(mconfig) + self._user = cast(str, settings.cluster.get("user")) + self._top_cmd = mconfig.get("top_cmd", "top") + self._args = mconfig.get("args", "-b -H -1 -n 30 > {top_dir}/top.out") + self._top_runners: list[Any] = [] + + def start(self, directory: str) -> None: + """Create the top output directory and start top collection.""" + top_dir = f"{directory}/top" + common.pdsh(self._nodes, f"mkdir -p -m0755 -- {top_dir}").communicate() # type: ignore[no-untyped-call] + + top_cmd = f"{self._top_cmd} {self._args}".format(top_dir=top_dir) + local_node = common.get_localnode(self._nodes) # type: ignore[no-untyped-call] + if local_node: + runner = common.sh(local_node, top_cmd) # type: ignore[no-untyped-call] + self._top_runners.append(runner) + else: + common.pdsh(self._nodes, top_cmd).communicate() # type: ignore[no-untyped-call] + + def stop(self, directory: Optional[str]) -> None: + """Stop top collection and adjust file ownership when needed.""" + if self._top_runners: + for runner in self._top_runners: + runner.kill() + else: + pkill_cmd = f"sudo pkill -SIGINT -f '{self._top_cmd} {self._args}'" + common.pdsh(self._nodes, pkill_cmd).communicate() # type: ignore[no-untyped-call] + if directory: + common.pdsh( # type: ignore[no-untyped-call] + self._nodes, + f"sudo chown {self._user}.{self._user} {directory}/top/*top.out", + ) + + +class OsdTopMonitoring(TopMonitoring): + """TopMonitoring specialised for Ceph OSD processes. + + Discovers the target PIDs by scanning PID files matching ``pid_glob`` + inside ``pid_dir`` (read from ``settings.cluster``), then launches a + separate ``top`` invocation per OSD PID. + """ + + def __init__(self, mconfig: dict[str, Any]) -> None: + """Initialize OSD top monitoring configuration.""" + super().__init__(mconfig) + self._pid_dir = cast(str, settings.cluster.get("pid_dir")) + self._pid_glob = mconfig.get("pid_glob", "osd.*.pid") + # Override default args to include per-pid placeholders. + self._args = mconfig.get("args", "-b -H -1 -p {pid} -n 30 > {top_dir}/{pid}_osd_top.out") + + def start(self, directory: str) -> None: + """Create the top output directory and start a top instance per OSD PID.""" + top_dir = f"{directory}/top" + common.pdsh(self._nodes, f"mkdir -p -m0755 -- {top_dir}").communicate() # type: ignore[no-untyped-call] + + top_template = f"{self._top_cmd} {self._args}" + local_node = common.get_localnode(self._nodes) # type: ignore[no-untyped-call] + if local_node: + logger.debug("OsdTopMonitoring: local_node pid_dir=%s", self._pid_dir) + pid_paths = glob.glob(os.path.join(self._pid_dir, self._pid_glob)) + if not pid_paths: + logger.warning( + "OsdTopMonitoring: no PID files matched %s in %s — no top processes started", + self._pid_glob, + self._pid_dir, + ) + for pid_path in pid_paths: + with open(pid_path, encoding="utf-8") as pidfile: + pid = pidfile.read().strip() + top_cmd = top_template.format(top_dir=top_dir, pid=pid) + runner = common.sh(local_node, top_cmd) # type: ignore[no-untyped-call] + self._top_runners.append(runner) + else: + logger.debug("OsdTopMonitoring: remote_node") + pid_glob_path = f"{self._pid_dir}/{self._pid_glob}" + ls_runner = common.pdsh(self._nodes, f"ls {pid_glob_path} 2>/dev/null") # type: ignore[no-untyped-call] + stdout, _ = ls_runner.communicate() + if not stdout.strip(): + logger.warning( + "OsdTopMonitoring: no PID files matched %s on remote nodes — no top processes started", + pid_glob_path, + ) + top_cmd = top_template.format(top_dir=top_dir, pid="${pid}") + common.pdsh( # type: ignore[no-untyped-call] + self._nodes, + [f"for pid in `cat {pid_glob_path}`;", "do", top_cmd, ";", "done"], + ).communicate() diff --git a/post_processing/formatter/common_output_formatter.py b/post_processing/formatter/common_output_formatter.py index bee8b642..961e163b 100644 --- a/post_processing/formatter/common_output_formatter.py +++ b/post_processing/formatter/common_output_formatter.py @@ -45,7 +45,9 @@ import json import re from pathlib import Path -from typing import Optional +from typing import Optional, Union + +from typing_extensions import override from post_processing.formatter.base_formatter import BaseFormatter from post_processing.post_processing_types import CommonFormatDataType, InternalFormattedOutputType @@ -259,6 +261,7 @@ def _write_operation_results( # pylint: disable=too-many-locals except OSError as e: self.log.error("Failed to write hockey-stick file %s: %s", filename, e) + @override def process(self) -> None: """ Process input data and convert to intermediate format. @@ -290,13 +293,13 @@ def process(self) -> None: self.log.debug( "We have more than one directory for test run %s so using the compatibility method", testrun_id ) - self._process_compatibility_mode() + _ = self._process_compatibility_mode() # For compatibility mode, still need to add metadata and write self._add_common_metadata() self._add_peak_metrics() else: # Memory-efficient mode: results written during _process_single_testrun - self._process_single_testrun(testrun_directories[0]) + _ = self._process_single_testrun(testrun_directories[0]) def _find_maximum_bandwidth_and_iops_with_latency( self, test_run_data: CommonFormatDataType @@ -344,7 +347,35 @@ def _find_max_resource_usage(self, test_run_data: CommonFormatDataType) -> tuple for _, data in test_run_data.items(): if isinstance(data, dict): - max_cpu = max(max_cpu, float(data["cpu"])) - # max memory here, when we start recording it + # Handle multi-source resource data structure + # data["cpu"] is a dict like {"fio": "10.5", "collectl": "12.3"} + cpu_data: Union[dict[str, str], str, int, float] = data.get("cpu", {}) + if isinstance(cpu_data, dict): + # Find max across all sources + for source_cpu in cpu_data.values(): + try: + max_cpu = max(max_cpu, float(source_cpu)) + except (ValueError, TypeError): + self.log.warning("Invalid CPU value: %s", source_cpu) + else: + # Backward compatibility: handle old single-value format + try: + max_cpu = max(max_cpu, float(cpu_data)) + except (ValueError, TypeError): + self.log.warning("Invalid CPU value: %s", cpu_data) + + # Handle memory similarly (when implemented) + memory_data: Union[dict[str, str], str, int, float] = data.get("memory", {}) + if isinstance(memory_data, dict): + for source_memory in memory_data.values(): + try: + max_memory = max(max_memory, float(source_memory)) + except (ValueError, TypeError): + self.log.warning("Invalid memory value: %s", source_memory) + else: + try: + max_memory = max(max_memory, float(memory_data)) + except (ValueError, TypeError): + self.log.warning("Invalid memory value: %s", memory_data) return f"{max_cpu}", f"{max_memory}" diff --git a/post_processing/plotter/common_format_plotter.py b/post_processing/plotter/common_format_plotter.py index ddbf8f4b..73c81a9b 100644 --- a/post_processing/plotter/common_format_plotter.py +++ b/post_processing/plotter/common_format_plotter.py @@ -10,7 +10,7 @@ # the ModuleType does exists in the types module, so no idea why pylint is # flagging this from types import ModuleType -from typing import Optional +from typing import Any, Optional, Union, cast from matplotlib.axes import Axes @@ -41,8 +41,8 @@ class CommonFormatPlotter(ABC): The base class for plotting results curves """ - def __init__(self, plotter: ModuleType): - self._plotter = plotter + def __init__(self, plotter: ModuleType) -> None: + self._plotter: ModuleType = plotter @abstractmethod def draw_and_save(self) -> None: @@ -70,7 +70,7 @@ def _add_title(self, source_files: list[Path]) -> None: else: title = self._construct_title_from_list_of_file_names(source_files) - self._plotter.title(title) + _: Any = self._plotter.title(title) def _construct_title_from_list_of_file_names(self, file_paths: list[Path]) -> str: """ @@ -142,8 +142,8 @@ def _set_axis(self, maximum_values: Optional[tuple[int, int]] = None) -> None: maximum_x = maximum_values[0] maximum_y = maximum_values[1] - self._plotter.xlim(0, maximum_x) - self._plotter.ylim(0, maximum_y) + _: Any = self._plotter.xlim(0, maximum_x) + _: Any = self._plotter.ylim(0, maximum_y) def _sort_plot_data(self, unsorted_data: CommonFormatDataType) -> PlotDataType: """ @@ -255,6 +255,74 @@ def _validate_input_data(self, sorted_plot_data: PlotDataType) -> None: if not sorted_plot_data: raise ValueError("Cannot extract plot data from empty dataset") + def _process_cpu_data( + self, + data: dict[str, str], + queue_depth: str, + cpu_plotter: CPUPlotter, + resource_plotting_enabled: bool, + ) -> bool: + """ + Process CPU data from a data point, handling both single-value and multi-source formats. + + Args: + data: Data dictionary containing CPU information + queue_depth: Queue depth identifier for error messages + cpu_plotter: CPU plotter to receive CPU data + resource_plotting_enabled: Whether resource plotting is requested + + Returns: + True if CPU data was successfully processed, False otherwise + """ + if not resource_plotting_enabled: + return False + + cpu_value: Union[dict[str, str], str, None] = data.get("cpu") + if cpu_value is None: + return False + + # Pass CPU data directly to plotter - it handles both dict and single value formats + try: + if isinstance(cpu_value, dict): + # Validate that dict contains at least one valid numeric value + valid_sources = 0 + cpu_dict = cast(dict[str, str], cpu_value) + for source, value in cpu_dict.items(): + try: + _ = float(value) # Validate it's convertible to float + valid_sources += 1 + except (ValueError, TypeError): + log.warning( + "Invalid CPU value from source '%s' for queue depth %s: %s. Skipping this source.", + source, + queue_depth, + value, + ) + + if valid_sources == 0: + log.warning( + "No valid CPU values found for queue depth %s: %s. Skipping CPU data.", + queue_depth, + cpu_value, + ) + return False + + # Pass the entire dict to plotter so it can plot separate lines + cpu_plotter.add_y_data(cpu_value) + return True + + # Single value format (backward compatibility) + _ = float(cpu_value) # Validate it's convertible to float + cpu_plotter.add_y_data(cpu_value) + return True + except (ValueError, TypeError): + log.warning( + "Invalid CPU value for queue depth %s: %s. Skipping CPU data.", + queue_depth, + cpu_value, + ) + return False + def _process_data_point( # pylint: disable=too-many-arguments,too-many-positional-arguments self, data: dict[str, str], @@ -291,7 +359,7 @@ def _process_data_point( # pylint: disable=too-many-arguments,too-many-position # Validate latency is numeric latency_value = data["latency"] try: - float(latency_value) # Validate it's convertible to float + _ = float(latency_value) # Validate it's convertible to float except (ValueError, TypeError) as e: raise ValueError(f"Invalid latency value for queue depth {queue_depth}: {latency_value}") from e @@ -302,23 +370,7 @@ def _process_data_point( # pylint: disable=too-many-arguments,too-many-position io_plotter.add_y_data(latency_value) # Handle CPU data with proper validation - resource_available = resource_plotting_enabled - if resource_plotting_enabled: - cpu_value = data.get("cpu") - if cpu_value is None: - resource_available = False - else: - # Validate CPU value is numeric - try: - float(cpu_value) - cpu_plotter.add_y_data(cpu_value) - except (ValueError, TypeError): - log.warning( - "Invalid CPU value for queue depth %s: %s. Skipping CPU data.", - queue_depth, - cpu_value, - ) - resource_available = False + resource_available = self._process_cpu_data(data, queue_depth, cpu_plotter, resource_plotting_enabled) # Calculate error bars error_bar = self._calculate_error_bar(data, plot_error_bars, resource_available) @@ -380,14 +432,13 @@ def _extract_plot_data( # pylint: disable=too-many-arguments,too-many-positiona # Set x-axis label once (from first valid data point) if x_label is None: x_label = point_result.x_label - main_axes.set_xlabel(x_label) # pyright: ignore[reportUnknownMemberType] + _: Any = main_axes.set_xlabel(x_label) # pyright: ignore[reportUnknownMemberType] # Disable resource plotting if CPU data unavailable (only check once) if resource_plotting_enabled and not point_result.resource_available: if not cpu_warning_logged: log.warning( - "Unable to plot CPU usage: CPU data not found in intermediate files. " - "Disabling resource usage plotting." + "Unable to plot CPU usage: CPU data not found in intermediate files. Disabling resource usage plotting." ) cpu_warning_logged = True resource_plotting_enabled = False @@ -474,11 +525,11 @@ def _save_plot(self, file_path: str) -> None: The bbox_inches="tight" option makes sure that the legend is included in the plot and not cut off """ - self._plotter.savefig(file_path, format=f"{PLOT_FILE_EXTENSION}", bbox_inches="tight") + _: Any = self._plotter.savefig(file_path, format=f"{PLOT_FILE_EXTENSION}", bbox_inches="tight") def _clear_plot(self) -> None: """ Clear the plot data """ - self._plotter.close() + _: Any = self._plotter.close() # self._plotter.clf() diff --git a/post_processing/plotter/cpu_plotter.py b/post_processing/plotter/cpu_plotter.py index 56b3fcfe..f42afc3e 100644 --- a/post_processing/plotter/cpu_plotter.py +++ b/post_processing/plotter/cpu_plotter.py @@ -4,32 +4,102 @@ """ from logging import Logger, getLogger +from typing import Union + +from matplotlib.axes import Axes from post_processing.plotter.axis_plotter import AxisPlotter log: Logger = getLogger("plotter") -CPU_PLOT_DEFAULT_COLOUR: str = "xkcd:leaf green" # Leaf green from xkcd color survey CPU_Y_LABEL: str = "System CPU use (%)" CPU_PLOT_LABEL: str = "CPU use" +# Color mapping for different resource sources +CPU_SOURCE_COLOURS: dict[str, str] = { + "fio": "xkcd:teal", + "collectl": "xkcd:sky blue", + "top": "xkcd:olive", + "default": "xkcd:orange", +} + class CPUPlotter(AxisPlotter): """ - A class to add the resource use measurements to a plot as separate axes + A class to add CPU usage measurements to a plot as separate axes. + + Supports both single-source (legacy) and multi-source formats: + - Legacy: data_value is a string "45.2" + - Multi-source: data_value is a dict {"fio": "45.2", "collectl": "47.8"} + + When multiple sources are present, plots separate lines for each source. """ - def add_y_data(self, data_value: str) -> None: + def __init__(self, main_axis: "Axes") -> None: + """Initialize CPUPlotter with support for multiple data sources.""" + super().__init__(main_axis) + # Store data per source: {"fio": [val1, val2, ...], "collectl": [...]} + self._y_data_by_source: dict[str, list[float]] = {} + + def add_y_data(self, data_value: Union[str, dict[str, str]]) -> None: """ - Add a point of CPU data for this plot + Add a point of CPU data for this plot. + + Supports both legacy single-value format and new multi-source format. - :param cpu_value: A single value for CPU usage - :type cpu_value: str + Args: + data_value: Either a single string value (legacy) or dict of {source: value} """ - self._y_data.append(float(data_value)) + if isinstance(data_value, dict): + # Multi-source format: {"fio": "45.2", "collectl": "47.8"} + for source, value in data_value.items(): + if source not in self._y_data_by_source: + self._y_data_by_source[source] = [] + try: + self._y_data_by_source[source].append(float(value)) + except (ValueError, TypeError) as e: + log.warning("Invalid CPU value for source %s: %s (%s)", source, value, e) + self._y_data_by_source[source].append(0.0) + else: + # Legacy single-value format: "45.2" + if "default" not in self._y_data_by_source: + self._y_data_by_source["default"] = [] + try: + self._y_data_by_source["default"].append(float(data_value)) + except (ValueError, TypeError) as e: + log.warning("Invalid CPU value: %s (%s)", data_value, e) + self._y_data_by_source["default"].append(0.0) def plot(self, x_data: list[float], colour: str = "") -> None: + """ + Plot CPU data, creating separate lines for each source. + + Args: + x_data: X-axis data points (typically queue depths or time) + colour: Ignored - colors are determined per source + """ + if not self._y_data_by_source: + log.debug("No CPU data to plot") + return + cpu_axis = self._main_axes.twinx() - self._label = CPU_PLOT_LABEL - self._y_label = CPU_Y_LABEL - self._plot(x_data=x_data, axis=cpu_axis, colour=CPU_PLOT_DEFAULT_COLOUR) + cpu_axis.set_ylabel(CPU_Y_LABEL) + + # Plot a line for each source + for source in sorted(self._y_data_by_source.keys()): + y_data = self._y_data_by_source[source] + + # Determine label and color for this source + if source == "default": + label = CPU_PLOT_LABEL + else: + label = f"{CPU_PLOT_LABEL} ({source})" + + source_colour = CPU_SOURCE_COLOURS.get(source, CPU_SOURCE_COLOURS["default"]) + + # Plot this source's data + cpu_axis.plot(x_data, y_data, label=label, color=source_colour, linestyle="-", linewidth=1.5, marker="o") + + # Add legend if multiple sources + if len(self._y_data_by_source) > 1: + cpu_axis.legend(loc="upper right") diff --git a/post_processing/plotter/io_plotter.py b/post_processing/plotter/io_plotter.py index f3b976fa..86d8cc9b 100644 --- a/post_processing/plotter/io_plotter.py +++ b/post_processing/plotter/io_plotter.py @@ -9,7 +9,7 @@ log: Logger = getLogger("plotter") -IO_PLOT_DEFAULT_COLOUR: str = "xkcd:leaf green" # Leaf green from xkcd color survey +IO_PLOT_DEFAULT_COLOUR: str = "xkcd:teal" # Teal from xkcd color survey IO_Y_LABEL: str = "Latency (ms)" IO_PLOT_LABEL: str = "IO Details" @@ -48,5 +48,12 @@ def plot_with_error_bars(self, x_data: list[float], error_data: list[float], cap io_axis.set_ylabel(self.y_label) # io_axis.tick_params(axis="y") # pyright: ignore[reportUnknownMemberType] io_axis.errorbar( # pyright: ignore[reportUnknownMemberType] - x_data, self._y_data, yerr=error_data, fmt="+-", capsize=cap_size, ecolor="red", label=self._label + x_data, + self._y_data, + yerr=error_data, + fmt="+-", + capsize=cap_size, + color=IO_PLOT_DEFAULT_COLOUR, + ecolor="xkcd:red", + label=self._label, ) diff --git a/post_processing/plotter/memory_plotter.py b/post_processing/plotter/memory_plotter.py index db884d63..da31cb79 100644 --- a/post_processing/plotter/memory_plotter.py +++ b/post_processing/plotter/memory_plotter.py @@ -6,31 +6,103 @@ from logging import Logger, getLogger from typing import Union +from matplotlib.axes import Axes +from typing_extensions import override + from post_processing.plotter.axis_plotter import AxisPlotter log: Logger = getLogger("plotter") -MEMORY_PLOT_DEFAULT_COLOUR: str = "#4b006e" MEMORY_Y_LABEL: str = "Memory use (Mb)" MEMORY_PLOT_LABEL: str = "Memory use" +# Color mapping for different resource sources +MEMORY_SOURCE_COLOURS: dict[str, str] = { + "fio": "xkcd:purple", + "collectl": "xkcd:red", + "top": "xkcd:dark teal", + "default": "xkcd:brown", +} + class MemoryPlotter(AxisPlotter): """ - A class to add the resource use measurements to a plot as separate axes + A class to add memory usage measurements to a plot as separate axes. + + Supports both single-source (legacy) and multi-source formats: + - Legacy: data_value is a string "1024.5" + - Multi-source: data_value is a dict {"fio": "1024.5", "collectl": "2048.0"} + + When multiple sources are present, plots separate lines for each source. """ - def add_y_data(self, data_value: str) -> None: + def __init__(self, main_axis: Axes) -> None: + """Initialize MemoryPlotter with support for multiple data sources.""" + super().__init__(main_axis) + # Store data per source: {"fio": [val1, val2, ...], "collectl": [...]} + self._y_data_by_source: dict[str, list[float]] = {} + + @override + def add_y_data(self, data_value: Union[str, dict[str, str]]) -> None: """ - Add a point of memory usage data for this plot + Add a point of memory data for this plot. + + Supports both legacy single-value format and new multi-source format. - :param memory_value: A single value for memory usage - :type memory_value: str + Args: + data_value: Either a single string value (legacy) or dict of {source: value} """ - self._y_data.append(float(data_value)) + if isinstance(data_value, dict): + # Multi-source format: {"fio": "1024.5", "collectl": "2048.0"} + for source, value in data_value.items(): + if source not in self._y_data_by_source: + self._y_data_by_source[source] = [] + try: + self._y_data_by_source[source].append(float(value)) + except (ValueError, TypeError) as e: + log.warning("Invalid memory value for source %s: %s (%s)", source, value, e) + self._y_data_by_source[source].append(0.0) + else: + # Legacy single-value format: "1024.5" + if "default" not in self._y_data_by_source: + self._y_data_by_source["default"] = [] + try: + self._y_data_by_source["default"].append(float(data_value)) + except (ValueError, TypeError) as e: + log.warning("Invalid memory value: %s (%s)", data_value, e) + self._y_data_by_source["default"].append(0.0) + @override def plot(self, x_data: list[Union[int, float]], colour: str = "") -> None: - memory_axis = self._main_axes.twinx() - self._label = MEMORY_PLOT_LABEL - self._y_label = MEMORY_Y_LABEL - self._plot(x_data=x_data, axis=memory_axis, colour=MEMORY_PLOT_DEFAULT_COLOUR) + """ + Plot memory data, creating separate lines for each source. + + Args: + x_data: X-axis data points (typically queue depths or time) + colour: Ignored - colors are determined per source + """ + if not self._y_data_by_source: + log.debug("No memory data to plot") + return + + memory_axis: Axes = self._main_axes.twinx() + memory_axis.set_ylabel(MEMORY_Y_LABEL) + + # Plot a line for each source + for source in sorted(self._y_data_by_source.keys()): + y_data = self._y_data_by_source[source] + + # Determine label and color for this source + if source == "default": + label = MEMORY_PLOT_LABEL + else: + label = f"{MEMORY_PLOT_LABEL} ({source})" + + source_colour = MEMORY_SOURCE_COLOURS.get(source, MEMORY_SOURCE_COLOURS["default"]) + + # Plot this source's data + memory_axis.plot(x_data, y_data, label=label, color=source_colour, linestyle="-", linewidth=1.5, marker="s") + + # Add legend if multiple sources + if len(self._y_data_by_source) > 1: + memory_axis.legend(loc="upper right") diff --git a/post_processing/plotter/time_series_latency_plotter.py b/post_processing/plotter/time_series_latency_plotter.py index e3a0773d..e83de039 100644 --- a/post_processing/plotter/time_series_latency_plotter.py +++ b/post_processing/plotter/time_series_latency_plotter.py @@ -15,9 +15,9 @@ log: Logger = getLogger("plotter") LATENCY_MEAN_COLOR: str = "xkcd:orange" # Orange from xkcd color survey -LATENCY_P50_COLOR: str = "xkcd:green" # Green from xkcd color survey +LATENCY_P50_COLOR: str = "xkcd:blue" # Blue from xkcd color survey LATENCY_P95_COLOR: str = "xkcd:red" # Red from xkcd color survey -LATENCY_P99_COLOR: str = "xkcd:dark red" # Dark red from xkcd color survey +LATENCY_P99_COLOR: str = "xkcd:purple" # Purple from xkcd color survey LATENCY_MAX_COLOR: str = "xkcd:dark grey" # Dark grey from xkcd color survey LATENCY_Y_LABEL: str = "Latency (ms)" LATENCY_PLOT_LABEL: str = "Mean Latency" diff --git a/post_processing/run_results/resource_result_factory.py b/post_processing/run_results/resource_result_factory.py new file mode 100644 index 00000000..abd34ed3 --- /dev/null +++ b/post_processing/run_results/resource_result_factory.py @@ -0,0 +1,71 @@ +""" +Factory for discovering and creating all available resource result parsers. + +This module provides functionality to automatically discover and instantiate +all available resource monitoring parsers (FIO, Collectl, etc.) for a given +benchmark output file. +""" + +from logging import Logger, getLogger +from pathlib import Path + +from post_processing.run_results.resource_result import ResourceResult +from post_processing.run_results.resources.collectl_resource import CollectlResource +from post_processing.run_results.resources.fio_resource import FIOResource +from post_processing.run_results.resources.top_resource import TopResource + +log: Logger = getLogger("formatter") + + +def get_all_resources(file_path: Path) -> list[ResourceResult]: + """ + Discover and instantiate all available resource parsers for a benchmark file. + + This function attempts to create resource parsers for all available monitoring + sources. It will try FIO (embedded in benchmark output) and Collectl (separate + monitoring files) if available. + + Args: + file_path: Path to the benchmark output file (e.g., json_output.0) + + Returns: + List of ResourceResult instances for all available sources. + Returns empty list if no sources are available. + """ + resources: list[ResourceResult] = [] + + # Always try FIO resource (embedded in benchmark output) + try: + fio_resource = FIOResource(file_path) + resources.append(fio_resource) + log.debug("Added FIO resource for %s", file_path) + except Exception as e: + log.warning("Could not create FIO resource for %s: %s", file_path, e) + + # Check for collectl data + collectl_dir = file_path.parent / "collectl" + if collectl_dir.exists() and collectl_dir.is_dir(): + try: + collectl_resource = CollectlResource(file_path) + resources.append(collectl_resource) + log.debug("Added Collectl resource for %s", file_path) + except Exception as e: + log.warning("Could not create Collectl resource for %s: %s", file_path, e) + + # Check for top monitoring data + top_dir = file_path.parent / "top" + if top_dir.exists() and top_dir.is_dir(): + try: + top_resource = TopResource(file_path) + resources.append(top_resource) + log.debug("Added Top resource for %s", file_path) + except Exception as e: + log.warning("Could not create Top resource for %s: %s", file_path, e) + + if not resources: + log.error("No resource parsers available for %s", file_path) + + return resources + + +# Made with Bob diff --git a/post_processing/run_results/resources/collectl_resource.py b/post_processing/run_results/resources/collectl_resource.py new file mode 100644 index 00000000..0a0130c8 --- /dev/null +++ b/post_processing/run_results/resources/collectl_resource.py @@ -0,0 +1,209 @@ +""" +Process CPU statistics from Collectl monitoring output. + +Collectl is a system monitoring tool that captures detailed performance metrics. +This module parses collectl's semicolon-separated CPU output files to extract +CPU usage statistics. +""" + +from logging import Logger, getLogger +from pathlib import Path +from typing import Any + +from post_processing.run_results.resource_result import ResourceResult + +log: Logger = getLogger("formatter") + + +class CollectlResource(ResourceResult): + """ + Processes resource usage statistics from Collectl monitoring output. + + Collectl produces detailed system monitoring data in semicolon-separated + format. This class extracts CPU usage by aggregating across all cores + and time samples. + + The collectl output files are expected to be in a 'collectl' subdirectory + relative to the benchmark output file, with filenames matching the pattern: + -.cpu + """ + + @property + def source(self) -> str: + """Return the source identifier for this resource parser.""" + return "collectl" + + def _get_resource_output_file_from_file_path(self, file_path: Path) -> Path: + """ + Locate the collectl CPU file from the benchmark output file path. + + Args: + file_path: Path to benchmark output (e.g., .../json_output.0) + + Returns: + Path to collectl .cpu file + + Raises: + FileNotFoundError: If collectl directory or CPU file not found + """ + collectl_dir = file_path.parent / "collectl" + + if not collectl_dir.exists(): + raise FileNotFoundError(f"Collectl directory not found: {collectl_dir}") + + # Find .cpu files (format: hostname-YYYYMMDD.cpu) + cpu_files = list(collectl_dir.glob("*.cpu")) + + if not cpu_files: + raise FileNotFoundError(f"No .cpu files found in {collectl_dir}") + + if len(cpu_files) > 1: + log.warning("Multiple .cpu files found in %s, using first: %s", collectl_dir, cpu_files[0]) + + return cpu_files[0] + + def _parse(self, data: dict[str, Any]) -> None: + """ + Parse collectl CPU data and calculate average CPU usage. + + Reads the semicolon-separated CPU file, extracts per-core metrics, + and calculates the average total CPU usage across all cores and time samples. + + Args: + data: Not used for collectl (reads directly from file) + """ + try: + cpu_usage = self._parse_cpu_file() + self._cpu = f"{cpu_usage:.2f}" + self._memory = "0.00" # Memory not implemented yet + self._has_been_parsed = True + except Exception as e: + log.error("Failed to parse collectl data from %s: %s", self._resource_file_path, e) + self._cpu = "0.00" + self._memory = "0.00" + self._has_been_parsed = True + + def _parse_cpu_file(self) -> float: + """ + Parse collectl .cpu file and calculate average CPU usage. + + Returns: + Average CPU usage percentage across all cores and time samples + """ + with open(self._resource_file_path, encoding="utf-8") as f: + lines = f.readlines() + + # Find header line (starts with #Date;Time;) + header_line = None + data_start_idx = 0 + + for idx, line in enumerate(lines): + if line.startswith("#Date;Time;"): + header_line = line + data_start_idx = idx + 1 + break + + if not header_line: + raise ValueError("Could not find header line in collectl CPU file") + + # Parse header to find CPU column indices + headers = header_line.strip().split(";") + cpu_indices = self._find_cpu_columns(headers) + + if not cpu_indices["user"] and not cpu_indices["totl"]: + raise ValueError("No CPU columns found in collectl header") + + # Parse data lines and calculate average + total_cpu_samples: list[float] = [] + + for line in lines[data_start_idx:]: + line = line.strip() + if not line or line.startswith("#"): + continue + + values = line.split(";") + if len(values) < len(headers): + log.warning("Skipping malformed line: %s", line) + continue + + # Calculate total CPU for this sample (sum across all cores) + sample_cpu = self._calculate_sample_cpu(values, cpu_indices) + total_cpu_samples.append(sample_cpu) + + if not total_cpu_samples: + log.warning("No valid CPU samples found in %s", self._resource_file_path) + return 0.0 + + # Return average CPU across all samples + avg_cpu = sum(total_cpu_samples) / len(total_cpu_samples) + log.debug("Parsed %d CPU samples, average: %.2f%%", len(total_cpu_samples), avg_cpu) + + return avg_cpu + + def _find_cpu_columns(self, headers: list[str]) -> dict[str, list[int]]: + """ + Find column indices for CPU metrics (User%, Sys%, etc.) for each core. + + Args: + headers: List of column headers from collectl file + + Returns: + Dict mapping metric names to lists of column indices + """ + cpu_columns: dict[str, list[int]] = {"user": [], "sys": [], "totl": []} + + for idx, header in enumerate(headers): + # Match patterns like [CPU:0]User%, [CPU:15]Sys%, etc. + if "User%" in header: + cpu_columns["user"].append(idx) + elif "Sys%" in header: + cpu_columns["sys"].append(idx) + elif "Totl%" in header: + cpu_columns["totl"].append(idx) + + return cpu_columns + + def _calculate_sample_cpu(self, values: list[str], cpu_indices: dict[str, list[int]]) -> float: + """ + Calculate total CPU usage for a single time sample. + + Args: + values: List of values from a data line + cpu_indices: Dict of CPU column indices + + Returns: + Average CPU usage across all cores for this sample + """ + # If Totl% is available, use it directly + if cpu_indices["totl"]: + totals = [float(values[idx]) for idx in cpu_indices["totl"] if idx < len(values)] + return sum(totals) / len(totals) if totals else 0.0 + + # Otherwise calculate from User% + Sys% + num_cores = len(cpu_indices["user"]) + if num_cores == 0: + return 0.0 + + core_totals: list[float] = [] + for core_idx in range(num_cores): + user_idx = cpu_indices["user"][core_idx] + sys_idx = cpu_indices["sys"][core_idx] + + if user_idx < len(values) and sys_idx < len(values): + user = float(values[user_idx]) + sys = float(values[sys_idx]) + core_totals.append(user + sys) + + return sum(core_totals) / len(core_totals) if core_totals else 0.0 + + def _read_results_from_file(self) -> dict[str, Any]: + """ + Override parent method - collectl doesn't use JSON format. + + Returns: + Empty dict (parsing happens in _parse_cpu_file) + """ + return {} + + +# Made with Bob diff --git a/post_processing/run_results/resources/top_resource.py b/post_processing/run_results/resources/top_resource.py new file mode 100644 index 00000000..b0abb324 --- /dev/null +++ b/post_processing/run_results/resources/top_resource.py @@ -0,0 +1,197 @@ +""" +Process CPU and memory statistics from Linux top batch-mode output files. + +CBT runs ``top -b -H -1 -p {pid} -n {count}`` for each monitored PID and +writes output to a ``top/`` subdirectory of the benchmark run directory +(e.g. ``results/.../iodepth-000128/top/``). + +The output filename is controlled by the ``args`` template in +``TopMonitoring`` and can be anything the user configures. ``TopResource`` +therefore reads **every file** in the ``top/`` directory rather than +filtering by a fixed glob, keeping the two components fully decoupled. + +The -H flag shows individual threads, so each snapshot may contain many lines. +CPU and memory values are averaged across all threads, all snapshots, and all +files to produce a single representative value per metric. +""" + +from logging import Logger, getLogger +from pathlib import Path +from typing import Any + +from post_processing.run_results.resource_result import ResourceResult + +log: Logger = getLogger("formatter") + + +class TopResource(ResourceResult): + """ + Processes resource usage from Linux top batch-mode output files. + + CBT stores one file per monitored PID in a ``top/`` subdirectory alongside + the benchmark output file. Each file contains multiple top snapshots + (because top is run with ``-n ``). + + All files present in the ``top/`` directory are parsed — the output + filename is determined by the ``args`` template in ``TopMonitoring`` and + may be anything the user configures. + + CPU and memory values are averaged across all threads, all snapshots, and + all files to produce a single representative value per metric. + + Args: + file_path: Path to the benchmark output file (e.g., ``json_output.0``) + """ + + @property + def source(self) -> str: + """Return the source identifier for this resource parser.""" + return "top" + + def _get_resource_output_file_from_file_path(self, file_path: Path) -> Path: + """ + Locate the top output directory from the benchmark result file path. + + The ``top/`` directory must exist next to the benchmark output file + and contain at least one file. All files present are parsed — the + naming convention is not enforced here. + + Args: + file_path: Path to the benchmark output file (e.g., ``json_output.0``) + + Returns: + Path to the top output directory + + Raises: + FileNotFoundError: If the top directory does not exist or is empty + """ + top_dir = file_path.parent / "top" + + if not top_dir.exists(): + raise FileNotFoundError(f"top directory not found: {top_dir}") + + if not any(top_dir.iterdir()): + raise FileNotFoundError(f"top directory is empty: {top_dir}") + + # Return the directory so _parse() can iterate all files inside it + return top_dir + + def _read_results_from_file(self) -> dict[str, Any]: + """ + Override parent: top output is plain text, not JSON. + + Returns: + Empty dict; parsing happens directly in _parse_top_directory() + """ + return {} + + def _parse(self, data: dict[str, Any]) -> None: + """ + Parse all *_osd_top.out files and average CPU and memory across all + threads, snapshots, and PID files. + + Args: + data: Not used (top output is plain text, not JSON) + """ + try: + cpu_usage, memory_usage = self._parse_top_directory() + self._cpu = f"{cpu_usage:.2f}" + self._memory = f"{memory_usage:.2f}" + self._has_been_parsed = True + except Exception as e: # pylint: disable=broad-except + log.error("Failed to parse top data from %s: %s", self._resource_file_path, e) + self._cpu = "0.00" + self._memory = "0.00" + self._has_been_parsed = True + + def _parse_top_directory(self) -> tuple[float, float]: + """ + Aggregate CPU and memory from all files in the top directory. + + Returns: + Tuple of (average_cpu_percent, average_memory_percent) + """ + top_files = [f for f in self._resource_file_path.iterdir() if f.is_file()] + + all_cpu: list[float] = [] + all_mem: list[float] = [] + + for top_file in top_files: + cpu_samples, mem_samples = self._parse_top_file(top_file) + all_cpu.extend(cpu_samples) + all_mem.extend(mem_samples) + + if not all_cpu: + log.warning("No valid CPU samples found in %s", self._resource_file_path) + return 0.0, 0.0 + + avg_cpu = sum(all_cpu) / len(all_cpu) + avg_mem = sum(all_mem) / len(all_mem) + log.debug("Parsed %d CPU samples from top, average: %.2f%%", len(all_cpu), avg_cpu) + + return avg_cpu, avg_mem + + def _parse_top_file(self, file_path: Path) -> tuple[list[float], list[float]]: + """ + Parse a single top batch-mode output file. + + top -b produces multiple snapshots concatenated in one file. Each + snapshot has a column-header line starting with "PID", followed by one + data row per thread. We locate the %CPU and %MEM columns from the + header and extract values from every subsequent data row until the next + snapshot boundary (a line beginning with 'top'). + + Args: + file_path: Path to a single *_osd_top.out file + + Returns: + Tuple of (cpu_sample_list, mem_sample_list) + """ + cpu_samples: list[float] = [] + mem_samples: list[float] = [] + + with open(file_path, encoding="utf-8") as f: + lines = f.readlines() + + in_process_block = False + cpu_col: int = -1 + mem_col: int = -1 + + for line in lines: + stripped = line.strip() + + # Column-header line anchors the column positions for this snapshot + if stripped.startswith("PID"): + headers = stripped.split() + try: + cpu_col = headers.index("%CPU") + mem_col = headers.index("%MEM") + except ValueError: + log.warning("Could not find %%CPU/%%MEM columns in header: %s", stripped) + in_process_block = False + continue + in_process_block = True + continue + + if not in_process_block: + continue + + # Non-process lines that mark a snapshot boundary or header section + if any(stripped.startswith(kw) for kw in ("top", "%Cpu", "Tasks", "MiB", "KiB", "Cpu")): + in_process_block = False + continue + + if not stripped: + continue + + parts = stripped.split() + if len(parts) <= max(cpu_col, mem_col): + continue + + try: + cpu_samples.append(float(parts[cpu_col])) + mem_samples.append(float(parts[mem_col])) + except ValueError: + log.debug("Skipping unparseable line in %s: %s", file_path.name, stripped) + + return cpu_samples, mem_samples diff --git a/post_processing/run_results/run_result.py b/post_processing/run_results/run_result.py index cabb4cc8..56ae63ba 100644 --- a/post_processing/run_results/run_result.py +++ b/post_processing/run_results/run_result.py @@ -19,13 +19,12 @@ ) from post_processing.run_results.benchmark_result import BenchmarkResult from post_processing.run_results.resource_result import ResourceResult - -# from post_processing.run_results.resources.resource_result import ResourceResult +from post_processing.run_results.resource_result_factory import get_all_resources log: Logger = getLogger("formatter") -class RunResult(ABC): +class RunResult(ABC): # pylint: disable=too-many-instance-attributes """ A result run file that needs processing """ @@ -39,6 +38,9 @@ def __init__(self, directory: Path, file_name_root: str, include_timeseries: boo self._processed_data: InternalFormattedOutputType = {} self._timeseries_data: dict[str, TimeSeriesFormatType] = {} self._timeseries_by_directory: dict[Path, dict[str, TimeSeriesFormatType]] = {} + # Tracks how many volume files have been merged per test configuration key + # (operation, blocksize, iodepth, numjobs) — used by _merge_resource_data. + self._resource_volume_counts: dict[tuple[str, str, str, str], int] = {} @abstractmethod def _find_files_for_testrun(self, file_name_root: str) -> list[Path]: @@ -218,7 +220,7 @@ def _build_test_result_data( test_config: tuple[str, str, str, str], io_details: IodepthDataType, global_details: dict[str, str], - resource_data: dict[str, str], + resource_data: dict[str, dict[str, str]], ) -> InternalNumJobsDataType: """ Build the complete nested data structure for a test result. @@ -227,7 +229,8 @@ def _build_test_result_data( test_config: Tuple of (operation, blocksize, iodepth, number_of_jobs) io_details: Merged IO performance details global_details: Global benchmark options - resource_data: Resource usage statistics + resource_data: Resource usage statistics with structure: + {"cpu": {"source1": "value1", ...}, "memory": {...}} Returns: Nested dictionary structure: {numjobs: {blocksize: {iodepth: data}}} @@ -235,10 +238,11 @@ def _build_test_result_data( _, blocksize, iodepth, number_of_jobs = test_config # Build from innermost to outermost level + # Merge all data including the nested resource data iodepth_data = {**global_details, **io_details, **resource_data} iodepth_details = {iodepth: iodepth_data} blocksize_details = cast(InternalBlocksizeDataType, {blocksize: iodepth_details}) - numjobs_details = cast(InternalNumJobsDataType, {number_of_jobs: blocksize_details}) + numjobs_details: InternalNumJobsDataType = {number_of_jobs: blocksize_details} return numjobs_details @@ -384,6 +388,117 @@ def _write_and_clear_timeseries_by_directory(self) -> None: self._timeseries_by_directory.clear() log.debug("Cleared timeseries data from memory") + # Sources where each volume file yields an independent CPU measurement that + # should be summed to get the total load across all volumes. All other + # sources (collectl, top) capture a shared system view from one directory + # that is read once per volume file, so they are averaged instead. + _SUMMED_RESOURCE_SOURCES: frozenset[str] = frozenset({"fio"}) + + def _collect_multi_source_resources(self, resources: list[ResourceResult]) -> dict[str, dict[str, str]]: + """ + Collect resource data from multiple sources into nested dict format. + + Args: + resources: List of ResourceResult instances + + Returns: + Dict with structure: {"cpu": {"source1": "value1", ...}, "memory": {...}} + """ + cpu_data: dict[str, str] = {} + memory_data: dict[str, str] = {} + + for resource in resources: + source = resource.source + resource_dict = resource.get() + + cpu_data[source] = resource_dict.get("cpu", "0.00") + memory_data[source] = resource_dict.get("memory", "0.00") + + return {"cpu": cpu_data, "memory": memory_data} + + def _aggregate_metric(self, source: str, prev: float, new: float, volume_count: int) -> str: + """ + Aggregate a single CPU or memory metric value for one source across volumes. + + Args: + source: Resource source identifier (e.g. "fio", "collectl", "top") + prev: Previously accumulated value + new: New value from the current volume + volume_count: Number of volumes already accumulated + + Returns: + Aggregated value as a formatted string + """ + if source in self._SUMMED_RESOURCE_SOURCES: + return f"{prev + new:.2f}" + # Shared-directory source: running_avg = (prev * n + new) / (n + 1) + return f"{(prev * volume_count + new) / (volume_count + 1):.2f}" + + def _get_existing_iodepth(self, test_config: tuple[str, str, str, str]) -> Optional[Union[str, IodepthDataType]]: + """ + Look up the already-stored iodepth entry for a test configuration. + + Args: + test_config: Tuple of (operation, blocksize, iodepth, number_of_jobs) + + Returns: + The stored iodepth data dict, a bare str, or None if not yet stored + """ + operation, blocksize, iodepth, number_of_jobs = test_config + return self._processed_data.get(operation, {}).get(number_of_jobs, {}).get(blocksize, {}).get(iodepth) + + def _merge_resource_data( + self, + test_config: tuple[str, str, str, str], + new_resource_data: dict[str, dict[str, str]], + ) -> dict[str, dict[str, str]]: + """ + Merge new resource data with any previously stored data for the same test + configuration, aggregating correctly across multiple volumes. + + Aggregation strategy per source: + - FIO: each volume file contains an independent per-job CPU measurement, + so values are **summed** across volumes. + - collectl / top: both point at a shared directory alongside the benchmark + files, so every volume file in the same directory reads the same data. + Values are **averaged** (divide running sum by volume count) so the + final figure is not artificially inflated. + + Args: + test_config: Tuple of (operation, blocksize, iodepth, number_of_jobs) + new_resource_data: Resource data from the current volume file + + Returns: + Merged resource data dict with the same structure as the input + """ + if test_config not in self._resource_volume_counts: + # First volume — nothing to merge yet + return new_resource_data + + existing_iodepth = self._get_existing_iodepth(test_config) + if not isinstance(existing_iodepth, dict): + return new_resource_data + + volume_count: int = self._resource_volume_counts[test_config] + raw_cpu = existing_iodepth.get("cpu", "") + raw_mem = existing_iodepth.get("memory", "") + existing_cpu: dict[str, str] = raw_cpu if isinstance(raw_cpu, dict) else {} + existing_mem: dict[str, str] = raw_mem if isinstance(raw_mem, dict) else {} + new_cpu_map = new_resource_data.get("cpu", {}) + new_mem_map = new_resource_data.get("memory", {}) + merged_cpu: dict[str, str] = {} + merged_mem: dict[str, str] = {} + + for source in set(existing_cpu) | set(new_cpu_map): + merged_cpu[source] = self._aggregate_metric( + source, float(existing_cpu.get(source, "0.00")), float(new_cpu_map.get(source, "0.00")), volume_count + ) + merged_mem[source] = self._aggregate_metric( + source, float(existing_mem.get(source, "0.00")), float(new_mem_map.get(source, "0.00")), volume_count + ) + + return {"cpu": merged_cpu, "memory": merged_mem} + def _convert_file(self, file_path: Path) -> None: """ Convert an individual benchmark result file to the common intermediate format. @@ -392,6 +507,11 @@ def _convert_file(self, file_path: Path) -> None: statistics, and stores them in the internal data structure organized by operation type, blocksize, and IO depth. + Now supports multiple resource sources (FIO, Collectl, etc.) simultaneously. + Resource values are aggregated across multiple volumes: sources that report + independent per-volume measurements (e.g. FIO) are summed; sources that + capture a shared system view (e.g. collectl, top) are averaged. + If include_timeseries is True, also extracts time-series data from log files. Args: @@ -402,21 +522,33 @@ def _convert_file(self, file_path: Path) -> None: KeyError: If required data fields are missing from results """ try: - # Use factory methods to get the correct classes + # Use factory method for benchmark result io: BenchmarkResult = self._create_benchmark_result(file_path) - resource: ResourceResult = self._create_resource_result(file_path) + + # Get ALL available resource sources using factory + resources: list[ResourceResult] = get_all_resources(file_path) test_config = self._extract_test_configuration(io) # Merge IO details with existing data if present io_details = self._merge_io_details(test_config, io.io_details) + # Collect resource data from all sources then merge with any + # previously accumulated data for the same test configuration + resource_data = self._collect_multi_source_resources(resources) + resource_data = self._merge_resource_data(test_config, resource_data) + # Build complete test result data structure - numjobs_details = self._build_test_result_data(test_config, io_details, io.global_options, resource.get()) + numjobs_details = self._build_test_result_data(test_config, io_details, io.global_options, resource_data) # Update internal processed data self._update_processed_data(test_config, numjobs_details) + # Increment the per-configuration volume counter so that subsequent + # volumes can compute a correct running average for shared-directory + # resource sources (collectl, top). + self._resource_volume_counts[test_config] = self._resource_volume_counts.get(test_config, 0) + 1 + # Process time-series data if requested if self._include_timeseries: self._process_timeseries_data(test_config, io) diff --git a/tests/test_collectl_resource.py b/tests/test_collectl_resource.py new file mode 100644 index 00000000..bc3f025e --- /dev/null +++ b/tests/test_collectl_resource.py @@ -0,0 +1,325 @@ +""" +Unit tests for CollectlResource class +""" + +# pyright: strict, reportPrivateUsage=false + +import tempfile +from pathlib import Path + +import pytest + +from post_processing.run_results.resources.collectl_resource import CollectlResource + + +class TestCollectlResourceInitialization: + """Test CollectlResource initialization""" + + def test_initialization_with_valid_setup(self) -> None: + """Test initialization with proper collectl directory structure""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test_file.json" + file_path.touch() + + # Create collectl directory with a CPU file + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;50\n") + + resource = CollectlResource(file_path) + + assert resource.source == "collectl" + # Accessing cpu property triggers parsing + assert resource.cpu == "50.00" + assert resource.memory == "0.00" + + def test_initialization_without_collectl_directory(self) -> None: + """Test initialization fails when collectl directory doesn't exist""" + with tempfile.TemporaryDirectory() as tmpdir: + file_path = Path(tmpdir) / "test_file.json" + file_path.touch() + + with pytest.raises(FileNotFoundError, match="Collectl directory not found"): + CollectlResource(file_path) + + def test_initialization_without_cpu_files(self) -> None: + """Test initialization fails when no CPU files exist""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + + with pytest.raises(FileNotFoundError, match="No .cpu files found"): + CollectlResource(file_path) + + +class TestCollectlResourceParsing: + """Test CPU data parsing with various formats""" + + def test_parse_with_totl_column(self) -> None: + """Test parsing CPU data with Totl% column""" + cpu_data = """#Date;Time;[CPU:0]User%;[CPU:0]Sys%;[CPU:0]Totl%;[CPU:1]User%;[CPU:1]Sys%;[CPU:1]Totl% +20260619;17:06:30;30;20;50;32;18;50 +20260619;17:06:40;28;22;50;30;20;50 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + + assert resource.cpu == "50.00" + assert resource.memory == "0.00" + + def test_parse_without_totl_column(self) -> None: + """Test parsing CPU data without Totl% (calculates from User+Sys)""" + cpu_data = """#Date;Time;[CPU:0]User%;[CPU:0]Sys%;[CPU:1]User%;[CPU:1]Sys% +20260619;17:06:30;30;20;32;18 +20260619;17:06:40;28;22;30;20 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + + # Sample 1: (30+20 + 32+18)/2 = 50 + # Sample 2: (28+22 + 30+20)/2 = 50 + assert resource.cpu == "50.00" + + def test_parse_with_comments_and_malformed_lines(self) -> None: + """Test parsing CPU data with comments and malformed lines""" + cpu_data = """# This is a comment +#Date;Time;[CPU:0]Totl% +# Another comment +20260619;17:06:30;50 +malformed line without enough fields +20260619;17:06:40;50 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + + # Should skip malformed lines and average the good ones + assert resource.cpu == "50.00" + + def test_parse_single_core(self) -> None: + """Test parsing CPU data with single core""" + cpu_data = """#Date;Time;[CPU:0]Totl% +20260619;17:06:30;50 +20260619;17:06:40;50 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + + # Accessing cpu property triggers parsing + assert resource.cpu == "50.00" + + def test_parse_multiple_cores(self) -> None: + """Test parsing CPU data with multiple cores""" + cpu_data = """#Date;Time;[CPU:0]Totl%;[CPU:1]Totl%;[CPU:2]Totl%;[CPU:3]Totl% +20260619;17:06:30;40;50;60;70 +20260619;17:06:40;50;60;70;80 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + + # Sample 1: (40+50+60+70)/4 = 55 + # Sample 2: (50+60+70+80)/4 = 65 + # Average: 60.0 + assert resource.cpu == "60.00" + + def test_parse_empty_file(self) -> None: + """Test parsing empty CPU file returns 0""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text("") + + resource = CollectlResource(file_path) + + assert resource.cpu == "0.00" + + def test_parse_no_data_rows(self) -> None: + """Test parsing CPU file with header but no data""" + cpu_data = """#Date;Time;[CPU:0]Totl% +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + + assert resource.cpu == "0.00" + + +class TestCollectlResourceGet: + """Test get() method""" + + def test_get_returns_correct_format(self) -> None: + """Test that get() returns correct dictionary format""" + cpu_data = """#Date;Time;[CPU:0]Totl% +20260619;17:06:30;45 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + result = resource.get() + + assert result == {"cpu": "45.00", "memory": "0.00", "source": "collectl"} + + def test_get_triggers_parsing(self) -> None: + """Test get() automatically triggers parsing""" + cpu_data = """#Date;Time;[CPU:0]Totl% +20260619;17:06:30;50 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + result = resource.get() + + # get() triggers parsing automatically + assert result == {"cpu": "50.00", "memory": "0.00", "source": "collectl"} + + +class TestCollectlResourceEdgeCases: + """Test edge cases and error conditions""" + + def test_zero_cpu_values(self) -> None: + """Test handling of zero CPU values""" + cpu_data = """#Date;Time;[CPU:0]Totl% +20260619;17:06:30;0 +20260619;17:06:40;0 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + + assert resource.cpu == "0.00" + + def test_decimal_cpu_values(self) -> None: + """Test handling of decimal CPU values""" + cpu_data = """#Date;Time;[CPU:0]Totl% +20260619;17:06:30;45.5 +20260619;17:06:40;54.5 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text(cpu_data) + + resource = CollectlResource(file_path) + + assert resource.cpu == "50.00" + + def test_multiple_cpu_files_uses_first(self) -> None: + """Test that first CPU file is used when multiple exist""" + cpu_data1 = """#Date;Time;[CPU:0]Totl% +20260619;17:06:30;30 +""" + cpu_data2 = """#Date;Time;[CPU:0]Totl% +20260619;17:06:30;70 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "test.json" + file_path.touch() + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + + # Create two CPU files + (collectl_dir / "host1-20260619.cpu").write_text(cpu_data1) + (collectl_dir / "host2-20260619.cpu").write_text(cpu_data2) + + resource = CollectlResource(file_path) + + # Should use one of them (order may vary, but should be valid) + cpu_value = float(resource.cpu) + assert cpu_value in [30.0, 70.0] + + +# Made with Bob diff --git a/tests/test_common_format_plotter.py b/tests/test_common_format_plotter.py index 99cd8835..d7624288 100644 --- a/tests/test_common_format_plotter.py +++ b/tests/test_common_format_plotter.py @@ -30,7 +30,6 @@ class ConcreteCommonFormatPlotter(CommonFormatPlotter): def draw_and_save(self) -> None: """Dummy implementation""" - pass def _generate_output_file_name(self, files: list) -> str: # type: ignore[type-arg] """Dummy implementation""" @@ -511,9 +510,7 @@ def test_extract_plot_data_invalid_cpu_value(self, mock_log: MagicMock) -> None: mock_log.warning.assert_called() # Check for either the specific invalid CPU warning or the general CPU data not found warning warning_message = str(mock_log.warning.call_args) - self.assertTrue( - "Invalid CPU value" in warning_message or "CPU data not found" in warning_message - ) + self.assertTrue("Invalid CPU value" in warning_message or "CPU data not found" in warning_message) # Should still process valid data self.assertEqual(len(result.x_data), 2) @@ -548,6 +545,116 @@ def test_extract_plot_data_missing_cpu_disables_resource_plotting(self, mock_log # Resource plotting should be disabled self.assertFalse(result.plot_resource_usage) + @patch("post_processing.plotter.common_format_plotter.log") + def test_extract_plot_data_multi_source_cpu(self, mock_log: MagicMock) -> None: + """Test _extract_plot_data handles multi-source CPU data (dict format)""" + mock_axes = MagicMock(spec=Axes) + mock_io_plotter = MagicMock(spec=IOPlotter) + mock_cpu_plotter = MagicMock(spec=CPUPlotter) + + sorted_plot_data = { # type: ignore[arg-type] + "1": { + "blocksize": "4096", + "bandwidth_bytes": "1000000", + "iops": "250", + "latency": "5000000", + "cpu": {"collectl": "39.75", "fio": "0.288828"}, # Multi-source CPU data + }, + "2": { + "blocksize": "4096", + "bandwidth_bytes": "2000000", + "iops": "500", + "latency": "4000000", + "cpu": {"collectl": "45.5", "fio": "0.5"}, + }, + } + + result = self.plotter._extract_plot_data( + sorted_plot_data, mock_axes, mock_io_plotter, mock_cpu_plotter, False, True + ) + + # Should successfully extract data from both entries + self.assertEqual(len(result.x_data), 2) + # Resource plotting should remain enabled + self.assertTrue(result.plot_resource_usage) + # CPU plotter should have received the dict for each data point + self.assertEqual(mock_cpu_plotter.add_y_data.call_count, 2) + # First call should receive the entire dict + first_call_arg = mock_cpu_plotter.add_y_data.call_args_list[0][0][0] + self.assertIsInstance(first_call_arg, dict) + self.assertEqual(first_call_arg, {"collectl": "39.75", "fio": "0.288828"}) + # Second call should receive the entire dict + second_call_arg = mock_cpu_plotter.add_y_data.call_args_list[1][0][0] + self.assertIsInstance(second_call_arg, dict) + self.assertEqual(second_call_arg, {"collectl": "45.5", "fio": "0.5"}) + + @patch("post_processing.plotter.common_format_plotter.log") + def test_extract_plot_data_multi_source_cpu_with_invalid_values(self, mock_log: MagicMock) -> None: + """Test _extract_plot_data handles multi-source CPU data with some invalid values""" + mock_axes = MagicMock(spec=Axes) + mock_io_plotter = MagicMock(spec=IOPlotter) + mock_cpu_plotter = MagicMock(spec=CPUPlotter) + + sorted_plot_data = { # type: ignore[arg-type] + "1": { + "blocksize": "4096", + "bandwidth_bytes": "1000000", + "iops": "250", + "latency": "5000000", + "cpu": {"collectl": "39.75", "fio": "invalid"}, # One invalid source + }, + } + + result = self.plotter._extract_plot_data( + sorted_plot_data, mock_axes, mock_io_plotter, mock_cpu_plotter, False, True + ) + + # Should log warning about invalid CPU value from specific source + mock_log.warning.assert_called() + warning_calls = [str(call) for call in mock_log.warning.call_args_list] + self.assertTrue(any("Invalid CPU value from source" in call for call in warning_calls)) + + # Should still pass the dict (with both valid and invalid values) to the plotter + # The plotter itself will handle filtering invalid values + self.assertEqual(len(result.x_data), 1) + self.assertTrue(result.plot_resource_usage) + mock_cpu_plotter.add_y_data.assert_called_once() + call_arg = mock_cpu_plotter.add_y_data.call_args[0][0] + self.assertIsInstance(call_arg, dict) + self.assertEqual(call_arg, {"collectl": "39.75", "fio": "invalid"}) + + @patch("post_processing.plotter.common_format_plotter.log") + def test_extract_plot_data_multi_source_cpu_all_invalid(self, mock_log: MagicMock) -> None: + """Test _extract_plot_data handles multi-source CPU data with all invalid values""" + mock_axes = MagicMock(spec=Axes) + mock_io_plotter = MagicMock(spec=IOPlotter) + mock_cpu_plotter = MagicMock(spec=CPUPlotter) + + sorted_plot_data = { # type: ignore[arg-type] + "1": { + "blocksize": "4096", + "bandwidth_bytes": "1000000", + "iops": "250", + "latency": "5000000", + "cpu": {"collectl": "invalid1", "fio": "invalid2"}, # All invalid + }, + } + + result = self.plotter._extract_plot_data( + sorted_plot_data, mock_axes, mock_io_plotter, mock_cpu_plotter, False, True + ) + + # Should log warnings about invalid CPU values + self.assertGreaterEqual(mock_log.warning.call_count, 2) + warning_calls = [str(call) for call in mock_log.warning.call_args_list] + # Should warn about no valid CPU values found + self.assertTrue(any("No valid CPU values found" in call for call in warning_calls)) + + # Should still process the data point but disable resource plotting + self.assertEqual(len(result.x_data), 1) + self.assertFalse(result.plot_resource_usage) + mock_cpu_plotter.add_y_data.assert_not_called() + class TestCommonFormatPlotterTitleGeneration(unittest.TestCase): """Test cases for title generation methods""" diff --git a/tests/test_common_output_formatter.py b/tests/test_common_output_formatter.py index 336448a0..5df675d4 100644 --- a/tests/test_common_output_formatter.py +++ b/tests/test_common_output_formatter.py @@ -106,8 +106,7 @@ def test_find_max_resource_usage(self) -> None: max_cpu, max_memory = self.formatter._find_max_resource_usage(test_data) self.assertEqual(max_cpu, "45.8") - # Note: max_memory is not currently implemented in the code - self.assertEqual(max_memory, "0") + self.assertEqual(max_memory, "2048.0") def test_find_max_resource_usage_with_empty_data(self) -> None: """Test finding maximum resource usage with empty data""" diff --git a/tests/test_cpu_plotter.py b/tests/test_cpu_plotter.py index 93602038..d396d631 100644 --- a/tests/test_cpu_plotter.py +++ b/tests/test_cpu_plotter.py @@ -13,8 +13,8 @@ from matplotlib.axes import Axes from post_processing.plotter.cpu_plotter import ( - CPU_PLOT_DEFAULT_COLOUR, CPU_PLOT_LABEL, + CPU_SOURCE_COLOURS, CPU_Y_LABEL, CPUPlotter, ) @@ -33,19 +33,32 @@ def setUp(self) -> None: def test_initialization(self) -> None: """Test CPUPlotter initialization""" self.assertEqual(self.plotter._main_axes, self.mock_axes) - self.assertEqual(self.plotter._y_data, []) + self.assertEqual(self.plotter._y_data_by_source, {}) - def test_add_y_data(self) -> None: - """Test adding CPU data""" + def test_add_y_data_legacy_format(self) -> None: + """Test adding CPU data in legacy string format""" self.plotter.add_y_data("45.5") self.plotter.add_y_data("67.8") - self.assertEqual(len(self.plotter._y_data), 2) - self.assertAlmostEqual(self.plotter._y_data[0], 45.5) - self.assertAlmostEqual(self.plotter._y_data[1], 67.8) - - def test_plot(self) -> None: - """Test plotting CPU data""" + self.assertIn("default", self.plotter._y_data_by_source) + self.assertEqual(len(self.plotter._y_data_by_source["default"]), 2) + self.assertAlmostEqual(self.plotter._y_data_by_source["default"][0], 45.5) + self.assertAlmostEqual(self.plotter._y_data_by_source["default"][1], 67.8) + + def test_add_y_data_multi_source_format(self) -> None: + """Test adding CPU data in multi-source dict format""" + self.plotter.add_y_data({"fio": "45.5", "collectl": "47.8"}) + self.plotter.add_y_data({"fio": "50.0", "collectl": "52.3"}) + + self.assertIn("fio", self.plotter._y_data_by_source) + self.assertIn("collectl", self.plotter._y_data_by_source) + self.assertEqual(len(self.plotter._y_data_by_source["fio"]), 2) + self.assertEqual(len(self.plotter._y_data_by_source["collectl"]), 2) + self.assertAlmostEqual(self.plotter._y_data_by_source["fio"][0], 45.5) + self.assertAlmostEqual(self.plotter._y_data_by_source["collectl"][0], 47.8) + + def test_plot_legacy_single_source(self) -> None: + """Test plotting CPU data with legacy single source""" self.plotter.add_y_data("50.0") self.plotter.add_y_data("60.0") @@ -55,19 +68,39 @@ def test_plot(self) -> None: # Should create twin axes self.mock_axes.twinx.assert_called_once() - # Should set label and y_label - self.assertEqual(self.plotter._label, CPU_PLOT_LABEL) - self.assertEqual(self.plotter._y_label, CPU_Y_LABEL) + # Should set y_label + self.mock_twin_axes.set_ylabel.assert_called_once_with(CPU_Y_LABEL) # Should call plot on twin axes - self.mock_twin_axes.set_ylabel.assert_called_once_with(CPU_Y_LABEL) self.mock_twin_axes.plot.assert_called_once() + def test_plot_multi_source(self) -> None: + """Test plotting CPU data with multiple sources""" + self.plotter.add_y_data({"fio": "50.0", "collectl": "52.0"}) + self.plotter.add_y_data({"fio": "60.0", "collectl": "62.0"}) + + x_data = [100.0, 200.0] + self.plotter.plot(x_data) + + # Should create twin axes + self.mock_axes.twinx.assert_called_once() + + # Should set y_label + self.mock_twin_axes.set_ylabel.assert_called_once_with(CPU_Y_LABEL) + + # Should call plot twice (once per source) + self.assertEqual(self.mock_twin_axes.plot.call_count, 2) + + # Should add legend for multiple sources + self.mock_twin_axes.legend.assert_called_once() + def test_cpu_constants(self) -> None: """Test CPU plotter constants""" - self.assertEqual(CPU_PLOT_DEFAULT_COLOUR, "xkcd:leaf green") self.assertEqual(CPU_Y_LABEL, "System CPU use (%)") self.assertEqual(CPU_PLOT_LABEL, "CPU use") + self.assertIn("fio", CPU_SOURCE_COLOURS) + self.assertIn("collectl", CPU_SOURCE_COLOURS) + self.assertIn("default", CPU_SOURCE_COLOURS) # Made with Bob diff --git a/tests/test_io_plotter.py b/tests/test_io_plotter.py index 289f5ca5..24eb835b 100644 --- a/tests/test_io_plotter.py +++ b/tests/test_io_plotter.py @@ -71,7 +71,8 @@ def test_plot_with_error_bars(self) -> None: # Verify error bars self.assertEqual(list(call_args[1]["yerr"]), error_data) self.assertEqual(call_args[1]["capsize"], cap_size) - self.assertEqual(call_args[1]["ecolor"], "red") + self.assertEqual(call_args[1]["color"], IO_PLOT_DEFAULT_COLOUR) + self.assertEqual(call_args[1]["ecolor"], "xkcd:red") def test_plot_with_error_bars_no_caps(self) -> None: """Test plotting with cap_size=0 (no error bar caps)""" @@ -87,7 +88,7 @@ def test_plot_with_error_bars_no_caps(self) -> None: def test_io_constants(self) -> None: """Test IO plotter constants""" - self.assertEqual(IO_PLOT_DEFAULT_COLOUR, "xkcd:leaf green") + self.assertEqual(IO_PLOT_DEFAULT_COLOUR, "xkcd:teal") self.assertEqual(IO_Y_LABEL, "Latency (ms)") self.assertEqual(IO_PLOT_LABEL, "IO Details") diff --git a/tests/test_memory_plotter.py b/tests/test_memory_plotter.py index 2dd71542..284c9a72 100644 --- a/tests/test_memory_plotter.py +++ b/tests/test_memory_plotter.py @@ -2,13 +2,16 @@ Unit tests for the MemoryPlotter class """ -from unittest.mock import MagicMock, patch +# pyright: strict, reportPrivateUsage=false +# +# We are OK to ignore private use in unit tests as the whole point of the tests +# is to validate the functions contained in the module -import pytest +from unittest.mock import MagicMock from post_processing.plotter.memory_plotter import ( - MEMORY_PLOT_DEFAULT_COLOUR, MEMORY_PLOT_LABEL, + MEMORY_SOURCE_COLOURS, MEMORY_Y_LABEL, MemoryPlotter, ) @@ -23,18 +26,18 @@ def test_initialization(self) -> None: plotter = MemoryPlotter(main_axis=mock_axes) assert plotter._main_axes == mock_axes - assert plotter._y_data == [] - assert plotter._label == "" - assert plotter._y_label == "" + assert not plotter._y_data_by_source def test_memory_constants(self) -> None: """Test that memory constants are defined correctly""" - assert MEMORY_PLOT_DEFAULT_COLOUR == "#4b006e" assert MEMORY_Y_LABEL == "Memory use (Mb)" assert MEMORY_PLOT_LABEL == "Memory use" + assert "fio" in MEMORY_SOURCE_COLOURS + assert "collectl" in MEMORY_SOURCE_COLOURS + assert "default" in MEMORY_SOURCE_COLOURS - def test_add_y_data(self) -> None: - """Test adding memory data points""" + def test_add_y_data_legacy_format(self) -> None: + """Test adding memory data points in legacy string format""" mock_axes = MagicMock() plotter = MemoryPlotter(main_axis=mock_axes) @@ -42,7 +45,21 @@ def test_add_y_data(self) -> None: plotter.add_y_data("200.75") plotter.add_y_data("150.25") - assert plotter._y_data == [100.5, 200.75, 150.25] + assert "default" in plotter._y_data_by_source + assert plotter._y_data_by_source["default"] == [100.5, 200.75, 150.25] + + def test_add_y_data_multi_source_format(self) -> None: + """Test adding memory data in multi-source dict format""" + mock_axes = MagicMock() + plotter = MemoryPlotter(main_axis=mock_axes) + + plotter.add_y_data({"fio": "100.5", "collectl": "105.0"}) + plotter.add_y_data({"fio": "200.75", "collectl": "210.0"}) + + assert "fio" in plotter._y_data_by_source + assert "collectl" in plotter._y_data_by_source + assert plotter._y_data_by_source["fio"] == [100.5, 200.75] + assert plotter._y_data_by_source["collectl"] == [105.0, 210.0] def test_add_y_data_converts_string_to_float(self) -> None: """Test that add_y_data converts string values to float""" @@ -50,11 +67,11 @@ def test_add_y_data_converts_string_to_float(self) -> None: plotter = MemoryPlotter(main_axis=mock_axes) plotter.add_y_data("42") - assert plotter._y_data == [42.0] - assert isinstance(plotter._y_data[0], float) + assert plotter._y_data_by_source["default"] == [42.0] + assert isinstance(plotter._y_data_by_source["default"][0], float) - def test_plot(self) -> None: - """Test plotting memory data""" + def test_plot_legacy_single_source(self) -> None: + """Test plotting memory data with legacy single source""" mock_main_axes = MagicMock() mock_memory_axes = MagicMock() mock_main_axes.twinx.return_value = mock_memory_axes @@ -69,15 +86,39 @@ def test_plot(self) -> None: # Verify twinx was called to create secondary axis mock_main_axes.twinx.assert_called_once() - # Verify labels were set - assert plotter._label == MEMORY_PLOT_LABEL - assert plotter._y_label == MEMORY_Y_LABEL + # Verify set_ylabel was called + mock_memory_axes.set_ylabel.assert_called_once_with(MEMORY_Y_LABEL) # Verify plot was called on the memory axis mock_memory_axes.plot.assert_called_once() + def test_plot_multi_source(self) -> None: + """Test plotting memory data with multiple sources""" + mock_main_axes = MagicMock() + mock_memory_axes = MagicMock() + mock_main_axes.twinx.return_value = mock_memory_axes + + plotter = MemoryPlotter(main_axis=mock_main_axes) + plotter.add_y_data({"fio": "100", "collectl": "105"}) + plotter.add_y_data({"fio": "200", "collectl": "210"}) + + x_data: list[float] = [1.0, 2.0] + plotter.plot(x_data=x_data) + + # Verify twinx was called to create secondary axis + mock_main_axes.twinx.assert_called_once() + + # Verify set_ylabel was called + mock_memory_axes.set_ylabel.assert_called_once_with(MEMORY_Y_LABEL) + + # Verify plot was called twice (once per source) + assert mock_memory_axes.plot.call_count == 2 + + # Verify legend was added for multiple sources + mock_memory_axes.legend.assert_called_once() + def test_plot_with_custom_colour_ignored(self) -> None: - """Test that custom colour parameter is ignored and default is used""" + """Test that custom colour parameter is ignored""" mock_main_axes = MagicMock() mock_memory_axes = MagicMock() mock_main_axes.twinx.return_value = mock_memory_axes @@ -89,9 +130,8 @@ def test_plot_with_custom_colour_ignored(self) -> None: # Pass a custom colour, but it should be ignored plotter.plot(x_data=x_data, colour="#FF0000") - # Verify the default colour is used - call_args = mock_memory_axes.plot.call_args - assert MEMORY_PLOT_DEFAULT_COLOUR in str(call_args) + # Verify plot was called (colour is determined internally) + mock_memory_axes.plot.assert_called_once() def test_plot_sets_y_label_on_axis(self) -> None: """Test that plot sets the y-axis label""" @@ -108,4 +148,5 @@ def test_plot_sets_y_label_on_axis(self) -> None: # Verify set_ylabel was called with correct label mock_memory_axes.set_ylabel.assert_called_once_with(MEMORY_Y_LABEL) + # Made with Bob diff --git a/tests/test_monitoring_base.py b/tests/test_monitoring_base.py new file mode 100644 index 00000000..cca439b6 --- /dev/null +++ b/tests/test_monitoring_base.py @@ -0,0 +1,73 @@ +"""Tests for the monitoring base class.""" + +# pylint: disable=protected-access + +from typing import ClassVar, Optional +from unittest.mock import patch + +import pytest + +from monitoring.monitoring import Monitoring + + +class MonitoringSubclass(Monitoring): + """Concrete monitoring subclass used for base class tests.""" + + DEFAULT_NODES: ClassVar[list[str]] = ["osds"] + + def start(self, directory: str) -> None: + pass + + def stop(self, directory: Optional[str]) -> None: + pass + + +class MissingDefaultNodesMonitoring(Monitoring): + """Concrete monitoring subclass without default nodes for error tests.""" + + def start(self, directory: str) -> None: + pass + + def stop(self, directory: Optional[str]) -> None: + pass + + +def test_init_uses_explicit_nodes() -> None: + """Use configured nodes when they are provided in monitoring config.""" + with patch("monitoring.monitoring.settings") as mock_settings: + mock_settings.getnodes.return_value = "node1,node2" + + monitor = MonitoringSubclass({"nodes": ["clients", "mons"]}) + + mock_settings.getnodes.assert_called_once_with("clients", "mons") + assert monitor._nodes == "node1,node2" + + +def test_init_falls_back_to_default_nodes() -> None: + """Use subclass default nodes when config does not provide nodes.""" + with patch("monitoring.monitoring.settings") as mock_settings: + mock_settings.getnodes.return_value = "osd-node" + + monitor = MonitoringSubclass({}) + + mock_settings.getnodes.assert_called_once_with("osds") + assert monitor._nodes == "osd-node" + + +def test_init_calls_settings_getnodes_with_resolved_nodes() -> None: + """Pass the resolved node groups to settings.getnodes.""" + with patch("monitoring.monitoring.settings") as mock_settings: + mock_settings.getnodes.return_value = "resolved-nodes" + + MonitoringSubclass({"nodes": ["rgws"]}) + + mock_settings.getnodes.assert_called_once_with("rgws") + + +def test_missing_default_nodes_raises_attribute_error() -> None: + """Raise an attribute error when a subclass omits default nodes.""" + with patch("monitoring.monitoring.settings") as mock_settings: + mock_settings.getnodes.return_value = "unused" + + with pytest.raises(AttributeError): + MissingDefaultNodesMonitoring({}) diff --git a/tests/test_monitoring_blktrace.py b/tests/test_monitoring_blktrace.py new file mode 100644 index 00000000..5a53c8d9 --- /dev/null +++ b/tests/test_monitoring_blktrace.py @@ -0,0 +1,160 @@ +"""Tests for the blktrace monitoring backend.""" + +# pylint: disable=protected-access + +from typing import Any, Optional +from unittest.mock import MagicMock, call, patch + +from monitoring.blktrace_monitoring import BlktraceMonitoring + + +def _make_monitor( + osds_per_node: int = 2, + use_existing: bool = True, + user: str = "ceph", + mconfig: Optional[dict[str, Any]] = None, +) -> BlktraceMonitoring: + """Construct a BlktraceMonitoring instance with mocked settings.""" + if mconfig is None: + mconfig = {} + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.blktrace_monitoring.settings") as mock_settings, + patch("monitoring.blktrace_monitoring.common.pdsh"), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "osds_per_node": osds_per_node, + "use_existing": use_existing, + "user": user, + }.get(key, default) + return BlktraceMonitoring(mconfig) + + +def test_default_nodes_is_osds() -> None: + """DEFAULT_NODES must be ['osds'].""" + assert BlktraceMonitoring.DEFAULT_NODES == ["osds"] + + +def test_init_stores_cluster_settings() -> None: + """__init__ reads osds_per_node, use_existing and user from settings.cluster.""" + monitor = _make_monitor(osds_per_node=4, use_existing=False, user="admin") + assert monitor._osds_per_node == 4 + assert monitor._use_existing is False + assert monitor._user == "admin" + + +def test_start_creates_directory_and_starts_traces() -> None: + """start() calls pdsh for mkdir and once per device.""" + mkdir_runner = MagicMock() + trace_runner = MagicMock() + + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.blktrace_monitoring.settings") as mock_settings, + patch("monitoring.blktrace_monitoring.common.pdsh") as mock_pdsh, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "osds_per_node": 2, + "use_existing": True, + "user": "ceph", + }.get(key, default) + mock_pdsh.side_effect = [mkdir_runner, trace_runner, trace_runner] + + monitor = BlktraceMonitoring({}) + monitor.start("/tmp/output") + + mock_pdsh.assert_any_call("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/blktrace") + mkdir_runner.communicate.assert_called_once_with() + + mock_pdsh.assert_any_call( + "resolved-nodes", + "cd /tmp/output/blktrace;sudo blktrace -o device0 -d /dev/disk/by-partlabel/osd-device-0-data", + ) + mock_pdsh.assert_any_call( + "resolved-nodes", + "cd /tmp/output/blktrace;sudo blktrace -o device1 -d /dev/disk/by-partlabel/osd-device-1-data", + ) + assert mock_pdsh.call_count == 3 # mkdir + 2 devices + + +def test_stop_issues_pkill() -> None: + """stop() always calls pdsh pkill blktrace.""" + pkill_runner = MagicMock() + monitor = _make_monitor() + + with ( + patch("monitoring.blktrace_monitoring.common.pdsh", return_value=pkill_runner) as mock_pdsh, + patch.object(monitor, "_make_movies") as mock_movies, + ): + monitor.stop(None) + + mock_pdsh.assert_called_once_with("resolved-nodes", "sudo pkill -SIGINT -f blktrace") + pkill_runner.communicate.assert_called_once_with() + mock_movies.assert_not_called() + + +def test_stop_calls_make_movies_when_not_use_existing() -> None: + """stop() calls _make_movies when use_existing is False and directory is provided.""" + pkill_runner = MagicMock() + monitor = _make_monitor(use_existing=False) + + with ( + patch("monitoring.blktrace_monitoring.common.pdsh", return_value=pkill_runner), + patch.object(monitor, "_make_movies") as mock_movies, + ): + monitor.stop("/tmp/output") + + mock_movies.assert_called_once_with("/tmp/output") + + +def test_stop_does_not_call_make_movies_when_use_existing() -> None: + """stop() skips _make_movies when use_existing is True.""" + pkill_runner = MagicMock() + monitor = _make_monitor(use_existing=True) + + with ( + patch("monitoring.blktrace_monitoring.common.pdsh", return_value=pkill_runner), + patch.object(monitor, "_make_movies") as mock_movies, + ): + monitor.stop("/tmp/output") + + mock_movies.assert_not_called() + + +def test_stop_does_not_call_make_movies_when_directory_is_none() -> None: + """stop() skips _make_movies when directory is None even if use_existing is False.""" + pkill_runner = MagicMock() + monitor = _make_monitor(use_existing=False) + + with ( + patch("monitoring.blktrace_monitoring.common.pdsh", return_value=pkill_runner), + patch.object(monitor, "_make_movies") as mock_movies, + ): + monitor.stop(None) + + mock_movies.assert_not_called() + + +def test_make_movies_issues_seekwatcher_per_device() -> None: + """_make_movies() calls pdsh with seekwatcher command for each device.""" + movie_runner = MagicMock() + monitor = _make_monitor(osds_per_node=2, user="ceph") + + with patch("monitoring.blktrace_monitoring.common.pdsh", return_value=movie_runner) as mock_pdsh: + monitor._make_movies("/tmp/output") + + expected_calls = [ + call( + "resolved-nodes", + "cd /tmp/output/blktrace;/home/ceph/bin/seekwatcher -t device0 -o device0.mpg --movie", + ), + call( + "resolved-nodes", + "cd /tmp/output/blktrace;/home/ceph/bin/seekwatcher -t device1 -o device1.mpg --movie", + ), + ] + mock_pdsh.assert_has_calls(expected_calls) + assert mock_pdsh.call_count == 2 + assert movie_runner.communicate.call_count == 2 diff --git a/tests/test_monitoring_collectl.py b/tests/test_monitoring_collectl.py new file mode 100644 index 00000000..e5aa1d81 --- /dev/null +++ b/tests/test_monitoring_collectl.py @@ -0,0 +1,69 @@ +"""Tests for the collectl monitoring backend.""" + +# pylint: disable=protected-access + +from unittest.mock import MagicMock, patch + +from monitoring.collectl_monitoring import CollectlMonitoring + + +def test_init_sets_default_args() -> None: + """Use the default collectl argument string when args are not configured.""" + with patch("monitoring.monitoring.settings") as mock_settings: + mock_settings.getnodes.return_value = "resolved-nodes" + + monitor = CollectlMonitoring({}) + + assert monitor._args == CollectlMonitoring.DEFAULT_ARGS + + +def test_init_uses_custom_args() -> None: + """Use custom collectl args from monitoring config when provided.""" + with patch("monitoring.monitoring.settings") as mock_settings: + mock_settings.getnodes.return_value = "resolved-nodes" + + monitor = CollectlMonitoring({"args": "--custom {collectl_dir}"}) + + assert monitor._args == "--custom {collectl_dir}" + + +def test_start_creates_directory_and_starts_collectl() -> None: + """Create the collectl directory and invoke collectl through pdsh.""" + mkdir_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_settings, + patch("monitoring.collectl_monitoring.common.pdsh") as mock_pdsh, + ): + mock_settings.getnodes.return_value = "resolved-nodes" + mock_pdsh.side_effect = [mkdir_runner, MagicMock()] + monitor = CollectlMonitoring({}) + + monitor.start("/tmp/output") + + mock_pdsh.assert_any_call("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/collectl") + mkdir_runner.communicate.assert_called_once_with() + mock_pdsh.assert_any_call( + "resolved-nodes", + ["collectl", monitor._args.format(collectl_dir="/tmp/output/collectl")], + ) + + +def test_stop_calls_pdsh_with_collectl_pkill() -> None: + """Stop collectl processes through pdsh.""" + stop_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_settings, + patch("monitoring.collectl_monitoring.common.pdsh", return_value=stop_runner) as mock_pdsh, + ): + mock_settings.getnodes.return_value = "resolved-nodes" + monitor = CollectlMonitoring({}) + + monitor.stop(None) + + mock_pdsh.assert_called_once_with("resolved-nodes", "pkill -SIGINT -f collectl") + stop_runner.communicate.assert_called_once_with() + + +def test_default_nodes_matches_collectl_configuration() -> None: + """Expose the expected default node groups for collectl monitoring.""" + assert CollectlMonitoring.DEFAULT_NODES == ["clients", "osds", "mons", "rgws"] diff --git a/tests/test_monitoring_factory.py b/tests/test_monitoring_factory.py new file mode 100644 index 00000000..4746bf86 --- /dev/null +++ b/tests/test_monitoring_factory.py @@ -0,0 +1,260 @@ +"""Tests for MonitoringFactory.""" + +# pylint: disable=protected-access + +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +from monitoring.blktrace_monitoring import BlktraceMonitoring +from monitoring.collectl_monitoring import CollectlMonitoring +from monitoring.monitoring_factory import MonitoringFactory +from monitoring.perf_monitoring import OsdPerfMonitoring, PerfMonitoring +from monitoring.top_monitoring import OsdTopMonitoring, TopMonitoring + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _patch_settings(profiles: dict[str, Any]) -> Any: + """Return a context manager that patches settings.monitoring_profiles.""" + return patch("monitoring.monitoring_factory.settings.monitoring_profiles", profiles) + + +def _stub_monitor() -> MagicMock: + """Return a MagicMock that behaves like a Monitoring instance.""" + m = MagicMock() + m.start = MagicMock() + m.stop = MagicMock() + return m + + +# --------------------------------------------------------------------------- +# get_object +# --------------------------------------------------------------------------- + + +def test_get_object_returns_collectl_monitoring() -> None: + """get_object('collectl') returns a CollectlMonitoring instance.""" + with patch("monitoring.monitoring.settings") as mock_base_settings: + mock_base_settings.getnodes.return_value = "node1" + instance = MonitoringFactory.get_object("collectl", {}) + assert isinstance(instance, CollectlMonitoring) + + +def test_get_object_returns_perf_monitoring() -> None: + """get_object('perf') returns a PerfMonitoring instance.""" + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "node1" + mock_settings.cluster.get.return_value = "dummy" + instance = MonitoringFactory.get_object("perf", {}) + assert isinstance(instance, PerfMonitoring) + + +def test_get_object_returns_blktrace_monitoring() -> None: + """get_object('blktrace') returns a BlktraceMonitoring instance.""" + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.blktrace_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "node1" + mock_settings.cluster.get.return_value = "dummy" + instance = MonitoringFactory.get_object("blktrace", {}) + assert isinstance(instance, BlktraceMonitoring) + + +def test_get_object_returns_top_monitoring() -> None: + """get_object('top') returns a TopMonitoring instance.""" + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "node1" + mock_settings.cluster.get.return_value = "dummy" + instance = MonitoringFactory.get_object("top", {}) + assert isinstance(instance, TopMonitoring) + + +def test_get_object_returns_osd_top_monitoring() -> None: + """get_object('osd_top') returns an OsdTopMonitoring instance.""" + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "node1" + mock_settings.cluster.get.return_value = "dummy" + instance = MonitoringFactory.get_object("osd_top", {}) + assert isinstance(instance, OsdTopMonitoring) + + +def test_get_object_returns_osd_perf_monitoring() -> None: + """get_object('osd_perf') returns an OsdPerfMonitoring instance.""" + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "node1" + mock_settings.cluster.get.return_value = "dummy" + instance = MonitoringFactory.get_object("osd_perf", {}) + assert isinstance(instance, OsdPerfMonitoring) + + +def test_get_object_raises_for_unknown_key() -> None: + """get_object() raises ValueError for an unrecognised backend name.""" + with pytest.raises(ValueError, match="Unknown monitoring backend: 'bogus'"): + MonitoringFactory.get_object("bogus", {}) + + +# --------------------------------------------------------------------------- +# get_all +# --------------------------------------------------------------------------- + + +def test_get_all_yields_one_instance_per_profile() -> None: + """get_all() yields one Monitoring instance for each configured profile.""" + profiles: dict[str, Any] = {"collectl": {}, "perf": {}} + collectl_inst = _stub_monitor() + perf_inst = _stub_monitor() + + with ( + _patch_settings(profiles), + patch.object(MonitoringFactory, "get_object") as mock_get_object, + ): + mock_get_object.side_effect = [collectl_inst, perf_inst] + result = list(MonitoringFactory.get_all()) + + assert result == [collectl_inst, perf_inst] + mock_get_object.assert_any_call("collectl", {}) + mock_get_object.assert_any_call("perf", {}) + + +def test_get_all_yields_nothing_when_no_profiles() -> None: + """get_all() yields nothing when monitoring_profiles is empty.""" + with _patch_settings({}): + result = list(MonitoringFactory.get_all()) + assert not result + + +# --------------------------------------------------------------------------- +# start +# --------------------------------------------------------------------------- + + +def test_start_calls_start_on_every_monitor() -> None: + """start() calls m.start(directory) for every monitor returned by get_all.""" + m1, m2 = _stub_monitor(), _stub_monitor() + with patch.object(MonitoringFactory, "get_all", return_value=iter([m1, m2])): + MonitoringFactory.start("/tmp/out") + + m1.start.assert_called_once_with("/tmp/out") + m2.start.assert_called_once_with("/tmp/out") + + +# --------------------------------------------------------------------------- +# stop +# --------------------------------------------------------------------------- + + +def test_stop_calls_stop_on_every_monitor() -> None: + """stop() calls m.stop(directory) for every monitor returned by get_all.""" + m1, m2 = _stub_monitor(), _stub_monitor() + with patch.object(MonitoringFactory, "get_all", return_value=iter([m1, m2])): + MonitoringFactory.stop("/tmp/out") + + m1.stop.assert_called_once_with("/tmp/out") + m2.stop.assert_called_once_with("/tmp/out") + + +def test_stop_passes_none_by_default() -> None: + """stop() passes None as directory when called with no argument.""" + m = _stub_monitor() + with patch.object(MonitoringFactory, "get_all", return_value=iter([m])): + MonitoringFactory.stop() + + m.stop.assert_called_once_with(None) + + +# --------------------------------------------------------------------------- +# monitor (context manager) +# --------------------------------------------------------------------------- + + +def test_monitor_starts_then_stops_all() -> None: + """monitor() starts all monitors before yield and stops all after.""" + m1, m2 = _stub_monitor(), _stub_monitor() + call_order: list[str] = [] + + m1.start.side_effect = lambda d: call_order.append("m1.start") + m2.start.side_effect = lambda d: call_order.append("m2.start") + m1.stop.side_effect = lambda d: call_order.append("m1.stop") + m2.stop.side_effect = lambda d: call_order.append("m2.stop") + + with (patch.object(MonitoringFactory, "get_all", return_value=iter([m1, m2])),): + with MonitoringFactory.monitor("/tmp/out"): + call_order.append("body") + + assert call_order == ["m1.start", "m2.start", "body", "m1.stop", "m2.stop"] + + +def test_monitor_stops_all_even_if_body_raises() -> None: + """monitor() runs stop for all monitors even when the body raises.""" + m = _stub_monitor() + with patch.object(MonitoringFactory, "get_all", return_value=iter([m])): + with pytest.raises(RuntimeError): + with MonitoringFactory.monitor("/tmp/out"): + raise RuntimeError("boom") + + m.stop.assert_called_once_with("/tmp/out") + + +# --------------------------------------------------------------------------- +# get_cpu_cycles +# --------------------------------------------------------------------------- + + +def test_get_cpu_cycles_delegates_to_perf_monitor() -> None: + """get_cpu_cycles() returns the value from the first PerfMonitoring instance.""" + perf_inst = MagicMock(spec=PerfMonitoring) + perf_inst.get_cpu_cycles.return_value = 42000 + + with patch.object(MonitoringFactory, "get_all", return_value=iter([perf_inst])): + result = MonitoringFactory.get_cpu_cycles("/tmp/out") + + assert result == 42000 + perf_inst.get_cpu_cycles.assert_called_once_with("/tmp/out") + + +def test_get_cpu_cycles_skips_non_perf_monitors() -> None: + """get_cpu_cycles() skips non-PerfMonitoring instances.""" + collectl_inst = MagicMock(spec=CollectlMonitoring) + perf_inst = MagicMock(spec=PerfMonitoring) + perf_inst.get_cpu_cycles.return_value = 99 + + with patch.object(MonitoringFactory, "get_all", return_value=iter([collectl_inst, perf_inst])): + result = MonitoringFactory.get_cpu_cycles("/tmp/out") + + assert result == 99 + assert not hasattr(collectl_inst, "get_cpu_cycles") or not collectl_inst.get_cpu_cycles.called + + +def test_get_cpu_cycles_returns_none_when_no_perf_configured() -> None: + """get_cpu_cycles() returns None when no PerfMonitoring is in the profiles.""" + collectl_inst = MagicMock(spec=CollectlMonitoring) + + with patch.object(MonitoringFactory, "get_all", return_value=iter([collectl_inst])): + result = MonitoringFactory.get_cpu_cycles("/tmp/out") + + assert result is None + + +def test_get_cpu_cycles_returns_none_when_profiles_empty() -> None: + """get_cpu_cycles() returns None when no profiles are configured at all.""" + with patch.object(MonitoringFactory, "get_all", return_value=iter([])): + result = MonitoringFactory.get_cpu_cycles("/tmp/out") + + assert result is None diff --git a/tests/test_monitoring_perf.py b/tests/test_monitoring_perf.py new file mode 100644 index 00000000..4df43d60 --- /dev/null +++ b/tests/test_monitoring_perf.py @@ -0,0 +1,288 @@ +"""Tests for the perf monitoring backend.""" + +# pylint: disable=protected-access + +from typing import Any, Optional +from unittest.mock import MagicMock, mock_open, patch + +from monitoring.perf_monitoring import OsdPerfMonitoring, PerfMonitoring + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_ARGS = "stat -p {pid} -o {perf_dir}/perf_stat.{pid}" + + +def _make_perf_monitor( + user: str = "ceph", + mconfig: Optional[dict[str, Any]] = None, +) -> PerfMonitoring: + """Construct a PerfMonitoring instance with mocked settings.""" + if mconfig is None: + mconfig = {"args": _ARGS} + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: {"user": user}.get(key, default) + return PerfMonitoring(mconfig) + + +def _make_osd_perf_monitor( + pid_dir: str = "/var/run/ceph", + user: str = "ceph", + mconfig: Optional[dict[str, Any]] = None, +) -> OsdPerfMonitoring: + """Construct an OsdPerfMonitoring instance with mocked settings.""" + if mconfig is None: + mconfig = {"args": _ARGS} + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "pid_dir": pid_dir, + "user": user, + }.get(key, default) + return OsdPerfMonitoring(mconfig) + + +# --------------------------------------------------------------------------- +# PerfMonitoring +# --------------------------------------------------------------------------- + + +def test_perf_default_nodes_is_osds() -> None: + """PerfMonitoring.DEFAULT_NODES must be ['osds'].""" + assert PerfMonitoring.DEFAULT_NODES == ["osds"] + + +def test_perf_init_stores_user_and_defaults() -> None: + """PerfMonitoring.__init__ reads user from settings and stores perf_cmd.""" + monitor = _make_perf_monitor(user="admin") + assert monitor._user == "admin" + assert monitor._perf_cmd == "sudo perf" + + +def test_perf_has_no_pid_dir_or_pid_glob() -> None: + """PerfMonitoring must not have _pid_dir or _pid_glob attributes.""" + monitor = _make_perf_monitor() + assert not hasattr(monitor, "_pid_dir") + assert not hasattr(monitor, "_pid_glob") + + +def test_perf_start_local_node_runs_perf() -> None: + """PerfMonitoring.start() runs a single perf command locally.""" + mkdir_runner = MagicMock() + local_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + patch("monitoring.perf_monitoring.common.pdsh", return_value=mkdir_runner) as mock_pdsh, + patch("monitoring.perf_monitoring.common.get_localnode", return_value="node1"), + patch("monitoring.perf_monitoring.common.sh", return_value=local_runner) as mock_sh, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: {"user": "ceph"}.get(key, default) + monitor = PerfMonitoring({"args": "stat -o {perf_dir}/perf_stat.out"}) + + monitor.start("/tmp/output") + + mock_pdsh.assert_called_once_with("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/perf") + mkdir_runner.communicate.assert_called_once_with() + mock_sh.assert_called_once_with("node1", "sudo perf stat -o /tmp/output/perf/perf_stat.out &") + assert monitor._perf_runners == [local_runner] + assert monitor._perf_dir == "/tmp/output/perf" + + +def test_perf_start_remote_node_uses_pdsh() -> None: + """PerfMonitoring.start() dispatches a single pdsh command for remote nodes.""" + mkdir_runner = MagicMock() + remote_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + patch("monitoring.perf_monitoring.common.pdsh") as mock_pdsh, + patch("monitoring.perf_monitoring.common.get_localnode", return_value=None), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: {"user": "ceph"}.get(key, default) + mock_pdsh.side_effect = [mkdir_runner, remote_runner] + PerfMonitoring({"args": "stat -o {perf_dir}/perf_stat.out"}).start("/tmp/output") + + mock_pdsh.assert_any_call("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/perf") + mock_pdsh.assert_any_call("resolved-nodes", "sudo perf stat -o /tmp/output/perf/perf_stat.out &") + + +def test_perf_stop_kills_local_runners() -> None: + """PerfMonitoring.stop() kills locally started runners when present.""" + runner = MagicMock() + monitor = _make_perf_monitor() + monitor._perf_runners = [runner] + + with patch("monitoring.perf_monitoring.common.pdsh"): + monitor.stop(None) + + runner.kill.assert_called_once_with() + + +def test_perf_stop_uses_pdsh_when_no_local_runners() -> None: + """PerfMonitoring.stop() issues pkill via pdsh when no local runners are tracked.""" + stop_runner = MagicMock() + monitor = _make_perf_monitor() + + with patch("monitoring.perf_monitoring.common.pdsh", return_value=stop_runner) as mock_pdsh: + monitor.stop(None) + + mock_pdsh.assert_called_once_with("resolved-nodes", r"sudo pkill -SIGINT -f perf\ ") + stop_runner.communicate.assert_called_once_with() + + +def test_perf_stop_chowns_output_files_when_directory_provided() -> None: + """PerfMonitoring.stop() adjusts ownership of generated perf files when a directory is given.""" + stop_runner = MagicMock() + chown_data_runner = MagicMock() + chown_stat_runner = MagicMock() + monitor = _make_perf_monitor() + + with patch("monitoring.perf_monitoring.common.pdsh") as mock_pdsh: + mock_pdsh.side_effect = [stop_runner, chown_data_runner, chown_stat_runner] + monitor.stop("/tmp/output") + + mock_pdsh.assert_any_call("resolved-nodes", r"sudo pkill -SIGINT -f perf\ ") + mock_pdsh.assert_any_call("resolved-nodes", "sudo chown ceph.ceph /tmp/output/perf/perf.data") + mock_pdsh.assert_any_call("resolved-nodes", "sudo chown ceph.ceph /tmp/output/perf/perf_stat.*") + + +def test_perf_get_cpu_cycles_returns_total_cycles() -> None: + """PerfMonitoring.get_cpu_cycles() sums cycle counts from all perf stat output files.""" + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + patch("monitoring.perf_monitoring.glob.glob", return_value=["/tmp/output/perf"]), + patch("monitoring.perf_monitoring.os.listdir", return_value=["perf_stat.1", "perf_stat.2"]), + patch( + "builtins.open", + side_effect=[ + mock_open(read_data="1,000 cycles user\n").return_value, + mock_open(read_data="2,500 cycles user\n").return_value, + ], + ), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: {"user": "ceph"}.get(key, default) + monitor = PerfMonitoring({"args": _ARGS}) + + assert monitor.get_cpu_cycles("/tmp/output") == 3500 + + +def test_perf_get_cpu_cycles_returns_none_when_cycles_missing() -> None: + """PerfMonitoring.get_cpu_cycles() returns None when perf output has no cycles line.""" + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + patch("monitoring.perf_monitoring.glob.glob", return_value=["/tmp/output/perf"]), + patch("monitoring.perf_monitoring.os.listdir", return_value=["perf_stat.1"]), + patch("builtins.open", mock_open(read_data="nothing to match\n")), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: {"user": "ceph"}.get(key, default) + monitor = PerfMonitoring({"args": _ARGS}) + + assert monitor.get_cpu_cycles("/tmp/output") is None + + +# --------------------------------------------------------------------------- +# OsdPerfMonitoring +# --------------------------------------------------------------------------- + + +def test_osd_perf_default_nodes_is_osds() -> None: + """OsdPerfMonitoring.DEFAULT_NODES must be ['osds'] (inherited).""" + assert OsdPerfMonitoring.DEFAULT_NODES == ["osds"] + + +def test_osd_perf_is_subclass_of_perf_monitoring() -> None: + """OsdPerfMonitoring must be a subclass of PerfMonitoring.""" + assert issubclass(OsdPerfMonitoring, PerfMonitoring) + + +def test_osd_perf_init_stores_pid_dir_and_glob() -> None: + """OsdPerfMonitoring.__init__ reads pid_dir from cluster settings and sets pid_glob.""" + monitor = _make_osd_perf_monitor(pid_dir="/run/ceph") + assert monitor._pid_dir == "/run/ceph" + assert monitor._pid_glob == "osd.*.pid" + + +def test_osd_perf_init_accepts_custom_pid_glob() -> None: + """OsdPerfMonitoring accepts a custom pid_glob from mconfig.""" + monitor = _make_osd_perf_monitor(mconfig={"args": _ARGS, "pid_glob": "*.pid"}) + assert monitor._pid_glob == "*.pid" + + +def test_osd_perf_start_local_node_runs_perf_per_pid() -> None: + """OsdPerfMonitoring.start() runs perf locally for each matching pid file.""" + mkdir_runner = MagicMock() + local_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + patch("monitoring.perf_monitoring.common.pdsh", return_value=mkdir_runner) as mock_pdsh, + patch("monitoring.perf_monitoring.common.get_localnode", return_value="node1"), + patch("monitoring.perf_monitoring.common.sh", return_value=local_runner) as mock_sh, + patch("monitoring.perf_monitoring.glob.glob", return_value=["/var/run/ceph/osd.1.pid"]), + patch("builtins.open", mock_open(read_data="123\n")), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "pid_dir": "/var/run/ceph", + "user": "ceph", + }.get(key, default) + monitor = OsdPerfMonitoring({"args": _ARGS}) + + monitor.start("/tmp/output") + + mock_pdsh.assert_called_once_with("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/perf") + mkdir_runner.communicate.assert_called_once_with() + mock_sh.assert_called_once_with("node1", "sudo perf stat -p 123 -o /tmp/output/perf/perf_stat.123 &") + assert monitor._perf_runners == [local_runner] + assert monitor._perf_dir == "/tmp/output/perf" + + +def test_osd_perf_start_remote_node_uses_pdsh_loop() -> None: + """OsdPerfMonitoring.start() dispatches via pdsh for-loop when no local node is available.""" + mkdir_runner = MagicMock() + remote_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.perf_monitoring.settings") as mock_settings, + patch("monitoring.perf_monitoring.common.pdsh") as mock_pdsh, + patch("monitoring.perf_monitoring.common.get_localnode", return_value=None), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "pid_dir": "/var/run/ceph", + "user": "ceph", + }.get(key, default) + mock_pdsh.side_effect = [mkdir_runner, remote_runner] + OsdPerfMonitoring({"args": _ARGS}).start("/tmp/output") + + mock_pdsh.assert_any_call("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/perf") + mock_pdsh.assert_any_call( + "resolved-nodes", + [ + "for pid in `cat /var/run/ceph/osd.*.pid`;", + "do", + "sudo perf stat -p ${pid} -o /tmp/output/perf/perf_stat.${pid} &", + ";", + "done", + ], + ) + + +def test_osd_perf_stop_inherited_from_perf_monitoring() -> None: + """OsdPerfMonitoring inherits stop() from PerfMonitoring without override.""" + assert OsdPerfMonitoring.stop is PerfMonitoring.stop diff --git a/tests/test_monitoring_top.py b/tests/test_monitoring_top.py new file mode 100644 index 00000000..f6e198b9 --- /dev/null +++ b/tests/test_monitoring_top.py @@ -0,0 +1,315 @@ +"""Tests for the top monitoring backend.""" + +# pylint: disable=protected-access + +from typing import Any, Optional +from unittest.mock import MagicMock, mock_open, patch + +from monitoring.top_monitoring import OsdTopMonitoring, TopMonitoring + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_top_monitor( + user: str = "ceph", + mconfig: Optional[dict[str, Any]] = None, +) -> TopMonitoring: + """Construct a TopMonitoring instance with mocked settings.""" + if mconfig is None: + mconfig = {} + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: {"user": user}.get(key, default) + return TopMonitoring(mconfig) + + +def _make_osd_top_monitor( + pid_dir: str = "/var/run/ceph", + user: str = "ceph", + mconfig: Optional[dict[str, Any]] = None, +) -> OsdTopMonitoring: + """Construct an OsdTopMonitoring instance with mocked settings.""" + if mconfig is None: + mconfig = {} + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "pid_dir": pid_dir, + "user": user, + }.get(key, default) + return OsdTopMonitoring(mconfig) + + +# --------------------------------------------------------------------------- +# TopMonitoring +# --------------------------------------------------------------------------- + + +def test_top_default_nodes_is_osds() -> None: + """TopMonitoring.DEFAULT_NODES must be ['osds'].""" + assert TopMonitoring.DEFAULT_NODES == ["osds"] + + +def test_top_init_stores_user_and_defaults() -> None: + """TopMonitoring.__init__ reads user from settings and applies default args.""" + monitor = _make_top_monitor(user="admin") + assert monitor._user == "admin" + assert monitor._top_cmd == "top" + assert "{top_dir}" in monitor._args + assert "{pid}" not in monitor._args + + +def test_top_init_accepts_custom_args() -> None: + """TopMonitoring accepts custom top_cmd and args from mconfig.""" + monitor = _make_top_monitor(mconfig={"top_cmd": "htop", "args": "-d 1 > {top_dir}/out.txt"}) + assert monitor._top_cmd == "htop" + assert monitor._args == "-d 1 > {top_dir}/out.txt" + + +def test_top_has_no_pid_dir_or_pid_glob() -> None: + """TopMonitoring must not have _pid_dir or _pid_glob attributes.""" + monitor = _make_top_monitor() + assert not hasattr(monitor, "_pid_dir") + assert not hasattr(monitor, "_pid_glob") + + +def test_top_start_local_node_runs_top() -> None: + """TopMonitoring.start() runs top locally without per-pid iteration.""" + mkdir_runner = MagicMock() + local_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + patch("monitoring.top_monitoring.common.pdsh", return_value=mkdir_runner) as mock_pdsh, + patch("monitoring.top_monitoring.common.get_localnode", return_value="node1"), + patch("monitoring.top_monitoring.common.sh", return_value=local_runner) as mock_sh, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: {"user": "ceph"}.get(key, default) + monitor = TopMonitoring({}) + + monitor.start("/tmp/output") + + mock_pdsh.assert_called_once_with("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/top") + mkdir_runner.communicate.assert_called_once_with() + expected_cmd = "top -b -H -1 -n 30 > /tmp/output/top/top.out" + mock_sh.assert_called_once_with("node1", expected_cmd) + assert monitor._top_runners == [local_runner] + + +def test_top_start_remote_node_uses_pdsh() -> None: + """TopMonitoring.start() dispatches a single pdsh command for remote nodes.""" + mkdir_runner = MagicMock() + remote_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + patch("monitoring.top_monitoring.common.pdsh") as mock_pdsh, + patch("monitoring.top_monitoring.common.get_localnode", return_value=None), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: {"user": "ceph"}.get(key, default) + mock_pdsh.side_effect = [mkdir_runner, remote_runner] + monitor = TopMonitoring({}) + + monitor.start("/tmp/output") + + mock_pdsh.assert_any_call("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/top") + mock_pdsh.assert_any_call("resolved-nodes", "top -b -H -1 -n 30 > /tmp/output/top/top.out") + + +def test_top_stop_kills_local_runners() -> None: + """TopMonitoring.stop() kills locally started runners when present.""" + runner = MagicMock() + monitor = _make_top_monitor() + monitor._top_runners = [runner] + + with patch("monitoring.top_monitoring.common.pdsh") as mock_pdsh: + monitor.stop(None) + + runner.kill.assert_called_once_with() + mock_pdsh.assert_not_called() + + +def test_top_stop_uses_pdsh_when_no_local_runners() -> None: + """TopMonitoring.stop() issues pkill via pdsh when no local runners are tracked.""" + stop_runner = MagicMock() + monitor = _make_top_monitor() + expected_pkill = f"sudo pkill -SIGINT -f '{monitor._top_cmd} {monitor._args}'" + + with patch("monitoring.top_monitoring.common.pdsh", return_value=stop_runner) as mock_pdsh: + monitor.stop(None) + + mock_pdsh.assert_called_once_with("resolved-nodes", expected_pkill) + stop_runner.communicate.assert_called_once_with() + + +def test_top_stop_chowns_output_files_when_directory_provided() -> None: + """TopMonitoring.stop() adjusts ownership of top output files when a directory is given.""" + stop_runner = MagicMock() + chown_runner = MagicMock() + monitor = _make_top_monitor(user="ceph") + + with patch("monitoring.top_monitoring.common.pdsh") as mock_pdsh: + mock_pdsh.side_effect = [stop_runner, chown_runner] + monitor.stop("/tmp/output") + + mock_pdsh.assert_any_call("resolved-nodes", "sudo chown ceph.ceph /tmp/output/top/*top.out") + + +# --------------------------------------------------------------------------- +# OsdTopMonitoring +# --------------------------------------------------------------------------- + + +def test_osd_top_default_nodes_is_osds() -> None: + """OsdTopMonitoring.DEFAULT_NODES must be ['osds'] (inherited).""" + assert OsdTopMonitoring.DEFAULT_NODES == ["osds"] + + +def test_osd_top_is_subclass_of_top_monitoring() -> None: + """OsdTopMonitoring must be a subclass of TopMonitoring.""" + assert issubclass(OsdTopMonitoring, TopMonitoring) + + +def test_osd_top_init_stores_pid_dir_and_glob() -> None: + """OsdTopMonitoring.__init__ reads pid_dir from cluster settings and sets pid_glob.""" + monitor = _make_osd_top_monitor(pid_dir="/run/ceph") + assert monitor._pid_dir == "/run/ceph" + assert monitor._pid_glob == "osd.*.pid" + + +def test_osd_top_init_args_include_pid_placeholder() -> None: + """OsdTopMonitoring default args template must contain {pid}.""" + monitor = _make_osd_top_monitor() + assert "{pid}" in monitor._args + assert "{top_dir}" in monitor._args + + +def test_osd_top_init_accepts_custom_pid_glob() -> None: + """OsdTopMonitoring accepts a custom pid_glob from mconfig.""" + monitor = _make_osd_top_monitor(mconfig={"pid_glob": "*.pid"}) + assert monitor._pid_glob == "*.pid" + + +def test_osd_top_start_local_node_runs_top_per_pid() -> None: + """OsdTopMonitoring.start() runs top locally for each matching pid file.""" + mkdir_runner = MagicMock() + local_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + patch("monitoring.top_monitoring.common.pdsh", return_value=mkdir_runner) as mock_pdsh, + patch("monitoring.top_monitoring.common.get_localnode", return_value="node1"), + patch("monitoring.top_monitoring.common.sh", return_value=local_runner) as mock_sh, + patch("monitoring.top_monitoring.glob.glob", return_value=["/var/run/ceph/osd.1.pid"]), + patch("builtins.open", mock_open(read_data="42\n")), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "pid_dir": "/var/run/ceph", + "user": "ceph", + }.get(key, default) + monitor = OsdTopMonitoring({}) + + monitor.start("/tmp/output") + + mock_pdsh.assert_called_once_with("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/top") + mkdir_runner.communicate.assert_called_once_with() + expected_cmd = "top -b -H -1 -p 42 -n 30 > /tmp/output/top/42_osd_top.out" + mock_sh.assert_called_once_with("node1", expected_cmd) + assert monitor._top_runners == [local_runner] + + +def test_osd_top_start_local_node_warns_when_no_pid_files() -> None: + """OsdTopMonitoring.start() logs a warning when no local PID files are found.""" + mkdir_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + patch("monitoring.top_monitoring.common.pdsh", return_value=mkdir_runner), + patch("monitoring.top_monitoring.common.get_localnode", return_value="node1"), + patch("monitoring.top_monitoring.glob.glob", return_value=[]), + patch("monitoring.top_monitoring.logger") as mock_logger, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "pid_dir": "/var/run/ceph", + "user": "ceph", + }.get(key, default) + OsdTopMonitoring({}).start("/tmp/output") + + warning_calls = [str(c) for c in mock_logger.warning.call_args_list] + assert any("no PID files" in c for c in warning_calls) + + +def test_osd_top_start_remote_node_uses_pdsh_loop() -> None: + """OsdTopMonitoring.start() dispatches via pdsh for-loop when no local node is available.""" + mkdir_runner = MagicMock() + ls_runner = MagicMock() + ls_runner.communicate.return_value = ("/var/run/ceph/osd.1.pid\n", "") + remote_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + patch("monitoring.top_monitoring.common.pdsh") as mock_pdsh, + patch("monitoring.top_monitoring.common.get_localnode", return_value=None), + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "pid_dir": "/var/run/ceph", + "user": "ceph", + }.get(key, default) + mock_pdsh.side_effect = [mkdir_runner, ls_runner, remote_runner] + OsdTopMonitoring({}).start("/tmp/output") + + mock_pdsh.assert_any_call("resolved-nodes", "mkdir -p -m0755 -- /tmp/output/top") + mock_pdsh.assert_any_call("resolved-nodes", "ls /var/run/ceph/osd.*.pid 2>/dev/null") + mock_pdsh.assert_any_call( + "resolved-nodes", + [ + "for pid in `cat /var/run/ceph/osd.*.pid`;", + "do", + "top -b -H -1 -p ${pid} -n 30 > /tmp/output/top/${pid}_osd_top.out", + ";", + "done", + ], + ) + + +def test_osd_top_start_remote_node_warns_when_no_pid_files() -> None: + """OsdTopMonitoring.start() logs a warning when the remote ls finds no PID files.""" + mkdir_runner = MagicMock() + ls_runner = MagicMock() + ls_runner.communicate.return_value = ("", "") + remote_runner = MagicMock() + with ( + patch("monitoring.monitoring.settings") as mock_base_settings, + patch("monitoring.top_monitoring.settings") as mock_settings, + patch("monitoring.top_monitoring.common.pdsh") as mock_pdsh, + patch("monitoring.top_monitoring.common.get_localnode", return_value=None), + patch("monitoring.top_monitoring.logger") as mock_logger, + ): + mock_base_settings.getnodes.return_value = "resolved-nodes" + mock_settings.cluster.get.side_effect = lambda key, default=None: { + "pid_dir": "/var/run/ceph", + "user": "ceph", + }.get(key, default) + mock_pdsh.side_effect = [mkdir_runner, ls_runner, remote_runner] + OsdTopMonitoring({}).start("/tmp/output") + + warning_calls = [str(c) for c in mock_logger.warning.call_args_list] + assert any("no PID files" in c for c in warning_calls) + + +def test_osd_top_stop_inherited_from_top_monitoring() -> None: + """OsdTopMonitoring inherits stop() from TopMonitoring without override.""" + assert OsdTopMonitoring.stop is TopMonitoring.stop diff --git a/tests/test_resource_result_factory.py b/tests/test_resource_result_factory.py new file mode 100644 index 00000000..706d5579 --- /dev/null +++ b/tests/test_resource_result_factory.py @@ -0,0 +1,475 @@ +""" +Unit tests for resource_result_factory module +""" + +# pyright: strict, reportPrivateUsage=false + +import json +import tempfile +from pathlib import Path + +from post_processing.run_results.resource_result_factory import get_all_resources +from post_processing.run_results.resources.collectl_resource import CollectlResource +from post_processing.run_results.resources.fio_resource import FIOResource +from post_processing.run_results.resources.top_resource import TopResource + + +class TestGetAllResourcesFIOOnly: + """Test get_all_resources with only FIO data available""" + + def test_fio_only_returns_single_resource(self) -> None: + """Test that only FIO resource is returned when collectl dir missing""" + fio_data = { + "jobs": [ + { + "job_name": "test", + "usr_cpu": 45.5, + "sys_cpu": 10.2, + "ctx": 1000, + "majf": 0, + "minf": 100, + } + ] + } + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + resources = get_all_resources(file_path) + + assert len(resources) == 1 + assert isinstance(resources[0], FIOResource) + assert resources[0].source == "fio" + + def test_fio_only_with_empty_directory(self) -> None: + """Test FIO-only scenario with empty parent directory""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 30.0, "sys_cpu": 20.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + resources = get_all_resources(file_path) + + assert len(resources) == 1 + assert resources[0].source == "fio" + + +class TestGetAllResourcesWithInvalidData: + """Test get_all_resources with invalid data (both sources still created)""" + + def test_both_created_even_with_invalid_fio_json(self) -> None: + """Test that both resources are created even if FIO JSON is invalid""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text("invalid json data") + + # Create collectl directory with CPU file + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;50\n") + + resources = get_all_resources(file_path) + + # Both resources are created (FIO handles invalid JSON gracefully) + assert len(resources) == 2 + sources = {r.source for r in resources} + assert sources == {"fio", "collectl"} + + def test_both_created_with_empty_fio_file(self) -> None: + """Test both resources created when FIO file is empty""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text("") + + # Create collectl directory + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;45\n") + + resources = get_all_resources(file_path) + + # Both created (empty file is handled gracefully) + assert len(resources) == 2 + sources = {r.source for r in resources} + assert sources == {"fio", "collectl"} + + +class TestGetAllResourcesBothSources: + """Test get_all_resources with both FIO and Collectl available""" + + def test_both_sources_returns_two_resources(self) -> None: + """Test that both FIO and Collectl resources are returned""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 30.0, "sys_cpu": 15.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + # Create collectl directory + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;50\n") + + resources = get_all_resources(file_path) + + assert len(resources) == 2 + + # Check that we have both sources + sources = {r.source for r in resources} + assert sources == {"fio", "collectl"} + + # Verify types + fio_resources = [r for r in resources if isinstance(r, FIOResource)] + collectl_resources = [r for r in resources if isinstance(r, CollectlResource)] + + assert len(fio_resources) == 1 + assert len(collectl_resources) == 1 + + def test_both_sources_order(self) -> None: + """Test that FIO is returned before Collectl (order matters for consistency)""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 25.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;60\n") + + resources = get_all_resources(file_path) + + # FIO should be first + assert resources[0].source == "fio" + assert resources[1].source == "collectl" + + +class TestGetAllResourcesMinimalScenarios: + """Test get_all_resources in minimal scenarios""" + + def test_fio_always_created_if_file_exists(self) -> None: + """Test that FIO resource is always created if file exists""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text("invalid json") + + # No collectl directory + resources = get_all_resources(file_path) + + # FIO is always created (handles invalid data gracefully) + assert len(resources) == 1 + assert resources[0].source == "fio" + + def test_fio_only_with_empty_collectl_dir(self) -> None: + """Test FIO-only when collectl dir exists but has no CPU files""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text("not valid json") + + # Create empty collectl directory (no .cpu files) + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + + resources = get_all_resources(file_path) + + # Only FIO (collectl has no CPU files) + assert len(resources) == 1 + assert resources[0].source == "fio" + + +class TestGetAllResourcesErrorHandling: + """Test error handling in get_all_resources""" + + def test_handles_collectl_exception_gracefully(self) -> None: + """Test that Collectl exceptions don't prevent FIO from being added""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 40.0, "sys_cpu": 20.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + # Create collectl dir but with no CPU files (will raise exception) + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + # Don't create any .cpu files + + resources = get_all_resources(file_path) + + # Should still get FIO even though Collectl failed + assert len(resources) == 1 + assert resources[0].source == "fio" + + def test_handles_missing_fio_file(self) -> None: + """Test that missing FIO file is handled gracefully""" + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "nonexistent.json" + # Don't create the file + + # Create valid collectl data + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;55\n") + + resources = get_all_resources(file_path) + + # Both are created (FIO handles missing file gracefully) + assert len(resources) == 2 + sources = {r.source for r in resources} + assert sources == {"fio", "collectl"} + + +class TestGetAllResourcesCollectlDirectoryChecks: + """Test collectl directory existence checks""" + + def test_collectl_dir_is_file_not_directory(self) -> None: + """Test that collectl is skipped if it's a file, not a directory""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 35.0, "sys_cpu": 15.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + # Create 'collectl' as a file, not directory + collectl_file = base_path / "collectl" + collectl_file.write_text("this is a file") + + resources = get_all_resources(file_path) + + # Should only get FIO (collectl is not a directory) + assert len(resources) == 1 + assert resources[0].source == "fio" + + def test_collectl_dir_does_not_exist(self) -> None: + """Test that missing collectl directory is handled correctly""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 50.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + # Don't create collectl directory + resources = get_all_resources(file_path) + + assert len(resources) == 1 + assert resources[0].source == "fio" + + +class TestGetAllResourcesIntegration: + """Integration tests for get_all_resources""" + + def test_realistic_scenario_both_sources(self) -> None: + """Test realistic scenario with both FIO and Collectl data""" + fio_data = { + "jobs": [ + { + "job_name": "seq_read", + "usr_cpu": 25.5, + "sys_cpu": 15.3, + "ctx": 5000, + "majf": 0, + "minf": 250, + } + ] + } + + cpu_data = """#Date;Time;[CPU:0]User%;[CPU:0]Sys%;[CPU:0]Totl%;[CPU:1]User%;[CPU:1]Sys%;[CPU:1]Totl% +20260619;17:06:30;30;20;50;32;18;50 +20260619;17:06:40;28;22;50;30;20;50 +20260619;17:06:50;29;21;50;31;19;50 +""" + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "cephalasquad3-20260619.cpu" + cpu_file.write_text(cpu_data) + + resources = get_all_resources(file_path) + + assert len(resources) == 2 + + # Verify both resources can be used + for resource in resources: + result = resource.get() + assert "cpu" in result + assert "memory" in result + assert "source" in result + assert result["source"] in ["fio", "collectl"] + + def test_can_retrieve_data_from_all_resources(self) -> None: + """Test that data can be retrieved from all returned resources""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 20.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base_path = Path(tmpdir) + file_path = base_path / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + collectl_dir = base_path / "collectl" + collectl_dir.mkdir() + cpu_file = collectl_dir / "host-20260619.cpu" + cpu_file.write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;45\n") + + resources = get_all_resources(file_path) + + # Get data from all resources + results = [r.get() for r in resources] + + assert len(results) == 2 + assert all("cpu" in r for r in results) + assert all("memory" in r for r in results) + assert all("source" in r for r in results) + + # Verify sources are different + sources = [r["source"] for r in results] + assert set(sources) == {"fio", "collectl"} + + +class TestGetAllResourcesWithTop: + """Test get_all_resources when top monitoring data is present.""" + + def _make_top_dir(self, base: Path, content: str) -> None: + top_dir = base / "top" + top_dir.mkdir() + (top_dir / "12345_osd_top.out").write_text(content) + + _TOP_CONTENT = ( + "top - 14:32:01 up 1 day\n%Cpu(s): 5.0 us\n\n" + " PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND\n" + "12345 ceph 20 0 1.0g 100m 10m S 30.0 0.5 0:01.00 crimson-osd\n" + ) + + def test_top_only_returns_fio_and_top(self) -> None: + """FIO and Top resources are returned when only a top dir is present.""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 20.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + self._make_top_dir(base, self._TOP_CONTENT) + + resources = get_all_resources(file_path) + + assert len(resources) == 2 + sources = {r.source for r in resources} + assert sources == {"fio", "top"} + + def test_top_resource_is_top_resource_instance(self) -> None: + """The top entry in the list is a TopResource instance.""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 20.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + self._make_top_dir(base, self._TOP_CONTENT) + + resources = get_all_resources(file_path) + + top_resources = [r for r in resources if isinstance(r, TopResource)] + assert len(top_resources) == 1 + + def test_all_three_sources_when_collectl_and_top_present(self) -> None: + """FIO, Collectl, and Top are all returned when both dirs exist.""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 20.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + # Collectl dir + collectl_dir = base / "collectl" + collectl_dir.mkdir() + (collectl_dir / "host-20260619.cpu").write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;50\n") + + # Top dir + self._make_top_dir(base, self._TOP_CONTENT) + + resources = get_all_resources(file_path) + + assert len(resources) == 3 + sources = {r.source for r in resources} + assert sources == {"fio", "collectl", "top"} + + def test_source_order_fio_collectl_top(self) -> None: + """Resources are returned in fio → collectl → top order.""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 20.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + + collectl_dir = base / "collectl" + collectl_dir.mkdir() + (collectl_dir / "host-20260619.cpu").write_text("#Date;Time;[CPU:0]Totl%\n20260619;17:06:30;50\n") + + self._make_top_dir(base, self._TOP_CONTENT) + + resources = get_all_resources(file_path) + + assert resources[0].source == "fio" + assert resources[1].source == "collectl" + assert resources[2].source == "top" + + def test_top_dir_without_out_files_skipped(self) -> None: + """An empty top dir does not add a TopResource.""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 20.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + (base / "top").mkdir() # exists but empty + + resources = get_all_resources(file_path) + + sources = {r.source for r in resources} + assert "top" not in sources + + def test_top_resource_data_retrievable(self) -> None: + """get() on the TopResource returns the expected dict structure.""" + fio_data = {"jobs": [{"job_name": "test", "usr_cpu": 20.0, "sys_cpu": 10.0}]} + + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.write_text(json.dumps(fio_data)) + self._make_top_dir(base, self._TOP_CONTENT) + + resources = get_all_resources(file_path) + top_resource = next(r for r in resources if r.source == "top") + result = top_resource.get() + + assert result["source"] == "top" + assert "cpu" in result + assert "memory" in result + assert float(result["cpu"]) == 30.0 + + +# Made with Bob diff --git a/tests/test_run_result.py b/tests/test_run_result.py index 7292780d..2fccf287 100644 --- a/tests/test_run_result.py +++ b/tests/test_run_result.py @@ -25,7 +25,7 @@ def test_initialization_basic(self): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) result = RBDFIO(path, "json_output") - + assert result._path == path assert result._has_been_processed is False assert result._include_timeseries is False @@ -38,7 +38,7 @@ def test_initialization_with_timeseries(self): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) result = RBDFIO(path, "json_output", include_timeseries=True) - + assert result._include_timeseries is True @@ -50,10 +50,10 @@ def test_process_with_no_files(self): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) result = RBDFIO(path, "json_output") - + # No files created, so _files should be empty result.process() - + assert result._has_been_processed is True assert len(result._processed_data) == 0 @@ -61,7 +61,7 @@ def test_process_with_files(self): """Test process() with valid files.""" with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) - + # Create test data test_data = { "global options": { @@ -71,34 +71,36 @@ def test_process_with_files(self): "numjobs": "1", "runtime": "60", }, - "jobs": [{ - "read": { - "io_bytes": 1000000000, - "bw_bytes": 16666666, - "iops": 4000.0, - "total_ios": 244140, - "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, - }, - "write": { - "io_bytes": 0, - "bw_bytes": 0, - "iops": 0.0, - "total_ios": 0, - "clat_ns": {"mean": 0.0, "stddev": 0.0}, - }, - "sys_cpu": 5.5, - "usr_cpu": 10.2, - }], + "jobs": [ + { + "read": { + "io_bytes": 1000000000, + "bw_bytes": 16666666, + "iops": 4000.0, + "total_ios": 244140, + "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, + }, + "write": { + "io_bytes": 0, + "bw_bytes": 0, + "iops": 0.0, + "total_ios": 0, + "clat_ns": {"mean": 0.0, "stddev": 0.0}, + }, + "sys_cpu": 5.5, + "usr_cpu": 10.2, + } + ], } - + # Create test file test_file = path / "json_output.0" with open(test_file, "w") as f: json.dump(test_data, f) - + result = RBDFIO(path, "json_output") result.process() - + assert result._has_been_processed is True assert len(result._processed_data) > 0 @@ -111,11 +113,11 @@ def test_get_without_processing(self): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) result = RBDFIO(path, "json_output") - + assert result._has_been_processed is False - + data = result.get() - + assert result._has_been_processed is True assert isinstance(data, dict) @@ -124,10 +126,10 @@ def test_get_after_processing(self): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) result = RBDFIO(path, "json_output") - + result.process() data = result.get() - + assert isinstance(data, dict) # Tests for get_timeseries() removed - with memory-efficient approach, @@ -141,14 +143,14 @@ def test_process_empty_file(self): """Test processing skips empty files.""" with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) - + # Create empty file empty_file = path / "json_output.0" empty_file.touch() - + result = RBDFIO(path, "json_output") result.process() - + # Should complete without error, but no data processed assert result._has_been_processed is True @@ -156,7 +158,7 @@ def test_process_precondition_file(self): """Test processing skips precondition files.""" with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) - + # Create test data test_data = { "global options": { @@ -166,35 +168,37 @@ def test_process_precondition_file(self): "numjobs": "1", "runtime": "60", }, - "jobs": [{ - "jobname": "precondition", - "read": { - "io_bytes": 1000000000, - "bw_bytes": 16666666, - "iops": 4000.0, - "total_ios": 244140, - "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, - }, - "write": { - "io_bytes": 0, - "bw_bytes": 0, - "iops": 0.0, - "total_ios": 0, - "clat_ns": {"mean": 0.0, "stddev": 0.0}, - }, - "sys_cpu": 5.5, - "usr_cpu": 10.2, - }], + "jobs": [ + { + "jobname": "precondition", + "read": { + "io_bytes": 1000000000, + "bw_bytes": 16666666, + "iops": 4000.0, + "total_ios": 244140, + "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, + }, + "write": { + "io_bytes": 0, + "bw_bytes": 0, + "iops": 0.0, + "total_ios": 0, + "clat_ns": {"mean": 0.0, "stddev": 0.0}, + }, + "sys_cpu": 5.5, + "usr_cpu": 10.2, + } + ], } - + # Create precondition file precond_file = path / "json_output.0" with open(precond_file, "w") as f: json.dump(test_data, f) - + result = RBDFIO(path, "json_output") result.process() - + # Should complete without error assert result._has_been_processed is True @@ -207,16 +211,16 @@ def test_extract_configuration(self): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) result = RBDFIO(path, "json_output") - + # Create mock benchmark result mock_benchmark = Mock(spec=BenchmarkResult) mock_benchmark.operation = "randread" mock_benchmark.blocksize = "4096" mock_benchmark.iodepth = "32" mock_benchmark.number_of_jobs = "1" - + config = result._extract_test_configuration(mock_benchmark) - + assert config == ("randread", "4096", "32", "1") @@ -228,7 +232,7 @@ def test_merge_with_no_existing_data(self): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) result = RBDFIO(path, "json_output") - + test_config = ("randread", "4096", "32", "1") new_io_details: IodepthDataType = { "io_bytes": "1000000000", @@ -238,9 +242,9 @@ def test_merge_with_no_existing_data(self): "latency": "8.0", "std_deviation": "0.5", } - + merged = result._merge_io_details(test_config, new_io_details) - + # Should return new_io_details unchanged assert merged == new_io_details @@ -249,7 +253,7 @@ def test_merge_with_existing_data(self): with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) result = RBDFIO(path, "json_output") - + # Set up existing data test_config = ("randread", "4096", "32", "1") existing_io: IodepthDataType = { @@ -260,17 +264,9 @@ def test_merge_with_existing_data(self): "latency": "8.0", "std_deviation": "0.5", } - - result._processed_data = { - "randread": { - "1": { - "4096": { - "32": existing_io - } - } - } - } - + + result._processed_data = {"randread": {"1": {"4096": {"32": existing_io}}}} + new_io_details: IodepthDataType = { "io_bytes": "1000000000", "iops": "4000.0", @@ -279,9 +275,9 @@ def test_merge_with_existing_data(self): "latency": "8.0", "std_deviation": "0.5", } - + merged = result._merge_io_details(test_config, new_io_details) - + # Should sum the values assert float(merged["io_bytes"]) == 2000000000.0 assert float(merged["iops"]) == 8000.0 @@ -294,7 +290,7 @@ def test_convert_file_success(self): """Test successful file conversion.""" with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) - + # Create test data test_data = { "global options": { @@ -304,33 +300,35 @@ def test_convert_file_success(self): "numjobs": "1", "runtime": "60", }, - "jobs": [{ - "read": { - "io_bytes": 1000000000, - "bw_bytes": 16666666, - "iops": 4000.0, - "total_ios": 244140, - "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, - }, - "write": { - "io_bytes": 0, - "bw_bytes": 0, - "iops": 0.0, - "total_ios": 0, - "clat_ns": {"mean": 0.0, "stddev": 0.0}, - }, - "sys_cpu": 5.5, - "usr_cpu": 10.2, - }], + "jobs": [ + { + "read": { + "io_bytes": 1000000000, + "bw_bytes": 16666666, + "iops": 4000.0, + "total_ios": 244140, + "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, + }, + "write": { + "io_bytes": 0, + "bw_bytes": 0, + "iops": 0.0, + "total_ios": 0, + "clat_ns": {"mean": 0.0, "stddev": 0.0}, + }, + "sys_cpu": 5.5, + "usr_cpu": 10.2, + } + ], } - + test_file = path / "json_output.0" with open(test_file, "w") as f: json.dump(test_data, f) - + result = RBDFIO(path, "json_output") result._convert_file(test_file) - + # Should have processed data assert len(result._processed_data) > 0 @@ -338,14 +336,14 @@ def test_convert_file_with_error(self): """Test file conversion with error handling.""" with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) - + # Create invalid JSON file test_file = path / "json_output.0" with open(test_file, "w") as f: f.write("invalid json") - + result = RBDFIO(path, "json_output") - + # Should raise an exception with pytest.raises(Exception): result._convert_file(test_file) @@ -358,7 +356,7 @@ def test_full_workflow_single_volume(self): """Test complete workflow with single volume.""" with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) - + # Create test data test_data = { "global options": { @@ -368,33 +366,35 @@ def test_full_workflow_single_volume(self): "numjobs": "1", "runtime": "60", }, - "jobs": [{ - "read": { - "io_bytes": 1000000000, - "bw_bytes": 16666666, - "iops": 4000.0, - "total_ios": 244140, - "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, - }, - "write": { - "io_bytes": 0, - "bw_bytes": 0, - "iops": 0.0, - "total_ios": 0, - "clat_ns": {"mean": 0.0, "stddev": 0.0}, - }, - "sys_cpu": 5.5, - "usr_cpu": 10.2, - }], + "jobs": [ + { + "read": { + "io_bytes": 1000000000, + "bw_bytes": 16666666, + "iops": 4000.0, + "total_ios": 244140, + "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, + }, + "write": { + "io_bytes": 0, + "bw_bytes": 0, + "iops": 0.0, + "total_ios": 0, + "clat_ns": {"mean": 0.0, "stddev": 0.0}, + }, + "sys_cpu": 5.5, + "usr_cpu": 10.2, + } + ], } - + test_file = path / "json_output.0" with open(test_file, "w") as f: json.dump(test_data, f) - + result = RBDFIO(path, "json_output") data = result.get() - + assert "randread" in data assert "1" in data["randread"] assert "4096" in data["randread"]["1"] @@ -404,7 +404,7 @@ def test_full_workflow_multiple_volumes(self): """Test complete workflow with multiple volumes.""" with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) - + # Create test data for multiple volumes test_data = { "global options": { @@ -414,35 +414,37 @@ def test_full_workflow_multiple_volumes(self): "numjobs": "1", "runtime": "60", }, - "jobs": [{ - "read": { - "io_bytes": 1000000000, - "bw_bytes": 16666666, - "iops": 4000.0, - "total_ios": 244140, - "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, - }, - "write": { - "io_bytes": 0, - "bw_bytes": 0, - "iops": 0.0, - "total_ios": 0, - "clat_ns": {"mean": 0.0, "stddev": 0.0}, - }, - "sys_cpu": 5.5, - "usr_cpu": 10.2, - }], + "jobs": [ + { + "read": { + "io_bytes": 1000000000, + "bw_bytes": 16666666, + "iops": 4000.0, + "total_ios": 244140, + "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, + }, + "write": { + "io_bytes": 0, + "bw_bytes": 0, + "iops": 0.0, + "total_ios": 0, + "clat_ns": {"mean": 0.0, "stddev": 0.0}, + }, + "sys_cpu": 5.5, + "usr_cpu": 10.2, + } + ], } - + # Create multiple volume files for i in range(3): test_file = path / f"json_output.{i}" with open(test_file, "w") as f: json.dump(test_data, f) - + result = RBDFIO(path, "json_output") data = result.get() - + # Data should be aggregated from all volumes assert "randread" in data # Navigate through nested structure @@ -459,4 +461,186 @@ def test_full_workflow_multiple_volumes(self): assert iops_value == pytest.approx(12000.0, rel=0.01) +class TestMergeResourceData: + """Test _merge_resource_data() aggregates CPU/memory correctly across volumes.""" + + def _make_rbdfio(self, tmpdir: str) -> RBDFIO: + return RBDFIO(Path(tmpdir), "json_output") + + def test_first_volume_returns_unchanged(self): + """First volume has no existing data — resource data is returned as-is.""" + with tempfile.TemporaryDirectory() as tmpdir: + result = self._make_rbdfio(tmpdir) + test_config = ("randread", "4096", "32", "1") + resource_data = {"cpu": {"fio": "15.70"}, "memory": {"fio": "0.00"}} + + merged = result._merge_resource_data(test_config, resource_data) + + assert merged == resource_data + + def test_fio_cpu_is_summed_across_volumes(self): + """FIO CPU values should be summed because each volume file is independent.""" + with tempfile.TemporaryDirectory() as tmpdir: + result = self._make_rbdfio(tmpdir) + test_config = ("randread", "4096", "32", "1") + + # Simulate first volume already stored + result._processed_data = { + "randread": {"1": {"4096": {"32": {"cpu": {"fio": "10.00"}, "memory": {"fio": "0.00"}}}}} + } + result._resource_volume_counts[test_config] = 1 + + resource_data = {"cpu": {"fio": "15.00"}, "memory": {"fio": "0.00"}} + merged = result._merge_resource_data(test_config, resource_data) + + assert float(merged["cpu"]["fio"]) == pytest.approx(25.00) + + def test_shared_source_cpu_is_averaged_across_volumes(self): + """collectl/top CPU should be averaged because they share one directory.""" + with tempfile.TemporaryDirectory() as tmpdir: + result = self._make_rbdfio(tmpdir) + test_config = ("randread", "4096", "32", "1") + + # One volume already stored, count == 1 + result._processed_data = { + "randread": {"1": {"4096": {"32": {"cpu": {"collectl": "40.00"}, "memory": {"collectl": "0.00"}}}}} + } + result._resource_volume_counts[test_config] = 1 + + resource_data = {"cpu": {"collectl": "60.00"}, "memory": {"collectl": "0.00"}} + merged = result._merge_resource_data(test_config, resource_data) + + # (40 * 1 + 60) / 2 = 50 + assert float(merged["cpu"]["collectl"]) == pytest.approx(50.00) + + def test_shared_source_average_uses_volume_count(self): + """Running average uses the stored volume count for a third+ volume.""" + with tempfile.TemporaryDirectory() as tmpdir: + result = self._make_rbdfio(tmpdir) + test_config = ("randread", "4096", "32", "1") + + # Two volumes already processed, running average is 50, volume_count == 2 + result._processed_data = { + "randread": {"1": {"4096": {"32": {"cpu": {"top": "50.00"}, "memory": {"top": "0.00"}}}}} + } + result._resource_volume_counts[test_config] = 2 + + resource_data = {"cpu": {"top": "62.00"}, "memory": {"top": "0.00"}} + merged = result._merge_resource_data(test_config, resource_data) + + # (50 * 2 + 62) / 3 = 162 / 3 = 54 + assert float(merged["cpu"]["top"]) == pytest.approx(54.00) + + def test_multiple_sources_merged_independently(self): + """FIO is summed while collectl is averaged in the same merge call.""" + with tempfile.TemporaryDirectory() as tmpdir: + result = self._make_rbdfio(tmpdir) + test_config = ("randread", "4096", "32", "1") + + result._processed_data = { + "randread": { + "1": { + "4096": { + "32": { + "cpu": {"fio": "10.00", "collectl": "40.00"}, + "memory": {"fio": "0.00", "collectl": "0.00"}, + } + } + } + } + } + result._resource_volume_counts[test_config] = 1 + + resource_data = { + "cpu": {"fio": "15.00", "collectl": "60.00"}, + "memory": {"fio": "0.00", "collectl": "0.00"}, + } + merged = result._merge_resource_data(test_config, resource_data) + + assert float(merged["cpu"]["fio"]) == pytest.approx(25.00) # summed + assert float(merged["cpu"]["collectl"]) == pytest.approx(50.00) # averaged + + +class TestResourceAggregationIntegration: + """Integration tests confirming CPU is correctly aggregated across multiple volumes.""" + + def _make_volume_data(self, sys_cpu: float = 5.0, usr_cpu: float = 10.0) -> dict: + return { + "global options": { + "bs": "4096", + "rw": "randread", + "iodepth": "32", + "numjobs": "1", + "runtime": "60", + }, + "jobs": [ + { + "read": { + "io_bytes": 1000000000, + "bw_bytes": 16666666, + "iops": 4000.0, + "total_ios": 244140, + "clat_ns": {"mean": 8000000.0, "stddev": 500000.0}, + }, + "write": { + "io_bytes": 0, + "bw_bytes": 0, + "iops": 0.0, + "total_ios": 0, + "clat_ns": {"mean": 0.0, "stddev": 0.0}, + }, + "sys_cpu": sys_cpu, + "usr_cpu": usr_cpu, + } + ], + } + + def test_fio_cpu_summed_across_three_volumes(self): + """FIO CPU from three identical volumes should appear as 3x the per-volume value.""" + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) + per_volume_cpu = 5.5 + 10.2 # sys_cpu + usr_cpu = 15.7 + + for i in range(3): + (path / f"json_output.{i}").write_text(json.dumps(self._make_volume_data(sys_cpu=5.5, usr_cpu=10.2))) + + result = RBDFIO(path, "json_output") + data = result.get() + iodepth_entry = data["randread"]["1"]["4096"]["32"] + + fio_cpu = float(iodepth_entry["cpu"]["fio"]) + assert fio_cpu == pytest.approx(per_volume_cpu * 3, rel=0.01) + + def test_single_volume_fio_cpu_unchanged(self): + """Single-volume result should have exactly the per-volume FIO CPU value.""" + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) + per_volume_cpu = 5.5 + 10.2 + + (path / "json_output.0").write_text(json.dumps(self._make_volume_data(sys_cpu=5.5, usr_cpu=10.2))) + + result = RBDFIO(path, "json_output") + data = result.get() + iodepth_entry = data["randread"]["1"]["4096"]["32"] + + fio_cpu = float(iodepth_entry["cpu"]["fio"]) + assert fio_cpu == pytest.approx(per_volume_cpu, rel=0.01) + + def test_volume_counter_not_stored_in_processed_data(self): + """The internal volume counter must not appear anywhere in the output data.""" + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) + + for i in range(2): + (path / f"json_output.{i}").write_text(json.dumps(self._make_volume_data())) + + result = RBDFIO(path, "json_output") + data = result.get() + iodepth_entry = data["randread"]["1"]["4096"]["32"] + + # Counter lives in _resource_volume_counts, not inside _processed_data + assert "_resource_volume_count" not in iodepth_entry + assert "_resource_volume_count" not in iodepth_entry.get("cpu", {}) + + # Made with Bob diff --git a/tests/test_time_series_latency_plotter.py b/tests/test_time_series_latency_plotter.py index 4966694f..c18b4503 100644 --- a/tests/test_time_series_latency_plotter.py +++ b/tests/test_time_series_latency_plotter.py @@ -148,9 +148,9 @@ def test_plot_skips_max_when_zero(self) -> None: def test_latency_constants(self) -> None: """Test latency plotter constants""" self.assertEqual(LATENCY_MEAN_COLOR, "xkcd:orange") - self.assertEqual(LATENCY_P50_COLOR, "xkcd:green") + self.assertEqual(LATENCY_P50_COLOR, "xkcd:blue") self.assertEqual(LATENCY_P95_COLOR, "xkcd:red") - self.assertEqual(LATENCY_P99_COLOR, "xkcd:dark red") + self.assertEqual(LATENCY_P99_COLOR, "xkcd:purple") self.assertEqual(LATENCY_Y_LABEL, "Latency (ms)") self.assertEqual(LATENCY_PLOT_LABEL, "Mean Latency") diff --git a/tests/test_top_resource.py b/tests/test_top_resource.py new file mode 100644 index 00000000..faccd465 --- /dev/null +++ b/tests/test_top_resource.py @@ -0,0 +1,469 @@ +""" +Unit tests for TopResource class. +""" + +# pyright: strict, reportPrivateUsage=false + +import tempfile +from pathlib import Path +from unittest import mock + +import pytest + +from post_processing.run_results.resources.top_resource import TopResource + +# --------------------------------------------------------------------------- +# Shared fixture helpers +# --------------------------------------------------------------------------- + +# A minimal two-snapshot top -b output for a single PID. +# Snapshot 1: one thread at 25.0 % CPU, 0.6 % MEM +# Snapshot 2: one thread at 35.0 % CPU, 0.8 % MEM +# Expected averages: cpu = (25.0 + 35.0) / 2 = 30.0, mem = (0.6 + 0.8) / 2 = 0.7 +SINGLE_PID_TWO_SNAPSHOTS = """\ +top - 14:32:01 up 2 days, 3:10, 0 users, load average: 6.06, 7.09, 8.13 +Tasks: 1 total, 0 running, 1 sleeping, 0 stopped, 0 zombie +%Cpu(s): 12.3 us, 4.1 sy, 0.0 ni, 83.2 id, 0.3 wa +MiB Mem : 376023.2 total, 333174.8 free, 38078.1 used + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND +12345 ceph 20 0 1.234g 234.0m 12.0m S 25.0 0.6 1:23.45 crimson-osd + +top - 14:32:02 up 2 days, 3:10, 0 users, load average: 6.06, 7.09, 8.13 +Tasks: 1 total, 0 running, 1 sleeping, 0 stopped, 0 zombie +%Cpu(s): 13.1 us, 3.9 sy, 0.0 ni, 82.6 id, 0.4 wa +MiB Mem : 376023.2 total, 333174.8 free, 38078.1 used + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND +12345 ceph 20 0 1.234g 234.0m 12.0m S 35.0 0.8 1:24.50 crimson-osd +""" + +# Two threads per snapshot (as produced by top -H). +# Snapshot 1: thread A 20.0 cpu 0.4 mem, thread B 10.0 cpu 0.2 mem +# Snapshot 2: thread A 30.0 cpu 0.5 mem, thread B 10.0 cpu 0.2 mem +# Expected averages: cpu = (20+10+30+10)/4 = 17.5, mem = (0.4+0.2+0.5+0.2)/4 = 0.325 +MULTI_THREAD_TWO_SNAPSHOTS = """\ +top - 14:32:01 up 1 day +Tasks: 2 total +%Cpu(s): 5.0 us +MiB Mem : 376023.2 total + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND + 1001 ceph 20 0 1.0g 100m 10m S 20.0 0.4 0:10.00 reactor-0 + 1002 ceph 20 0 1.0g 100m 10m S 10.0 0.2 0:05.00 reactor-1 + +top - 14:32:02 up 1 day +Tasks: 2 total +%Cpu(s): 5.0 us +MiB Mem : 376023.2 total + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND + 1001 ceph 20 0 1.0g 100m 10m S 30.0 0.5 0:11.00 reactor-0 + 1002 ceph 20 0 1.0g 100m 10m S 10.0 0.2 0:06.00 reactor-1 +""" + + +def _make_top_dir(base: Path, files: dict[str, str]) -> None: + """Create a 'top' subdirectory containing the given {filename: content} files.""" + top_dir = base / "top" + top_dir.mkdir() + for name, content in files.items(): + (top_dir / name).write_text(content) + + +# --------------------------------------------------------------------------- +# Initialisation +# --------------------------------------------------------------------------- + + +class TestTopResourceInitialization: + """Test TopResource initialisation.""" + + def test_valid_setup(self) -> None: + """Initialisation succeeds when top dir exists with an output file.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": SINGLE_PID_TWO_SNAPSHOTS}) + + resource = TopResource(file_path) + + assert resource.source == "top" + + def test_missing_top_directory_raises(self) -> None: + """FileNotFoundError raised when the top directory does not exist.""" + with tempfile.TemporaryDirectory() as tmpdir: + file_path = Path(tmpdir) / "json_output.0" + file_path.touch() + + with pytest.raises(FileNotFoundError, match="top directory not found"): + TopResource(file_path) + + def test_empty_top_directory_raises(self) -> None: + """FileNotFoundError raised when the top directory is empty.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + (base / "top").mkdir() + + with pytest.raises(FileNotFoundError, match="top directory is empty"): + TopResource(file_path) + + +# --------------------------------------------------------------------------- +# Parsing — single file +# --------------------------------------------------------------------------- + + +class TestTopResourceParsingSingleFile: + """Test CPU/memory extraction from a single top output file.""" + + def test_single_snapshot_single_thread(self) -> None: + """Parse one snapshot with one process line.""" + content = """\ +top - 14:32:01 up 1 day +Tasks: 1 total +%Cpu(s): 5.0 us +MiB Mem : 376023.2 total + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND +12345 ceph 20 0 1.0g 100m 10m S 40.0 1.2 0:10.00 crimson-osd +""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": content}) + + resource = TopResource(file_path) + + assert resource.cpu == "40.00" + assert resource.memory == "1.20" + + def test_two_snapshots_averaged(self) -> None: + """CPU and memory are averaged across all snapshots.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": SINGLE_PID_TWO_SNAPSHOTS}) + + resource = TopResource(file_path) + + assert resource.cpu == "30.00" + assert resource.memory == "0.70" + + def test_multi_thread_two_snapshots(self) -> None: + """Multiple threads per snapshot are all included in the average.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"1001_osd_top.out": MULTI_THREAD_TWO_SNAPSHOTS}) + + resource = TopResource(file_path) + + assert resource.cpu == "17.50" + assert resource.memory == "0.33" + + def test_zero_cpu_values(self) -> None: + """Handles zero CPU/memory without errors.""" + content = """\ +top - 14:32:01 up 1 day +%Cpu(s): 0.0 us +MiB Mem : 100.0 total + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND +99999 root 20 0 100m 10m 1m S 0.0 0.0 0:00.00 idle +""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"99999_osd_top.out": content}) + + resource = TopResource(file_path) + + assert resource.cpu == "0.00" + assert resource.memory == "0.00" + + def test_header_only_no_data_rows(self) -> None: + """A file with a PID header but no data rows returns 0.""" + content = """\ +top - 14:32:01 up 1 day +%Cpu(s): 5.0 us + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND +""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": content}) + + resource = TopResource(file_path) + + assert resource.cpu == "0.00" + assert resource.memory == "0.00" + + def test_empty_file_returns_zero(self) -> None: + """An empty .out file does not crash and returns 0.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": ""}) + + resource = TopResource(file_path) + + assert resource.cpu == "0.00" + assert resource.memory == "0.00" + + +# --------------------------------------------------------------------------- +# Parsing — multiple files +# --------------------------------------------------------------------------- + + +class TestTopResourceParsingMultipleFiles: + """Test aggregation across multiple PID output files.""" + + def test_two_pid_files_averaged(self) -> None: + """CPU is averaged across all PID files.""" + # pid A: one snapshot, one thread at 20.0 % + content_a = """\ +top - 14:32:01 up 1 day +%Cpu(s): 5.0 us + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND + 1001 ceph 20 0 1.0g 100m 10m S 20.0 0.4 0:10.00 reactor-0 +""" + # pid B: one snapshot, one thread at 60.0 % + content_b = """\ +top - 14:32:01 up 1 day +%Cpu(s): 20.0 us + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND + 2001 ceph 20 0 1.0g 100m 10m S 60.0 0.8 0:10.00 reactor-1 +""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"1001_osd_top.out": content_a, "2001_osd_top.out": content_b}) + + resource = TopResource(file_path) + + # (20.0 + 60.0) / 2 = 40.0 + assert resource.cpu == "40.00" + + def test_three_pid_files(self) -> None: + """Aggregation works for three files.""" + + def _make_content(cpu_val: float, mem_val: float) -> str: + return ( + "top - 14:32:01\n%Cpu(s): 5.0 us\n\n" + " PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND\n" + f" 1000 ceph 20 0 1.0g 100m 10m S {cpu_val} {mem_val} 0:01.00 osd\n" + ) + + files = { + "1000_osd_top.out": _make_content(10.0, 0.2), + "2000_osd_top.out": _make_content(20.0, 0.4), + "3000_osd_top.out": _make_content(30.0, 0.6), + } + + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, files) + + resource = TopResource(file_path) + + # (10 + 20 + 30) / 3 = 20.0 + assert resource.cpu == "20.00" + # (0.2 + 0.4 + 0.6) / 3 = 0.4 + assert resource.memory == "0.40" + + +# --------------------------------------------------------------------------- +# get() method +# --------------------------------------------------------------------------- + + +class TestTopResourceGet: + """Test the get() method output contract.""" + + def test_get_returns_correct_keys(self) -> None: + """get() returns a dict with source, cpu, and memory keys.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": SINGLE_PID_TWO_SNAPSHOTS}) + + result = TopResource(file_path).get() + + assert result == {"source": "top", "cpu": "30.00", "memory": "0.70"} + + def test_get_triggers_parsing_once(self) -> None: + """Calling get() twice does not re-parse.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": SINGLE_PID_TWO_SNAPSHOTS}) + + resource = TopResource(file_path) + first = resource.get() + second = resource.get() + + assert first == second + assert resource._has_been_parsed is True # pylint: disable=protected-access + + def test_source_property(self) -> None: + """source property returns 'top' before any parsing.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": SINGLE_PID_TWO_SNAPSHOTS}) + + resource = TopResource(file_path) + + assert resource.source == "top" + + +# --------------------------------------------------------------------------- +# Edge cases — error paths not covered by the happy-path tests +# --------------------------------------------------------------------------- + + +class TestTopResourceEdgeCases: + """Test uncommon but reachable branches.""" + + def test_pid_header_missing_cpu_column_is_skipped(self) -> None: + """A PID header without a %%CPU column logs a warning and is skipped.""" + # No %CPU or %MEM in the header — the block should be silently skipped. + content = """\ +top - 14:32:01 up 1 day +%Cpu(s): 5.0 us + + PID USER PR NI VIRT RES SHR S NOCPU NOMEM TIME+ COMMAND +12345 ceph 20 0 1.0g 100m 10m S 40.0 1.2 0:10.00 crimson-osd +""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": content}) + + resource = TopResource(file_path) + + # No valid samples collected — should fall back to 0 + assert resource.cpu == "0.00" + assert resource.memory == "0.00" + + def test_data_row_too_short_is_skipped(self) -> None: + """A process row that has fewer columns than the %CPU/%MEM indices is skipped.""" + # The PID header puts %CPU at column 8, %MEM at column 9. + # The data row below has only 4 columns and must be silently skipped. + content = """\ +top - 14:32:01 up 1 day +%Cpu(s): 5.0 us + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND +12345 ceph too_short +""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": content}) + + resource = TopResource(file_path) + + assert resource.cpu == "0.00" + assert resource.memory == "0.00" + + def test_unparseable_cpu_value_is_skipped(self) -> None: + """A data row with a non-numeric CPU value is skipped without crashing.""" + content = """\ +top - 14:32:01 up 1 day +%Cpu(s): 5.0 us + + PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND +12345 ceph 20 0 1.0g 100m 10m S N/A N/A 0:10.00 crimson-osd +""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": content}) + + resource = TopResource(file_path) + + assert resource.cpu == "0.00" + assert resource.memory == "0.00" + + def test_parse_exception_returns_zero(self) -> None: + """If _parse_top_directory raises unexpectedly, cpu/memory fall back to 0.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"12345_osd_top.out": SINGLE_PID_TWO_SNAPSHOTS}) + + resource = TopResource(file_path) + with mock.patch.object(resource, "_parse_top_directory", side_effect=RuntimeError("boom")): + resource._parse({}) # pylint: disable=protected-access + + assert resource.cpu == "0.00" + assert resource.memory == "0.00" + assert resource._has_been_parsed is True # pylint: disable=protected-access + + +# --------------------------------------------------------------------------- +# All-files behaviour +# --------------------------------------------------------------------------- + + +class TestTopResourceAllFiles: + """TopResource reads every file in the top/ directory regardless of name.""" + + def test_arbitrarily_named_file_is_parsed(self) -> None: + """A file with any name in top/ is parsed.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + _make_top_dir(base, {"my_custom_output.txt": SINGLE_PID_TWO_SNAPSHOTS}) + + resource = TopResource(file_path) + + assert resource.cpu == "30.00" + assert resource.memory == "0.70" + + def test_mixed_naming_all_files_aggregated(self) -> None: + """Files with different naming conventions are all included in the average.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + file_path = base / "json_output.0" + file_path.touch() + # One file with legacy OSD name, one with a custom name — both parsed. + _make_top_dir( + base, + { + "12345_osd_top.out": SINGLE_PID_TWO_SNAPSHOTS, + "99999_mon_top.out": SINGLE_PID_TWO_SNAPSHOTS, + }, + ) + + resource = TopResource(file_path) + + # Both files have the same content so the average is unchanged, + # but the sample count doubles — result stays 30.00. + assert resource.cpu == "30.00" diff --git a/tests/test_workloads.py b/tests/test_workloads.py index 7ff58c56..484eaa07 100644 --- a/tests/test_workloads.py +++ b/tests/test_workloads.py @@ -136,7 +136,7 @@ def test_get_global_options_excludes_prefill(self) -> None: @patch("workloads.workloads.pdsh") @patch("workloads.workloads.make_remote_dir") - @patch("workloads.workloads.monitoring") + @patch("workloads.workloads.MonitoringFactory") @patch("workloads.workloads.getnodes") @patch("workloads.workloads.sleep") def test_run_with_workloads( @@ -164,7 +164,7 @@ def test_run_with_workloads( @patch("workloads.workloads.pdsh") @patch("workloads.workloads.make_remote_dir") - @patch("workloads.workloads.monitoring") + @patch("workloads.workloads.MonitoringFactory") @patch("workloads.workloads.getnodes") def test_run_with_script( self, @@ -226,7 +226,7 @@ def test_run_without_workloads(self) -> None: @patch("workloads.workloads.pdsh") @patch("workloads.workloads.make_remote_dir") - @patch("workloads.workloads.monitoring") + @patch("workloads.workloads.MonitoringFactory") @patch("workloads.workloads.getnodes") @patch("workloads.workloads.sleep") def test_run_with_ramp_time( diff --git a/workloads/workloads.py b/workloads/workloads.py index 877285c5..cb89de48 100644 --- a/workloads/workloads.py +++ b/workloads/workloads.py @@ -6,8 +6,8 @@ from time import sleep from typing import Optional, Union -import monitoring from common import CheckedPopen, CheckedPopenLocal, make_remote_dir, pdsh # pyright: ignore[reportUnknownVariableType] +from monitoring.monitoring_factory import MonitoringFactory from settings import getnodes # pyright: ignore[reportUnknownVariableType] from workloads.workload import Workload from workloads.workload_types import BenchmarkConfigurationType, WorkloadType, WorkloadYamlType @@ -92,12 +92,12 @@ def run(self) -> None: if ramp_time: sleep(int(ramp_time)) - monitoring.start(output_directory) # type: ignore[no-untyped-call] + MonitoringFactory.start(output_directory) for process in processes: process.wait() # type: ignore[no-untyped-call] - monitoring.stop() # type: ignore[no-untyped-call] + MonitoringFactory.stop() log.info("== Workloads completed ==")