fix(agent): keep Windows volume names intact when matching I/O devices (#2419)

filepath.Base is platform-dependent for a bare volume name: on Windows it
strips the "C:" specifier and returns "\", so every drive letter
normalizes to the same key. findIoDevice's exact match then returns
whichever disk.IOCounters entry the map yielded first, which registers the
root filesystem under a random drive. With EXTRA_FILESYSTEMS=D:,P: the
root disk disappears from the hub, and without it the root can be
registered under another drive's I/O device.

Normalize a bare volume specifier before taking a path base, and stop
taking a path base of the device in addPartitionRootFs, where it mangled
the value before findIoDevice could see it. registerFilesystemStats
already avoided this by only taking a base when the agent is not on
Windows.

Fixes #2417

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]

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)
+}