diff --git a/internal/sensors/collector.go b/internal/sensors/collector.go index b4f502dba8..894900e28e 100644 --- a/internal/sensors/collector.go +++ b/internal/sensors/collector.go @@ -2,6 +2,7 @@ package sensors import ( "context" + "encoding/json" "errors" "fmt" "io" @@ -26,9 +27,9 @@ var ( hwmonRoot = "/sys/class/hwmon" ) -// CollectLocal reads lm-sensors JSON, or a recognised CPU thermal sysfs source -// when lm-sensors is unavailable or returns no data. A valid lm-sensors result -// is kept intact so drive, GPU and fan readings are not lost. +// CollectLocal reads lm-sensors JSON and supplements it with a recognised CPU +// thermal sysfs source when it contains no CPU chip. Other lm-sensors readings +// are retained; sysfs is also used when lm-sensors is unavailable or empty. func CollectLocal(ctx context.Context) (string, error) { ctx = normalizeCollectionContext(ctx) @@ -47,7 +48,7 @@ func CollectLocal(ctx context.Context) (string, error) { } outputStr := strings.TrimSpace(string(output)) if outputStr != "" && outputStr != "{}" { - return outputStr, nil + return addMissingSysfsCPU(ctx, outputStr), nil } if cmdCtx.Err() != nil { return "", fmt.Errorf("failed to execute sensors: %w", cmdCtx.Err()) @@ -67,6 +68,37 @@ func CollectLocal(ctx context.Context) (string, error) { return "", fmt.Errorf("sensors returned empty output and CPU thermal fallback failed: %w", fallbackErr) } +// addMissingSysfsCPU leaves existing output unchanged unless it is a JSON +// object without a recognised CPU chip and an identified sysfs source is +// available. A failed optional lookup must not discard working sensors data. +func addMissingSysfsCPU(ctx context.Context, sensorsJSON string) string { + var chips map[string]json.RawMessage + if err := json.Unmarshal([]byte(sensorsJSON), &chips); err != nil || chips == nil { + return sensorsJSON + } + for name := range chips { + if isCPUChip(strings.ToLower(name)) { + return sensorsJSON + } + } + fallbackJSON, err := collectSysfsCPUTemperature(ctx) + if err != nil { + return sensorsJSON + } + var fallback map[string]json.RawMessage + if err := json.Unmarshal([]byte(fallbackJSON), &fallback); err != nil { + return sensorsJSON + } + for name, value := range fallback { + chips[name] = value + } + merged, err := json.Marshal(chips) + if err != nil || len(merged) > maxSensorsOutputSizeBytes { + return sensorsJSON + } + return string(merged) +} + // collectSysfsCPUTemperature deliberately accepts only identified CPU/SoC // sensors. thermal_zone0 is not necessarily a CPU on every Linux machine. func collectSysfsCPUTemperature(ctx context.Context) (string, error) { diff --git a/internal/sensors/collector_additional_test.go b/internal/sensors/collector_additional_test.go index b7a8dc4edd..90f75da625 100644 --- a/internal/sensors/collector_additional_test.go +++ b/internal/sensors/collector_additional_test.go @@ -82,7 +82,7 @@ func TestCollectLocal_SysfsContinuesPastBadSource(t *testing.T) { } } -func TestCollectLocal_SensorsJSONPrecedesSysfs(t *testing.T) { +func TestCollectLocal_SensorsJSONGainsMissingSysfsCPU(t *testing.T) { dir := t.TempDir() writeScript(t, dir, "sensors", "#!/bin/sh\necho '{\"nvme-pci-0100\":{\"Composite\":{\"temp1_input\":39}}}'\n") t.Setenv("PATH", dir+":"+os.Getenv("PATH")) @@ -92,8 +92,44 @@ func TestCollectLocal_SensorsJSONPrecedesSysfs(t *testing.T) { if err != nil { t.Fatal(err) } - if !strings.Contains(out, "nvme-pci-0100") || strings.Contains(out, "cpu_thermal") { - t.Fatalf("lm-sensors result was replaced: %s", out) + parsed, err := Parse(out) + if err != nil { + t.Fatal(err) + } + if parsed.CPUPackage != 57 || parsed.NVMe["nvme0"] != 39 { + t.Fatalf("lm-sensors data and sysfs CPU were not combined: %+v", parsed) + } +} + +func TestCollectLocal_ExistingCPUPrecedesSysfs(t *testing.T) { + dir := t.TempDir() + const original = `{"coretemp-isa-0000":{"Package id 0":{"temp1_input":42}}}` + writeScript(t, dir, "sensors", "#!/bin/sh\necho '"+original+"'\n") + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "57000") + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + if out != original { + t.Fatalf("existing CPU reading was changed: %s", out) + } +} + +func TestCollectLocal_InvalidSysfsDoesNotDiscardOtherSensors(t *testing.T) { + dir := t.TempDir() + const original = `{"nvme-pci-0100":{"Composite":{"temp1_input":39}}}` + writeScript(t, dir, "sensors", "#!/bin/sh\necho '"+original+"'\n") + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + thermal, _ := useTestSysfsRoots(t) + writeSysfsSensor(t, thermal, "thermal_zone0", "type", "armada_thermal", "bad") + out, err := CollectLocal(context.Background()) + if err != nil { + t.Fatal(err) + } + if out != original { + t.Fatalf("valid lm-sensors data was discarded: %s", out) } }