From a8741dc3107a205ba5a28aa8c3e18747ceed2ba3 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Mon, 3 Feb 2025 11:38:45 -0500 Subject: [PATCH] Migrate more events to telemetry::event! macro (#24102) I believe this takes care of the remaining events running through the old flow that requires transformation at the collab server level. I think all events are now going through `telemetry::event!()`. For anyone curious where the new telemetry names are coming from, you can check the `for_snowflake` function within `crates/collab/src/api/events.rs`, to see how collab is currently transforming the events going through the old flow. Release Notes: - N/A --- Cargo.lock | 2 + crates/client/src/telemetry.rs | 102 +++++++----------- crates/collab/src/api/events.rs | 4 + crates/diagnostics/src/diagnostics.rs | 2 +- crates/editor/src/git/project_diff.rs | 2 +- crates/extensions_ui/src/extensions_ui.rs | 2 +- .../src/markdown_preview_view.rs | 2 +- crates/recent_projects/Cargo.toml | 7 +- crates/recent_projects/src/remote_servers.rs | 23 +--- crates/repl/src/repl_sessions_ui.rs | 2 +- crates/search/src/project_search.rs | 2 +- crates/welcome/src/welcome.rs | 56 ++++------ crates/workspace/Cargo.toml | 7 +- crates/workspace/src/workspace.rs | 12 +-- crates/zed/src/main.rs | 8 +- crates/zed/src/zed.rs | 5 +- 16 files changed, 88 insertions(+), 150 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 24a4524c07..622105ed60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10585,6 +10585,7 @@ dependencies = [ "settings", "smol", "task", + "telemetry", "theme", "ui", "util", @@ -16042,6 +16043,7 @@ dependencies = [ "sqlez", "strum", "task", + "telemetry", "tempfile", "theme", "ui", diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 56ed6e6c03..b652b42599 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -3,7 +3,6 @@ mod event_coalescer; use crate::TelemetrySettings; use anyhow::Result; use clock::SystemClock; -use collections::{HashMap, HashSet}; use futures::channel::mpsc; use futures::{Future, StreamExt}; use gpui::{App, BackgroundExecutor, Task}; @@ -12,14 +11,13 @@ use parking_lot::Mutex; use release_channel::ReleaseChannel; use settings::{Settings, SettingsStore}; use sha2::{Digest, Sha256}; +use std::collections::{HashMap, HashSet}; use std::fs::File; use std::io::Write; use std::sync::LazyLock; use std::time::Instant; use std::{env, mem, path::PathBuf, sync::Arc, time::Duration}; -use telemetry_events::{ - AppEvent, AssistantEvent, AssistantPhase, EditEvent, Event, EventRequestBody, EventWrapper, -}; +use telemetry_events::{AssistantEvent, AssistantPhase, Event, EventRequestBody, EventWrapper}; use util::{ResultExt, TryFutureExt}; use worktree::{UpdatedEntriesSet, WorktreeId}; @@ -285,7 +283,7 @@ impl Telemetry { // TestAppContext ends up calling this function on shutdown and it panics when trying to find the TelemetrySettings #[cfg(not(any(test, feature = "test-support")))] fn shutdown_telemetry(self: &Arc) -> impl Future { - self.report_app_event("close".to_string()); + telemetry::event!("App Closed"); // TODO: close final edit period and make sure it's sent Task::ready(()) } @@ -355,30 +353,23 @@ impl Telemetry { ); } - pub fn report_app_event(self: &Arc, operation: String) -> Event { - let event = Event::App(AppEvent { operation }); - - self.report_event(event.clone()); - - event - } - pub fn log_edit_event(self: &Arc, environment: &'static str, is_via_ssh: bool) { let mut state = self.state.lock(); let period_data = state.event_coalescer.log_event(environment); drop(state); if let Some((start, end, environment)) = period_data { - let event = Event::Edit(EditEvent { - duration: end - .saturating_duration_since(start) - .min(Duration::from_secs(60 * 60 * 24)) - .as_millis() as i64, - environment: environment.to_string(), - is_via_ssh, - }); + let duration_milliseconds = end + .saturating_duration_since(start) + .min(Duration::from_secs(60 * 60 * 24)) + .as_millis() as i64; - self.report_event(event); + telemetry::event!( + "Editor Edited", + duration_milliseconds = duration_milliseconds, + environment = environment.to_string(), + is_via_ssh = is_via_ssh + ); } } @@ -422,9 +413,8 @@ impl Telemetry { .collect() }; - // Done on purpose to avoid calling `self.state.lock()` multiple times for project_type_name in project_type_names { - self.report_app_event(format!("open {} project", project_type_name)); + telemetry::event!("Project Opened", project_type = project_type_name); } } @@ -590,6 +580,7 @@ mod tests { use clock::FakeSystemClock; use gpui::TestAppContext; use http_client::FakeHttpClient; + use telemetry_events::FlexibleEvent; #[gpui::test] fn test_telemetry_flush_on_max_queue_size(cx: &mut TestAppContext) { @@ -609,15 +600,17 @@ mod tests { assert!(is_empty_state(&telemetry)); let first_date_time = clock.utc_now(); - let operation = "test".to_string(); + let event_properties = HashMap::from_iter([( + "test_key".to_string(), + serde_json::Value::String("test_value".to_string()), + )]); - let event = telemetry.report_app_event(operation.clone()); - assert_eq!( - event, - Event::App(AppEvent { - operation: operation.clone(), - }) - ); + let event = FlexibleEvent { + event_type: "test".to_string(), + event_properties, + }; + + telemetry.report_event(Event::Flexible(event.clone())); assert_eq!(telemetry.state.lock().events_queue.len(), 1); assert!(telemetry.state.lock().flush_events_task.is_some()); assert_eq!( @@ -627,13 +620,7 @@ mod tests { clock.advance(Duration::from_millis(100)); - let event = telemetry.report_app_event(operation.clone()); - assert_eq!( - event, - Event::App(AppEvent { - operation: operation.clone(), - }) - ); + telemetry.report_event(Event::Flexible(event.clone())); assert_eq!(telemetry.state.lock().events_queue.len(), 2); assert!(telemetry.state.lock().flush_events_task.is_some()); assert_eq!( @@ -643,13 +630,7 @@ mod tests { clock.advance(Duration::from_millis(100)); - let event = telemetry.report_app_event(operation.clone()); - assert_eq!( - event, - Event::App(AppEvent { - operation: operation.clone(), - }) - ); + telemetry.report_event(Event::Flexible(event.clone())); assert_eq!(telemetry.state.lock().events_queue.len(), 3); assert!(telemetry.state.lock().flush_events_task.is_some()); assert_eq!( @@ -660,14 +641,7 @@ mod tests { clock.advance(Duration::from_millis(100)); // Adding a 4th event should cause a flush - let event = telemetry.report_app_event(operation.clone()); - assert_eq!( - event, - Event::App(AppEvent { - operation: operation.clone(), - }) - ); - + telemetry.report_event(Event::Flexible(event)); assert!(is_empty_state(&telemetry)); }); } @@ -690,17 +664,19 @@ mod tests { telemetry.start(system_id, installation_id, session_id, cx); assert!(is_empty_state(&telemetry)); - let first_date_time = clock.utc_now(); - let operation = "test".to_string(); - let event = telemetry.report_app_event(operation.clone()); - assert_eq!( - event, - Event::App(AppEvent { - operation: operation.clone(), - }) - ); + let event_properties = HashMap::from_iter([( + "test_key".to_string(), + serde_json::Value::String("test_value".to_string()), + )]); + + let event = FlexibleEvent { + event_type: "test".to_string(), + event_properties, + }; + + telemetry.report_event(Event::Flexible(event)); assert_eq!(telemetry.state.lock().events_queue.len(), 1); assert!(telemetry.state.lock().flush_events_task.is_some()); assert_eq!( diff --git a/crates/collab/src/api/events.rs b/crates/collab/src/api/events.rs index 5e1593c207..5854db3990 100644 --- a/crates/collab/src/api/events.rs +++ b/crates/collab/src/api/events.rs @@ -495,6 +495,10 @@ fn for_snowflake( body.events.into_iter().flat_map(move |event| { let timestamp = first_event_at + Duration::milliseconds(event.milliseconds_since_first_event); + // We will need to double check, but I believe all of the events that + // are being transformed here are now migrated over to use the + // telemetry::event! macro, as of this commit so this code can go away + // when we feel enough users have upgraded past this point. let (event_type, mut event_properties) = match &event.event { Event::Editor(e) => ( match e.operation.as_str() { diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index d9bbb9dae9..586e82bf8f 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -750,7 +750,7 @@ impl Item for ProjectDiagnosticsEditor { } fn telemetry_event_text(&self) -> Option<&'static str> { - Some("project diagnostics") + Some("Project Diagnostics Opened") } fn for_each_project_item( diff --git a/crates/editor/src/git/project_diff.rs b/crates/editor/src/git/project_diff.rs index 6bc12c1782..3a867e5967 100644 --- a/crates/editor/src/git/project_diff.rs +++ b/crates/editor/src/git/project_diff.rs @@ -995,7 +995,7 @@ impl Item for ProjectDiffEditor { } fn telemetry_event_text(&self) -> Option<&'static str> { - Some("project diagnostics") + Some("Project Diagnostics Opened") } fn for_each_project_item( diff --git a/crates/extensions_ui/src/extensions_ui.rs b/crates/extensions_ui/src/extensions_ui.rs index 99c0ace156..15fee8e630 100644 --- a/crates/extensions_ui/src/extensions_ui.rs +++ b/crates/extensions_ui/src/extensions_ui.rs @@ -1207,7 +1207,7 @@ impl Item for ExtensionsPage { } fn telemetry_event_text(&self) -> Option<&'static str> { - Some("extensions page") + Some("Extensions Page Opened") } fn show_toolbar(&self) -> bool { diff --git a/crates/markdown_preview/src/markdown_preview_view.rs b/crates/markdown_preview/src/markdown_preview_view.rs index 886a0b4d26..382a42f19c 100644 --- a/crates/markdown_preview/src/markdown_preview_view.rs +++ b/crates/markdown_preview/src/markdown_preview_view.rs @@ -497,7 +497,7 @@ impl Item for MarkdownPreviewView { } fn telemetry_event_text(&self) -> Option<&'static str> { - Some("markdown preview") + Some("Markdown Preview Opened") } fn to_item_events(_event: &Self::Event, _f: impl FnMut(workspace::item::ItemEvent)) {} diff --git a/crates/recent_projects/Cargo.toml b/crates/recent_projects/Cargo.toml index c74c51be46..fa611e958a 100644 --- a/crates/recent_projects/Cargo.toml +++ b/crates/recent_projects/Cargo.toml @@ -15,31 +15,32 @@ doctest = false [dependencies] anyhow.workspace = true auto_update.workspace = true -release_channel.workspace = true editor.workspace = true extension_host.workspace = true file_finder.workspace = true futures.workspace = true fuzzy.workspace = true gpui.workspace = true -log.workspace = true language.workspace = true +log.workspace = true markdown.workspace = true menu.workspace = true ordered-float.workspace = true +paths.workspace = true picker.workspace = true project.workspace = true +release_channel.workspace = true remote.workspace = true schemars.workspace = true serde.workspace = true settings.workspace = true smol.workspace = true task.workspace = true +telemetry.workspace = true theme.workspace = true ui.workspace = true util.workspace = true workspace.workspace = true -paths.workspace = true zed_actions.workspace = true [dev-dependencies] diff --git a/crates/recent_projects/src/remote_servers.rs b/crates/recent_projects/src/remote_servers.rs index 418a7844ee..fc3e8bb17a 100644 --- a/crates/recent_projects/src/remote_servers.rs +++ b/crates/recent_projects/src/remote_servers.rs @@ -201,20 +201,8 @@ impl ProjectPicker { }); cx.new(|cx| { - let workspace = Workspace::new( - None, - project.clone(), - app_state.clone(), - window, - cx, - ); - - workspace - .client() - .telemetry() - .report_app_event("create ssh project".to_string()); - - workspace + telemetry::event!("SSH Project Created"); + Workspace::new(None, project.clone(), app_state.clone(), window, cx) }) }) .log_err(); @@ -420,12 +408,7 @@ impl RemoteServerProjects { match connection.await { Some(Some(client)) => this .update(&mut cx, |this, cx| { - let _ = this.workspace.update(cx, |workspace, _| { - workspace - .client() - .telemetry() - .report_app_event("create ssh server".to_string()) - }); + telemetry::event!("SSH Server Created"); this.retained_connections.push(client); this.add_ssh_server(connection_options, cx); this.mode = Mode::default_mode(cx); diff --git a/crates/repl/src/repl_sessions_ui.rs b/crates/repl/src/repl_sessions_ui.rs index 480958976a..b878c65921 100644 --- a/crates/repl/src/repl_sessions_ui.rs +++ b/crates/repl/src/repl_sessions_ui.rs @@ -183,7 +183,7 @@ impl Item for ReplSessionsPage { } fn telemetry_event_text(&self) -> Option<&'static str> { - Some("repl sessions") + Some("REPL Session Started") } fn show_toolbar(&self) -> bool { diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 130531b732..35f872d0e8 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -453,7 +453,7 @@ impl Item for ProjectSearchView { } fn telemetry_event_text(&self) -> Option<&'static str> { - Some("project search") + Some("Project Search Opened") } fn for_each_project_item( diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index 399da0efa7..1346138aa7 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -132,9 +132,7 @@ impl Render for WelcomePage { .icon_color(Color::Muted) .icon_position(IconPosition::Start) .on_click(cx.listener(|this, _, window, cx| { - this.telemetry.report_app_event( - "welcome page: change theme".to_string(), - ); + telemetry::event!("Welcome Theme Changed"); this.workspace .update(cx, |_workspace, cx| { window.dispatch_action(zed_actions::theme_selector::Toggle::default().boxed_clone(), cx); @@ -149,9 +147,7 @@ impl Render for WelcomePage { .icon_color(Color::Muted) .icon_position(IconPosition::Start) .on_click(cx.listener(|this, _, window, cx| { - this.telemetry.report_app_event( - "welcome page: change keymap".to_string(), - ); + telemetry::event!("Welcome Keymap Changed"); this.workspace .update(cx, |workspace, cx| { base_keymap_picker::toggle( @@ -173,10 +169,8 @@ impl Render for WelcomePage { .icon_color(Color::Muted) .icon_position(IconPosition::Start) .on_click( - cx.listener(|this, _, window, cx| { - this.telemetry.report_app_event( - "welcome page: sign in to copilot".to_string(), - ); + cx.listener(|_, _, window, cx| { + telemetry::event!("Welcome Copilot Signed In"); copilot::initiate_sign_in(window, cx); }), ), @@ -187,10 +181,8 @@ impl Render for WelcomePage { .icon_size(IconSize::XSmall) .icon_color(Color::Muted) .icon_position(IconPosition::Start) - .on_click(cx.listener(|this, _, window, cx| { - this.telemetry.report_app_event( - "welcome page: edit settings".to_string(), - ); + .on_click(cx.listener(|_, _, window, cx| { + telemetry::event!("Welcome Settings Edited"); window.dispatch_action(Box::new( zed_actions::OpenSettings, ), cx); @@ -214,10 +206,8 @@ impl Render for WelcomePage { .icon_size(IconSize::XSmall) .icon_color(Color::Muted) .icon_position(IconPosition::Start) - .on_click(cx.listener(|this, _, _, cx| { - this.telemetry.report_app_event( - "welcome page: install cli".to_string(), - ); + .on_click(cx.listener(|_, _, _, cx| { + telemetry::event!("Welcome CLI Installed"); cx .spawn(|_, cx| async move { install_cli::install_cli(&cx).await @@ -232,10 +222,8 @@ impl Render for WelcomePage { .icon_size(IconSize::XSmall) .icon_color(Color::Muted) .icon_position(IconPosition::Start) - .on_click(cx.listener(|this, _, _, cx| { - this.telemetry.report_app_event( - "welcome page: view docs".to_string(), - ); + .on_click(cx.listener(|_, _, _, cx| { + telemetry::event!("Welcome Documentation Viewed"); cx.open_url(DOCS_URL); })), ) @@ -245,10 +233,8 @@ impl Render for WelcomePage { .icon_size(IconSize::XSmall) .icon_color(Color::Muted) .icon_position(IconPosition::Start) - .on_click(cx.listener(|this, _, window, cx| { - this.telemetry.report_app_event( - "welcome page: open extensions".to_string(), - ); + .on_click(cx.listener(|_, _, window, cx| { + telemetry::event!("Welcome Extensions Page Opened"); window.dispatch_action(Box::new( zed_actions::Extensions, ), cx); @@ -282,8 +268,7 @@ impl Render for WelcomePage { ui::ToggleState::Unselected }, cx.listener(move |this, selection, _window, cx| { - this.telemetry - .report_app_event("welcome page: toggle vim".to_string()); + telemetry::event!("Welcome Vim Mode Toggled"); this.update_settings::( selection, cx, @@ -314,9 +299,7 @@ impl Render for WelcomePage { ui::ToggleState::Unselected }, cx.listener(move |this, selection, _window, cx| { - this.telemetry.report_app_event( - "welcome page: toggle diagnostic telemetry".to_string(), - ); + telemetry::event!("Welcome Diagnostic Telemetry Toggled"); this.update_settings::(selection, cx, { move |settings, value| { settings.diagnostics = Some(value); @@ -342,9 +325,7 @@ impl Render for WelcomePage { ui::ToggleState::Unselected }, cx.listener(move |this, selection, _window, cx| { - this.telemetry.report_app_event( - "welcome page: toggle metric telemetry".to_string(), - ); + telemetry::event!("Welcome Metric Telemetry Toggled"); this.update_settings::(selection, cx, { move |settings, value| { settings.metrics = Some(value); @@ -368,9 +349,8 @@ impl Render for WelcomePage { impl WelcomePage { pub fn new(workspace: &Workspace, cx: &mut Context) -> Entity { let this = cx.new(|cx| { - cx.on_release(|this: &mut Self, _| { - this.telemetry - .report_app_event("welcome page: close".to_string()); + cx.on_release(|_: &mut Self, _| { + telemetry::event!("Welcome Page Closed"); }) .detach(); @@ -431,7 +411,7 @@ impl Item for WelcomePage { } fn telemetry_event_text(&self) -> Option<&'static str> { - Some("welcome page") + Some("Welcome Page Opened") } fn show_toolbar(&self) -> bool { diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index 29445ef987..81bd40970e 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -26,8 +26,8 @@ test-support = [ ] [dependencies] -anyhow.workspace = true any_vec.workspace = true +anyhow.workspace = true async-recursion.workspace = true bincode = "1.2.1" call.workspace = true @@ -47,7 +47,6 @@ node_runtime.workspace = true parking_lot.workspace = true postage.workspace = true project.workspace = true -task.workspace = true remote.workspace = true schemars.workspace = true serde.workspace = true @@ -56,11 +55,13 @@ session.workspace = true settings.workspace = true smallvec.workspace = true sqlez.workspace = true +strum.workspace = true +task.workspace = true +telemetry.workspace = true theme.workspace = true ui.workspace = true util.workspace = true uuid.workspace = true -strum.workspace = true [dev-dependencies] call = { workspace = true, features = ["test-support"] } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index d052f55892..2a5f5c74b0 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2707,9 +2707,7 @@ impl Workspace { cx: &mut App, ) { if let Some(text) = item.telemetry_event_text(cx) { - self.client() - .telemetry() - .report_app_event(format!("{}: open", text)); + telemetry::event!(text); } pane.update(cx, |pane, cx| { @@ -6165,14 +6163,10 @@ pub fn open_ssh_project( cx.update_window(window.into(), |_, window, cx| { window.replace_root(cx, |window, cx| { + telemetry::event!("SSH Project Opened"); + let mut workspace = Workspace::new(Some(workspace_id), project, app_state.clone(), window, cx); - - workspace - .client() - .telemetry() - .report_app_event("open ssh project".to_string()); - workspace.set_serialized_ssh_project(serialized_ssh_project); workspace }); diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 881d256f5b..bd0275cb45 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -376,14 +376,14 @@ fn main() { if let (Some(system_id), Some(installation_id)) = (&system_id, &installation_id) { match (&system_id, &installation_id) { (IdType::New(_), IdType::New(_)) => { - telemetry.report_app_event("first open".to_string()); - telemetry.report_app_event("first open for release channel".to_string()); + telemetry::event!("App First Opened"); + telemetry::event!("App First Opened For Release Channel"); } (IdType::Existing(_), IdType::New(_)) => { - telemetry.report_app_event("first open for release channel".to_string()); + telemetry::event!("App First Opened For Release Channel"); } (_, IdType::Existing(_)) => { - telemetry.report_app_event("open".to_string()); + telemetry::event!("App Opened"); } } } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 5d90d582c9..5e0bbef48e 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -512,10 +512,7 @@ fn register_actions( }) .register_action(|_, action: &OpenBrowser, _window, cx| cx.open_url(&action.url)) .register_action(|workspace, _: &workspace::Open, window, cx| { - workspace - .client() - .telemetry() - .report_app_event("open project".to_string()); + telemetry::event!("Project Opened"); let paths = workspace.prompt_for_open_path( PathPromptOptions { files: true,