Skip to content

feat(scout): time storage device cleanup#3958

Open
chet wants to merge 1 commit into
NVIDIA:mainfrom
chet:gh-issue-3908
Open

feat(scout): time storage device cleanup#3958
chet wants to merge 1 commit into
NVIDIA:mainfrom
chet:gh-issue-3908

Conversation

@chet

@chet chet commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Scout now reports per-device cleanup latency and result through one terminal Event while retaining the existing NVMe and HDD/SAS diagnostics.

  • Replaces the terminal cleanup records with ScoutStorageDeviceCleanup while preserving their INFO/ERROR levels and messages.
  • Records carbide_scout_storage_device_cleanup_duration_seconds by bounded device_type and outcome labels.
  • Covers all four device/result combinations with table-driven log and histogram assertions.

Related issues

This supports #3908

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated

  • Integration tests added/updated

  • Manual testing performed

  • No testing required (docs, internal refactor, etc.)

  • cargo test -p carbide-scout metrics::tests --bin forge-scout

  • cargo make format-nightly

  • cargo make clippy

  • cargo make carbide-lints

  • cargo make check-event-names

  • cargo xtask check-metric-docs

Additional Notes

Device paths remain on the existing cleanup spans so the histogram has only the four bounded device_type/outcome series.

Scout now reports per-device cleanup latency and result through one terminal Event while retaining the existing NVMe and HDD/SAS diagnostics.

- Replaces the terminal cleanup records with `ScoutStorageDeviceCleanup` while preserving their `INFO`/`ERROR` levels and messages.
- Records `carbide_scout_storage_device_cleanup_duration_seconds` by bounded `device_type` and `outcome` labels.
- Covers all four device/result combinations with table-driven log and histogram assertions.

Tests added!

This supports NVIDIA#3908

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
@chet
chet requested a review from a team as a code owner July 23, 2026 06:57
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 836cf4b2-3c87-4368-b664-45da3786e032

📥 Commits

Reviewing files that changed from the base of the PR and between 2de970b and 2a1e895.

📒 Files selected for processing (3)
  • crates/scout/src/deprovision/scrabbing.rs
  • crates/scout/src/metrics.rs
  • docs/observability/core_metrics.md

Summary by CodeRabbit

  • New Features

    • Added per-device storage cleanup observability for both NVMe and HDD/SAS, including device type, outcome, cleanup duration, and error details.
    • Cleanup now emits clearer, dynamically-leveled success vs. failure logs alongside the duration metric.
  • Documentation

    • Documented the new carbide_scout_storage_device_cleanup_duration_seconds histogram metric with device type and outcome breakdown.
  • Tests

    • Added coverage to verify emitted log fields and that histogram counts/sums match recorded cleanup durations for success and failure cases.

Walkthrough

Storage cleanup now emits structured per-device metrics and dynamic success/failure logs for NVMe and HDD/SAS operations. Tests validate labels, error context, duration observations, and log fields; observability documentation lists the new histogram metric.

Changes

Storage cleanup observability

Layer / File(s) Summary
Cleanup metric event
crates/scout/src/metrics.rs
Adds device-type and outcome labels, duration histogram observations, dynamic log levels/messages, and error context for cleanup results.
Cleanup task instrumentation
crates/scout/src/deprovision/scrabbing.rs
Emits the cleanup event for NVMe and HDD/SAS devices while retaining failure aggregation.
Observability validation and documentation
crates/scout/src/metrics.rs, docs/observability/core_metrics.md
Tests emitted fields and histogram deltas and documents carbide_scout_storage_device_cleanup_duration_seconds.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Sequence Diagram(s)

sequenceDiagram
  participant CleanupTask
  participant ScoutStorageDeviceCleanup
  participant CarbideInstrument
  CleanupTask->>ScoutStorageDeviceCleanup: construct device type, duration, and result
  ScoutStorageDeviceCleanup->>CarbideInstrument: emit structured log and histogram observation
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: timing storage device cleanup in Scout.
Description check ✅ Passed The description matches the changeset and accurately describes the new event, metric, and tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@chet

chet commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full_review, thanks!

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

@chet: I’ll perform a complete re-review of the current changes, including the event semantics, metric-label cardinality, error paths, and test coverage.

✅ Action performed

Full review finished.

@github-actions

Copy link
Copy Markdown

@chet
chet enabled auto-merge (squash) July 23, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant