feat(health): include alert details in OTLP health report records - #4500
feat(health): include alert details in OTLP health report records#4500ambermingxin wants to merge 1 commit into
Conversation
Signed-off-by: Amber Xue <ambermingxin@nvidia.com>
Summary by CodeRabbit
WalkthroughThe change adds per-target OTLP health alert-detail configuration. Enabled targets serialize up to 64 alert details into health-report records and report truncation. The export pipeline passes the setting from ChangesOTLP health alert details
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant OtlpTargetConfig
participant OtlpDrain
participant build_export_request
participant HealthReportConverter
OtlpTargetConfig->>OtlpDrain: include_alert_details
OtlpDrain->>build_export_request: pass alert-detail policy
build_export_request->>HealthReportConverter: convert health-report events
HealthReportConverter->>HealthReportConverter: serialize up to 64 alerts
HealthReportConverter-->>build_export_request: OTLP record with alert attributes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/operations/monitoring-health.md (1)
274-284: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDocument the OTLP alert-detail fallback.
Probe::GpuInventorymaps to"SkuValidation", so retain that description. Add that serialization failure logs a warning, omits the alert attributes, and still emits the health report record.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/operations/monitoring-health.md` around lines 274 - 284, Update the OTLP alert-details documentation near the existing Probe::GpuInventory/SkuValidation description to state that serialization failures log a warning, omit the alert attributes, and still emit the health report record.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/operations/monitoring-health.md`:
- Around line 274-284: Update the OTLP alert-details documentation near the
existing Probe::GpuInventory/SkuValidation description to state that
serialization failures log a warning, omit the alert attributes, and still emit
the health report record.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d839b230-b3a9-432a-874c-c7fbcb425228
📒 Files selected for processing (7)
crates/health/example/config.example.tomlcrates/health/src/config.rscrates/health/src/otlp/convert.rscrates/health/src/otlp/drain.rscrates/health/src/otlp/mod.rscrates/health/src/sink/otlp.rsdocs/operations/monitoring-health.md
Health reports exported over OTLP today carry only counts. An operator looking
at a degraded machine sees
health report: 2 alerts, 35 ok (source: BmcSensors)and has no way to tell which component alerted or why, because
convert_eventusesreport.alertsonly for.len(). The detail exists onCollectorEvent::HealthReportand theTracingSinkalready logs it, but onlyto stdout, so it is unreachable for any OTLP consumer.
This adds an opt-in, per-target
include_alert_detailsflag. When enabled, ahealth report record with alerts gains a
health_report.alertsattributeholding a JSON array of the individual alerts (
probe_id,message,classifications, andtargetwhen the alert names one).include_diagnosticsdoes not help here: it is folded in at enqueue time and applies only to
CollectorEvent::Logrecords.The flag is per target rather than sink-global because
OtlpSink::new_manybuilds one sink and drain per configured target, so a debugging destination can
receive detail while a long-term store keeps receiving only counts.
Probe and classification identities use the existing stable wire names rather
than new serde derives, so OOB GPU inventory alerts appear as
SkuValidation,matching how they already dedup against the machine-controller's in-band SKU
alerts.
Backward compatibility
The body string, severity mapping, and
event.typeattribute are unchanged, andthe new attribute is purely additive. With the flag off the emitted record is
byte-identical to today's;
health_report_omits_alert_details_when_disabledguards this by asserting the record carries exactly one attribute.
Bounding the attribute size
At most 64 alerts are serialized per record. A truncated record also carries
health_report.alerts.droppedwith the number omitted.The cap is not cosmetic. Measured against a mock reporting every sensor as
failed, 64 alerts serialize to ~15.5 KB, or roughly 242 bytes per alert. On
hardware where the BmcSensors probe covers 256 sensors, an uncapped attribute
would be ~62 KB per record, and at the default
batch_size = 512a singleexport could approach 30 MB against a collector defaulting to a 4 MB receive
limit.
ResourceExhaustedis in the drain's retryable set, so an oversizedbatch would be retried five times and then dropped in full, taking unrelated
records with it.
Reviewer input welcome on whether 64 is the right limit and whether it should be
configurable rather than a constant.
Related issues
None.
Type of Change
Breaking Changes
Testing
Unit tests cover the flag off, the flag on with multiple alerts parsed field by
field, a report with no alerts, an alert with no target (key omitted rather than
null), and truncation past the cap.Manual testing ran the service against real hardware exporting to an
otel/opentelemetry-collector, on a machine reporting an actual leak condition.All three probes produced detail correctly:
The
TrayLeakDetectionreport exercised the absent-targetpath, and theBmcSensorsreport carried the underlying detector voltages against theirthresholds. Re-running the same fault with the flag off produced records with
only
event.type.Additional Notes
Serialization failure logs a warning and skips the attribute rather than
unwrapping, so a malformed report costs the detail and not the record.