From cdd5cb16ed896b2ee3bbb041983ee7cb812f6991 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Sat, 6 Jan 2024 14:41:35 -0500 Subject: [PATCH 1/8] WIP --- crates/assistant/src/assistant_panel.rs | 2 +- crates/call/src/call.rs | 16 +- crates/client/src/client.rs | 3 +- crates/client/src/telemetry.rs | 123 +++--- crates/client/src/user.rs | 1 - crates/collab_ui/src/collab_ui.rs | 4 +- crates/editor/src/editor.rs | 3 +- crates/editor/src/items.rs | 1 + crates/theme_selector/src/theme_selector.rs | 2 +- crates/welcome/src/base_keymap_picker.rs | 11 +- crates/welcome/src/base_keymap_setting.rs | 14 + crates/welcome/src/welcome.rs | 400 ++++++++++++-------- crates/workspace/src/workspace.rs | 2 +- crates/zed/src/main.rs | 24 +- 14 files changed, 348 insertions(+), 258 deletions(-) diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index 385c6f5239..6e2ab02cad 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -3542,5 +3542,5 @@ fn report_assistant_event( .default_open_ai_model .clone(); - telemetry.report_assistant_event(conversation_id, assistant_kind, model.full_name(), cx) + telemetry.report_assistant_event(conversation_id, assistant_kind, model.full_name()) } diff --git a/crates/call/src/call.rs b/crates/call/src/call.rs index c419043a72..4f9ec08059 100644 --- a/crates/call/src/call.rs +++ b/crates/call/src/call.rs @@ -310,14 +310,14 @@ impl ActiveCall { }) } - pub fn decline_incoming(&mut self, cx: &mut ModelContext) -> Result<()> { + pub fn decline_incoming(&mut self, _cx: &mut ModelContext) -> Result<()> { let call = self .incoming_call .0 .borrow_mut() .take() .ok_or_else(|| anyhow!("no incoming call"))?; - report_call_event_for_room("decline incoming", call.room_id, None, &self.client, cx); + report_call_event_for_room("decline incoming", call.room_id, None, &self.client); self.client.send(proto::DeclineCall { room_id: call.room_id, })?; @@ -467,7 +467,7 @@ impl ActiveCall { pub fn report_call_event(&self, operation: &'static str, cx: &mut 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); + report_call_event_for_room(operation, room.id(), room.channel_id(), &self.client); } } } @@ -477,11 +477,10 @@ pub fn report_call_event_for_room( room_id: u64, channel_id: Option, client: &Arc, - cx: &mut AppContext, ) { let telemetry = client.telemetry(); - telemetry.report_call_event(operation, Some(room_id), channel_id, cx) + telemetry.report_call_event(operation, Some(room_id), channel_id) } pub fn report_call_event_for_channel( @@ -494,12 +493,7 @@ pub fn report_call_event_for_channel( let telemetry = client.telemetry(); - telemetry.report_call_event( - operation, - room.map(|r| r.read(cx).id()), - Some(channel_id), - cx, - ) + telemetry.report_call_event(operation, room.map(|r| r.read(cx).id()), Some(channel_id)) } #[cfg(test)] diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 2f1e234b73..d070cae375 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -501,8 +501,7 @@ impl Client { })); } Status::SignedOut | Status::UpgradeRequired => { - cx.update(|cx| self.telemetry.set_authenticated_user_info(None, false, cx)) - .log_err(); + self.telemetry.set_authenticated_user_info(None, false); state._reconnect_task.take(); } _ => {} diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 2391c5f3b5..6b6b4b0a08 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -5,7 +5,7 @@ use gpui::{serde_json, AppContext, AppMetadata, BackgroundExecutor, Task}; use lazy_static::lazy_static; use parking_lot::Mutex; use serde::Serialize; -use settings::Settings; +use settings::{Settings, SettingsStore}; use std::{env, io::Write, mem, path::PathBuf, sync::Arc, time::Duration}; use sysinfo::{ CpuRefreshKind, Pid, PidExt, ProcessExt, ProcessRefreshKind, RefreshKind, System, SystemExt, @@ -17,10 +17,11 @@ use util::{channel::ReleaseChannel, TryFutureExt}; pub struct Telemetry { http_client: Arc, executor: BackgroundExecutor, - state: Mutex, + state: Arc>, } struct TelemetryState { + settings: TelemetrySettings, metrics_id: Option>, // Per logged-in user installation_id: Option>, // Per app installation (different for dev, nightly, preview, and stable) session_id: Option>, // Per app launch @@ -139,45 +140,60 @@ impl Telemetry { None }; + TelemetrySettings::register(cx); + + let state = Arc::new(Mutex::new(TelemetryState { + settings: TelemetrySettings::get_global(cx).clone(), + app_metadata: cx.app_metadata(), + architecture: env::consts::ARCH, + release_channel, + installation_id: None, + metrics_id: None, + session_id: None, + clickhouse_events_queue: Default::default(), + flush_clickhouse_events_task: Default::default(), + log_file: None, + is_staff: None, + first_event_datetime: None, + })); + + cx.observe_global::({ + let state = state.clone(); + + move |cx| { + let mut state = state.lock(); + state.settings = TelemetrySettings::get_global(cx).clone(); + } + }) + .detach(); + // TODO: Replace all hardware stuff with nested SystemSpecs json let this = Arc::new(Self { http_client: client, executor: cx.background_executor().clone(), - state: Mutex::new(TelemetryState { - app_metadata: cx.app_metadata(), - architecture: env::consts::ARCH, - release_channel, - installation_id: None, - metrics_id: None, - session_id: None, - clickhouse_events_queue: Default::default(), - flush_clickhouse_events_task: Default::default(), - log_file: None, - is_staff: None, - first_event_datetime: None, - }), + state, }); // We should only ever have one instance of Telemetry, leak the subscription to keep it alive // rather than store in TelemetryState, complicating spawn as subscriptions are not Send std::mem::forget(cx.on_app_quit({ let this = this.clone(); - move |cx| this.shutdown_telemetry(cx) + move |_| this.shutdown_telemetry() })); this } #[cfg(any(test, feature = "test-support"))] - fn shutdown_telemetry(self: &Arc, _: &mut AppContext) -> impl Future { + fn shutdown_telemetry(self: &Arc) -> impl Future { Task::ready(()) } // Skip calling this function in tests. // 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, cx: &mut AppContext) -> impl Future { - self.report_app_event("close", true, cx); + fn shutdown_telemetry(self: &Arc) -> impl Future { + self.report_app_event("close", true); Task::ready(()) } @@ -197,7 +213,7 @@ impl Telemetry { drop(state); let this = self.clone(); - cx.spawn(|cx| async move { + cx.spawn(|_cx| async move { // Avoiding calling `System::new_all()`, as there have been crashes related to it let refresh_kind = RefreshKind::new() .with_memory() // For memory usage @@ -226,11 +242,8 @@ impl Telemetry { return; }; - cx.update(|cx| { - this.report_memory_event(process.memory(), process.virtual_memory(), cx); - this.report_cpu_event(process.cpu_usage(), system.cpus().len() as u32, cx); - }) - .ok(); + this.report_memory_event(process.memory(), process.virtual_memory()); + this.report_cpu_event(process.cpu_usage(), system.cpus().len() as u32); } }) .detach(); @@ -240,13 +253,13 @@ impl Telemetry { self: &Arc, metrics_id: Option, is_staff: bool, - cx: &AppContext, ) { - if !TelemetrySettings::get_global(cx).metrics { + let mut state = self.state.lock(); + + if !state.settings.metrics { return; } - let mut state = self.state.lock(); let metrics_id: Option> = metrics_id.map(|id| id.into()); state.metrics_id = metrics_id.clone(); state.is_staff = Some(is_staff); @@ -260,7 +273,6 @@ impl Telemetry { operation: &'static str, copilot_enabled: bool, copilot_enabled_for_language: bool, - cx: &AppContext, ) { let event = ClickhouseEvent::Editor { file_extension, @@ -271,7 +283,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false, cx) + self.report_clickhouse_event(event, false) } pub fn report_copilot_event( @@ -279,7 +291,6 @@ impl Telemetry { suggestion_id: Option, suggestion_accepted: bool, file_extension: Option, - cx: &AppContext, ) { let event = ClickhouseEvent::Copilot { suggestion_id, @@ -288,7 +299,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false, cx) + self.report_clickhouse_event(event, false) } pub fn report_assistant_event( @@ -296,7 +307,6 @@ impl Telemetry { conversation_id: Option, kind: AssistantKind, model: &'static str, - cx: &AppContext, ) { let event = ClickhouseEvent::Assistant { conversation_id, @@ -305,7 +315,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false, cx) + self.report_clickhouse_event(event, false) } pub fn report_call_event( @@ -313,7 +323,6 @@ impl Telemetry { operation: &'static str, room_id: Option, channel_id: Option, - cx: &AppContext, ) { let event = ClickhouseEvent::Call { operation, @@ -322,29 +331,23 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false, cx) + self.report_clickhouse_event(event, false) } - pub fn report_cpu_event( - self: &Arc, - usage_as_percentage: f32, - core_count: u32, - cx: &AppContext, - ) { + pub fn report_cpu_event(self: &Arc, usage_as_percentage: f32, core_count: u32) { let event = ClickhouseEvent::Cpu { usage_as_percentage, core_count, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false, cx) + self.report_clickhouse_event(event, false) } pub fn report_memory_event( self: &Arc, memory_in_bytes: u64, virtual_memory_in_bytes: u64, - cx: &AppContext, ) { let event = ClickhouseEvent::Memory { memory_in_bytes, @@ -352,36 +355,26 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false, cx) + self.report_clickhouse_event(event, false) } - pub fn report_app_event( - self: &Arc, - operation: &'static str, - immediate_flush: bool, - cx: &AppContext, - ) { + pub fn report_app_event(self: &Arc, operation: &'static str, immediate_flush: bool) { let event = ClickhouseEvent::App { operation, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, immediate_flush, cx) + self.report_clickhouse_event(event, immediate_flush) } - pub fn report_setting_event( - self: &Arc, - setting: &'static str, - value: String, - cx: &AppContext, - ) { + pub fn report_setting_event(self: &Arc, setting: &'static str, value: String) { let event = ClickhouseEvent::Setting { setting, value, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false, cx) + self.report_clickhouse_event(event, false) } fn milliseconds_since_first_event(&self) -> i64 { @@ -398,17 +391,13 @@ impl Telemetry { } } - fn report_clickhouse_event( - self: &Arc, - event: ClickhouseEvent, - immediate_flush: bool, - cx: &AppContext, - ) { - if !TelemetrySettings::get_global(cx).metrics { + fn report_clickhouse_event(self: &Arc, event: ClickhouseEvent, immediate_flush: bool) { + let mut state = self.state.lock(); + + if !state.settings.metrics { return; } - let mut state = self.state.lock(); let signed_in = state.metrics_id.is_some(); state .clickhouse_events_queue diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs index b08d423cae..1c288c875d 100644 --- a/crates/client/src/user.rs +++ b/crates/client/src/user.rs @@ -164,7 +164,6 @@ impl UserStore { client.telemetry.set_authenticated_user_info( Some(info.metrics_id.clone()), info.staff, - cx, ) } })?; diff --git a/crates/collab_ui/src/collab_ui.rs b/crates/collab_ui/src/collab_ui.rs index 6b81998a8a..3c0473e67d 100644 --- a/crates/collab_ui/src/collab_ui.rs +++ b/crates/collab_ui/src/collab_ui.rs @@ -58,7 +58,6 @@ pub fn toggle_screen_sharing(_: &ToggleScreenSharing, cx: &mut AppContext) { room.id(), room.channel_id(), &client, - cx, ); Task::ready(room.unshare_screen(cx)) } else { @@ -67,7 +66,6 @@ pub fn toggle_screen_sharing(_: &ToggleScreenSharing, cx: &mut AppContext) { room.id(), room.channel_id(), &client, - cx, ); room.share_screen(cx) } @@ -86,7 +84,7 @@ pub fn toggle_mute(_: &ToggleMute, cx: &mut AppContext) { } else { "disable microphone" }; - report_call_event_for_room(operation, room.id(), room.channel_id(), &client, cx); + report_call_event_for_room(operation, room.id(), room.channel_id(), &client); room.toggle_mute(cx) }) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 455db1d715..231f76218a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -8874,7 +8874,7 @@ impl Editor { let telemetry = project.read(cx).client().telemetry().clone(); - telemetry.report_copilot_event(suggestion_id, suggestion_accepted, file_extension, cx) + telemetry.report_copilot_event(suggestion_id, suggestion_accepted, file_extension) } #[cfg(any(test, feature = "test-support"))] @@ -8926,7 +8926,6 @@ impl Editor { operation, copilot_enabled, copilot_enabled_for_language, - cx, ) } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 78f9b15051..a3f247b7b9 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -866,6 +866,7 @@ impl Item for Editor { } fn to_item_events(event: &EditorEvent, mut f: impl FnMut(ItemEvent)) { + dbg!(event); match event { EditorEvent::Closed => f(ItemEvent::CloseItem), diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index cfb98ccd74..2bb8c6648c 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -182,7 +182,7 @@ impl PickerDelegate for ThemeSelectorDelegate { let theme_name = cx.theme().name.clone(); self.telemetry - .report_setting_event("theme", theme_name.to_string(), cx); + .report_setting_event("theme", theme_name.to_string()); update_settings_file::(self.fs.clone(), cx, move |settings| { settings.theme = Some(theme_name.to_string()); diff --git a/crates/welcome/src/base_keymap_picker.rs b/crates/welcome/src/base_keymap_picker.rs index 9a8edf0eb3..58ad777757 100644 --- a/crates/welcome/src/base_keymap_picker.rs +++ b/crates/welcome/src/base_keymap_picker.rs @@ -1,4 +1,5 @@ use super::base_keymap_setting::BaseKeymap; +use client::telemetry::Telemetry; use fuzzy::{match_strings, StringMatch, StringMatchCandidate}; use gpui::{ actions, AppContext, DismissEvent, EventEmitter, FocusableView, Render, Task, View, @@ -27,9 +28,10 @@ pub fn toggle( cx: &mut ViewContext, ) { let fs = workspace.app_state().fs.clone(); + let telemetry = workspace.client().telemetry().clone(); workspace.toggle_modal(cx, |cx| { BaseKeymapSelector::new( - BaseKeymapSelectorDelegate::new(cx.view().downgrade(), fs, cx), + BaseKeymapSelectorDelegate::new(cx.view().downgrade(), fs, telemetry, cx), cx, ) }); @@ -73,6 +75,7 @@ pub struct BaseKeymapSelectorDelegate { view: WeakView, matches: Vec, selected_index: usize, + telemetry: Arc, fs: Arc, } @@ -80,6 +83,7 @@ impl BaseKeymapSelectorDelegate { fn new( weak_view: WeakView, fs: Arc, + telemetry: Arc, cx: &mut ViewContext, ) -> Self { let base = BaseKeymap::get(None, cx); @@ -91,6 +95,7 @@ impl BaseKeymapSelectorDelegate { view: weak_view, matches: Vec::new(), selected_index, + telemetry, fs, } } @@ -172,6 +177,10 @@ impl PickerDelegate for BaseKeymapSelectorDelegate { fn confirm(&mut self, _: bool, cx: &mut ViewContext>) { if let Some(selection) = self.matches.get(self.selected_index) { let base_keymap = BaseKeymap::from_names(&selection.string); + + self.telemetry + .report_setting_event("keymap", base_keymap.to_string()); + update_settings_file::(self.fs.clone(), cx, move |setting| { *setting = Some(base_keymap) }); diff --git a/crates/welcome/src/base_keymap_setting.rs b/crates/welcome/src/base_keymap_setting.rs index cad6e894f9..411caa820e 100644 --- a/crates/welcome/src/base_keymap_setting.rs +++ b/crates/welcome/src/base_keymap_setting.rs @@ -1,3 +1,5 @@ +use std::fmt::{Display, Formatter}; + use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use settings::Settings; @@ -12,6 +14,18 @@ pub enum BaseKeymap { TextMate, } +impl Display for BaseKeymap { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + BaseKeymap::VSCode => write!(f, "VSCode"), + BaseKeymap::JetBrains => write!(f, "JetBrains"), + BaseKeymap::SublimeText => write!(f, "Sublime Text"), + BaseKeymap::Atom => write!(f, "Atom"), + BaseKeymap::TextMate => write!(f, "TextMate"), + } + } +} + impl BaseKeymap { pub const OPTIONS: [(&'static str, Self); 5] = [ ("VSCode (Default)", Self::VSCode), diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index d096248a28..71d98e6c9d 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -1,7 +1,7 @@ mod base_keymap_picker; mod base_keymap_setting; -use client::TelemetrySettings; +use client::{telemetry::Telemetry, TelemetrySettings}; use db::kvp::KEY_VALUE_STORE; use gpui::{ svg, AnyElement, AppContext, EventEmitter, FocusHandle, FocusableView, InteractiveElement, @@ -14,7 +14,7 @@ use ui::{prelude::*, Checkbox}; use vim::VimModeSetting; use workspace::{ dock::DockPosition, - item::{Item, ItemEvent}, + item::{Item, ItemEvent, ItemHandle}, open_new, AppState, Welcome, Workspace, WorkspaceId, }; @@ -27,7 +27,7 @@ pub fn init(cx: &mut AppContext) { cx.observe_new_views(|workspace: &mut Workspace, _cx| { workspace.register_action(|workspace, _: &Welcome, cx| { - let welcome_page = cx.new_view(|cx| WelcomePage::new(workspace, cx)); + let welcome_page = WelcomePage::new(workspace, cx); workspace.add_item(Box::new(welcome_page), cx) }); }) @@ -39,7 +39,7 @@ pub fn init(cx: &mut AppContext) { pub fn show_welcome_view(app_state: &Arc, cx: &mut AppContext) { open_new(&app_state, cx, |workspace, cx| { workspace.toggle_dock(DockPosition::Left, cx); - let welcome_page = cx.new_view(|cx| WelcomePage::new(workspace, cx)); + let welcome_page = WelcomePage::new(workspace, cx); workspace.add_item_to_center(Box::new(welcome_page.clone()), cx); cx.focus_view(&welcome_page); cx.notify(); @@ -54,174 +54,248 @@ pub fn show_welcome_view(app_state: &Arc, cx: &mut AppContext) { pub struct WelcomePage { workspace: WeakView, focus_handle: FocusHandle, + telemetry: Arc, _settings_subscription: Subscription, } impl Render for WelcomePage { fn render(&mut self, cx: &mut gpui::ViewContext) -> impl IntoElement { - h_stack() - .full() - .bg(cx.theme().colors().editor_background) - .track_focus(&self.focus_handle) - .child( - v_stack() - .w_96() - .gap_4() - .mx_auto() - .child( - svg() - .path("icons/logo_96.svg") - .text_color(gpui::white()) - .w(px(96.)) - .h(px(96.)) - .mx_auto(), - ) - .child( - h_stack() - .justify_center() - .child(Label::new("Code at the speed of thought")), - ) - .child( - v_stack() - .gap_2() - .child( - Button::new("choose-theme", "Choose a theme") - .full_width() - .on_click(cx.listener(|this, _, cx| { - this.workspace - .update(cx, |workspace, cx| { - theme_selector::toggle( - workspace, - &Default::default(), - cx, - ) - }) - .ok(); - })), - ) - .child( - Button::new("choose-keymap", "Choose a keymap") - .full_width() - .on_click(cx.listener(|this, _, cx| { - this.workspace - .update(cx, |workspace, cx| { - base_keymap_picker::toggle( - workspace, - &Default::default(), - cx, - ) - }) - .ok(); - })), - ) - .child( - Button::new("install-cli", "Install the CLI") - .full_width() - .on_click(cx.listener(|_, _, cx| { - cx.app_mut() - .spawn(|cx| async move { - install_cli::install_cli(&cx).await - }) - .detach_and_log_err(cx); - })), - ), - ) - .child( - v_stack() - .p_3() - .gap_2() - .bg(cx.theme().colors().elevated_surface_background) - .border_1() - .border_color(cx.theme().colors().border) - .rounded_md() - .child( - h_stack() - .gap_2() - .child( - Checkbox::new( - "enable-vim", - if VimModeSetting::get_global(cx).0 { - ui::Selection::Selected - } else { - ui::Selection::Unselected - }, + h_stack().full().track_focus(&self.focus_handle).child( + v_stack() + .w_96() + .gap_4() + .mx_auto() + .child( + svg() + .path("icons/logo_96.svg") + .text_color(gpui::white()) + .w(px(96.)) + .h(px(96.)) + .mx_auto(), + ) + .child( + h_stack() + .justify_center() + .child(Label::new("Code at the speed of thought")), + ) + .child( + v_stack() + .gap_2() + .child( + Button::new("choose-theme", "Choose a theme") + .full_width() + .on_click(cx.listener(|this, _, cx| { + this.telemetry + .report_app_event("welcome page button: theme", false); + this.workspace + .update(cx, |workspace, cx| { + theme_selector::toggle( + workspace, + &Default::default(), + cx, + ) + }) + .ok(); + })), + ) + .child( + Button::new("choose-keymap", "Choose a keymap") + .full_width() + .on_click(cx.listener(|this, _, cx| { + this.telemetry + .report_app_event("welcome page button: keymap", false); + this.workspace + .update(cx, |workspace, cx| { + base_keymap_picker::toggle( + workspace, + &Default::default(), + cx, + ) + }) + .ok(); + })), + ) + .child( + Button::new("install-cli", "Install the CLI") + .full_width() + .on_click(cx.listener(|this, _, cx| { + this.telemetry.report_app_event( + "welcome page button: install cli", + false, + ); + cx.app_mut() + .spawn( + |cx| async move { install_cli::install_cli(&cx).await }, ) - .on_click( - cx.listener(move |this, selection, cx| { - this.update_settings::( - selection, - cx, - |setting, value| *setting = Some(value), - ); - }), - ), + .detach_and_log_err(cx); + })), + ), + ) + .child( + v_stack() + .p_3() + .gap_2() + .bg(cx.theme().colors().elevated_surface_background) + .border_1() + .border_color(cx.theme().colors().border) + .rounded_md() + .child( + h_stack() + .gap_2() + .child( + Checkbox::new( + "enable-vim", + if VimModeSetting::get_global(cx).0 { + ui::Selection::Selected + } else { + ui::Selection::Unselected + }, ) - .child(Label::new("Enable vim mode")), - ) - .child( - h_stack() - .gap_2() - .child( - Checkbox::new( - "enable-telemetry", - if TelemetrySettings::get_global(cx).metrics { - ui::Selection::Selected - } else { - ui::Selection::Unselected - }, - ) - .on_click( - cx.listener(move |this, selection, cx| { - this.update_settings::( - selection, - cx, - |settings, value| { - settings.metrics = Some(value) - }, - ); - }), - ), + .on_click(cx.listener( + move |this, selection, cx| { + this.telemetry.report_app_event( + "welcome page button: vim", + false, + ); + this.update_settings::( + selection, + cx, + |setting, value| *setting = Some(value), + ); + }, + )), + ) + .child(Label::new("Enable vim mode")), + ) + .child( + h_stack() + .gap_2() + .child( + Checkbox::new( + "enable-telemetry", + if TelemetrySettings::get_global(cx).metrics { + ui::Selection::Selected + } else { + ui::Selection::Unselected + }, ) - .child(Label::new("Send anonymous usage data")), - ) - .child( - h_stack() - .gap_2() - .child( - Checkbox::new( - "enable-crash", - if TelemetrySettings::get_global(cx).diagnostics { - ui::Selection::Selected - } else { - ui::Selection::Unselected - }, - ) - .on_click( - cx.listener(move |this, selection, cx| { - this.update_settings::( - selection, - cx, - |settings, value| { - settings.diagnostics = Some(value) - }, - ); - }), - ), + .on_click(cx.listener( + move |this, selection, cx| { + this.telemetry.report_app_event( + "welcome page button: user telemetry", + false, + ); + this.update_settings::( + selection, + cx, + { + let telemetry = this.telemetry.clone(); + + move |settings, value| { + settings.metrics = Some(value); + + telemetry.report_setting_event( + "user telemetry", + value.to_string(), + ); + } + }, + ); + }, + )), + ) + .child(Label::new("Send anonymous usage data")), + ) + .child( + h_stack() + .gap_2() + .child( + Checkbox::new( + "enable-crash", + if TelemetrySettings::get_global(cx).diagnostics { + ui::Selection::Selected + } else { + ui::Selection::Unselected + }, ) - .child(Label::new("Send crash reports")), - ), - ), - ) + .on_click(cx.listener( + move |this, selection, cx| { + this.telemetry.report_app_event( + "welcome page button: crash diagnostics", + false, + ); + this.update_settings::( + selection, + cx, + { + let telemetry = this.telemetry.clone(); + + move |settings, value| { + settings.diagnostics = Some(value); + + telemetry.report_setting_event( + "crash diagnostics", + value.to_string(), + ); + } + }, + ); + }, + )), + ) + .child(Label::new("Send crash reports")), + ), + ), + ) } } impl WelcomePage { - pub fn new(workspace: &Workspace, cx: &mut ViewContext) -> Self { - WelcomePage { + pub fn new(workspace: &Workspace, cx: &mut ViewContext) -> View { + let this = cx.new_view(|cx| WelcomePage { focus_handle: cx.focus_handle(), workspace: workspace.weak_handle(), + telemetry: workspace.client().telemetry().clone(), _settings_subscription: cx.observe_global::(move |_, cx| cx.notify()), - } + }); + + this.on_release( + cx, + Box::new(|cx| { + this.update(cx, |this, _| { + this.telemetry.report_app_event("close welcome page", false); + }) + }), + ) + .detach(); + + // this.subscribe_to_item_events( + // cx, + // Box::new(|event: ItemEvent, cx| { + // // if event == ItemEvent::CloseItem { + // dbg!(event); + // // welcome.update(cx, |welcome, _| { + // // welcome + // // .telemetry + // // .report_app_event("close welcome page", false); + // // }) + // // } + // }), + // ) + // .detach(); + + cx.subscribe(&this, |_, welcome, event, cx| { + if *event == ItemEvent::CloseItem { + welcome.update(cx, |welcome, _| { + welcome + .telemetry + .report_app_event("close welcome page", false); + }) + } + }) + .detach(); + + this } fn update_settings( @@ -279,6 +353,7 @@ impl Item for WelcomePage { Some(cx.new_view(|cx| WelcomePage { focus_handle: cx.focus_handle(), workspace: self.workspace.clone(), + telemetry: self.telemetry.clone(), _settings_subscription: cx.observe_global::(move |_, cx| cx.notify()), })) } @@ -287,3 +362,16 @@ impl Item for WelcomePage { f(*event) } } + +// TODO +// - [X] get theme value +// - [X] In selector +// - [X] In main +// - [ ] get value of keymap selector +// - [X] In selector +// - [X] In main +// - [ ] get all button clicks +// - [ ] get value of usage data enabled +// - [ ] get value of crash reports enabled +// - [ ] get welcome screen close +// - [ ] test all events diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 6b29496f2c..653f777084 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1260,7 +1260,7 @@ impl Workspace { pub fn open(&mut self, _: &Open, cx: &mut ViewContext) { self.client() .telemetry() - .report_app_event("open project", false, cx); + .report_app_event("open project", false); let paths = cx.prompt_for_paths(PathPromptOptions { files: true, directories: true, diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index e0da81edc4..22436e10a9 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -45,7 +45,7 @@ use util::{ paths, ResultExt, }; use uuid::Uuid; -use welcome::{show_welcome_view, FIRST_OPEN}; +use welcome::{show_welcome_view, BaseKeymap, FIRST_OPEN}; use workspace::{AppState, WorkspaceStore}; use zed::{ app_menus, build_window_options, ensure_only_instance, handle_cli_connection, @@ -171,17 +171,17 @@ fn main() { }) .detach(); - client.telemetry().start(installation_id, session_id, cx); - client - .telemetry() - .report_setting_event("theme", cx.theme().name.to_string(), cx); - let event_operation = match existing_installation_id_found { - Some(false) => "first open", - _ => "open", - }; - client - .telemetry() - .report_app_event(event_operation, true, cx); + let telemetry = client.telemetry(); + telemetry.start(installation_id, session_id, cx); + telemetry.report_setting_event("theme", cx.theme().name.to_string()); + telemetry.report_setting_event("keymap", BaseKeymap::get_global(cx).to_string()); + telemetry.report_app_event( + match existing_installation_id_found { + Some(false) => "first open", + _ => "open", + }, + true, + ); let app_state = Arc::new(AppState { languages: languages.clone(), From 167a0b590f86cff809e535768e00f1d5a2480337 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Sat, 6 Jan 2024 15:17:28 -0500 Subject: [PATCH 2/8] Add event for welcome page close --- crates/editor/src/items.rs | 1 - crates/welcome/src/welcome.rs | 54 +++++++++-------------------------- 2 files changed, 13 insertions(+), 42 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index a3f247b7b9..78f9b15051 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -866,7 +866,6 @@ impl Item for Editor { } fn to_item_events(event: &EditorEvent, mut f: impl FnMut(ItemEvent)) { - dbg!(event); match event { EditorEvent::Closed => f(ItemEvent::CloseItem), diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index 71d98e6c9d..6650ff4594 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -14,7 +14,7 @@ use ui::{prelude::*, Checkbox}; use vim::VimModeSetting; use workspace::{ dock::DockPosition, - item::{Item, ItemEvent, ItemHandle}, + item::{Item, ItemEvent}, open_new, AppState, Welcome, Workspace, WorkspaceId, }; @@ -252,48 +252,20 @@ impl Render for WelcomePage { impl WelcomePage { pub fn new(workspace: &Workspace, cx: &mut ViewContext) -> View { - let this = cx.new_view(|cx| WelcomePage { - focus_handle: cx.focus_handle(), - workspace: workspace.weak_handle(), - telemetry: workspace.client().telemetry().clone(), - _settings_subscription: cx.observe_global::(move |_, cx| cx.notify()), - }); + let this = cx.new_view(|cx| { + cx.on_release(|this: &mut Self, _, _| { + this.telemetry.report_app_event("close welcome page", false); + }) + .detach(); - this.on_release( - cx, - Box::new(|cx| { - this.update(cx, |this, _| { - this.telemetry.report_app_event("close welcome page", false); - }) - }), - ) - .detach(); - - // this.subscribe_to_item_events( - // cx, - // Box::new(|event: ItemEvent, cx| { - // // if event == ItemEvent::CloseItem { - // dbg!(event); - // // welcome.update(cx, |welcome, _| { - // // welcome - // // .telemetry - // // .report_app_event("close welcome page", false); - // // }) - // // } - // }), - // ) - // .detach(); - - cx.subscribe(&this, |_, welcome, event, cx| { - if *event == ItemEvent::CloseItem { - welcome.update(cx, |welcome, _| { - welcome - .telemetry - .report_app_event("close welcome page", false); - }) + WelcomePage { + focus_handle: cx.focus_handle(), + workspace: workspace.weak_handle(), + telemetry: workspace.client().telemetry().clone(), + _settings_subscription: cx + .observe_global::(move |_, cx| cx.notify()), } - }) - .detach(); + }); this } From 800c9958a377e95e7abfc3aea9071a9ae9461a71 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Sat, 6 Jan 2024 15:31:16 -0500 Subject: [PATCH 3/8] Clean up code --- crates/call/src/call.rs | 2 +- crates/client/src/telemetry.rs | 2 +- crates/welcome/src/welcome.rs | 13 ------------- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/crates/call/src/call.rs b/crates/call/src/call.rs index 4f9ec08059..3561cc3385 100644 --- a/crates/call/src/call.rs +++ b/crates/call/src/call.rs @@ -310,7 +310,7 @@ impl ActiveCall { }) } - pub fn decline_incoming(&mut self, _cx: &mut ModelContext) -> Result<()> { + pub fn decline_incoming(&mut self, _: &mut ModelContext) -> Result<()> { let call = self .incoming_call .0 diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 6b6b4b0a08..076ac15710 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -213,7 +213,7 @@ impl Telemetry { drop(state); let this = self.clone(); - cx.spawn(|_cx| async move { + cx.spawn(|_| async move { // Avoiding calling `System::new_all()`, as there have been crashes related to it let refresh_kind = RefreshKind::new() .with_memory() // For memory usage diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index 6650ff4594..1c0b747208 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -334,16 +334,3 @@ impl Item for WelcomePage { f(*event) } } - -// TODO -// - [X] get theme value -// - [X] In selector -// - [X] In main -// - [ ] get value of keymap selector -// - [X] In selector -// - [X] In main -// - [ ] get all button clicks -// - [ ] get value of usage data enabled -// - [ ] get value of crash reports enabled -// - [ ] get welcome screen close -// - [ ] test all events From 520c433af53c13517445b1949f537908f4f2cd50 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Sat, 6 Jan 2024 16:10:40 -0500 Subject: [PATCH 4/8] Fix tests --- crates/channel/src/channel_store_tests.rs | 5 +++-- crates/client/src/client.rs | 14 ++++++++++++++ crates/collab_ui/src/chat_panel/message_editor.rs | 5 +++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/channel/src/channel_store_tests.rs b/crates/channel/src/channel_store_tests.rs index 20413d7a76..0b07918acf 100644 --- a/crates/channel/src/channel_store_tests.rs +++ b/crates/channel/src/channel_store_tests.rs @@ -343,12 +343,13 @@ async fn test_channel_messages(cx: &mut TestAppContext) { } fn init_test(cx: &mut AppContext) -> Model { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + let http = FakeHttpClient::with_404_response(); let client = Client::new(http.clone(), cx); let user_store = cx.new_model(|cx| UserStore::new(client.clone(), cx)); - let settings_store = SettingsStore::test(cx); - cx.set_global(settings_store); client::init(&client, cx); crate::init(&client, user_store, cx); diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index d070cae375..1451039b3a 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -1404,11 +1404,13 @@ mod tests { use gpui::{BackgroundExecutor, Context, TestAppContext}; use parking_lot::Mutex; + use settings::SettingsStore; use std::future; use util::http::FakeHttpClient; #[gpui::test(iterations = 10)] async fn test_reconnection(cx: &mut TestAppContext) { + init_test(cx); let user_id = 5; let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; @@ -1443,6 +1445,7 @@ mod tests { #[gpui::test(iterations = 10)] async fn test_connection_timeout(executor: BackgroundExecutor, cx: &mut TestAppContext) { + init_test(cx); let user_id = 5; let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let mut status = client.status(); @@ -1514,6 +1517,7 @@ mod tests { cx: &mut TestAppContext, executor: BackgroundExecutor, ) { + init_test(cx); let auth_count = Arc::new(Mutex::new(0)); let dropped_auth_count = Arc::new(Mutex::new(0)); let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); @@ -1562,6 +1566,7 @@ mod tests { #[gpui::test] async fn test_subscribing_to_entity(cx: &mut TestAppContext) { + init_test(cx); let user_id = 5; let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; @@ -1615,6 +1620,7 @@ mod tests { #[gpui::test] async fn test_subscribing_after_dropping_subscription(cx: &mut TestAppContext) { + init_test(cx); let user_id = 5; let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; @@ -1643,6 +1649,7 @@ mod tests { #[gpui::test] async fn test_dropping_subscription_in_handler(cx: &mut TestAppContext) { + init_test(cx); let user_id = 5; let client = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let server = FakeServer::for_client(user_id, &client, cx).await; @@ -1671,4 +1678,11 @@ mod tests { id: usize, subscription: Option, } + + fn init_test(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + }); + } } diff --git a/crates/collab_ui/src/chat_panel/message_editor.rs b/crates/collab_ui/src/chat_panel/message_editor.rs index 522db1042d..517fac4fbb 100644 --- a/crates/collab_ui/src/chat_panel/message_editor.rs +++ b/crates/collab_ui/src/chat_panel/message_editor.rs @@ -271,11 +271,12 @@ mod tests { fn init_test(cx: &mut TestAppContext) -> Arc { cx.update(|cx| { + let settings = SettingsStore::test(cx); + cx.set_global(settings); + let http = FakeHttpClient::with_404_response(); let client = Client::new(http.clone(), cx); let user_store = cx.new_model(|cx| UserStore::new(client.clone(), cx)); - let settings = SettingsStore::test(cx); - cx.set_global(settings); theme::init(theme::LoadThemes::JustBase, cx); language::init(cx); editor::init(cx); From 5344296c9a6343192f5e3fe88e8b02d49be47152 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Sat, 6 Jan 2024 20:27:30 -0500 Subject: [PATCH 5/8] Remove immediate flush mode Allow flush method to be called publicly. This is a better, simpler solution, that allows for better control over flushing. --- crates/client/src/telemetry.rs | 27 ++++++++++++++------------- crates/welcome/src/welcome.rs | 20 +++++++------------- crates/workspace/src/workspace.rs | 4 +--- crates/zed/src/main.rs | 12 +++++------- 4 files changed, 27 insertions(+), 36 deletions(-) diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 076ac15710..d0a92b681d 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -193,7 +193,8 @@ 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", true); + self.report_app_event("close"); + self.flush_clickhouse_events(); Task::ready(()) } @@ -283,7 +284,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false) + self.report_clickhouse_event(event) } pub fn report_copilot_event( @@ -299,7 +300,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false) + self.report_clickhouse_event(event) } pub fn report_assistant_event( @@ -315,7 +316,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false) + self.report_clickhouse_event(event) } pub fn report_call_event( @@ -331,7 +332,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false) + self.report_clickhouse_event(event) } pub fn report_cpu_event(self: &Arc, usage_as_percentage: f32, core_count: u32) { @@ -341,7 +342,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false) + self.report_clickhouse_event(event) } pub fn report_memory_event( @@ -355,16 +356,16 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false) + self.report_clickhouse_event(event) } - pub fn report_app_event(self: &Arc, operation: &'static str, immediate_flush: bool) { + pub fn report_app_event(self: &Arc, operation: &'static str) { let event = ClickhouseEvent::App { operation, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, immediate_flush) + self.report_clickhouse_event(event) } pub fn report_setting_event(self: &Arc, setting: &'static str, value: String) { @@ -374,7 +375,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, false) + self.report_clickhouse_event(event) } fn milliseconds_since_first_event(&self) -> i64 { @@ -391,7 +392,7 @@ impl Telemetry { } } - fn report_clickhouse_event(self: &Arc, event: ClickhouseEvent, immediate_flush: bool) { + fn report_clickhouse_event(self: &Arc, event: ClickhouseEvent) { let mut state = self.state.lock(); if !state.settings.metrics { @@ -404,7 +405,7 @@ impl Telemetry { .push(ClickhouseEventWrapper { signed_in, event }); if state.installation_id.is_some() { - if immediate_flush || state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { + if state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { drop(state); self.flush_clickhouse_events(); } else { @@ -430,7 +431,7 @@ impl Telemetry { self.state.lock().is_staff } - fn flush_clickhouse_events(self: &Arc) { + pub fn flush_clickhouse_events(self: &Arc) { let mut state = self.state.lock(); state.first_event_datetime = None; let mut events = mem::take(&mut state.clickhouse_events_queue); diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index 1c0b747208..3d89810679 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -86,7 +86,7 @@ impl Render for WelcomePage { .full_width() .on_click(cx.listener(|this, _, cx| { this.telemetry - .report_app_event("welcome page button: theme", false); + .report_app_event("welcome page button: theme"); this.workspace .update(cx, |workspace, cx| { theme_selector::toggle( @@ -103,7 +103,7 @@ impl Render for WelcomePage { .full_width() .on_click(cx.listener(|this, _, cx| { this.telemetry - .report_app_event("welcome page button: keymap", false); + .report_app_event("welcome page button: keymap"); this.workspace .update(cx, |workspace, cx| { base_keymap_picker::toggle( @@ -119,10 +119,8 @@ impl Render for WelcomePage { Button::new("install-cli", "Install the CLI") .full_width() .on_click(cx.listener(|this, _, cx| { - this.telemetry.report_app_event( - "welcome page button: install cli", - false, - ); + this.telemetry + .report_app_event("welcome page button: install cli"); cx.app_mut() .spawn( |cx| async move { install_cli::install_cli(&cx).await }, @@ -153,10 +151,8 @@ impl Render for WelcomePage { ) .on_click(cx.listener( move |this, selection, cx| { - this.telemetry.report_app_event( - "welcome page button: vim", - false, - ); + this.telemetry + .report_app_event("welcome page button: vim"); this.update_settings::( selection, cx, @@ -183,7 +179,6 @@ impl Render for WelcomePage { move |this, selection, cx| { this.telemetry.report_app_event( "welcome page button: user telemetry", - false, ); this.update_settings::( selection, @@ -222,7 +217,6 @@ impl Render for WelcomePage { move |this, selection, cx| { this.telemetry.report_app_event( "welcome page button: crash diagnostics", - false, ); this.update_settings::( selection, @@ -254,7 +248,7 @@ impl WelcomePage { pub fn new(workspace: &Workspace, cx: &mut ViewContext) -> View { let this = cx.new_view(|cx| { cx.on_release(|this: &mut Self, _, _| { - this.telemetry.report_app_event("close welcome page", false); + this.telemetry.report_app_event("close welcome page"); }) .detach(); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 653f777084..967e76efd6 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1258,9 +1258,7 @@ impl Workspace { } pub fn open(&mut self, _: &Open, cx: &mut ViewContext) { - self.client() - .telemetry() - .report_app_event("open project", false); + self.client().telemetry().report_app_event("open project"); let paths = cx.prompt_for_paths(PathPromptOptions { files: true, directories: true, diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 22436e10a9..ff07af03bd 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -175,13 +175,11 @@ fn main() { telemetry.start(installation_id, session_id, cx); telemetry.report_setting_event("theme", cx.theme().name.to_string()); telemetry.report_setting_event("keymap", BaseKeymap::get_global(cx).to_string()); - telemetry.report_app_event( - match existing_installation_id_found { - Some(false) => "first open", - _ => "open", - }, - true, - ); + telemetry.report_app_event(match existing_installation_id_found { + Some(false) => "first open", + _ => "open", + }); + telemetry.flush_clickhouse_events(); let app_state = Arc::new(AppState { languages: languages.clone(), From f4c78d3f4000b3cdf5376d60170286d7cf8470f8 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Sat, 6 Jan 2024 23:27:23 -0500 Subject: [PATCH 6/8] Remove "clickhouse" from telemetry code The client sends events to our end point, and the endpoint is what determines what analytics database is used to store the data. The client should be generic and not mention the name of the service being proxied to through our server. --- crates/client/src/client.rs | 2 +- crates/client/src/telemetry.rs | 81 ++++++++++++++++------------------ crates/zed/src/main.rs | 2 +- 3 files changed, 41 insertions(+), 44 deletions(-) diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 1451039b3a..b07dddc006 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -45,7 +45,7 @@ use util::http::HttpClient; use util::{ResultExt, TryFutureExt}; pub use rpc::*; -pub use telemetry::ClickhouseEvent; +pub use telemetry::Event; pub use user::*; lazy_static! { diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index d0a92b681d..94d8369a69 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -28,22 +28,21 @@ struct TelemetryState { release_channel: Option<&'static str>, app_metadata: AppMetadata, architecture: &'static str, - clickhouse_events_queue: Vec, - flush_clickhouse_events_task: Option>, + events_queue: Vec, + flush_events_task: Option>, log_file: Option, is_staff: Option, first_event_datetime: Option>, } -const CLICKHOUSE_EVENTS_URL_PATH: &'static str = "/api/events"; +const EVENTS_URL_PATH: &'static str = "/api/events"; lazy_static! { - static ref CLICKHOUSE_EVENTS_URL: String = - format!("{}{}", *ZED_SERVER_URL, CLICKHOUSE_EVENTS_URL_PATH); + static ref EVENTS_URL: String = format!("{}{}", *ZED_SERVER_URL, EVENTS_URL_PATH); } #[derive(Serialize, Debug)] -struct ClickhouseEventRequestBody { +struct EventRequestBody { token: &'static str, installation_id: Option>, session_id: Option>, @@ -53,14 +52,14 @@ struct ClickhouseEventRequestBody { os_version: Option, architecture: &'static str, release_channel: Option<&'static str>, - events: Vec, + events: Vec, } #[derive(Serialize, Debug)] -struct ClickhouseEventWrapper { +struct EventWrapper { signed_in: bool, #[serde(flatten)] - event: ClickhouseEvent, + event: Event, } #[derive(Serialize, Debug)] @@ -72,7 +71,7 @@ pub enum AssistantKind { #[derive(Serialize, Debug)] #[serde(tag = "type")] -pub enum ClickhouseEvent { +pub enum Event { Editor { operation: &'static str, file_extension: Option, @@ -150,8 +149,8 @@ impl Telemetry { installation_id: None, metrics_id: None, session_id: None, - clickhouse_events_queue: Default::default(), - flush_clickhouse_events_task: Default::default(), + events_queue: Default::default(), + flush_events_task: Default::default(), log_file: None, is_staff: None, first_event_datetime: None, @@ -194,7 +193,7 @@ impl Telemetry { #[cfg(not(any(test, feature = "test-support")))] fn shutdown_telemetry(self: &Arc) -> impl Future { self.report_app_event("close"); - self.flush_clickhouse_events(); + self.flush_events(); Task::ready(()) } @@ -275,7 +274,7 @@ impl Telemetry { copilot_enabled: bool, copilot_enabled_for_language: bool, ) { - let event = ClickhouseEvent::Editor { + let event = Event::Editor { file_extension, vim_mode, operation, @@ -284,7 +283,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event) + self.report_event(event) } pub fn report_copilot_event( @@ -293,14 +292,14 @@ impl Telemetry { suggestion_accepted: bool, file_extension: Option, ) { - let event = ClickhouseEvent::Copilot { + let event = Event::Copilot { suggestion_id, suggestion_accepted, file_extension, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event) + self.report_event(event) } pub fn report_assistant_event( @@ -309,14 +308,14 @@ impl Telemetry { kind: AssistantKind, model: &'static str, ) { - let event = ClickhouseEvent::Assistant { + let event = Event::Assistant { conversation_id, kind, model, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event) + self.report_event(event) } pub fn report_call_event( @@ -325,24 +324,24 @@ impl Telemetry { room_id: Option, channel_id: Option, ) { - let event = ClickhouseEvent::Call { + let event = Event::Call { operation, room_id, channel_id, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event) + self.report_event(event) } pub fn report_cpu_event(self: &Arc, usage_as_percentage: f32, core_count: u32) { - let event = ClickhouseEvent::Cpu { + let event = Event::Cpu { usage_as_percentage, core_count, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event) + self.report_event(event) } pub fn report_memory_event( @@ -350,32 +349,32 @@ impl Telemetry { memory_in_bytes: u64, virtual_memory_in_bytes: u64, ) { - let event = ClickhouseEvent::Memory { + let event = Event::Memory { memory_in_bytes, virtual_memory_in_bytes, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event) + self.report_event(event) } pub fn report_app_event(self: &Arc, operation: &'static str) { - let event = ClickhouseEvent::App { + let event = Event::App { operation, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event) + self.report_event(event) } pub fn report_setting_event(self: &Arc, setting: &'static str, value: String) { - let event = ClickhouseEvent::Setting { + let event = Event::Setting { setting, value, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event) + self.report_event(event) } fn milliseconds_since_first_event(&self) -> i64 { @@ -392,7 +391,7 @@ impl Telemetry { } } - fn report_clickhouse_event(self: &Arc, event: ClickhouseEvent) { + fn report_event(self: &Arc, event: Event) { let mut state = self.state.lock(); if !state.settings.metrics { @@ -400,20 +399,18 @@ impl Telemetry { } let signed_in = state.metrics_id.is_some(); - state - .clickhouse_events_queue - .push(ClickhouseEventWrapper { signed_in, event }); + state.events_queue.push(EventWrapper { signed_in, event }); if state.installation_id.is_some() { - if state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { + if state.events_queue.len() >= MAX_QUEUE_LEN { drop(state); - self.flush_clickhouse_events(); + self.flush_events(); } else { let this = self.clone(); let executor = self.executor.clone(); - state.flush_clickhouse_events_task = Some(self.executor.spawn(async move { + state.flush_events_task = Some(self.executor.spawn(async move { executor.timer(DEBOUNCE_INTERVAL).await; - this.flush_clickhouse_events(); + this.flush_events(); })); } } @@ -431,11 +428,11 @@ impl Telemetry { self.state.lock().is_staff } - pub fn flush_clickhouse_events(self: &Arc) { + pub fn flush_events(self: &Arc) { let mut state = self.state.lock(); state.first_event_datetime = None; - let mut events = mem::take(&mut state.clickhouse_events_queue); - state.flush_clickhouse_events_task.take(); + let mut events = mem::take(&mut state.events_queue); + state.flush_events_task.take(); drop(state); let this = self.clone(); @@ -456,7 +453,7 @@ impl Telemetry { { let state = this.state.lock(); - let request_body = ClickhouseEventRequestBody { + let request_body = EventRequestBody { token: ZED_SECRET_CLIENT_TOKEN, installation_id: state.installation_id.clone(), session_id: state.session_id.clone(), @@ -480,7 +477,7 @@ impl Telemetry { } this.http_client - .post_json(CLICKHOUSE_EVENTS_URL.as_str(), json_bytes.into()) + .post_json(EVENTS_URL.as_str(), json_bytes.into()) .await?; anyhow::Ok(()) } diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index ff07af03bd..56109d9c9a 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -179,7 +179,7 @@ fn main() { Some(false) => "first open", _ => "open", }); - telemetry.flush_clickhouse_events(); + telemetry.flush_events(); let app_state = Arc::new(AppState { languages: languages.clone(), From 1bcee19ed5be667e27ca9b9b8aaa96aaf476471e Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Sat, 6 Jan 2024 23:30:53 -0500 Subject: [PATCH 7/8] Improve operation name consistency for welcome page --- crates/welcome/src/welcome.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index 3d89810679..79114a1aad 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -86,7 +86,7 @@ impl Render for WelcomePage { .full_width() .on_click(cx.listener(|this, _, cx| { this.telemetry - .report_app_event("welcome page button: theme"); + .report_app_event("welcome page: change theme"); this.workspace .update(cx, |workspace, cx| { theme_selector::toggle( @@ -103,7 +103,7 @@ impl Render for WelcomePage { .full_width() .on_click(cx.listener(|this, _, cx| { this.telemetry - .report_app_event("welcome page button: keymap"); + .report_app_event("welcome page: change keymap"); this.workspace .update(cx, |workspace, cx| { base_keymap_picker::toggle( @@ -119,8 +119,7 @@ impl Render for WelcomePage { Button::new("install-cli", "Install the CLI") .full_width() .on_click(cx.listener(|this, _, cx| { - this.telemetry - .report_app_event("welcome page button: install cli"); + this.telemetry.report_app_event("welcome page: install cli"); cx.app_mut() .spawn( |cx| async move { install_cli::install_cli(&cx).await }, @@ -152,7 +151,7 @@ impl Render for WelcomePage { .on_click(cx.listener( move |this, selection, cx| { this.telemetry - .report_app_event("welcome page button: vim"); + .report_app_event("welcome page: toggle vim"); this.update_settings::( selection, cx, @@ -178,7 +177,7 @@ impl Render for WelcomePage { .on_click(cx.listener( move |this, selection, cx| { this.telemetry.report_app_event( - "welcome page button: user telemetry", + "welcome page: toggle metric telemetry", ); this.update_settings::( selection, @@ -216,7 +215,7 @@ impl Render for WelcomePage { .on_click(cx.listener( move |this, selection, cx| { this.telemetry.report_app_event( - "welcome page button: crash diagnostics", + "welcome page: toggle diagnostic telemetry", ); this.update_settings::( selection, @@ -248,7 +247,7 @@ impl WelcomePage { pub fn new(workspace: &Workspace, cx: &mut ViewContext) -> View { let this = cx.new_view(|cx| { cx.on_release(|this: &mut Self, _, _| { - this.telemetry.report_app_event("close welcome page"); + this.telemetry.report_app_event("welcome page: close"); }) .detach(); From 44bc5aef391d10c4d1ba2f6b3a1fa78f2bc1c492 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Sat, 6 Jan 2024 23:34:46 -0500 Subject: [PATCH 8/8] Improve setting name consistency for welcome page --- crates/welcome/src/welcome.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index 79114a1aad..76988fadb0 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -189,7 +189,7 @@ impl Render for WelcomePage { settings.metrics = Some(value); telemetry.report_setting_event( - "user telemetry", + "metric telemetry", value.to_string(), ); } @@ -227,7 +227,7 @@ impl Render for WelcomePage { settings.diagnostics = Some(value); telemetry.report_setting_event( - "crash diagnostics", + "diagnostic telemetry", value.to_string(), ); }