Rework call events api (#3038)

Joseph T. Lyons created

There were times when events with bad data were being emitted. What we
found was that places where certain collaboration-related code could
fail, like sending an invite, would still send events; those events
would be in a bad state, as certain elements, such as a room, weren't
constructed as expected, causing the event to have missing data. The new
API guarantees that we have data in the correct configuration. In the
future, we will add events for certain types of failures within Zed, to
cover things like invites failing.

Release Notes:

- N/A

Change summary

crates/call/src/call.rs              | 80 +++++++++++++++++------------
crates/collab_ui/src/channel_view.rs | 11 +---
crates/collab_ui/src/collab_ui.rs    | 32 ++++-------
3 files changed, 61 insertions(+), 62 deletions(-)

Detailed changes

crates/call/src/call.rs 🔗

@@ -206,9 +206,14 @@ impl ActiveCall {
 
         cx.spawn(|this, mut cx| async move {
             let result = invite.await;
+            if result.is_ok() {
+                this.update(&mut cx, |this, cx| this.report_call_event("invite", cx));
+            } else {
+                // TODO: Resport collaboration error
+            }
+
             this.update(&mut cx, |this, cx| {
                 this.pending_invites.remove(&called_user_id);
-                this.report_call_event("invite", cx);
                 cx.notify();
             });
             result
@@ -273,13 +278,7 @@ impl ActiveCall {
             .borrow_mut()
             .take()
             .ok_or_else(|| anyhow!("no incoming call"))?;
-        Self::report_call_event_for_room(
-            "decline incoming",
-            Some(call.room_id),
-            None,
-            &self.client,
-            cx,
-        );
+        report_call_event_for_room("decline incoming", call.room_id, None, &self.client, cx);
         self.client.send(proto::DeclineCall {
             room_id: call.room_id,
         })?;
@@ -409,31 +408,46 @@ impl ActiveCall {
         &self.pending_invites
     }
 
-    fn report_call_event(&self, operation: &'static str, cx: &AppContext) {
-        let (room_id, channel_id) = match self.room() {
-            Some(room) => {
-                let room = room.read(cx);
-                (Some(room.id()), room.channel_id())
-            }
-            None => (None, None),
-        };
-        Self::report_call_event_for_room(operation, room_id, channel_id, &self.client, cx)
+    pub fn report_call_event(&self, operation: &'static str, cx: &AppContext) {
+        if let Some(room) = self.room() {
+            let room = room.read(cx);
+            report_call_event_for_room(operation, room.id(), room.channel_id(), &self.client, cx);
+        }
     }
+}
 
-    pub fn report_call_event_for_room(
-        operation: &'static str,
-        room_id: Option<u64>,
-        channel_id: Option<u64>,
-        client: &Arc<Client>,
-        cx: &AppContext,
-    ) {
-        let telemetry = client.telemetry();
-        let telemetry_settings = *settings::get::<TelemetrySettings>(cx);
-        let event = ClickhouseEvent::Call {
-            operation,
-            room_id,
-            channel_id,
-        };
-        telemetry.report_clickhouse_event(event, telemetry_settings);
-    }
+pub fn report_call_event_for_room(
+    operation: &'static str,
+    room_id: u64,
+    channel_id: Option<u64>,
+    client: &Arc<Client>,
+    cx: &AppContext,
+) {
+    let telemetry = client.telemetry();
+    let telemetry_settings = *settings::get::<TelemetrySettings>(cx);
+    let event = ClickhouseEvent::Call {
+        operation,
+        room_id: Some(room_id),
+        channel_id,
+    };
+    telemetry.report_clickhouse_event(event, telemetry_settings);
+}
+
+pub fn report_call_event_for_channel(
+    operation: &'static str,
+    channel_id: u64,
+    client: &Arc<Client>,
+    cx: &AppContext,
+) {
+    let room = ActiveCall::global(cx).read(cx).room();
+
+    let telemetry = client.telemetry();
+    let telemetry_settings = *settings::get::<TelemetrySettings>(cx);
+
+    let event = ClickhouseEvent::Call {
+        operation,
+        room_id: room.map(|r| r.read(cx).id()),
+        channel_id: Some(channel_id),
+    };
+    telemetry.report_clickhouse_event(event, telemetry_settings);
 }

crates/collab_ui/src/channel_view.rs 🔗

@@ -1,5 +1,5 @@
 use anyhow::{anyhow, Result};
-use call::ActiveCall;
+use call::report_call_event_for_channel;
 use channel::{ChannelBuffer, ChannelBufferEvent, ChannelId};
 use client::proto;
 use clock::ReplicaId;
@@ -42,14 +42,9 @@ impl ChannelView {
         cx.spawn(|mut cx| async move {
             let channel_view = channel_view.await?;
             pane.update(&mut cx, |pane, cx| {
-                let room_id = ActiveCall::global(cx)
-                    .read(cx)
-                    .room()
-                    .map(|room| room.read(cx).id());
-                ActiveCall::report_call_event_for_room(
+                report_call_event_for_channel(
                     "open channel notes",
-                    room_id,
-                    Some(channel_id),
+                    channel_id,
                     &workspace.read(cx).app_state().client,
                     cx,
                 );

crates/collab_ui/src/collab_ui.rs 🔗

@@ -10,7 +10,7 @@ mod panel_settings;
 mod project_shared_notification;
 mod sharing_status_indicator;
 
-use call::{ActiveCall, Room};
+use call::{report_call_event_for_room, ActiveCall, Room};
 use gpui::{
     actions,
     geometry::{
@@ -55,18 +55,18 @@ pub fn toggle_screen_sharing(_: &ToggleScreenSharing, cx: &mut AppContext) {
         let client = call.client();
         let toggle_screen_sharing = room.update(cx, |room, cx| {
             if room.is_screen_sharing() {
-                ActiveCall::report_call_event_for_room(
+                report_call_event_for_room(
                     "disable screen share",
-                    Some(room.id()),
+                    room.id(),
                     room.channel_id(),
                     &client,
                     cx,
                 );
                 Task::ready(room.unshare_screen(cx))
             } else {
-                ActiveCall::report_call_event_for_room(
+                report_call_event_for_room(
                     "enable screen share",
-                    Some(room.id()),
+                    room.id(),
                     room.channel_id(),
                     &client,
                     cx,
@@ -83,23 +83,13 @@ pub fn toggle_mute(_: &ToggleMute, cx: &mut AppContext) {
     if let Some(room) = call.room().cloned() {
         let client = call.client();
         room.update(cx, |room, cx| {
-            if room.is_muted(cx) {
-                ActiveCall::report_call_event_for_room(
-                    "enable microphone",
-                    Some(room.id()),
-                    room.channel_id(),
-                    &client,
-                    cx,
-                );
+            let operation = if room.is_muted(cx) {
+                "enable microphone"
             } else {
-                ActiveCall::report_call_event_for_room(
-                    "disable microphone",
-                    Some(room.id()),
-                    room.channel_id(),
-                    &client,
-                    cx,
-                );
-            }
+                "disable microphone"
+            };
+            report_call_event_for_room(operation, room.id(), room.channel_id(), &client, cx);
+
             room.toggle_mute(cx)
         })
         .map(|task| task.detach_and_log_err(cx))