Skip to content

Commit 7eeedcb

Browse files
committed
fix(adapters): read in-process first so a slow macOS helper can't blank metrics
v2.4.1 bounded the helper call but left too little of the 1s budget for the fallback, so a slow powermetrics helper on an M3 Max still timed out the whole privileged adapter (verified: removing the helper was the only way to recover). Flip the strategy: collect in-process first, and consult the helper only when the daemon couldn't read a CPU power rail itself (power.cpu/total/pkg). On macOS the daemon reads SMC/IOReport unprivileged, so it has power without the helper and never calls it — running the helper on Apple Silicon is now harmless. On an unprivileged Linux daemon there's no RAPL in-process, so the helper is still consulted for it (bounded, so a slow one can't hang). GPU power alone doesn't count, so a Linux+NVIDIA box still gets the helper for RAPL.
1 parent 5848dba commit 7eeedcb

3 files changed

Lines changed: 71 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,20 @@ and the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
## [2.4.2] - 2026-07-01
11+
12+
### Fixed
13+
- **macOS: running the helper no longer blanks power/GPU — the real fix.** v2.4.1
14+
only bounded the helper call, which still left too little of the 1 s budget for
15+
the fallback, so a slow `powermetrics` helper on an M-series Pro/Max kept timing
16+
out the whole privileged adapter. The adapter now reads **in-process first** and
17+
consults the helper only when the daemon can't read a CPU power rail itself
18+
(`power.cpu`/`power.total`/`power.pkg`) — i.e. an unprivileged Linux daemon that
19+
needs the root helper for RAPL. On macOS the daemon already has power from
20+
SMC/IOReport, so the helper is never on the critical path: running it on Apple
21+
Silicon is finally harmless, not destructive. (GPU power alone doesn't count as
22+
"has power" — a Linux box with an NVIDIA GPU still needs the helper for RAPL.)
23+
1024
## [2.4.1] - 2026-07-01
1125

1226
### Fixed

app/internal/adapters/helper.go

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,38 +43,61 @@ func (Helper) Describe() domain.AdapterInfo {
4343
}
4444

4545
func (h Helper) Collect(ctx context.Context) ([]domain.Metric, error) {
46+
direct := h.Direct
47+
if direct == nil {
48+
direct = helper.PrivilegedMetrics
49+
}
50+
51+
// In-process first. Where the daemon can already read a privileged CPU power
52+
// rail itself — macOS (SMC/IOReport are unprivileged) or a root daemon reading
53+
// RAPL — the helper can only duplicate it, so we return the in-process read and
54+
// never call the helper. That's what makes running the helper on Apple Silicon
55+
// harmless: the daemon has power without it, so a slow helper (macOS
56+
// powermetrics can take ~1s) is never on the critical path.
57+
inproc := direct(ctx)
58+
if hasOKPower(inproc) {
59+
return inproc, nil
60+
}
61+
62+
// Otherwise the daemon is unprivileged and can't read the CPU power rail
63+
// (typically a Linux daemon that needs the root helper for RAPL). Consult the
64+
// helper, bounded so a slow or hung one can't stall the collection cycle. A
65+
// reachable-but-empty helper must never shadow the in-process read, so we only
66+
// take the helper's result when it has an ok metric; else we fall back to
67+
// whatever in-process produced.
4668
var client metricCollector = h.Client
4769
if client == nil {
4870
client = helper.Client{}
4971
}
50-
// Use the helper only when it actually produced an ok reading. A reachable
51-
// helper that returns nothing ok (e.g. a non-cgo helper, or one that can't
52-
// read IOReport) must NOT shadow a working in-process source — otherwise
53-
// running the helper on a Mac, where IOReport is unprivileged, blanks out the
54-
// power/gpu the daemon was reading itself.
55-
//
56-
// The helper call is bounded to a slice of the deadline so a *slow* helper
57-
// (e.g. macOS powermetrics, which can take ~1s) can't consume the whole
58-
// adapter budget and starve the in-process fallback below — the helper stays
59-
// additive, never subtractive, even under latency.
6072
hctx, cancel := helperCallContext(ctx)
6173
defer cancel()
6274
if ms, err := client.Collect(hctx); err == nil && anyOK(ms) {
6375
return ms, nil
6476
}
65-
direct := h.Direct
66-
if direct == nil {
67-
direct = helper.PrivilegedMetrics
68-
}
69-
if ms := direct(ctx); anyOK(ms) {
70-
return ms, nil
77+
if anyOK(inproc) {
78+
return inproc, nil
7179
}
7280
return []domain.Metric{
7381
{Name: "power.cpu", Status: domain.StatusInsufficientPermission, Detail: "needs helper"},
7482
{Name: "gpu.util", Status: domain.StatusInsufficientPermission, Detail: "needs helper"},
7583
}, nil
7684
}
7785

86+
// hasOKPower reports whether the metrics already include a privileged CPU power
87+
// rail the daemon read itself — the signal that the helper would only duplicate.
88+
func hasOKPower(ms []domain.Metric) bool {
89+
for _, m := range ms {
90+
if m.Status != domain.StatusOK {
91+
continue
92+
}
93+
switch m.Name {
94+
case "power.total", "power.cpu", "power.pkg":
95+
return true
96+
}
97+
}
98+
return false
99+
}
100+
78101
// helperCallContext caps the helper socket call so it leaves budget for the
79102
// in-process fallback within the adapter deadline. Without a deadline it applies
80103
// a sane default; when the deadline is already near, the helper still gets a

app/internal/adapters/helper_test.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -144,20 +144,30 @@ func TestHelperEmptyReplyFallsBackToDirect(t *testing.T) {
144144
}
145145
}
146146

147-
// A reachable helper WITH ok metrics is authoritative — Direct must not run.
148-
func TestHelperOKReplyIsUsedWithoutDirect(t *testing.T) {
149-
directRan := false
147+
// When the daemon can't read a privileged CPU power rail in-process (an
148+
// unprivileged Linux daemon: RAPL needs root — note GPU power alone doesn't
149+
// count, since RAPL still requires the helper), the helper is consulted and its
150+
// ok reading is used.
151+
func TestHelperUsedWhenInProcessLacksCPUPower(t *testing.T) {
150152
a := Helper{
151153
Client: fakeMetricClient{ms: []domain.Metric{
152154
{Name: "power.cpu", Unit: "W", Status: domain.StatusOK, Gauge: 15},
155+
{Name: "gpu.util", Unit: "%", Status: domain.StatusOK, Gauge: 40},
153156
}},
154-
Direct: func(context.Context) []domain.Metric { directRan = true; return nil },
157+
Direct: func(context.Context) []domain.Metric {
158+
// In-process sees the GPU (nvidia) but not the root-only CPU rail.
159+
return []domain.Metric{
160+
{Name: "gpu.util", Unit: "%", Status: domain.StatusOK, Gauge: 40},
161+
{Name: "power.gpu", Unit: "W", Status: domain.StatusOK, Gauge: 60},
162+
}
163+
},
155164
}
156165
ms, _ := a.Collect(context.Background())
157-
if directRan {
158-
t.Fatal("Direct must not run when the helper already has ok metrics")
166+
got := make(map[string]domain.Metric, len(ms))
167+
for _, m := range ms {
168+
got[m.Name] = m
159169
}
160-
if len(ms) != 1 || ms[0].Name != "power.cpu" || ms[0].Status != domain.StatusOK {
161-
t.Fatalf("want helper power.cpu ok, got %+v", ms)
170+
if got["power.cpu"].Status != domain.StatusOK || got["power.cpu"].Gauge != 15 {
171+
t.Fatalf("want helper power.cpu 15 (in-process had no CPU rail), got %+v", got["power.cpu"])
162172
}
163173
}

0 commit comments

Comments
 (0)