From a68e0050f88874dcb72ecda2fef0e4ad4a692fa4 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Tue, 10 Feb 2026 20:49:29 +0000 Subject: [PATCH] fix(docker): use manual CPU delta tracking instead of stale PreCPUStats (#1229) Docker's one-shot stats API (stream=false) returns PreCPUStats from the daemon's internal cache, which many Docker versions don't update between non-streaming reads. This causes every call to return the same stale PreCPUStats from container start, producing a constant lifetime-average CPU% (e.g. 3.4%) instead of current usage. Switch to always using manual delta tracking, which stores the previous sample from our own reads and computes accurate deltas between collection cycles. The first cycle returns 0 while establishing a baseline; all subsequent cycles produce correct current CPU percentages. --- internal/dockeragent/agent.go | 43 +++++++++++----------- internal/dockeragent/agent_cpu_test.go | 49 +++++++++++++++++--------- internal/dockeragent/collect.go | 23 +++--------- 3 files changed, 58 insertions(+), 57 deletions(-) diff --git a/internal/dockeragent/agent.go b/internal/dockeragent/agent.go index c448ddcc6..8fa3435eb 100644 --- a/internal/dockeragent/agent.go +++ b/internal/dockeragent/agent.go @@ -76,28 +76,27 @@ const ( // Agent collects Docker metrics and posts them to Pulse. type Agent struct { - cfg Config - docker dockerClient - daemonHost string - daemonID string // Cached at init; Podman can return unstable IDs across calls - runtime RuntimeKind - runtimeVer string - agentVersion string - supportsSwarm bool - httpClients map[bool]*http.Client - logger zerolog.Logger - machineID string - hostName string - cpuCount int - targets []TargetConfig - allowedStates map[string]struct{} - stateFilters []string - hostID string - prevContainerCPU map[string]cpuSample - cpuMu sync.Mutex // protects prevContainerCPU and preCPUStatsFailures - preCPUStatsFailures int - reportBuffer *buffer.Queue[agentsdocker.Report] - registryChecker *RegistryChecker // For checking container image updates + cfg Config + docker dockerClient + daemonHost string + daemonID string // Cached at init; Podman can return unstable IDs across calls + runtime RuntimeKind + runtimeVer string + agentVersion string + supportsSwarm bool + httpClients map[bool]*http.Client + logger zerolog.Logger + machineID string + hostName string + cpuCount int + targets []TargetConfig + allowedStates map[string]struct{} + stateFilters []string + hostID string + prevContainerCPU map[string]cpuSample + cpuMu sync.Mutex // protects prevContainerCPU + reportBuffer *buffer.Queue[agentsdocker.Report] + registryChecker *RegistryChecker // For checking container image updates } // ErrStopRequested indicates the agent should terminate gracefully after acknowledging a stop command. diff --git a/internal/dockeragent/agent_cpu_test.go b/internal/dockeragent/agent_cpu_test.go index 2c88b3eb6..9224e8b35 100644 --- a/internal/dockeragent/agent_cpu_test.go +++ b/internal/dockeragent/agent_cpu_test.go @@ -11,32 +11,42 @@ import ( func TestCalculateContainerCPUPercent(t *testing.T) { logger := zerolog.Nop() - t.Run("uses precpu stats", func(t *testing.T) { + t.Run("manual delta across two calls", func(t *testing.T) { agent := &Agent{ logger: logger, prevContainerCPU: make(map[string]cpuSample), cpuCount: 2, } - stats := containertypes.StatsResponse{ + // First call: stores sample, returns 0 + stats1 := containertypes.StatsResponse{ + Read: time.Now().Add(-time.Second), + CPUStats: containertypes.CPUStats{ + CPUUsage: containertypes.CPUUsage{TotalUsage: 100}, + SystemUsage: 1000, + OnlineCPUs: 2, + }, + } + got := agent.calculateContainerCPUPercent("container-123456", stats1) + if got != 0 { + t.Fatalf("first call: expected 0, got %f", got) + } + if _, ok := agent.prevContainerCPU["container-123456"]; !ok { + t.Fatal("expected sample to be stored after first call") + } + + // Second call: computes delta from stored sample + stats2 := containertypes.StatsResponse{ Read: time.Now(), CPUStats: containertypes.CPUStats{ CPUUsage: containertypes.CPUUsage{TotalUsage: 200}, SystemUsage: 2000, OnlineCPUs: 2, }, - PreCPUStats: containertypes.CPUStats{ - CPUUsage: containertypes.CPUUsage{TotalUsage: 100}, - SystemUsage: 1000, - }, } - - got := agent.calculateContainerCPUPercent("container-123456", stats) + got = agent.calculateContainerCPUPercent("container-123456", stats2) if got <= 0 { - t.Fatalf("expected percent > 0, got %f", got) - } - if _, ok := agent.prevContainerCPU["container-123456"]; !ok { - t.Fatal("expected current sample to be stored") + t.Fatalf("second call: expected percent > 0, got %f", got) } }) @@ -286,17 +296,21 @@ func TestCalculateContainerCPUPercent(t *testing.T) { } }) - t.Run("warn after repeated failures", func(t *testing.T) { + t.Run("ignores stale precpu stats", func(t *testing.T) { agent := &Agent{ logger: logger, prevContainerCPU: make(map[string]cpuSample), + cpuCount: 2, } + // Even with valid-looking PreCPUStats, we use manual tracking. + // First call stores sample and returns 0. stats := containertypes.StatsResponse{ Read: time.Now(), CPUStats: containertypes.CPUStats{ - CPUUsage: containertypes.CPUUsage{TotalUsage: 100}, - SystemUsage: 1000, + CPUUsage: containertypes.CPUUsage{TotalUsage: 200}, + SystemUsage: 2000, + OnlineCPUs: 2, }, PreCPUStats: containertypes.CPUStats{ CPUUsage: containertypes.CPUUsage{TotalUsage: 100}, @@ -304,8 +318,9 @@ func TestCalculateContainerCPUPercent(t *testing.T) { }, } - for i := 0; i < 10; i++ { - _ = agent.calculateContainerCPUPercent("container-123456", stats) + got := agent.calculateContainerCPUPercent("container-123456", stats) + if got != 0 { + t.Fatalf("expected 0 on first call (manual tracking), got %f", got) } }) } diff --git a/internal/dockeragent/collect.go b/internal/dockeragent/collect.go index a2579b953..664e4a4ae 100644 --- a/internal/dockeragent/collect.go +++ b/internal/dockeragent/collect.go @@ -605,24 +605,11 @@ func (a *Agent) calculateContainerCPUPercent(id string, stats containertypes.Sta read: stats.Read, } - // Try to use PreCPUStats if available - percent := calculateCPUPercent(stats, a.cpuCount) - if percent > 0 { - a.prevContainerCPU[id] = current - a.logger.Debug(). - Str("container_id", id[:12]). - Float64("cpu_percent", percent). - Msg("CPU calculated from PreCPUStats") - return percent - } - - // PreCPUStats not available or invalid, use manual tracking - a.preCPUStatsFailures++ - if a.preCPUStatsFailures == 10 { - a.logger.Warn(). - Str("runtime", string(a.runtime)). - Msg("PreCPUStats consistently unavailable from Docker API - using manual CPU tracking (this is normal for one-shot stats)") - } + // Always use manual delta tracking. Docker's PreCPUStats is unreliable for + // one-shot stats because many Docker versions don't update their internal + // cache between non-streaming reads, causing PreCPUStats to remain stale + // (from container start) and producing a constant lifetime-average CPU% + // instead of a current value. prev, ok := a.prevContainerCPU[id] if !ok { // First time seeing this container - store current sample and return 0