diff --git a/nexus/src/app/background/tasks/ereport_ingester.rs b/nexus/src/app/background/tasks/ereport_ingester.rs index 3c5bac631b7..f2f03ec4d9f 100644 --- a/nexus/src/app/background/tasks/ereport_ingester.rs +++ b/nexus/src/app/background/tasks/ereport_ingester.rs @@ -12,7 +12,6 @@ use crate::app::background::BackgroundTask; use chrono::Utc; use ereport_types::Ena; -use ereport_types::Ereport; use ereport_types::EreportId; use futures::future::BoxFuture; use internal_dns_types::names::ServiceName; @@ -245,76 +244,19 @@ impl Ingester { } else { status.get_or_insert_default().requests += 1; } - + let time_collected = Utc::now(); let received = reports.items.len(); let status = status.get_or_insert_default(); status.ereports_received += received; let db_ereports = reports.items.into_iter().map(|ereport| { - const MISSING_VPD: &str = - " (perhaps the SP doesn't know its own VPD?)"; - let part_number = get_sp_metadata_string( - "baseboard_part_number", - &ereport, - &restart_id, + EreportData::from_sp_ereport( &opctx.log, - MISSING_VPD, - ); - let serial_number = get_sp_metadata_string( - "baseboard_serial_number", - &ereport, - &restart_id, - &opctx.log, - MISSING_VPD, - ); - let ena = ereport.ena; - let class = ereport - .data - // "k" (for "kind") is used as an abbreviation of - // "class" to save 4 bytes of ereport. - .get("k") - .or_else(|| ereport.data.get("class")); - let class = match (class, ereport.data.get("lost")) { - (Some(serde_json::Value::String(class)), _) => { - Some(class.to_string()) - } - (Some(v), _) => { - slog::warn!( - &opctx.log, - "malformed ereport: value for 'k'/'class' \ - should be a string, but found: {v:?}"; - "ena" => ?ena, - "restart_id" => ?restart_id, - ); - None - } - // This is a loss record! I know this! - (None, Some(serde_json::Value::Null)) => { - Some("ereport.data_loss.possible".to_string()) - } - (None, Some(serde_json::Value::Number(_))) => { - Some("ereport.data_loss.certain".to_string()) - } - (None, _) => { - slog::warn!( - &opctx.log, - "ereport missing 'k'/'class' key"; - "ena" => ?ena, - "restart_id" => ?restart_id, - ); - None - } - }; - - EreportData { - id: EreportId { restart_id, ena }, - time_collected: Utc::now(), - collector_id: self.nexus_id, - part_number, - serial_number, - class, - report: serde_json::Value::Object(ereport.data), - } + restart_id, + ereport, + time_collected, + self.nexus_id, + ) }); let created = match self .datastore @@ -427,41 +369,6 @@ impl Ingester { } } -/// Attempt to extract a VPD metadata from an SP ereport, logging a warning if -/// it's missing. We still want to keep such ereports, as the error condition -/// could be that the SP couldn't determine the metadata field, but it's -/// uncomfortable, so we ought to complain about it. -fn get_sp_metadata_string( - key: &str, - ereport: &Ereport, - restart_id: &EreporterRestartUuid, - log: &slog::Logger, - extra_context: &'static str, -) -> Option { - match ereport.data.get(key) { - Some(serde_json::Value::String(s)) => Some(s.clone()), - Some(v) => { - slog::warn!( - &log, - "malformed ereport: value for '{key}' should be a string, \ - but found: {v:?}"; - "ena" => ?ereport.ena, - "restart_id" => ?restart_id, - ); - None - } - None => { - slog::warn!( - &log, - "ereport missing '{key}'{extra_context}"; - "ena" => ?ereport.ena, - "restart_id" => ?restart_id, - ); - None - } - } -} - struct EreportQueryParams { committed_ena: Option, start_ena: Option, diff --git a/nexus/types/src/fm/ereport.rs b/nexus/types/src/fm/ereport.rs index 06012927bb6..dd840550c3d 100644 --- a/nexus/types/src/fm/ereport.rs +++ b/nexus/types/src/fm/ereport.rs @@ -6,6 +6,7 @@ use crate::inventory::SpType; use chrono::{DateTime, Utc}; +use omicron_uuid_kinds::EreporterRestartUuid; use omicron_uuid_kinds::OmicronZoneUuid; use omicron_uuid_kinds::SledUuid; use serde::Deserialize; @@ -35,6 +36,91 @@ pub struct EreportData { pub report: serde_json::Value, } +impl EreportData { + /// Interpret a service processor ereport from a raw JSON blobule, plus the + /// restart ID and collection metadata. + /// + /// This conversion is lossy; if some information is not present in the raw + /// ereport JSON, such as the SP's VPD identity, we log a warning, rather + /// than returning an error, and leave those fields empty in the returned + /// `EreportData`. This is because, if we receive an ereport that is + /// incomplete, we would still like to preserve whatever information *is* + /// present, rather than throwing the whole thing away. Thus, this function + /// also takes a `slog::Logger` for logging warnings if some expected fields + /// are not present or malformed. + pub fn from_sp_ereport( + log: &slog::Logger, + restart_id: EreporterRestartUuid, + ereport: ereport_types::Ereport, + time_collected: DateTime, + collector_id: OmicronZoneUuid, + ) -> Self { + const MISSING_VPD: &str = " (perhaps the SP doesn't know its own VPD?)"; + let part_number = get_sp_metadata_string( + "baseboard_part_number", + &ereport, + &restart_id, + &log, + MISSING_VPD, + ); + let serial_number = get_sp_metadata_string( + "baseboard_serial_number", + &ereport, + &restart_id, + &log, + MISSING_VPD, + ); + let ena = ereport.ena; + let class = ereport + .data + // "k" (for "kind") is used as an abbreviation of + // "class" to save 4 bytes of ereport. + .get("k") + .or_else(|| ereport.data.get("class")); + let class = match (class, ereport.data.get("lost")) { + (Some(serde_json::Value::String(class)), _) => { + Some(class.to_string()) + } + (Some(v), _) => { + slog::warn!( + &log, + "malformed ereport: value for 'k'/'class' \ + should be a string, but found: {v:?}"; + "ena" => ?ena, + "restart_id" => ?restart_id, + ); + None + } + // This is a loss record! I know this! + (None, Some(serde_json::Value::Null)) => { + Some("ereport.data_loss.possible".to_string()) + } + (None, Some(serde_json::Value::Number(_))) => { + Some("ereport.data_loss.certain".to_string()) + } + (None, _) => { + slog::warn!( + &log, + "ereport missing 'k'/'class' key"; + "ena" => ?ena, + "restart_id" => ?restart_id, + ); + None + } + }; + + EreportData { + id: EreportId { restart_id, ena }, + time_collected, + collector_id, + part_number, + serial_number, + class, + report: serde_json::Value::Object(ereport.data), + } + } +} + /// Describes the source of an ereport. #[derive( Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize, @@ -63,3 +149,38 @@ impl fmt::Display for Reporter { } } } + +/// Attempt to extract a VPD metadata from an SP ereport, logging a warning if +/// it's missing. We still want to keep such ereports, as the error condition +/// could be that the SP couldn't determine the metadata field, but it's +/// uncomfortable, so we ought to complain about it. +fn get_sp_metadata_string( + key: &str, + ereport: &ereport_types::Ereport, + restart_id: &EreporterRestartUuid, + log: &slog::Logger, + extra_context: &'static str, +) -> Option { + match ereport.data.get(key) { + Some(serde_json::Value::String(s)) => Some(s.clone()), + Some(v) => { + slog::warn!( + &log, + "malformed ereport: value for '{key}' should be a string, \ + but found: {v:?}"; + "ena" => ?ereport.ena, + "restart_id" => ?restart_id, + ); + None + } + None => { + slog::warn!( + &log, + "ereport missing '{key}'{extra_context}"; + "ena" => ?ereport.ena, + "restart_id" => ?restart_id, + ); + None + } + } +}