From 2aaa709b0eac327c2c42f7680192226dc092f58e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 22 Nov 2023 13:04:36 +0000 Subject: [PATCH] Reinstate tracing instrumentation on `ReadOnlyDevice::encrypt` (#2873) Until https://github.com/matrix-org/matrix-rust-sdk/pull/2862, `GroupSessionManager::encrypt_session_for` called `Device::encrypt` (via `Device::maybe_encrypt_room_key`. `Device::encrypt` is a thin wrapper for `ReadOnlyDevice::encrypt`, but it also has an `instrument` annotation. https://github.com/matrix-org/matrix-rust-sdk/pull/2862 short-cuts `Device::encrypt` and calls `ReadOnlyDevice::encrypt` directly: that was functionally fine but of course means that we no longer benefit from the `instrument` annotation. This PR rectifies the situation by pushing the annotation down to `ReadOnlyDevice::encrypt`. It also adds some documentation for that function, since we are using it in more places now. (Longer-term, I think we should probably aim to get rid of `Device::encrypt` altogether, but that's a refactor I don't want to take on today.) --- .../src/identities/device.rs | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/identities/device.rs b/crates/matrix-sdk-crypto/src/identities/device.rs index 893850c15..7f307a80e 100644 --- a/crates/matrix-sdk-crypto/src/identities/device.rs +++ b/crates/matrix-sdk-crypto/src/identities/device.rs @@ -412,17 +412,6 @@ impl Device { /// # Arguments /// /// * `content` - The content of the event that should be encrypted. - #[instrument( - skip_all, - fields( - recipient = ?self.user_id(), - recipient_device = ?self.device_id(), - recipient_key = ?self.curve25519_key(), - event_type, - session, - message_id, - )) - ] pub(crate) async fn encrypt( &self, event_type: &str, @@ -731,6 +720,34 @@ impl ReadOnlyDevice { ) } + /// Encrypt the given content for this device. + /// + /// # Arguments + /// + /// * `store` - The crypto store. Used to find an established Olm session + /// for this device. + /// * `event_type` - The type of the event that should be encrypted. + /// * `content` - The content of the event that should be encrypted. + /// + /// # Returns + /// + /// On success, a tuple `(session, content)`, where `session` is the Olm + /// [`Session`] that was used to encrypt the content, and `content` is + /// the content for the `m.room.encrypted` to-device event. + /// + /// If an Olm session has not already been established with this device, + /// returns `Err(OlmError::MissingSession)`. + #[instrument( + skip_all, + fields( + recipient = ?self.user_id(), + recipient_device = ?self.device_id(), + recipient_key = ?self.curve25519_key(), + event_type, + session, + message_id, + )) + ] pub(crate) async fn encrypt( &self, store: &CryptoStoreWrapper,