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 <piseysai0202@gmail.com>

Author: SaiPisey2

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_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
 }
 

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{}{}
+	}
+}

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