Store `PlanningReport`s in database for debugging only (PR 1/2) #9029

jgallagher · 2025-09-16T19:15:06Z

#8631 introduced PlanningReports for blueprints. We've wanted to store them in the database all along, but have punted on doing so because:

They're highly structured, making them difficult to map to SQL / diesel
We expect to want to change them pretty frequently at least in the near/mid term, which compounds problem 1

Chatting with @davepacheco, we came up with a compromise:

These reports are not intended for programmatic consumption, and are primarily useful as a debugging aid. (We want to report something to operators, but not at this level of detail. More on this momentarily.)
Therefore, it seems okay to punt on the diesel / SQL problem by storing these as serialized JSON blobs, under the condition that we never attempt to parse them in Nexus proper, and therefore don't have to deal with versioning.
We can (and this PR does) add an omdb subcommand that attempts to parse them; if omdb and the Nexus that produced the report are out of sync, this parsing may fail, but that's okay in that we can still at least dump the raw JSON out. This PR hedges on this a bit by storing the git commit in the JSON blob, so at least omdb can warn if it's expected that it can't parse it (and note the git commit from which the support operator would need an omdb to parse it successfully).

Subsequent work will:

Remove the report from the Blueprint structure altogether, since we're not going to faithfully store and load it in the db along with the other blueprint fields. (This work is done, and I'll stack a PR on top of this one.)
Extend the PlanningReport with some kind of "operator view" that is flattened both at the Rust level, so it's easier to store in the db, and semantically, so it makes sense to operators without exposing details that we don't want to explain (nor should an operator need to understand); e.g., various zone IDs, the details of mupdate overrides, etc. We will store this flattened view in a Blueprint and in the db, even though that means we have a bit of denormalization, since the flattened view could be derived from the full PlanningReport if we had it available. (This work is not done, but is next on my TODO list.)

plotnick

Thank you for following up with this work; it looks great to me. I think the compromise on JSON is perfectly reasonable, and using the git commit as an approximate version check is very clever.

davepacheco · 2025-09-17T17:12:38Z

dev-tools/omdb/build.rs

+    // sync with the other build.rs files in this repository.
    omicron_rpaths::configure_default_omicron_rpaths();
+
+    EmitBuilder::builder()


Can you add a comment about why we're using this? I'm just imagining future-us wanting for whatever reason to rip it out and not knowing what's consuming it.

added in 3fada07

davepacheco · 2025-09-17T17:16:31Z

Cargo.toml

 usdt = "0.5.0"
 uuid = { version = "1.17.0", features = ["serde", "v4"] }
 uzers = "0.12"
+vergen = { version = "8.3.2", features = ["git", "git2"] }


I see this is not the current release and the current release doesn't have some of these same interfaces. Are we planning to stick with this version forever?

I picked this version because it's the one we already have in Cargo.lock because it's used by crucible-common (https://github.com/oxidecomputer/crucible/blob/385be2e991708ead4fe9f28d57c66b841f2a92cd/common/Cargo.toml#L36). I think when I tried bumping it I ran into a cargo unsatisfiable dependency error; I'll try again. Maybe I should go bump it in crucible first?

I see. Up to you. I was asking because it always feels risky to jump off any dependency's train so it seems dicey to take on a new dependency where they've ripped out the thing we need, but it sounds like this is not a new dependency so no need to address it now.

Turns out they didn't rip it out, exactly, just moved it to a separate crate. I opened oxidecomputer/crucible#1770 to update Crucible, and can work around that for now by depending on their flavor that uses the git CLI instead of libgit. Done in c68985c

davepacheco · 2025-09-17T17:20:26Z

nexus/db-model/src/deployment.rs

+        // should only be useful to humans, potentially via omdb, and they (and
+        // omdb) can duplicate these fields to understand it.
+        let git_commit = if env!("VERGEN_GIT_DIRTY") == "true" {
+            concat!(env!("VERGEN_GIT_SHA"), "-dirty")


I didn't find docs on where these variables are defined. In the past, I've generally used git describe [--all] --dirty for stuff like this. I guess in those places, it's useful for me to know if I'm on a non-main or release branch, and maybe we don't care here.

I think I'd rather have the actual commit SHA than a git describe - even if just running locally, if I'm on my-branch, build Nexus, make some changes, do a new commit, then build omdb, I want to know that there might be differences (whereas I think git describe --all would just report my-branch for both?).

The version I've used before can uniquely identify the commit while preserving a symbolic name. I had to dig it up. It's git describe --all --long --dirty. In my workspace I see:

$ git describe --all --long --dirty heads/dap/handoff-live-test-0-gc0b5e3bc3

That's:

the symbolic name (by default an annotated tag, but --all allows it to pick up the branch name heads/dap/handoff-live-test)

0: the number of commits since the "tag" (probably always 0 if it found a branch that you're on)

the part after the g is the abbreviated commit name

And if I change a file locally, I get the expected -dirty suffix.

I'm not sure this is useful if we can't control what vergen does. And none of this is that important.

nexus/db-queries/src/db/datastore/deployment.rs

schema/crdb/dbinit.sql

dev-tools/omdb/src/bin/omdb/db.rs

…ebug-log

@davepacheco

them in the database all along, but have punted on doing so because: 1. They're highly structured, making them difficult to map to SQL / diesel 2. We expect to want to change them pretty frequently at least in the near/mid term, which compounds problem 1 Chatting with @davepacheco, we came up with a compromise: * These reports are not intended for programmatic consumption, and are primarily useful as a debugging aid. (We want to report _something_ to operators, but not at this level of detail. More on this momentarily.) * Therefore, it seems okay to punt on the diesel / SQL problem by storing these as serialized JSON blobs, under the condition that we never attempt to parse them in Nexus proper, and therefore don't have to deal with versioning. * We can (and this PR does) add an `omdb` subcommand that _attempts_ to parse them; if omdb and the Nexus that produced the report are out of sync, this parsing may fail, but that's okay in that we can still at least dump the raw JSON out. This PR hedges on this a bit by storing the git commit in the JSON blob, so at least omdb can warn if it's expected that it can't parse it (and note the git commit from which the support operator would need an omdb to parse it successfully).

jgallagher added 7 commits September 16, 2025 12:59

add debug log json blob for blueprint planner reports

7fbff0d

schema migration

c300ee9

add omdb db blueprints ... to show planner reports

a312b20

cargo fmt

a19c2d2

println -> eprintln

ae3210f

fix up duplicate IDs

1a3c9b7

expectorate

c8b92f0

jgallagher requested review from davepacheco and plotnick September 16, 2025 19:15

hakari

ee40da6

jgallagher mentioned this pull request Sep 16, 2025

Move planning report out of Blueprint #9032

Closed

plotnick approved these changes Sep 17, 2025

View reviewed changes

davepacheco reviewed Sep 17, 2025

View reviewed changes

jgallagher added 7 commits September 17, 2025 15:10

move to latest vergen

c68985c

pr feedback

db30e59

be consistent with plural of blueprint

711fbb7

Merge remote-tracking branch 'origin/main' into john/planner-report-d…

369ce85

…ebug-log

add comments noting why we want git SHAs

3fada07

Merge branch 'main' into john/planner-report-debug-log

6a4aea6

Merge branch 'main' into john/planner-report-debug-log

e36e601

jgallagher merged commit 45c032b into main Sep 19, 2025
18 checks passed

jgallagher deleted the john/planner-report-debug-log branch September 19, 2025 20:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Store `PlanningReport`s in database for debugging only (PR 1/2) #9029

Store `PlanningReport`s in database for debugging only (PR 1/2) #9029

Uh oh!

jgallagher commented Sep 16, 2025

Uh oh!

plotnick left a comment

Uh oh!

davepacheco Sep 17, 2025

Uh oh!

jgallagher Sep 17, 2025

Uh oh!

davepacheco Sep 17, 2025

Uh oh!

jgallagher Sep 17, 2025

Uh oh!

davepacheco Sep 17, 2025

Uh oh!

jgallagher Sep 17, 2025

Uh oh!

davepacheco Sep 17, 2025

Uh oh!

jgallagher Sep 17, 2025

Uh oh!

davepacheco Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Store PlanningReports in database for debugging only (PR 1/2) #9029

Store PlanningReports in database for debugging only (PR 1/2) #9029

Uh oh!

Conversation

jgallagher commented Sep 16, 2025

Uh oh!

plotnick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Store `PlanningReport`s in database for debugging only (PR 1/2) #9029

Store `PlanningReport`s in database for debugging only (PR 1/2) #9029