diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 8ba28b731..5ef0ce51b 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -1492,10 +1492,21 @@ the intentionally sparse public response. GPU temperature, utilization, and VRAM readings. Linux may supplement `lm-sensors` with that query or use it as a best-effort fallback when `lm-sensors` is unavailable; Windows may use it as its supported direct-GPU - provider without implying support for CPU, motherboard, or storage - temperatures. The report must keep typed GPU readings in `sensors.gpu` - while mapping only direct `temperature.gpu` readings into existing - `sensors.temperatureCelsius` `gpu_nvidia_` keys for compatibility. + provider without implying support for CPU or motherboard temperatures. + Windows physical-disk temperature is a separate best-effort native source: + `internal/hostagent/windows_storage_sensors.go` may issue one fixed, + non-interactive, five-second Windows Storage-module query for at most 128 + physical disks and map only device-reported reliability temperatures from + `Get-StorageReliabilityCounter` into the existing `sensors.smart` physical + disk contract. Its JSON output, device identities, model labels, and + temperatures must be bounded and validated; absent cmdlets, missing + counters, malformed output, and unsupported devices omit that telemetry + without failing the host report. This path must not use ACPI thermal zones, + infer disk health, execute server-authored PowerShell, or imply support for + CPU or motherboard temperatures. The report must keep typed GPU readings in + `sensors.gpu` while mapping only direct `temperature.gpu` readings into + existing `sensors.temperatureCelsius` `gpu_nvidia_` keys for + compatibility. Authenticated server ingest may reduce those typed samples into bounded host-level GPU utilization, VRAM pressure, and GPU temperature history on the existing agent identity, but that monitoring projection does not change diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 9e0d2ce84..614a7f025 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1279,6 +1279,14 @@ the shared model conversion helpers must preserve `sensors.thermalState` through ingest, read-state projection, and frontend conversion, while leaving `agent.temperature` and `metric=temperature` unset unless a real Celsius value exists. +Windows Storage-module reliability temperatures use the existing host-agent +physical-disk route rather than a provider-specific monitoring payload. +Authenticated ingest must preserve each validated `sensors.smart` device, +model, transport, capacity, temperature, and field-level +`windows-storage-reliability` provenance through the canonical host resource, +disk-temperature presentation, history, and alert boundaries. An unknown +health value remains unknown; monitoring must not convert the presence of a +temperature counter into SMART health evidence. Host-agent GPU sensor summaries follow that same descriptive-host-telemetry path. Monitoring must preserve typed GPU id, name, temperature, utilization, and VRAM readings from agent reports through models, read-state projection, and diff --git a/internal/hostagent/agent.go b/internal/hostagent/agent.go index 11624d9ff..e19029298 100644 --- a/internal/hostagent/agent.go +++ b/internal/hostagent/agent.go @@ -1548,10 +1548,10 @@ func (a *Agent) collectTemperatures(ctx context.Context) agentshost.Sensors { case "freebsd": return a.collectFreeBSDTemperatures(ctx) case "windows": - // Windows does not expose a reliable built-in CPU or motherboard - // temperature API. NVIDIA's driver does ship nvidia-smi, however, and - // the bounded query used on Linux is portable to Windows. - return a.collectNVIDIATemperatureSensors(ctx) + // Windows has no reliable built-in CPU or motherboard temperature API. + // Its Storage module does expose device-reported physical-disk + // reliability temperatures, and NVIDIA's driver supplies nvidia-smi. + return a.collectWindowsTemperatureSensors(ctx) default: return agentshost.Sensors{} } diff --git a/internal/hostagent/agent_metrics_test.go b/internal/hostagent/agent_metrics_test.go index d3da30cbd..20175dd7c 100644 --- a/internal/hostagent/agent_metrics_test.go +++ b/internal/hostagent/agent_metrics_test.go @@ -593,19 +593,28 @@ func TestBuildReportIncludesNVIDIASMITelemetryOnWindows(t *testing.T) { return hostmetrics.Snapshot{}, nil }, lookPathFn: func(file string) (string, error) { - if file == "nvidia-smi" { + switch file { + case "powershell.exe": + return `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`, nil + case "nvidia-smi": return `C:\Windows\System32\nvidia-smi.exe`, nil + default: + return "", os.ErrNotExist } - return "", os.ErrNotExist }, commandCombinedOutputFn: func(_ context.Context, name string, arg ...string) (string, error) { - if name != `C:\Windows\System32\nvidia-smi.exe` { - t.Fatalf("command name = %q, want Windows nvidia-smi path", name) + switch name { + case `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`: + return `[{"deviceId":"0","friendlyName":"Windows NVMe","busType":"NVMe","mediaType":"SSD","sizeBytes":1000000000000,"temperature":39}]`, nil + case `C:\Windows\System32\nvidia-smi.exe`: + if len(arg) != 2 || arg[0] != "--query-gpu=index,name,temperature.gpu,utilization.gpu,memory.used,memory.total" || arg[1] != "--format=csv,noheader,nounits" { + t.Fatalf("command args = %#v, want NVIDIA stats query", arg) + } + return "0, NVIDIA GeForce RTX 3070, 57, 31, 2048, 8192\r\n", nil + default: + t.Fatalf("unexpected Windows telemetry command %q", name) + return "", nil } - if len(arg) != 2 || arg[0] != "--query-gpu=index,name,temperature.gpu,utilization.gpu,memory.used,memory.total" || arg[1] != "--format=csv,noheader,nounits" { - t.Fatalf("command args = %#v, want NVIDIA stats query", arg) - } - return "0, NVIDIA GeForce RTX 3070, 57, 31, 2048, 8192\r\n", nil }, } @@ -639,6 +648,12 @@ func TestBuildReportIncludesNVIDIASMITelemetryOnWindows(t *testing.T) { if report.Sensors.GPU[0].MemoryTotalBytes == nil || *report.Sensors.GPU[0].MemoryTotalBytes != 8192*1024*1024 { t.Fatalf("Windows GPU memory total = %#v, want 8192 MiB", report.Sensors.GPU[0].MemoryTotalBytes) } + if len(report.Sensors.SMART) != 1 || + report.Sensors.SMART[0].Device != "PhysicalDisk0" || + report.Sensors.SMART[0].Type != "nvme" || + report.Sensors.SMART[0].Temperature != 39 { + t.Fatalf("Windows storage temperatures = %+v, want native PhysicalDisk0 reading", report.Sensors.SMART) + } } func TestBuildReportUsesResolvedNASOSIdentity(t *testing.T) { diff --git a/internal/hostagent/agent_sensors_test.go b/internal/hostagent/agent_sensors_test.go index 5e7a91e87..8f3fa0fc1 100644 --- a/internal/hostagent/agent_sensors_test.go +++ b/internal/hostagent/agent_sensors_test.go @@ -250,6 +250,9 @@ func TestAgent_collectTemperatures_CollectsNVIDIAOnWindows(t *testing.T) { mc := &mockCollector{ goos: "windows", lookPathFn: func(file string) (string, error) { + if file == "powershell.exe" || file == "powershell" { + return "", os.ErrNotExist + } if file != "nvidia-smi" { t.Fatalf("look path file = %q, want nvidia-smi", file) } diff --git a/internal/hostagent/windows_storage_sensors.go b/internal/hostagent/windows_storage_sensors.go new file mode 100644 index 000000000..e5a094aa8 --- /dev/null +++ b/internal/hostagent/windows_storage_sensors.go @@ -0,0 +1,241 @@ +package hostagent + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "math" + "os" + "os/exec" + "sort" + "strconv" + "strings" + "time" + + agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" +) + +const ( + windowsStorageQueryTimeout = 5 * time.Second + windowsStorageMaxOutputBytes = 64 * 1024 + windowsStorageMaxDisks = 128 + windowsStorageSource = "windows-storage-reliability" +) + +// The script is fixed agent code, not operator or server input. It asks the +// built-in Windows Storage module for only the bounded fields Pulse reports. +const windowsStorageTemperatureScript = `$ErrorActionPreference = 'Stop' +$rows = @( + Get-PhysicalDisk -ErrorAction Stop | + Select-Object -First 128 | + ForEach-Object { + $disk = $_ + $counter = $disk | Get-StorageReliabilityCounter -ErrorAction SilentlyContinue + if ($null -ne $counter -and $null -ne $counter.Temperature) { + $name = [string]$disk.FriendlyName + if ($name.Length -gt 128) { $name = $name.Substring(0, 128) } + $deviceId = [string]$disk.DeviceId + if ($deviceId.Length -gt 32) { $deviceId = $deviceId.Substring(0, 32) } + [pscustomobject]@{ + deviceId = $deviceId + friendlyName = $name + busType = [string]$disk.BusType + mediaType = [string]$disk.MediaType + sizeBytes = [uint64]$disk.Size + temperature = [double]$counter.Temperature + } + } + } +) +ConvertTo-Json -InputObject $rows -Compress` + +type windowsStorageTemperatureReading struct { + DeviceID string `json:"deviceId"` + FriendlyName string `json:"friendlyName"` + BusType string `json:"busType"` + MediaType string `json:"mediaType"` + SizeBytes *int64 `json:"sizeBytes"` + Temperature *float64 `json:"temperature"` +} + +func (a *Agent) collectWindowsTemperatureSensors(ctx context.Context) agentshost.Sensors { + result := a.collectWindowsStorageTemperatures(ctx) + a.mergeNVIDIATemperatures(ctx, &result) + return result +} + +func (a *Agent) collectWindowsStorageTemperatures(ctx context.Context) agentshost.Sensors { + powerShellPath, err := a.resolveWindowsPowerShell() + if err != nil { + if !errors.Is(err, exec.ErrNotFound) && !os.IsNotExist(err) { + a.logger.Debug().Err(err).Msg("Failed to locate Windows PowerShell for storage temperatures") + } + return agentshost.Sensors{} + } + + queryCtx, cancel := context.WithTimeout(ctx, windowsStorageQueryTimeout) + defer cancel() + + output, err := a.collector.CommandCombinedOutput( + queryCtx, + powerShellPath, + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + windowsStorageTemperatureScript, + ) + if err != nil { + a.logger.Debug().Err(err).Msg("Failed to collect Windows storage reliability temperatures") + return agentshost.Sensors{} + } + + disks, err := parseWindowsStorageTemperatures(output) + if err != nil { + a.logger.Debug().Err(err).Msg("Failed to parse Windows storage reliability temperatures") + return agentshost.Sensors{} + } + if len(disks) == 0 { + return agentshost.Sensors{} + } + + a.logger.Debug(). + Int("diskCount", len(disks)). + Msg("Collected Windows storage reliability temperatures") + return agentshost.Sensors{SMART: disks} +} + +func (a *Agent) resolveWindowsPowerShell() (string, error) { + var lastErr error + for _, candidate := range []string{"powershell.exe", "powershell"} { + path, err := a.collector.LookPath(candidate) + if err == nil { + return path, nil + } + lastErr = err + if !errors.Is(err, exec.ErrNotFound) && !os.IsNotExist(err) { + return "", fmt.Errorf("locate %s: %w", candidate, err) + } + } + if lastErr == nil { + lastErr = exec.ErrNotFound + } + return "", lastErr +} + +func parseWindowsStorageTemperatures(output string) ([]agentshost.DiskSMART, error) { + if len(output) > windowsStorageMaxOutputBytes { + return nil, fmt.Errorf( + "Windows storage reliability output exceeds %d bytes", + windowsStorageMaxOutputBytes, + ) + } + + output = strings.TrimSpace(strings.TrimPrefix(output, "\ufeff")) + if output == "" || output == "null" { + return nil, nil + } + + var readings []windowsStorageTemperatureReading + if err := json.Unmarshal([]byte(output), &readings); err != nil { + var single windowsStorageTemperatureReading + if singleErr := json.Unmarshal([]byte(output), &single); singleErr != nil { + return nil, err + } + readings = []windowsStorageTemperatureReading{single} + } + if len(readings) > windowsStorageMaxDisks { + readings = readings[:windowsStorageMaxDisks] + } + + result := make([]agentshost.DiskSMART, 0, len(readings)) + seen := make(map[string]struct{}, len(readings)) + for index, reading := range readings { + if reading.Temperature == nil || + math.IsNaN(*reading.Temperature) || + math.IsInf(*reading.Temperature, 0) || + *reading.Temperature <= 0 || + *reading.Temperature > 150 { + continue + } + + deviceID := normalizeWindowsStorageDeviceID(reading.DeviceID, index) + device := "PhysicalDisk" + deviceID + key := strings.ToLower(device) + if _, exists := seen[key]; exists { + continue + } + seen[key] = struct{}{} + + sizeBytes := int64(0) + if reading.SizeBytes != nil && *reading.SizeBytes > 0 { + sizeBytes = *reading.SizeBytes + } + result = append(result, agentshost.DiskSMART{ + Device: device, + Model: truncateWindowsStorageLabel(reading.FriendlyName, 128), + Type: normalizeWindowsStorageType(reading.BusType, reading.MediaType), + SizeBytes: sizeBytes, + Temperature: int(math.Round(*reading.Temperature)), + Health: "UNKNOWN", + Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Available(windowsStorageSource), + }, + }) + } + + sort.Slice(result, func(i, j int) bool { + return result[i].Device < result[j].Device + }) + if len(result) == 0 { + return nil, nil + } + return result, nil +} + +func normalizeWindowsStorageDeviceID(value string, fallback int) string { + value = truncateWindowsStorageLabel(strings.TrimSpace(value), 32) + var normalized strings.Builder + normalized.Grow(len(value)) + for _, r := range value { + if (r >= 'a' && r <= 'z') || + (r >= 'A' && r <= 'Z') || + (r >= '0' && r <= '9') || + r == '-' || + r == '_' || + r == '.' { + normalized.WriteRune(r) + } else { + normalized.WriteByte('_') + } + } + result := strings.Trim(normalized.String(), "_.-") + if result == "" { + return strconv.Itoa(fallback) + } + return result +} + +func normalizeWindowsStorageType(busType, mediaType string) string { + switch normalized := strings.ToLower(strings.TrimSpace(busType)); normalized { + case "nvme", "sata", "sas", "usb": + return normalized + } + switch normalized := strings.ToLower(strings.TrimSpace(mediaType)); normalized { + case "ssd", "hdd", "scm": + return normalized + default: + return "" + } +} + +func truncateWindowsStorageLabel(value string, maxRunes int) string { + value = strings.TrimSpace(value) + runes := []rune(value) + if len(runes) <= maxRunes { + return value + } + return string(runes[:maxRunes]) +} diff --git a/internal/hostagent/windows_storage_sensors_test.go b/internal/hostagent/windows_storage_sensors_test.go new file mode 100644 index 000000000..163378079 --- /dev/null +++ b/internal/hostagent/windows_storage_sensors_test.go @@ -0,0 +1,189 @@ +package hostagent + +import ( + "context" + "errors" + "os" + "strings" + "testing" + + "github.com/rs/zerolog" +) + +func TestAgentCollectWindowsTemperatureSensorsMergesNativeStorageAndNVIDIA(t *testing.T) { + const powerShellPath = `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe` + const nvidiaPath = `C:\Windows\System32\nvidia-smi.exe` + + collector := &mockCollector{ + goos: "windows", + lookPathFn: func(file string) (string, error) { + switch file { + case "powershell.exe": + return powerShellPath, nil + case "nvidia-smi": + return nvidiaPath, nil + default: + return "", os.ErrNotExist + } + }, + commandCombinedOutputFn: func(_ context.Context, name string, args ...string) (string, error) { + switch name { + case powerShellPath: + if len(args) != 5 || + args[0] != "-NoLogo" || + args[1] != "-NoProfile" || + args[2] != "-NonInteractive" || + args[3] != "-Command" || + args[4] != windowsStorageTemperatureScript { + t.Fatalf("unexpected PowerShell args: %#v", args) + } + return `[ + {"deviceId":"1","friendlyName":"Samsung SSD 990 PRO","busType":"NVMe","mediaType":"SSD","sizeBytes":2000398934016,"temperature":42}, + {"deviceId":"0","friendlyName":"Archive Disk","busType":"SATA","mediaType":"HDD","sizeBytes":4000787030016,"temperature":35} + ]`, nil + case nvidiaPath: + return "0, NVIDIA GeForce RTX 4090, 61, 7, 4096, 24576\r\n", nil + default: + t.Fatalf("unexpected command %q", name) + return "", nil + } + }, + } + agent := &Agent{logger: zerolog.Nop(), collector: collector} + + got := agent.collectTemperatures(context.Background()) + if len(got.SMART) != 2 { + t.Fatalf("Windows storage disks = %d, want 2: %+v", len(got.SMART), got.SMART) + } + if got.SMART[0].Device != "PhysicalDisk0" || + got.SMART[0].Model != "Archive Disk" || + got.SMART[0].Type != "sata" || + got.SMART[0].Temperature != 35 { + t.Fatalf("unexpected first storage disk: %+v", got.SMART[0]) + } + if got.SMART[1].Device != "PhysicalDisk1" || + got.SMART[1].Model != "Samsung SSD 990 PRO" || + got.SMART[1].Type != "nvme" || + got.SMART[1].SizeBytes != 2000398934016 || + got.SMART[1].Temperature != 42 { + t.Fatalf("unexpected second storage disk: %+v", got.SMART[1]) + } + if got.SMART[1].Collection == nil || + got.SMART[1].Collection.Temperature.Source != windowsStorageSource || + got.SMART[1].Collection.Temperature.State != "available" { + t.Fatalf("storage temperature provenance = %+v", got.SMART[1].Collection) + } + if got.TemperatureCelsius["gpu_nvidia_0"] != 61 { + t.Fatalf("NVIDIA temperature = %v, want 61", got.TemperatureCelsius["gpu_nvidia_0"]) + } + if len(got.GPU) != 1 || got.GPU[0].UtilizationPercent == nil || *got.GPU[0].UtilizationPercent != 7 { + t.Fatalf("typed NVIDIA telemetry = %+v", got.GPU) + } +} + +func TestParseWindowsStorageTemperaturesValidatesAndBoundsReadings(t *testing.T) { + longName := strings.Repeat("温", 140) + output := `[ + {"deviceId":" 0 ","friendlyName":"` + longName + `","busType":"USB","mediaType":"SSD","sizeBytes":1000,"temperature":40.6}, + {"deviceId":"0","friendlyName":"duplicate","busType":"NVMe","temperature":50}, + {"deviceId":"bad id/with spaces","friendlyName":"Disk B","busType":"Unknown","mediaType":"SSD","sizeBytes":-1,"temperature":33}, + {"deviceId":"hot","temperature":151}, + {"deviceId":"zero","temperature":0}, + {"deviceId":"missing"} + ]` + + got, err := parseWindowsStorageTemperatures(output) + if err != nil { + t.Fatalf("parseWindowsStorageTemperatures returned error: %v", err) + } + if len(got) != 2 { + t.Fatalf("parsed disks = %d, want 2: %+v", len(got), got) + } + if got[0].Device != "PhysicalDisk0" || got[0].Temperature != 41 || len([]rune(got[0].Model)) != 128 { + t.Fatalf("bounded first disk = %+v", got[0]) + } + if got[1].Device != "PhysicalDiskbad_id_with_spaces" || + got[1].Type != "ssd" || + got[1].SizeBytes != 0 || + got[1].Temperature != 33 { + t.Fatalf("normalized second disk = %+v", got[1]) + } +} + +func TestParseWindowsStorageTemperaturesAcceptsSingleObjectAndEmptyOutput(t *testing.T) { + got, err := parseWindowsStorageTemperatures( + `{"deviceId":"7","friendlyName":"Single Disk","busType":"SAS","temperature":29}`, + ) + if err != nil { + t.Fatalf("parse single object: %v", err) + } + if len(got) != 1 || got[0].Device != "PhysicalDisk7" || got[0].Type != "sas" { + t.Fatalf("single object result = %+v", got) + } + + for _, output := range []string{"", "null", "\ufeff [] "} { + got, err := parseWindowsStorageTemperatures(output) + if err != nil || got != nil { + t.Fatalf("empty output %q = (%+v, %v), want nil, nil", output, got, err) + } + } +} + +func TestParseWindowsStorageTemperaturesRejectsMalformedAndOversizedOutput(t *testing.T) { + if _, err := parseWindowsStorageTemperatures("{"); err == nil { + t.Fatal("expected malformed JSON error") + } + if _, err := parseWindowsStorageTemperatures(strings.Repeat("x", windowsStorageMaxOutputBytes+1)); err == nil { + t.Fatal("expected oversized output error") + } +} + +func TestAgentCollectWindowsStorageTemperaturesIsBestEffort(t *testing.T) { + tests := []struct { + name string + lookPath func(string) (string, error) + runOutput func(context.Context, string, ...string) (string, error) + }{ + { + name: "PowerShell missing", + lookPath: func(string) (string, error) { + return "", os.ErrNotExist + }, + }, + { + name: "query fails", + lookPath: func(string) (string, error) { + return `C:\powershell.exe`, nil + }, + runOutput: func(context.Context, string, ...string) (string, error) { + return "", errors.New("storage provider unavailable") + }, + }, + { + name: "query returns invalid JSON", + lookPath: func(string) (string, error) { + return `C:\powershell.exe`, nil + }, + runOutput: func(context.Context, string, ...string) (string, error) { + return "{", nil + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + agent := &Agent{ + logger: zerolog.Nop(), + collector: &mockCollector{ + goos: "windows", + lookPathFn: tc.lookPath, + commandCombinedOutputFn: tc.runOutput, + }, + } + got := agent.collectWindowsStorageTemperatures(context.Background()) + if len(got.SMART) != 0 { + t.Fatalf("best-effort result = %+v, want empty", got) + } + }) + } +}