AmdSmiPlugin updates and fixes

nodescraper/plugins/inband/amdsmi/amdsmi_collector.py

@@ -91,6 +91,42 @@
 
 T = TypeVar("T", bound=BaseModel)
 
+_AFID_ABSENT = {"-", "n/a", "na"}
+# First integer in the AFID column. The file name can contain digits.
+_AFID_VALUE_RE = re.compile(r"\d+")
+
+
+def _parse_afid_output(output: str) -> Optional[int]:
+    """Return the AFID from amd-smi output, or None when the value is absent.
+
+    Args:
+        output: stdout from ras --afid --cper-file.
+
+    Returns:
+        The AFID integer, or None for a dash or N/A.
+    """
+    lines = [line.strip() for line in output.splitlines() if line.strip()]
+    if not lines:
+        raise ValueError("empty AFID output")
+    header_index = next(
+        (index for index, line in enumerate(lines) if line.lower().startswith("file_name")),
+        None,
+    )
+    if header_index is None:
+        data_line = lines[0]
+    elif header_index + 1 >= len(lines):
+        raise ValueError("AFID table has no data row")
+    else:
+        data_line = lines[header_index + 1]
+    tokens = data_line.split()
+    cell = tokens[0] if len(tokens) == 1 else " ".join(tokens[1:])
+    if cell.lower() in _AFID_ABSENT:
+        return None
+    match = _AFID_VALUE_RE.search(cell)
+    if match is None:
+        raise ValueError(f"no AFID in {cell!r}")
+    return int(match.group(0))
+
 
 class AmdSmiCollector(InBandDataCollector[AmdSmiDataModel, AmdSmiCollectorArgs]):
     """Class for collection of inband tool amd-smi data."""
@@ -208,6 +244,7 @@ def _run_amd_smi(self, cmd: str) -> Optional[str]:
                         "command": cmd,
                         "exit_code": cmd_ret.exit_code,
                         "stderr": cmd_ret.stderr,
+                        "stdout": cmd_ret.stdout,
                     },
                     priority=EventPriority.WARNING,
                     console_log=True,
@@ -221,6 +258,7 @@ def _run_amd_smi(self, cmd: str) -> Optional[str]:
                         "command": cmd,
                         "exit_code": cmd_ret.exit_code,
                         "stderr": cmd_ret.stderr,
+                        "stdout": cmd_ret.stdout,
                     },
                     priority=EventPriority.ERROR,
                     console_log=True,
@@ -266,7 +304,8 @@ def _run_amd_smi_dict(self, cmd: str) -> Optional[Union[dict, list[dict]]]:
         Returns:
             Optional[Union[dict, list[dict]]]: parsed JSON output or None on error
         """
-        cmd += " --json"
+        if "--json" not in cmd.split():
+            cmd += " --json"
         cmd_ret = self._run_amd_smi(cmd)
         if cmd_ret:
             try:
@@ -1312,7 +1351,13 @@ def get_static(self) -> Optional[list[AmdSmiStatic]]:
                 )
 
             # NUMA
-            numa_node = numa.get("node")
+            numa_node_raw = numa.get("node")
+            if numa_node_raw in (None, "", "N/A"):
+                numa_node: Union[int, str] = "N/A"
+            elif isinstance(numa_node_raw, str):
+                numa_node = numa_node_raw
+            else:
+                numa_node = int(numa_node_raw) if numa_node_raw is not None else "N/A"
             affinity_raw = numa.get("affinity")
             if affinity_raw in (None, "", "N/A"):
                 affinity_val: Union[int, str] = "N/A"
@@ -1883,24 +1928,33 @@ def _get_cper_afid(self, cper_file_path: str) -> Optional[int]:
             return None
 
         try:
-            afid = int(result.strip())
+            afid = _parse_afid_output(result)
+        except ValueError:
             self._log_event(
                 category=EventCategory.APPLICATION,
-                description=f"Successfully retrieved AFID from CPER file: {cper_file_path}",
-                data={"afid": afid, "cper_file": cper_file_path},
-                priority=EventPriority.INFO,
+                description=f"Failed to parse AFID value from output: {result}",
+                data={"output": result, "cper_file": cper_file_path},
+                priority=EventPriority.ERROR,
                 console_log=True,
             )
-            return afid
-        except ValueError:
+            return None
+        if afid is None:
             self._log_event(
                 category=EventCategory.APPLICATION,
-                description=f"Failed to parse AFID value from output: {result}",
+                description=f"No AFID reported for CPER file: {cper_file_path}",
                 data={"output": result, "cper_file": cper_file_path},
-                priority=EventPriority.ERROR,
+                priority=EventPriority.INFO,
                 console_log=True,
             )
             return None
+        self._log_event(
+            category=EventCategory.APPLICATION,
+            description=f"Successfully retrieved AFID from CPER file: {cper_file_path}",
+            data={"afid": afid, "cper_file": cper_file_path},
+            priority=EventPriority.INFO,
+            console_log=True,
+        )
+        return afid
 
     def collect_data(
         self,

nodescraper/plugins/inband/amdsmi/amdsmidata.py

@@ -523,12 +523,10 @@ class StaticXgmiPlpd(BaseModel):
 
 
 class StaticNuma(BaseModel):
-
     model_config = ConfigDict(extra="allow")
 
-    node: Optional[int]
+    node: Union[int, str]  # can be N/A
     affinity: Union[int, str]  # can be N/A
-    na_validator = field_validator("node", mode="before")(na_to_none)
 
 
 class StaticVram(AmdSmiBaseModel):
@@ -1286,6 +1284,21 @@ def clock_rails(self) -> MetricClockRails:
         """Enumerated view of clock catching a dropped/renamed known rail."""
         return MetricClockRails.model_validate(self.clock or {})
 
+    @model_validator(mode="before")
+    @classmethod
+    def _normalize_metric_input(cls, value: object) -> object:
+        """Normalize one metric object before field validation.
+
+        Args:
+            value: Raw metric input.
+
+        Returns:
+            The input, with per-block clocks and the PCIe count alias normalized.
+        """
+        if isinstance(value, dict):
+            return normalize_amdsmi_metric_dict(value)
+        return value
+
     @field_validator("ecc_blocks", mode="before")
     @classmethod
     def validate_ecc_blocks(cls, value: Union[dict[str, EccData], str]) -> dict[str, EccData]:

test/unit/plugin/test_amdsmi_analyzer.py

@@ -45,6 +45,7 @@
     Fw,
     FwListItem,
     LinkStatusTable,
+    MetricClockData,
     MetricEccTotals,
     MetricPcie,
     Partition,
@@ -1108,6 +1109,44 @@ def _minimal_amdsmi_metric(
     )
 
 
+def test_metric_accepts_legacy_and_per_block_clocks():
+    """Older clock objects and per-block clocks validate together."""
+    raw = _minimal_amdsmi_metric().model_dump()
+    raw["clock"] = {
+        "GFX_0": {
+            "clk": {"value": 132, "unit": "MHz"},
+            "min_clk": {"value": 500, "unit": "MHz"},
+            "max_clk": {"value": 2100, "unit": "MHz"},
+            "clk_locked": 0,
+            "deep_sleep": 0,
+        },
+        "uclk_aid": {
+            "aid_0": {"value": 1900, "unit": "MHz"},
+            "aid_1": {"value": 1900, "unit": "MHz"},
+        },
+        "socclks_mid": {
+            "mid_0": {"value": 1350, "unit": "MHz"},
+            "mid_1": "N/A",
+        },
+    }
+    raw["pcie"].pop("lc_perf_other_end_recovery", None)
+    raw["pcie"]["lc_perf_other_end_recovery_count"] = 4
+
+    metric = AmdSmiMetric.model_validate(raw)
+
+    assert isinstance(metric.clock["GFX_0"], MetricClockData)
+    assert metric.clock["GFX_0"].min_clk.value == 500
+    assert metric.clock["uclk_aid"]["aid_0"]["value"] == 1900
+    assert metric.clock["socclks_mid"]["mid_1"] == "N/A"
+    assert metric.pcie.lc_perf_other_end_recovery == 4
+
+    legacy = _minimal_amdsmi_metric()
+    legacy_raw = legacy.model_dump()
+    legacy_raw["pcie"]["lc_perf_other_end_recovery"] = 2
+    legacy_raw["pcie"]["lc_perf_other_end_recovery_count"] = 9
+    assert AmdSmiMetric.model_validate(legacy_raw).pcie.lc_perf_other_end_recovery == 2
+
+
 def test_gpu_mismatch_description_helpers():
     """Per-GPU mismatch helpers include GPU id, expected, and actual in output."""
     desc, details = _gpu_mismatch_description(