diff --git a/crates/health/src/api_client.rs b/crates/health/src/api_client.rs index 8c19789dd1..de2626c575 100644 --- a/crates/health/src/api_client.rs +++ b/crates/health/src/api_client.rs @@ -601,19 +601,12 @@ impl ApiEndpointSource { )); }; let addr = BmcAddr::try_from(bmc_info)?; - let serial = power_shelf - .config - .as_ref() - .map(|config| config.name.clone()) - .ok_or(HealthError::GenericError( - "Power shelf endpoint does not have serial".to_string(), - ))?; self.endpoint_for( addr, Some(EndpointMetadata::PowerShelf(PowerShelfData { id: power_shelf.id, - serial, + serial: None, })), power_shelf.rack_id.clone(), ApiCredentialKind::Bmc, @@ -977,6 +970,10 @@ mod tests { })?; assert_eq!(endpoint.rack_id.as_ref(), Some(&rack_id)); + let Some(EndpointMetadata::PowerShelf(power_shelf)) = endpoint.metadata.as_ref() else { + panic!("expected power shelf metadata"); + }; + assert_eq!(power_shelf.serial, None); Ok(()) } diff --git a/crates/health/src/endpoint/model.rs b/crates/health/src/endpoint/model.rs index c641ab694b..c3ae4fd304 100644 --- a/crates/health/src/endpoint/model.rs +++ b/crates/health/src/endpoint/model.rs @@ -95,13 +95,25 @@ impl BmcEndpoint { ) } + /// Returns the endpoint identity used for collector log state. + /// + /// Machines prefer their NICo ID, switches use their serial number, and PowerShelves prefer + /// their serial number followed by their NICo ID. Other cases use the BMC MAC address. pub fn log_identity(&self) -> Cow<'_, str> { match &self.metadata { Some(EndpointMetadata::Machine(MachineData { machine_id: Some(id), .. })) => Cow::Owned(id.to_string()), - Some(EndpointMetadata::PowerShelf(power_shelf)) => Cow::Borrowed(&power_shelf.serial), + Some(EndpointMetadata::PowerShelf(power_shelf)) => { + if let Some(serial) = power_shelf.serial.as_deref() { + Cow::Borrowed(serial) + } else if let Some(id) = power_shelf.id { + Cow::Owned(id.to_string()) + } else { + Cow::Owned(self.addr.mac.to_string()) + } + } Some(EndpointMetadata::Switch(switch)) => Cow::Borrowed(&switch.serial), _ => Cow::Owned(self.addr.mac.to_string()), } @@ -159,10 +171,14 @@ impl EndpointMetadata { } } + /// Returns the hardware serial number when the endpoint metadata provides one. + /// + /// Machine and PowerShelf serial numbers may be absent; switch serial numbers are always + /// present. pub fn serial_number(&self) -> Option<&str> { match self { EndpointMetadata::Machine(machine) => machine.machine_serial.as_deref(), - EndpointMetadata::PowerShelf(power_shelf) => Some(power_shelf.serial.as_str()), + EndpointMetadata::PowerShelf(power_shelf) => power_shelf.serial.as_deref(), EndpointMetadata::Switch(switch) => Some(switch.serial.as_str()), } } @@ -214,7 +230,8 @@ pub struct MachineData { #[derive(Clone, Debug, PartialEq)] pub struct PowerShelfData { pub id: Option, - pub serial: String, + /// Hardware serial number, when explicitly known. + pub serial: Option, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -306,6 +323,7 @@ mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; use carbide_test_support::{Check, check_values}; + use carbide_uuid::power_shelf::PowerShelfId; use mac_address::MacAddress; use super::{ @@ -409,7 +427,7 @@ mod tests { scenario: "power shelf is not eligible", input: Some(EndpointMetadata::PowerShelf(PowerShelfData { id: None, - serial: "power-shelf".to_string(), + serial: None, })), expect: false, }, @@ -428,6 +446,37 @@ mod tests { ); } + #[test] + fn power_shelf_log_identity_falls_back_to_id_then_mac() { + let power_shelf_id = + PowerShelfId::from_str("ps100ht038bg3qsho433vkg684heguv282qaggmrsh2ugn1qk096n2c6hcg") + .expect("valid power shelf id"); + + check_values( + [ + Check { + scenario: "PowerShelf ID is available", + input: Some(power_shelf_id), + expect: power_shelf_id.to_string(), + }, + Check { + scenario: "PowerShelf ID is unavailable", + input: None, + expect: "00:11:22:33:44:55".to_string(), + }, + ], + |id| { + let mut endpoint = test_endpoint(mac("00:11:22:33:44:55")); + endpoint.metadata = Some(EndpointMetadata::PowerShelf(PowerShelfData { + id, + serial: None, + })); + + endpoint.log_identity().into_owned() + }, + ); + } + #[tokio::test] async fn shared_system_uuid_caches_absent_result_across_clones() { let state = SharedSystemUuid::default(); diff --git a/crates/health/src/endpoint/sources.rs b/crates/health/src/endpoint/sources.rs index d0e7da17b8..48a6989a16 100644 --- a/crates/health/src/endpoint/sources.rs +++ b/crates/health/src/endpoint/sources.rs @@ -122,11 +122,7 @@ impl StaticEndpointSource { None } }); - let serial = power_shelf - .serial - .clone() - .or_else(|| power_shelf.id.clone()) - .unwrap_or_else(|| cfg.mac.clone()); + let serial = power_shelf.serial.clone(); Some(EndpointMetadata::PowerShelf(PowerShelfData { id, serial })) } else if let Some(switch) = &cfg.switch { @@ -501,7 +497,39 @@ mod tests { match &endpoints[0].metadata { Some(EndpointMetadata::PowerShelf(power_shelf)) => { assert_eq!(power_shelf.id, Some(power_shelf_id)); - assert_eq!(power_shelf.serial, "PS-001"); + assert_eq!(power_shelf.serial.as_deref(), Some("PS-001")); + } + other => panic!("expected PowerShelf metadata, got {other:?}"), + } + } + + #[tokio::test] + async fn test_static_endpoint_without_power_shelf_serial_preserves_absence() { + let power_shelf_id = test_power_shelf_id("power-shelf-without-serial"); + let configs = vec![StaticBmcEndpoint { + ip: ip("10.0.2.2"), + port: Some(443), + mac: "22:33:44:55:66:88".to_string(), + username: "admin".to_string(), + password: Some("pass".to_string()), + machine: None, + power_shelf: Some(StaticPowerShelfEndpoint { + id: Some(power_shelf_id.to_string()), + serial: None, + }), + switch: None, + rack_id: None, + labels: Default::default(), + }]; + + let source = StaticEndpointSource::from_config(&configs, &reqwest(), None, 10, None); + let endpoints = source.fetch_bmc_hosts().await.unwrap(); + + assert_eq!(endpoints.len(), 1); + match &endpoints[0].metadata { + Some(EndpointMetadata::PowerShelf(power_shelf)) => { + assert_eq!(power_shelf.id, Some(power_shelf_id)); + assert_eq!(power_shelf.serial, None); } other => panic!("expected PowerShelf metadata, got {other:?}"), } diff --git a/crates/health/src/otlp/convert.rs b/crates/health/src/otlp/convert.rs index a0c87f7ff8..04d3d1f0c4 100644 --- a/crates/health/src/otlp/convert.rs +++ b/crates/health/src/otlp/convert.rs @@ -112,6 +112,17 @@ fn resource_attributes(context: &EventContext) -> Vec { if let Some(component_type) = context.component_type() { attrs.push(KeyValue::new("component.type", component_type.to_string())); } + if let Some(power_shelf_id) = context.power_shelf_id() { + attrs.push(KeyValue::new("power_shelf.id", power_shelf_id.to_string())); + } + if context.component_type() == Some("power_shelf") + && let Some(serial) = context.serial_number() + { + attrs.push(KeyValue::new( + "power_shelf.serial_number", + serial.to_string(), + )); + } if let Some(switch_id) = context.switch_id() { attrs.push(KeyValue::new("switch.id", switch_id.to_string())); } @@ -904,10 +915,11 @@ mod tests { } #[test] - fn resource_attributes_include_power_shelf_component_type() { + fn resource_attributes_include_power_shelf_identity() { let power_shelf_id = PowerShelfId::from_str("ps100ht038bg3qsho433vkg684heguv282qaggmrsh2ugn1qk096n2c6hcg") .expect("valid power shelf id"); + let power_shelf_id_string = power_shelf_id.to_string(); let context = EventContext { endpoint_key: "33:44:55:66:77:88".to_string(), addr: BmcAddr { @@ -919,7 +931,7 @@ mod tests { labels: Default::default(), metadata: Some(EndpointMetadata::PowerShelf(PowerShelfData { id: Some(power_shelf_id), - serial: "SN-PS-001".to_string(), + serial: Some("SN-PS-001".to_string()), })), rack_id: Some(RackId::new("RACK_4")), }; @@ -927,9 +939,45 @@ mod tests { let attrs = otlp_resource_attributes(&context); assert_eq!(attr_value(&attrs, "component.type"), Some("power_shelf")); + assert_eq!( + attr_value(&attrs, "power_shelf.id"), + Some(power_shelf_id_string.as_str()) + ); + assert_eq!( + attr_value(&attrs, "power_shelf.serial_number"), + Some("SN-PS-001") + ); assert_eq!(attr_value(&attrs, "rack.id"), Some("RACK_4")); } + #[test] + fn resource_attributes_omit_missing_power_shelf_serial() { + let context = EventContext { + endpoint_key: "33:44:55:66:77:88".to_string(), + addr: BmcAddr { + ip: IpAddr::V4(Ipv4Addr::new(10, 0, 3, 1)), + port: Some(443), + mac: MacAddress::from_str("33:44:55:66:77:88").expect("valid mac"), + }, + collector_type: "sensor_collector", + labels: Default::default(), + metadata: Some(EndpointMetadata::PowerShelf(PowerShelfData { + id: Some( + PowerShelfId::from_str( + "ps100ht038bg3qsho433vkg684heguv282qaggmrsh2ugn1qk096n2c6hcg", + ) + .expect("valid power shelf id"), + ), + serial: None, + })), + rack_id: Some(RackId::new("RACK_4")), + }; + + let attrs = otlp_resource_attributes(&context); + + assert_eq!(attr_value(&attrs, "power_shelf.serial_number"), None); + } + #[test] fn log_event_converts_to_otlp_record() { let ctx = test_context(); diff --git a/crates/health/src/sink/events.rs b/crates/health/src/sink/events.rs index 27f5e70e78..7f97e7fcbc 100644 --- a/crates/health/src/sink/events.rs +++ b/crates/health/src/sink/events.rs @@ -745,7 +745,7 @@ mod tests { })), ContextKind::PowerShelf => Some(EndpointMetadata::PowerShelf(PowerShelfData { id: Some(power_shelf_id()), - serial: "PS-001".to_string(), + serial: Some("PS-001".to_string()), })), }; diff --git a/docs/architecture/health_aggregation.md b/docs/architecture/health_aggregation.md index 55c36f2c39..3af1215f0c 100644 --- a/docs/architecture/health_aggregation.md +++ b/docs/architecture/health_aggregation.md @@ -310,7 +310,7 @@ The publishing sinks expose that inventory context using the conventions of the - `[sinks.prometheus]` adds _machine_ metadata as metric labels named `machine_id`, `system_uuid`, `serial_number`, `rack_id`, `machine_slot_number`, `machine_tray_index`, and `nvlink_domain_uuid`. _Switch_ metadata labels are `switch_id`, `serial_number`, `rack_id`, `switch_slot_number`, `switch_tray_index`, and `nvlink_domain_uuid`. _Power-shelf_ metadata labels are `serial_number` and `rack_id`. Static endpoint custom labels keep their configured names. - `[sinks.tracing]` adds `rack_id` to every collector-event log when the endpoint supplies one. Endpoint-source, collector diagnostic, lifecycle, cancellation, and failure logs use the same optional field when they have endpoint context. - `[sinks.log_file]` adds `rack_id` as a top-level JSONL string field when the endpoint supplies one. -- `[sinks.otlp]` adds the string resource attributes `collector.type` and either `bmc.endpoint` and `bmc.ip`, or `switch.endpoint` and `switch.ip` for host-side switch collection. Typed inventory adds the strings `component.type` and, when present, `rack.id`. _Machine_ metadata attributes are the strings `machine.id`, `system.uuid`, `machine.serial`, `driver.version`, and `nvlink.domain.uuid`, plus the integers `machine.slot_number` and `machine.tray_index`. _Switch_ metadata attributes are the strings `switch.id`, `switch.serial_number`, `switch.endpoint_role`, and `nvlink.domain.uuid`, the boolean `switch.is_primary`, and the integers `switch.slot_number` and `switch.tray_index`. Static endpoint custom labels are string resource attributes and keep their configured names. +- `[sinks.otlp]` adds the string resource attributes `collector.type` and either `bmc.endpoint` and `bmc.ip`, or `switch.endpoint` and `switch.ip` for host-side switch collection. Typed inventory adds the strings `component.type` and, when present, `rack.id`. _Machine_ metadata attributes are the strings `machine.id`, `system.uuid`, `machine.serial`, `driver.version`, and `nvlink.domain.uuid`, plus the integers `machine.slot_number` and `machine.tray_index`. _Switch_ metadata attributes are the strings `switch.id`, `switch.serial_number`, `switch.endpoint_role`, and `nvlink.domain.uuid`, the boolean `switch.is_primary`, and the integers `switch.slot_number` and `switch.tray_index`. _Power-shelf_ metadata attributes are the strings `power_shelf.id` and `power_shelf.serial_number`; each is omitted when the corresponding metadata is unavailable. Static endpoint custom labels are string resource attributes and keep their configured names. - `[sinks.health_report]`, `[sinks.rack_health_report]`, `[sinks.switch_health_report]`, and `[sinks.power_shelf_health_report]` use the same event context when submitting assessed health reports back to NICo API. The persisted `HealthReport` and `HealthProbeAlert` schemas remain the probe success/alert model described above. Collector runtime metrics and gNMI stream metrics include a `rack_id` label when the endpoint supplies one. The label is omitted when discovery does not supply a rack ID. Existing `collector_type` and `endpoint_key` label semantics remain unchanged.