Merge pull request #32 from redhat-performance/feat/RPOPC-1304-fio-multi-metric

RPOPC-1304: Add multi-metric support for FIO benchmark

Author: grdumas

src/chronicler/processors/fio_processor.py

@@ -8,7 +8,7 @@
 
 from .base_processor import BaseProcessor, ProcessorError
 from ..schema import (
-    Run, TimeSeriesPoint, TimeSeriesSummary, PrimaryMetric,
+    Run, TimeSeriesPoint, TimeSeriesSummary, PrimaryMetric, StatisticalSummary,
     create_run_key, create_sequence_key
 )
 from ..utils.parser_utils import (
@@ -77,42 +77,78 @@ class FioProcessor(BaseProcessor):
     def get_test_name(self) -> str:
         return "fio"
 
-    def build_results(self) -> Any:
+    def _extract_primary_metrics(
+        self, runs: Dict[str, Any],
+        overall_stats: Optional[StatisticalSummary]
+    ) -> Optional[List[PrimaryMetric]]:
         """
-        Build Results object with overall primary metric.
+        Extract bandwidth, IOPS, and latency as coequal primary metrics.
 
-        Primary metric is the maximum bandwidth achieved across all workloads.
+        FIO is a multi-metric benchmark measuring disk I/O performance across three
+        dimensions. All three metrics are equally important for characterizing performance.
 
-        Returns:
-            Results object
+        Returns list of PrimaryMetric objects for: bandwidth, iops, latency.
         """
-        # Call parent to build basic Results object
-        results = super().build_results()
-
-        if not results or not results.runs:
-            return results
-
-        # Find the run with the highest bandwidth
-        max_bw = 0
-        max_bw_run = None
+        if not runs:
+            return None
+
+        # Collect metric values from all runs
+        bandwidth_values = []
+        iops_values = []
+        latency_values = []
+
+        for run_key, run in runs.items():
+            # Handle both dict and Run dataclass objects
+            metrics = None
+            if isinstance(run, dict) and 'metrics' in run:
+                metrics = run['metrics']
+            elif hasattr(run, 'metrics') and run.metrics:
+                metrics = run.metrics
+
+            if metrics:
+                # Extract bandwidth
+                if 'total_bandwidth_kbps' in metrics and metrics['total_bandwidth_kbps'] is not None:
+                    bandwidth_values.append(metrics['total_bandwidth_kbps'])
+
+                # Extract IOPS
+                if 'total_iops' in metrics and metrics['total_iops'] is not None:
+                    iops_values.append(metrics['total_iops'])
+
+                # Extract latency
+                if 'avg_latency_mean_ns' in metrics and metrics['avg_latency_mean_ns'] is not None:
+                    latency_values.append(metrics['avg_latency_mean_ns'])
+
+        # Build list of primary metrics (only include metrics with data)
+        primary_metrics = []
+
+        if bandwidth_values:
+            primary_metrics.append(
+                PrimaryMetric(
+                    name='bandwidth',
+                    value=statistics.mean(bandwidth_values),
+                    unit='KiB/s'
+                )
+            )
 
-        for run_key, run in results.runs.items():
-            bw = run.metrics.get('total_bandwidth_kbps', 0)
-            if bw > max_bw:
-                max_bw = bw
-                max_bw_run = run
+        if iops_values:
+            primary_metrics.append(
+                PrimaryMetric(
+                    name='iops',
+                    value=statistics.mean(iops_values),
+                    unit='IOPS'
+                )
+            )
 
-        # Set primary metrics (single-element list for now; see issue #27 for multi-metric)
-        if max_bw_run and max_bw > 0:
-            results.primary_metrics = [
+        if latency_values:
+            primary_metrics.append(
                 PrimaryMetric(
-                    name='max_bandwidth',
-                    value=max_bw,
-                    unit='KiB/s'
+                    name='latency',
+                    value=statistics.mean(latency_values),
+                    unit='nanoseconds'
                 )
-            ]
+            )
 
-        return results
+        return primary_metrics if primary_metrics else None
 
     def parse_runs(self, extracted_result: Dict[str, Any]) -> Dict[str, Any]:
         """

tests/test_fio_multi_metric.py

@@ -0,0 +1,220 @@
+"""
+FIO processor: multi-metric extraction (bandwidth, IOPS, latency).
+
+Tests that FIO extracts all three coequal metrics as primary_metrics.
+"""
+
+import pytest
+import json
+from pathlib import Path
+from unittest.mock import patch
+
+from chronicler.processors.fio_processor import FioProcessor
+from chronicler.processors.base_processor import ProcessorError
+from chronicler.schema import Run
+
+pytestmark = pytest.mark.unit
+
+
+def test_fio_extracts_all_three_primary_metrics(result_dir):
+    """
+    FIO should extract bandwidth, IOPS, and latency as primary_metrics.
+
+    Test validates RPOPC-1304: multi-metric support for FIO.
+    """
+    # Create FIO results JSON with multiple runs to test mean calculation
+    # Run 0: read-4KiB workload
+    fio_data_run0 = {
+        "timestamp": 1707004800,  # 2024-02-04 00:00:00 UTC
+        "jobs": [
+            {
+                "jobname": "job0",
+                "read": {
+                    "bw": 500000,
+                    "iops": 125000,
+                    "io_bytes": 1024000000,
+                    "total_ios": 250000,
+                    "lat_ns": {
+                        "mean": 120000,
+                        "min": 100000,
+                        "max": 150000
+                    },
+                    "clat_ns": {
+                        "mean": 118000,
+                        "min": 98000,
+                        "max": 148000
+                    },
+                    "slat_ns": {
+                        "mean": 2000,
+                        "min": 1000,
+                        "max": 3000
+                    }
+                },
+                "elapsed": 60
+            }
+        ]
+    }
+
+    # Run 1: read-1024KiB workload (different metrics)
+    fio_data_run1 = {
+        "timestamp": 1707004920,  # 2024-02-04 00:02:00 UTC
+        "jobs": [
+            {
+                "jobname": "job0",
+                "read": {
+                    "bw": 800000,
+                    "iops": 200000,
+                    "io_bytes": 2048000000,
+                    "total_ios": 500000,
+                    "lat_ns": {
+                        "mean": 140000,
+                        "min": 120000,
+                        "max": 170000
+                    },
+                    "clat_ns": {
+                        "mean": 138000,
+                        "min": 118000,
+                        "max": 168000
+                    },
+                    "slat_ns": {
+                        "mean": 2000,
+                        "min": 1000,
+                        "max": 3000
+                    }
+                },
+                "elapsed": 60
+            }
+        ]
+    }
+
+    # Create directory structure for two workloads
+    export_dir = result_dir / "export_fio_data_test"
+    export_dir.mkdir()
+
+    config_dir = export_dir / "fio_ndisks_1_disksize_10_GiB_njobs_1_ioengine_libaio_iodepth_16_2024.02.04T00.00.00"
+    config_dir.mkdir()
+
+    # Workload 0
+    workload0_dir = config_dir / "1-read-4KiB"
+    workload0_dir.mkdir()
+    (workload0_dir / "fio-results.json").write_text(json.dumps(fio_data_run0))
+
+    # Workload 1
+    workload1_dir = config_dir / "2-read-1024KiB"
+    workload1_dir.mkdir()
+    (workload1_dir / "fio-results.json").write_text(json.dumps(fio_data_run1))
+
+    # Create test_results_report (status)
+    (export_dir / "test_results_report").write_text("Ran 2 tests")
+
+    # Create dummy zip
+    dummy_zip = result_dir / "results_fio.zip"
+    dummy_zip.write_bytes(b"")
+
+    # Process FIO results
+    processor = FioProcessor(str(result_dir))
+    extracted_result = {"files": {}, "extracted_path": str(result_dir)}
+
+    # Mock archive extraction and call build_results
+    with patch.object(processor.archive_handler, "extract_result_archive") as mock_extract:
+        mock_extract.return_value = extracted_result
+        results = processor.build_results()
+
+    # Verify primary_metrics exists and has 3 metrics
+    assert results.primary_metrics is not None, "primary_metrics should not be None"
+    assert len(results.primary_metrics) == 3, f"Expected 3 metrics, got {len(results.primary_metrics)}"
+
+    # Extract metric names and values
+    metrics_by_name = {m.name: m for m in results.primary_metrics}
+
+    # Verify all three metrics are present
+    assert "bandwidth" in metrics_by_name, "bandwidth metric missing"
+    assert "iops" in metrics_by_name, "iops metric missing"
+    assert "latency" in metrics_by_name, "latency metric missing"
+
+    # Verify units are correct
+    assert metrics_by_name["bandwidth"].unit == "KiB/s"
+    assert metrics_by_name["iops"].unit == "IOPS"
+    assert metrics_by_name["latency"].unit == "nanoseconds"
+
+    # Verify values are means across both runs
+    # Bandwidth mean: (500000 + 800000) / 2 = 650000
+    assert metrics_by_name["bandwidth"].value == 650000.0
+
+    # IOPS mean: (125000 + 200000) / 2 = 162500
+    assert metrics_by_name["iops"].value == 162500.0
+
+    # Latency mean: (120000 + 140000) / 2 = 130000
+    assert metrics_by_name["latency"].value == 130000.0
+
+
+def test_fio_handles_single_run(result_dir):
+    """
+    FIO should extract metrics correctly from single run.
+
+    No averaging needed when only one run exists.
+    """
+    fio_data = {
+        "timestamp": 1707004800,
+        "jobs": [
+            {
+                "jobname": "job0",
+                "read": {
+                    "bw": 600000,
+                    "iops": 150000,
+                    "io_bytes": 1024000000,
+                    "total_ios": 250000,
+                    "lat_ns": {
+                        "mean": 125000,
+                        "min": 110000,
+                        "max": 160000
+                    },
+                    "clat_ns": {
+                        "mean": 123000,
+                        "min": 108000,
+                        "max": 158000
+                    },
+                    "slat_ns": {
+                        "mean": 2000,
+                        "min": 1000,
+                        "max": 3000
+                    }
+                },
+                "elapsed": 60
+            }
+        ]
+    }
+
+    # Create proper directory structure (needed for operation type detection)
+    export_dir = result_dir / "export_fio_data_test"
+    export_dir.mkdir()
+
+    config_dir = export_dir / "fio_ndisks_1_disksize_10_GiB_njobs_1_ioengine_libaio_iodepth_16_2024.02.04T00.00.00"
+    config_dir.mkdir()
+
+    workload_dir = config_dir / "1-read-4KiB"
+    workload_dir.mkdir()
+    (workload_dir / "fio-results.json").write_text(json.dumps(fio_data))
+
+    (export_dir / "test_results_report").write_text("Ran 1 test")
+
+    dummy_zip = result_dir / "results_fio.zip"
+    dummy_zip.write_bytes(b"")
+
+    processor = FioProcessor(str(result_dir))
+    extracted_result = {"files": {}, "extracted_path": str(result_dir)}
+
+    with patch.object(processor.archive_handler, "extract_result_archive") as mock_extract:
+        mock_extract.return_value = extracted_result
+        results = processor.build_results()
+
+    # Should have exactly 3 metrics from single run
+    assert results.primary_metrics is not None
+    assert len(results.primary_metrics) == 3
+
+    metrics_by_name = {m.name: m for m in results.primary_metrics}
+
+    # Values should match the single run (no averaging)
+    assert metrics_by_name["bandwidth"].value == 600000.0
+    assert metrics_by_name["iops"].value == 150000.0
+    assert metrics_by_name["latency"].value == 125000.0