From 34984dcea1bc5cb21ebc2877fb6f8a06e33b09e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Ramos=20Medina?= Date: Thu, 9 Oct 2025 00:53:07 +0100 Subject: [PATCH 1/2] Refactor disk metrics collection and improve comments disk: support LVM/device-mapper by resolving /dev/mapper to dm-* and trying multiple candidates in collectIOStats --- internal/metric/disk.go | 101 +++++++++++++++++++++++++++++----------- 1 file changed, 75 insertions(+), 26 deletions(-) diff --git a/internal/metric/disk.go b/internal/metric/disk.go index 7081a73..f61c4c3 100644 --- a/internal/metric/disk.go +++ b/internal/metric/disk.go @@ -1,6 +1,7 @@ package metric import ( + "path/filepath" "runtime" "strings" @@ -24,7 +25,6 @@ func isDevPrefixed(p disk.PartitionStat) bool { // isWindowsDrive checks if the device is a Windows drive (C:, D:, etc.). func isWindowsDrive(p disk.PartitionStat) bool { - // Windows drives typically look like "C:", "D:", etc. device := strings.TrimSpace(p.Device) if len(device) >= 2 { return device[1] == ':' && ((device[0] >= 'A' && device[0] <= 'Z') || (device[0] >= 'a' && device[0] <= 'z')) @@ -99,7 +99,7 @@ func collectPartitionMetrics(partition disk.PartitionStat) (*DiskData, CustomErr return nil, *usageErr } - // Combine all metrics into DiskData structure + // Combine all metrics into a DiskData structure return &DiskData{ Device: partition.Device, TotalBytes: &usageStats.Total, @@ -119,31 +119,80 @@ func collectPartitionMetrics(partition disk.PartitionStat) (*DiskData, CustomErr }, CustomErr{} } -// collectIOStats collects IO-related metrics for a device. +// collectIOStats gathers IO-related metrics for a device. +// Supports LVM/device-mapper by resolving /dev/mapper/* -> /dev/dm-* +// and trying multiple key candidates against the map returned by disk.IOCounters(). func collectIOStats(device string) (*disk.IOCountersStat, *CustomErr) { - diskIOCounts, diskIOErr := disk.IOCounters(device) - if diskIOErr != nil { + // Get all counters once and look up by key + all, err := disk.IOCounters() + if err != nil { return nil, &CustomErr{ Metric: []string{"disk.read_bytes", "disk.write_bytes", "disk.read_time", "disk.write_time"}, - Error: diskIOErr.Error() + " " + device, + Error: err.Error() + " " + device, } } - // Extract device name for lookup (handle cross-platform differences) - deviceName := device - if runtime.GOOS != "windows" { - deviceName = strings.TrimPrefix(device, "/dev/") + candidates := buildDeviceKeyCandidates(device) + + // 1) Direct map key match + for _, k := range candidates { + if stat, ok := all[k]; ok { + return &stat, nil + } } - stats, exists := diskIOCounts[deviceName] - if !exists { - return nil, &CustomErr{ - Metric: []string{"disk.read_bytes", "disk.write_bytes", "disk.read_time", "disk.write_time"}, - Error: "device stats not found: " + device, + // 2) Fallback: match by stat.Name field + for _, stat := range all { + for _, k := range candidates { + if stat.Name == k { + s := stat + return &s, nil + } } } - return &stats, nil + return nil, &CustomErr{ + Metric: []string{"disk.read_bytes", "disk.write_bytes", "disk.read_time", "disk.write_time"}, + Error: "device stats not found: " + device + " (tried: " + strings.Join(candidates, ", ") + ")", + } +} + +// buildDeviceKeyCandidates returns possible keys for the disk.IOCounters() map. +// Handles paths like /dev/sda, /dev/nvme0n1, /dev/mapper/vg-lv -> dm-0, etc. +func buildDeviceKeyCandidates(device string) []string { + if runtime.GOOS == "windows" { + // On Windows, gopsutil uses names like "C:", so keep as-is. + d := strings.TrimSpace(device) + return []string{d} + } + + var out []string + d := strings.TrimSpace(device) + + // Strip /dev/ + out = append(out, strings.TrimPrefix(d, "/dev/")) + // Basename (e.g., /dev/mapper/vg-lv -> vg-lv) + out = append(out, filepath.Base(d)) + + // Resolve symlinks: /dev/mapper/vg-lv -> /dev/dm-0 -> dm-0 + if resolved, err := filepath.EvalSymlinks(d); err == nil && resolved != "" { + out = append(out, strings.TrimPrefix(resolved, "/dev/")) + out = append(out, filepath.Base(resolved)) + } + + // Deduplicate + seen := map[string]struct{}{} + uniq := make([]string, 0, len(out)) + for _, k := range out { + if k == "" { + continue + } + if _, ok := seen[k]; !ok { + seen[k] = struct{}{} + uniq = append(uniq, k) + } + } + return uniq } // collectUsageStats collects usage-related metrics for a mountpoint. @@ -161,9 +210,9 @@ func collectUsageStats(mountpoint string) (*disk.UsageStat, *CustomErr) { } // CollectDiskMetrics collects disk metrics following the disk metric flow specification. -// List all partitions on the system using disk.Partitions(all=true) -// Check each partition for filtering conditions -// For each valid partition, gather the specified metrics +// Lists all partitions on the system using disk.Partitions(all=true). +// Checks each partition for filtering conditions. +// For each valid partition, gathers the specified metrics. func CollectDiskMetrics() (MetricsSlice, []CustomErr) { defaultDiskData := []*DiskData{ { @@ -185,9 +234,9 @@ func CollectDiskMetrics() (MetricsSlice, []CustomErr) { var diskErrors []CustomErr var metricsSlice MetricsSlice - var checkedDevices = make(map[string]struct{}) // To keep track of checked partitions + var checkedDevices = make(map[string]struct{}) // Track already processed devices - // List all partitions on the system. Using disk.Partitions(all=true) + // List all partitions on the system partitions, partErr := disk.Partitions(true) if partErr != nil { diskErrors = append(diskErrors, CustomErr{ @@ -197,26 +246,26 @@ func CollectDiskMetrics() (MetricsSlice, []CustomErr) { return MetricsSlice{defaultDiskData[0]}, diskErrors } - // Check each partition for the filtering + // Iterate through partitions and apply filters for _, partition := range partitions { - // Check if the partition is already checked to avoid duplicates + // Skip duplicates if _, ok := checkedDevices[partition.Device]; ok { continue } - // Apply filtering logic based on the disk metric flow + // Apply filtering logic if !shouldIncludePartition(partition) { continue } - // For each valid partition, gather the specified metrics + // Gather metrics for valid partitions diskMetrics, err := collectPartitionMetrics(partition) if err.Error != "" { diskErrors = append(diskErrors, err) continue } - checkedDevices[partition.Device] = struct{}{} // Mark this partition as checked + checkedDevices[partition.Device] = struct{}{} // Mark as checked metricsSlice = append(metricsSlice, diskMetrics) } From adfce7080dede068989cb68ddd8ad4bd26a4a758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Ramos=20Medina?= Date: Thu, 9 Oct 2025 13:40:06 +0100 Subject: [PATCH 2/2] Improve comments and device mapper resolution in disk.go Enhanced comments for clarity and added functionality to resolve device mapper paths. --- internal/metric/disk.go | 71 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 6 deletions(-) diff --git a/internal/metric/disk.go b/internal/metric/disk.go index f61c4c3..23c3b4c 100644 --- a/internal/metric/disk.go +++ b/internal/metric/disk.go @@ -1,6 +1,7 @@ package metric import ( + "os" "path/filepath" "runtime" "strings" @@ -120,8 +121,9 @@ func collectPartitionMetrics(partition disk.PartitionStat) (*DiskData, CustomErr } // collectIOStats gathers IO-related metrics for a device. -// Supports LVM/device-mapper by resolving /dev/mapper/* -> /dev/dm-* -// and trying multiple key candidates against the map returned by disk.IOCounters(). +// Supports LVM/device-mapper by resolving /dev/mapper/* -> /dev/dm-*, +// searching /sys/block for the matching dm-* device, and trying multiple +// key candidates against the map returned by disk.IOCounters(). func collectIOStats(device string) (*disk.IOCountersStat, *CustomErr) { // Get all counters once and look up by key all, err := disk.IOCounters() @@ -158,7 +160,11 @@ func collectIOStats(device string) (*disk.IOCountersStat, *CustomErr) { } // buildDeviceKeyCandidates returns possible keys for the disk.IOCounters() map. -// Handles paths like /dev/sda, /dev/nvme0n1, /dev/mapper/vg-lv -> dm-0, etc. +// Handles paths like /dev/sda, /dev/nvme0n1, and /dev/mapper/vg-lv by: +// - stripping /dev/ +// - taking the basename +// - resolving symlinks when applicable +// - scanning /sys/block/dm-*/dm/name to find the matching dm-* device func buildDeviceKeyCandidates(device string) []string { if runtime.GOOS == "windows" { // On Windows, gopsutil uses names like "C:", so keep as-is. @@ -172,18 +178,28 @@ func buildDeviceKeyCandidates(device string) []string { // Strip /dev/ out = append(out, strings.TrimPrefix(d, "/dev/")) // Basename (e.g., /dev/mapper/vg-lv -> vg-lv) - out = append(out, filepath.Base(d)) + base := filepath.Base(d) + out = append(out, base) - // Resolve symlinks: /dev/mapper/vg-lv -> /dev/dm-0 -> dm-0 + // Resolve symlinks (works for typical udev/by-id/by-uuid, not for mapper pseudo-devices) if resolved, err := filepath.EvalSymlinks(d); err == nil && resolved != "" { out = append(out, strings.TrimPrefix(resolved, "/dev/")) out = append(out, filepath.Base(resolved)) } - // Deduplicate + // If it's an LVM/device-mapper path, try to discover dm-* via /sys/block + if strings.HasPrefix(d, "/dev/mapper/") || strings.HasPrefix(base, "dm-") { + if dm := findDMForMapperBase(base); dm != "" { + // Add dm-* key (this is what gopsutil uses in IOCounters) + out = append(out, dm) + } + } + + // Deduplicate and drop empties seen := map[string]struct{}{} uniq := make([]string, 0, len(out)) for _, k := range out { + k = strings.TrimSpace(k) if k == "" { continue } @@ -195,6 +211,49 @@ func buildDeviceKeyCandidates(device string) []string { return uniq } +// findDMForMapperBase tries to map a /dev/mapper/ basename to its dm-* +// by scanning /sys/block/dm-*/dm/name and comparing values. +// +// For LVM, the mapper basename typically matches the content of /sys/block/dm-*/dm/name. +// Example: +// /dev/mapper/ubuntu--vg-ubuntu--lv -> /sys/block/dm-0/dm/name == "ubuntu--vg-ubuntu--lv" => dm-0 +func findDMForMapperBase(mapperBase string) string { + const sysBlock = "/sys/block" + entries, err := os.ReadDir(sysBlock) + if err != nil { + return "" + } + + for _, e := range entries { + name := e.Name() + if !strings.HasPrefix(name, "dm-") { + continue + } + // Read /sys/block/dm-*/dm/name to get the logical name + dmNamePath := filepath.Join(sysBlock, name, "dm", "name") + b, err := os.ReadFile(dmNamePath) + if err != nil { + continue + } + dmLogical := strings.TrimSpace(string(b)) + + // Compare mapper basename to dm logical name. + // LVM encodes '-' as '--' in names; mapperBase already carries that encoding, + // and /sys/block/.../dm/name typically matches the same encoding. + if dmLogical == mapperBase { + return name + } + + // Extra tolerance: also try a relaxed comparison removing all slashes + // and comparing lowercased (helps in edge cases with udev rules). + if strings.EqualFold(strings.ReplaceAll(dmLogical, "/", ""), strings.ReplaceAll(mapperBase, "/", "")) { + return name + } + } + + return "" +} + // collectUsageStats collects usage-related metrics for a mountpoint. func collectUsageStats(mountpoint string) (*disk.UsageStat, *CustomErr) { diskUsage, diskUsageErr := disk.Usage(mountpoint)