fix(inputs.cpu): Compute Windows total from individual CPUs#19286
fix(inputs.cpu): Compute Windows total from individual CPUs#19286srebhan wants to merge 1 commit into
Conversation
telegraf-tiger
Bot
added
fix
403d9ad to
6e481c2
Compare
srebhan
added
area/system
ready for final review
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
skartikey
left a comment
skartikey
left a comment
There was a problem hiding this comment.
plugins/inputs/cpu/cpu_test.go TestCases skips on non-Linux and it's the only test, so none of the 245 lines of new Windows syscall code (queryWithGroups, group concat, total accumulation) ever runs. test-go-windows going green only means it compiles. Split the arithmetic out of the querying so it can be table-tested with a synthetic []systemProcessorPerformanceInformation.
| } | ||
|
|
||
| // Debug print the raw data for invalid values | ||
| if c.Log.Level().Includes(telegraf.Trace) && report { |
There was a problem hiding this comment.
is this trace block staying? The zero-delta path at line 104 sets report=true, which isn't an invalid value, so on an idle box this logs "Detected invalid field values!" on healthy data, and it dumps every CPU instead of the ones that actually failed valid().
| total.User += uint64(s.UserTime) | ||
| total.System += uint64(s.KernelTime - s.IdleTime) | ||
| total.Idle += uint64(s.IdleTime) | ||
| total.Irq += uint64(s.InterruptTime) |
There was a problem hiding this comment.
summing Irq into the total here (and into the denominator at line 109) changes the Windows cpu-total: usage_irq was structurally always zero via GetSystemTimes and now isn't, and usage_user/system/idle shift down as the denominator grew. Intended? Per-CPU is unaffected, but this moves existing Windows dashboards
| @@ -19,7 +19,7 @@ var sampleConfig string | |||
|
|
|||
| type CPU struct { | |||
| ps psutil.PS | |||
There was a problem hiding this comment.
ps is now write-only, set in the init factory and the test but never read since Gather stopped calling c.ps.CPUTimes. Drop the field, the two assignments, and the psutil import.
2 participants
Summary
Testing for now...
Checklist
Related issues
resolves #18141