Skip to content

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

Open
SaiPisey2 wants to merge 1 commit into
prometheus:masterfrom
SaiPisey2:fix/thermal-darwin-no-cpu-power-status
Open

fix(collector): keep collecting temperatures without CPU power status#3767
SaiPisey2 wants to merge 1 commit into
prometheus:masterfrom
SaiPisey2:fix/thermal-darwin-no-cpu-power-status

Conversation

@SaiPisey2

Copy link
Copy Markdown

Addresses #2906.

Apple Silicon does not implement IOPMCopyCPUPowerStatus, so fetchCPUPowerStatus gets back kIOReturnNotFound. Update returned that error straight away, which meant it never reached updateTemperatures, so no temperature metrics were collected at all. The error also isn't ErrNoData, so it was logged at error level on every scrape.

Measured on an M5 Pro (darwin/arm64) against master:

fetchCPUPowerStatus()      -> status=map[] err=no CPU power status has been recorded
Update()                   -> err=no CPU power status has been recorded ; metrics emitted=0
IsNoDataError(err)         -> false
updateTemperatures() alone -> err=<nil> ; temperature metrics available=52

52 usable temperature sensors on that machine, none of them exported, because an expected and unrelated condition aborted the collector.

Change

Treat kIOReturnNotFound as "this system does not report CPU power status" rather than a failure: skip the three CPU power metrics, log at debug level, and continue on to the temperature sensors. Any other non-success return code is still returned as an error, exactly as before, so systems that do report CPU power status are unaffected.

After the change, on the same machine, Update() returns no error and emits all 52 temperature metrics.

Scope

This does not make the CPU power metrics appear on Apple Silicon and does not resolve #2218 — the underlying API provides no data there, as @rexagod already established on #2906. It only stops their absence from suppressing the temperature metrics. I've left Closes #2906 out of the commit deliberately, since whether that issue is fully answered by this is a maintainer call.

Testing

Added collector/thermal_darwin_test.go, which fails on master:

--- FAIL: TestThermalUpdateWithoutCPUPowerStatus
    thermal_darwin_test.go:44: Update returned errNoCPUPowerStatus; a system
    without CPU power status must still collect temperatures

and passes with the change. Also verified locally:

  • go build ./...
  • go vet ./collector/
  • gofmt -l clean on both touched files
  • go test ./collector/ full package
  • go build -tags notherm ./collector/

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 <piseysai0202@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SIGTRAP: trace trap on M1

1 participant