Merge pull request #1262 from fahedouch/fix-stats-values

Fix stats values

cmd/nerdctl/stats.go

@@ -407,9 +407,8 @@ func collect(cmd *cobra.Command, s *statsutil.Stats, waitFirst *sync.WaitGroup,
 	}
 
 	go func() {
-
-		previousStats := make(map[string]uint64)
-
+		previousStats := new(statsutil.ContainerStats)
+		firstSet := true
 		for {
 			//task is in the for loop to avoid nil task just after Container creation
 			task, err := container.Task(ctx, nil)
@@ -425,9 +424,6 @@ func collect(cmd *cobra.Command, s *statsutil.Stats, waitFirst *sync.WaitGroup,
 				continue
 			}
 
-			//sleep to create distant CPU readings
-			time.Sleep(500 * time.Millisecond)
-
 			metric, err := task.Metrics(ctx)
 			if err != nil {
 				u <- err
@@ -445,16 +441,23 @@ func collect(cmd *cobra.Command, s *statsutil.Stats, waitFirst *sync.WaitGroup,
 				continue
 			}
 
-			statsEntry, err := renderStatsEntry(previousStats, anydata, int(task.Pid()), netNS.Interfaces)
+			// when (firstSet == true), we only set container stats without rendering stat entry
+			statsEntry, err := setContainerStatsAndRenderStatsEntry(previousStats, firstSet, anydata, int(task.Pid()), netNS.Interfaces)
 			if err != nil {
 				u <- err
 				continue
 			}
 			statsEntry.Name = clabels[labels.Name]
 			statsEntry.ID = container.ID()
 
-			s.SetStatistics(statsEntry)
+			if firstSet {
+				firstSet = false
+			} else {
+				s.SetStatistics(statsEntry)
+			}
 			u <- nil
+			//sleep to create distant CPU readings
+			time.Sleep(500 * time.Millisecond)
 		}
 	}()
 	for {

cmd/nerdctl/stats_freebsd.go

@@ -21,8 +21,6 @@ import (
 	"github.com/containerd/nerdctl/pkg/statsutil"
 )
 
-func renderStatsEntry(previousStats map[string]uint64, anydata interface{}, pid int, interfaces []native.NetInterface) (statsutil.StatsEntry, error) {
-
+func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStats, firstSet bool, anydata interface{}, pid int, interfaces []native.NetInterface) (statsutil.StatsEntry, error) {
 	return statsutil.StatsEntry{}, nil
-
 }

cmd/nerdctl/stats_linux.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"net"
 	"strings"
+	"time"
 
 	v1 "github.com/containerd/cgroups/stats/v1"
 	v2 "github.com/containerd/cgroups/v2/stats"
@@ -30,7 +31,7 @@ import (
 	"github.com/vishvananda/netns"
 )
 
-func renderStatsEntry(previousStats map[string]uint64, anydata interface{}, pid int, interfaces []native.NetInterface) (statsutil.StatsEntry, error) {
+func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStats, firstSet bool, anydata interface{}, pid int, interfaces []native.NetInterface) (statsutil.StatsEntry, error) {
 
 	var (
 		data       *v1.Metrics
@@ -47,55 +48,64 @@ func renderStatsEntry(previousStats map[string]uint64, anydata interface{}, pid
 		return statsutil.StatsEntry{}, errors.New("cannot convert metric data to cgroups.Metrics")
 	}
 
-	var (
-		err      error
-		nlinks   []netlink.Link
-		nlink    netlink.Link
-		nlHandle *netlink.Handle
-		ns       netns.NsHandle
-	)
+	var nlinks []netlink.Link
 
-	ns, err = netns.GetFromPid(pid)
-	if err != nil {
-		return statsutil.StatsEntry{}, fmt.Errorf("failed to retrieve the statistics in netns %s: %v", ns, err)
-	}
+	if !firstSet {
+		var (
+			err      error
+			nlink    netlink.Link
+			nlHandle *netlink.Handle
+			ns       netns.NsHandle
+		)
 
-	nlHandle, err = netlink.NewHandleAt(ns)
-	if err != nil {
-		return statsutil.StatsEntry{}, fmt.Errorf("failed to retrieve the statistics in netns %s: %v", ns, err)
-	}
+		ns, err = netns.GetFromPid(pid)
+		if err != nil {
+			return statsutil.StatsEntry{}, fmt.Errorf("failed to retrieve the statistics in netns %s: %v", ns, err)
+		}
 
-	for _, v := range interfaces {
-		nlink, err = nlHandle.LinkByIndex(v.Index)
+		nlHandle, err = netlink.NewHandleAt(ns)
 		if err != nil {
-			return statsutil.StatsEntry{}, fmt.Errorf("failed to retrieve the statistics for %s in netns %s: %v", v.Name, ns, err)
+			return statsutil.StatsEntry{}, fmt.Errorf("failed to retrieve the statistics in netns %s: %v", ns, err)
 		}
-		//exclude inactive interface
-		if nlink.Attrs().Flags&net.FlagUp != 0 {
 
-			//exclude loopback interface
-			if nlink.Attrs().Flags&net.FlagLoopback != 0 || strings.HasPrefix(nlink.Attrs().Name, "lo") {
-				continue
+		for _, v := range interfaces {
+			nlink, err = nlHandle.LinkByIndex(v.Index)
+			if err != nil {
+				return statsutil.StatsEntry{}, fmt.Errorf("failed to retrieve the statistics for %s in netns %s: %v", v.Name, ns, err)
+			}
+			//exclude inactive interface
+			if nlink.Attrs().Flags&net.FlagUp != 0 {
+
+				//exclude loopback interface
+				if nlink.Attrs().Flags&net.FlagLoopback != 0 || strings.HasPrefix(nlink.Attrs().Name, "lo") {
+					continue
+				}
+				nlinks = append(nlinks, nlink)
 			}
-			nlinks = append(nlinks, nlink)
 		}
 	}
 
+	var err error
 	if data != nil {
-		statsEntry, err = statsutil.SetCgroupStatsFields(previousStats["CgroupCPU"], previousStats["CgroupSystem"], data, nlinks)
-		previousStats["CgroupCPU"] = data.CPU.Usage.Total
-		previousStats["CgroupSystem"] = data.CPU.Usage.Kernel
+		if !firstSet {
+			statsEntry, err = statsutil.SetCgroupStatsFields(previousStats, data, nlinks)
+		}
+		previousStats.CgroupCPU = data.CPU.Usage.Total
+		previousStats.CgroupSystem = data.CPU.Usage.Kernel
 		if err != nil {
 			return statsutil.StatsEntry{}, err
 		}
 	} else if data2 != nil {
-		statsEntry, err = statsutil.SetCgroup2StatsFields(previousStats["Cgroup2CPU"], previousStats["Cgroup2System"], data2, nlinks)
-		previousStats["Cgroup2CPU"] = data2.CPU.UsageUsec * 1000
-		previousStats["Cgroup2System"] = data2.CPU.SystemUsec * 1000
+		if !firstSet {
+			statsEntry, err = statsutil.SetCgroup2StatsFields(previousStats, data2, nlinks)
+		}
+		previousStats.Cgroup2CPU = data2.CPU.UsageUsec * 1000
+		previousStats.Cgroup2System = data2.CPU.SystemUsec * 1000
 		if err != nil {
 			return statsutil.StatsEntry{}, err
 		}
 	}
+	previousStats.Time = time.Now()
 
 	return statsEntry, nil
 }

cmd/nerdctl/stats_windows.go

@@ -21,8 +21,6 @@ import (
 	"github.com/containerd/nerdctl/pkg/statsutil"
 )
 
-func renderStatsEntry(previousStats map[string]uint64, anydata interface{}, pid int, interfaces []native.NetInterface) (statsutil.StatsEntry, error) {
-
+func setContainerStatsAndRenderStatsEntry(previousStats *statsutil.ContainerStats, firstSet bool, anydata interface{}, pid int, interfaces []native.NetInterface) (statsutil.StatsEntry, error) {
 	return statsutil.StatsEntry{}, nil
-
 }

pkg/statsutil/stats.go

@@ -19,6 +19,7 @@ package statsutil
 import (
 	"fmt"
 	"sync"
+	"time"
 
 	units "github.com/docker/go-units"
 )
@@ -59,6 +60,13 @@ type Stats struct {
 	err error
 }
 
+// ContainerStats represents the runtime container stats
+type ContainerStats struct {
+	Time                        time.Time
+	CgroupCPU, Cgroup2CPU       uint64
+	CgroupSystem, Cgroup2System uint64
+}
+
 //NewStats is from https://github.com/docker/cli/blob/3fb4fb83dfb5db0c0753a8316f21aea54dab32c5/cli/command/container/formatter_stats.go#L113-L116
 func NewStats(container string) *Stats {
 	return &Stats{StatsEntry: StatsEntry{Container: container}}

pkg/statsutil/stats_linux.go

@@ -24,9 +24,9 @@ import (
 	"github.com/vishvananda/netlink"
 )
 
-func SetCgroupStatsFields(previousCgroupCPU, previousCgroupSystem uint64, data *v1.Metrics, links []netlink.Link) (StatsEntry, error) {
+func SetCgroupStatsFields(previousStats *ContainerStats, data *v1.Metrics, links []netlink.Link) (StatsEntry, error) {
 
-	cpuPercent := calculateCgroupCPUPercent(previousCgroupCPU, previousCgroupSystem, data)
+	cpuPercent := calculateCgroupCPUPercent(previousStats, data)
 	blkRead, blkWrite := calculateCgroupBlockIO(data)
 	mem := calculateCgroupMemUsage(data)
 	memLimit := float64(data.Memory.Usage.Limit)
@@ -48,9 +48,9 @@ func SetCgroupStatsFields(previousCgroupCPU, previousCgroupSystem uint64, data *
 
 }
 
-func SetCgroup2StatsFields(previousCgroup2CPU, previousCgroup2System uint64, metrics *v2.Metrics, links []netlink.Link) (StatsEntry, error) {
+func SetCgroup2StatsFields(previousStats *ContainerStats, metrics *v2.Metrics, links []netlink.Link) (StatsEntry, error) {
 
-	cpuPercent := calculateCgroup2CPUPercent(previousCgroup2CPU, previousCgroup2System, metrics)
+	cpuPercent := calculateCgroup2CPUPercent(previousStats, metrics)
 	blkRead, blkWrite := calculateCgroup2IO(metrics)
 	mem := calculateCgroup2MemUsage(metrics)
 	memLimit := float64(metrics.Memory.UsageLimit)
@@ -72,13 +72,13 @@ func SetCgroup2StatsFields(previousCgroup2CPU, previousCgroup2System uint64, met
 
 }
 
-func calculateCgroupCPUPercent(previousCPU, previousSystem uint64, metrics *v1.Metrics) float64 {
+func calculateCgroupCPUPercent(previousStats *ContainerStats, metrics *v1.Metrics) 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(previousCPU)
+		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(previousSystem)
+		systemDelta = float64(metrics.CPU.Usage.Kernel) - float64(previousStats.CgroupSystem)
 		onlineCPUs  = float64(len(metrics.CPU.Usage.PerCPU))
 	)
 
@@ -89,18 +89,18 @@ func calculateCgroupCPUPercent(previousCPU, previousSystem uint64, metrics *v1.M
 }
 
 //PercpuUsage is not supported in CgroupV2
-func calculateCgroup2CPUPercent(previousCPU, previousSystem uint64, metrics *v2.Metrics) float64 {
+func calculateCgroup2CPUPercent(previousStats *ContainerStats, metrics *v2.Metrics) float64 {
 	var (
 		cpuPercent = 0.0
 		// calculate the change for the cpu usage of the container in between readings
-		cpuDelta = float64(metrics.CPU.UsageUsec*1000) - float64(previousCPU)
+		cpuDelta = float64(metrics.CPU.UsageUsec*1000) - float64(previousStats.Cgroup2CPU)
 		// calculate the change for the entire system between readings
-		systemDelta = float64(metrics.CPU.SystemUsec*1000) - float64(previousSystem)
+		_ = float64(metrics.CPU.SystemUsec*1000) - float64(previousStats.Cgroup2System)
+		// time duration
+		timeDelta = time.Since(previousStats.Time)
 	)
-
-	u, _ := time.ParseDuration("500ms")
-	if systemDelta > 0.0 && cpuDelta > 0.0 {
-		cpuPercent = (cpuDelta + systemDelta) / float64(u.Nanoseconds()) * 100.0
+	if cpuDelta > 0.0 {
+		cpuPercent = cpuDelta / float64(timeDelta.Nanoseconds()) * 100.0
 	}
 	return cpuPercent
 }