fix: issue #131, disk metric not shown when using LVM

[adosaiguas]

Fixes issue #131

If an LVM device is detected (/dev/mapper), we handle this other use case.
Added some unit tests.

[coderabbitai]

Walkthrough

Enhanced disk metrics collection in the internal/metric package with improved error aggregation for partial data collection, device-mapper device resolution via sysfs lookup, and test-friendly sysfsRoot override mechanism. Added comprehensive tests for device mapper name resolution and key candidate generation. No exported API changes.

Changes

Cohort / File(s)	Change Summary
Disk Metrics Enhancement internal/metric/disk.go	Introduced sysfsRoot override for testing, expanded error handling to support partial IO/usage metrics collection without aborting on individual errors. Added mergeDiskErrors helper to consolidate multiple errors with unique metrics and concatenated messages. Enhanced device key resolution via resolveDMNameFromMapperWithRoot to discover device-mapper mappings from sysfs. Aggregates available metrics into DiskData object with improved error merging logic. Added os and sort imports.
Disk Metrics Tests internal/metric/disk_test.go	New Linux-only test file with two tests: TestResolveDMNameFromMapperWithRoot validates sysfs-based DM name resolution returns "dm-0" for /dev/mapper names; TestBuildDeviceKeyCandidates_AddsDMForMapper verifies device key candidates include dm- mappings when sysfsRoot is configured. Both tests manipulate temporary sysfs directory structures and validate DM device discovery.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Error aggregation logic: mergeDiskErrors consolidates multiple CustomErr values while deduplicating metrics—requires careful review of merge semantics and error handling correctness
• Device mapper resolution: resolveDMNameFromMapperWithRoot sysfs traversal and path construction needs validation for edge cases and symlink handling
• Test infrastructure: Verify sysfsRoot override doesn't introduce test pollution or interfere with concurrent test execution; temporary directory cleanup handling

Poem

🐰 Hop hop, the disk stats now flow so true,
Partial metrics collected, errors merged anew,
Device mappers discovered in sysfs's deep burrow,
No longer shall errors cause metrics to sorrow! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically identifies the fix for issue #131 regarding disk metrics not showing with LVM, directly matching the changeset's primary purpose.
Description check	✅ Passed	The description accurately relates to the changeset, referencing the specific issue being fixed, the LVM device handling approach, and mentions the addition of unit tests.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb8b06b and 615c3fe.

📒 Files selected for processing (2)

• internal/metric/disk.go (5 hunks)
• internal/metric/disk_test.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

internal/metric/disk.go (2)

internal/metric/metric.go (3)

• CustomErr (40-43)
• Metric (7-9)
• DiskData (66-80)

internal/metric/metric_math.go (1)

• RoundFloatPtr (11-14)

🔇 Additional comments (10)

internal/metric/disk_test.go (2)

11-34: LGTM! Good test coverage for DM name resolution.

The test properly simulates sysfs structure and validates the core LVM support functionality. The test creates a realistic directory hierarchy and verifies correct resolution from mapper name to dm device.

---

41-45: No changes needed. Tests in the same package run sequentially by default, and this test doesn't call t.Parallel(). The t.Cleanup() approach correctly preserves and restores the package-level sysfsRoot variable, which is safe for sequential execution.

internal/metric/disk.go (8)

4-4: LGTM! Import additions align with new functionality.

The os and sort imports are needed for the new sysfs file reading and error aggregation features.

Also applies to: 7-7

---

13-14: LGTM! Good testability pattern.

The sysfsRoot variable allows tests to inject temporary sysfs structures while maintaining the standard /sys path in production.

---

94-141: Excellent improvement! Partial data collection enhances resilience.

The refactored collectPartitionMetrics now accumulates errors and returns partial results when available, rather than failing completely if one metric is unavailable. This is especially valuable for LVM devices where IO or usage stats might not always be accessible through the same paths.

The progressive construction of DiskData (lines 117-134) ensures that users see whatever metrics are available, improving the overall user experience.

---

143-166: LGTM! Well-implemented error aggregation.

The mergeDiskErrors function properly deduplicates metrics, filters empty messages, and ensures deterministic output through sorting. This provides clean, consolidated error reporting when multiple metrics fail.

---

229-234: Core LVM fix! Device-mapper resolution via sysfs.

This enhancement resolves /dev/mapper/* devices to their corresponding dm-N identifiers by querying sysfs, which is essential for matching LVM devices against the keys returned by disk.IOCounters(). This directly addresses issue #131.

---

251-277: LGTM! Robust sysfs-based device mapper resolution.

The implementation correctly:

• Validates input (line 252)
• Uses glob patterns to find all dm devices (line 256)
• Handles read errors gracefully by continuing (line 265)
• Uses TrimSpace to normalize names from sysfs files (line 267)
• Extracts the dm-N identifier through proper path manipulation (lines 272-273)

The function is resilient to missing devices and read errors, making it suitable for production use.

---

338-341: Good refactor: Using empty struct for set membership.

Using struct{}{} instead of true for map-based sets is idiomatic Go and more memory-efficient, as empty structs consume zero bytes.

---

348-350: LGTM! Defensive programming prevents nil metrics.

The nil check ensures that only valid DiskData entries are appended to the metrics slice, which aligns with the refactored collectPartitionMetrics that may return nil when no stats are available (lines 109-114).

[adosaiguas]

@coderabbitai generate docstrings

[coderabbitai]

✅ Actions performed

Initiated docstring generation; will generate only if new commits exist.

[coderabbitai]

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #144