feat: CPU temperature for remote Sparks and in the CPU panel - #34
Open
VoidSec wants to merge 1 commit into
Open
Conversation
Remote Sparks always reported 0: _getRemoteCpu() hardcoded the value and never queried a sensor. The CPU panel had no temperature row at all, so the field went unused for local Sparks too. Collect it within the existing SSH round trip by appending sensor candidates to the same command, in the priority order the local path already uses (hwmon coretemp/k10temp/zenpower/acpitz, then thermal zones). _parseSensorTemp takes the first plausible reading using the same accept range as _getCPUTemperature, and returns 0 when nothing is readable. Display it in CpuPanel between Usage and Power, mirroring GpuPanel: honors the Celsius/Fahrenheit setting and draws a sparkline from a new cpu.temp history series. Thresholds follow DGX_SPARK.THERMAL_THRESHOLDS.junction (warn 85, crit 95) instead of the GPU's 65/85, which would sit amber permanently at a GB10's normal ~70C load. The row is hidden when the reading is 0, so a host with no readable sensor shows no bogus 0C. Verified on two GB10 nodes: local and remote paths both select hwmon0/acpitz/temp1_input (identical to thermal_zone0), agreeing within 0.2C of a direct sysfs read.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two gaps that compound each other:
SystemCollector._getRemoteCpu()returned a hardcodedtemperature: 0and never queried a sensor, so remote Sparks could not report CPU temperature at all.CpuPanelhad no temperature row, soCpuMetrics.temperaturewent unrendered even for local Sparks where the collector did populate it.Changes
Collect it for remote Sparks (
SystemCollector.js). Sensor candidates are appended to the existing SSH command, so this stays one round trip per poll. They are emitted in the same priority order the local path uses: hwmon namedcoretemp/k10temp/zenpower/acpitzfirst, then thermal zones. A new_parseSensorTemp()takes the first plausible reading with the same accept range as_getCPUTemperature()(0 < raw < 200000) and returns 0 when nothing is readable, matching_defaultCpu().The shared allowlist matters on this hardware: GB10 boxes also expose
nvmeand fourmlx5ConnectX-7 sensors, and the mlx5 readings sit only ~3C off the CPU one, so a mismatched filter would be easy to miss by eye.Display it (
CpuPanel.tsx,SparkPage.tsx,metricsStore.ts). A Temperature row between Usage and Power, mirroringGpuPanel: honors the Celsius/Fahrenheit setting via the same conversion, and draws a sparkline from a newcpu.temphistory series. Two deliberate differences from the GPU panel:DGX_SPARK.THERMAL_THRESHOLDS.junction(warn 85, crit 95) rather than the GPU's 65/85. A GB10 CPU sits around 70C under normal load, so the GPU thresholds would pin the row amber permanently and make a real thermal event indistinguishable from idle.0°C. This follows how the panel already gatesramAvailand OOM risk. The history series skips zero samples for the same reason.Verification
Tested on two GB10 nodes, one local and one remote over SSH:
hwmon0/acpitz/temp1_input, so cross-node readings are comparable rather than merely self-consistent.thermal_zoneNandhwmon0/temp(N+1)_inputare the same sensor surfaced through two kernel interfaces, byte-identical when read simultaneously across all 7 zones on both nodes.7 new unit tests in
server/collectors/__tests__/SystemCollector.cpuTemp.test.jscover the conversion, first-plausible-wins ordering, the blank line left by the---section split, unreadable sensors, out-of-range rejection, and rounding.tsc --noEmitis clean.Note:
npm run testhas one pre-existing failure inshowcasePrompts.test.jsunrelated to this change; it readssrc/components/ShowcasePage/showcasePrompts.ts, which is absent when the suite runs inside the production image.Not included
No
CHANGELOG.mdentry, since slotting this under a version heading looked like a release decision. Happy to add one wherever you'd like it.