Average host CPU usage across the report interval (#1648)

collectCPUUsage took a blocking 1-second spot sample once per report
interval, which overstates CPU on mostly-idle guests (reported on a
2-vCPU aarch64 KVM guest) and delayed every Collect() by a second.
Track the previous cumulative cpu.Times sample and report busy/total
across the elapsed interval instead, falling back to the spot sample
only for the first collection and counter resets (reboot, migration).
CPUUsagePercent keeps its meaning in the report schema.

Contract-Neutral: hostmetrics sampling behavior change with no subsystem contract surface delta
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

internal/hostmetrics/collector.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host"
@@ -21,6 +22,7 @@ import (
 var (
 	cpuCounts      = gocpu.CountsWithContext
 	cpuPercent     = gocpu.PercentWithContext
+	cpuTimes       = gocpu.TimesWithContext
 	loadAvg        = goload.AvgWithContext
 	virtualMemory  = gomem.VirtualMemoryWithContext
 	diskPartitions = godisk.PartitionsWithContext
@@ -146,7 +148,66 @@ func memoryUsagePercent(usedBytes, totalBytes uint64) float64 {
 	return percent
 }
 
+// cpuUsageTracker computes CPU usage as the busy/total delta of the cumulative
+// CPU time counters between consecutive collections, so the reported value
+// averages over the whole report interval instead of a 1-second spot sample.
+// Short spot samples badly overstate CPU on mostly-idle guests and block the
+// collect path for a full second (issue #1648).
+type cpuUsageTracker struct {
+	mu      sync.Mutex
+	prev    gocpu.TimesStat
+	hasPrev bool
+}
+
+// defaultCPUUsage backs Collect. The state is process-wide by design: every
+// consumer of Collect reports a busy average over the window since the
+// previous collection, which stays correct even if collections interleave.
+var defaultCPUUsage = &cpuUsageTracker{}
+
 func collectCPUUsage(ctx context.Context) (float64, error) {
+	return defaultCPUUsage.collect(ctx)
+}
+
+func (t *cpuUsageTracker) collect(ctx context.Context) (float64, error) {
+	times, err := cpuTimes(ctx, false)
+	if err != nil || len(times) == 0 {
+		return spotCPUUsage(ctx)
+	}
+	cur := times[0]
+
+	t.mu.Lock()
+	prev, hasPrev := t.prev, t.hasPrev
+	t.prev, t.hasPrev = cur, true
+	t.mu.Unlock()
+
+	if !hasPrev {
+		// No baseline yet on the very first collection.
+		return spotCPUUsage(ctx)
+	}
+
+	prevTotal, prevBusy := cpuBusyTotal(prev)
+	curTotal, curBusy := cpuBusyTotal(cur)
+	if curBusy < prevBusy || curTotal <= prevTotal {
+		// Counters went backwards (reboot, VM migration) or did not advance.
+		return spotCPUUsage(ctx)
+	}
+
+	return clampPercent((curBusy - prevBusy) / (curTotal - prevTotal) * 100), nil
+}
+
+// cpuBusyTotal mirrors gopsutil's getAllBusy: guest time is already folded
+// into user/nice by the Linux kernel, so it is subtracted from the total to
+// avoid double counting (it is zero on other platforms), and iowait counts
+// as idle.
+func cpuBusyTotal(t gocpu.TimesStat) (total, busy float64) {
+	total = t.Total() - t.Guest - t.GuestNice
+	busy = total - t.Idle - t.Iowait
+	return total, busy
+}
+
+// spotCPUUsage is the legacy blocking 1-second sample, kept as the fallback
+// when no cross-interval delta is available.
+func spotCPUUsage(ctx context.Context) (float64, error) {
 	percentages, err := cpuPercent(ctx, time.Second, false)
 	if err != nil {
 		return 0, err
@@ -154,15 +215,7 @@ func collectCPUUsage(ctx context.Context) (float64, error) {
 	if len(percentages) == 0 {
 		return 0, nil
 	}
-
-	usage := percentages[0]
-	if usage < 0 {
-		usage = 0
-	}
-	if usage > 100 {
-		usage = 100
-	}
-	return usage, nil
+	return clampPercent(percentages[0]), nil
 }
 
 func collectDisks(ctx context.Context, diskExclude []string) []agentshost.Disk {

internal/hostmetrics/collector_memory_regression_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	gocpu "github.com/shirou/gopsutil/v4/cpu"
 	godisk "github.com/shirou/gopsutil/v4/disk"
 	goload "github.com/shirou/gopsutil/v4/load"
 	gomem "github.com/shirou/gopsutil/v4/mem"
@@ -20,6 +21,7 @@ func TestCollectMemoryStability(t *testing.T) {
 
 	origCPUCounts := cpuCounts
 	origCPUPercent := cpuPercent
+	origCPUTimes := cpuTimes
 	origLoadAvg := loadAvg
 	origVirtualMemory := virtualMemory
 	origDiskPartitions := diskPartitions
@@ -32,6 +34,9 @@ func TestCollectMemoryStability(t *testing.T) {
 	cpuPercent = func(ctx context.Context, interval time.Duration, percpu bool) ([]float64, error) {
 		return []float64{5.0}, nil
 	}
+	cpuTimes = func(ctx context.Context, percpu bool) ([]gocpu.TimesStat, error) {
+		return []gocpu.TimesStat{{CPU: "cpu-total", User: 100, System: 50, Idle: 1000}}, nil
+	}
 	loadAvg = func(ctx context.Context) (*goload.AvgStat, error) {
 		return &goload.AvgStat{Load1: 0.1, Load5: 0.2, Load15: 0.3}, nil
 	}
@@ -78,6 +83,7 @@ func TestCollectMemoryStability(t *testing.T) {
 	t.Cleanup(func() {
 		cpuCounts = origCPUCounts
 		cpuPercent = origCPUPercent
+		cpuTimes = origCPUTimes
 		loadAvg = origLoadAvg
 		virtualMemory = origVirtualMemory
 		diskPartitions = origDiskPartitions

internal/hostmetrics/coverage_additional_test.go

@@ -11,15 +11,24 @@ import (
 	"testing"
 	"time"
 
+	gocpu "github.com/shirou/gopsutil/v4/cpu"
 	godisk "github.com/shirou/gopsutil/v4/disk"
 )
 
 func TestCollectCPUUsageBranches(t *testing.T) {
 	origCPUPercent := cpuPercent
+	origCPUTimes := cpuTimes
 	t.Cleanup(func() {
 		cpuPercent = origCPUPercent
+		cpuTimes = origCPUTimes
 	})
 
+	// Force the spot-sample fallback so the branches below stay deterministic
+	// regardless of the cross-interval delta state (issue #1648).
+	cpuTimes = func(ctx context.Context, percpu bool) ([]gocpu.TimesStat, error) {
+		return nil, errors.New("times unavailable")
+	}
+
 	cpuPercent = func(ctx context.Context, interval time.Duration, percpu bool) ([]float64, error) {
 		return nil, errors.New("boom")
 	}

internal/hostmetrics/issue1648_cpu_interval_delta_test.go

@@ -0,0 +1,105 @@
+package hostmetrics
+
+import (
+	"context"
+	"math"
+	"testing"
+	"time"
+
+	gocpu "github.com/shirou/gopsutil/v4/cpu"
+)
+
+// Issue #1648: a 1-second spot sample once per report interval overstates CPU
+// on mostly-idle guests. CPU usage must be computed as the busy/total delta of
+// the cumulative counters across the whole gap between collections, with the
+// spot sample only as a fallback for the first collection and counter resets.
+func TestIssue1648CPUUsageIntervalDelta(t *testing.T) {
+	origCPUPercent := cpuPercent
+	origCPUTimes := cpuTimes
+	t.Cleanup(func() {
+		cpuPercent = origCPUPercent
+		cpuTimes = origCPUTimes
+	})
+
+	// What a 1-second window would read on the idle 2-vCPU guest from the
+	// report: a short burst makes the instant view claim ~28% CPU.
+	const spotReading = 28.4
+	spotCalls := 0
+	cpuPercent = func(ctx context.Context, interval time.Duration, percpu bool) ([]float64, error) {
+		spotCalls++
+		return []float64{spotReading}, nil
+	}
+
+	// Cumulative counter samples 30s apart on a 2-vCPU guest (60 CPU-seconds
+	// per interval). Between samples[0] and samples[1] only 3 of those 60
+	// seconds are busy, so the interval average is 5%. Guest time advances
+	// inside user and must not count as busy on top of it.
+	samples := []gocpu.TimesStat{
+		{CPU: "cpu-total", User: 120, Nice: 1, System: 60, Idle: 5000, Iowait: 12, Irq: 2, Softirq: 3, Steal: 1, Guest: 10},
+		{CPU: "cpu-total", User: 121.5, Nice: 1.25, System: 61, Idle: 5056.5, Iowait: 12.5, Irq: 2, Softirq: 3, Steal: 1.25, Guest: 10.5},
+		// Counters restart far below the previous sample (reboot or migration).
+		{CPU: "cpu-total", User: 1, System: 0.5, Idle: 20},
+		// One interval after the reset: 1 busy second out of 60.
+		{CPU: "cpu-total", User: 1.5, System: 1, Idle: 78, Iowait: 1},
+	}
+	sampleIdx := 0
+	cpuTimes = func(ctx context.Context, percpu bool) ([]gocpu.TimesStat, error) {
+		s := samples[sampleIdx]
+		sampleIdx++
+		return []gocpu.TimesStat{s}, nil
+	}
+
+	tracker := &cpuUsageTracker{}
+	ctx := context.Background()
+
+	// First collection has no baseline and must fall back to the spot sample.
+	usage, err := tracker.collect(ctx)
+	if err != nil {
+		t.Fatalf("first collect: %v", err)
+	}
+	if usage != spotReading {
+		t.Fatalf("first collect should use spot fallback, got %v want %v", usage, spotReading)
+	}
+	if spotCalls != 1 {
+		t.Fatalf("expected 1 spot sample after first collect, got %d", spotCalls)
+	}
+
+	// Second collection must average across the interval, not re-run the
+	// blocking spot sample that would have reported ~28%.
+	usage, err = tracker.collect(ctx)
+	if err != nil {
+		t.Fatalf("second collect: %v", err)
+	}
+	if math.Abs(usage-5.0) > 1e-9 {
+		t.Fatalf("interval delta usage = %v, want 5.0 (spot window would claim %v)", usage, spotReading)
+	}
+	if spotCalls != 1 {
+		t.Fatalf("interval delta must not invoke the blocking spot sample, got %d calls", spotCalls)
+	}
+
+	// A counter reset makes the delta negative; that collection falls back to
+	// the spot sample instead of reporting garbage.
+	usage, err = tracker.collect(ctx)
+	if err != nil {
+		t.Fatalf("post-reset collect: %v", err)
+	}
+	if usage != spotReading {
+		t.Fatalf("counter reset should use spot fallback, got %v want %v", usage, spotReading)
+	}
+	if spotCalls != 2 {
+		t.Fatalf("expected 2 spot samples after counter reset, got %d", spotCalls)
+	}
+
+	// The reset sample rebaselines the tracker, so the next collection
+	// resumes interval averaging: 1 busy CPU-second out of 60.
+	usage, err = tracker.collect(ctx)
+	if err != nil {
+		t.Fatalf("post-rebaseline collect: %v", err)
+	}
+	if math.Abs(usage-100.0/60.0) > 1e-9 {
+		t.Fatalf("post-rebaseline usage = %v, want %v", usage, 100.0/60.0)
+	}
+	if spotCalls != 2 {
+		t.Fatalf("rebaselined delta must not invoke the spot sample, got %d calls", spotCalls)
+	}
+}