code refactoring

RedHatInsights · Jan 31, 2025 · 55e62a0 · 55e62a0
1 parent d6b8472
commit 55e62a0
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 89 deletions.
diff --git a/ros/processor/suggestions_engine.py b/ros/processor/suggestions_engine.py
@@ -21,19 +21,6 @@
 
 logging = get_logger(__name__)
 
-PCP_METRICS = [
-        'disk.dev.total',
-        'hinv.ncpu',
-        'kernel.all.cpu.idle',
-        'kernel.all.pressure.cpu.some.avg',
-        'kernel.all.pressure.io.full.avg',
-        'kernel.all.pressure.io.some.avg',
-        'kernel.all.pressure.memory.full.avg',
-        'kernel.all.pressure.memory.some.avg',
-        'mem.physmem',
-        'mem.util.available',
-]
-
 
 class SuggestionsEngine:
     def __init__(self):
@@ -133,45 +120,56 @@ def create_output_dir(request_id, host):
     return output_dir
 
 
+def run_pmlogextract(service, event, host, index_file_path, output_dir):
+    """Run the pmlogextract command."""
+
+    pmlogextract_command = [
+        "pmlogextract",
+        "-c",
+        "ros/lib/pcp_extract_config",
+        index_file_path,
+        output_dir
+    ]
+
+    logging.debug(f"{service} - {event} - Running pmlogextract command for system {host.get('id')}.")
+    try:
+        subprocess.run(pmlogextract_command, check=True, )
+        logging.debug(f"{service} - {event} - Successfully ran pmlogextract command for system {host.get('id')}.")
+    except subprocess.CalledProcessError as error:
+        logging.error(
+            f"{service} - {event} - Error running pmlogextract command for system {host.get('id')}: {error.stdout}"
+        )
+        raise
+
+
+def run_pmlogsummary(service, event, host, output_dir):
+    """Run the pmlogsummary command."""
+
+    pmlogsummary_command = [
+        "pmlogsummary",
+        "-F",
+        os.path.join(output_dir, ".index")
+    ]
+
+    logging.debug(f"{service} - {event} - Running pmlogsummary command for system {host.get('id')}.")
+    try:
+        subprocess.run(pmlogsummary_command, check=True, text=True, capture_output=True)
+        logging.debug(f"{service} - {event} - Successfully ran pmlogsummary command for system {host.get('id')}.")
+    except subprocess.CalledProcessError as error:
+        logging.error(
+            f"{service} - {event} - Error running pmlogsummary command for system {host.get('id')}: {error.stdout}"
+        )
+        raise
+
+
 def run_pcp_commands(service, event, host, index_file_path, request_id):
     try:
         sanitized_request_id = request_id.replace("/", "_")
         output_dir = create_output_dir(sanitized_request_id, host)
 
-        pmlogextract_command = [
-            "pmlogextract",
-            "-c",
-            "ros/lib/pcp_extract_config",
-            index_file_path,
-            output_dir
-        ]
+        run_pmlogextract(service, event, host, index_file_path, output_dir)
 
-        logging.info(f"{service} - {event} - Running pmlogextract command for system {host.get('id')}.")
-        try:
-            subprocess.run(pmlogextract_command, check=True)
-            logging.info(f"{service} - {event} - Successfully ran pmlogextract command for system {host.get('id')}.")
-        except subprocess.CalledProcessError as error:
-            logging.error(
-                f"{service} - {event} - Error running pmlogextract command for system {host.get('id')}: {error.stdout}"
-            )
-            return
-
-        pmlogsummary_command = [
-            "pmlogsummary",
-            "-f",
-            os.path.join(output_dir, ".index"),
-            *PCP_METRICS
-        ]
-
-        logging.info(f"{service} - {event} - Running pmlogsummary command for system {host.get('id')}.")
-        try:
-            subprocess.run(pmlogsummary_command, check=True, capture_output=True, text=True)
-            logging.info(f"{service} - {event} - Successfully ran pmlogsummary command for system {host.get('id')}.")
-        except subprocess.CalledProcessError as error:
-            logging.error(
-                f"{service} - {event} - Error running pmlogsummary command for system {host.get('id')}: {error.stdout}"
-            )
-            return
+        run_pmlogsummary(service, event, host, output_dir)
 
     except Exception as error:
         logging.error(
@@ -181,7 +179,7 @@ def run_pcp_commands(service, event, host, index_file_path, request_id):
         try:
             if os.path.exists(output_dir):
                 shutil.rmtree(output_dir)
-                logging.info(f"{service} - {event} - Cleaned up output directory for system {host.get('id')}.")
+                logging.debug(f"{service} - {event} - Cleaned up output directory for system {host.get('id')}.")
         except Exception as error:
             logging.error(
                 f"{service} - {event} - Error cleaning up the output directory for system {host.get('id')}: {error}"
@@ -190,7 +188,7 @@ def run_pcp_commands(service, event, host, index_file_path, request_id):
 
 @contextmanager
 def download_and_extract(service, event, archive_URL, host, org_id):
-    logging.info(f"{service} - {event} - Downloading the report for system {host.get('id')}.")
+    logging.debug(f"{service} - {event} - Downloading the report for system {host.get('id')}.")
 
     try:
         response = requests.get(archive_URL, timeout=10)

diff --git a/tests/test_suggestions_engine.py b/tests/test_suggestions_engine.py
@@ -8,8 +8,7 @@
     download_and_extract,
     get_index_file_path,
     create_output_dir,
-    find_root_directory,
-    PCP_METRICS
+    find_root_directory
 )
 
 
@@ -159,44 +158,5 @@ def test_create_output_dir(self, mock_path_exists, mock_makedirs):
         self.assertEqual(output_dir, "/tmp/pmlogextract-output-12345/")
 
 
-class TestPCPMetrics(unittest.TestCase):
-    def test_pcp_metrics_check(self):
-        expected_metrics = [
-            "disk.dev.total",
-            "hinv.ncpu",
-            "kernel.all.cpu.idle",
-            "kernel.all.pressure.cpu.some.avg",
-            "kernel.all.pressure.io.full.avg",
-            "kernel.all.pressure.io.some.avg",
-            "kernel.all.pressure.memory.full.avg",
-            "kernel.all.pressure.memory.some.avg",
-            "mem.physmem",
-            "mem.util.available",
-        ]
-        self.assertEqual(PCP_METRICS, expected_metrics)
-
-    def test_pcp_metrics_in_command(self):
-        output_dir = "/tmp/pmlogextract-output-12345"
-        command = ["pmlogsummary", "-f", output_dir, *PCP_METRICS]
-
-        expected_command = [
-            "pmlogsummary",
-            "-f",
-            output_dir,
-            "disk.dev.total",
-            "hinv.ncpu",
-            "kernel.all.cpu.idle",
-            "kernel.all.pressure.cpu.some.avg",
-            "kernel.all.pressure.io.full.avg",
-            "kernel.all.pressure.io.some.avg",
-            "kernel.all.pressure.memory.full.avg",
-            "kernel.all.pressure.memory.some.avg",
-            "mem.physmem",
-            "mem.util.available",
-        ]
-
-        self.assertEqual(command, expected_command)
-
-
 if __name__ == '__main__':
     unittest.main()