From 77b32dbe6bc5bbb1f6b839f6cb2992efda12f89a Mon Sep 17 00:00:00 2001 From: soskuei Date: Thu, 15 Oct 2026 13:31:25 -0700 Subject: [PATCH 1/3] collector pickups --- .../plugins/inband/amdsmi/amdsmi_collector.py | 253 ++++++++++++++++++ 1 file changed, 253 insertions(+) diff --git a/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py b/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py index d04b3fd7..3bc4e52a 100644 --- a/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py +++ b/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py @@ -43,7 +43,14 @@ AmdSmiVersion, BadPages, CoreMetric, + CpuDimmPowConsumption, + CpuDimmTempRangeRate, + CpuDimmThermalSensor, + CpuIoBandwidth, + CpuLclkDpmLevel, CpuMetric, + CpuSvi3VrControllerTemp, + CpuXgmiBandwidth, EccState, Fabric, Fw, @@ -116,6 +123,32 @@ class AmdSmiCollector(InBandDataCollector[AmdSmiDataModel, AmdSmiCollectorArgs]) CMD_NODE = "node" CMD_METRIC_CPU = "metric --cpu all" CMD_METRIC_CORE = "metric --core all" + CMD_CPU_DIMM_POW = "metric --cpu-dimm-pow-consumption {dimm_addr}" + CMD_CPU_DIMM_THERMAL = "metric --cpu-dimm-thermal-sensor {dimm_addr}" + CMD_CPU_DIMM_TEMP_RANGE_RATE = "metric --cpu-dimm-temp-range-rate {dimm_addr}" + CMD_CPU_SVI3_HOTTEST = "metric --cpu-svi3-vr-controller-temp 0" + CMD_CPU_SVI3_RAIL = "metric --cpu-svi3-vr-controller-temp 1 {rail_index}" + CMD_CPU_XGMI_BANDWIDTH = "metric --cpu-xgmi-bandwidth {bw} {link_id}" + CMD_CPU_IO_BANDWIDTH = "metric --cpu-io-bandwidth {bw} {link_id}" + CMD_CPU_LCLK_DPM_LEVEL = "metric --cpu-lclk-dpm-level {nbio_id}" + CPU_LINK_ID_CANDIDATES: tuple[str, ...] = ( + "P2", + "P3", + "G0", + "G1", + "G2", + "G3", + "G4", + "G5", + "G6", + "G7", + ) + CPU_XGMI_BW_VALUES: tuple[int, ...] = (1, 2, 4) + CPU_IO_BW_VALUES: tuple[int, ...] = (1,) + CPU_SVI3_RAIL_CANDIDATES: tuple[int, ...] = (0, 1, 2, 3, 4) + MAX_DIMM_ADDR = 24 + MAX_NBIO_ID = 8 + CPU_SEQUENTIAL_NA_GRACE = 2 def _check_amdsmi_installed(self) -> bool: """Check if amd-smi is installed @@ -538,6 +571,212 @@ def get_core_metric(self) -> List[CoreMetric]: built = self._build_amdsmi_sub_data(CoreMetric, data) return built if isinstance(built, list) else ([built] if built else []) + def _amdsmi_cpu_entries(self, cmd: str) -> List[dict]: + """Run an amd-smi ``metric --cpu-*`` command and return its ``cpu_data`` list. + + Returns an empty list on any hard failure so callers can treat "no + cpu_data at all" as a definitive stop signal. + """ + ret = self._run_amd_smi_dict(cmd) + if not isinstance(ret, dict): + return [] + data = ret.get("cpu_data") + return data if isinstance(data, list) else [] + + def _append_amdsmi_model( + self, + entries: list, + model_cls: type[BaseModel], + data: dict, + context: str, + ) -> None: + """Validate ``data`` against ``model_cls`` and append; log+skip on failure.""" + try: + entries.append(model_cls.model_validate(data)) + except ValidationError as err: + self._log_event( + category=EventCategory.APPLICATION, + description=f"Failed to build {model_cls.__name__} entry; skipping", + data={"errors": err.errors(include_url=False), "context": context}, + priority=EventPriority.WARNING, + ) + + def _collect_cpu_sequential_metric( + self, + cmd_template: str, + path: tuple[str, ...], + model_cls: type[BaseModel], + max_value: int, + id_field: str, + ) -> list: + """Probe a sequential CPU metric argument (DIMM_ADDR, NBIOID) per host. + + Calls ``cmd_template`` for ``id_field`` values ``0, 1, 2, ...`` and + descends ``path`` into each returned ``cpu_data`` entry. Stops once + ``CPU_SEQUENTIAL_NA_GRACE`` consecutive values produce no dict at + ``path``, or immediately if the command itself hard-fails. + """ + entries: list = [] + consecutive_na = 0 + for value in range(max_value): + cpu_entries = self._amdsmi_cpu_entries(cmd_template.format(**{id_field: value})) + if not cpu_entries: + break + + found_any = False + for cpu_entry in cpu_entries: + node: Any = cpu_entry + for key in path: + node = node.get(key) if isinstance(node, dict) else None + if not isinstance(node, dict): + continue + found_any = True + self._append_amdsmi_model( + entries, + model_cls, + {"cpu": cpu_entry.get("cpu", 0), id_field: value, **node}, + context=f"{id_field}={value}", + ) + + if found_any: + consecutive_na = 0 + else: + consecutive_na += 1 + if consecutive_na >= self.CPU_SEQUENTIAL_NA_GRACE: + break + + return entries + + def get_cpu_dimm_pow_consumption(self) -> List[CpuDimmPowConsumption]: + """Get per-DIMM power consumption from amd-smi metric --cpu-dimm-pow-consumption.""" + return self._collect_cpu_sequential_metric( + self.CMD_CPU_DIMM_POW, + ("dimm_pow_consumption", "response"), + CpuDimmPowConsumption, + self.MAX_DIMM_ADDR, + "dimm_addr", + ) + + def get_cpu_dimm_thermal_sensor(self) -> List[CpuDimmThermalSensor]: + """Get per-DIMM thermal sensor data from amd-smi metric --cpu-dimm-thermal-sensor.""" + return self._collect_cpu_sequential_metric( + self.CMD_CPU_DIMM_THERMAL, + ("dimm_thermal_sensor", "response"), + CpuDimmThermalSensor, + self.MAX_DIMM_ADDR, + "dimm_addr", + ) + + def get_cpu_dimm_temp_range_rate(self) -> List[CpuDimmTempRangeRate]: + """Get per-DIMM temp range/refresh rate from amd-smi metric --cpu-dimm-temp-range-rate.""" + return self._collect_cpu_sequential_metric( + self.CMD_CPU_DIMM_TEMP_RANGE_RATE, + ("dimm_temp_range_rate", "response"), + CpuDimmTempRangeRate, + self.MAX_DIMM_ADDR, + "dimm_addr", + ) + + def get_cpu_lclk_dpm_level(self) -> List[CpuLclkDpmLevel]: + """Get per-NBIO LCLK DPM level range from amd-smi metric --cpu-lclk-dpm-level.""" + return self._collect_cpu_sequential_metric( + self.CMD_CPU_LCLK_DPM_LEVEL, + ("socket_dpm", "dpml_level_range"), + CpuLclkDpmLevel, + self.MAX_NBIO_ID, + "nbio_id", + ) + + def get_cpu_svi3_vr_controller_temp(self) -> List[CpuSvi3VrControllerTemp]: + """Get SVI3 VR controller temp from amd-smi metric --cpu-svi3-vr-controller-temp. + + Always probes the hottest-rail reading (TYPE=0, no rail index) plus + every documented individual rail (TYPE=1, RAIL_INDEX 0-4). + """ + entries: List[CpuSvi3VrControllerTemp] = [] + + for cpu_entry in self._amdsmi_cpu_entries(self.CMD_CPU_SVI3_HOTTEST): + group = cpu_entry.get("svi3_vr_controller_temp") + if isinstance(group, dict): + self._append_amdsmi_model( + entries, + CpuSvi3VrControllerTemp, + {"cpu": cpu_entry.get("cpu", 0), "rail_type": 0, "rail_index": None, **group}, + context="rail_type=0", + ) + + for rail_index in self.CPU_SVI3_RAIL_CANDIDATES: + cmd = self.CMD_CPU_SVI3_RAIL.format(rail_index=rail_index) + for cpu_entry in self._amdsmi_cpu_entries(cmd): + group = cpu_entry.get("svi3_vr_controller_temp") + if isinstance(group, dict): + self._append_amdsmi_model( + entries, + CpuSvi3VrControllerTemp, + { + "cpu": cpu_entry.get("cpu", 0), + "rail_type": 1, + "rail_index": rail_index, + **group, + }, + context=f"rail_type=1 rail_index={rail_index}", + ) + + return entries + + def get_cpu_xgmi_bandwidth(self) -> List[CpuXgmiBandwidth]: + """Get per-link XGMI bandwidth from amd-smi metric --cpu-xgmi-bandwidth. + + Probes every candidate LINKID_NAME at every candidate XGMI_BW; gaps + are architectural, so every combination is kept. + """ + entries: List[CpuXgmiBandwidth] = [] + for bw in self.CPU_XGMI_BW_VALUES: + for link_id in self.CPU_LINK_ID_CANDIDATES: + cmd = self.CMD_CPU_XGMI_BANDWIDTH.format(bw=bw, link_id=link_id) + for cpu_entry in self._amdsmi_cpu_entries(cmd): + group = cpu_entry.get("xgmi_bandwidth") + if isinstance(group, dict): + self._append_amdsmi_model( + entries, + CpuXgmiBandwidth, + { + "cpu": cpu_entry.get("cpu", 0), + "xgmi_bw": bw, + "link_id": link_id, + **group, + }, + context=f"xgmi_bw={bw} link_id={link_id}", + ) + return entries + + def get_cpu_io_bandwidth(self) -> List[CpuIoBandwidth]: + """Get per-link IO bandwidth from amd-smi metric --cpu-io-bandwidth. + + Same mechanism as ``get_cpu_xgmi_bandwidth``. valid link ids are not + assumed to match those found for xgmi-bandwidth, so this probes the + full candidate set independently. + """ + entries: List[CpuIoBandwidth] = [] + for bw in self.CPU_IO_BW_VALUES: + for link_id in self.CPU_LINK_ID_CANDIDATES: + cmd = self.CMD_CPU_IO_BANDWIDTH.format(bw=bw, link_id=link_id) + for cpu_entry in self._amdsmi_cpu_entries(cmd): + group = cpu_entry.get("io_bandwidth") + if isinstance(group, dict): + self._append_amdsmi_model( + entries, + CpuIoBandwidth, + { + "cpu": cpu_entry.get("cpu", 0), + "io_bw": bw, + "link_id": link_id, + **group, + }, + context=f"io_bw={bw} link_id={link_id}", + ) + return entries + def _get_amdsmi_data( self, args: Optional[AmdSmiCollectorArgs] = None ) -> Optional[AmdSmiDataModel]: @@ -561,6 +800,13 @@ def _get_amdsmi_data( node = self.get_node() cpu_metric = self.get_cpu_metric() core_metric = self.get_core_metric() + cpu_dimm_pow_consumption = self.get_cpu_dimm_pow_consumption() + cpu_dimm_thermal_sensor = self.get_cpu_dimm_thermal_sensor() + cpu_dimm_temp_range_rate = self.get_cpu_dimm_temp_range_rate() + cpu_svi3_vr_controller_temp = self.get_cpu_svi3_vr_controller_temp() + cpu_xgmi_bandwidth = self.get_cpu_xgmi_bandwidth() + cpu_io_bandwidth = self.get_cpu_io_bandwidth() + cpu_lclk_dpm_level = self.get_cpu_lclk_dpm_level() cper_data, cper_afids = self.get_cper_data() except Exception as e: self._log_event( @@ -591,6 +837,13 @@ def _get_amdsmi_data( node=node or [], cpu_metric=cpu_metric or [], core_metric=core_metric or [], + cpu_dimm_pow_consumption=cpu_dimm_pow_consumption or [], + cpu_dimm_thermal_sensor=cpu_dimm_thermal_sensor or [], + cpu_dimm_temp_range_rate=cpu_dimm_temp_range_rate or [], + cpu_svi3_vr_controller_temp=cpu_svi3_vr_controller_temp or [], + cpu_xgmi_bandwidth=cpu_xgmi_bandwidth or [], + cpu_io_bandwidth=cpu_io_bandwidth or [], + cpu_lclk_dpm_level=cpu_lclk_dpm_level or [], cper_data=cper_data, cper_afids=cper_afids, analysis_firmware_ids=fw_ids, From 732868e2e6bb660b0ba1a918fc57a3930a436557 Mon Sep 17 00:00:00 2001 From: soskuei Date: Thu, 15 Oct 2026 13:50:31 -0700 Subject: [PATCH 2/3] collector tests and amdsmidata collections --- .../plugins/inband/amdsmi/amdsmidata.py | 150 +++++++++++ test/unit/plugin/test_amdsmi_collector.py | 255 ++++++++++++++++++ 2 files changed, 405 insertions(+) diff --git a/nodescraper/plugins/inband/amdsmi/amdsmidata.py b/nodescraper/plugins/inband/amdsmi/amdsmidata.py index 07d8e83a..acc74f1e 100644 --- a/nodescraper/plugins/inband/amdsmi/amdsmidata.py +++ b/nodescraper/plugins/inband/amdsmi/amdsmidata.py @@ -183,6 +183,29 @@ def coerce_value_unit_input(v: Any) -> Any: return _coerce_value_unit_raw(v) +_SPACED_UNIT_RE = re.compile(r"^\s*([-+]?\d+(?:\.\d+)?)\s+(.+?)\s*$") + + +def coerce_spaced_unit_value_unit_input(v: Any) -> Any: + """Similar to ``coerce_value_unit_input``, but for units containing a space + (e.g. amd-smi's ``"0.0 Degrees C"``). + + ``_NUM_UNIT_RE`` disallows whitespace inside the unit portion + so a multi-word unit falls through to being stored verbatim as + ``value``. + """ + if v is None: + return None + if isinstance(v, ValueUnit): + return v + if isinstance(v, str) and not _value_unit_is_na(v): + m = _SPACED_UNIT_RE.match(v.strip()) + if m: + num, unit = m.groups() + return {"value": float(num) if "." in num else int(num), "unit": unit} + return coerce_value_unit_input(v) + + # Process class ProcessMemoryUsage(BaseModel): gtt_mem: Optional[ValueUnit] @@ -1744,6 +1767,124 @@ class CpuMetric(BaseModel): )(coerce_value_unit_input) +class CpuDimmPowConsumption(BaseModel): + """Per-DIMM data from ``amd-smi metric --cpu-dimm-pow-consumption DIMM_ADDR --json``. + + ``dimm_addr`` is the address the collector queried with, not the (unreliable) echoed value. + """ + + model_config = ConfigDict(extra="allow") + + cpu: int + dimm_addr: int + dimm_power_consumed: Optional[ValueUnit] = None + dimm_power_update_rate: Optional[ValueUnit] = None + + _value_unit = field_validator("dimm_power_consumed", "dimm_power_update_rate", mode="before")( + coerce_value_unit_input + ) + + +class CpuDimmThermalSensor(BaseModel): + """Per-DIMM data from ``amd-smi metric --cpu-dimm-thermal-sensor DIMM_ADDR --json``.""" + + model_config = ConfigDict(extra="allow") + + cpu: int + dimm_addr: int + dimm_thermal_sensor_value: Optional[int] = None + dimm_thermal_update_rate: Optional[ValueUnit] = None + dimm_thermal_temperature: Optional[ValueUnit] = None + + na_validator = field_validator("dimm_thermal_sensor_value", mode="before")(na_to_none) + _value_unit = field_validator("dimm_thermal_update_rate", mode="before")( + coerce_value_unit_input + ) + _temperature_value_unit = field_validator("dimm_thermal_temperature", mode="before")( + coerce_spaced_unit_value_unit_input + ) + + +class CpuDimmTempRangeRate(BaseModel): + """Per-DIMM data from ``amd-smi metric --cpu-dimm-temp-range-rate DIMM_ADDR --json``.""" + + model_config = ConfigDict(extra="allow") + + cpu: int + dimm_addr: int + dimm_temperature_range: Optional[int] = None + dimm_refresh_rate: Optional[int] = None + + na_validator = field_validator("dimm_temperature_range", "dimm_refresh_rate", mode="before")( + na_to_none + ) + + +class CpuSvi3VrControllerTemp(BaseModel): + """Per-rail data from ``amd-smi metric --cpu-svi3-vr-controller-temp TYPE [RAIL_INDEX] --json``. + + ``rail_type``/``rail_index`` are the arguments the collector queried with + (0=HottestRail/1=IndividualRail; rail 0-4 maps to VDDCR_CPU0, VDDCR_CPU1, + VDDCR_SOC, VDDIO, VDDIO_MEM_S3). + """ + + model_config = ConfigDict(extra="allow", populate_by_name=True) + + cpu: int + rail_type: int + rail_index: Optional[int] = None + rail_selection: Optional[str] = Field(default=None, alias="RAIL_SELECTION") + reported_rail_index: Optional[int] = Field(default=None, alias="RAIL_INDEX") + temperature: Optional[ValueUnit] = Field(default=None, alias="TEMPERATURE") + + na_validator = field_validator("rail_selection", "reported_rail_index", mode="before")( + na_to_none + ) + _value_unit = field_validator("temperature", mode="before")(coerce_value_unit_input) + + +class CpuXgmiBandwidth(BaseModel): + """Per-link data from ``amd-smi metric --cpu-xgmi-bandwidth XGMI_BW LINKID_NAME --json``.""" + + model_config = ConfigDict(extra="allow") + + cpu: int + xgmi_bw: int + link_id: str + band_width: Optional[ValueUnit] = None + + _value_unit = field_validator("band_width", mode="before")(coerce_value_unit_input) + + +class CpuIoBandwidth(BaseModel): + """Per-link data from ``amd-smi metric --cpu-io-bandwidth IO_BW LINKID_NAME --json``. + """ + + model_config = ConfigDict(extra="allow") + + cpu: int + io_bw: int + link_id: str + band_width: Optional[ValueUnit] = None + + _value_unit = field_validator("band_width", mode="before")(coerce_value_unit_input) + + +class CpuLclkDpmLevel(BaseModel): + """Per-NBIO data from ``amd-smi metric --cpu-lclk-dpm-level NBIOID --json``.""" + + model_config = ConfigDict(extra="allow") + + cpu: int + nbio_id: int + nbio_min_dpm_level: Optional[int] = None + nbio_max_dpm_level: Optional[int] = None + + na_validator = field_validator("nbio_min_dpm_level", "nbio_max_dpm_level", mode="before")( + na_to_none + ) + + class AmdSmiAnalysisRef(BaseModel): """Collector-filled summary for reference config""" @@ -1793,6 +1934,15 @@ class AmdSmiDataModel(DataModel): node: Optional[list[NodeInfo]] = Field(default_factory=list) cpu_metric: Optional[list[CpuMetric]] = Field(default_factory=list) core_metric: Optional[list[CoreMetric]] = Field(default_factory=list) + cpu_dimm_pow_consumption: Optional[list[CpuDimmPowConsumption]] = Field(default_factory=list) + cpu_dimm_thermal_sensor: Optional[list[CpuDimmThermalSensor]] = Field(default_factory=list) + cpu_dimm_temp_range_rate: Optional[list[CpuDimmTempRangeRate]] = Field(default_factory=list) + cpu_svi3_vr_controller_temp: Optional[list[CpuSvi3VrControllerTemp]] = Field( + default_factory=list + ) + cpu_xgmi_bandwidth: Optional[list[CpuXgmiBandwidth]] = Field(default_factory=list) + cpu_io_bandwidth: Optional[list[CpuIoBandwidth]] = Field(default_factory=list) + cpu_lclk_dpm_level: Optional[list[CpuLclkDpmLevel]] = Field(default_factory=list) cper_data: Optional[list[FileModel]] = Field(default_factory=list) cper_afids: dict[str, int] = Field(default_factory=dict) diff --git a/test/unit/plugin/test_amdsmi_collector.py b/test/unit/plugin/test_amdsmi_collector.py index 85988d3e..de7fe2e0 100644 --- a/test/unit/plugin/test_amdsmi_collector.py +++ b/test/unit/plugin/test_amdsmi_collector.py @@ -1233,3 +1233,258 @@ def test_get_core_metric_command_failure(conn_mock, system_info, monkeypatch): c = make_cpu_core_collector(conn_mock, system_info, monkeypatch, core_payload=None) assert c.get_core_metric() == [] + + +# --------------------------------------------------------------------------- +# CPU metric flags that require an explicit argument (not covered by +# ``metric --cpu all``). +# --------------------------------------------------------------------------- + + +def make_cpu_arg_collector( + conn_mock, system_info, monkeypatch, responses: dict[str, Any] +) -> tuple[AmdSmiCollector, list[str]]: + """Create a collector driven by an exact command -> JSON-payload map. + """ + calls: list[str] = [] + + def mock_run_sut_cmd(cmd: str) -> MagicMock: + if "which amd-smi" in cmd: + return make_cmd_result("/usr/bin/amd-smi") + assert cmd.startswith("amd-smi ") and cmd.endswith(" --json") + key = cmd[len("amd-smi ") : -len(" --json")] + calls.append(key) + if key not in responses: + return make_cmd_result("", "not captured in test", 1) + return make_cmd_result(make_json_response(responses[key])) + + c = AmdSmiCollector( + system_info=system_info, + system_interaction_level=SystemInteractionLevel.PASSIVE, + connection=conn_mock, + ) + monkeypatch.setattr(c, "_run_sut_cmd", mock_run_sut_cmd) + return c, calls + + +def _dimm_payload(group_key: str, response: Any) -> dict: + """Build a ``{"cpu_data": [...]}`` payload for a DIMM-flag response group.""" + return {"cpu_data": [{"cpu": 0, group_key: {"response": response}}]} + + +def test_get_cpu_dimm_pow_consumption_stops_after_na_grace(conn_mock, system_info, monkeypatch): + """DIMM 0-1 populated, 2-3 N/A (real capture shape): collect the 2 real + entries and stop after CPU_SEQUENTIAL_NA_GRACE consecutive misses, never + probing address 4+.""" + responses = { + "metric --cpu-dimm-pow-consumption 0": _dimm_payload( + "dimm_pow_consumption", + { + "dimm_power_consumed": "3688 mW", + "dimm_power_update_rate": "0 ms", + "dimm_dimm_addr": 0, + }, + ), + "metric --cpu-dimm-pow-consumption 1": _dimm_payload( + "dimm_pow_consumption", + { + "dimm_power_consumed": "3562 mW", + "dimm_power_update_rate": "256 ms", + "dimm_dimm_addr": 0, + }, + ), + "metric --cpu-dimm-pow-consumption 2": _dimm_payload("dimm_pow_consumption", "N/A"), + "metric --cpu-dimm-pow-consumption 3": _dimm_payload("dimm_pow_consumption", "N/A"), + } + c, calls = make_cpu_arg_collector(conn_mock, system_info, monkeypatch, responses) + + entries = c.get_cpu_dimm_pow_consumption() + + assert [e.dimm_addr for e in entries] == [0, 1] + assert entries[0].dimm_power_consumed.value == 3688 + assert entries[0].dimm_power_consumed.unit == "mW" + assert entries[1].dimm_power_update_rate.value == 256 + assert calls == [ + "metric --cpu-dimm-pow-consumption 0", + "metric --cpu-dimm-pow-consumption 1", + "metric --cpu-dimm-pow-consumption 2", + "metric --cpu-dimm-pow-consumption 3", + ] + + +def test_get_cpu_dimm_pow_consumption_command_failure_stops_immediately( + conn_mock, system_info, monkeypatch +): + """A hard command failure at address 0 stops enumeration immediately.""" + c, calls = make_cpu_arg_collector(conn_mock, system_info, monkeypatch, responses={}) + + assert c.get_cpu_dimm_pow_consumption() == [] + assert calls == ["metric --cpu-dimm-pow-consumption 0"] + + +def test_get_cpu_dimm_thermal_sensor_parses_spaced_unit_temperature( + conn_mock, system_info, monkeypatch +): + """``"0.0 Degrees C"`` must parse into value=0.0/unit="Degrees C" (the + shared ValueUnit regex doesn't allow a space inside the unit, so this + field needs ``coerce_spaced_unit_value_unit_input``).""" + responses = { + "metric --cpu-dimm-thermal-sensor 0": _dimm_payload( + "dimm_thermal_sensor", + { + "dimm_thermal_sensor_value": 0, + "dimm_thermal_update_rate": "0 ms", + "dimm_thermal_dimm_addr": 0, + "dimm_thermal_temperature": "0.0 Degrees C", + }, + ), + "metric --cpu-dimm-thermal-sensor 1": _dimm_payload("dimm_thermal_sensor", "N/A"), + "metric --cpu-dimm-thermal-sensor 2": _dimm_payload("dimm_thermal_sensor", "N/A"), + } + c, _ = make_cpu_arg_collector(conn_mock, system_info, monkeypatch, responses) + + entries = c.get_cpu_dimm_thermal_sensor() + + assert len(entries) == 1 + assert entries[0].dimm_thermal_temperature.value == 0.0 + assert entries[0].dimm_thermal_temperature.unit == "Degrees C" + assert entries[0].dimm_thermal_sensor_value == 0 + + +def test_get_cpu_dimm_temp_range_rate(conn_mock, system_info, monkeypatch): + """Plain-int fields (no units) parse directly; N/A collapses the group.""" + responses = { + "metric --cpu-dimm-temp-range-rate 0": _dimm_payload( + "dimm_temp_range_rate", {"dimm_temperature_range": 0, "dimm_refresh_rate": 0} + ), + "metric --cpu-dimm-temp-range-rate 1": _dimm_payload("dimm_temp_range_rate", "N/A"), + "metric --cpu-dimm-temp-range-rate 2": _dimm_payload("dimm_temp_range_rate", "N/A"), + } + c, _ = make_cpu_arg_collector(conn_mock, system_info, monkeypatch, responses) + + entries = c.get_cpu_dimm_temp_range_rate() + + assert [e.dimm_addr for e in entries] == [0] + assert entries[0].dimm_temperature_range == 0 + assert entries[0].dimm_refresh_rate == 0 + + +def test_get_cpu_lclk_dpm_level_stops_after_na_grace(conn_mock, system_info, monkeypatch): + """NBIO 0-3 populated, stop after 2 consecutive N/A. + """ + dpm_range = {"nbio_max_dpm_level": 1, "nbio_min_dpm_level": 0} + responses = { + f"metric --cpu-lclk-dpm-level {i}": { + "cpu_data": [{"cpu": 0, "socket_dpm": {"dpml_level_range": dpm_range}}] + } + for i in range(4) + } + responses["metric --cpu-lclk-dpm-level 4"] = { + "cpu_data": [{"cpu": 0, "socket_dpm": {"dpml_level_range": "N/A"}}] + } + responses["metric --cpu-lclk-dpm-level 5"] = { + "cpu_data": [{"cpu": 0, "socket_dpm": {"dpml_level_range": "N/A"}}] + } + c, calls = make_cpu_arg_collector(conn_mock, system_info, monkeypatch, responses) + + entries = c.get_cpu_lclk_dpm_level() + + assert [e.nbio_id for e in entries] == [0, 1, 2, 3] + assert entries[0].nbio_max_dpm_level == 1 + assert entries[0].nbio_min_dpm_level == 0 + assert calls == [f"metric --cpu-lclk-dpm-level {i}" for i in range(6)] + + +def test_get_cpu_svi3_vr_controller_temp_probes_full_fixed_candidate_set( + conn_mock, system_info, monkeypatch +): + """SVI3's candidate set (TYPE 0, TYPE 1 x rail 0-4) is small and fully + documented by amd-smi --help, so every combination is always probed and + kept, including all-N/A.""" + na_group = {"RAIL_SELECTION": "N/A", "RAIL_INDEX": "N/A", "TEMPERATURE": "N/A"} + responses = { + "metric --cpu-svi3-vr-controller-temp 0": { + "cpu_data": [{"cpu": 0, "svi3_vr_controller_temp": na_group}] + }, + **{ + f"metric --cpu-svi3-vr-controller-temp 1 {rail}": { + "cpu_data": [{"cpu": 0, "svi3_vr_controller_temp": na_group}] + } + for rail in range(5) + }, + } + c, calls = make_cpu_arg_collector(conn_mock, system_info, monkeypatch, responses) + + entries = c.get_cpu_svi3_vr_controller_temp() + + assert [(e.rail_type, e.rail_index) for e in entries] == [ + (0, None), + (1, 0), + (1, 1), + (1, 2), + (1, 3), + (1, 4), + ] + assert all(e.temperature is None for e in entries) + assert calls == [ + "metric --cpu-svi3-vr-controller-temp 0", + "metric --cpu-svi3-vr-controller-temp 1 0", + "metric --cpu-svi3-vr-controller-temp 1 1", + "metric --cpu-svi3-vr-controller-temp 1 2", + "metric --cpu-svi3-vr-controller-temp 1 3", + "metric --cpu-svi3-vr-controller-temp 1 4", + ] + + +def test_get_cpu_xgmi_bandwidth_keeps_na_links_and_probes_every_bw( + conn_mock, system_info, monkeypatch +): + """Every candidate link is kept (N/A included, no filtering) and + every XGMI_BW value is probed for every link (no shared-link-id + optimization across BW values).""" + + def payload(band_width): + return {"cpu_data": [{"cpu": 0, "xgmi_bandwidth": {"band_width": band_width}}]} + + real_links = {"G0": "37 Mbps", "G1": "37 Mbps", "G2": "92444 Mbps"} + responses = {} + for bw in (1, 2, 4): + for link_id in AmdSmiCollector.CPU_LINK_ID_CANDIDATES: + responses[f"metric --cpu-xgmi-bandwidth {bw} {link_id}"] = payload( + real_links.get(link_id, "N/A") + ) + c, calls = make_cpu_arg_collector(conn_mock, system_info, monkeypatch, responses) + + entries = c.get_cpu_xgmi_bandwidth() + + assert len(entries) == 3 * len(AmdSmiCollector.CPU_LINK_ID_CANDIDATES) + assert len(calls) == len(entries) + by_bw_link = {(e.xgmi_bw, e.link_id): e.band_width for e in entries} + assert by_bw_link[(1, "G2")].value == 92444 + assert by_bw_link[(1, "G2")].unit == "Mbps" + assert by_bw_link[(1, "P2")] is None + assert by_bw_link[(4, "G7")] is None + + +def test_get_cpu_io_bandwidth_independent_from_xgmi_validity(conn_mock, system_info, monkeypatch): + """IO bandwidth's valid-link subset does not have to match xgmi-bandwidth's, + it must be probed and kept independently.""" + + def payload(band_width): + return {"cpu_data": [{"cpu": 0, "io_bandwidth": {"band_width": band_width}}]} + + real_links = {"P2": "0 Mbps", "G0": "0 Mbps", "G1": "1 Mbps", "G2": "0 Mbps"} + responses = { + f"metric --cpu-io-bandwidth 1 {link_id}": payload(real_links.get(link_id, "N/A")) + for link_id in AmdSmiCollector.CPU_LINK_ID_CANDIDATES + } + c, calls = make_cpu_arg_collector(conn_mock, system_info, monkeypatch, responses) + + entries = c.get_cpu_io_bandwidth() + + assert len(entries) == len(AmdSmiCollector.CPU_LINK_ID_CANDIDATES) + assert len(calls) == len(entries) + by_link = {e.link_id: e.band_width for e in entries} + assert by_link["P2"].value == 0 + assert by_link["G1"].value == 1 + assert by_link["G3"] is None From f894021224cee2e6f8d7e9425670159ca4e352ff Mon Sep 17 00:00:00 2001 From: soskuei Date: Thu, 15 Oct 2026 14:24:23 -0700 Subject: [PATCH 3/3] precommit cleanups --- .../plugins/inband/amdsmi/amdsmi_collector.py | 2 +- nodescraper/plugins/inband/amdsmi/amdsmidata.py | 5 ++--- .../plugins/inband/fabrics/fabrics_collector.py | 12 ++++++------ test/unit/plugin/test_amdsmi_collector.py | 10 ++++------ 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py b/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py index 3bc4e52a..84ff2bf5 100644 --- a/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py +++ b/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py @@ -574,7 +574,7 @@ def get_core_metric(self) -> List[CoreMetric]: def _amdsmi_cpu_entries(self, cmd: str) -> List[dict]: """Run an amd-smi ``metric --cpu-*`` command and return its ``cpu_data`` list. - Returns an empty list on any hard failure so callers can treat "no + Returns an empty list on any hard failure so callers can treat "no cpu_data at all" as a definitive stop signal. """ ret = self._run_amd_smi_dict(cmd) diff --git a/nodescraper/plugins/inband/amdsmi/amdsmidata.py b/nodescraper/plugins/inband/amdsmi/amdsmidata.py index acc74f1e..a26c49c3 100644 --- a/nodescraper/plugins/inband/amdsmi/amdsmidata.py +++ b/nodescraper/plugins/inband/amdsmi/amdsmidata.py @@ -191,7 +191,7 @@ def coerce_spaced_unit_value_unit_input(v: Any) -> Any: (e.g. amd-smi's ``"0.0 Degrees C"``). ``_NUM_UNIT_RE`` disallows whitespace inside the unit portion - so a multi-word unit falls through to being stored verbatim as + so a multi-word unit falls through to being stored verbatim as ``value``. """ if v is None: @@ -1857,8 +1857,7 @@ class CpuXgmiBandwidth(BaseModel): class CpuIoBandwidth(BaseModel): - """Per-link data from ``amd-smi metric --cpu-io-bandwidth IO_BW LINKID_NAME --json``. - """ + """Per-link data from ``amd-smi metric --cpu-io-bandwidth IO_BW LINKID_NAME --json``.""" model_config = ConfigDict(extra="allow") diff --git a/nodescraper/plugins/inband/fabrics/fabrics_collector.py b/nodescraper/plugins/inband/fabrics/fabrics_collector.py index 4c7501a2..d68afb8f 100644 --- a/nodescraper/plugins/inband/fabrics/fabrics_collector.py +++ b/nodescraper/plugins/inband/fabrics/fabrics_collector.py @@ -24,7 +24,7 @@ # ############################################################################### import re -from typing import Dict, List, Optional, Tuple +from typing import Dict, List, Optional, Tuple, Union from nodescraper.base import InBandDataCollector from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus @@ -457,14 +457,14 @@ def collect_data( for ibv_device in ibv_devices: ports = [] for port_num, attrs in ibv_device.ports.items(): - port = {"port": port_num} + port_entry: Dict[str, Union[int, str]] = {"port": port_num} if "state" in attrs: - port["state"] = attrs["state"] + port_entry["state"] = attrs["state"] if "link_layer" in attrs: - port["link_layer"] = attrs["link_layer"] + port_entry["link_layer"] = attrs["link_layer"] if "active_mtu" in attrs: - port["active_mtu"] = attrs["active_mtu"] - ports.append(port) + port_entry["active_mtu"] = attrs["active_mtu"] + ports.append(port_entry) ibv_device_info.append( { "device": ibv_device.device, diff --git a/test/unit/plugin/test_amdsmi_collector.py b/test/unit/plugin/test_amdsmi_collector.py index de7fe2e0..d47dd65f 100644 --- a/test/unit/plugin/test_amdsmi_collector.py +++ b/test/unit/plugin/test_amdsmi_collector.py @@ -1244,8 +1244,7 @@ def test_get_core_metric_command_failure(conn_mock, system_info, monkeypatch): def make_cpu_arg_collector( conn_mock, system_info, monkeypatch, responses: dict[str, Any] ) -> tuple[AmdSmiCollector, list[str]]: - """Create a collector driven by an exact command -> JSON-payload map. - """ + """Create a collector driven by an exact command -> JSON-payload map.""" calls: list[str] = [] def mock_run_sut_cmd(cmd: str) -> MagicMock: @@ -1370,8 +1369,7 @@ def test_get_cpu_dimm_temp_range_rate(conn_mock, system_info, monkeypatch): def test_get_cpu_lclk_dpm_level_stops_after_na_grace(conn_mock, system_info, monkeypatch): - """NBIO 0-3 populated, stop after 2 consecutive N/A. - """ + """NBIO 0-3 populated, stop after 2 consecutive N/A.""" dpm_range = {"nbio_max_dpm_level": 1, "nbio_min_dpm_level": 0} responses = { f"metric --cpu-lclk-dpm-level {i}": { @@ -1439,8 +1437,8 @@ def test_get_cpu_svi3_vr_controller_temp_probes_full_fixed_candidate_set( def test_get_cpu_xgmi_bandwidth_keeps_na_links_and_probes_every_bw( conn_mock, system_info, monkeypatch ): - """Every candidate link is kept (N/A included, no filtering) and - every XGMI_BW value is probed for every link (no shared-link-id + """Every candidate link is kept (N/A included, no filtering) and + every XGMI_BW value is probed for every link (no shared-link-id optimization across BW values).""" def payload(band_width):