diff --git a/hotsos/cli.py b/hotsos/cli.py index 7985dbcc3..919277a96 100755 --- a/hotsos/cli.py +++ b/hotsos/cli.py @@ -5,6 +5,7 @@ import sys import threading from importlib import metadata, resources +from dataclasses import dataclass, fields import click import distro @@ -15,7 +16,7 @@ from hotsos.core.log import log, LoggingManager from hotsos.client import ( HotSOSClient, - OutputManager, + SUPPORTED_SUMMARY_FORMATS ) SNAP_ERROR_MSG = """ERROR: hotsos is installed as a snap which only supports @@ -116,7 +117,7 @@ def set_plugin_options(f): def get_defs_path(): - """ Get patch to HotSOS defs. """ + """ Get path to HotSOS defs. """ # source defs = os.path.join(get_hotsos_root(), 'defs') @@ -188,7 +189,52 @@ def progress_spinner(show_spinner, path): thread.join() -def main(): # pylint: disable=R0915 +def filter_unexpected_fields(cls): + original_init = cls.__init__ + + def new_init(self, *args, **kwargs): + def_fields = {field.name for field in fields(cls)} + cleaned_kwargs = { + key: value for key, value in kwargs.items() if key in def_fields + } + original_init(self, *args, **cleaned_kwargs) + + cls.__init__ = new_init + return cls + + +@filter_unexpected_fields +@dataclass(frozen=True) +class CLIArgs: + """Command line arguments.""" + + data_root: click.Path + version: bool + defs_path: str + templates_path: str + all_logs: bool + debug: bool + quiet: bool + save: bool + output_format: str + html_escape: bool + short: bool + very_short: bool + force: bool + event_tally_granularity: str + max_logrotate_depth: int + max_parallel_tasks: int + list_plugins: bool + machine_readable: bool + output_path: str + command_timeout: int + sos_unpack_dir: str + scenario: str + event: str + + +# pylint: disable=R0915 +def main(): @click.command(name='hotsos') @click.option('--event', default='', help=('Filter a particular event name. Useful for ' @@ -248,7 +294,7 @@ def main(): # pylint: disable=R0915 help=('Apply html escaping to the output so that it is safe ' 'to display in html.')) @click.option('--format', '--output-format', 'output_format', - type=click.Choice(OutputManager.SUMMARY_FORMATS), + type=click.Choice(SUPPORTED_SUMMARY_FORMATS), default='yaml', show_default=True, help='Summary output format.') @@ -275,11 +321,7 @@ def main(): # pylint: disable=R0915 help='Show the version.') @set_plugin_options @click.argument('data_root', required=False, type=click.Path(exists=True)) - def cli(data_root, version, defs_path, templates_path, all_logs, debug, - quiet, save, output_format, html_escape, short, very_short, - force, event_tally_granularity, max_logrotate_depth, - max_parallel_tasks, list_plugins, machine_readable, output_path, - command_timeout, sos_unpack_dir, scenario, event, **kwargs): + def cli(*args, **kwargs): """ Run this tool on a host or against a sosreport to perform analysis of specific applications and the host itself. A summary of @@ -303,36 +345,39 @@ def cli(data_root, version, defs_path, templates_path, all_logs, debug, host. """ # noqa + arguments = CLIArgs(*args, **kwargs) + _version = get_version() - if version: + if arguments.version: print(_version) return config = {'repo_info': get_repo_info(), - 'force_mode': force, + 'force_mode': arguments.force, 'hotsos_version': _version, - 'command_timeout': command_timeout, - 'use_all_logs': all_logs, - 'plugin_yaml_defs': defs_path, - 'templates_path': templates_path, - 'event_tally_granularity': event_tally_granularity, - 'max_logrotate_depth': max_logrotate_depth, - 'max_parallel_tasks': max_parallel_tasks, - 'machine_readable': machine_readable, - 'debug_mode': debug, - 'scenario_filter': scenario, - 'event_filter': event} + 'command_timeout': arguments.command_timeout, + 'use_all_logs': arguments.all_logs, + 'plugin_yaml_defs': arguments.defs_path, + 'templates_path': arguments.templates_path, + 'event_tally_granularity': arguments.event_tally_granularity, + 'max_logrotate_depth': arguments.max_logrotate_depth, + 'max_parallel_tasks': arguments.max_parallel_tasks, + 'machine_readable': arguments.machine_readable, + 'debug_mode': arguments.debug, + 'scenario_filter': arguments.scenario, + 'event_filter': arguments.event} HotSOSConfig.set(**config) with LoggingManager() as logmanager: - with DataRootManager(data_root, - sos_unpack_dir=sos_unpack_dir) as drm: + with DataRootManager( + arguments.data_root, sos_unpack_dir=arguments.sos_unpack_dir + ) as drm: HotSOSConfig.data_root = drm.data_root if is_snap() and drm.data_root == '/': print(SNAP_ERROR_MSG) sys.exit(1) - if debug and quiet: + if arguments.debug and arguments.quiet: sys.stderr.write('ERROR: cannot use both --debug and ' '--quiet\n') return @@ -340,12 +385,13 @@ def cli(data_root, version, defs_path, templates_path, all_logs, debug, # Set a name so that logs have this until real plugins are run. log.name = 'hotsos.cli' - if list_plugins: + if arguments.list_plugins: sys.stdout.write('\n'.join(plugintools.PLUGINS.keys())) sys.stdout.write('\n') return - with progress_spinner(not quiet and not debug, drm.name): + with progress_spinner( + not arguments.quiet and not arguments.debug, drm.name): plugins = [] for k, v in kwargs.items(): if v is True: @@ -371,21 +417,28 @@ def cli(data_root, version, defs_path, templates_path, all_logs, debug, summary = client.summary - if save: - path = summary.save(drm.basename, html_escape=html_escape, - output_path=output_path) + if arguments.save: + path = summary.save( + drm.basename, + html_escape=arguments.html_escape, + output_path=arguments.output_path, + ) sys.stdout.write(f"INFO: output saved to {path}\n") else: - if short: + if arguments.short: minimal_mode = 'short' - elif very_short: + elif arguments.very_short: minimal_mode = 'very-short' else: minimal_mode = None - out = summary.get(fmt=output_format, - html_escape=html_escape, - minimal_mode=minimal_mode) + output = summary.get_builder() + output.minimal(minimal_mode) + out = output.to( + fmt=arguments.output_format, + html_escape=arguments.html_escape + ) + if out: sys.stdout.write(f"{out}\n") diff --git a/hotsos/client.py b/hotsos/client.py index 647c2d74c..eced36fc3 100755 --- a/hotsos/client.py +++ b/hotsos/client.py @@ -5,6 +5,7 @@ import os import shutil import tempfile +from typing import Literal # load all plugins import hotsos.plugin_extensions # noqa: F401, pylint: disable=W0611 @@ -39,19 +40,41 @@ def summary(self): return out -class OutputManager(): - """ Handle conversion of plugin output into summary format. """ - FILTER_SCHEMA = [IssuesManager.SUMMARY_OUT_ISSUES_ROOT, - IssuesManager.SUMMARY_OUT_BUGS_ROOT] - SUMMARY_FORMATS = ['yaml', 'json', 'markdown', 'html'] +FILTER_SCHEMA = [IssuesManager.SUMMARY_OUT_ISSUES_ROOT, + IssuesManager.SUMMARY_OUT_BUGS_ROOT] - def __init__(self, initial=None): - self._summary = initial or {} +SUPPORTED_SUMMARY_FORMATS = ['yaml', 'json', 'markdown', 'html'] + + +class OutputBuilder: + """Builder class for generating desired output format from + raw dictionary.""" + + def __init__(self, content): + self.content = content + + @staticmethod + def _minimise(summary, mode): + """ Converts the master output to include only issues and bugs. """ + + log.debug("Minimising output (mode=%s).", mode) + + if not summary: + return summary + + if mode == 'short': + return OutputBuilder._get_short_format(summary) + if mode == 'very-short': + return OutputBuilder._get_very_short_format(summary) + + log.warning("Unknown minimalmode '%s'", mode) + return summary - def _get_short_format(self, summary): + @staticmethod + def _get_short_format(summary): filtered = {} for plugin in summary: - for key in self.FILTER_SCHEMA: + for key in FILTER_SCHEMA: if key not in summary[plugin]: continue @@ -63,10 +86,11 @@ def _get_short_format(self, summary): return filtered - def _get_very_short_format(self, summary): + @staticmethod + def _get_very_short_format(summary): filtered = {} for plugin in summary: - for key in self.FILTER_SCHEMA: + for key in FILTER_SCHEMA: if key not in summary[plugin]: continue @@ -101,61 +125,62 @@ def _get_very_short_format(self, summary): return filtered - def minimise(self, summary, mode): - """ Converts the master output to include only issues and bugs. """ + def filter(self, plugin_name=None): + if plugin_name: + self.content = {plugin_name: self.content[plugin_name]} + return self + + def minimal(self, mode=None): + if mode: + self.content = self._minimise(self.content, mode) + return self + + def to(self, fmt: Literal[SUPPORTED_SUMMARY_FORMATS], + **kwargs): + if fmt == "html": + return self.to_html(**kwargs) + if fmt == "json": + return self.to_json() + if fmt == "yaml": + return self.to_yaml() + if fmt == "markdown": + return self.to_markdown() + + raise UnsupportedFormatError(fmt) + + def to_html(self, *, max_level=2, escape=False): + hostname = CLIHelper().hostname() or "" + result = plugintools.HTMLFormatter( + hostname=hostname, + max_level=max_level + ).dump(self.content) - log.debug("Minimising output (mode=%s).", mode) - if not summary: - return summary + return result if not escape else html.escape(result) - if mode == 'short': - return self._get_short_format(summary) - if mode == 'very-short': - return self._get_very_short_format(summary) + def to_json(self): + return json.dumps(self.content, indent=2, sort_keys=True) - log.warning("Unknown minimalmode '%s'", mode) - return summary + def to_yaml(self): + return plugintools.yaml_dump(self.content) - def get(self, fmt='yaml', html_escape=False, minimal_mode=None, - plugin=None, max_level=2): - if plugin: - filtered = {plugin: self._summary[plugin]} - else: - filtered = self._summary + def to_markdown(self): + return plugintools.MarkdownFormatter().dump(self.content) - if minimal_mode: - filtered = self.minimise(filtered, minimal_mode) - if fmt not in self.SUMMARY_FORMATS: - raise UnsupportedFormatError( - f"unsupported summary format '{fmt}'") +class OutputManager(): + """ Handle conversion of plugin output into summary format. """ - hostname = CLIHelper().hostname() or "" - log.debug('Saving summary as %s', fmt) - if fmt == 'yaml': - filtered = plugintools.yaml_dump(filtered) - elif fmt == 'json': - filtered = json.dumps(filtered, indent=2, sort_keys=True) - elif fmt == 'markdown': - filtered = plugintools.MarkdownFormatter().dump(filtered) - elif fmt == 'html': - filtered = plugintools.HTMLFormatter( - hostname=hostname, - max_level=max_level).dump(filtered) - - if html_escape: - log.debug('Applying html escaping to summary') - filtered = html.escape(filtered) + def __init__(self, initial=None): + self._summary = initial or {} - return filtered + def get_builder(self): + return OutputBuilder(self._summary) - def _save(self, path, fmt, html_escape=None, minimal_mode=None, - plugin=None): - content = self.get(fmt=fmt, html_escape=html_escape, - minimal_mode=minimal_mode, plugin=plugin) - with open(path, 'w', encoding='utf-8') as fd: + @staticmethod + def _save_to_file(path, content): + with open(path, "w", encoding="utf-8") as fd: fd.write(content) - fd.write('\n') + fd.write("\n") def save(self, name, html_escape=False, output_path=None): """ @@ -171,7 +196,7 @@ def save(self, name, html_escape=False, output_path=None): for minimal_mode in ['full', 'short', 'very-short']: _minimal_mode = minimal_mode.replace('-', '_') - for fmt in self.SUMMARY_FORMATS: + for fmt in SUPPORTED_SUMMARY_FORMATS: output_path = os.path.join(output_root, name, 'summary', _minimal_mode, fmt) if minimal_mode == 'full': @@ -180,15 +205,25 @@ def save(self, name, html_escape=False, output_path=None): if not os.path.exists(output_path): os.makedirs(output_path) + # Save per-plugin summary for plugin in self._summary: path = os.path.join(output_path, f"hotsos-summary.{plugin}.{fmt}") - self._save(path, fmt, html_escape=html_escape, - minimal_mode=minimal_mode, plugin=plugin) - + output = self.get_builder() + output.filter(plugin).minimal(minimal_mode) + formatted_output = output.to(fmt=fmt, escape=html_escape) + log.debug('Saving plugin %s summary as %s', + plugin, + fmt) + self._save_to_file(path, formatted_output) + + # Save all summary path = os.path.join(output_path, f"hotsos-summary.all.{fmt}") - self._save(path, fmt, html_escape=html_escape, - minimal_mode=minimal_mode) + output = self.get_builder() + output.minimal(minimal_mode) + formatted_output = output.to(fmt=fmt, escape=html_escape) + log.debug('Saving all summary as %s', fmt) + self._save_to_file(path, formatted_output) if not minimal_mode: dst = os.path.join(output_root, f'{name}.summary.{fmt}') diff --git a/hotsos/core/analytics.py b/hotsos/core/analytics.py index 752610dc3..4bdd395c2 100644 --- a/hotsos/core/analytics.py +++ b/hotsos/core/analytics.py @@ -1,5 +1,6 @@ import statistics from datetime import datetime +from dataclasses import dataclass class EventCollection(): @@ -151,24 +152,23 @@ def calculate_event_deltas(self): start_item["end"] = end_ts +@dataclass(frozen=True) class SearchResultIndices(): """ Used to know where to find required information within a SearchResult. + + The indexes refer to python.re groups. + + The minimum required information that a result must contain is day, + secs and event_id. Results will be referred to using whatever event_id + is set to. """ - def __init__(self, day_idx=1, secs_idx=2, event_id_idx=3, - metadata_idx=None, metadata_key=None): - """ - The indexes refer to python.re groups. - The minimum required information that a result must contain is day, - secs and event_id. Results will be referred to using whatever event_id - is set to. - """ - self.day = day_idx - self.secs = secs_idx - self.event_id = event_id_idx - self.metadata = metadata_idx - self.metadata_key = metadata_key + day: int = 1 + secs: int = 2 + event_id: int = 3 + metadata: int = None + metadata_key: str = None class LogEventStats(): diff --git a/hotsos/core/host_helpers/network.py b/hotsos/core/host_helpers/network.py index 89e00320b..e8d352419 100644 --- a/hotsos/core/host_helpers/network.py +++ b/hotsos/core/host_helpers/network.py @@ -1,6 +1,7 @@ import copy import os import re +from dataclasses import dataclass # NOTE: we import direct from searchkit rather than hotsos.core.search to # avoid circular dependency issues. @@ -27,17 +28,20 @@ IP_EOF = r"^$" +@dataclass() class NetworkPort(HostHelpersBase): """ Representation of a network port. """ - def __init__(self, name, addresses, hwaddr, state, encap_info, - mtu, namespace=None): - self.name = name - self.addresses = addresses - self.hwaddr = hwaddr - self.state = state - self.encap_info = encap_info - self.mtu = mtu - self.namespace = namespace + + name: str + addresses: list + hwaddr: str + state: str + encap_info: str + mtu: str + namespace: str = None + + def __post_init__(self): + # Required for initializing the self.cache super().__init__() @property diff --git a/hotsos/core/plugins/openstack/openstack.py b/hotsos/core/plugins/openstack/openstack.py index 04c997ee2..2fde27f65 100644 --- a/hotsos/core/plugins/openstack/openstack.py +++ b/hotsos/core/plugins/openstack/openstack.py @@ -1,6 +1,7 @@ import os from datetime import datetime from functools import cached_property +from dataclasses import dataclass, field from hotsos.core.config import HotSOSConfig from hotsos.core import host_helpers @@ -300,6 +301,40 @@ def __getattr__(self, key): return self.get(key) +@dataclass(frozen=True) +class OSTProjectParameters: + """Parameters for OSTProject class. + + @param name: name of this project + @param config: dict of config files keyed by a label used to identify + them. All projects should have a config file labelled + 'main'. + @param apt_core_alt: optional list of apt packages (regex) that are + used by this project where the name of the project + is not the same as the name used for its packages. + @param systemd_masked_services: optional list of services that are + expected to be masked in systemd e.g. if they are actually being + run by apache. + @param systemd_deprecated_services: optional list of services that are + deprecated. This can be helpful if e.g. you want to skip checks + for resources related to these services. + @param systemd_extra_services: optional list of systemd services that + are used. This is useful e.g. if components are run under + apache or if a package runs components using services whose name + don't match the name of the project. + @param log_path_overrides: specify log path for a given service that + overrides the default format i.e. + /var/log//.log. + """ + name: str = None + config: dict = None + apt_core_alt: list = None + systemd_masked_services: list = field(default_factory=lambda: []) + systemd_deprecated_services: list = field(default_factory=lambda: []) + systemd_extra_services: list = field(default_factory=lambda: []) + log_path_overrides: dict = field(default_factory=lambda: {}) + + class OSTProject(): """ Representation of an Openstack project/service. @@ -309,11 +344,7 @@ class OSTProject(): SVC_VALID_SUFFIX = r'[0-9a-zA-Z-_]*' PY_CLIENT_PREFIX = r"python3?-{}\S*" - def __init__(self, name, config=None, apt_core_alt=None, - systemd_masked_services=None, - systemd_deprecated_services=None, - log_path_overrides=None, - systemd_extra_services=None): + def __init__(self, params: OSTProjectParameters): """ @param name: name of this project @param config: dict of config files keyed by a label used to identify @@ -336,26 +367,34 @@ def __init__(self, name, config=None, apt_core_alt=None, apache or if a package runs components using services whose name don't match the name of the project. """ - self.name = name - self.packages_deps = [self.PY_CLIENT_PREFIX.format(name)] - self.packages_core = [name] - if apt_core_alt: - self.packages_core.extend(apt_core_alt) - for c in apt_core_alt: + self.name = params.name + self.packages_deps = [self.PY_CLIENT_PREFIX.format(params.name)] + self.packages_core = [params.name] + if params.apt_core_alt: + self.packages_core.extend(params.apt_core_alt) + for c in params.apt_core_alt: self.packages_deps.append(self.PY_CLIENT_PREFIX.format(c)) self.config = {} - if config: - for label, path in config.items(): - path = os.path.join(HotSOSConfig.data_root, 'etc', name, path) + if params.config: + for label, path in params.config.items(): + path = os.path.join( + HotSOSConfig.data_root, + "etc", + params.name, + path + ) self.config[label] = OpenstackConfig(path) - self.systemd_extra_services = systemd_extra_services or [] - self.systemd_masked_services = systemd_masked_services or [] - self.systemd_deprecated_services = systemd_deprecated_services or [] - self.logs_path = os.path.join('var/log', name) - self.log_path_overrides = log_path_overrides or {} - self.exceptions = EXCEPTIONS_COMMON + OST_EXCEPTIONS.get(name, []) + self.systemd_extra_services = params.systemd_extra_services + self.systemd_masked_services = params.systemd_masked_services + self.systemd_deprecated_services = params.systemd_deprecated_services + self.logs_path = os.path.join("var/log", params.name) + self.log_path_overrides = params.log_path_overrides + self.exceptions = EXCEPTIONS_COMMON + OST_EXCEPTIONS.get( + params.name, + [] + ) @cached_property def installed(self): @@ -544,7 +583,8 @@ def default_masked_services(self): return sorted(masked) def add(self, name, *args, **kwargs): - self._projects[name] = OSTProject(name, *args, **kwargs) + params = OSTProjectParameters(name=name, *args, **kwargs) + self._projects[name] = OSTProject(params) @cached_property def packages_core_exprs(self): diff --git a/hotsos/core/ycheck/events.py b/hotsos/core/ycheck/events.py index a61ce9b53..9d4be8bea 100644 --- a/hotsos/core/ycheck/events.py +++ b/hotsos/core/ycheck/events.py @@ -1,5 +1,6 @@ import abc from functools import cached_property +from dataclasses import dataclass from hotsos.core.config import HotSOSConfig from hotsos.core.log import log @@ -54,34 +55,41 @@ def __init__(cls, _name, _mro, members): CALLBACKS[event] = cls -class EventCheckResult(): - """ This is passed to an event check callback when matches are found """ +@dataclass(frozen=True) +class EventCheckResult: + """ This is passed to an event check callback when matches are found. - def __init__(self, defs_section, defs_event, search_results, search_tag, - searcher, sequence_def=None): - """ - @param defs_section: section name from yaml - @param defs_event: event label/name from yaml - @param search_results: searchkit.SearchResultsCollection + @param name: event label/name from yaml + @param section_name: section name from yaml + @param results: searchkit.SearchResultsCollection @param search_tag: unique tag used to identify the results @param searcher: global FileSearcher object @param sequence_def: if set the search results are from a searchkit.SequenceSearchDef and are therefore grouped as sections of results rather than a single set of results. - """ - self.section = defs_section - self.name = defs_event - self.search_tag = search_tag - self.results = search_results - self.searcher = searcher - self.sequence_def = sequence_def + """ + name: str + section_name: str + results: object + search_tag: str + searcher: object + sequence_def: object = None class EventProcessingUtils(): """ A set of helper methods to help with the processing of event results. """ + + @dataclass() + class EventProcessingOptions: + """Common options for EventProcessingUtils functions.""" + key_by_date: bool = True + include_time: bool = False + squash_if_none_keys: bool = False + max_results_per_date: int = None + @classmethod def _get_event_results(cls, event): """ @@ -123,10 +131,9 @@ def _get_event_results(cls, event): "fields", _fail_count, event.name) - @classmethod - def _get_tally(cls, result, info, key_by_date, include_time, - squash_if_none_keys): - if key_by_date: + @staticmethod + def _get_tally(result, info, options: EventProcessingOptions): + if options.key_by_date: key = result['date'] value = result['key'] else: @@ -136,7 +143,7 @@ def _get_tally(cls, result, info, key_by_date, include_time, if key not in info: info[key] = {} - if value is None and squash_if_none_keys: + if value is None and options.squash_if_none_keys: if info[key] == {}: info[key] = 1 else: @@ -144,12 +151,12 @@ def _get_tally(cls, result, info, key_by_date, include_time, else: ts_time = result.get('time') if value not in info[key]: - if ts_time is not None and include_time: + if ts_time is not None and options.include_time: info[key][value] = {} else: info[key][value] = 0 - if ts_time is not None and include_time: + if ts_time is not None and options.include_time: if ts_time not in info[key][value]: info[key][value][ts_time] = 1 else: @@ -157,21 +164,21 @@ def _get_tally(cls, result, info, key_by_date, include_time, else: info[key][value] += 1 - @classmethod - def _sort_results(cls, categorised_results, key_by_date, include_time): + @staticmethod + def _sort_results(categorised_results, options: EventProcessingOptions): log.debug("sorting categorised results") - if all([key_by_date, include_time]): + if all([options.key_by_date, options.include_time]): return {} # sort main dict keys categorised_results = sorted_dict(categorised_results, - reverse=not key_by_date) + reverse=not options.key_by_date) # Sort values if they are dicts. for key, value in categorised_results.items(): # If not using date as key we need to sort the values so that they # will have the same order as if they were sorted by date key. - if not key_by_date: + if not options.key_by_date: categorised_results[key] = sorted_dict(value) continue @@ -199,9 +206,12 @@ def global_event_tally_time_granularity_override(cls): return False @classmethod - def categorise_events(cls, event, results=None, key_by_date=True, - include_time=False, squash_if_none_keys=False, - max_results_per_date=None): + def categorise_events( + cls, + event, + results=None, + options: EventProcessingOptions = None, + ): """ Provides a generic way to categorise events. The default is to group events by key which is typically some kind of resource id or event @@ -234,36 +244,40 @@ def categorise_events(cls, event, results=None, key_by_date=True, True this will pick the top N entries with the highest count. """ + if options is None: + options = EventProcessingUtils.EventProcessingOptions() + if results is None: results = cls._get_event_results(event) if cls.global_event_tally_time_granularity_override() is True: - include_time = HotSOSConfig.event_tally_granularity == 'time' + options.include_time = HotSOSConfig.event_tally_granularity == \ + "time" categorised_results = {} for r in results: - cls._get_tally(r, categorised_results, key_by_date, include_time, - squash_if_none_keys) + cls._get_tally(r, categorised_results, options) if not categorised_results: return {} - categorised_results = cls._sort_results(categorised_results, - key_by_date, include_time) - if not (key_by_date and max_results_per_date): + categorised_results = cls._sort_results(categorised_results, options) + if not (options.key_by_date and options.max_results_per_date): return categorised_results shortened = {} for date, entries in categorised_results.items(): - if len(entries) <= max_results_per_date: + if len(entries) <= options.max_results_per_date: shortened[date] = entries continue top_n = dict(sorted(entries.items(), key=lambda e: e[1], - reverse=True)[:max_results_per_date]) - shortened[date] = {'total': len(entries), - f"top{max_results_per_date}": top_n} + reverse=True)[:options.max_results_per_date]) + shortened[date] = { + "total": len(entries), + f"top{options.max_results_per_date}": top_n, + } return shortened @@ -484,14 +498,15 @@ def run(self): callback = CALLBACKS[callback_name] event_result = EventCheckResult( - section_name, - event, - search_results, - search_tag, - self.global_searcher.searcher, - sequence_def=seq_def) + name=event, + section_name=section_name, + results=search_results, + search_tag=search_tag, + searcher=self.global_searcher.searcher, + sequence_def=seq_def + ) log.debug("executing event %s.%s callback '%s'", - event_result.section, event, callback_name) + event_result.section_name, event, callback_name) ret = callback()(event_result) if not ret: continue diff --git a/hotsos/plugin_extensions/openstack/agent/events.py b/hotsos/plugin_extensions/openstack/agent/events.py index 5553b0763..c426b00cf 100644 --- a/hotsos/plugin_extensions/openstack/agent/events.py +++ b/hotsos/plugin_extensions/openstack/agent/events.py @@ -1,7 +1,10 @@ import datetime import yaml -from hotsos.core.analytics import LogEventStats, SearchResultIndices +from hotsos.core.analytics import ( + LogEventStats, + SearchResultIndices +) from hotsos.core.config import HotSOSConfig from hotsos.core.host_helpers import CLIHelper from hotsos.core.issues import ( @@ -139,7 +142,7 @@ def _get_event_stats(results, tag_prefix, custom_idxs=None): "stats": stats.get_event_stats()} def __call__(self, event): - agent = event.section + agent = event.section_name if event.name == 'ovsdb-monitor-router-binding-transitions': # The ovsdbmonitor will print bindings which will eventually # reflect transitions so we need to filter out contiguous bindings @@ -174,7 +177,10 @@ def __call__(self, event): elif event.name in self.ovsdbapp_event_names + \ self.ovn_mech_driver_events: if event.name == 'ovn-resource-revision-bump': - ret = self.categorise_events(event, max_results_per_date=5) + ret = self.categorise_events( + event, + options=self.EventProcessingOptions(max_results_per_date=5) + ) elif event.name == 'ovsdb-transaction-aborted': ret = {} for result in event.results: @@ -195,9 +201,9 @@ def __call__(self, event): if ret: return {event.name: ret}, agent else: - sri = SearchResultIndices(event_id_idx=4, - metadata_idx=3, - metadata_key='router') + sri = SearchResultIndices( + event_id=4, metadata=3, metadata_key="router" + ) ret = self._get_event_stats(event.results, event.search_tag, custom_idxs=sri) if ret: @@ -241,14 +247,19 @@ def __call__(self, event): results.append({'date': e.get(1), 'time': e.get(2), 'key': lb_id}) - ret = self.categorise_events(event, results=results, - key_by_date=False) + ret = self.categorise_events( + event, + results=results, + options=type(self).EventProcessingOptions(key_by_date=False) + ) if ret: failover_type = event.name.rpartition('-')[2] return {failover_type: ret}, 'lb-failovers' else: - missed_heartbeats = self.categorise_events(event, - key_by_date=False) + missed_heartbeats = self.categorise_events( + event, + options=type(self).EventProcessingOptions(key_by_date=False) + ) if not missed_heartbeats: return None @@ -314,12 +325,15 @@ def __call__(self, event): results = [{'date': f"{r.get(1)} {r.get(2)}", 'time': r.get(3), 'key': r.get(4)} for r in event.results] - ret = self.categorise_events(event, results=results, - key_by_date=False) + ret = self.categorise_events( + event, + results=results, + options=type(self).EventProcessingOptions(key_by_date=False), + ) if ret: # event.name must be the service name, event.section is the aa # action. - return {event.name: ret}, event.section + return {event.name: ret}, event.section_name return None @@ -379,8 +393,11 @@ def __call__(self, event): results.append({'date': r.get(1), 'time': r.get(2), 'key': router.uuid}) - transitions = self.categorise_events(event, results=results, - key_by_date=False) + transitions = self.categorise_events( + event, + results=results, + options=type(self).EventProcessingOptions(key_by_date=False), + ) if transitions: # run checks self.check_vrrp_transitions(transitions) diff --git a/hotsos/plugin_extensions/openstack/vm_info.py b/hotsos/plugin_extensions/openstack/vm_info.py index 70a1fdefa..053e154d4 100644 --- a/hotsos/plugin_extensions/openstack/vm_info.py +++ b/hotsos/plugin_extensions/openstack/vm_info.py @@ -153,7 +153,7 @@ def __call__(self, event): migration_info[vm_uuid] = [info] # section name expected to be live-migration - return migration_info, event.section + return migration_info, event.section_name class PrePostLiveMigrationCallback(OpenstackEventCallbackBase): @@ -186,7 +186,7 @@ def _migration_stats_info(event): def __call__(self, event): # section name expected to be live-migration - return self._migration_stats_info(event), event.section + return self._migration_stats_info(event), event.section_name class NovaServerMigrationAnalysis(OpenstackEventHandlerBase): diff --git a/hotsos/plugin_extensions/openvswitch/event_checks.py b/hotsos/plugin_extensions/openvswitch/event_checks.py index 905eda01b..e51db5593 100644 --- a/hotsos/plugin_extensions/openvswitch/event_checks.py +++ b/hotsos/plugin_extensions/openvswitch/event_checks.py @@ -15,7 +15,10 @@ class OVSEventCallbackVSwitchd(OpenvSwitchEventCallbackBase): event_names = ['bridge-no-such-device', 'netdev-linux-no-such-device'] def __call__(self, event): - ret = self.categorise_events(event, max_results_per_date=5) + ret = self.categorise_events( + event, + options=type(self).EventProcessingOptions(max_results_per_date=5) + ) if ret: return {event.name: ret}, 'ovs-vswitchd' @@ -33,14 +36,14 @@ class OVSEventCallbackLogs(OpenvSwitchEventCallbackBase): 'unreasonably-long-poll-interval'] def __call__(self, event): - key_by_date = True + options = type(self).EventProcessingOptions(squash_if_none_keys=True) + if event.name in ['ovs-vswitchd', 'ovsdb-server']: - key_by_date = False + options.key_by_date = False - ret = self.categorise_events(event, key_by_date=key_by_date, - squash_if_none_keys=True) + ret = self.categorise_events(event, options=options) if ret: - return {event.name: ret}, event.section + return {event.name: ret}, event.section_name return None @@ -54,9 +57,13 @@ def __call__(self, event): results = [{'date': f"{r.get(1)} {r.get(2)}", 'time': r.get(3), 'key': r.get(4)} for r in event.results] - ret = self.categorise_events(event, results=results, key_by_date=False) + ret = self.categorise_events( + event, + results=results, + options=type(self).EventProcessingOptions(key_by_date=False) + ) if ret: - return {event.name: ret}, event.section + return {event.name: ret}, event.section_name return None @@ -148,7 +155,7 @@ def __call__(self, event): for k in sorted(stats): stats_sorted[k] = stats[k] - output_key = f"{event.section}-port-stats" + output_key = f"{event.section_name}-port-stats" return stats_sorted, output_key return None @@ -225,7 +232,7 @@ def __call__(self, event): for key, value in aggregated.items(): aggregated[key] = sorted_dict(value) - return {event.name: aggregated}, event.section + return {event.name: aggregated}, event.section_name class OVSEventChecks(OpenvSwitchEventHandlerBase): @@ -248,15 +255,15 @@ class OVNEventCallbackLogs(OpenvSwitchEventCallbackBase): 'leadership-acquired'] def __call__(self, event): - key_by_date = True + options = type(self).EventProcessingOptions(squash_if_none_keys=True) + if event.name in ['ovsdb-server-nb', 'ovsdb-server-sb', 'ovn-northd', 'ovn-controller']: - key_by_date = False + options.key_by_date = False - ret = self.categorise_events(event, key_by_date=key_by_date, - squash_if_none_keys=True) + ret = self.categorise_events(event, options=options) if ret: - return {event.name: ret}, event.section + return {event.name: ret}, event.section_name return None @@ -283,7 +290,7 @@ def __call__(self, event): for key, value in aggregated.items(): aggregated[key] = sorted_dict(value) - return {event.name: sorted_dict(aggregated)}, event.section + return {event.name: sorted_dict(aggregated)}, event.section_name class OVNEventChecks(OpenvSwitchEventHandlerBase): diff --git a/hotsos/plugin_extensions/storage/ceph_event_checks.py b/hotsos/plugin_extensions/storage/ceph_event_checks.py index 1ebd79b0f..489655e89 100644 --- a/hotsos/plugin_extensions/storage/ceph_event_checks.py +++ b/hotsos/plugin_extensions/storage/ceph_event_checks.py @@ -25,8 +25,10 @@ def __call__(self, event): IssuesManager().add(CephOSDError(msg)) return None - key_by_date = event.name == 'heartbeat-no-reply' - return self.categorise_events(event, key_by_date=key_by_date) + options = type(self).EventProcessingOptions( + key_by_date=event.name == "heartbeat-no-reply" + ) + return self.categorise_events(event, options=options) class EventCallbackSlowRequests(CephEventCallbackBase): @@ -65,8 +67,8 @@ def __call__(self, event): results.append({'date': r.get(1), 'key': key}) - ret = self.categorise_events(event, results=results, - squash_if_none_keys=True) + options = type(self).EventProcessingOptions(squash_if_none_keys=True) + ret = self.categorise_events(event, results=results, options=options) # If on any particular day there were > 3 crc errors for a # particular osd we raise an issue since that indicates they are @@ -112,8 +114,8 @@ def __call__(self, event): results.append({'date': r.get(1), 'key': key}) - return self.categorise_events(event, results=results, - squash_if_none_keys=True) + options = type(self).EventProcessingOptions(squash_if_none_keys=True) + return self.categorise_events(event, results=results, options=options) class CephEventHandler(CephChecks, EventHandlerBase): diff --git a/pylintrc b/pylintrc index 85a654448..1e9e3689c 100644 --- a/pylintrc +++ b/pylintrc @@ -27,7 +27,6 @@ disable= protected-access, too-few-public-methods, too-many-ancestors, - too-many-arguments, too-many-branches, too-many-instance-attributes, too-many-locals, diff --git a/tests/unit/test_openvswitch.py b/tests/unit/test_openvswitch.py index d845a3590..ea55bddd6 100644 --- a/tests/unit/test_openvswitch.py +++ b/tests/unit/test_openvswitch.py @@ -188,10 +188,20 @@ def test_summary_ovs_tunnels_no_remotes(self): 'sos_commands/openvswitch/ovs-vsctl_-t_5_list_' 'Open_vSwitch': OVS_DB_GENEVE_ENCAP}) def test_summary_ovn_tunnels_no_remotes(self): - with mock.patch('hotsos.core.host_helpers.HostNetworkingHelper.' - 'host_interfaces_all', - [NetworkPort('bondX', ['10.3.4.24'], None, - None, None, None)]): + with mock.patch( + "hotsos.core.host_helpers.HostNetworkingHelper." + "host_interfaces_all", + [ + NetworkPort( + name="bondX", + addresses=["10.3.4.24"], + hwaddr=None, + state=None, + encap_info=None, + mtu=None, + ) + ], + ): expected = {'geneve': { 'iface': { 'bondX': {'addresses': ['10.3.4.24'], @@ -210,10 +220,20 @@ def test_summary_ovn_tunnels_no_remotes(self): 'sos_commands/openvswitch/ovs-vsctl_-t_5_list_' 'Open_vSwitch': OVS_DB_GENEVE_ENCAP}) def test_summary_tunnels_ovn(self): - with mock.patch('hotsos.core.host_helpers.HostNetworkingHelper.' - 'host_interfaces_all', - [NetworkPort('bondX', ['10.3.4.24'], None, - None, None, None)]): + with mock.patch( + "hotsos.core.host_helpers.HostNetworkingHelper." + "host_interfaces_all", + [ + NetworkPort( + name="bondX", + addresses=["10.3.4.24"], + hwaddr=None, + state=None, + encap_info=None, + mtu=None, + ) + ], + ): expected = {'geneve': { 'iface': { 'bondX': {'addresses': ['10.3.4.24'], diff --git a/tests/unit/test_plugintools.py b/tests/unit/test_plugintools.py index d21a77d0a..fad5b8bff 100644 --- a/tests/unit/test_plugintools.py +++ b/tests/unit/test_plugintools.py @@ -1,6 +1,6 @@ import json -from hotsos.client import OutputManager +from hotsos.client import OutputManager, OutputBuilder from hotsos.core.host_helpers.cli import CLIHelper from hotsos.core.issues import IssuesManager from hotsos.core import plugintools @@ -161,7 +161,7 @@ class TestPluginTools(utils.BaseTestCase): """ Unit tests for plugintools code. """ def test_summary_empty(self): - filtered = OutputManager().get() + filtered = OutputManager().get_builder().to(fmt="json") self.assertEqual(filtered, '{}') def test_summary_mode_short_legacy(self): @@ -174,8 +174,7 @@ def test_summary_mode_short_legacy(self): 'id': '1234', 'message': 'a msg'}]}} - filtered = OutputManager().minimise(ISSUES_LEGACY_FORMAT, - mode='short') + filtered = OutputBuilder._minimise(ISSUES_LEGACY_FORMAT, mode='short') self.assertEqual(filtered, expected) def test_summary_mode_short(self): @@ -186,7 +185,7 @@ def test_summary_mode_short(self): 'testplugin': [{ 'id': '1234', 'message': 'a msg'}]}} - filtered = OutputManager().minimise(ISSUES_NEW_FORMAT, mode='short') + filtered = OutputBuilder._minimise(ISSUES_NEW_FORMAT, mode='short') self.assertEqual(filtered, expected) def test_summary_mode_very_short_legacy(self): @@ -195,8 +194,8 @@ def test_summary_mode_very_short_legacy(self): 'MemoryWarning': 1}}, IssuesManager.SUMMARY_OUT_BUGS_ROOT: { 'testplugin': ['1234']}} - filtered = OutputManager().minimise(ISSUES_LEGACY_FORMAT, - mode='very-short') + filtered = OutputBuilder._minimise(ISSUES_LEGACY_FORMAT, + mode='very-short') self.assertEqual(filtered, expected) def test_summary_mode_very_short(self): @@ -206,18 +205,13 @@ def test_summary_mode_very_short(self): IssuesManager.SUMMARY_OUT_BUGS_ROOT: { 'testplugin': ['1234']}} - filtered = OutputManager().minimise(ISSUES_NEW_FORMAT, - mode='very-short') + filtered = OutputBuilder._minimise(ISSUES_NEW_FORMAT, + mode='very-short') self.assertEqual(filtered, expected) - def test_apply_output_formatting_defaults(self): - summary = {'opt': 'value'} - filtered = OutputManager(summary).get() - self.assertEqual(filtered, plugintools.yaml_dump(summary)) - def test_apply_output_formatting_json(self): summary = {'opt': 'value'} - filtered = OutputManager(summary).get(fmt='json') + filtered = OutputManager(summary).get_builder().to("json") self.assertEqual(filtered, json.dumps(summary, indent=2, sort_keys=True)) @@ -256,7 +250,7 @@ def test_apply_output_formatting_markdown(self): - a - b - c''' - filtered = OutputManager(summary).get(fmt='markdown') + filtered = OutputManager(summary).get_builder().to(fmt="markdown") self.assertEqual(filtered, expected) def test_apply_output_formatting_html_1(self): @@ -276,7 +270,7 @@ def test_apply_output_formatting_html_1(self): ['a', 'b', 'c'], } expected = htmlout.header + HTML1 + htmlout.footer - filtered = OutputManager(summary).get(fmt='html') + filtered = OutputManager(summary).get_builder().to(fmt="html") self.assertEqual(filtered, expected) def test_apply_output_formatting_html_2(self): @@ -296,7 +290,7 @@ def test_apply_output_formatting_html_2(self): ['a', 'b', 'c'], } expected = htmlout.header + HTML2 + htmlout.footer - filtered = OutputManager(summary).get(fmt='html') + filtered = OutputManager(summary).get_builder().to(fmt="html") self.assertEqual(filtered, expected) def test_apply_output_formatting_html_3(self): @@ -316,5 +310,5 @@ def test_apply_output_formatting_html_3(self): ['a', 'b', 'c'], } expected = htmlout.header + HTML3 + htmlout.footer - filtered = OutputManager(summary).get(fmt='html') + filtered = OutputManager(summary).get_builder().to(fmt="html") self.assertEqual(filtered, expected) diff --git a/tests/unit/test_ut_scenario_loader.py b/tests/unit/test_ut_scenario_loader.py index 797de0284..8958c7a58 100644 --- a/tests/unit/test_ut_scenario_loader.py +++ b/tests/unit/test_ut_scenario_loader.py @@ -220,8 +220,10 @@ def test_is_def_filter(self): fd.write(FAKE_SCENARIO) with mock.patch.object(utils, 'DEFS_TESTS_DIR', dtmp): utils.TemplatedTest( - os.path.join(dtmp, 'test1.yaml'), - {}, [], [], [], '/')()(self) + target_path=os.path.join(dtmp, "test1.yaml"), + data_root={}, + sub_root="/", + )()(self) def test_scenarios(self): with tempfile.TemporaryDirectory() as dtmp, \ diff --git a/tests/unit/utils.py b/tests/unit/utils.py index c2c016c8c..42ca8ce98 100644 --- a/tests/unit/utils.py +++ b/tests/unit/utils.py @@ -6,6 +6,7 @@ import tempfile import unittest from unittest import mock +from dataclasses import dataclass, field import yaml from hotsos.core.config import HotSOSConfig @@ -75,6 +76,7 @@ def _inner(cls): return _inner +@dataclass(frozen=True) class TemplatedTest(): """ Helper for working with templated tests i.e. Unit tests written in yaml @@ -82,23 +84,27 @@ class TemplatedTest(): This class implements the functionality to process the test results. """ - def __init__(self, target_path, data_root, mocks, expected_bugs, - expected_issues, sub_root): - self.sub_root = sub_root - self.target_path = target_path + target_path: str + data_root: dict + sub_root: str + mocks: dict = field(default_factory=lambda: {}) + expected_bugs: dict = field(default_factory=lambda: {}) + expected_issues: dict = field(default_factory=lambda: {}) + + def __post_init__(self): if not os.path.exists(self.target_scenario_path): raise FileNotFoundError("Scenario file " f"{self.target_scenario_path}" " not found!") - self.data_root = data_root - self.mocks = mocks - self.expected_bugs = expected_bugs - self.expected_issues = expected_issues @property def target_scenario_path(self): - return os.path.join(DEFS_DIR, self.sub_root, self.target_path) + return os.path.join( + DEFS_DIR, + self.sub_root, + self.target_path + ) @staticmethod def check_raised_bugs(test_inst, expected, actual): @@ -172,7 +178,8 @@ def __call__(self): @create_data_root(self.data_root.get('files'), self.data_root.get('copy-from-original')) @mock.patch('hotsos.core.ycheck.engine.YDefsLoader._is_def', - new=is_def_filter(self.target_path, self.sub_root)) + new=is_def_filter(self.target_path, + self.sub_root)) def inner(test_inst): patch_contexts = [] if 'patch' in self.mocks: @@ -184,7 +191,8 @@ def inner(test_inst): c.start() if 'patch.object' in self.mocks: - for target, patch_params in self.mocks['patch.object'].items(): + for target, patch_params in \ + self.mocks['patch.object'].items(): mod, _, cls_name = target.rpartition('.') obj = getattr(importlib.import_module(mod), cls_name) patch_args = patch_params.get('args', []) @@ -204,9 +212,16 @@ def inner(test_inst): for c in patch_contexts: c.stop() - self.check_raised_bugs(test_inst, self.expected_bugs, raised_bugs) - self.check_raised_issues(test_inst, self.expected_issues, - raised_issues) + self.check_raised_bugs( + test_inst, + self.expected_bugs, + raised_bugs + ) + self.check_raised_issues( + test_inst, + self.expected_issues, + raised_issues + ) return inner @@ -269,12 +284,14 @@ def test_method_name(self): def _generate(self): """ Generate a test from a template. """ - data_root = self.testdef.get('data-root', {}) - mocks = self.testdef.get('mock', {}) - bugs = self.testdef.get('raised-bugs') - issues = self.testdef.get('raised-issues') - return TemplatedTest(self.target_path, data_root, mocks, bugs, - issues, self.test_defs_root)() + return TemplatedTest( + target_path=self.target_path, + sub_root=self.test_defs_root, + data_root=self.testdef.get("data-root", {}), + mocks=self.testdef.get("mock", {}), + expected_bugs=self.testdef.get("raised-bugs"), + expected_issues=self.testdef.get("raised-issues"), + )() def expand_log_template(template, hours=None, mins=None, secs=None, diff --git a/tests/unit/ycheck/test_events.py b/tests/unit/ycheck/test_events.py index e48e78901..d6fa3c2ae 100644 --- a/tests/unit/ycheck/test_events.py +++ b/tests/unit/ycheck/test_events.py @@ -288,15 +288,16 @@ def test_processing_utils_key_by_date_true(self): {'date': '2000-01-01', 'key': 'f3'}, {'date': '2000-01-02', 'key': 'f2'}] for r in results: - EventProcessingUtils._get_tally(r, info, key_by_date=True, - include_time=False, - squash_if_none_keys=False) + EventProcessingUtils._get_tally( + r, info, options=EventProcessingUtils.EventProcessingOptions() + ) self.assertEqual(info, {'2000-01-04': {'f4': 1}, '2000-01-01': {'f1': 1, 'f3': 1}, '2000-01-02': {'f2': 1}}) - ret = EventProcessingUtils._sort_results(info, key_by_date=True, - include_time=False,) + ret = EventProcessingUtils._sort_results( + info, options=EventProcessingUtils.EventProcessingOptions() + ) expected = {'2000-01-01': {'f1': 1, 'f3': 1}, '2000-01-02': {'f2': 1}, @@ -313,16 +314,21 @@ def test_processing_utils_key_by_date_false(self): {'date': '2000-01-03', 'key': 'f3'}, {'date': '2000-01-02', 'key': 'f2'}] for r in results: - EventProcessingUtils._get_tally(r, info, key_by_date=False, - include_time=False, - squash_if_none_keys=False) + EventProcessingUtils._get_tally( + r, + info, + options=EventProcessingUtils.EventProcessingOptions( + key_by_date=False), + ) self.assertEqual(info, {'f1': {'2000-01-01': 1}, 'f2': {'2000-01-02': 1}, 'f3': {'2000-01-01': 1, '2000-01-03': 1}, 'f4': {'2000-01-04': 1}}) - ret = EventProcessingUtils._sort_results(info, key_by_date=False, - include_time=False,) + ret = EventProcessingUtils._sort_results( + info, options=EventProcessingUtils.EventProcessingOptions( + key_by_date=False) + ) expected = {'f4': {'2000-01-04': 1}, 'f3': {'2000-01-01': 1, '2000-01-03': 1}, diff --git a/tests/unit/ycheck/test_scenarios.py b/tests/unit/ycheck/test_scenarios.py index 584e1c3ba..253738f11 100644 --- a/tests/unit/ycheck/test_scenarios.py +++ b/tests/unit/ycheck/test_scenarios.py @@ -241,6 +241,7 @@ def test_yaml_def_mapped_overrides(self, global_searcher): @mock.patch('hotsos.core.ycheck.engine.properties.common.log') @utils.init_test_scenario(test_data.SCENARIO_W_ERROR) @utils.global_search_context + # pylint: disable-next=too-many-arguments def test_failed_scenario_caught(self, global_searcher, mock_log1, mock_log2, _mock_log3, mock_log4, mock_log5, mock_log6): @@ -353,6 +354,7 @@ def test_vars(self, global_searcher): @mock.patch('hotsos.core.ycheck.engine.properties.common.log') @utils.init_test_scenario(test_data.LOGIC_TEST) @utils.global_search_context + # pylint: disable-next=too-many-arguments def test_logical_collection_and_with_fail(self, global_searcher, mock_log1, mock_log2, _mock_log3, mock_log4, mock_log5, mock_log6,