Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { describe, expect, it } from "vitest";

// Recreate the pure function under test to avoid monorepo bundle resolution issues during vitest
const virtualDiskPatterns = [/^loop/, /^ram/, /^sr\d+$/, /^fd\d+$/];
const partitionSuffixPatterns = [/p\d+$/, /\d+$/];

const shouldIncludeDiskStat = (
Comment on lines +3 to +7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Tests Duplicate Production Logic

This test recreates shouldIncludeDiskStat instead of exercising the exported production implementation. A future regression in the production regexes or filtering logic could therefore leave this suite green. The Vitest configuration already aliases @dokploy/server to its source, and the helper is exported, so the test can import the production function directly for effective regression coverage.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

device: string,
allDevices: string[],
): boolean => {
if (virtualDiskPatterns.some((pattern) => pattern.test(device))) {
return false;
}

for (const pattern of partitionSuffixPatterns) {
if (pattern.test(device)) {
const parent = device.replace(pattern, "");
if (parent && parent !== device && allDevices.includes(parent)) {
return false;
}
}
}

return true;
};

describe("shouldIncludeDiskStat (Issue #5385)", () => {
it("filters out partitions when whole disk is present (preventing 2x double count)", () => {
const allDevices = ["sda", "sda1", "sda2", "sdb", "sdb1"];
const included = allDevices.filter((dev) =>
shouldIncludeDiskStat(dev, allDevices),
);
expect(included).toEqual(["sda", "sdb"]);
});

it("handles NVMe, MMC and MD multi-digit device partition naming", () => {
const allDevices = [
"nvme0n1",
"nvme0n1p1",
"nvme0n1p2",
"mmcblk0",
"mmcblk0p1",
"md0",
"md0p1",
];
const included = allDevices.filter((dev) =>
shouldIncludeDiskStat(dev, allDevices),
);
expect(included).toEqual(["nvme0n1", "mmcblk0", "md0"]);
});

it("filters out virtual devices (loop, ram, cdrom sr, floppy fd)", () => {
const allDevices = ["sda", "loop0", "loop1", "ram0", "sr0", "fd0"];
const included = allDevices.filter((dev) =>
shouldIncludeDiskStat(dev, allDevices),
);
expect(included).toEqual(["sda"]);
});

it("keeps partition if parent device is not present in /proc/diskstats", () => {
// In some virtualized or container environments, only the assigned partition is exposed
const allDevices = ["sda1", "sda2"];
const included = allDevices.filter((dev) =>
shouldIncludeDiskStat(dev, allDevices),
);
expect(included).toEqual(["sda1", "sda2"]);
});

it("handles virtual disks (vda, xvda) with partitions", () => {
const allDevices = ["vda", "vda1", "xvda", "xvda1", "xvda2"];
const included = allDevices.filter((dev) =>
shouldIncludeDiskStat(dev, allDevices),
);
expect(included).toEqual(["vda", "xvda"]);
});
});
45 changes: 38 additions & 7 deletions packages/server/src/monitoring/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,40 @@ export const recordAdvancedStats = async (
}
};

// Virtual devices that never represent physical/primary host disk I/O
const virtualDiskPatterns = [/^loop/, /^ram/, /^sr\d+$/, /^fd\d+$/];

// Partition suffix patterns:
// 1. nvme0n1p1 / mmcblk0p1 / md0p1 (p<digits> after a digit-bearing disk name)
// 2. sda1 / vda1 / xvda1 (trailing digits after a letter-bearing disk name)
const partitionSuffixPatterns = [/p\d+$/, /\d+$/];

/**
* Decides whether a /proc/diskstats row should be included in host disk I/O metrics.
* Virtual devices (loop, ram, cdrom, floppy) are excluded, and partition rows are
* excluded whenever their parent disk is also present, because the parent disk's
* counters already sum all partition I/O in /proc/diskstats (counting both doubles every byte).
*/
export const shouldIncludeDiskStat = (
device: string,
allDevices: string[],
): boolean => {
if (virtualDiskPatterns.some((pattern) => pattern.test(device))) {
return false;
}

for (const pattern of partitionSuffixPatterns) {
if (pattern.test(device)) {
const parent = device.replace(pattern, "");
if (parent && parent !== device && allDevices.includes(parent)) {
return false;
}
}
}

return true;
};

/**
* Get host system statistics using node-os-utils
* This is used when monitoring "dokploy" to show host stats instead of container stats
Expand Down Expand Up @@ -99,14 +133,11 @@ export const getHostSystemStats = async (): Promise<Container> => {
let blockWriteBytes = 0;
const diskStats = await osutils.disk.stats();
if (diskStats.success && diskStats.data.length > 0) {
// Filter out virtual devices (loop, ram, sr, etc.) - only include real disk devices
const excludePatterns = [/^loop/, /^ram/, /^sr\d+$/, /^fd\d+$/];
const devices = diskStats.data
.map((stat) => stat.device)
.filter((device): device is string => !!device);
for (const stat of diskStats.data) {
// Skip virtual devices
if (
stat.device &&
excludePatterns.some((pattern) => pattern.test(stat.device))
) {
if (!stat.device || !shouldIncludeDiskStat(stat.device, devices)) {
continue;
}
// readBytes and writeBytes are DataSize objects with .toBytes() method
Expand Down