From ec4dcd417d265edc0d03d364aa92edb0a9ae2639 Mon Sep 17 00:00:00 2001 From: Sven Rebhan <36194019+srebhan@users.noreply.github.com> Date: Mon, 18 Mar 2024 22:05:00 +0100 Subject: [PATCH] fix(inputs.diskio): Add missing udev properties (#15003) (cherry picked from commit c9fb4e74be36aa14d3e3f54d25dc0fa5f81c573a) --- plugins/inputs/diskio/README.md | 26 ++--- plugins/inputs/diskio/diskio.go | 15 +++ plugins/inputs/diskio/diskio_linux.go | 125 ++++++++++++++------- plugins/inputs/diskio/diskio_linux_test.go | 99 ++++++---------- plugins/inputs/diskio/diskio_other.go | 19 +--- plugins/inputs/diskio/sample.conf | 26 ++--- plugins/inputs/diskio/testdata/udev.txt | 4 + plugins/inputs/diskio/testdata/uevent | 5 + 8 files changed, 166 insertions(+), 153 deletions(-) create mode 100644 plugins/inputs/diskio/testdata/udev.txt create mode 100644 plugins/inputs/diskio/testdata/uevent diff --git a/plugins/inputs/diskio/README.md b/plugins/inputs/diskio/README.md index c9573678dc8ee..de550f32eae41 100644 --- a/plugins/inputs/diskio/README.md +++ b/plugins/inputs/diskio/README.md @@ -16,24 +16,20 @@ See the [CONFIGURATION.md][CONFIGURATION.md] for more details. ```toml @sample.conf # Read metrics about disk IO by device [[inputs.diskio]] - ## By default, telegraf will gather stats for all devices including - ## disk partitions. - ## Setting devices will restrict the stats to the specified devices. - ## NOTE: Globbing expressions (e.g. asterix) are not supported for - ## disk synonyms like '/dev/disk/by-id'. - # devices = ["sda", "sdb", "vd*", "/dev/disk/by-id/nvme-eui.00123deadc0de123"] - ## Uncomment the following line if you need disk serial numbers. - # skip_serial_number = false - # - ## On systems which support it, device metadata can be added in the form of - ## tags. - ## 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' + ## Devices to collect stats for + ## Wildcards are supported except for disk synonyms like '/dev/disk/by-id'. + ## ex. devices = ["sda", "sdb", "vd*", "/dev/disk/by-id/nvme-eui.00123deadc0de123"] + # devices = ["*"] + + ## Skip gathering of the disk's serial numbers. + # skip_serial_number = true + + ## Device metadata tags to add on systems supporting it (Linux only) + ## Use 'udevadm info -q property -n ' to get a list of properties. ## Note: Most, but not all, udev properties can be accessed this way. Properties ## that are currently inaccessible include DEVTYPE, DEVNAME, and DEVPATH. # device_tags = ["ID_FS_TYPE", "ID_FS_USAGE"] - # + ## Using the same metadata source as device_tags, you can also customize the ## name of the device via templates. ## The 'name_templates' parameter is a list of templates to try and apply to diff --git a/plugins/inputs/diskio/diskio.go b/plugins/inputs/diskio/diskio.go index 5b376c9f7398b..974cc82d1e96d 100644 --- a/plugins/inputs/diskio/diskio.go +++ b/plugins/inputs/diskio/diskio.go @@ -25,6 +25,18 @@ func hasMeta(s string) bool { return strings.ContainsAny(s, "*?[") } +type DiskIO struct { + Devices []string `toml:"devices"` + DeviceTags []string `toml:"device_tags"` + NameTemplates []string `toml:"name_templates"` + SkipSerialNumber bool `toml:"skip_serial_number"` + Log telegraf.Logger `toml:"-"` + + ps system.PS + infoCache map[string]diskInfoCache + deviceFilter filter.Filter +} + func (*DiskIO) SampleConfig() string { return sampleConfig } @@ -39,6 +51,9 @@ func (d *DiskIO) Init() error { d.deviceFilter = deviceFilter } } + + d.infoCache = make(map[string]diskInfoCache) + return nil } diff --git a/plugins/inputs/diskio/diskio_linux.go b/plugins/inputs/diskio/diskio_linux.go index 69c668f30e4ee..e541604d85072 100644 --- a/plugins/inputs/diskio/diskio_linux.go +++ b/plugins/inputs/diskio/diskio_linux.go @@ -11,51 +11,30 @@ import ( "strings" "golang.org/x/sys/unix" - - "github.com/influxdata/telegraf" - "github.com/influxdata/telegraf/filter" - "github.com/influxdata/telegraf/plugins/inputs/system" ) -type DiskIO struct { - ps system.PS - - Devices []string - DeviceTags []string - NameTemplates []string - SkipSerialNumber bool - - Log telegraf.Logger - - infoCache map[string]diskInfoCache - deviceFilter filter.Filter -} - type diskInfoCache struct { modifiedAt int64 // Unix Nano timestamp of the last modification of the device. This value is used to invalidate the cache udevDataPath string + sysBlockPath string values map[string]string } func (d *DiskIO) diskInfo(devName string) (map[string]string, error) { - var err error - var stat unix.Stat_t - + // Check if the device exists path := "/dev/" + devName - err = unix.Stat(path, &stat) - if err != nil { + var stat unix.Stat_t + if err := unix.Stat(path, &stat); err != nil { return nil, err } - if d.infoCache == nil { - d.infoCache = map[string]diskInfoCache{} - } + // Check if we already got a cached and valid entry ic, ok := d.infoCache[devName] - if ok && stat.Mtim.Nano() == ic.modifiedAt { return ic.values, nil } + // Determine udev properties var udevDataPath string if ok && len(ic.udevDataPath) > 0 { // We can reuse the udev data path from a "previous" entry. @@ -65,33 +44,60 @@ func (d *DiskIO) diskInfo(devName string) (map[string]string, error) { major := unix.Major(uint64(stat.Rdev)) //nolint:unconvert // Conversion needed for some architectures minor := unix.Minor(uint64(stat.Rdev)) //nolint:unconvert // Conversion needed for some architectures udevDataPath = fmt.Sprintf("/run/udev/data/b%d:%d", major, minor) - - _, err := os.Stat(udevDataPath) - if err != nil { + if _, err := os.Stat(udevDataPath); err != nil { // This path failed, try the fallback .udev style (non-systemd) udevDataPath = "/dev/.udev/db/block:" + devName - _, err := os.Stat(udevDataPath) - if err != nil { + if _, err := os.Stat(udevDataPath); err != nil { // Giving up, cannot retrieve disk info return nil, err } } } - // Final open of the confirmed (or the previously detected/used) udev file - f, err := os.Open(udevDataPath) + + info, err := readUdevData(udevDataPath) if err != nil { return nil, err } - defer f.Close() - di := map[string]string{} + // Read additional device properties + var sysBlockPath string + if ok && len(ic.sysBlockPath) > 0 { + // We can reuse the /sys block path from a "previous" entry. + // This allows us to also "poison" it during test scenarios + sysBlockPath = ic.sysBlockPath + } else { + sysBlockPath = "/sys/block/" + devName + if _, err := os.Stat(sysBlockPath); err != nil { + // Giving up, cannot retrieve additional info + return nil, err + } + } + devInfo, err := readDevData(sysBlockPath) + if err != nil { + return nil, err + } + for k, v := range devInfo { + info[k] = v + } d.infoCache[devName] = diskInfoCache{ modifiedAt: stat.Mtim.Nano(), udevDataPath: udevDataPath, - values: di, + values: info, } + return info, nil +} + +func readUdevData(path string) (map[string]string, error) { + // Final open of the confirmed (or the previously detected/used) udev file + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + info := make(map[string]string) scnr := bufio.NewScanner(f) var devlinks bytes.Buffer for scnr.Scan() { @@ -114,14 +120,51 @@ func (d *DiskIO) diskInfo(devName string) (map[string]string, error) { if len(kv) < 2 { continue } - di[kv[0]] = kv[1] + info[kv[0]] = kv[1] } if devlinks.Len() > 0 { - di["DEVLINKS"] = devlinks.String() + info["DEVLINKS"] = devlinks.String() + } + + return info, nil +} + +func readDevData(path string) (map[string]string, error) { + // Open the file and read line-wise + f, err := os.Open(filepath.Join(path, "uevent")) + if err != nil { + return nil, err + } + defer f.Close() + + // Read DEVNAME and DEVTYPE + info := make(map[string]string) + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := scanner.Text() + if !strings.HasPrefix(line, "DEV") { + continue + } + + k, v, found := strings.Cut(line, "=") + if !found { + continue + } + info[strings.TrimSpace(k)] = strings.TrimSpace(v) + } + if d, found := info["DEVNAME"]; found && !strings.HasPrefix(d, "/dev") { + info["DEVNAME"] = "/dev/" + d + } + + // Find the DEVPATH property + if devlnk, err := filepath.EvalSymlinks(filepath.Join(path, "device")); err == nil { + devlnk = filepath.Join(devlnk, filepath.Base(path)) + devlnk = strings.TrimPrefix(devlnk, "/sys") + info["DEVPATH"] = devlnk } - return di, nil + return info, nil } func resolveName(name string) string { @@ -129,7 +172,7 @@ func resolveName(name string) string { if err == nil { return resolved } - if err != nil && !errors.Is(err, fs.ErrNotExist) { + if !errors.Is(err, fs.ErrNotExist) { return name } // Try to prepend "/dev" diff --git a/plugins/inputs/diskio/diskio_linux_test.go b/plugins/inputs/diskio/diskio_linux_test.go index 350b65203a1b3..dd69cd1c2c884 100644 --- a/plugins/inputs/diskio/diskio_linux_test.go +++ b/plugins/inputs/diskio/diskio_linux_test.go @@ -3,72 +3,29 @@ package diskio import ( - "os" + "fmt" "testing" "github.com/stretchr/testify/require" ) -var nullDiskInfo = []byte(` -E:MY_PARAM_1=myval1 -E:MY_PARAM_2=myval2 -S:foo/bar/devlink -S:foo/bar/devlink1 -`) - -// setupNullDisk sets up fake udev info as if /dev/null were a disk. -func setupNullDisk(t *testing.T, s *DiskIO, devName string) func() { - td, err := os.CreateTemp("", ".telegraf.DiskInfoTest") - require.NoError(t, err) - - if s.infoCache == nil { - s.infoCache = make(map[string]diskInfoCache) - } - ic, ok := s.infoCache[devName] - if !ok { - // No previous calls for the device were done, easy to poison the cache - s.infoCache[devName] = diskInfoCache{ - modifiedAt: 0, - udevDataPath: td.Name(), - values: map[string]string{}, - } - } - origUdevPath := ic.udevDataPath - - cleanFunc := func() { - ic.udevDataPath = origUdevPath - os.Remove(td.Name()) - } - - ic.udevDataPath = td.Name() - _, err = td.Write(nullDiskInfo) - if err != nil { - cleanFunc() - t.Fatal(err) - } - - return cleanFunc -} - func TestDiskInfo(t *testing.T) { - s := &DiskIO{} - clean := setupNullDisk(t, s, "null") - defer clean() - di, err := s.diskInfo("null") - require.NoError(t, err) - require.Equal(t, "myval1", di["MY_PARAM_1"]) - require.Equal(t, "myval2", di["MY_PARAM_2"]) - require.Equal(t, "/dev/foo/bar/devlink /dev/foo/bar/devlink1", di["DEVLINKS"]) - - // test that data is cached - clean() + plugin := &DiskIO{ + infoCache: map[string]diskInfoCache{ + "null": { + modifiedAt: 0, + udevDataPath: "testdata/udev.txt", + sysBlockPath: "testdata", + values: map[string]string{}, + }, + }, + } - di, err = s.diskInfo("null") + di, err := plugin.diskInfo("null") require.NoError(t, err) require.Equal(t, "myval1", di["MY_PARAM_1"]) require.Equal(t, "myval2", di["MY_PARAM_2"]) require.Equal(t, "/dev/foo/bar/devlink /dev/foo/bar/devlink1", di["DEVLINKS"]) - // unfortunately we can't adjust mtime on /dev/null to test cache invalidation } // DiskIOStats.diskName isn't a linux specific function, but dependent @@ -89,25 +46,39 @@ func TestDiskIOStats_diskName(t *testing.T) { {[]string{"$MY_PARAM_2/$MISSING"}, "null"}, } - for _, tc := range tests { - func() { - s := DiskIO{ + for i, tc := range tests { + t.Run(fmt.Sprintf("template %d", i), func(t *testing.T) { + plugin := DiskIO{ NameTemplates: tc.templates, + infoCache: map[string]diskInfoCache{ + "null": { + modifiedAt: 0, + udevDataPath: "testdata/udev.txt", + sysBlockPath: "testdata", + values: map[string]string{}, + }, + }, } - defer setupNullDisk(t, &s, "null")() //nolint:revive // done on purpose, cleaning will be executed properly - name, _ := s.diskName("null") + name, _ := plugin.diskName("null") require.Equal(t, tc.expected, name, "Templates: %#v", tc.templates) - }() + }) } } // DiskIOStats.diskTags isn't a linux specific function, but dependent // functions are a no-op on non-Linux. func TestDiskIOStats_diskTags(t *testing.T) { - s := &DiskIO{ + plugin := &DiskIO{ DeviceTags: []string{"MY_PARAM_2"}, + infoCache: map[string]diskInfoCache{ + "null": { + modifiedAt: 0, + udevDataPath: "testdata/udev.txt", + sysBlockPath: "testdata", + values: map[string]string{}, + }, + }, } - defer setupNullDisk(t, s, "null")() //nolint:revive // done on purpose, cleaning will be executed properly - dt := s.diskTags("null") + dt := plugin.diskTags("null") require.Equal(t, map[string]string{"MY_PARAM_2": "myval2"}, dt) } diff --git a/plugins/inputs/diskio/diskio_other.go b/plugins/inputs/diskio/diskio_other.go index 63a9d24168e90..44507ae3f45da 100644 --- a/plugins/inputs/diskio/diskio_other.go +++ b/plugins/inputs/diskio/diskio_other.go @@ -2,24 +2,7 @@ package diskio -import ( - "github.com/influxdata/telegraf" - "github.com/influxdata/telegraf/filter" - "github.com/influxdata/telegraf/plugins/inputs/system" -) - -type DiskIO struct { - ps system.PS - - Devices []string - DeviceTags []string - NameTemplates []string - SkipSerialNumber bool - - Log telegraf.Logger - - deviceFilter filter.Filter -} +type diskInfoCache struct{} func (*DiskIO) diskInfo(_ string) (map[string]string, error) { return nil, nil diff --git a/plugins/inputs/diskio/sample.conf b/plugins/inputs/diskio/sample.conf index 99ff4aa71e0da..145fbbb083377 100644 --- a/plugins/inputs/diskio/sample.conf +++ b/plugins/inputs/diskio/sample.conf @@ -1,23 +1,19 @@ # Read metrics about disk IO by device [[inputs.diskio]] - ## By default, telegraf will gather stats for all devices including - ## disk partitions. - ## Setting devices will restrict the stats to the specified devices. - ## NOTE: Globbing expressions (e.g. asterix) are not supported for - ## disk synonyms like '/dev/disk/by-id'. - # devices = ["sda", "sdb", "vd*", "/dev/disk/by-id/nvme-eui.00123deadc0de123"] - ## Uncomment the following line if you need disk serial numbers. - # skip_serial_number = false - # - ## On systems which support it, device metadata can be added in the form of - ## tags. - ## 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' + ## Devices to collect stats for + ## Wildcards are supported except for disk synonyms like '/dev/disk/by-id'. + ## ex. devices = ["sda", "sdb", "vd*", "/dev/disk/by-id/nvme-eui.00123deadc0de123"] + # devices = ["*"] + + ## Skip gathering of the disk's serial numbers. + # skip_serial_number = true + + ## Device metadata tags to add on systems supporting it (Linux only) + ## Use 'udevadm info -q property -n ' to get a list of properties. ## Note: Most, but not all, udev properties can be accessed this way. Properties ## that are currently inaccessible include DEVTYPE, DEVNAME, and DEVPATH. # device_tags = ["ID_FS_TYPE", "ID_FS_USAGE"] - # + ## Using the same metadata source as device_tags, you can also customize the ## name of the device via templates. ## The 'name_templates' parameter is a list of templates to try and apply to diff --git a/plugins/inputs/diskio/testdata/udev.txt b/plugins/inputs/diskio/testdata/udev.txt new file mode 100644 index 0000000000000..dcb591db35e1c --- /dev/null +++ b/plugins/inputs/diskio/testdata/udev.txt @@ -0,0 +1,4 @@ +E:MY_PARAM_1=myval1 +E:MY_PARAM_2=myval2 +S:foo/bar/devlink +S:foo/bar/devlink1 \ No newline at end of file diff --git a/plugins/inputs/diskio/testdata/uevent b/plugins/inputs/diskio/testdata/uevent new file mode 100644 index 0000000000000..be6501339ea01 --- /dev/null +++ b/plugins/inputs/diskio/testdata/uevent @@ -0,0 +1,5 @@ +MAJOR=259 +MINOR=1 +DEVNAME=null +DEVTYPE=disk +DISKSEQ=2