Add per-core CPU frequency & governor (D5)#55
Conversation
Reads /sys/devices/system/cpu/cpuN/cpufreq/{scaling_cur_freq,scaling_governor}
per core and adds them as an additive cpu_frequency array in the snapshot.
Availability of cpufreq varies across Pi models and kernels, so the
collector degrades to an empty reading (rather than an error) when no
cpufreq driver is present, and silently skips individual cores that are
offline or missing one of the two sysfs files.
There was a problem hiding this comment.
Reviewed against the project's Go/security/API conventions (.claude/skills/review-pr checklist). Everything checks out — this would be an approval, but GitHub doesn't allow approving one's own PR, so submitting as a comment.
Verification on the PR branch (a3545f4): go build ./..., go vet ./..., go test ./..., and golangci-lint run all pass (0 issues).
Checklist results:
- Error handling:
Collect()degrades to an empty reading when no cpufreq sysfs tree exists and skips cores that are offline or missing one of the two files, instead of failing the snapshot — matches how other hardware-optional metrics (CPU temp,vcgencmd) behave. The collection loop logs a warning on error rather than aborting the tick. - Resource leaks: only
os.ReadFileis used; nothing to leak. No new goroutines/tickers. - Parsing robustness: malformed
scaling_cur_freq, missing files, no-driver, and out-of-order core dirs are all covered by fixture-based unit tests (t.TempDir()synthetic sysfs), per the project convention that collectors are testable without real/proc//sys. - Command execution: none — pure sysfs reads.
- API stability:
cpu_frequencyis a new field withomitempty, purely additive to the/api/v1/metricsshape — no breaking change,docs/API.mdupdated and consistent with the actual JSON (omitted when empty, sorted by core index). - Concurrency: a fresh slice is built each tick and assigned to
c.latest.CPUFrequencyunderc.mu, matching the existingDisks/Networkpattern;Snapshot()readers are safe. - Dependencies/language: stdlib only, all English.
One deliberate deviation from issue #17 worth noting: the issue suggested reusing CoreCount(), but the PR discovers cores by globbing cpu[0-9]*/cpufreq directly. That's the better choice here — it naturally handles offline cores and drivers that only expose cpufreq for some cores, and the glob correctly excludes the non-core /sys/devices/system/cpu/cpufreq policy directory.
One non-blocking doc nit inline on the README feature bullet.
Addresses review feedback: the surrounding bullets describe dashboard behavior, but this metric is not yet rendered in the UI.
2 participants
Summary
internal/collector/cpufreq.gocollector that reads per-coreCPU clock speed and active scaling governor from sysfs
(
/sys/devices/system/cpu/cpuN/cpufreq/{scaling_cur_freq,scaling_governor}).cpu_frequencyarray onthe
/api/v1/metricssnapshot (docs/API.mdupdated with the new fieldand its notes).
cpufreqavailability varies across Pi models/kernels, so the collectordegrades to an empty reading (no error) when no driver is present, and
silently skips any core that is offline or exposes only one of the two
files, rather than failing the whole snapshot.
Related Issue
Closes #17
Verification
go build ./...,go vet ./...,go test ./..., andgolangci-lint runall pass locally.
cpufreq.gouse synthetic sysfs fixture directories(
t.TempDir()), covering: normal per-core parsing, sort-by-core-index,no-driver-present (empty result, no error), a core missing one of the two
files (skipped), and malformed
scaling_cur_freqcontent.used for this change has no
cpufreqsysfs tree, so the collector wasexercised purely through the fixture-based unit tests plus the existing
collector_test.gointegration tests (which tolerate an emptycpu_frequencyreading, matching how other hardware-optional metrics arealready tested). Hardware verification on an actual Pi is still
recommended before relying on this in production.
Checklist
go test ./...passes locally)go vet ./...andgolangci-lint runare cleandocs/API.md),configuration (
README.md,packaging/pimonitor.example.yaml), orinstallation/packaging (
packaging/install.sh, systemd units)/api/v1/...response shapes, or a new APIversion was introduced instead