fix(disk): Enhance partition filtering to include ZFS filesystems #64
Conversation
#55 Signed-off-by: Mert Şişmanoğlu <[email protected]>
WalkthroughThe disk partition filtering logic in the metric collection function was updated to include ZFS filesystems. The new logic continues to exclude duplicate and loop devices but now allows partitions with a filesystem type of "zfs" even if their device names do not start with "/dev". Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/metric/disk.go (1)
43-48: Well implemented ZFS filesystem inclusion logic.The updated filtering logic correctly addresses the PR objective by including ZFS filesystems in the disk metrics collection. The solution maintains the existing filtering criteria while intelligently identifying ZFS partitions that wouldn't have been captured previously.
The added comments clearly explain the rationale behind the changes, making the code more maintainable.
While the implementation is correct, consider restructuring the conditional logic for better readability:
- if slices.Contains(checkedSlice, p.Device) || - (strings.Contains(p.Device, "/dev/loop") || (!strings.HasPrefix(p.Device, "/dev") && p.Fstype != "zfs")) { - continue - } + // Skip if already processed + if slices.Contains(checkedSlice, p.Device) { + continue + } + + // Skip loop devices + if strings.Contains(p.Device, "/dev/loop") { + continue + } + + // Skip non-/dev devices unless they are ZFS filesystems + if !strings.HasPrefix(p.Device, "/dev") && p.Fstype != "zfs" { + continue + }This approach makes each condition clearer and easier to maintain, while maintaining the exact same behavior.
Issue: #55
Updated the filtering logic to include ZFS filesystems, which are not prefixed with
/dev, while continuing to exclude/dev/loopdevices. This improves support for ZFS-based systems and refines the partition selection process.How to test
shell:bash
Your
df -houtput will be like thatRun the capture and go to
http://localhost:59232/api/v1/metrics/diskendpoint. It will show themypoolandmypool/myfsIf everything is ok, you can delete the pool and loop device
Summary by CodeRabbit