diff --git a/.changelog/18835.txt b/.changelog/18835.txt new file mode 100644 index 000000000..99d4ae17e --- /dev/null +++ b/.changelog/18835.txt @@ -0,0 +1,3 @@ +```release-note:bug +metrics: Fixed a bug where CPU counters could report errors for negative values +``` diff --git a/client/stats/host.go b/client/stats/host.go index bd6edb32d..a28e2716e 100644 --- a/client/stats/host.go +++ b/client/stats/host.go @@ -277,7 +277,7 @@ func (h *HostCpuStatsCalculator) Calculate(times cpu.TimesStat) (idle float64, u currentIdle := times.Idle currentUser := times.User currentSystem := times.System - currentTotal := times.Total() + currentTotal := times.Total() // this is Idle + currentBusy currentBusy := times.User + times.System + times.Nice + times.Iowait + times.Irq + times.Softirq + times.Steal + times.Guest + times.GuestNice @@ -288,16 +288,16 @@ func (h *HostCpuStatsCalculator) Calculate(times cpu.TimesStat) (idle float64, u total = ((currentBusy - h.prevBusy) / deltaTotal) * 100 // Protect against any invalid values - if math.IsNaN(idle) || math.IsInf(idle, 0) { + if math.IsNaN(idle) || math.IsInf(idle, 0) || idle < 0.0 { idle = 100.0 } - if math.IsNaN(user) || math.IsInf(user, 0) { + if math.IsNaN(user) || math.IsInf(user, 0) || user < 0.0 { user = 0.0 } - if math.IsNaN(system) || math.IsInf(system, 0) { + if math.IsNaN(system) || math.IsInf(system, 0) || system < 0.0 { system = 0.0 } - if math.IsNaN(total) || math.IsInf(total, 0) { + if math.IsNaN(total) || math.IsInf(total, 0) || total < 0 { total = 0.0 } diff --git a/client/stats/host_test.go b/client/stats/host_test.go index 3d939cd10..4aada7ddf 100644 --- a/client/stats/host_test.go +++ b/client/stats/host_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/shirou/gopsutil/v3/cpu" + "github.com/shoenig/test/must" ) func TestHostCpuStatsCalculator_Nan(t *testing.T) { @@ -20,16 +21,34 @@ func TestHostCpuStatsCalculator_Nan(t *testing.T) { calculator.Calculate(times) idle, user, system, total := calculator.Calculate(times) - if idle != 100.0 { - t.Errorf("idle: Expected: %f, Got %f", 100.0, idle) - } - if user != 0.0 { - t.Errorf("user: Expected: %f, Got %f", 0.0, user) - } - if system != 0.0 { - t.Errorf("system: Expected: %f, Got %f", 0.0, system) - } - if total != 0.0 { - t.Errorf("total: Expected: %f, Got %f", 0.0, total) - } + must.Eq(t, 100.0, idle, must.Sprint("unexpected idle stats")) + must.Eq(t, 0.0, user, must.Sprint("unexpected user stats")) + must.Eq(t, 0.0, system, must.Sprint("unexpected system stats")) + must.Eq(t, 0.0, total, must.Sprint("unexpected total stats")) +} + +func TestHostCpuStatsCalculator_DecreasedIOWait(t *testing.T) { + times := cpu.TimesStat{ + CPU: "cpu0", + User: 20000, + Nice: 100, + System: 9000, + Idle: 370000, + Iowait: 700, + } + + calculator := NewHostCpuStatsCalculator() + calculator.Calculate(times) + + times = cpu.TimesStat{ + CPU: "cpu0", + User: 20000, + Nice: 100, + System: 9000, + Idle: 380000, + Iowait: 600, + } + + _, _, _, total := calculator.Calculate(times) + must.GreaterEq(t, 0.0, total, must.Sprint("total must never be negative")) }