From 3279e73b6c582baaca674ae7e32bb1c02b076561 Mon Sep 17 00:00:00 2001 From: SaiPisey2 Date: Fri, 31 Jul 2026 20:41:26 +0530 Subject: [PATCH 1/2] fix(collector): keep collecting temperatures without CPU power status Apple Silicon does not implement IOPMCopyCPUPowerStatus, so fetchCPUPowerStatus returns kIOReturnNotFound there. Update returned that error straight away, which aborted the collector before updateTemperatures ran, so no temperature metrics were collected at all. The error was also not ErrNoData, so it was logged at error level on every scrape. On an M5 Pro, Update emitted 0 metrics and failed, while updateTemperatures on its own returned 52 temperature metrics. Treat kIOReturnNotFound as a system that does not report CPU power status: skip the three CPU power metrics, log at debug level and carry on to the temperature sensors. Any other non-success return code is still returned as an error. Systems that do report CPU power status are unaffected. This does not make the CPU power metrics available on Apple Silicon, since the underlying API provides no data. It only stops their absence from suppressing the temperature metrics. Adds a regression test covering the case. Signed-off-by: SaiPisey2 --- collector/thermal_darwin.go | 34 ++++++++++++++------- collector/thermal_darwin_test.go | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 11 deletions(-) create mode 100644 collector/thermal_darwin_test.go diff --git a/collector/thermal_darwin.go b/collector/thermal_darwin.go index c55b9b2845..0a126cf2e3 100644 --- a/collector/thermal_darwin.go +++ b/collector/thermal_darwin.go @@ -62,6 +62,11 @@ type thermCollector struct { const thermal = "thermal" +// errNoCPUPowerStatus is returned when the system does not report any CPU power +// status. Apple Silicon does not implement IOPMCopyCPUPowerStatus, so this is an +// expected condition on those systems rather than a failure. +var errNoCPUPowerStatus = errors.New("no CPU power status has been recorded") + func init() { registerCollector(thermal, defaultEnabled, NewThermCollector) } @@ -110,18 +115,25 @@ func NewThermCollector(logger *slog.Logger) (Collector, error) { func (c *thermCollector) Update(ch chan<- prometheus.Metric) error { cpuPowerStatus, err := fetchCPUPowerStatus() - if err != nil { + switch { + case err == nil: + if value, ok := cpuPowerStatus[(string(C.kIOPMCPUPowerLimitSchedulerTimeKey))]; ok { + ch <- c.cpuSchedulerLimit.mustNewConstMetric(float64(value) / 100.0) + } + if value, ok := cpuPowerStatus[(string(C.kIOPMCPUPowerLimitProcessorCountKey))]; ok { + ch <- c.cpuAvailableCPU.mustNewConstMetric(float64(value)) + } + if value, ok := cpuPowerStatus[(string(C.kIOPMCPUPowerLimitProcessorSpeedKey))]; ok { + ch <- c.cpuSpeedLimit.mustNewConstMetric(float64(value) / 100.0) + } + case errors.Is(err, errNoCPUPowerStatus): + // Apple Silicon does not report CPU power status. The temperature + // sensors collected below are still available, so this must not abort + // the collector. + c.logger.Debug("No CPU power status reported by the system, skipping CPU power metrics") + default: return err } - if value, ok := cpuPowerStatus[(string(C.kIOPMCPUPowerLimitSchedulerTimeKey))]; ok { - ch <- c.cpuSchedulerLimit.mustNewConstMetric(float64(value) / 100.0) - } - if value, ok := cpuPowerStatus[(string(C.kIOPMCPUPowerLimitProcessorCountKey))]; ok { - ch <- c.cpuAvailableCPU.mustNewConstMetric(float64(value)) - } - if value, ok := cpuPowerStatus[(string(C.kIOPMCPUPowerLimitProcessorSpeedKey))]; ok { - ch <- c.cpuSpeedLimit.mustNewConstMetric(float64(value) / 100.0) - } return c.updateTemperatures(ch) } @@ -135,7 +147,7 @@ func fetchCPUPowerStatus() (map[string]int, error) { }() if C.kIOReturnNotFound == cfDictRef.ret { - return nil, errors.New("no CPU power status has been recorded") + return nil, errNoCPUPowerStatus } if C.kIOReturnSuccess != cfDictRef.ret { diff --git a/collector/thermal_darwin_test.go b/collector/thermal_darwin_test.go new file mode 100644 index 0000000000..511fb98cc4 --- /dev/null +++ b/collector/thermal_darwin_test.go @@ -0,0 +1,52 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !notherm && darwin && cgo + +package collector + +import ( + "errors" + "io" + "log/slog" + "testing" + + "github.com/prometheus/client_golang/prometheus" +) + +// Apple Silicon does not implement IOPMCopyCPUPowerStatus, so fetchCPUPowerStatus +// reports errNoCPUPowerStatus there. That is an expected condition and must not +// abort the collector, otherwise the temperature sensors, which are read after +// the CPU power status, are never collected. +func TestThermalUpdateWithoutCPUPowerStatus(t *testing.T) { + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + c, err := NewThermCollector(logger) + if err != nil { + t.Fatalf("failed to create collector: %v", err) + } + + ch := make(chan prometheus.Metric, 1024) + err = c.Update(ch) + close(ch) + + if errors.Is(err, errNoCPUPowerStatus) { + t.Fatal("Update returned errNoCPUPowerStatus; a system without CPU power status must still collect temperatures") + } + if err != nil { + t.Fatalf("Update failed: %v", err) + } + + for range ch { + } +} From 907bf2fc72a0d7fb217fea9436feb871b00734e2 Mon Sep 17 00:00:00 2001 From: SaiPisey2 Date: Thu, 6 Aug 2026 15:51:44 +0530 Subject: [PATCH 2/2] fix(collector): report each thermal sensor once and update e2e fixture Collecting temperatures on Apple Silicon surfaced a second problem that was previously unreachable, because Update returned before updateTemperatures ever ran. A sensor is identified only by its IOHID "Product" name, and several services report the same name. On an M5 Pro, 76 temperature-capable services carry only 25 distinct names, and the services expose nothing that tells them apart: RegistryID, UniqueID and SerialNumber are all unset, and two services can share both Product and LocationID. Emitting the same label set twice makes the registry reject those samples and log an error on every scrape, so only the first reading for a name is reported now and the rest are counted in a debug message. Also update collector/fixtures/e2e-output-darwin.txt for node_scrape_collector_success{collector="thermal"}, which is now 1 because the collector no longer fails. Two fixes to end-to-end-test.sh so that regenerating that fixture on a Darwin host does the right thing: - The Darwin fixture path was built with "${fixture_metrics::-4}". A negative substring length needs bash 4.2 and macOS still ships bash 3.2, where the expansion fails and fixture_metrics keeps pointing at the Linux file. Running ./end-to-end-test.sh -u on macOS therefore overwrote collector/fixtures/e2e-output.txt with Darwin output. "${fixture_metrics%.txt}" is equivalent and portable. - node_thermal_temperature_celsius is per-machine, both in its sensor names and its values, so it is stripped along with the other non-deterministic metrics. The CI runner reports no sensors at all, but a developer running the suite on real hardware would otherwise see the whole sensor list as a diff. Signed-off-by: SaiPisey2 --- collector/fixtures/e2e-output-darwin.txt | 2 +- collector/thermal_darwin_arm64.go | 21 ++++++++++++++ collector/thermal_darwin_test.go | 37 ++++++++++++++++++++++++ end-to-end-test.sh | 6 +++- 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/collector/fixtures/e2e-output-darwin.txt b/collector/fixtures/e2e-output-darwin.txt index cf29fb473c..a968780066 100644 --- a/collector/fixtures/e2e-output-darwin.txt +++ b/collector/fixtures/e2e-output-darwin.txt @@ -159,7 +159,7 @@ node_scrape_collector_success{collector="netdev"} 1 node_scrape_collector_success{collector="os"} 1 node_scrape_collector_success{collector="powersupplyclass"} 1 node_scrape_collector_success{collector="textfile"} 1 -node_scrape_collector_success{collector="thermal"} 0 +node_scrape_collector_success{collector="thermal"} 1 node_scrape_collector_success{collector="time"} 1 node_scrape_collector_success{collector="xfrm"} 1 # HELP node_textfile_mtime_seconds Unixtime mtime of textfiles successfully read. diff --git a/collector/thermal_darwin_arm64.go b/collector/thermal_darwin_arm64.go index 24558a1c9d..7bc0bf1317 100644 --- a/collector/thermal_darwin_arm64.go +++ b/collector/thermal_darwin_arm64.go @@ -101,6 +101,16 @@ func (c *thermCollector) updateTemperatures(ch chan<- prometheus.Metric) error { cfProdKey := C.CFStringCreateWithCString(C.kCFAllocatorDefault, prodKey, C.kCFStringEncodingUTF8) defer C.CFRelease(C.CFTypeRef(cfProdKey)) + // A sensor is identified only by its product name. Several services report + // the same name, either because the same sensor is listed more than once or + // because two distinct sensors share a name, and the services carry no + // property that tells them apart: RegistryID, UniqueID and SerialNumber are + // all unset here. Emitting the same label set twice makes the registry + // reject those samples and fail the whole scrape, so only the first reading + // for a name is reported. + seen := make(map[string]struct{}, int(count)) + skipped := 0 + for i := 0; i < int(count); i++ { service := C.CFArrayGetValueAtIndex(services, C.CFIndex(i)) @@ -125,8 +135,19 @@ func (c *thermCollector) updateTemperatures(ch chan<- prometheus.Metric) error { C.CFRelease(C.CFTypeRef(nameRef)) } + if _, duplicate := seen[name]; duplicate { + skipped++ + continue + } + seen[name] = struct{}{} + ch <- c.temperature.mustNewConstMetric(float64(temp), name) } + + if skipped > 0 { + c.logger.Debug("Skipped thermal sensors reporting a duplicate name", "skipped", skipped, "reported", len(seen)) + } + return nil } diff --git a/collector/thermal_darwin_test.go b/collector/thermal_darwin_test.go index 511fb98cc4..4d8624ea1b 100644 --- a/collector/thermal_darwin_test.go +++ b/collector/thermal_darwin_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" ) // Apple Silicon does not implement IOPMCopyCPUPowerStatus, so fetchCPUPowerStatus @@ -50,3 +51,39 @@ func TestThermalUpdateWithoutCPUPowerStatus(t *testing.T) { for range ch { } } + +// Several IOHID services report the same sensor name and carry no property that +// tells them apart, so the collector must report each name once. Duplicate label +// sets are rejected by the registry and fail the whole scrape. +func TestThermalTemperaturesAreUnique(t *testing.T) { + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + c, err := NewThermCollector(logger) + if err != nil { + t.Fatalf("failed to create collector: %v", err) + } + + ch := make(chan prometheus.Metric, 4096) + if err := c.Update(ch); err != nil { + t.Fatalf("Update failed: %v", err) + } + close(ch) + + seen := make(map[string]struct{}) + for m := range ch { + var pb dto.Metric + if err := m.Write(&pb); err != nil { + t.Fatalf("cannot read metric: %v", err) + } + + key := m.Desc().String() + for _, l := range pb.GetLabel() { + key += "," + l.GetName() + "=" + l.GetValue() + } + + if _, duplicate := seen[key]; duplicate { + t.Errorf("duplicate metric collected: %s", key) + } + seen[key] = struct{}{} + } +} diff --git a/end-to-end-test.sh b/end-to-end-test.sh index b439cea556..a4473a3809 100755 --- a/end-to-end-test.sh +++ b/end-to-end-test.sh @@ -279,7 +279,10 @@ generated_metrics="${tmpdir}/e2e-output.txt" for os in freebsd openbsd netbsd solaris dragonfly darwin; do if [ "${GOHOSTOS}" = "${os}" ]; then generated_metrics="${tmpdir}/e2e-output-${GOHOSTOS}.txt" - fixture_metrics="${fixture_metrics::-4}-${GOHOSTOS}.txt" + # Not "${fixture_metrics::-4}": a negative length needs bash 4.2, and macOS + # still ships bash 3.2, where the expansion fails and the Linux fixture is + # used instead. + fixture_metrics="${fixture_metrics%.txt}-${GOHOSTOS}.txt" fi done @@ -390,6 +393,7 @@ non_deterministic_metrics=$(cat << METRICS node_network_receive_bytes_total node_network_receive_multicast_total node_network_transmit_multicast_total + node_thermal_temperature_celsius node_zfs_abdstats_linear_count_total node_zfs_abdstats_linear_data_bytes node_zfs_abdstats_scatter_chunk_waste_bytes