Add installation_id to panic events (#2642)

Joseph T. Lyons created 2 years ago

We want to add installation_id to the panic events so that we can easily
know if multiple panics are coming from the same person or different
people. 5 panics from one person isn't as bad as 5 panics from 5 people.

[zed.dev pr](https://github.com/zed-industries/zed.dev/pull/343)

Release Notes:

- N/A

Change summary

crates/client/src/telemetry.rs | 42 ++++++-----------------------------
crates/zed/src/main.rs         | 27 ++++++++++++++++++++--
2 files changed, 32 insertions(+), 37 deletions(-)

Detailed changes

crates/client/src/telemetry.rs

@@ -1,5 +1,4 @@
 use crate::{TelemetrySettings, ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL};
-use db::kvp::KEY_VALUE_STORE;
 use gpui::{executor::Background, serde_json, AppContext, Task};
 use lazy_static::lazy_static;
 use parking_lot::Mutex;
@@ -8,7 +7,6 @@ use std::{env, io::Write, mem, path::PathBuf, sync::Arc, time::Duration};
 use tempfile::NamedTempFile;
 use util::http::HttpClient;
 use util::{channel::ReleaseChannel, TryFutureExt};
-use uuid::Uuid;
 
 pub struct Telemetry {
     http_client: Arc<dyn HttpClient>,
@@ -120,39 +118,15 @@ impl Telemetry {
         Some(self.state.lock().log_file.as_ref()?.path().to_path_buf())
     }
 
-    pub fn start(self: &Arc<Self>) {
-        let this = self.clone();
-        self.executor
-            .spawn(
-                async move {
-                    let installation_id =
-                        if let Ok(Some(installation_id)) = KEY_VALUE_STORE.read_kvp("device_id") {
-                            installation_id
-                        } else {
-                            let installation_id = Uuid::new_v4().to_string();
-                            KEY_VALUE_STORE
-                                .write_kvp("device_id".to_string(), installation_id.clone())
-                                .await?;
-                            installation_id
-                        };
-
-                    let installation_id: Arc<str> = installation_id.into();
-                    let mut state = this.state.lock();
-                    state.installation_id = Some(installation_id.clone());
-
-                    let has_clickhouse_events = !state.clickhouse_events_queue.is_empty();
-
-                    drop(state);
-
-                    if has_clickhouse_events {
-                        this.flush_clickhouse_events();
-                    }
+    pub fn start(self: &Arc<Self>, installation_id: Option<String>) {
+        let mut state = self.state.lock();
+        state.installation_id = installation_id.map(|id| id.into());
+        let has_clickhouse_events = !state.clickhouse_events_queue.is_empty();
+        drop(state);
 
-                    anyhow::Ok(())
-                }
-                .log_err(),
-            )
-            .detach();
+        if has_clickhouse_events {
+            self.flush_clickhouse_events();
+        }
     }
 
     /// This method takes the entire TelemetrySettings struct in order to force client code

crates/zed/src/main.rs

@@ -48,6 +48,7 @@ use util::{
     http::{self, HttpClient},
     paths::PathLikeWithPosition,
 };
+use uuid::Uuid;
 use welcome::{show_welcome_experience, FIRST_OPEN};
 
 use fs::RealFs;
@@ -68,7 +69,8 @@ fn main() {
     log::info!("========== starting zed ==========");
     let mut app = gpui::App::new(Assets).unwrap();
 
-    init_panic_hook(&app);
+    let installation_id = app.background().block(installation_id()).ok();
+    init_panic_hook(&app, installation_id.clone());
 
     app.background();
 
@@ -168,7 +170,7 @@ fn main() {
         })
         .detach();
 
-        client.telemetry().start();
+        client.telemetry().start(installation_id);
 
         let app_state = Arc::new(AppState {
             languages,
@@ -268,6 +270,22 @@ fn main() {
     });
 }
 
+async fn installation_id() -> Result<String> {
+    let legacy_key_name = "device_id";
+
+    if let Ok(Some(installation_id)) = KEY_VALUE_STORE.read_kvp(legacy_key_name) {
+        Ok(installation_id)
+    } else {
+        let installation_id = Uuid::new_v4().to_string();
+
+        KEY_VALUE_STORE
+            .write_kvp(legacy_key_name.to_string(), installation_id.clone())
+            .await?;
+
+        Ok(installation_id)
+    }
+}
+
 fn open_urls(
     urls: Vec<String>,
     cli_connections_tx: &mpsc::UnboundedSender<(
@@ -371,6 +389,8 @@ struct Panic {
     os_version: Option<String>,
     architecture: String,
     panicked_on: u128,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    installation_id: Option<String>,
 }
 
 #[derive(Serialize)]
@@ -379,7 +399,7 @@ struct PanicRequest {
     token: String,
 }
 
-fn init_panic_hook(app: &App) {
+fn init_panic_hook(app: &App, installation_id: Option<String>) {
     let is_pty = stdout_is_a_pty();
     let platform = app.platform();
 
@@ -432,6 +452,7 @@ fn init_panic_hook(app: &App) {
                 .unwrap()
                 .as_millis(),
             backtrace,
+            installation_id: installation_id.clone(),
         };
 
         if is_pty {