From 36f8ee8d4828bf9b354bff85505787efd8ff4dcc Mon Sep 17 00:00:00 2001 From: Matthew_Cash Date: Sun, 19 Jul 2026 11:15:06 -0700 Subject: [PATCH 1/2] Fix reported CPU usage on podman --- environment/docker/stats.go | 87 +++++++++++++++++++----- environment/docker/stats_test.go | 110 +++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 environment/docker/stats_test.go diff --git a/environment/docker/stats.go b/environment/docker/stats.go index 37a6c648..ada11124 100644 --- a/environment/docker/stats.go +++ b/environment/docker/stats.go @@ -4,15 +4,24 @@ import ( "context" "io" "math" + "strings" + "sync" "time" "emperror.dev/errors" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/goccy/go-json" "github.com/pelican-dev/wings/environment" ) +var runtimeDetection struct { + sync.Mutex + detected bool + isPodman bool +} + // Uptime returns the current uptime of the container in milliseconds. If the // container is not currently running this will return 0. func (e *Environment) Uptime(ctx context.Context) (int64, error) { @@ -51,6 +60,12 @@ func (e *Environment) pollResources(ctx context.Context) error { e.log().WithField("error", err).Warn("failed to calculate container uptime") } + isPodman, err := e.isPodman(ctx) + if err != nil { + e.log().WithField("error", err).Warn("failed to detect container runtime, using wall time for CPU calculation") + isPodman = true + } + dec := json.NewDecoder(stats.Body) for { select { @@ -81,7 +96,7 @@ func (e *Environment) pollResources(ctx context.Context) error { Uptime: uptime, Memory: calculateDockerMemory(v.MemoryStats), MemoryLimit: v.MemoryStats.Limit, - CpuAbsolute: calculateDockerAbsoluteCpu(v.PreCPUStats, v.CPUStats), + CpuAbsolute: calculateDockerAbsoluteCpu(v, isPodman), Network: environment.NetworkStats{}, } @@ -118,28 +133,66 @@ func calculateDockerMemory(stats container.MemoryStats) uint64 { // Calculates the absolute CPU usage used by the server process on the system, not constrained // by the defined CPU limits on the container. // -// @see https://github.com/docker/cli/blob/aa097cf1aa19099da70930460250797c8920b709/cli/command/container/stats_helpers.go#L166 -func calculateDockerAbsoluteCpu(pStats container.CPUStats, stats container.CPUStats) float64 { - // Calculate the change in CPU usage between the current and previous reading. - cpuDelta := float64(stats.CPUUsage.TotalUsage) - float64(pStats.CPUUsage.TotalUsage) +// Podman's Docker-compatible API does not provide Docker-equivalent values for SystemUsage, so +// its CPU usage must instead be compared to the elapsed time between samples. +func calculateDockerAbsoluteCpu(stats container.StatsResponse, useWallTime bool) float64 { + current := stats.CPUStats.CPUUsage.TotalUsage + previous := stats.PreCPUStats.CPUUsage.TotalUsage + if current <= previous { + return 0 + } - // Calculate the change for the entire system's CPU usage between current and previous reading. - systemDelta := float64(stats.SystemUsage) - float64(pStats.SystemUsage) + cpuDelta := float64(current - previous) + if useWallTime { + if stats.PreRead.IsZero() || !stats.Read.After(stats.PreRead) { + return 0 + } - // Calculate the total number of CPU cores being used. - cpus := float64(stats.OnlineCPUs) - if cpus == 0.0 { - cpus = float64(len(stats.CPUUsage.PercpuUsage)) + timeDelta := float64(stats.Read.Sub(stats.PreRead).Nanoseconds()) + return math.Round((cpuDelta/timeDelta)*100*1000) / 1000 } - percent := 0.0 - if systemDelta > 0.0 && cpuDelta > 0.0 { - percent = (cpuDelta / systemDelta) * 100.0 + currentSystem := stats.CPUStats.SystemUsage + previousSystem := stats.PreCPUStats.SystemUsage + if currentSystem <= previousSystem { + return 0 + } - if cpus > 0 { - percent *= cpus - } + cpus := float64(stats.CPUStats.OnlineCPUs) + if cpus == 0 { + cpus = float64(len(stats.CPUStats.CPUUsage.PercpuUsage)) + } + + percent := (cpuDelta / float64(currentSystem-previousSystem)) * 100 + if cpus > 0 { + percent *= cpus } return math.Round(percent*1000) / 1000 } + +func (e *Environment) isPodman(ctx context.Context) (bool, error) { + runtimeDetection.Lock() + defer runtimeDetection.Unlock() + if runtimeDetection.detected { + return runtimeDetection.isPodman, nil + } + + version, err := e.client.ServerVersion(ctx) + if err != nil { + return false, err + } + + runtimeDetection.detected = true + runtimeDetection.isPodman = isPodmanVersion(version) + return runtimeDetection.isPodman, nil +} + +func isPodmanVersion(version types.Version) bool { + for _, component := range version.Components { + if strings.EqualFold(component.Name, "Podman Engine") { + return true + } + } + return false +} diff --git a/environment/docker/stats_test.go b/environment/docker/stats_test.go new file mode 100644 index 00000000..a20b855d --- /dev/null +++ b/environment/docker/stats_test.go @@ -0,0 +1,110 @@ +package docker + +import ( + "testing" + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/stretchr/testify/require" +) + +func TestCalculateDockerAbsoluteCpu(t *testing.T) { + base := time.Date(2026, time.July, 13, 12, 0, 0, 0, time.UTC) + + tests := []struct { + name string + stats container.StatsResponse + useWallTime bool + expected float64 + }{ + { + name: "Docker system usage", + stats: func() container.StatsResponse { + stats := cpuStatsResponse(base, 2*time.Second, 5_000_000_000, 6_000_000_000) + stats.PreCPUStats.SystemUsage = 10_000_000_000 + stats.CPUStats.SystemUsage = 74_000_000_000 + stats.CPUStats.OnlineCPUs = 64 + return stats + }(), + expected: 100, + }, + { + name: "multiple cores with wall time", + stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 7_500_000_000), + useWallTime: true, + expected: 250, + }, + { + name: "rounds wall time to three decimal places", + stats: cpuStatsResponse(base, 3*time.Second, 5_000_000_000, 6_000_000_000), + useWallTime: true, + expected: 33.333, + }, + { + name: "Podman system usage", + stats: func() container.StatsResponse { + stats := cpuStatsResponse(base, time.Second, 5_000_000_000, 6_000_000_000) + stats.PreCPUStats.SystemUsage = 1_000_000_000 + stats.CPUStats.SystemUsage = 2_000_000_000 + stats.CPUStats.OnlineCPUs = 64 + return stats + }(), + useWallTime: true, + expected: 100, + }, + { + name: "no CPU usage", + stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 5_000_000_000), + useWallTime: true, + expected: 0, + }, + { + name: "CPU counter reset", + stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 1_000_000_000), + useWallTime: true, + expected: 0, + }, + { + name: "missing previous timestamp", + stats: cpuStatsResponse(time.Time{}, time.Second, 5_000_000_000, 6_000_000_000), + useWallTime: true, + expected: 0, + }, + { + name: "non-increasing timestamp", + stats: cpuStatsResponse(base, 0, 5_000_000_000, 6_000_000_000), + useWallTime: true, + expected: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expected, calculateDockerAbsoluteCpu(tt.stats, tt.useWallTime)) + }) + } +} + +func TestIsPodmanVersion(t *testing.T) { + version := types.Version{ + Components: []types.ComponentVersion{{Name: "Podman Engine"}}, + } + require.True(t, isPodmanVersion(version)) + + version.Components[0].Name = "Engine" + require.False(t, isPodmanVersion(version)) +} + +func cpuStatsResponse(preRead time.Time, elapsed time.Duration, previous, current uint64) container.StatsResponse { + return container.StatsResponse{ + Read: preRead.Add(elapsed), + PreRead: preRead, + CPUStats: container.CPUStats{ + CPUUsage: container.CPUUsage{TotalUsage: current}, + }, + PreCPUStats: container.CPUStats{ + CPUUsage: container.CPUUsage{TotalUsage: previous}, + }, + } +} From 00d673203744109530c9b123da898999ee0aded2 Mon Sep 17 00:00:00 2001 From: Matthew_Cash Date: Tue, 25 Aug 2026 19:38:13 -0700 Subject: [PATCH 2/2] Use wall clock time for container cpu usage --- environment/docker/stats.go | 78 +++----------------------------- environment/docker/stats_test.go | 78 ++++++++++---------------------- 2 files changed, 31 insertions(+), 125 deletions(-) diff --git a/environment/docker/stats.go b/environment/docker/stats.go index 85332f01..48fa25e5 100644 --- a/environment/docker/stats.go +++ b/environment/docker/stats.go @@ -5,23 +5,15 @@ import ( "io" "math" "strings" - "sync" "time" "emperror.dev/errors" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/goccy/go-json" "github.com/pelican/wings/environment" ) -var runtimeDetection struct { - sync.Mutex - detected bool - isPodman bool -} - // Uptime returns the current uptime of the container in milliseconds. If the // container is not currently running this will return 0. func (e *Environment) Uptime(ctx context.Context) (int64, error) { @@ -60,12 +52,6 @@ func (e *Environment) pollResources(ctx context.Context) error { e.log().WithField("error", err).Warn("failed to calculate container uptime") } - isPodman, err := e.isPodman(ctx) - if err != nil { - e.log().WithField("error", err).Warn("failed to detect container runtime, using wall time for CPU calculation") - isPodman = true - } - dec := json.NewDecoder(stats.Body) for { select { @@ -96,7 +82,7 @@ func (e *Environment) pollResources(ctx context.Context) error { Uptime: uptime, Memory: calculateDockerMemory(v.MemoryStats), MemoryLimit: v.MemoryStats.Limit, - CpuAbsolute: calculateDockerAbsoluteCpu(v, isPodman), + CpuAbsolute: calculateDockerAbsoluteCpu(v), Network: environment.NetworkStats{}, } @@ -144,66 +130,16 @@ func calculateDockerMemory(stats container.MemoryStats) uint64 { // Calculates the absolute CPU usage used by the server process on the system, not constrained // by the defined CPU limits on the container. // -// Podman's Docker-compatible API does not provide Docker-equivalent values for SystemUsage, so -// its CPU usage must instead be compared to the elapsed time between samples. -func calculateDockerAbsoluteCpu(stats container.StatsResponse, useWallTime bool) float64 { +// CPU time is compared to the elapsed time between samples because Podman's Docker-compatible API +// does not provide Docker-equivalent SystemUsage values. +func calculateDockerAbsoluteCpu(stats container.StatsResponse) float64 { current := stats.CPUStats.CPUUsage.TotalUsage previous := stats.PreCPUStats.CPUUsage.TotalUsage - if current <= previous { + if current <= previous || stats.PreRead.IsZero() || !stats.Read.After(stats.PreRead) { return 0 } cpuDelta := float64(current - previous) - if useWallTime { - if stats.PreRead.IsZero() || !stats.Read.After(stats.PreRead) { - return 0 - } - - timeDelta := float64(stats.Read.Sub(stats.PreRead).Nanoseconds()) - return math.Round((cpuDelta/timeDelta)*100*1000) / 1000 - } - - currentSystem := stats.CPUStats.SystemUsage - previousSystem := stats.PreCPUStats.SystemUsage - if currentSystem <= previousSystem { - return 0 - } - - cpus := float64(stats.CPUStats.OnlineCPUs) - if cpus == 0 { - cpus = float64(len(stats.CPUStats.CPUUsage.PercpuUsage)) - } - - percent := (cpuDelta / float64(currentSystem-previousSystem)) * 100 - if cpus > 0 { - percent *= cpus - } - - return math.Round(percent*1000) / 1000 -} - -func (e *Environment) isPodman(ctx context.Context) (bool, error) { - runtimeDetection.Lock() - defer runtimeDetection.Unlock() - if runtimeDetection.detected { - return runtimeDetection.isPodman, nil - } - - version, err := e.client.ServerVersion(ctx) - if err != nil { - return false, err - } - - runtimeDetection.detected = true - runtimeDetection.isPodman = isPodmanVersion(version) - return runtimeDetection.isPodman, nil -} - -func isPodmanVersion(version types.Version) bool { - for _, component := range version.Components { - if strings.EqualFold(component.Name, "Podman Engine") { - return true - } - } - return false + timeDelta := float64(stats.Read.Sub(stats.PreRead).Nanoseconds()) + return math.Round((cpuDelta/timeDelta)*100*1000) / 1000 } diff --git a/environment/docker/stats_test.go b/environment/docker/stats_test.go index a20b855d..6da32857 100644 --- a/environment/docker/stats_test.go +++ b/environment/docker/stats_test.go @@ -4,7 +4,6 @@ import ( "testing" "time" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/stretchr/testify/require" ) @@ -13,13 +12,12 @@ func TestCalculateDockerAbsoluteCpu(t *testing.T) { base := time.Date(2026, time.July, 13, 12, 0, 0, 0, time.UTC) tests := []struct { - name string - stats container.StatsResponse - useWallTime bool - expected float64 + name string + stats container.StatsResponse + expected float64 }{ { - name: "Docker system usage", + name: "uses elapsed wall time", stats: func() container.StatsResponse { stats := cpuStatsResponse(base, 2*time.Second, 5_000_000_000, 6_000_000_000) stats.PreCPUStats.SystemUsage = 10_000_000_000 @@ -27,75 +25,47 @@ func TestCalculateDockerAbsoluteCpu(t *testing.T) { stats.CPUStats.OnlineCPUs = 64 return stats }(), - expected: 100, + expected: 50, }, { - name: "multiple cores with wall time", - stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 7_500_000_000), - useWallTime: true, - expected: 250, + name: "multiple cores", + stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 7_500_000_000), + expected: 250, }, { - name: "rounds wall time to three decimal places", - stats: cpuStatsResponse(base, 3*time.Second, 5_000_000_000, 6_000_000_000), - useWallTime: true, - expected: 33.333, + name: "rounds to three decimal places", + stats: cpuStatsResponse(base, 3*time.Second, 5_000_000_000, 6_000_000_000), + expected: 33.333, }, { - name: "Podman system usage", - stats: func() container.StatsResponse { - stats := cpuStatsResponse(base, time.Second, 5_000_000_000, 6_000_000_000) - stats.PreCPUStats.SystemUsage = 1_000_000_000 - stats.CPUStats.SystemUsage = 2_000_000_000 - stats.CPUStats.OnlineCPUs = 64 - return stats - }(), - useWallTime: true, - expected: 100, - }, - { - name: "no CPU usage", - stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 5_000_000_000), - useWallTime: true, - expected: 0, + name: "no CPU usage", + stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 5_000_000_000), + expected: 0, }, { - name: "CPU counter reset", - stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 1_000_000_000), - useWallTime: true, - expected: 0, + name: "CPU counter reset", + stats: cpuStatsResponse(base, time.Second, 5_000_000_000, 1_000_000_000), + expected: 0, }, { - name: "missing previous timestamp", - stats: cpuStatsResponse(time.Time{}, time.Second, 5_000_000_000, 6_000_000_000), - useWallTime: true, - expected: 0, + name: "missing previous timestamp", + stats: cpuStatsResponse(time.Time{}, time.Second, 5_000_000_000, 6_000_000_000), + expected: 0, }, { - name: "non-increasing timestamp", - stats: cpuStatsResponse(base, 0, 5_000_000_000, 6_000_000_000), - useWallTime: true, - expected: 0, + name: "non-increasing timestamp", + stats: cpuStatsResponse(base, 0, 5_000_000_000, 6_000_000_000), + expected: 0, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.expected, calculateDockerAbsoluteCpu(tt.stats, tt.useWallTime)) + require.Equal(t, tt.expected, calculateDockerAbsoluteCpu(tt.stats)) }) } } -func TestIsPodmanVersion(t *testing.T) { - version := types.Version{ - Components: []types.ComponentVersion{{Name: "Podman Engine"}}, - } - require.True(t, isPodmanVersion(version)) - - version.Components[0].Name = "Engine" - require.False(t, isPodmanVersion(version)) -} - func cpuStatsResponse(preRead time.Time, elapsed time.Duration, previous, current uint64) container.StatsResponse { return container.StatsResponse{ Read: preRead.Add(elapsed),