From c19a08f71c12a060b3c01f7d89948b2c61832808 Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Tue, 14 May 2024 20:15:53 +0200 Subject: [PATCH] Keep track of the RRD protocol version and display it where relevant (#6324) This augments `StoreInfo` with an in-memory only (!) `store_version` field. Our `Decoder` and `StreamDecoder` have been patched to fill out this field using the RRD version present in the stream's header. This version is now visible in the UI, using the CLI, and in the analytics. Viewer: ![image](https://github.com/rerun-io/rerun/assets/2910679/c7b7f211-1a16-41d4-b43a-d8c661819122) CLI: ![image](https://github.com/rerun-io/rerun/assets/2910679/df15efa9-e2a0-42b8-b17a-b2c10dc7a46d) Analytics: ![image](https://github.com/rerun-io/rerun/assets/2910679/b9cf5745-2eb1-4f8a-8a4c-2db9cb874c0b) - Fixes https://github.com/rerun-io/rerun/issues/6280 --- Cargo.lock | 7 ++ crates/re_analytics/src/event.rs | 32 ++++++--- crates/re_build_info/Cargo.toml | 13 ++++ crates/re_build_info/src/crate_version.rs | 6 ++ crates/re_data_loader/Cargo.toml | 1 + crates/re_data_loader/src/load_file.rs | 9 ++- crates/re_data_ui/src/entity_db.rs | 9 +++ crates/re_data_ui/src/log_msg.rs | 9 +++ crates/re_entity_db/Cargo.toml | 1 + crates/re_entity_db/examples/memory_usage.rs | 9 ++- crates/re_entity_db/src/store_bundle.rs | 1 + .../benches/msg_encode_benchmark.rs | 8 ++- crates/re_log_encoding/src/decoder/mod.rs | 38 +++++++--- crates/re_log_encoding/src/decoder/stream.rs | 24 ++++++- crates/re_log_encoding/src/encoder.rs | 19 +++-- crates/re_log_encoding/src/file_sink.rs | 12 +++- crates/re_log_types/Cargo.toml | 2 + crates/re_log_types/src/lib.rs | 9 +++ crates/re_sdk/src/binary_stream_sink.rs | 7 +- crates/re_sdk/src/lib.rs | 1 + crates/re_sdk/src/log_sink.rs | 14 ++-- crates/re_sdk/src/recording_stream.rs | 1 + crates/re_sdk_comms/Cargo.toml | 1 + crates/re_sdk_comms/src/buffered_client.rs | 5 +- crates/re_viewer/src/app.rs | 39 ++++++++-- crates/re_viewer/src/saving.rs | 3 +- .../re_viewer/src/viewer_analytics/event.rs | 71 +++++++++++-------- crates/rerun/src/run.rs | 7 +- 28 files changed, 280 insertions(+), 78 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1dc04a6d3b94..9bb3d19215bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4153,6 +4153,9 @@ dependencies = [ [[package]] name = "re_build_info" version = "0.16.0-alpha.4" +dependencies = [ + "serde", +] [[package]] name = "re_build_tools" @@ -4189,6 +4192,7 @@ dependencies = [ "once_cell", "parking_lot", "rayon", + "re_build_info", "re_build_tools", "re_log", "re_log_encoding", @@ -4318,6 +4322,7 @@ dependencies = [ "nohash-hasher", "parking_lot", "rand", + "re_build_info", "re_data_store", "re_format", "re_int_histogram", @@ -4429,6 +4434,7 @@ dependencies = [ "num-derive", "num-traits", "re_arrow2", + "re_build_info", "re_format", "re_format_arrow", "re_log", @@ -4614,6 +4620,7 @@ dependencies = [ "crossbeam", "document-features", "rand", + "re_build_info", "re_log", "re_log_encoding", "re_log_types", diff --git a/crates/re_analytics/src/event.rs b/crates/re_analytics/src/event.rs index bc7545bab807..e04f4d466ee2 100644 --- a/crates/re_analytics/src/event.rs +++ b/crates/re_analytics/src/event.rs @@ -90,6 +90,9 @@ pub struct StoreInfo { /// Where data is being logged. pub store_source: String, + /// The Rerun version that was used to encode the RRD data. + pub store_version: String, + // Various versions of the host environment. pub rust_version: Option, pub llvm_version: Option, @@ -225,16 +228,29 @@ impl Properties for OpenRecording { event.insert("app_env", app_env); if let Some(store_info) = store_info { - event.insert("application_id", store_info.application_id); - event.insert("recording_id", store_info.recording_id); - event.insert("store_source", store_info.store_source); - event.insert_opt("rust_version", store_info.rust_version); - event.insert_opt("llvm_version", store_info.llvm_version); - event.insert_opt("python_version", store_info.python_version); - event.insert("is_official_example", store_info.is_official_example); + let StoreInfo { + application_id, + recording_id, + store_source, + store_version, + rust_version, + llvm_version, + python_version, + is_official_example, + app_id_starts_with_rerun_example, + } = store_info; + + event.insert("application_id", application_id); + event.insert("recording_id", recording_id); + event.insert("store_source", store_source); + event.insert("store_version", store_version); + event.insert_opt("rust_version", rust_version); + event.insert_opt("llvm_version", llvm_version); + event.insert_opt("python_version", python_version); + event.insert("is_official_example", is_official_example); event.insert( "app_id_starts_with_rerun_example", - store_info.app_id_starts_with_rerun_example, + app_id_starts_with_rerun_example, ); } diff --git a/crates/re_build_info/Cargo.toml b/crates/re_build_info/Cargo.toml index 2bc3a0f19228..a37d63ef9c54 100644 --- a/crates/re_build_info/Cargo.toml +++ b/crates/re_build_info/Cargo.toml @@ -17,3 +17,16 @@ workspace = true [package.metadata.docs.rs] all-features = true + + +[features] +default = [] + +## Enable (de)serialization using serde. +serde = ["dep:serde"] + + +[dependencies] + +# Optional dependencies: +serde = { workspace = true, optional = true, features = ["derive", "rc"] } diff --git a/crates/re_build_info/src/crate_version.rs b/crates/re_build_info/src/crate_version.rs index eaac871671fd..4a576a704cb0 100644 --- a/crates/re_build_info/src/crate_version.rs +++ b/crates/re_build_info/src/crate_version.rs @@ -41,6 +41,7 @@ mod meta { /// - `01NNNNNN` -> `-rc.N` /// - `00000000` -> none of the above #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub struct CrateVersion { pub major: u8, pub minor: u8, @@ -48,7 +49,12 @@ pub struct CrateVersion { pub meta: Option, } +impl CrateVersion { + pub const LOCAL: Self = Self::parse(env!("CARGO_PKG_VERSION")); +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub enum Meta { Rc(u8), Alpha(u8), diff --git a/crates/re_data_loader/Cargo.toml b/crates/re_data_loader/Cargo.toml index c8e0301c8712..e82b460db455 100644 --- a/crates/re_data_loader/Cargo.toml +++ b/crates/re_data_loader/Cargo.toml @@ -24,6 +24,7 @@ default = [] [dependencies] +re_build_info.workspace = true re_log_encoding = { workspace = true, features = ["decoder"] } re_log_types.workspace = true re_log.workspace = true diff --git a/crates/re_data_loader/src/load_file.rs b/crates/re_data_loader/src/load_file.rs index 267eefff37c0..26f678b9ef10 100644 --- a/crates/re_data_loader/src/load_file.rs +++ b/crates/re_data_loader/src/load_file.rs @@ -101,7 +101,8 @@ pub(crate) fn prepare_store_info( let app_id = re_log_types::ApplicationId(path.display().to_string()); let store_source = re_log_types::StoreSource::File { file_source }; - let is_rrd = crate::SUPPORTED_RERUN_EXTENSIONS.contains(&extension(path).as_str()); + let ext = extension(path); + let is_rrd = crate::SUPPORTED_RERUN_EXTENSIONS.contains(&ext.as_str()); (!is_rrd).then(|| { LogMsg::SetStoreInfo(SetStoreInfo { @@ -113,6 +114,12 @@ pub(crate) fn prepare_store_info( is_official_example: false, started: re_log_types::Time::now(), store_source, + // NOTE: If this is a natively supported file, it will go through one of the + // builtin dataloaders, i.e. the local version. + // Otherwise, it will go through an arbitrary external loader, at which point we + // have no certainty what the version is. + store_version: crate::is_supported_file_extension(ext.as_str()) + .then_some(re_build_info::CrateVersion::LOCAL), }, }) }) diff --git a/crates/re_data_ui/src/entity_db.rs b/crates/re_data_ui/src/entity_db.rs index 5fc24e022a67..36258a5dfb98 100644 --- a/crates/re_data_ui/src/entity_db.rs +++ b/crates/re_data_ui/src/entity_db.rs @@ -44,6 +44,7 @@ impl crate::DataUi for EntityDb { is_official_example: _, started, store_source, + store_version, } = store_info; if let Some(cloned_from) = cloned_from { @@ -60,6 +61,14 @@ impl crate::DataUi for EntityDb { ui.label(store_source.to_string()); ui.end_row(); + if let Some(store_version) = store_version { + re_ui.grid_left_hand_label(ui, "Source RRD version"); + ui.label(store_version.to_string()); + ui.end_row(); + } else { + re_log::debug_once!("store version is undefined for this recording, this is a bug"); + } + re_ui.grid_left_hand_label(ui, "Kind"); ui.label(store_id.kind.to_string()); ui.end_row(); diff --git a/crates/re_data_ui/src/log_msg.rs b/crates/re_data_ui/src/log_msg.rs index 9a2c8a3d90d4..5a3ad7ef6148 100644 --- a/crates/re_data_ui/src/log_msg.rs +++ b/crates/re_data_ui/src/log_msg.rs @@ -48,6 +48,7 @@ impl DataUi for SetStoreInfo { started, store_source, is_official_example, + store_version, } = info; let re_ui = &ctx.re_ui; @@ -77,6 +78,14 @@ impl DataUi for SetStoreInfo { ui.label(format!("{store_source}")); ui.end_row(); + if let Some(store_version) = store_version { + re_ui.grid_left_hand_label(ui, "store_version:"); + ui.label(format!("{store_version}")); + ui.end_row(); + } else { + re_log::debug_once!("store version is undefined for this recording, this is a bug"); + } + re_ui.grid_left_hand_label(ui, "is_official_example:"); ui.label(format!("{is_official_example}")); ui.end_row(); diff --git a/crates/re_entity_db/Cargo.toml b/crates/re_entity_db/Cargo.toml index d5ed52af6722..5ed1e9829408 100644 --- a/crates/re_entity_db/Cargo.toml +++ b/crates/re_entity_db/Cargo.toml @@ -27,6 +27,7 @@ serde = ["dep:serde", "dep:rmp-serde", "re_log_types/serde"] [dependencies] +re_build_info.workspace = true re_data_store.workspace = true re_format.workspace = true re_int_histogram.workspace = true diff --git a/crates/re_entity_db/examples/memory_usage.rs b/crates/re_entity_db/examples/memory_usage.rs index 3d1e4626bb80..2effe5505b2c 100644 --- a/crates/re_entity_db/examples/memory_usage.rs +++ b/crates/re_entity_db/examples/memory_usage.rs @@ -63,8 +63,13 @@ fn log_messages() { fn encode_log_msg(log_msg: &LogMsg) -> Vec { let mut bytes = vec![]; let encoding_options = re_log_encoding::EncodingOptions::COMPRESSED; - re_log_encoding::encoder::encode(encoding_options, std::iter::once(log_msg), &mut bytes) - .unwrap(); + re_log_encoding::encoder::encode( + re_build_info::CrateVersion::LOCAL, + encoding_options, + std::iter::once(log_msg), + &mut bytes, + ) + .unwrap(); bytes } diff --git a/crates/re_entity_db/src/store_bundle.rs b/crates/re_entity_db/src/store_bundle.rs index 1cf0c21e6e4c..fd25eee34650 100644 --- a/crates/re_entity_db/src/store_bundle.rs +++ b/crates/re_entity_db/src/store_bundle.rs @@ -107,6 +107,7 @@ impl StoreBundle { is_official_example: false, started: re_log_types::Time::now(), store_source: re_log_types::StoreSource::Other("viewer".to_owned()), + store_version: Some(re_build_info::CrateVersion::LOCAL), }, }); diff --git a/crates/re_log_encoding/benches/msg_encode_benchmark.rs b/crates/re_log_encoding/benches/msg_encode_benchmark.rs index 5e609657ed7d..68c378408898 100644 --- a/crates/re_log_encoding/benches/msg_encode_benchmark.rs +++ b/crates/re_log_encoding/benches/msg_encode_benchmark.rs @@ -30,7 +30,13 @@ criterion_main!(benches); fn encode_log_msgs(messages: &[LogMsg]) -> Vec { let encoding_options = re_log_encoding::EncodingOptions::COMPRESSED; let mut bytes = vec![]; - re_log_encoding::encoder::encode(encoding_options, messages.iter(), &mut bytes).unwrap(); + re_log_encoding::encoder::encode( + re_build_info::CrateVersion::LOCAL, + encoding_options, + messages.iter(), + &mut bytes, + ) + .unwrap(); assert!(bytes.len() > messages.len()); bytes } diff --git a/crates/re_log_encoding/src/decoder/mod.rs b/crates/re_log_encoding/src/decoder/mod.rs index a1ea2fdc1696..8178f8c3e6f6 100644 --- a/crates/re_log_encoding/src/decoder/mod.rs +++ b/crates/re_log_encoding/src/decoder/mod.rs @@ -37,23 +37,22 @@ fn warn_on_version_mismatch( CrateVersion::from_bytes(encoded_version) }; - const LOCAL_VERSION: CrateVersion = CrateVersion::parse(env!("CARGO_PKG_VERSION")); - - if encoded_version.is_compatible_with(LOCAL_VERSION) { + if encoded_version.is_compatible_with(CrateVersion::LOCAL) { Ok(()) } else { match version_policy { VersionPolicy::Warn => { re_log::warn_once!( "Found log stream with Rerun version {encoded_version}, \ - which is incompatible with the local Rerun version {LOCAL_VERSION}. \ - Loading will try to continue, but might fail in subtle ways." + which is incompatible with the local Rerun version {}. \ + Loading will try to continue, but might fail in subtle ways.", + CrateVersion::LOCAL, ); Ok(()) } VersionPolicy::Error => Err(DecodeError::IncompatibleRerunVersion { file: encoded_version, - local: LOCAL_VERSION, + local: CrateVersion::LOCAL, }), } } @@ -109,7 +108,7 @@ pub fn decode_bytes( pub fn read_options( version_policy: VersionPolicy, bytes: &[u8], -) -> Result { +) -> Result<(CrateVersion, EncodingOptions), DecodeError> { let mut read = std::io::Cursor::new(bytes); let FileHeader { @@ -130,10 +129,11 @@ pub fn read_options( Serializer::MsgPack => {} } - Ok(options) + Ok((CrateVersion::from_bytes(version), options)) } pub struct Decoder { + version: CrateVersion, compression: Compression, read: R, uncompressed: Vec, // scratch space @@ -146,15 +146,24 @@ impl Decoder { let mut data = [0_u8; FileHeader::SIZE]; read.read_exact(&mut data).map_err(DecodeError::Read)?; - let compression = read_options(version_policy, &data)?.compression; + + let (version, options) = read_options(version_policy, &data)?; + let compression = options.compression; Ok(Self { + version, compression, read, uncompressed: vec![], compressed: vec![], }) } + + /// Returns the Rerun version that was used to encode the data in the first place. + #[inline] + pub fn version(&self) -> CrateVersion { + self.version + } } impl Iterator for Decoder { @@ -211,6 +220,12 @@ impl Iterator for Decoder { re_tracing::profile_scope!("MsgPack deser"); match rmp_serde::from_slice(&self.uncompressed[..uncompressed_len]) { + Ok(re_log_types::LogMsg::SetStoreInfo(mut msg)) => { + // Propagate the protocol version from the header into the `StoreInfo` so that all + // parts of the app can easily access it. + msg.info.store_version = Some(self.version()); + Some(Ok(re_log_types::LogMsg::SetStoreInfo(msg))) + } Ok(msg) => Some(Ok(msg)), Err(err) => Some(Err(err.into())), } @@ -227,6 +242,8 @@ fn test_encode_decode() { Time, }; + let rrd_version = CrateVersion::LOCAL; + let messages = vec![LogMsg::SetStoreInfo(SetStoreInfo { row_id: RowId::new(), info: StoreInfo { @@ -239,6 +256,7 @@ fn test_encode_decode() { rustc_version: String::new(), llvm_version: String::new(), }, + store_version: Some(rrd_version), }, })]; @@ -255,7 +273,7 @@ fn test_encode_decode() { for options in options { let mut file = vec![]; - crate::encoder::encode(options, messages.iter(), &mut file).unwrap(); + crate::encoder::encode(rrd_version, options, messages.iter(), &mut file).unwrap(); let decoded_messages = Decoder::new(VersionPolicy::Error, &mut file.as_slice()) .unwrap() diff --git a/crates/re_log_encoding/src/decoder/stream.rs b/crates/re_log_encoding/src/decoder/stream.rs index 3d2d27b86ccc..dfc8b8297006 100644 --- a/crates/re_log_encoding/src/decoder/stream.rs +++ b/crates/re_log_encoding/src/decoder/stream.rs @@ -2,6 +2,7 @@ use std::collections::VecDeque; use std::io::Cursor; use std::io::Read; +use re_build_info::CrateVersion; use re_log_types::LogMsg; use crate::decoder::read_options; @@ -17,6 +18,11 @@ use super::{DecodeError, VersionPolicy}; /// Chunks are given to the stream via `StreamDecoder::push_chunk`, /// and messages are read back via `StreamDecoder::try_read`. pub struct StreamDecoder { + /// The Rerun version used to encode the RRD data. + /// + /// `None` until a Rerun header has been processed. + version: Option, + /// How to handle version mismatches version_policy: VersionPolicy, @@ -72,6 +78,7 @@ enum State { impl StreamDecoder { pub fn new(version_policy: VersionPolicy) -> Self { Self { + version: None, version_policy, compression: Compression::Off, chunks: ChunkBuffer::new(), @@ -89,7 +96,9 @@ impl StreamDecoder { State::StreamHeader => { if let Some(header) = self.chunks.try_read(FileHeader::SIZE) { // header contains version and compression options - self.compression = read_options(self.version_policy, header)?.compression; + let (version, options) = read_options(self.version_policy, header)?; + self.version = Some(version); + self.compression = options.compression; // we might have data left in the current chunk, // immediately try to read length of the next message @@ -123,7 +132,15 @@ impl StreamDecoder { let message = rmp_serde::from_slice(bytes).map_err(DecodeError::MsgPack)?; self.state = State::MessageHeader; - return Ok(Some(message)); + + return if let re_log_types::LogMsg::SetStoreInfo(mut msg) = message { + // Propagate the protocol version from the header into the `StoreInfo` so that all + // parts of the app can easily access it. + msg.info.store_version = self.version; + Ok(Some(re_log_types::LogMsg::SetStoreInfo(msg))) + } else { + Ok(Some(message)) + }; } } } @@ -239,6 +256,7 @@ mod tests { is_official_example: false, started: Time::from_ns_since_epoch(0), store_source: StoreSource::Unknown, + store_version: Some(CrateVersion::LOCAL), }, }) } @@ -247,7 +265,7 @@ mod tests { let messages: Vec<_> = (0..n).map(|_| fake_log_msg()).collect(); let mut buffer = Vec::new(); - let mut encoder = Encoder::new(options, &mut buffer).unwrap(); + let mut encoder = Encoder::new(CrateVersion::LOCAL, options, &mut buffer).unwrap(); for message in &messages { encoder.append(message).unwrap(); } diff --git a/crates/re_log_encoding/src/encoder.rs b/crates/re_log_encoding/src/encoder.rs index fa00f120493d..f7ac2567f617 100644 --- a/crates/re_log_encoding/src/encoder.rs +++ b/crates/re_log_encoding/src/encoder.rs @@ -28,12 +28,13 @@ pub enum EncodeError { // ---------------------------------------------------------------------------- pub fn encode_to_bytes<'a>( + version: CrateVersion, options: EncodingOptions, msgs: impl IntoIterator, ) -> Result, EncodeError> { let mut bytes: Vec = vec![]; { - let mut encoder = Encoder::new(options, std::io::Cursor::new(&mut bytes))?; + let mut encoder = Encoder::new(version, options, std::io::Cursor::new(&mut bytes))?; for msg in msgs { encoder.append(msg)?; } @@ -52,12 +53,14 @@ pub struct Encoder { } impl Encoder { - pub fn new(options: EncodingOptions, mut write: W) -> Result { - const RERUN_VERSION: CrateVersion = CrateVersion::parse(env!("CARGO_PKG_VERSION")); - + pub fn new( + version: CrateVersion, + options: EncodingOptions, + mut write: W, + ) -> Result { FileHeader { magic: *crate::RRD_HEADER, - version: RERUN_VERSION.to_bytes(), + version: version.to_bytes(), options, } .encode(&mut write)?; @@ -117,12 +120,13 @@ impl Encoder { } pub fn encode<'a>( + version: CrateVersion, options: EncodingOptions, messages: impl Iterator, write: &mut impl std::io::Write, ) -> Result<(), EncodeError> { re_tracing::profile_function!(); - let mut encoder = Encoder::new(options, write)?; + let mut encoder = Encoder::new(version, options, write)?; for message in messages { encoder.append(message)?; } @@ -130,12 +134,13 @@ pub fn encode<'a>( } pub fn encode_as_bytes<'a>( + version: CrateVersion, options: EncodingOptions, messages: impl Iterator, ) -> Result, EncodeError> { re_tracing::profile_function!(); let mut bytes: Vec = vec![]; - let mut encoder = Encoder::new(options, &mut bytes)?; + let mut encoder = Encoder::new(version, options, &mut bytes)?; for message in messages { encoder.append(message)?; } diff --git a/crates/re_log_encoding/src/file_sink.rs b/crates/re_log_encoding/src/file_sink.rs index 38bc0b802064..7bebfd9e3f15 100644 --- a/crates/re_log_encoding/src/file_sink.rs +++ b/crates/re_log_encoding/src/file_sink.rs @@ -75,7 +75,11 @@ impl FileSink { let file = std::fs::File::create(&path) .map_err(|err| FileSinkError::CreateFile(path.clone(), err))?; - let encoder = crate::encoder::Encoder::new(encoding_options, file)?; + let encoder = crate::encoder::Encoder::new( + re_build_info::CrateVersion::LOCAL, + encoding_options, + file, + )?; let join_handle = spawn_and_stream(Some(&path), encoder, rx)?; Ok(Self { @@ -93,7 +97,11 @@ impl FileSink { re_log::debug!("Writing to stdout…"); - let encoder = crate::encoder::Encoder::new(encoding_options, std::io::stdout())?; + let encoder = crate::encoder::Encoder::new( + re_build_info::CrateVersion::LOCAL, + encoding_options, + std::io::stdout(), + )?; let join_handle = spawn_and_stream(None, encoder, rx)?; Ok(Self { diff --git a/crates/re_log_types/Cargo.toml b/crates/re_log_types/Cargo.toml index 1813ad254772..31bc52880f18 100644 --- a/crates/re_log_types/Cargo.toml +++ b/crates/re_log_types/Cargo.toml @@ -32,6 +32,7 @@ default = [] serde = [ "dep:serde", "dep:serde_bytes", + "re_build_info/serde", "re_string_interner/serde", "re_tuid/serde", "re_types_core/serde", @@ -40,6 +41,7 @@ serde = [ [dependencies] # Rerun +re_build_info.workspace = true re_format.workspace = true re_format_arrow.workspace = true re_log.workspace = true diff --git a/crates/re_log_types/src/lib.rs b/crates/re_log_types/src/lib.rs index 5c0b7ec4dfe0..8013a1741d17 100644 --- a/crates/re_log_types/src/lib.rs +++ b/crates/re_log_types/src/lib.rs @@ -37,6 +37,8 @@ mod data_table_batcher; use std::sync::Arc; +use re_build_info::CrateVersion; + pub use self::arrow_msg::{ArrowChunkReleaseCallback, ArrowMsg}; pub use self::data_cell::{DataCell, DataCellError, DataCellInner, DataCellResult}; pub use self::data_row::{ @@ -361,6 +363,13 @@ pub struct StoreInfo { pub started: Time, pub store_source: StoreSource, + + /// The Rerun version used to encoded the RRD data. + /// + // NOTE: The version comes directly from the decoded RRD stream's header, duplicating it here + // would probably only lead to more issues down the line. + #[serde(skip)] + pub store_version: Option, } impl StoreInfo { diff --git a/crates/re_sdk/src/binary_stream_sink.rs b/crates/re_sdk/src/binary_stream_sink.rs index f6ee5127264b..6a74c869ece5 100644 --- a/crates/re_sdk/src/binary_stream_sink.rs +++ b/crates/re_sdk/src/binary_stream_sink.rs @@ -133,8 +133,11 @@ impl BinaryStreamSink { let (tx, rx) = std::sync::mpsc::channel(); - let encoder = - re_log_encoding::encoder::Encoder::new(encoding_options, storage.inner.clone())?; + let encoder = re_log_encoding::encoder::Encoder::new( + re_build_info::CrateVersion::LOCAL, + encoding_options, + storage.inner.clone(), + )?; let join_handle = spawn_and_stream(encoder, rx)?; diff --git a/crates/re_sdk/src/lib.rs b/crates/re_sdk/src/lib.rs index 18a9af9baa22..697e08ecb52c 100644 --- a/crates/re_sdk/src/lib.rs +++ b/crates/re_sdk/src/lib.rs @@ -183,6 +183,7 @@ pub fn new_store_info( rustc_version: env!("RE_BUILD_RUSTC_VERSION").into(), llvm_version: env!("RE_BUILD_LLVM_VERSION").into(), }, + store_version: Some(re_build_info::CrateVersion::LOCAL), } } diff --git a/crates/re_sdk/src/log_sink.rs b/crates/re_sdk/src/log_sink.rs index a67b649fe194..58bab041fcc0 100644 --- a/crates/re_sdk/src/log_sink.rs +++ b/crates/re_sdk/src/log_sink.rs @@ -255,8 +255,11 @@ impl MemorySinkStorage { { let encoding_options = re_log_encoding::EncodingOptions::COMPRESSED; - let mut encoder = - re_log_encoding::encoder::Encoder::new(encoding_options, &mut buffer)?; + let mut encoder = re_log_encoding::encoder::Encoder::new( + re_build_info::CrateVersion::LOCAL, + encoding_options, + &mut buffer, + )?; for sink in sinks { // NOTE: It's fine, this is an in-memory sink so by definition there's no I/O involved // in this flush; it's just a matter of making the table batcher tick early. @@ -285,8 +288,11 @@ impl MemorySinkStorage { { let encoding_options = re_log_encoding::EncodingOptions::COMPRESSED; - let mut encoder = - re_log_encoding::encoder::Encoder::new(encoding_options, &mut buffer)?; + let mut encoder = re_log_encoding::encoder::Encoder::new( + re_build_info::CrateVersion::LOCAL, + encoding_options, + &mut buffer, + )?; let mut inner = self.inner.lock(); inner.has_been_used = true; diff --git a/crates/re_sdk/src/recording_stream.rs b/crates/re_sdk/src/recording_stream.rs index 4bb800106378..6d9f0162d80c 100644 --- a/crates/re_sdk/src/recording_stream.rs +++ b/crates/re_sdk/src/recording_stream.rs @@ -555,6 +555,7 @@ impl RecordingStreamBuilder { is_official_example, started: Time::now(), store_source, + store_version: Some(re_build_info::CrateVersion::LOCAL), }; let batcher_config = diff --git a/crates/re_sdk_comms/Cargo.toml b/crates/re_sdk_comms/Cargo.toml index 9ce91ab96dca..01e906930407 100644 --- a/crates/re_sdk_comms/Cargo.toml +++ b/crates/re_sdk_comms/Cargo.toml @@ -28,6 +28,7 @@ server = ["rand", "re_log_encoding/decoder"] [dependencies] +re_build_info.workspace = true re_log_encoding.workspace = true re_log_types = { workspace = true, features = ["serde"] } re_log.workspace = true diff --git a/crates/re_sdk_comms/src/buffered_client.rs b/crates/re_sdk_comms/src/buffered_client.rs index fcc24e7badbb..6a48e553bd4b 100644 --- a/crates/re_sdk_comms/src/buffered_client.rs +++ b/crates/re_sdk_comms/src/buffered_client.rs @@ -177,7 +177,10 @@ fn msg_encode( let packet_msg = match &msg_msg { MsgMsg::LogMsg(log_msg) => { - match re_log_encoding::encoder::encode_to_bytes(encoding_options, std::iter::once(log_msg)) { + match re_log_encoding::encoder::encode_to_bytes( + re_build_info::CrateVersion::LOCAL, + encoding_options, std::iter::once(log_msg), + ) { Ok(packet) => { re_log::trace!("Encoded message of size {}", packet.len()); Some(PacketMsg::Packet(packet)) diff --git a/crates/re_viewer/src/app.rs b/crates/re_viewer/src/app.rs index 222c958afb71..5ebad59d3403 100644 --- a/crates/re_viewer/src/app.rs +++ b/crates/re_viewer/src/app.rs @@ -1,3 +1,4 @@ +use re_build_info::CrateVersion; use re_data_source::{DataSource, FileContents}; use re_entity_db::entity_db::EntityDb; use re_log_types::{ApplicationId, FileSource, LogMsg, StoreKind}; @@ -1326,10 +1327,14 @@ fn blueprint_loader() -> BlueprintPersistence { let blueprint_path = crate::saving::default_blueprint_path(app_id)?; let messages = blueprint.to_messages(None)?; + let rrd_version = blueprint + .store_info() + .and_then(|info| info.store_version) + .unwrap_or(re_build_info::CrateVersion::LOCAL); // TODO(jleibs): Should we push this into a background thread? Blueprints should generally // be small & fast to save, but maybe not once we start adding big pieces of user data? - crate::saving::encode_to_file(&blueprint_path, messages.iter())?; + crate::saving::encode_to_file(rrd_version, &blueprint_path, messages.iter())?; re_log::debug!("Saved blueprint for {app_id} to {blueprint_path:?}"); @@ -1730,6 +1735,11 @@ fn save_recording( anyhow::bail!("No recording data to save"); }; + let rrd_version = entity_db + .store_info() + .and_then(|info| info.store_version) + .unwrap_or(re_build_info::CrateVersion::LOCAL); + let file_name = "data.rrd"; let title = if loop_selection.is_some() { @@ -1738,9 +1748,13 @@ fn save_recording( "Save recording" }; - save_entity_db(app, file_name.to_owned(), title.to_owned(), || { - entity_db.to_messages(loop_selection) - }) + save_entity_db( + app, + rrd_version, + file_name.to_owned(), + title.to_owned(), + || entity_db.to_messages(loop_selection), + ) } fn save_blueprint(app: &mut App, store_context: Option<&StoreContext<'_>>) -> anyhow::Result<()> { @@ -1750,6 +1764,12 @@ fn save_blueprint(app: &mut App, store_context: Option<&StoreContext<'_>>) -> an re_tracing::profile_function!(); + let rrd_version = store_context + .blueprint + .store_info() + .and_then(|info| info.store_version) + .unwrap_or(re_build_info::CrateVersion::LOCAL); + // We change the recording id to a new random one, // otherwise when saving and loading a blueprint file, we can end up // in a situation where the store_id we're loading is the same as the currently active one, @@ -1767,12 +1787,15 @@ fn save_blueprint(app: &mut App, store_context: Option<&StoreContext<'_>>) -> an ); let title = "Save blueprint"; - save_entity_db(app, file_name, title.to_owned(), || Ok(messages)) + save_entity_db(app, rrd_version, file_name, title.to_owned(), || { + Ok(messages) + }) } #[allow(clippy::needless_pass_by_ref_mut)] // `app` is only used on native fn save_entity_db( #[allow(unused_variables)] app: &mut App, // only used on native + rrd_version: CrateVersion, file_name: String, title: String, to_log_messages: impl FnOnce() -> re_log_types::DataTableResult>, @@ -1785,7 +1808,7 @@ fn save_entity_db( let messages = to_log_messages()?; wasm_bindgen_futures::spawn_local(async move { - if let Err(err) = async_save_dialog(&file_name, &title, &messages).await { + if let Err(err) = async_save_dialog(rrd_version, &file_name, &title, &messages).await { re_log::error!("File saving failed: {err}"); } }); @@ -1804,7 +1827,7 @@ fn save_entity_db( if let Some(path) = path { let messages = to_log_messages()?; app.background_tasks.spawn_file_saver(move || { - crate::saving::encode_to_file(&path, messages.iter())?; + crate::saving::encode_to_file(rrd_version, &path, messages.iter())?; Ok(path) })?; } @@ -1815,6 +1838,7 @@ fn save_entity_db( #[cfg(target_arch = "wasm32")] async fn async_save_dialog( + rrd_version: CrateVersion, file_name: &str, title: &str, messages: &[LogMsg], @@ -1832,6 +1856,7 @@ async fn async_save_dialog( }; let bytes = re_log_encoding::encoder::encode_as_bytes( + rrd_version, re_log_encoding::EncodingOptions::COMPRESSED, messages.iter(), )?; diff --git a/crates/re_viewer/src/saving.rs b/crates/re_viewer/src/saving.rs index 1cd59e2dfeb4..4d2e5f7e08be 100644 --- a/crates/re_viewer/src/saving.rs +++ b/crates/re_viewer/src/saving.rs @@ -61,6 +61,7 @@ pub fn default_blueprint_path(app_id: &ApplicationId) -> anyhow::Result( + version: re_build_info::CrateVersion, path: &std::path::Path, messages: impl Iterator, ) -> anyhow::Result<()> { @@ -71,6 +72,6 @@ pub fn encode_to_file<'a>( .with_context(|| format!("Failed to create file at {path:?}"))?; let encoding_options = re_log_encoding::EncodingOptions::COMPRESSED; - re_log_encoding::encoder::encode(encoding_options, messages, &mut file) + re_log_encoding::encoder::encode(version, encoding_options, messages, &mut file) .context("Message encode") } diff --git a/crates/re_viewer/src/viewer_analytics/event.rs b/crates/re_viewer/src/viewer_analytics/event.rs index 128903c9aa5d..cc5104b844c6 100644 --- a/crates/re_viewer/src/viewer_analytics/event.rs +++ b/crates/re_viewer/src/viewer_analytics/event.rs @@ -44,20 +44,31 @@ pub fn open_recording( entity_db: &re_entity_db::EntityDb, ) -> OpenRecording { let store_info = entity_db.store_info().map(|store_info| { - let application_id = if store_info.is_official_example { - Id::Official(store_info.application_id.0.clone()) + let re_log_types::StoreInfo { + application_id, + store_id, + is_official_example, + store_source, + store_version, + + cloned_from: _, + started: _, + } = store_info; + + let application_id_preprocessed = if *is_official_example { + Id::Official(application_id.0.clone()) } else { - Id::Hashed(Property::from(store_info.application_id.0.clone()).hashed()) + Id::Hashed(Property::from(application_id.0.clone()).hashed()) }; - let recording_id = if store_info.is_official_example { - Id::Official(store_info.store_id.to_string()) + let recording_id_preprocessed = if *is_official_example { + Id::Official(store_id.to_string()) } else { - Id::Hashed(Property::from(store_info.store_id.to_string()).hashed()) + Id::Hashed(Property::from(store_id.to_string()).hashed()) }; use re_log_types::StoreSource as S; - let store_source = match &store_info.store_source { + let store_source_preprocessed = match &store_source { S::Unknown => "unknown".to_owned(), S::CSdk => "c_sdk".to_owned(), S::PythonSdk(_version) => "python_sdk".to_owned(), @@ -72,43 +83,47 @@ pub fn open_recording( S::Other(other) => other.clone(), }; + let store_version_preprocessed = if let Some(store_version) = store_version { + store_version.to_string() + } else { + re_log::debug_once!("store version is undefined for this recording, this is a bug"); + "undefined".to_owned() + }; + // `rust+llvm` and `python` versions are mutually exclusive - let mut rust_version = None; - let mut llvm_version = None; - let mut python_version = None; - match &store_info.store_source { + let mut rust_version_preprocessed = None; + let mut llvm_version_preprocessed = None; + let mut python_version_preprocessed = None; + match &store_source { S::File { .. } => { - rust_version = Some(env!("RE_BUILD_RUSTC_VERSION").to_owned()); - llvm_version = Some(env!("RE_BUILD_LLVM_VERSION").to_owned()); + rust_version_preprocessed = Some(env!("RE_BUILD_RUSTC_VERSION").to_owned()); + llvm_version_preprocessed = Some(env!("RE_BUILD_LLVM_VERSION").to_owned()); } S::RustSdk { rustc_version: rustc, llvm_version: llvm, } => { - rust_version = Some(rustc.to_string()); - llvm_version = Some(llvm.to_string()); + rust_version_preprocessed = Some(rustc.to_string()); + llvm_version_preprocessed = Some(llvm.to_string()); } S::PythonSdk(version) => { - python_version = Some(version.to_string()); + python_version_preprocessed = Some(version.to_string()); } // TODO(andreas): Send C SDK version and set it. S::CSdk | S::Unknown | S::Viewer | S::Other(_) => {} } - let is_official_example = store_info.is_official_example; - let app_id_starts_with_rerun_example = store_info - .application_id - .as_str() - .starts_with("rerun_example"); + let app_id_starts_with_rerun_example = application_id.as_str().starts_with("rerun_example"); StoreInfo { - application_id, - recording_id, - store_source, - rust_version, - llvm_version, - python_version, - is_official_example, + application_id: application_id_preprocessed, + recording_id: recording_id_preprocessed, + store_source: store_source_preprocessed, + store_version: store_version_preprocessed, + rust_version: rust_version_preprocessed, + llvm_version: llvm_version_preprocessed, + python_version: python_version_preprocessed, + is_official_example: app_id_starts_with_rerun_example, app_id_starts_with_rerun_example, } }); diff --git a/crates/rerun/src/run.rs b/crates/rerun/src/run.rs index 1a67a912895c..19062d0c7dbd 100644 --- a/crates/rerun/src/run.rs +++ b/crates/rerun/src/run.rs @@ -527,6 +527,7 @@ impl PrintCommand { let rrd_file = std::fs::File::open(rrd_path)?; let version_policy = re_log_encoding::decoder::VersionPolicy::Warn; let decoder = re_log_encoding::decoder::Decoder::new(version_policy, rrd_file)?; + println!("Decoded RRD stream v{}\n---", decoder.version()); for msg in decoder { let msg = msg.context("decode rrd message")?; match msg { @@ -869,7 +870,11 @@ fn stream_to_rrd_on_disk( let encoding_options = re_log_encoding::EncodingOptions::COMPRESSED; let file = std::fs::File::create(path).map_err(|err| FileSinkError::CreateFile(path.clone(), err))?; - let mut encoder = re_log_encoding::encoder::Encoder::new(encoding_options, file)?; + let mut encoder = re_log_encoding::encoder::Encoder::new( + re_build_info::CrateVersion::LOCAL, + encoding_options, + file, + )?; loop { match rx.recv() {