fix(cpu): enable per-core CPU stats on macOS#7
Merged
Conversation
macOS per-core CPU readings require cgo (gopsutil reads them via host_processor_info). The release workflow cross-compiled every target on ubuntu-latest, which forces CGO_ENABLED=0, so released macOS binaries shipped the cgo-less gopsutil stub that returns an error and an empty slice. collectStats silently discarded that error, leaving CPUCores empty and the per-core view blank. - ci(release): split builds into a job matrix so darwin binaries build natively on macos-latest with CGO_ENABLED=1, while linux/windows keep cross-compiling on ubuntu; collect artifacts in a dedicated release job - build(scripts): parameterize build.sh by target (all|linux-windows| darwin) and set CGO_ENABLED per target - fix(cpu): stop discarding the cpu.Percent error and fall back to the aggregate reading so overall CPU usage still shows when per-core data is unavailable - docs(readme): note the cgo requirement for per-core stats on macOS Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 5 minutes and 29 seconds. Learn how PR review limits work. To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: PinePeakDigital/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughBuild script converted to target-driven builds with platform-specific CGO; release workflow refactored into version/build/release jobs with a matrix and artifact uploads; new CI workflow added; collectStats() now falls back to aggregate CPU usage when per-core sampling fails; README documents macOS cgo dependency. ChangesBuild System and Release Pipeline Enhancement
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes missing per-core CPU bars (and 0% aggregate CPU) on macOS by ensuring darwin release binaries are built with CGO enabled (required by gopsutil for per-core CPU stats), and by making CPU collection degrade gracefully when per-core data isn’t available.
Changes:
- Updates the release workflow to build Linux/Windows on
ubuntu-latestand macOS onmacos-latest, then publish artifacts from a dedicated release job. - Extends
scripts/build.shwith target selection and per-targetCGO_ENABLEDsettings. - Improves
collectStats()to stop discarding per-core CPU errors and fall back to aggregate CPU usage. - Documents the macOS/cgo requirement in the README.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
scripts/build.sh |
Adds build targets and toggles CGO per target (darwin builds with CGO enabled). |
README.md |
Documents that per-core CPU stats on macOS require CGO. |
main.go |
Handles cpu.Percent errors and falls back to aggregate CPU usage when per-core stats are unavailable. |
.github/workflows/release.yml |
Splits release into version/build/release jobs and builds darwin natively on macOS. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 100-105: The workflow uses floating action tags; replace each
`uses:` entry with the corresponding immutable 40-hex commit SHA for those
actions (e.g., the occurrences of actions/checkout@v4,
ietf-tools/semver-action@v1, actions/setup-go@v6, actions/upload-artifact@v4,
actions/download-artifact@v4, softprops/action-gh-release@v1) by locating each
`uses:` line in .github/workflows/release.yml and updating the value to the
action's full commit SHA (owner/repo@<40-char-sha>); verify the SHAs against the
upstream repositories/releases, update all instances found, and run/validate the
workflow to ensure no breaking changes.
- Line 139: The workflow uses an outdated softprops action version; update the
GitHub Actions step that references "uses: softprops/action-gh-release@v1" to
the currently supported major release (e.g., change to
"softprops/action-gh-release@v3"); ensure the release job step named "Create
Release" (the step that contains the softprops/action-gh-release usage) is
updated accordingly and run on a compatible runner — if you must remain on Node
20 instead use "softprops/action-gh-release@v2.6.2".
In `@README.md`:
- Around line 43-46: Update the "Platform requirements" wording to reconcile
with the new "macOS note": change the Linux-only phrasing to indicate primary
support for Linux while noting macOS is supported with the caveat that per-core
CPU stats require cgo and will be empty if built with CGO_ENABLED=0; reference
the existing "macOS note" text and ensure the "Platform requirements" section
(or heading named Platform requirements) mentions macOS behavior and the CGO
caveat so the two sections are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PinePeakDigital/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1436c9a9-46d7-4e69-936d-5fbded55b676
📒 Files selected for processing (4)
.github/workflows/release.ymlREADME.mdmain.goscripts/build.sh
Run gofmt, vet, build, and tests on ubuntu-latest and macos-latest for every pull request and push to main. macOS coverage exercises the cgo per-core CPU path; Linux covers the pure-Go path. Previously the repo had no PR-time checks at all. Also gofmt main_test.go to satisfy the new formatting gate (trailing whitespace on blank lines only; no behavior change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 26-27: Replace floating tag references for GitHub Actions with
pinned commit SHAs: locate the uses entries for actions/checkout@v4 and
actions/setup-go@v6 in the CI workflow and replace the tag suffixes with the
corresponding full commit SHAs (e.g., actions/checkout@<commit-sha> and
actions/setup-go@<commit-sha>) so the workflow references immutable commits;
fetch the current commit SHA for each tag from the action's GitHub repo
releases/tags and update the workflow file accordingly, and optionally add a
short comment noting the pinned SHA and a process for updating it
(Dependabot/Renovate).
- Around line 26-27: The checkout step using actions/checkout@v4 currently
leaves the GitHub token persisted in .git/config; update the Checkout code block
to add the persist-credentials: false option so the actions/checkout@v4 step
does not persist credentials (i.e., set persist-credentials: false under the
checkout step using actions/checkout@v4).
- Around line 17-24: The CI job "test" currently has no timeout; add a
`timeout-minutes` key to the job definition for the test job (under the
top-level job named "test") to prevent indefinite hangs — e.g., set
`timeout-minutes` to a sensible value like 30 or 60 minutes; ensure the
`timeout-minutes` entry is at the same indentation level as `runs-on`/`strategy`
within the "test" job so GitHub Actions honors it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PinePeakDigital/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bf5b8e14-8e61-4140-bf40-070d58d5e653
📒 Files selected for processing (2)
.github/workflows/ci.ymlmain_test.go
The default `all` target ran build_darwin unconditionally, which fails on Linux because cgo cannot be cross-compiled for darwin. Guard it on a Darwin host check so a bare `./scripts/build.sh` succeeds on Linux CI/dev machines; the explicit `darwin` target still runs unconditionally so misuse fails loudly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin every `uses:` across release.yml, ci.yml, and cleanup-pr-releases.yml to immutable 40-char commit SHAs (with version comments) to harden against floating-tag supply-chain risks, as flagged by CodeRabbit/zizmor. Add .github/dependabot.yml (github-actions ecosystem, weekly) so the pinned SHAs and their version comments are kept up to date automatically. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
softprops/action-gh-release@v1 runs on the obsolete Node 16 runtime, which actionlint flags as too old for current GitHub-hosted runners and can prevent the release step from running. Upgrade to v3.0.0 (Node 24), pinned to its commit SHA. The inputs used here are unchanged across major versions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Requirements section listed Linux only, conflicting with the new macOS note and the macOS/Windows binaries the project ships. State Linux/macOS/Windows support and link to the macOS cgo caveat. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Safety net so a stuck step can't hang the job indefinitely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
actions/checkout persists GITHUB_TOKEN in .git/config by default; the test job needs no git credentials after checkout, so set persist-credentials: false to reduce token-exfiltration risk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On macOS, sysmon shows no per-core CPU bars even on machines with many cores — only the aggregate "CPU Usage" bar (which itself reads 0% on affected builds).
Root cause: gopsutil reads per-core CPU on macOS via
host_processor_info, which requires cgo. The release workflow cross-compiled every target onubuntu-latest, and Go cross-compilation forcesCGO_ENABLED=0. So released macOS binaries shipped gopsutil's cgo-less stub, wherecpu.Percent(interval, true)returnsErrNotImplementedErrorand an empty slice.collectStatsdiscarded that error (perCoreCPU, _ := ...), leavingCPUCoresempty and the per-core section blank. Since the overall average was also derived from that empty slice, aggregate CPU read 0% too.Linux (
/proc/stat) and Windows (syscalls) use pure-Go paths, so only macOS was affected.Fix
ci(release)— split the release intoversion→build(matrix) →releasejobs. macOS binaries now build natively onmacos-latestwithCGO_ENABLED=1(a single arm64 runner produces both darwin arches via the universal SDK); linux/windows keep cross-compiling onubuntu-latest. Binaries are collected as artifacts and attached in a dedicated release job.build(scripts)—build.shnow takes a target (all|linux-windows|darwin) and setsCGO_ENABLEDper target.fix(cpu)— stop discarding thecpu.Percenterror; fall back to the aggregate reading so overall CPU usage still displays even if per-core data is unavailable (graceful degradation rather than a silent 0%).docs(readme)— note the cgo requirement for per-core stats on macOS.Verification
Go isn't installed on my machine, so I couldn't build locally —
build.shpassesbash -nand CI will compile all targets. The macOS build job in this PR's run is the real test: it must producesysmon-darwin-{amd64,arm64}with cgo enabled. After merge, a released darwin binary should show populated per-core bars.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Tests
Chores