Add Label to disk plugin

[izenn]

Use Case

working with lvm and the disk plugin, filesystem usage is reported using the dm-# style device names. Per the diskio plugin this device is near-meaningless. In order to track disk usage through influxDB i would like the disk label added to the disk output.
Looking at the source of the code where the disk input plugin gets the disk info from (https://github.com/shirou/gopsutil/blob/master/disk/disk.go) the code to get the disk label looks to be there, it would just need to be folded into the disk.PartitionStat output:

// Label returns label of given device or empty string on error.
// Name of device is expected, eg. /dev/sda
// Supports label based on devicemapper name
// See https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-block-dm
func Label(name string) (string, error) {
	return LabelWithContext(context.Background(), name)
}

Expected behavior

it would be nice to have output from the disk plugin like this:
`{"device":"/dev/dm-0","mountpoint":"/","fstype":"xfs","opts":["rw","relatime"],"label":"rhel-root"}

instead of the current:
{"device":"/dev/dm-0","mountpoint":"/","fstype":"xfs","opts":["rw","relatime"]}

Actual behavior

I learned just enough go to be able to get a proof of concept to work, but i do not know struct's well enough to combine everything:

package main

import (
    "fmt"
    "regexp"
    "strings"
    "github.com/shirou/gopsutil/v3/disk"
)

func main() {
	parts, err := disk.Partitions(true)
	if err != nil {
		panic(err)
	}
	for _, part := range parts {
	    m, _ := regexp.MatchString(`^/dev/`, part.Device)
	    if (m) {
		dev := strings.Split(part.Device, "/")
		label, _ := disk.Label(dev[2])
		fmt.Println("Device:", part.Device, " Label", label)
	    }
        }
}

Additional info

No response

[powersj]

@izenn it looks like name from the Label method is read from /sys/block/<device>/dm/name. On my system using my reproducer from your forum post, that returns:

❯ cat /sys/block/dm-0/dm/name
prod_dent_d-prod_dent_d

disk,device=dm-1,fstype=xfs,host=ryzen,label=prod_dent_d-prod_dent_d,mode=rw,path=/storage/scsi2 total=17112760320i,free=16959598592i,used=153161728i,used_percent=0.8950147441789215,inodes_total=8388608i,inodes_free=8388605i,inodes_used=3i 1675358637000000000

Is that what you are after?

[izenn]

yes it is -- i believe i have figured out a way to get it into the disk plugin, but would appreciate a second set of eyes to see if what i did makes sense
(i'm trying to get rid of the regex match now) i was able to get the regex removed and have been testing locally. it looks good, but would still like a second set of eyes before i submit a pull request
master...izenn:telegraf:master

[powersj]

Thanks for working on this @izenn! Happy to see and have further conversation in a PR as well.

Since you are effectively taking the partition device and stripping the leading /dev/ off of it, why no do something similar to what is done here

strings.ReplaceAll(partitions[i].Device, "/dev/", "")

[izenn]

package main

import (
    "fmt"
    "strings"
    "path/filepath"
)

func main() {
	test_one := "/dev/dm-0"
	test_two := "/dev/disk/dev/dm-0"
	path_single, _ := filepath.Rel("/dev", test_one)
	path_multi, _ := filepath.Rel("/dev", test_two)
	fmt.Println("replace single: ", strings.Replace(test_one, "/dev/", "", 1))
	fmt.Println("replace multi: ", strings.Replace(test_two, "/dev/", "", 1))
	fmt.Println("all single: ", strings.ReplaceAll(test_one, "/dev/", ""))
	fmt.Println("all multi: ", strings.ReplaceAll(test_two, "/dev/", ""))
	fmt.Println("path single: ", path_single)
	fmt.Println("path multi: ", path_multi)
}

which would give:

replace single:  dm-0
replace multi:  disk/dev/dm-0
all single:  dm-0
all multi:  diskdm-0
path single:  dm-0
path multi:  disk/dev/dm-0

both replace and filepath give the same results. Is strings.replace/replaceall faster/lighter than filepath? i think strings.Replace would probably be safer than ReplaceAll just in case there is a second /dev/ in there somewhere.

[powersj]

Is strings.replace/replaceall faster/lighter than filepath?

I picked it only to be consistent

would probably be safer than ReplaceAll just in case there is a second /dev/ in there somewhere.

replace with a 1 is fine

[izenn]

removed the filepath and changed the disk.Label line to:
label, _ := disk.Label(strings.Replace(p.Device, "/dev/", "", 1))

let me know if you can think of anything else, otherwise i'll submit the pull request tomorrow

[powersj]

let me know if you can think of anything else, otherwise i'll submit the pull request tomorrow

Please do, it will have to be reviewed by myself and srebhan so putting it up sooner rather than later is good :)

[izenn]

submitted PR #12620

[powersj]

I'm going to re-open the issue till we land your PR. Thanks again!

[powersj]

@izenn can you try the artifacts in #12696? I'd still like to see you get the label as a tag, but this method is a bit more crude but specific to the disk plugin. Thanks!

[izenn]

initial tests look good, i'll deploy it to some of my development instances tomorrow to have a better test

[powersj]

initial tests look good, i'll deploy it to some of my development instances tomorrow to have a better test

Thanks for giving it a shot!