-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(inputs.net): Add speed metric #14625
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick PR! Question about the -1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree to have a test for the -1
value.
Also, wouldn't it be better to already be on-par with the upstream gopsutil and have a separate field for in and out? Like speed_in
and speed_out
? (Seems to be only supported by Windows to have different values)
The net plugin only supports linux. If someone eventually gets gopsutil support enabled for other distros, then that could be re-evaluted down the road for Windows users. |
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 |
Oh sorry, didn’t check that. Never mind my remark then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DStrand1 thanks for the clean PR! Just one question about the return value...
Summary
Adds a speed metric, reading
/sys/class/net/*/speed
as documented here in the kernel documentation. Currently only implemented for Linux. There is a PR to gopsutil, shirou/gopsutil#1473, which implements this metric as part of its interface and supports more platforms, but we decided to move forward with implementing this ourselves for the time being.Checklist
Related issues
resolves #4658