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.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_arm64.go b/collector/thermal_darwin_arm64.go index 24558a1c9d..7722a0df4a 100644 --- a/collector/thermal_darwin_arm64.go +++ b/collector/thermal_darwin_arm64.go @@ -39,10 +39,12 @@ 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" @@ -50,6 +52,18 @@ import ( 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 { diff --git a/collector/thermal_darwin_arm64_test.go b/collector/thermal_darwin_arm64_test.go new file mode 100644 index 0000000000..ba3e867a8b --- /dev/null +++ b/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{}{} + } +} diff --git a/collector/thermal_darwin_test.go b/collector/thermal_darwin_test.go new file mode 100644 index 0000000000..5b736169f0 --- /dev/null +++ b/collector/thermal_darwin_test.go @@ -0,0 +1,89 @@ +// 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" + dto "github.com/prometheus/client_model/go" +) + +// 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 { + } +} + +// Several IOHID services report the same product name, so the collector must +// qualify the colliding ones. 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{}{} + } +}