Conversation
| let cases: IdOrdMap<_> = inputs | ||
| .parent_sitrep() | ||
| .iter() | ||
| .flat_map(|s| s.open_cases()) | ||
| .map(|case| { | ||
| .flat_map(|s| s.cases.iter()) | ||
| .filter_map(|case| { | ||
| if !case.is_open() { | ||
| let unmarked_ereport = | ||
| case.ereports.iter().find(|ereport| { | ||
| unmarked_seen_ereports | ||
| .contains(ereport.ereport_id()) | ||
| }); | ||
| if let Some(ereport) = unmarked_ereport { | ||
| slog::debug!( | ||
| &log, | ||
| "closed case must still be copied forwards, as it \ | ||
| contains an ereport not yet marked as seen"; | ||
| "case_id" => %case.id, | ||
| "ereport_id" => %ereport.ereport_id(), | ||
| "case_ereport_id" => %ereport.id, | ||
| ); | ||
| } else { | ||
| slog::trace!( | ||
| &log, | ||
| "closed case need no longer be copied forwards"; | ||
| "case_id" => %case.id, | ||
| ); | ||
| return None; | ||
| } | ||
| } else { | ||
| slog::trace!( | ||
| &log, | ||
| "open case will be copied forwards"; | ||
| "case_id" => %case.id, | ||
| ); | ||
| } |
There was a problem hiding this comment.
This is kind of part of the "interesting bit" when it comes to ereport marking and such. I am wondering about possibly moving preparing the list of cases to copy into the new sitrep into the InputBuilder so that it can be included in the AnalysisInputReport we spit out...but maybe the SitrepBuilder should just do a similar thing of generating a report when it goes and does this.
| pub struct InputBuilder { | ||
| input: Input, | ||
| } |
There was a problem hiding this comment.
We don't really need a separate builder type, and could just move the methods on it to Input...but I liked having the mutation being kept separated at the type level from the type that's actually passed as input to stuff.
|
Uh. It looks like the |
| Self { log, sitrep_id, cases, rng } | ||
| } | ||
|
|
||
| pub fn open_case( |
There was a problem hiding this comment.
I don't think it should be part of this PR, but I'm curious how we're going to decide to "open a case" for something vs "use an existing case".
Cool to see the code coming together enough that we're close to that question!
Co-authored-by: Sean Klein <sean@oxide.computer>
This reverts commit 3bffd47.
| use std::collections::BTreeSet; | ||
| use std::sync::Arc; | ||
|
|
||
| pub struct Input { |
There was a problem hiding this comment.
Would it be reasonable to ask for some rustdoc here on Input, InputBuilder, AnalysisInputReport (or maybe in lib.rs) to walk through the design?
IIUC, what's going on here is: Input is meant to be the input to all the diagnosis engines, whereas AnalysisInputReport is sort of a summary, just metadata used for display. The builder produces both in one pass, and then they go their separate ways. Is that right-ish?
There was a problem hiding this comment.
Yeah, that's right. Eventually the report stuff will likely get serialized and stashed in other places for debugging purposes. This could absolutely use some commentary, sorry — I was being lazy.
There was a problem hiding this comment.
Added some comments here in 4e688df --- what do you think?
| pub const NUM_WIDTH: usize = 4; | ||
| } | ||
|
|
||
| fn print_task_fm_analysis(details: &serde_json::Value) { |
There was a problem hiding this comment.
I think if the user runs omdb nexus background-tasks show without selecting a specific task, they'll see the /!\ analysis failed: FM analysis is not yet implemented error. Is that something that's ok to ship, or would you want to suppress that for now?
There was a problem hiding this comment.
OMDB is intended for use by Oxide engineering and support, and is not easily accessible to the end user. I'm not super worried about this being a call generator, since it's not in a customer-facing UI...
3 participants
This branch adds an initial implementation of the preparation phase for fault management analysis. Currently, this consists only of loading new ereports which were not included in the parent sitrep. This is performed using the code I added in #10156 for tracking which ereports have been included in a sitrep. In addition to actually loading the new ereports, I've also done a bunch of additional plumbing, like adding a background task for actually running fm analysis and stuff for reporting what occurred during the analysis. Right now, the ultimate outcome of all this is just an error message that says "FM analysis isn't implemented yet", but this branch adds a lot of structure in which to implement it eventually.