Merge pull request #181 from amd/alex_no_console_log

No console logging: --no-console-log

Author: alexandraBara

nodescraper/cli/cli.py

@@ -161,6 +161,14 @@ def build_parser(
         help="Change python log level",
     )
 
+    parser.add_argument(
+        "--no-console-log",
+        action="store_true",
+        help="Write logs only to nodescraper.log under the run directory; do not print to stdout. "
+        "If no run log directory would be created (e.g. --log-path None), uses ./scraper_logs_<host>_<timestamp>/ "
+        "like the default layout.",
+    )
+
     parser.add_argument(
         "--gen-reference-config",
         dest="reference_config",
@@ -316,42 +324,55 @@ def build_parser(
             parser_builder = DynamicParserBuilder(plugin_subparser, plugin_class)
             model_type_map = parser_builder.build_plugin_parser()
         except Exception as e:
-            print(f"Exception building arg parsers for {plugin_name}: {str(e)}")  # noqa: T201
+            logging.getLogger(DEFAULT_LOGGER).error(
+                "Exception building arg parsers for %s: %s", plugin_name, e, exc_info=True
+            )
             continue
         plugin_subparser_map[plugin_name] = (plugin_subparser, model_type_map)
 
     return parser, plugin_subparser_map
 
 
-def setup_logger(log_level: str = "INFO", log_path: Optional[str] = None) -> logging.Logger:
+def setup_logger(
+    log_level: str = "INFO",
+    log_path: Optional[str] = None,
+    *,
+    console: bool = True,
+) -> logging.Logger:
     """set up root logger when using the CLI
 
     Args:
         log_level (str): log level to use
         log_path (Optional[str]): optional path to filesystem log location
+        console (bool): if False, omit the stdout StreamHandler (file-only when log_path is set)
 
     Returns:
         logging.Logger: logger intstance
     """
-    log_level = getattr(logging, log_level, "INFO")
+    log_level_no = getattr(logging, log_level, logging.INFO)
 
-    handlers = [logging.StreamHandler(stream=sys.stdout)]
+    handlers: list[logging.Handler] = []
+    if console:
+        handlers.append(logging.StreamHandler(stream=sys.stdout))
 
     if log_path:
         log_file_name = os.path.join(log_path, "nodescraper.log")
         handlers.append(
             logging.FileHandler(filename=log_file_name, mode="wt", encoding="utf-8"),
         )
 
+    if not handlers:
+        handlers.append(logging.NullHandler())
+
     logging.basicConfig(
         force=True,
-        level=log_level,
+        level=log_level_no,
         format="%(asctime)25s %(levelname)10s %(name)25s | %(message)s",
         datefmt="%Y-%m-%d %H:%M:%S %Z",
         handlers=handlers,
         encoding="utf-8",
     )
-    logging.root.setLevel(logging.INFO)
+    logging.root.setLevel(log_level_no)
     logging.getLogger("paramiko").setLevel(logging.ERROR)
 
     logger = logging.getLogger(DEFAULT_LOGGER)
@@ -391,11 +412,7 @@ def main(arg_input: Optional[list[str]] = None):
         sname = system_info.name.lower().replace("-", "_").replace(".", "_")
         timestamp = datetime.datetime.now().strftime("%Y_%m_%d-%I_%M_%S_%p")
 
-        if parsed_args.log_path and parsed_args.subcmd not in [
-            "gen-plugin-config",
-            "describe",
-            "compare-runs",
-        ]:
+        if parsed_args.log_path:
             log_path = os.path.join(
                 parsed_args.log_path,
                 f"scraper_logs_{sname}_{timestamp}",
@@ -404,7 +421,16 @@ def main(arg_input: Optional[list[str]] = None):
         else:
             log_path = None
 
-        logger = setup_logger(parsed_args.log_level, log_path)
+        if parsed_args.no_console_log and not log_path:
+            base_dir = parsed_args.log_path if parsed_args.log_path else "."
+            log_path = os.path.join(base_dir, f"scraper_logs_{sname}_{timestamp}")
+            os.makedirs(log_path, exist_ok=True)
+
+        logger = setup_logger(
+            parsed_args.log_level,
+            log_path,
+            console=not parsed_args.no_console_log,
+        )
         if log_path:
             logger.info("Log path: %s", log_path)
 
@@ -416,7 +442,12 @@ def main(arg_input: Optional[list[str]] = None):
             )
 
         if parsed_args.subcmd == "summary":
-            generate_summary(parsed_args.search_path, parsed_args.output_path, logger)
+            generate_summary(
+                parsed_args.search_path,
+                parsed_args.output_path,
+                logger,
+                artifact_dir=log_path,
+            )
             sys.exit(0)
 
         if parsed_args.subcmd == "describe":
@@ -431,6 +462,7 @@ def main(arg_input: Optional[list[str]] = None):
                 skip_plugins=getattr(parsed_args, "skip_plugins", None) or [],
                 include_plugins=getattr(parsed_args, "include_plugins", None),
                 truncate_message=not getattr(parsed_args, "dont_truncate", False),
+                artifact_dir=log_path,
             )
             sys.exit(0)
 
@@ -463,7 +495,7 @@ def main(arg_input: Optional[list[str]] = None):
                         "Could not read OEMDiagnosticDataType@Redfish.AllowableValues from LogService"
                     )
                     sys.exit(1)
-                print(json.dumps(allowable, indent=2))  # noqa: T201
+                logger.info("%s", json.dumps(allowable, indent=2))
             finally:
                 conn.close()
             sys.exit(0)
@@ -474,10 +506,8 @@ def main(arg_input: Optional[list[str]] = None):
                 ref_config = generate_reference_config_from_logs(
                     parsed_args.reference_config_from_logs, plugin_reg, logger
                 )
-                output_path = os.getcwd()
-                if parsed_args.output_path:
-                    output_path = parsed_args.output_path
-                path = os.path.join(output_path, "reference_config.json")
+                out_dir = log_path if log_path else parsed_args.output_path
+                path = os.path.join(out_dir, "reference_config.json")
                 try:
                     with open(path, "w") as f:
                         json.dump(
@@ -490,7 +520,9 @@ def main(arg_input: Optional[list[str]] = None):
                     logger.error(exp)
                 sys.exit(0)
 
-            parse_gen_plugin_config(parsed_args, plugin_reg, config_reg, logger)
+            parse_gen_plugin_config(
+                parsed_args, plugin_reg, config_reg, logger, artifact_dir=log_path
+            )
 
         parsed_plugin_args = {}
         for plugin, plugin_args in plugin_arg_map.items():

nodescraper/cli/compare_runs.py

@@ -25,6 +25,7 @@
 ###############################################################################
 import json
 import logging
+import os
 import re
 import sys
 from pathlib import Path
@@ -359,6 +360,7 @@ def run_compare_runs(
     include_plugins: Optional[Sequence[str]] = None,
     output_path: Optional[str] = None,
     truncate_message: bool = True,
+    artifact_dir: Optional[str] = None,
 ) -> None:
     """Compare datamodels from two run log directories and log results.
 
@@ -369,8 +371,10 @@ def run_compare_runs(
         logger: Logger for output.
         skip_plugins: Optional list of plugin names to exclude from comparison.
         include_plugins: Optional list of plugin names to include; if set, only these are compared.
-        output_path: Optional path for full diff report; default is <path1>_<path2>_diff.txt.
+        output_path: Optional path for full diff report; default is <path1>_<path2>_diff.txt
+            in the current directory, or under artifact_dir when set.
         truncate_message: If True, truncate message text and show only first 3 errors; if False, show full text and all.
+        artifact_dir: When set and output_path is not, write the diff file inside this directory (e.g. CLI run log dir).
     """
     p1 = Path(path1)
     p2 = Path(path2)
@@ -482,11 +486,11 @@ def run_compare_runs(
 
     out_file = output_path
     if not out_file:
-        out_file = f"{Path(path1).name}_{Path(path2).name}_diff.txt"
+        basename = f"{Path(path1).name}_{Path(path2).name}_diff.txt"
+        out_file = os.path.join(artifact_dir, basename) if artifact_dir else basename
     full_report = _build_full_diff_report(path1, path2, data1, data2, all_plugins)
     Path(out_file).write_text(full_report, encoding="utf-8")
     logger.info("Full diff report written to: %s", out_file)
 
     table_summary = TableSummary(logger=logger)
     table_summary.collate_results(plugin_results=plugin_results, connection_results=[])
-    print(f"Diff file written to {out_file}")  # noqa: T201

nodescraper/cli/helper.py

@@ -31,7 +31,7 @@
 import os
 import sys
 from pathlib import Path
-from typing import Optional, Tuple
+from typing import Optional, Sequence, Tuple
 
 from pydantic import BaseModel
 
@@ -187,6 +187,13 @@ def build_config(
     return config
 
 
+def log_cli_text_block(logger: logging.Logger, lines: Sequence[str]) -> None:
+    """Emit user-facing multi-line text through logging (respects handlers / --no-console-log)."""
+    text = "\n".join(lines).rstrip("\n")
+    if text:
+        logger.info("%s", text)
+
+
 def parse_describe(
     parsed_args: argparse.Namespace,
     plugin_reg: PluginRegistry,
@@ -202,35 +209,44 @@ def parse_describe(
         logger (logging.Logger): logger instance
     """
     if not parsed_args.name:
+        out: list[str] = []
         if parsed_args.type == "config":
-            print("Available built-in configs:")  # noqa: T201
+            out.append("Available built-in configs:")
             for name in config_reg.configs:
-                print(f"  {name}")  # noqa: T201
+                out.append(f"  {name}")
         elif parsed_args.type == "plugin":
-            print("Available plugins:")  # noqa: T201
+            out.append("Available plugins:")
             for name in plugin_reg.plugins:
-                print(f"  {name}")  # noqa: T201
-        print(f"\nUsage: describe {parsed_args.type} <name>")  # noqa: T201
+                out.append(f"  {name}")
+        out.append("")
+        out.append(f"Usage: describe {parsed_args.type} <name>")
+        log_cli_text_block(logger, out)
         sys.exit(0)
 
     if parsed_args.type == "config":
         if parsed_args.name not in config_reg.configs:
             logger.error("No config found for name: %s", parsed_args.name)
             sys.exit(1)
         config_model = config_reg.configs[parsed_args.name]
-        print(f"Config Name: {parsed_args.name}")  # noqa: T201
-        print(f"Description: {getattr(config_model, 'desc', '')}")  # noqa: T201
-        print("Plugins:")  # noqa: T201
+        out = [
+            f"Config Name: {parsed_args.name}",
+            f"Description: {getattr(config_model, 'desc', '')}",
+            "Plugins:",
+        ]
         for plugin in getattr(config_model, "plugins", []):
-            print(f"\t{plugin}")  # noqa: T201
+            out.append(f"\t{plugin}")
+        log_cli_text_block(logger, out)
 
     elif parsed_args.type == "plugin":
         if parsed_args.name not in plugin_reg.plugins:
             logger.error("No plugin found for name: %s", parsed_args.name)
             sys.exit(1)
         plugin_class = plugin_reg.plugins[parsed_args.name]
-        print(f"Plugin Name: {parsed_args.name}")  # noqa: T201
-        print(f"Description: {getattr(plugin_class, '__doc__', '')}")  # noqa: T201
+        out = [
+            f"Plugin Name: {parsed_args.name}",
+            f"Description: {getattr(plugin_class, '__doc__', '')}",
+        ]
+        log_cli_text_block(logger, out)
 
     sys.exit(0)
 
@@ -240,6 +256,7 @@ def parse_gen_plugin_config(
     plugin_reg: PluginRegistry,
     config_reg: ConfigRegistry,
     logger: logging.Logger,
+    artifact_dir: Optional[str] = None,
 ):
     """parse 'gen_plugin_config' cmd line argument
 
@@ -248,6 +265,7 @@ def parse_gen_plugin_config(
         plugin_reg (PluginRegistry): plugin registry instance
         config_reg (ConfigRegistry): config registry instance
         logger (logging.Logger): logger instance
+        artifact_dir (Optional[str]): if set, write the config under this directory (CLI run log dir)
     """
     try:
         config = build_config(
@@ -256,7 +274,8 @@ def parse_gen_plugin_config(
 
         config.name = parsed_args.config_name.split(".")[0]
         config.desc = "Auto generated config"
-        output_path = os.path.join(parsed_args.output_path, parsed_args.config_name)
+        out_dir = artifact_dir if artifact_dir else parsed_args.output_path
+        output_path = os.path.join(out_dir, parsed_args.config_name)
         with open(output_path, "w", encoding="utf-8") as out_file:
             out_file.write(config.model_dump_json(indent=2))
 
@@ -576,13 +595,19 @@ def dump_to_csv(all_rows: list, filename: str, fieldnames: list[str], logger: lo
     logger.info("Data written to csv file: %s", filename)
 
 
-def generate_summary(search_path: str, output_path: Optional[str], logger: logging.Logger):
+def generate_summary(
+    search_path: str,
+    output_path: Optional[str],
+    logger: logging.Logger,
+    artifact_dir: Optional[str] = None,
+):
     """Concatenate csv files into 1 summary csv file
 
     Args:
         search_path (str): Path for previous runs
-        output_path (Optional[str]): Path for new summary csv file
+        output_path (Optional[str]): Directory for new summary.csv (ignored when artifact_dir is set)
         logger (logging.Logger): instance of logger
+        artifact_dir (Optional[str]): if set, write summary.csv under this directory (CLI run log dir)
     """
 
     fieldnames = ["nodename", "plugin", "status", "timestamp", "message"]
@@ -606,8 +631,6 @@ def generate_summary(search_path: str, output_path: Optional[str], logger: loggi
         logger.error("No data rows found in matched CSV files.")
         return
 
-    if not output_path:
-        output_path = os.getcwd()
-
-    output_path = os.path.join(output_path, "summary.csv")
-    dump_to_csv(all_rows, output_path, fieldnames, logger)
+    base_dir = artifact_dir if artifact_dir else (output_path or os.getcwd())
+    out_file = os.path.join(base_dir, "summary.csv")
+    dump_to_csv(all_rows, out_file, fieldnames, logger)

nodescraper/models/taskresult.py

@@ -209,6 +209,18 @@ def log_result(self, log_path: str) -> None:
             with open(event_log, "w", encoding="utf-8") as log_file:
                 json.dump(all_events, log_file, indent=2)
 
+    @staticmethod
+    def _event_occurrence_count(event: Event) -> int:
+        """Occurrences represented by one event (RegexAnalyzer groups repeats in data['count'])."""
+        raw = event.data.get("count")
+        if raw is None:
+            return 1
+        try:
+            n = int(raw)
+        except (TypeError, ValueError):
+            return 1
+        return max(1, n)
+
     def _get_event_summary(self) -> str:
         """Get summary string for events
 
@@ -219,12 +231,13 @@ def _get_event_summary(self) -> str:
         warning_msg_counts: dict[str, int] = {}
 
         for event in self.events:
+            n = self._event_occurrence_count(event)
             if event.priority == EventPriority.WARNING:
                 warning_msg_counts[event.description] = (
-                    warning_msg_counts.get(event.description, 0) + 1
+                    warning_msg_counts.get(event.description, 0) + n
                 )
             elif event.priority >= EventPriority.ERROR:
-                error_msg_counts[event.description] = error_msg_counts.get(event.description, 0) + 1
+                error_msg_counts[event.description] = error_msg_counts.get(event.description, 0) + n
 
         summary_parts = []