Skip to content

Add EBS VolumeId to disk metrics - #44

Open
crohr wants to merge 1 commit into
mainfrom
auto/add-ebs-volume-id-metrics
Open

Add EBS VolumeId to disk metrics#44
crohr wants to merge 1 commit into
mainfrom
auto/add-ebs-volume-id-metrics

Conversation

@crohr

@crohr crohr commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • resolve the EBS VolumeId backing the configured Nitro NVMe disk
  • publish it as a disk-only CloudWatch dimension and expose it as the volume_id action output
  • reuse the saved value in post-step metric queries and summaries
  • verify the exact VolumeId-qualified metric across the x64/arm64 metrics matrix
  • rebuild the distributed action binaries

Why

Disk-capacity metrics currently identify an instance and mount path, but not the live EBS volume created from the snapshot. That forces alert responders to perform a separate instance-to-volume lookup. PR #28 attempted to add the query dimension without configuring CloudWatch Agent to publish the same metric identity; this change implements both sides and preserves the existing fallback when a volume cannot be resolved.

Validation

  • go test ./...
  • YAML parsing for action.yml and the metrics workflow
  • bash -n on the workflow verification script
  • git diff --check
  • upx -t main-linux-amd64 main-linux-arm64 main-windows-amd64.exe

The metrics workflow now polls CloudWatch using instance-profile credentials and fails unless disk_used_percent returns data for the exact InstanceId, VolumeId, fstype, and path dimensions.

go vet ./... continues to report the pre-existing unreachable code in enableDetailedMonitoring; this PR does not change that behavior.

@crohr
crohr marked this pull request as ready for review July 10, 2026 14:04
@crohr

crohr commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Pushback review pass — this PR is in good shape. Focused diff, the sysfs serial resolution is properly restricted before joining user input to a privileged path (filepath.Base check + NVMe device pattern), it degrades gracefully to the previous behavior when no volume ID resolves, and the workflow verify step pins the actual published VolumeId dimension rather than restating the implementation. No further review rounds from me.

Two non-blocking notes:

  • No CI checks have run on b8394d8. Re-push or rebase to get a green run before merging — the metrics-matrix verify step is the real test of this feature.
  • This appears to supersede fix: Fixing error on volumeID #28 and Add volume id to dimensions #23 (same VolumeId-dimension goal). Close those when this merges so the feature doesn't land twice.

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