diff --git a/agent/disk.go b/agent/disk.go index 13bb1b1ce..8574beff0 100644 --- a/agent/disk.go +++ b/agent/disk.go @@ -204,7 +204,10 @@ func isRootFallbackPartition(p disk.PartitionStat, rootMountPoint string) bool { // partition looks like the active root mount but still needs translating to an // I/O device key. func (d *diskDiscovery) addPartitionRootFs(device, mountpoint string) bool { - fs, match := findIoDevice(filepath.Base(device), d.ctx.diskIoCounters) + // device is passed through as-is: findIoDevice normalizes it, and + // filepath.Base would turn a Windows volume name such as "C:" into "\" + // on the way in (#2417). + fs, match := findIoDevice(device, d.ctx.diskIoCounters) if !match { return false } @@ -527,13 +530,45 @@ func filesystemMatchesPartitionSetting(filesystem string, p disk.PartitionStat) // normalizeDeviceName canonicalizes device strings for comparisons. func normalizeDeviceName(value string) string { - name := filepath.Base(strings.TrimSpace(value)) + name := strings.TrimSpace(value) + if volume, ok := windowsVolumeName(name); ok { + return volume + } + name = filepath.Base(name) if name == "." { return "" } return name } +// windowsVolumeName returns the canonical form of a bare Windows volume +// specifier, so that "C:", `C:\` and "C:/" all name the same drive. +// +// filepath.Base cannot do this. On Windows it treats "C:" as a volume name +// with no path element to take the base of and returns "\", so every drive +// letter normalizes to the same key. findIoDevice then returns whichever +// counter the map happened to yield first, which registers the root +// filesystem under a random drive (#2417). +func windowsVolumeName(value string) (string, bool) { + if len(value) < 2 || value[1] != ':' { + return "", false + } + if c := value[0]; !('a' <= c && c <= 'z' || 'A' <= c && c <= 'Z') { + return "", false + } + if len(value) == 2 { + return value, true + } + // Only separators may follow the specifier. "C:data" is a drive-relative + // path, not a volume. + for i := 2; i < len(value); i++ { + if value[i] != '\\' && value[i] != '/' { + return "", false + } + } + return value[:2], true +} + // Sets start values for disk I/O stats. func (a *Agent) initializeDiskIoStats(diskIoCounters map[string]disk.IOCountersStat) { a.fsNames = a.fsNames[:0] diff --git a/agent/disk_test.go b/agent/disk_test.go index 70fa2083b..59f423bfd 100644 --- a/agent/disk_test.go +++ b/agent/disk_test.go @@ -1057,3 +1057,70 @@ func TestIoTimeDelta(t *testing.T) { assert.Equal(t, uint64(0), ioTimeDelta(200, math.MaxUint32+1000)) } + +func TestNormalizeDeviceName(t *testing.T) { + // A Windows volume name is not a path element, so every spelling of the + // same drive has to normalize to the same key. filepath.Base cannot do + // this: on Windows it strips the "C:" specifier and returns "\", which + // collapses every drive letter onto one key (#2417). + for _, spelling := range []string{"C:", `C:\`, "C:/", `C:\\`} { + assert.Equal(t, "C:", normalizeDeviceName(spelling), "spelling %q", spelling) + } + // Case is left to the caller, as it already is for Linux device names. + assert.Equal(t, "d:", normalizeDeviceName("d:")) + assert.Equal(t, "c:", normalizeDeviceName(" c: ")) + + // Non-volume inputs keep using filepath.Base. + assert.Equal(t, "sda1", normalizeDeviceName("/dev/sda1")) + assert.Equal(t, "sda1", normalizeDeviceName("/dev/sda1/")) + assert.Equal(t, "nvme0n1p2", normalizeDeviceName(" /dev/nvme0n1p2 ")) + assert.Equal(t, "", normalizeDeviceName(".")) + assert.Equal(t, "", normalizeDeviceName(" ")) + + // A drive-relative path is a path, not a volume. + assert.Equal(t, `C:data`, normalizeDeviceName(`C:data`)) +} + +func TestFindIoDeviceWindowsVolumeNames(t *testing.T) { + // Every drive normalizes to a distinct key, so the root drive resolves + // exactly instead of to whichever counter the map yielded first (#2417). + ioCounters := map[string]disk.IOCountersStat{ + "C:": {Name: "C:", ReadBytes: 10, WriteBytes: 10}, + "D:": {Name: "D:", ReadBytes: 20, WriteBytes: 20}, + "P:": {Name: "P:", ReadBytes: 30, WriteBytes: 30}, + } + + for i := 0; i < 32; i++ { + device, ok := findIoDevice("C:", ioCounters) + assert.True(t, ok) + assert.Equal(t, "C:", device) + } + + // The drive may arrive with a trailing separator, as a mount point does. + device, ok := findIoDevice(`C:\`, ioCounters) + assert.True(t, ok) + assert.Equal(t, "C:", device) +} + +func TestAddPartitionRootFsWindowsDrive(t *testing.T) { + agent := &Agent{fsStats: make(map[string]*system.FsStats)} + discovery := diskDiscovery{ + agent: agent, + ctx: fsRegistrationContext{ + isWindows: true, + diskIoCounters: map[string]disk.IOCountersStat{ + "C:": {Name: "C:"}, + "D:": {Name: "D:"}, + "P:": {Name: "P:"}, + }, + }, + } + + ok := discovery.addPartitionRootFs("C:", `C:\`) + + assert.True(t, ok) + assert.Len(t, agent.fsStats, 1) + stats, exists := agent.fsStats["C:"] + assert.True(t, exists) + assert.True(t, stats.Root) +}