<fix> stats: CPU perusage use the wrong systemUsage

Signed-off-by: zzzzzzzzzy9 <[email protected]>

Author: zzzzzzzzzy9

pkg/cmd/container/stats.go

@@ -379,6 +379,17 @@ func collect(ctx context.Context, globalOptions types.GlobalCommandOptions, s *s
 				continue
 			}
 
+			// Sample system CPU usage close to container usage to avoid
+			// noise in metric calculations.
+			systemUsage, onlineCPUs, err := getSystemCPUUsage()
+			if err != nil {
+				u <- err
+				continue
+			}
+			systemInfo := statsutil.SystemInfo{
+				OnlineCPUs:  onlineCPUs,
+				SystemUsage: systemUsage,
+			}
 			metric, err := task.Metrics(ctx)
 			if err != nil {
 				u <- err
@@ -397,7 +408,7 @@ func collect(ctx context.Context, globalOptions types.GlobalCommandOptions, s *s
 			}
 
 			// when (firstSet == true), we only set container stats without rendering stat entry
-			statsEntry, err := setContainerStatsAndRenderStatsEntry(previousStats, firstSet, anydata, int(task.Pid()), netNS.Interfaces)
+			statsEntry, err := setContainerStatsAndRenderStatsEntry(previousStats, firstSet, anydata, int(task.Pid()), netNS.Interfaces, systemInfo)
 			if err != nil {
 				u <- err
 				continue

pkg/cmd/container/stats_linux.go

@@ -17,9 +17,13 @@
 package container
 
 import (
+	"bufio"
 	"errors"
 	"fmt"
+	"io"
 	"net"
+	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -33,8 +37,17 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/statsutil"
 )
 
+const (
+	// The value comes from `C.sysconf(C._SC_CLK_TCK)`, and
+	// on Linux it's a constant which is safe to be hard coded,
+	// so we can avoid using cgo here. For details, see:
+	// https://github.com/containerd/cgroups/pull/12
+	clockTicksPerSecond  = 100
+	nanoSecondsPerSecond = 1e9
+)
+
 //nolint:nakedret
-func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStats, firstSet bool, anydata interface{}, pid int, interfaces []native.NetInterface) (statsEntry statsutil.StatsEntry, err error) {
+func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStats, firstSet bool, anydata interface{}, pid int, interfaces []native.NetInterface, systemInfo statsutil.SystemInfo) (statsEntry statsutil.StatsEntry, err error) {
 
 	var (
 		data  *v1.Metrics
@@ -96,10 +109,10 @@ func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStat
 
 	if data != nil {
 		if !firstSet {
-			statsEntry, err = statsutil.SetCgroupStatsFields(previousStats, data, nlinks)
+			statsEntry, err = statsutil.SetCgroupStatsFields(previousStats, data, nlinks, systemInfo)
 		}
 		previousStats.CgroupCPU = data.CPU.Usage.Total
-		previousStats.CgroupSystem = data.CPU.Usage.Kernel
+		previousStats.CgroupSystem = systemInfo.SystemUsage
 		if err != nil {
 			return
 		}
@@ -117,3 +130,59 @@ func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStat
 
 	return
 }
+
+// getSystemCPUUsage reads the system's CPU usage from /proc/stat and returns
+// the total CPU usage in nanoseconds and the number of CPUs.
+func getSystemCPUUsage() (cpuUsage uint64, cpuNum uint32, _ error) {
+	f, err := os.Open("/proc/stat")
+	if err != nil {
+		return 0, 0, err
+	}
+	defer f.Close()
+
+	return readSystemCPUUsage(f)
+}
+
+// readSystemCPUUsage parses CPU usage information from a reader providing
+// /proc/stat format data. It returns the total CPU usage in nanoseconds
+// and the number of CPUs. More:
+// https://github.com/moby/moby/blob/26db31fdab628a2345ed8f179e575099384166a9/daemon/stats_unix.go#L327-L368
+func readSystemCPUUsage(r io.Reader) (cpuUsage uint64, cpuNum uint32, _ error) {
+	rdr := bufio.NewReaderSize(r, 1024)
+
+	for {
+		data, isPartial, err := rdr.ReadLine()
+
+		if err != nil {
+			return 0, 0, fmt.Errorf("error scanning /proc/stat file: %w", err)
+		}
+		// Assume all cpu* records are at the start of the file, like glibc:
+		// https://github.com/bminor/glibc/blob/5d00c201b9a2da768a79ea8d5311f257871c0b43/sysdeps/unix/sysv/linux/getsysstats.c#L108-L135
+		if isPartial || len(data) < 4 {
+			break
+		}
+		line := string(data)
+		if line[:3] != "cpu" {
+			break
+		}
+		if line[3] == ' ' {
+			parts := strings.Fields(line)
+			if len(parts) < 8 {
+				return 0, 0, fmt.Errorf("invalid number of cpu fields")
+			}
+			var totalClockTicks uint64
+			for _, i := range parts[1:8] {
+				v, err := strconv.ParseUint(i, 10, 64)
+				if err != nil {
+					return 0, 0, fmt.Errorf("unable to convert value %s to int: %w", i, err)
+				}
+				totalClockTicks += v
+			}
+			cpuUsage = (totalClockTicks * nanoSecondsPerSecond) / clockTicksPerSecond
+		}
+		if '0' <= line[3] && line[3] <= '9' {
+			cpuNum++
+		}
+	}
+	return cpuUsage, cpuNum, nil
+}

pkg/cmd/container/stats_nolinux.go

@@ -23,6 +23,12 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/statsutil"
 )
 
-func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStats, firstSet bool, anydata interface{}, pid int, interfaces []native.NetInterface) (statsutil.StatsEntry, error) {
+func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStats, firstSet bool, anydata interface{}, pid int, interfaces []native.NetInterface, systemInfo statsutil.SystemInfo) (statsutil.StatsEntry, error) {
 	return statsutil.StatsEntry{}, nil
 }
+
+// getSystemCPUUsage reads the system's CPU usage from /proc/stat and returns
+// the total CPU usage in nanoseconds and the number of CPUs.
+func getSystemCPUUsage() (uint64, uint32, error) {
+	return 0, 0, nil
+}

pkg/statsutil/stats.go

@@ -26,6 +26,11 @@ import (
 	units "github.com/docker/go-units"
 )
 
+type SystemInfo struct {
+	OnlineCPUs  uint32
+	SystemUsage uint64
+}
+
 // StatsEntry represents the statistics data collected from a container
 type StatsEntry struct {
 	Name             string

pkg/statsutil/stats_linux.go

@@ -38,8 +38,8 @@ func calculateMemPercent(limit float64, usedNo float64) float64 {
 	return 0
 }
 
-func SetCgroupStatsFields(previousStats *ContainerStats, data *v1.Metrics, links []netlink.Link) (StatsEntry, error) {
-	cpuPercent := calculateCgroupCPUPercent(previousStats, data)
+func SetCgroupStatsFields(previousStats *ContainerStats, data *v1.Metrics, links []netlink.Link, systemInfo SystemInfo) (StatsEntry, error) {
+	cpuPercent := calculateCgroupCPUPercent(previousStats, data, systemInfo)
 	blkRead, blkWrite := calculateCgroupBlockIO(data)
 	mem := calculateCgroupMemUsage(data)
 	memLimit := getCgroupMemLimit(float64(data.Memory.Usage.Limit))
@@ -114,18 +114,21 @@ func getHostMemLimit() float64 {
 	return float64(^uint64(0))
 }
 
-func calculateCgroupCPUPercent(previousStats *ContainerStats, metrics *v1.Metrics) float64 {
+func calculateCgroupCPUPercent(previousStats *ContainerStats, metrics *v1.Metrics, systemInfo SystemInfo) float64 {
 	var (
 		cpuPercent = 0.0
 		// calculate the change for the cpu usage of the container in between readings
 		cpuDelta = float64(metrics.CPU.Usage.Total) - float64(previousStats.CgroupCPU)
 		// calculate the change for the entire system between readings
-		systemDelta = float64(metrics.CPU.Usage.Kernel) - float64(previousStats.CgroupSystem)
-		onlineCPUs  = float64(len(metrics.CPU.Usage.PerCPU))
+		systemDelta = float64(systemInfo.SystemUsage) - float64(previousStats.CgroupSystem)
+		onlineCPUs  = systemInfo.OnlineCPUs
 	)
 
+	if onlineCPUs == 0 {
+		onlineCPUs = uint32(len(metrics.CPU.Usage.PerCPU))
+	}
 	if systemDelta > 0.0 && cpuDelta > 0.0 {
-		cpuPercent = (cpuDelta / systemDelta) * onlineCPUs * 100.0
+		cpuPercent = (cpuDelta / systemDelta) * float64(onlineCPUs) * 100.0
 	}
 	return cpuPercent
 }