diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index 75391282e76..0032bc9f428 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -702,7 +702,7 @@ task: "fm_analysis" /!\ analysis failed: FM analysis is not yet implemented fault management analysis inputs - ----- ---------- -------- ------ + -------------------------------- parent sitrep: inventory collection: ..................... no new ereports since the parent sitrep @@ -1378,7 +1378,7 @@ task: "fm_analysis" /!\ analysis failed: FM analysis is not yet implemented fault management analysis inputs - ----- ---------- -------- ------ + -------------------------------- parent sitrep: inventory collection: ..................... no new ereports since the parent sitrep diff --git a/nexus/fm/src/analysis_input.rs b/nexus/fm/src/analysis_input.rs index 5879c1400f3..7eb48f9b466 100644 --- a/nexus/fm/src/analysis_input.rs +++ b/nexus/fm/src/analysis_input.rs @@ -264,7 +264,9 @@ mod tests { let mut reporter = fm_test .reporters .reporter(Reporter::Sp { sp_type: SpType::Sled, slot: 0 }); - let ereport_in_open_case = + let ereport_in_open_case1 = + Arc::new(reporter.mk_ereport(now, Default::default())); + let ereport_in_open_case2 = Arc::new(reporter.mk_ereport(now, Default::default())); let ereport_in_closed_unmarked = Arc::new(reporter.mk_ereport(now, Default::default())); @@ -273,28 +275,30 @@ mod tests { let ereport_new = Arc::new(reporter.mk_ereport(now, Default::default())); - // Make a parent sitrep, with three cases: + // Make a parent sitrep, with four cases: // // 1. an open case - let open_case_id = CaseUuid::new_v4(); - // 2. a closed case with an unmarked ereport in it, + let open_case1_id = CaseUuid::new_v4(); + // 2. another open case + let open_case2_id = CaseUuid::new_v4(); + // 3. a closed case with an unmarked ereport in it, let closed_case_with_unmarked_id = CaseUuid::new_v4(); - // 3. a closed case with only marked ereports. + // 4. a closed case with only marked ereports. let closed_case_without_unmarked_id = CaseUuid::new_v4(); let parent_sitrep_id = SitrepUuid::new_v4(); let parent_sitrep = { - let open_case = { + let open_case1 = { let created_sitrep_id = parent_sitrep_id; fm::Case { - id: open_case_id, + id: open_case1_id, metadata: fm::case::Metadata { created_sitrep_id, closed_sitrep_id: None, de: DiagnosisEngineKind::PowerShelf, - comment: "open case".to_string(), + comment: "open case one".to_string(), }, ereports: [ - case_ereport(&ereport_in_open_case, parent_sitrep_id), + case_ereport(&ereport_in_open_case1, parent_sitrep_id), case_ereport( &ereport_in_closed_unmarked, created_sitrep_id, @@ -306,6 +310,26 @@ mod tests { support_bundles_requested: Default::default(), } }; + let open_case2 = { + let created_sitrep_id = parent_sitrep_id; + fm::Case { + id: open_case2_id, + metadata: fm::case::Metadata { + created_sitrep_id, + closed_sitrep_id: None, + de: DiagnosisEngineKind::PowerShelf, + comment: "open case two".to_string(), + }, + ereports: [case_ereport( + &ereport_in_open_case2, + parent_sitrep_id, + )] + .into_iter() + .collect(), + alerts_requested: Default::default(), + support_bundles_requested: Default::default(), + } + }; let closed_case_with_unmarked = { let created_sitrep_id = SitrepUuid::new_v4(); fm::Case { @@ -356,7 +380,8 @@ mod tests { }; let cases = [ - open_case, + open_case1, + open_case2, closed_case_with_unmarked, closed_case_without_unmarked, ] @@ -367,7 +392,8 @@ mod tests { // sitrep — add_unmarked_ereports uses this map to detect which ereports // have already appeared in the parent sitrep. let ereports_by_id = [ - ereport_in_open_case.clone(), + ereport_in_open_case1.clone(), + ereport_in_open_case2.clone(), ereport_in_closed_unmarked.clone(), ereport_in_closed_marked.clone(), ] @@ -399,21 +425,22 @@ mod tests { // Build analysis input let (input, report) = { let mut builder = Input::builder(Some(parent_sitrep), inv); - // Pass in three ereports: - // - one that is in the open case of the parent sitrep + // Pass in four ereports: + // - two that are in the open cases of the parent sitrep // - one that is in the (to-be-copied-forward) closed case // - one that is brand-new // // Notably, `ereport_in_closed_marked` is NOT passed here, // simulating that it was already marked seen in the database. builder.add_unmarked_ereports([ - (*ereport_in_open_case).clone(), + (*ereport_in_open_case1).clone(), + (*ereport_in_open_case2).clone(), (*ereport_in_closed_unmarked).clone(), (*ereport_new).clone(), ]); builder.build() }; - dbg!(report); + eprintln!("{}", report.display_multiline(0)); // Check the "new ereports" in the constructed input. assert!( @@ -422,8 +449,13 @@ mod tests { sitrep)" ); assert!( - !input.new_ereports().contains_key(ereport_in_open_case.id()), - "ereport_in_open_case should NOT be in new_ereports (it is \ + !input.new_ereports().contains_key(ereport_in_open_case1.id()), + "ereport_in_open_case1 should NOT be in new_ereports (it is \ + already associated with an open case in the parent sitrep)" + ); + assert!( + !input.new_ereports().contains_key(ereport_in_open_case2.id()), + "ereport_in_open_case2 should NOT be in new_ereports (it is \ already associated with an open case in the parent sitrep)" ); assert!( @@ -462,20 +494,28 @@ mod tests { // Check the contents of open cases. assert!( - input.cases().contains_key(&open_case_id), - "the open case from the parent sitrep should be in input.cases()" + input.cases().contains_key(&open_case1_id), + "open case 1 from the parent sitrep should be in input.cases()" ); - assert_eq!(input.cases().len(), 1, "exactly one case should be open"); + assert!( + input.cases().contains_key(&open_case2_id), + "open case 2 from the parent sitrep should be in input.cases()" + ); + assert_eq!(input.cases().len(), 2, "exactly two cases should be open"); // Start building a sitrep... let mut sitrep_builder = SitrepBuilder::new_with_rng(log, &input, fm_test.sitrep_rng); - // The open case from the parent sitrep must be accessible via - // case_mut() so that the diagnosis engine can update it. + // The open cases from the parent sitrep must be accessible via + // case_mut() so that the diagnosis engines can update it. + assert!( + sitrep_builder.cases.case_mut(&open_case1_id).is_some(), + "open case 1 should be accessible via case_mut()" + ); assert!( - sitrep_builder.cases.case_mut(&open_case_id).is_some(), - "the open case should be accessible via case_mut()" + sitrep_builder.cases.case_mut(&open_case2_id).is_some(), + "open case 2 should be accessible via case_mut()" ); assert!( sitrep_builder @@ -493,16 +533,64 @@ mod tests { "the closed_case_without_unmarked should NOT be accessible via \ case_mut() (closed cases are not open for modification)" ); + sitrep_builder.comment_mut().push_str("my cool sitrep"); + + let new_case_id = { + let mut new_case = + sitrep_builder.cases.open_case(DiagnosisEngineKind::PowerShelf); + new_case.add_ereport( + &ereport_new, + "this ereport is important to the case somehow", + ); + new_case + .request_alert( + nexus_types::alert::AlertClass::TestFooBar, + &serde_json::json!({"alert": true}), + "requesting an alert to tell someone about something", + ) + .unwrap(); + *new_case.id() + }; + + // Close open case 2 + sitrep_builder + .cases + .case_mut(&open_case2_id) + .unwrap() + .close("i'm closing it because i want to"); // Build the final sitrep - let output_sitrep = dbg!( - sitrep_builder.build(OmicronZoneUuid::new_v4(), chrono::Utc::now()) + let (output_sitrep, report) = + sitrep_builder.build(OmicronZoneUuid::new_v4(), chrono::Utc::now()); + eprintln!("{}", report.display_multiline(0)); + + let open_case1 = output_sitrep + .cases + .get(&open_case1_id) + .expect("open case 1 should be in the output sitrep's cases"); + assert!( + open_case1.is_open(), + "open case 1 should be open in the output sitrep" ); + let open_case2 = output_sitrep + .cases + .get(&open_case2_id) + .expect("open case 2 should be in the output sitrep's cases"); assert!( - output_sitrep.cases.contains_key(&open_case_id), - "open case should be in the output sitrep's cases" + !open_case2.is_open(), + "open case 2 should be closed in the output sitrep" ); + + let new_case = output_sitrep + .cases + .get(&new_case_id) + .expect("new case should be in the output sitrep's cases"); + assert!( + new_case.is_open(), + "new case should be open in the output sitrep" + ); + assert!( output_sitrep.cases.contains_key(&closed_case_with_unmarked_id), "closed cases with unmarked ereports should be copied forward \ @@ -515,9 +603,10 @@ mod tests { ); assert_eq!( output_sitrep.cases.len(), - 2, - "the output sitrep should have exactly 2 cases: the open case and \ - the closed-but-copied-forward case" + 4, + "the output sitrep should have exactly 4 cases: the open case, \ + the newly-closed case, the closed-but-copied-forward case, and \ + the new case" ); logctx.cleanup_successful(); diff --git a/nexus/fm/src/builder.rs b/nexus/fm/src/builder.rs index ca1c564e935..e3573a9488b 100644 --- a/nexus/fm/src/builder.rs +++ b/nexus/fm/src/builder.rs @@ -75,7 +75,7 @@ impl<'a> SitrepBuilder<'a> { &self.comment } - pub fn comment_mut(&mut self) -> &mut str { + pub fn comment_mut(&mut self) -> &mut String { &mut self.comment } @@ -83,20 +83,40 @@ impl<'a> SitrepBuilder<'a> { self, creator_id: OmicronZoneUuid, time_created: chrono::DateTime, - ) -> fm::Sitrep { + ) -> (fm::Sitrep, fm::analysis_reports::AnalysisReport) { let mut ereports_by_id = iddqd::IdOrdMap::new(); + let mut report_cases = IdOrdMap::new(); let cases = self .cases .cases .into_iter() - .map(fm::Case::from) + // Note that entries are only pushed to `report_cases` for open + // cases, as the closed cases which are just being copied forward + // into the next sitrep have, by construction, not been changed in + // this builder, since they weren't exposed for modification by the + // builder API. Thus, we really don't have anything new to say about + // them that's worth including in the report, as the fact that they + // were copied forward will be recorded in the input report. + .map(|case_builder| { + let (case, report) = case_builder.build(); + report_cases.insert_unique(report).expect( + "we are iterating over an IdOrdMap, so the entries \ + should already be unique", + ); + case + }) .chain(self.closed_cases_copied_forward.iter().cloned()) .inspect(|case| { ereports_by_id .extend(case.ereports.iter().map(|ce| ce.ereport.clone())); }) .collect(); - fm::Sitrep { + let report = fm::analysis_reports::AnalysisReport { + sitrep_id: self.sitrep_id, + comment: self.comment.clone(), + cases: report_cases, + }; + let sitrep = fm::Sitrep { metadata: fm::SitrepMetadata { id: self.sitrep_id, parent_sitrep_id: self.parent_sitrep.map(|s| s.metadata.id), @@ -107,6 +127,7 @@ impl<'a> SitrepBuilder<'a> { }, cases, ereports_by_id, - } + }; + (sitrep, report) } } diff --git a/nexus/fm/src/builder/case.rs b/nexus/fm/src/builder/case.rs index 9a502d4d764..bff87882a73 100644 --- a/nexus/fm/src/builder/case.rs +++ b/nexus/fm/src/builder/case.rs @@ -4,6 +4,7 @@ use super::rng; use anyhow::Context; +use fm::analysis_reports; use iddqd::id_ord_map::{self, IdOrdMap}; use nexus_types::alert::AlertClass; use nexus_types::fm; @@ -17,6 +18,7 @@ pub struct CaseBuilder { case: fm::Case, sitrep_id: SitrepUuid, rng: rng::CaseBuilderRng, + report_log: analysis_reports::DebugLog, } #[derive(Debug)] @@ -69,9 +71,10 @@ impl AllCases { alerts_requested: Default::default(), support_bundles_requested: Default::default(), }; - entry.insert(CaseBuilder::new( - &self.log, sitrep_id, case, case_rng, - )) + let mut builder = + CaseBuilder::new(&self.log, sitrep_id, case, case_rng); + builder.report_log.entry("opened case"); + entry.insert(builder) } }; @@ -117,13 +120,14 @@ impl CaseBuilder { "de" => case.metadata.de.to_string(), "created_sitrep_id" => case.metadata.created_sitrep_id.to_string(), )); - Self { log, case, sitrep_id, rng } + Self { log, case, sitrep_id, rng, report_log: Default::default() } } pub fn request_alert( &mut self, class: AlertClass, alert: &impl serde::Serialize, + comment: impl ToString, ) -> anyhow::Result<()> { let id = self.rng.next_alert(); let req = fm::case::AlertRequest { @@ -138,20 +142,31 @@ impl CaseBuilder { anyhow::anyhow!("an alert with ID {id:?} already exists") })?; + // TODO(eliza): add a comment field to the alert request record in the + // DB, as well... + let comment = comment.to_string(); slog::info!( &self.log, "requested an alert"; "alert_id" => %id, "alert_class" => ?class, + "comment" => %comment, ); + self.report_log + .entry("requested alert") + .kv("alert_id", id) + .kv("alert_class", &class) + .comment(comment); Ok(()) } - pub fn close(&mut self) { + pub fn close(&mut self, comment: impl ToString) { self.case.metadata.closed_sitrep_id = Some(self.sitrep_id); - slog::info!(&self.log, "case closed"); + let comment = comment.to_string(); + slog::info!(&self.log, "case closed"; "comment" => %comment); + self.report_log.entry("case closed").comment(comment); } pub fn add_ereport( @@ -159,12 +174,13 @@ impl CaseBuilder { report: &Arc, comment: impl ToString, ) { + let comment = comment.to_string(); let assignment_id = self.rng.next_case_ereport(); match self.case.ereports.insert_unique(fm::case::CaseEreport { id: assignment_id, ereport: report.clone(), assigned_sitrep_id: self.sitrep_id, - comment: comment.to_string(), + comment: comment.clone(), }) { Ok(_) => { slog::info!( @@ -173,7 +189,18 @@ impl CaseBuilder { "ereport_id" => %report.id(), "ereport_class" => ?report.class, "assignment_id" => %assignment_id, + "comment" => %comment, ); + + self.report_log + .entry("assigned ereport to case") + .comment(comment) + .kv("ereport_id", &format_args!("{}", report.id())) + .kv( + "ereport_class", + &report.class.as_deref().unwrap_or(""), + ) + .kv("assignment_id", assignment_id); } Err(_) => { slog::warn!( @@ -204,11 +231,15 @@ impl CaseBuilder { pub fn comment_mut(&mut self) -> &mut String { &mut self.case.metadata.comment } -} -impl From for fm::Case { - fn from(CaseBuilder { case, .. }: CaseBuilder) -> Self { - case + pub(crate) fn build(self) -> (fm::Case, fm::analysis_reports::CaseReport) { + let Self { case, report_log, .. } = self; + let report = fm::analysis_reports::CaseReport { + id: case.id, + metadata: case.metadata.clone(), + log: report_log, + }; + (case, report) } } diff --git a/nexus/types/output/analysis_input_report_empty.out b/nexus/types/output/analysis_input_report_empty.out index 6e19fc78550..f221b3960a0 100644 --- a/nexus/types/output/analysis_input_report_empty.out +++ b/nexus/types/output/analysis_input_report_empty.out @@ -1,5 +1,5 @@ fault management analysis inputs ------ ---------- -------- ------ +-------------------------------- parent sitrep: inventory collection: bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb no new ereports since the parent sitrep diff --git a/nexus/types/output/analysis_input_report_same_inv.out b/nexus/types/output/analysis_input_report_same_inv.out index 91a4de738b4..5ed6b60d151 100644 --- a/nexus/types/output/analysis_input_report_same_inv.out +++ b/nexus/types/output/analysis_input_report_same_inv.out @@ -1,5 +1,5 @@ fault management analysis inputs ------ ---------- -------- ------ +-------------------------------- parent sitrep: aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa inventory collection: bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb --> same collection as parent sitrep diff --git a/nexus/types/output/analysis_input_report_with_cases.out b/nexus/types/output/analysis_input_report_with_cases.out index 3cac8d5f9ad..f3af0fe55ff 100644 --- a/nexus/types/output/analysis_input_report_with_cases.out +++ b/nexus/types/output/analysis_input_report_with_cases.out @@ -1,25 +1,24 @@ fault management analysis inputs ------ ---------- -------- ------ +-------------------------------- parent sitrep: aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa inventory collection: bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb --> different from parent sitrep (collection cccccccc-cccc-cccc-cccc-cccccccccccc) new ereports (2 total): -* ereport dddddddd-dddd-dddd-dddd-dddddddddddd:3 -* ereport dddddddd-dddd-dddd-dddd-dddddddddddd:4 + * ereport dddddddd-dddd-dddd-dddd-dddddddddddd:3 + * ereport dddddddd-dddd-dddd-dddd-dddddddddddd:4 cases (2 total): open cases (1 total): - * case 11111111-1111-1111-1111-111111111111 - diagnosis engine: power_shelf - opened in sitrep: 22222222-2222-2222-2222-222222222222 - comment: PSU 0 faulted + * case 11111111-1111-1111-1111-111111111111 + // PSU 0 faulted + diagnosis engine: power_shelf + opened in sitrep: 22222222-2222-2222-2222-222222222222 closed cases copied forwards (1 total): - * case 33333333-3333-3333-3333-333333333333 - diagnosis engine: power_shelf - opened in sitrep: 44444444-4444-4444-4444-444444444444 - closed in sitrep: 55555555-5555-5555-5555-555555555555 - comment: PSU 1 replaced - - copied forwards because these ereports haven't been marked seen yet: - * ereport dddddddd-dddd-dddd-dddd-dddddddddddd:2 + * case 33333333-3333-3333-3333-333333333333 + // PSU 1 replaced + diagnosis engine: power_shelf + opened in sitrep: 44444444-4444-4444-4444-444444444444 + closed in sitrep: 55555555-5555-5555-5555-555555555555 + copied forwards because these ereports haven't been marked seen yet: + * ereport dddddddd-dddd-dddd-dddd-dddddddddddd:2 diff --git a/nexus/types/output/log_entry_display_nested_values.out b/nexus/types/output/log_entry_display_nested_values.out new file mode 100644 index 00000000000..9c6ad28daa5 --- /dev/null +++ b/nexus/types/output/log_entry_display_nested_values.out @@ -0,0 +1,9 @@ +nested example: + // shows nesting + * arr: + 1. 10 + 2. 20 + * flat_key: flat_value + * obj: + * inner_a: va + * inner_b: 2 diff --git a/nexus/types/src/fm/analysis_reports.rs b/nexus/types/src/fm/analysis_reports.rs index 303e91e2f6b..2ff93160d4e 100644 --- a/nexus/types/src/fm/analysis_reports.rs +++ b/nexus/types/src/fm/analysis_reports.rs @@ -7,12 +7,279 @@ use super::case; use super::ereport::EreportId; +use iddqd::IdOrdMap; use omicron_uuid_kinds::{CaseUuid, CollectionUuid, SitrepUuid}; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use std::collections::BTreeSet; use std::fmt; +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +pub struct AnalysisReport { + pub sitrep_id: SitrepUuid, + pub comment: String, + pub cases: IdOrdMap, +} + +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +pub struct CaseReport { + pub id: CaseUuid, + #[serde(flatten)] + pub metadata: case::Metadata, + pub log: DebugLog, +} + +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] +#[serde(transparent)] +pub struct DebugLog(Vec); + +/// An entry in an analysis report's debug log. +/// +/// This type is somewhat intentionally very "stringly-typed": through the use +/// of `#[serde(skip_serializing_if = "Option::is_none")]`, `#[serde(default)]`, +/// and `#[serde(flatten)]`, we are essentially saying that this consists of a +/// string field named `event`, and any number of other string fields, which are +/// just thrown in a bag of key-value pairs. If one is named 'comment', we +/// handle it separately." +/// +/// If we add additional fields that are handled specially, they will still go +/// in the bag of `kvs` if they are parsed by an OMDB version that doesn't +/// understand them. Conversely, any new special-cased fields should be +/// `Option`al, so that OMDB can still display event log entries emitted by +/// older versions of Nexus. +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +pub struct LogEntry { + event: String, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] + comment: Option, + #[serde(flatten)] + kvs: BTreeMap, +} + +impl iddqd::IdOrdItem for CaseReport { + type Key<'a> = &'a CaseUuid; + fn key(&self) -> Self::Key<'_> { + &self.id + } + + iddqd::id_upcast!(); +} + +impl AnalysisReport { + pub fn display_multiline(&self, indent: usize) -> impl fmt::Display + '_ { + struct AnalysisReportDisplayer<'a> { + report: &'a AnalysisReport, + indent: usize, + } + + impl<'a> fmt::Display for AnalysisReportDisplayer<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let &Self { + report: AnalysisReport { cases, sitrep_id, comment }, + indent, + } = self; + writeln!(f, "{:indent$}fault management analysis report", "")?; + writeln!(f, "{:indent$}--------------------------------", "")?; + if !comment.is_empty() { + writeln!(f, "{:indent$}// {comment}", "")?; + } + writeln!(f, "{:indent$}sitrep ID: {sitrep_id}", "")?; + writeln!(f, "{:indent$}cases:", "")?; + for case in cases { + case.display_multiline(indent + 2, Some(*sitrep_id)) + .fmt(f)?; + } + Ok(()) + } + } + + AnalysisReportDisplayer { report: self, indent } + } +} + +impl CaseReport { + pub fn display_multiline( + &self, + indent: usize, + this_sitrep: Option, + ) -> impl fmt::Display + '_ { + struct CaseReportDisplayer<'a> { + report: &'a CaseReport, + indent: usize, + this_sitrep: Option, + } + + impl<'a> fmt::Display for CaseReportDisplayer<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let &Self { + report: CaseReport { id, metadata, log: DebugLog(log) }, + indent, + this_sitrep, + } = self; + let bullet = if indent > 0 { "* " } else { "" }; + writeln!(f, "{:indent$}{bullet}case {id}", "")?; + let indent = indent + 2; + metadata.display_multiline(indent, this_sitrep).fmt(f)?; + if !log.is_empty() { + writeln!(f, "{:indent$}activity in this analysis:", "")?; + let indent = indent + 2; + for entry in &log[..] { + entry.display_indented(indent).fmt(f)?; + } + } else { + writeln!(f, "{:indent$}no activity in this analysis", "")?; + } + Ok(()) + } + } + + CaseReportDisplayer { report: self, indent, this_sitrep } + } +} + +impl DebugLog { + pub fn entry(&mut self, event: impl ToString) -> &mut LogEntry { + self.0.push(LogEntry::new(event)); + self.0.last_mut().expect("we just pushed it") + } +} + +/// Recursively format a JSON value as a bulleted list entry, nesting any +/// object or array children as indented sub-bullets. +fn fmt_json_value( + f: &mut fmt::Formatter<'_>, + key: &str, + value: &serde_json::Value, + indent: usize, +) -> fmt::Result { + match value { + serde_json::Value::Object(map) => { + writeln!(f, "{:indent$}* {key}:", "")?; + for (k, v) in map { + fmt_json_value(f, k, v, indent + 2)?; + } + Ok(()) + } + serde_json::Value::Array(arr) => { + writeln!(f, "{:indent$}* {key}:", "")?; + let indent = indent + 2; + for (i, v) in arr.iter().enumerate() { + fmt_json_array_item(f, i + 1, v, indent)?; + } + Ok(()) + } + serde_json::Value::String(s) => { + writeln!(f, "{:indent$}* {key}: {s}", "") + } + serde_json::Value::Null => { + writeln!(f, "{:indent$}* {key}: ", "") + } + serde_json::Value::Bool(b) => { + writeln!(f, "{:indent$}* {key}: {b}", "") + } + serde_json::Value::Number(n) => { + writeln!(f, "{:indent$}* {key}: {n}", "") + } + } +} + +/// Format a single element of a JSON array as a numbered list item, +/// e.g. `1. value` for scalars or `1.` followed by indented children for +/// objects and nested arrays. +fn fmt_json_array_item( + f: &mut fmt::Formatter<'_>, + n: usize, + value: &serde_json::Value, + indent: usize, +) -> fmt::Result { + match value { + serde_json::Value::Object(map) => { + writeln!(f, "{:indent$}{n}.", "")?; + for (k, v) in map { + fmt_json_value(f, k, v, indent + 2)?; + } + Ok(()) + } + serde_json::Value::Array(arr) => { + writeln!(f, "{:indent$}{n}.", "")?; + let indent = indent + 2; + for (i, v) in arr.iter().enumerate() { + fmt_json_array_item(f, i + 1, v, indent)?; + } + Ok(()) + } + serde_json::Value::String(s) => { + writeln!(f, "{:indent$}{n}. {s}", "") + } + serde_json::Value::Null => { + writeln!(f, "{:indent$}{n}. ", "") + } + serde_json::Value::Bool(b) => { + writeln!(f, "{:indent$}{n}. {b}", "") + } + serde_json::Value::Number(num) => { + writeln!(f, "{:indent$}{n}. {num}", "") + } + } +} + +impl LogEntry { + pub fn new(event: impl ToString) -> Self { + Self { event: event.to_string(), comment: None, kvs: BTreeMap::new() } + } + + pub fn comment(&mut self, comment: impl ToString) -> &mut Self { + self.comment = Some(comment.to_string()); + self + } + + pub fn kv( + &mut self, + key: impl ToString, + value: impl Serialize, + ) -> &mut Self { + self.kvs(std::iter::once((key, value))) + } + + pub fn kvs( + &mut self, + kvs: impl IntoIterator, + ) -> &mut Self { + self.kvs.extend(kvs.into_iter().map(|(k, v)| { + let v = serde_json::to_value(v) + .unwrap_or_else(|e| serde_json::Value::String(e.to_string())); + (k.to_string(), v) + })); + self + } + + pub fn display_indented(&self, indent: usize) -> impl fmt::Display + '_ { + struct LogEntryDisplayer<'a> { + entry: &'a LogEntry, + indent: usize, + } + + impl<'a> fmt::Display for LogEntryDisplayer<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let &Self { entry: LogEntry { event, comment, kvs }, indent } = + self; + let bullet = if indent > 0 { "* " } else { "" }; + let colon = if kvs.is_empty() { "" } else { ":" }; + writeln!(f, "{:indent$}{bullet}{event}{colon}", "")?; + if let Some(comment) = comment { + writeln!(f, "{:indent$} // {comment}", "")?; + } + for (k, v) in kvs { + fmt_json_value(f, k, v, indent + 2)?; + } + Ok(()) + } + } + LogEntryDisplayer { entry: self, indent } + } +} + /// Summarizes the inputs to sitrep analysis. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct InputReport { @@ -60,7 +327,7 @@ impl fmt::Display for InputReportMultilineDisplay<'_> { } = self; writeln!(f, "{:indent$}fault management analysis inputs", "")?; - writeln!(f, "{:indent$}----- ---------- -------- ------", "")?; + writeln!(f, "{:indent$}--------------------------------", "")?; if let Some(id) = parent_sitrep_id { writeln!(f, "{:indent$}parent sitrep: {id}", "",)?; } else { @@ -86,6 +353,7 @@ impl fmt::Display for InputReportMultilineDisplay<'_> { "", new_ereport_ids.len() )?; + let indent = indent + 2; for ereport_id in new_ereport_ids { writeln!(f, "{:indent$}* ereport {ereport_id}", "")?; } @@ -110,6 +378,7 @@ impl fmt::Display for InputReportMultilineDisplay<'_> { "", open_cases.len() )?; + let indent = indent + 2; for (case_id, metadata) in open_cases { writeln!(f, "{:indent$}* case {case_id}", "")?; metadata.display_multiline(indent + 2, None).fmt(f)?; @@ -129,6 +398,7 @@ impl fmt::Display for InputReportMultilineDisplay<'_> { "", closed_cases_copied_forward.len() )?; + let indent = indent + 2; for ( case_id, ClosedCaseReport { metadata, unmarked_ereports }, @@ -139,7 +409,7 @@ impl fmt::Display for InputReportMultilineDisplay<'_> { metadata.display_multiline(indent, None).fmt(f)?; writeln!( f, - "\n{:indent$}copied forwards because these ereports \ + "{:indent$}copied forwards because these ereports \ haven't been marked seen yet:", "" )?; @@ -301,4 +571,103 @@ mod tests { &output, ); } + + /// A JSON object containing only the required `event` field should + /// always deserialize as a valid `LogEntry`. + #[test] + fn test_log_entry_backwards_compat_event_only() { + let json = serde_json::json!({ "event": "something happened" }); + let entry: LogEntry = serde_json::from_value(json).unwrap(); + assert_eq!(entry.event, "something happened"); + assert_eq!(entry.comment, None); + assert!(entry.kvs.is_empty()); + } + + /// A JSON object with `event`, `comment`, and arbitrary additional + /// fields should deserialize as a valid `LogEntry` — the extra + /// fields land in the flattened `kvs` map. + #[test] + fn test_log_entry_forwards_compat_extra_fields() { + let json = serde_json::json!({ + "event": "something happened", + "comment": "this is a comment", + "extra_string": "extra_value", + "extra_number": 42, + "nested": { "a": 1, "b": "two" } + }); + let entry: LogEntry = serde_json::from_value(json).unwrap(); + assert_eq!(entry.event, "something happened"); + assert_eq!(entry.comment.as_deref(), Some("this is a comment")); + assert_eq!( + entry.kvs.get("extra_string"), + Some(&serde_json::Value::String("extra_value".to_string())) + ); + assert_eq!(entry.kvs.get("extra_number"), Some(&serde_json::json!(42))); + assert_eq!( + entry.kvs.get("nested"), + Some(&serde_json::json!({ "a": 1, "b": "two" })) + ); + } + + /// A JSON object with `event`, `comment`, and extra fields round-trips + /// through serialization: serialize → deserialize produces an + /// equivalent `LogEntry`. + #[test] + fn test_log_entry_roundtrip() { + let mut entry = LogEntry::new("test event"); + entry + .comment("a comment") + .kv("simple", "value") + .kv("number", 123) + .kv("flag", true); + + let serialized = serde_json::to_value(&entry).unwrap(); + let deserialized: LogEntry = + serde_json::from_value(serialized).unwrap(); + assert_eq!(entry, deserialized); + } + + /// Extra fields of various JSON types (string, number, bool, null, + /// array, nested object) are all preserved through deserialization. + #[test] + fn test_log_entry_forwards_compat_varied_types() { + let json = serde_json::json!({ + "event": "varied types", + "a_bool": true, + "a_null": null, + "an_array": [1, "two", false], + "deep": { "level1": { "level2": "hi" } } + }); + let entry: LogEntry = serde_json::from_value(json).unwrap(); + assert_eq!(entry.event, "varied types"); + assert_eq!(entry.kvs.get("a_bool"), Some(&serde_json::json!(true))); + assert_eq!(entry.kvs.get("a_null"), Some(&serde_json::Value::Null)); + assert_eq!( + entry.kvs.get("an_array"), + Some(&serde_json::json!([1, "two", false])) + ); + assert_eq!( + entry.kvs.get("deep"), + Some(&serde_json::json!({ "level1": { "level2": "hi" } })) + ); + } + + /// The pretty-printer handles nested objects and arrays by indenting + /// sub-entries under their parent bullet point. + #[test] + fn test_log_entry_display_nested_values() { + let json = serde_json::json!({ + "event": "nested example", + "comment": "shows nesting", + "flat_key": "flat_value", + "obj": { "inner_a": "va", "inner_b": 2 }, + "arr": [10, 20] + }); + let entry: LogEntry = serde_json::from_value(json).unwrap(); + let output = format!("{}", entry.display_indented(0)); + expectorate::assert_contents( + "output/log_entry_display_nested_values.out", + &output, + ); + } } diff --git a/nexus/types/src/fm/case.rs b/nexus/types/src/fm/case.rs index 2abb8c05e2a..2d0df4a0127 100644 --- a/nexus/types/src/fm/case.rs +++ b/nexus/types/src/fm/case.rs @@ -108,6 +108,10 @@ impl Metadata { const OPENED_IN: &str = "opened in sitrep:"; const CLOSED_IN: &str = "closed in sitrep:"; const WIDTH: usize = const_max_len(&[DE, OPENED_IN, CLOSED_IN]); + + if !comment.is_empty() { + writeln!(f, "{:>indent$}// {comment}", "")?; + } writeln!(f, "{:>indent$}{DE:indent$}comment: {comment}", "")?; - Ok(()) } }