Include DEVLINKS in available diskio udev properties #5116
Conversation
for elegance and to allow compatibility with Go 10.9
glinton
added
the
feat
| @@ -14,6 +14,7 @@ import ( | |||
| var nullDiskInfo = []byte(` | |||
| E:MY_PARAM_1=myval1 | |||
| E:MY_PARAM_2=myval2 | |||
| S:foo/bar/devlink foo/bar/devlink1 | |||
There was a problem hiding this comment.
Shouldn't this be multiple lines like:
S:foo/bar/devlink
S:foo/bar/devlink1
There was a problem hiding this comment.
Yes, my mistake. I will update accordingly.
| @@ -47,6 +48,7 @@ func TestDiskInfo(t *testing.T) { | |||
| require.NoError(t, err) | |||
| assert.Equal(t, "myval1", di["MY_PARAM_1"]) | |||
| assert.Equal(t, "myval2", di["MY_PARAM_2"]) | |||
| assert.Equal(t, "foo/bar/devlink foo/bar/devlink1", di["DEVLINKS"]) | |||
There was a problem hiding this comment.
Based on running udevadm we should prepend "/dev/" to the links: /dev/foo/bar/devlink /dev/foo/bar/devlink1. I'm not sure if there are situations where the prefix would be something other than /dev, maybe in docker, but I think we could assume it for now unless we have a way to determine this.
There was a problem hiding this comment.
Good point (although it really demands a change to the function itself). I may try to check docker just to see, but otherwise I will definitely do this.
plugins/inputs/diskio/README.md
Outdated
| @@ -19,6 +19,9 @@ The diskio input plugin gathers metrics about disk traffic and timing. | |||
| ## Currently only Linux is supported via udev properties. You can view | |||
| ## available properties for a device by running: | |||
| ## 'udevadm info -q property -n /dev/sda' | |||
| ## Note: Most, but not all, udev properties can be accessed this way. Properties | |||
| ## that are currently accessible include the "IS_*" series and the "DEVLINKS" | |||
There was a problem hiding this comment.
Should be able to access any of the E: values, so that would be at least ID* and DM* and probably others. Might be easier to document fields that cannot be looked up: DEVTYPE, DEVNAME, DEVPATH.
There was a problem hiding this comment.
Makes sense.
Note no focuses on excluded udev properties
hopefully fix fmt issue
switch to using bytes.Buffer
|
@danielnelson I think you should be satisfied now |
|
Woohoo! Thanks! |
|
Thank you! |
Partial fix for #3663. The diskio plugin claims to grab udev properties when specified, but can't actually grab all of them due to the way it is written. One property it didn't grab is "DEVLINKS." Our company, Skytap, has a business need for using diskio to tag with DEVLINKS, and it was only a few lines of code to add that capability.
Required for all PRs: