feat(inputs.nvidia_smi): add NVIDIA v13 schema support#18690
feat(inputs.nvidia_smi): add NVIDIA v13 schema support#18690lbatalha wants to merge 2 commits intoinfluxdata:masterfrom
Conversation
|
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 |
srebhan
left a comment
There was a problem hiding this comment.
Thanks for your contribution @lbatalha!
You are doing way too much in this PR and you should split it into at least three ones
- cleaning up existing code without any change in functionality
- fix issue in existing code around the set-if-used functions (if any)
- add support for schema v13
This makes PRs easier to review and allows us to pin-point behavior changes to a certain commit of issues appear...
| if v == "N/A" || v == "" || v == "Requested functionality has been deprecated" { | ||
| return | ||
| } |
There was a problem hiding this comment.
Can we move this check to a function please?
| if v == "N/A" || v == "" || v == "Requested functionality has been deprecated" { | |
| return | |
| } | |
| if !isUsed(v){ | |
| return | |
| } |
with
func isUsed(v string) bool {
return v == "N/A" || v == "" || v == "Requested functionality has been deprecated"
}However, you are now removing values from tags that were passed on before so you are changing the series produced by this plugin and in turn break backward compatibility!
There was a problem hiding this comment.
Hmm, then perhaps we should retain existing behavior.
|
Oh good point, I will clean up this PR to contain only the schema v13 changes for now. |
Summary
This adds support for nvsmi_device_v13.dtd schema.
I based myself on the work done in #18176.
I did not try to resolve the
pstatepart of #17417 but I did moveserialto a tag, shouldpstateno longer be a tag?I do not have a GPU capable of reporting ECC stats so I was unable to fully test that feature, though the two fields that do have values are reported correctly.
I have also fixed the typo in the error message and some GPUs were incorrectly labelled RTC instead of RTX
Checklist
Related issues
resolves #17482 and #17416 - partial #17417 fix