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