[fm] first draft analysis reporting

[hawkw]

This branch adds an initial and kind of crappy pass at having the SitrepBuilder API produce an AnalysisReport type providing a human-readable summary of what changes were made to the sitrep during analysis. This is intended to be used for debugging purposes only, and will be included in the fm_analysis background task status. Eventually, it may be stuffed in CRDB when a sitrep is committed, for eventual retrieval and display by OMDB.

I think there room to make this a lot nicer over time as we keep working on this stuff, but I kinda want to just put a pin in it now so that I can move on to actually starting some initial diagnosis engine skeletons. As we actually implement more FM analysis, we can keep iterating on what goes in the reports and how they're formatted and stored.

[hawkw]

looks like helios / package failed due to 502 errors while trying to download from pkg.oxide.computer, which I suspect is not related to this change...

[hawkw]

for reference, the output from the analysis_input::tests::test_analysis_input_builder_and_sitrep_builder test (which probably shouldn't live in analysis_input anymore now that i'm trying to exercise the builder as well) looks like this:

  stdout ───

    running 1 test
    test analysis_input::tests::test_analysis_input_builder_and_sitrep_builder ... ok

    test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

  stderr ───
    log file: /tmp/nexus_fm-4d01e5ac1c058cde-test_analysis_input_builder_and_sitrep_builder.3573138.0.log
    note: configured to log to "/tmp/nexus_fm-4d01e5ac1c058cde-test_analysis_input_builder_and_sitrep_builder.3573138.0.log"
    fault management analysis inputs
    ----- ---------- -------- ------
    parent sitrep:        40315d33-63e9-455f-af4f-bf4c90d63400
    inventory collection: c4ebc143-2c69-4bd7-8c01-ff8b40689b2a
     --> different from parent sitrep (collection a5fc3a41-5178-4930-b714-5bcfb3a002cc)

    new ereports (1 total):
      * ereport 1abedbd3-768e-475b-9e8b-777e9fb4b2fc:5

    cases (2 total):
      open cases (1 total):
        * case dfbbe750-44a0-4d79-8c8f-73dc7ae0e60c
          diagnosis engine: power_shelf
          opened in sitrep: 40315d33-63e9-455f-af4f-bf4c90d63400
          comment: open case
      closed cases copied forwards (1 total):
        * case 96e58b56-e467-4a67-958e-f7f5de993152
          diagnosis engine: power_shelf
          opened in sitrep: 5bc8925e-129a-4a78-b4c7-6b8d4e6703f1
          closed in sitrep: 40315d33-63e9-455f-af4f-bf4c90d63400
          comment: closed case, has an unmarked ereport
          copied forwards because these ereports haven't been marked seen yet:
          * ereport 1abedbd3-768e-475b-9e8b-777e9fb4b2fc:3

    fault management analysis report
    ----- ---------- -------- ------
    // my cool sitrep
    sitrep ID: 489ea4b9-e2a3-4f62-83bb-23536da86f29
    cases:
      * case 0110ae2d-250d-4e5f-be72-38b1a353bff6
        diagnosis engine: power_shelf
        opened in sitrep: 489ea4b9-e2a3-4f62-83bb-23536da86f29 <-- this sitrep
        activity in this analysis:
          * opened case
            // this ereport is important to the case somehow
          * assigned ereport to case:
            * assignment_id: fa467964-1eb4-463b-8e5f-84061289c6f5
            * ereport_class: <none>
            * ereport_id: 1abedbd3-768e-475b-9e8b-777e9fb4b2fc:5
          * requested alert:
            * alert_class: test.foo.bar
            * alert_id: e91c3407-c189-4b1c-b176-d871fc013da0
      * case dfbbe750-44a0-4d79-8c8f-73dc7ae0e60c
        diagnosis engine: power_shelf
        opened in sitrep: 40315d33-63e9-455f-af4f-bf4c90d63400
        comment: open case
        no activity in this analysis

[smklein]

nit: can we remove the spaces? I parsed this as a table at first

[hawkw]

the inputs report is formatted similarly, but i'll get rid of it there as well!

[smklein]

It's probably fine for this to be extremely generic right now - this whole thing is pretty much "Stringly typed" - but we may want a more strongly-typed set of decisions in the future.

E.g.; I'm taking an "Action", or I'm "Awaiting More Information", or something else. etc. This would probably help with testing / ensuring coverage / consistency / etc

But for now this is fine

[hawkw]

So, again, this is intended to just be a human-readable log. I can imagine wanting to have an enum or something so that we can format different kinds of log entry differently, but I think that if we're testing the actual DE logic by asserting that the log contains certain entries, something has already gone wrong. And, part of why we might want it to be relatively stringly-typed is so that we can have some loose backwards/forwards-compatibility when displaying them.

The current thing uses #[serde(flatten)] on the KV map so that it's essentially saying "there's a field named "entry" in here, as well as any arbitrary k/v pairs in here. if one happens to be named "comment", we special-case it and format it differently." This way, we can add additional things that are handled in a special way later but if there's version drift, OMDB is not going to choke on it and die.

[hawkw]

I think I'm gonna go ahead and change the BTreeMap to String, JsonValue so you can stuff arbitrarily nested objects in here if you want to

[hawkw]

@smklein okay, i added a comment describing the stringiness in 8326205, and e4ce794 changes it so that you can just slam any impl Serialize thing in there. what do you think?

[smklein]

So my comment below about strong/weak types is, I guess, a little more about this:

• I 100% understand the desire for weak types in displays, to deal with backwards compatibility, etc
• ... but jumping directly to weak types, from the builder, makes it a little tougher to test that e.g. "adding an ereport adds this event log entry". My interpretation is that this event log could also be used by DEs? so it could be pretty arbitrary "what conditions cause it to trigger"

Anyway, I was just saying - if we have a strongly-typed interface + output here, and can convert that to the weakly-typed interface you're using for a nice display.

[smklein]

but also: we don't need to do this now

[hawkw]

Oh, I see, your point was about testing that the event log is correct, not using the event log as a signal to make assertions about. Hmm. I do agree that there is value in being able to test that the debugging output is correct, but I also feel like...well, we don't test every single slog macro, and I don't think we should. And, I don't want to make adding a new event log entry take a lot of effort.

How would you feel about a little more structure, in the form of an enum like

pub enum CaseEventType {
    Opened,
    Closed,
    AddedEreport,
    RequestedAlert,
    RequestedBundle,
    ArbitraryDeThing(String),
}

or something?

My resistance here is mostly that I would prefer to make it as low friction as possible to record arbitrary strings from a DE, and that I'm not super convinced that there's that much value in testing that an event log entry is added when the way you do that is just one function call. The entire goal of the report system is just to be able to print some strings in OMDB...

[hawkw]

Per our additional back-and-forth on this on Matrix yesterday, I've renamed this to "debug logs", and we've agreed that the more structured "planning report" analogue would be added if we need that to actually track state across planning steps in a machineable way.