fix(collector): keep collecting temperatures without CPU power status

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.

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 {

collector/thermal_darwin_arm64.go

@@ -39,17 +39,31 @@ CFArrayRef IOHIDEventSystemClientCopyServices(IOHIDEventSystemClientRef client);
 IOHIDEventRef IOHIDServiceClientCopyEvent(IOHIDServiceClientRef service, int64_t type, int32_t options, int64_t timestamp);
 double IOHIDEventGetFloatValue(IOHIDEventRef event, int32_t field);
 CFTypeRef IOHIDServiceClientCopyProperty(IOHIDServiceClientRef service, CFStringRef key);
+uint64_t IOHIDServiceClientGetRegistryID(IOHIDServiceClientRef service);
 */
 import "C"
 
 import (
+	"strconv"
 	"unsafe"
 
 	"github.com/prometheus/client_golang/prometheus"
 )
 
 const absoluteZeroCelsius = -273.15
 
+// A thermal sensor is labelled with its IOHID "Product" property, which is not
+// unique. Apple Silicon reports several services under one product name: some
+// are distinct sensors that only share the name and can be told apart by their
+// location, while others share the location too and differ only by the registry
+// ID of the service.
+type thermalSensor struct {
+	name       string
+	location   string
+	registryID string
+	temp       float64
+}
+
 func (c *thermCollector) updateTemperatures(ch chan<- prometheus.Metric) error {
 	client := C.IOHIDEventSystemClientCreate(C.kCFAllocatorDefault)
 	if client == nil {
@@ -101,10 +115,18 @@ func (c *thermCollector) updateTemperatures(ch chan<- prometheus.Metric) error {
 	cfProdKey := C.CFStringCreateWithCString(C.kCFAllocatorDefault, prodKey, C.kCFStringEncodingUTF8)
 	defer C.CFRelease(C.CFTypeRef(cfProdKey))
 
+	locKey := C.CString("LocationID")
+	defer C.free(unsafe.Pointer(locKey))
+	cfLocKey := C.CFStringCreateWithCString(C.kCFAllocatorDefault, locKey, C.kCFStringEncodingUTF8)
+	defer C.CFRelease(C.CFTypeRef(cfLocKey))
+
+	// Read every sensor first, so that colliding product names can be detected
+	// before any metric is emitted.
+	sensors := make([]thermalSensor, 0, int(count))
 	for i := 0; i < int(count); i++ {
-		service := C.CFArrayGetValueAtIndex(services, C.CFIndex(i))
+		service := (C.IOHIDServiceClientRef)(C.CFArrayGetValueAtIndex(services, C.CFIndex(i)))
 
-		event := C.IOHIDServiceClientCopyEvent((C.IOHIDServiceClientRef)(service), C.kIOHIDEventTypeTemperature, 0, 0)
+		event := C.IOHIDServiceClientCopyEvent(service, C.kIOHIDEventTypeTemperature, 0, 0)
 		if event == nil {
 			continue
 		}
@@ -118,18 +140,80 @@ func (c *thermCollector) updateTemperatures(ch chan<- prometheus.Metric) error {
 			continue
 		}
 
-		nameRef := C.IOHIDServiceClientCopyProperty((C.IOHIDServiceClientRef)(service), cfProdKey)
+		nameRef := C.IOHIDServiceClientCopyProperty(service, cfProdKey)
 		name := "Unknown"
 		if nameRef != 0 {
 			name = cfStringToString((C.CFStringRef)(nameRef))
 			C.CFRelease(C.CFTypeRef(nameRef))
 		}
 
-		ch <- c.temperature.mustNewConstMetric(float64(temp), name)
+		sensors = append(sensors, thermalSensor{
+			name:       name,
+			location:   serviceNumberProperty(service, cfLocKey),
+			registryID: strconv.FormatUint(uint64(C.IOHIDServiceClientGetRegistryID(service)), 10),
+			temp:       float64(temp),
+		})
 	}
+
+	for i, label := range resolveSensorNames(sensors) {
+		ch <- c.temperature.mustNewConstMetric(sensors[i].temp, label)
+	}
+
 	return nil
 }
 
+// resolveSensorNames returns the sensor label to report for each sensor, in the
+// order the sensors were given.
+//
+// Emitting the same label set twice makes the registry reject the samples and
+// fail the whole scrape. A product name shared by several services is therefore
+// qualified with the service location, falling back to the registry ID, which is
+// unique per service, when the location does not tell them apart either.
+func resolveSensorNames(sensors []thermalSensor) []string {
+	nameCount := make(map[string]int, len(sensors))
+	nameLocationCount := make(map[string]int, len(sensors))
+	for _, s := range sensors {
+		nameCount[s.name]++
+		nameLocationCount[s.name+"\x00"+s.location]++
+	}
+
+	labels := make([]string, 0, len(sensors))
+	for _, s := range sensors {
+		label := s.name
+		if nameCount[s.name] > 1 {
+			suffix := s.location
+			if suffix == "" || nameLocationCount[s.name+"\x00"+s.location] > 1 {
+				suffix = s.registryID
+			}
+			label = s.name + "_" + suffix
+		}
+		labels = append(labels, label)
+	}
+
+	return labels
+}
+
+// serviceNumberProperty reads a numeric IOHID service property, returning an
+// empty string when it is absent or not a number.
+func serviceNumberProperty(service C.IOHIDServiceClientRef, key C.CFStringRef) string {
+	ref := C.IOHIDServiceClientCopyProperty(service, key)
+	if ref == 0 {
+		return ""
+	}
+	defer C.CFRelease(ref)
+
+	if C.CFGetTypeID(ref) != C.CFNumberGetTypeID() {
+		return ""
+	}
+
+	var value C.longlong
+	if C.CFNumberGetValue(C.CFNumberRef(ref), C.kCFNumberLongLongType, unsafe.Pointer(&value)) == 0 {
+		return ""
+	}
+
+	return strconv.FormatInt(int64(value), 10)
+}
+
 func cfStringToString(s C.CFStringRef) string {
 	p := C.CFStringGetCStringPtr(s, C.kCFStringEncodingUTF8)
 	if p != nil {

collector/thermal_darwin_arm64_test.go

@@ -0,0 +1,121 @@
+// 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 && arm64 && cgo
+
+package collector
+
+import "testing"
+
+func TestResolveSensorNames(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		sensors []thermalSensor
+		want    []string
+	}{
+		{
+			name:    "unique names are left alone",
+			sensors: []thermalSensor{{name: "NAND CH0 temp", location: "1", registryID: "10"}},
+			want:    []string{"NAND CH0 temp"},
+		},
+		{
+			// Several "gas gauge battery" services are distinct sensors that
+			// only share a product name.
+			name: "a shared name is qualified with the location",
+			sensors: []thermalSensor{
+				{name: "gas gauge battery", location: "1413951555", registryID: "10"},
+				{name: "gas gauge battery", location: "1413951574", registryID: "11"},
+			},
+			want: []string{"gas gauge battery_1413951555", "gas gauge battery_1413951574"},
+		},
+		{
+			// The PMU sensors report the same product name and the same
+			// location, so only the registry ID separates them.
+			name: "a shared location falls back to the registry ID",
+			sensors: []thermalSensor{
+				{name: "PMU tdie2", location: "1414541922", registryID: "10"},
+				{name: "PMU tdie2", location: "1414541922", registryID: "11"},
+				{name: "PMU tdie2", location: "1414541922", registryID: "12"},
+			},
+			want: []string{"PMU tdie2_10", "PMU tdie2_11", "PMU tdie2_12"},
+		},
+		{
+			name: "a missing location falls back to the registry ID",
+			sensors: []thermalSensor{
+				{name: "sensor", location: "", registryID: "10"},
+				{name: "sensor", location: "", registryID: "11"},
+			},
+			want: []string{"sensor_10", "sensor_11"},
+		},
+		{
+			// Only the services that cannot be told apart by location fall back
+			// to the registry ID.
+			name: "location and registry ID are mixed within one name",
+			sensors: []thermalSensor{
+				{name: "sensor", location: "1", registryID: "10"},
+				{name: "sensor", location: "2", registryID: "11"},
+				{name: "sensor", location: "2", registryID: "12"},
+			},
+			want: []string{"sensor_1", "sensor_11", "sensor_12"},
+		},
+		{
+			name:    "no sensors",
+			sensors: nil,
+			want:    []string{},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			got := resolveSensorNames(tc.sensors)
+			if len(got) != len(tc.want) {
+				t.Fatalf("got %d labels, want %d: %q", len(got), len(tc.want), got)
+			}
+			for i := range tc.want {
+				if got[i] != tc.want[i] {
+					t.Errorf("label %d: got %q, want %q", i, got[i], tc.want[i])
+				}
+			}
+
+			seen := make(map[string]struct{}, len(got))
+			for _, l := range got {
+				if _, dup := seen[l]; dup {
+					t.Errorf("duplicate label %q", l)
+				}
+				seen[l] = struct{}{}
+			}
+		})
+	}
+}
+
+// Every reading must be reported. Sensors sharing a product name are
+// disambiguated rather than dropped.
+func TestResolveSensorNamesKeepsEveryReading(t *testing.T) {
+	sensors := []thermalSensor{
+		{name: "a", location: "1", registryID: "10"},
+		{name: "a", location: "1", registryID: "11"},
+		{name: "a", location: "2", registryID: "12"},
+		{name: "b", location: "1", registryID: "13"},
+	}
+
+	got := resolveSensorNames(sensors)
+	if len(got) != len(sensors) {
+		t.Fatalf("got %d labels for %d sensors", len(got), len(sensors))
+	}
+
+	seen := make(map[string]struct{}, len(got))
+	for _, l := range got {
+		if _, dup := seen[l]; dup {
+			t.Errorf("duplicate label %q", l)
+		}
+		seen[l] = struct{}{}
+	}
+}