Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove LogMsg #8631

Open
jprochazk opened this issue Jan 9, 2025 · 0 comments
Open

Remove LogMsg #8631

jprochazk opened this issue Jan 9, 2025 · 0 comments
Labels
🟦 blueprint The data that defines our UI 🔩 data model enhancement New feature or request 🪵 Log & send APIs Affects the user-facing API for all languages

Comments

@jprochazk
Copy link
Member

Currently, all communication between SDK and Viewer is done through LogMsg:

pub enum LogMsg {
/// A new recording has begun.
///
/// Should usually be the first message sent.
SetStoreInfo(SetStoreInfo),
/// Log an entity using an [`ArrowMsg`].
//
// TODO(#6574): the store ID should be in the metadata here so we can remove the layer on top
ArrowMsg(StoreId, ArrowMsg),
/// Send after all messages in a blueprint to signal that the blueprint is complete.
///
/// This is so that the viewer can wait with activating the blueprint until it is
/// fully transmitted. Showing a half-transmitted blueprint can cause confusion,
/// and also lead to problems with view heuristics.
BlueprintActivationCommand(BlueprintActivationCommand),
}

With LogMsg, we're mixing a lot of different concerns into one thing. Now that we are migrating our SDK/Viewer comms to gRPC, we can do better. For example:

  • An endpoint to acquire the store info for a recording ID, replacing SetStoreInfo.
  • An endpoint to stream in all chunks from a recording, replacing ArrowMsg, and removing the need to pair each chunk with a store ID, because it's implicitly known by the client when opening the stream.
  • An endpoint to fetch the blueprint for a recording. This wouldn't have to be streamed at all, so we wouldn't need a BlueprintActivationCommand as a "stream end" marker.

On the rrd file side, we could:

  • Migrate it to only hold one recording, and put StoreInfo in the file header, removing the need for SetStoreInfo.
  • Use a separate file type for blueprints, removing the need for BlueprintActivationCommand. We already do this with .rbl, but we'd need to actually enforce it.

We probably still want multi-recording (including mixed with blueprint) files, but we could wrap these into a "rerun archive" file format, and provide the tools necessary to produce these files, as we currently do with rerun rrd.

@jprochazk jprochazk added enhancement New feature or request 🔩 data model 🟦 blueprint The data that defines our UI 🪵 Log & send APIs Affects the user-facing API for all languages labels Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🟦 blueprint The data that defines our UI 🔩 data model enhancement New feature or request 🪵 Log & send APIs Affects the user-facing API for all languages
Projects
None yet
Development

No branches or pull requests

1 participant