From aa6ea920e2b8fe4e38c09b489aab9d7065c264ce Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 10 May 2023 12:17:52 -0700 Subject: [PATCH] Define telemetry settings in the client crate --- Cargo.lock | 2 + Cargo.toml | 1 + crates/auto_update/src/auto_update.rs | 6 +- crates/client/Cargo.toml | 1 + crates/client/src/client.rs | 61 ++++++++++++++++----- crates/client/src/telemetry.rs | 11 ++-- crates/client/src/user.rs | 13 +++-- crates/copilot_button/src/copilot_button.rs | 6 +- crates/editor/src/editor.rs | 8 +-- crates/settings/Cargo.toml | 2 +- crates/settings/src/settings.rs | 59 -------------------- crates/settings/src/settings_file.rs | 11 ++-- crates/theme_selector/src/theme_selector.rs | 2 +- crates/welcome/Cargo.toml | 1 + crates/welcome/src/base_keymap_picker.rs | 2 +- crates/welcome/src/welcome.rs | 26 +++++---- crates/zed/src/main.rs | 8 +-- 17 files changed, 100 insertions(+), 120 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 266b0e6d41..baba47b216 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1155,6 +1155,7 @@ dependencies = [ "postage", "rand 0.8.5", "rpc", + "schemars", "serde", "serde_derive", "settings", @@ -8315,6 +8316,7 @@ name = "welcome" version = "0.1.0" dependencies = [ "anyhow", + "client", "db", "editor", "fs", diff --git a/Cargo.toml b/Cargo.toml index 15df687d41..0a73b878f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,6 +85,7 @@ parking_lot = { version = "0.11.1" } postage = { version = "0.5", features = ["futures-traits"] } rand = { version = "0.8.5" } regex = { version = "1.5" } +schemars = { version = "0.8" } serde = { version = "1.0", features = ["derive", "rc"] } serde_derive = { version = "1.0", features = ["deserialize_in_place"] } serde_json = { version = "1.0", features = ["preserve_order", "raw_value"] } diff --git a/crates/auto_update/src/auto_update.rs b/crates/auto_update/src/auto_update.rs index 2e00a204ba..54f3f93b00 100644 --- a/crates/auto_update/src/auto_update.rs +++ b/crates/auto_update/src/auto_update.rs @@ -1,7 +1,7 @@ mod update_notification; use anyhow::{anyhow, Context, Result}; -use client::{Client, ZED_APP_PATH, ZED_APP_VERSION, ZED_SECRET_CLIENT_TOKEN}; +use client::{Client, TelemetrySettings, ZED_APP_PATH, ZED_APP_VERSION, ZED_SECRET_CLIENT_TOKEN}; use db::kvp::KEY_VALUE_STORE; use gpui::{ actions, platform::AppVersion, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, @@ -10,7 +10,7 @@ use gpui::{ use isahc::AsyncBody; use serde::Deserialize; use serde_derive::Serialize; -use settings::{Setting, Settings, SettingsStore}; +use settings::{Setting, SettingsStore}; use smol::{fs::File, io::AsyncReadExt, process::Command}; use std::{ffi::OsString, sync::Arc, time::Duration}; use update_notification::UpdateNotification; @@ -279,7 +279,7 @@ impl AutoUpdater { let release_channel = cx .has_global::() .then(|| cx.global::().display_name()); - let telemetry = cx.global::().telemetry().metrics(); + let telemetry = settings::get_setting::(None, cx).metrics; (installation_id, release_channel, telemetry) }); diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index 99c492d638..3ecc515986 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -31,6 +31,7 @@ log.workspace = true parking_lot.workspace = true postage.workspace = true rand.workspace = true +schemars.workspace = true smol.workspace = true thiserror.workspace = true time.workspace = true diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 18a0f32ed6..9d24254b40 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -15,19 +15,17 @@ use futures::{ TryStreamExt, }; use gpui::{ - actions, - platform::AppVersion, - serde_json::{self}, - AnyModelHandle, AnyWeakModelHandle, AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, - ModelHandle, Task, View, ViewContext, WeakViewHandle, + actions, platform::AppVersion, serde_json, AnyModelHandle, AnyWeakModelHandle, + AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, ModelHandle, Task, View, ViewContext, + WeakViewHandle, }; use lazy_static::lazy_static; use parking_lot::RwLock; use postage::watch; use rand::prelude::*; use rpc::proto::{AnyTypedEnvelope, EntityMessage, EnvelopedMessage, PeerId, RequestMessage}; -use serde::Deserialize; -use settings::Settings; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; use std::{ any::TypeId, collections::HashMap, @@ -73,6 +71,8 @@ pub const CONNECTION_TIMEOUT: Duration = Duration::from_secs(5); actions!(client, [SignIn, SignOut]); pub fn init(client: Arc, cx: &mut AppContext) { + settings::register_setting::(cx); + cx.add_global_action({ let client = client.clone(); move |_: &SignIn, cx| { @@ -326,6 +326,41 @@ impl Drop for PendingEntitySubscription { } } +#[derive(Copy, Clone)] +pub struct TelemetrySettings { + pub diagnostics: bool, + pub metrics: bool, +} + +#[derive(Clone, Serialize, Deserialize, JsonSchema)] +pub struct TelemetrySettingsContent { + pub diagnostics: Option, + pub metrics: Option, +} + +impl settings::Setting for TelemetrySettings { + const KEY: Option<&'static str> = Some("telemetry"); + + type FileContent = TelemetrySettingsContent; + + fn load( + default_value: &Self::FileContent, + user_values: &[&Self::FileContent], + _: &AppContext, + ) -> Self { + Self { + diagnostics: user_values + .first() + .and_then(|v| v.diagnostics) + .unwrap_or(default_value.diagnostics.unwrap()), + metrics: user_values + .first() + .and_then(|v| v.metrics) + .unwrap_or(default_value.metrics.unwrap()), + } + } +} + impl Client { pub fn new(http: Arc, cx: &AppContext) -> Arc { Arc::new(Self { @@ -447,9 +482,7 @@ impl Client { })); } Status::SignedOut | Status::UpgradeRequired => { - let telemetry_settings = cx.read(|cx| cx.global::().telemetry()); - self.telemetry - .set_authenticated_user_info(None, false, telemetry_settings); + cx.read(|cx| self.telemetry.set_authenticated_user_info(None, false, cx)); state._reconnect_task.take(); } _ => {} @@ -740,7 +773,7 @@ impl Client { self.telemetry().report_mixpanel_event( "read credentials from keychain", Default::default(), - cx.global::().telemetry(), + *settings::get_setting::(None, cx), ); }); } @@ -1033,7 +1066,9 @@ impl Client { let executor = cx.background(); let telemetry = self.telemetry.clone(); let http = self.http.clone(); - let metrics_enabled = cx.read(|cx| cx.global::().telemetry()); + + let telemetry_settings = + cx.read(|cx| *settings::get_setting::(None, cx)); executor.clone().spawn(async move { // Generate a pair of asymmetric encryption keys. The public key will be used by the @@ -1120,7 +1155,7 @@ impl Client { telemetry.report_mixpanel_event( "authenticate with browser", Default::default(), - metrics_enabled, + telemetry_settings, ); Ok(Credentials { diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 7151dcd7bb..5c8f208137 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -1,4 +1,4 @@ -use crate::{ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL}; +use crate::{TelemetrySettings, ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL}; use db::kvp::KEY_VALUE_STORE; use gpui::{ executor::Background, @@ -9,7 +9,6 @@ use lazy_static::lazy_static; use parking_lot::Mutex; use serde::Serialize; use serde_json::json; -use settings::TelemetrySettings; use std::{ io::Write, mem, @@ -241,9 +240,9 @@ impl Telemetry { self: &Arc, metrics_id: Option, is_staff: bool, - telemetry_settings: TelemetrySettings, + cx: &AppContext, ) { - if !telemetry_settings.metrics() { + if !settings::get_setting::(None, cx).metrics { return; } @@ -285,7 +284,7 @@ impl Telemetry { event: ClickhouseEvent, telemetry_settings: TelemetrySettings, ) { - if !telemetry_settings.metrics() { + if !telemetry_settings.metrics { return; } @@ -321,7 +320,7 @@ impl Telemetry { properties: Value, telemetry_settings: TelemetrySettings, ) { - if !telemetry_settings.metrics() { + if !telemetry_settings.metrics { return; } diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs index 6b3aa7e442..4c2721ffeb 100644 --- a/crates/client/src/user.rs +++ b/crates/client/src/user.rs @@ -5,7 +5,6 @@ use futures::{channel::mpsc, future, AsyncReadExt, Future, StreamExt}; use gpui::{AsyncAppContext, Entity, ImageData, ModelContext, ModelHandle, Task}; use postage::{sink::Sink, watch}; use rpc::proto::{RequestMessage, UsersResponse}; -use settings::Settings; use staff_mode::StaffMode; use std::sync::{Arc, Weak}; use util::http::HttpClient; @@ -144,11 +143,13 @@ impl UserStore { let fetch_metrics_id = client.request(proto::GetPrivateUserInfo {}).log_err(); let (user, info) = futures::join!(fetch_user, fetch_metrics_id); - client.telemetry.set_authenticated_user_info( - info.as_ref().map(|info| info.metrics_id.clone()), - info.as_ref().map(|info| info.staff).unwrap_or(false), - cx.read(|cx| cx.global::().telemetry()), - ); + cx.read(|cx| { + client.telemetry.set_authenticated_user_info( + info.as_ref().map(|info| info.metrics_id.clone()), + info.as_ref().map(|info| info.staff).unwrap_or(false), + cx, + ) + }); cx.update(|cx| { cx.update_default_global(|staff_mode: &mut StaffMode, _| { diff --git a/crates/copilot_button/src/copilot_button.rs b/crates/copilot_button/src/copilot_button.rs index fec48f1f34..61c84bc517 100644 --- a/crates/copilot_button/src/copilot_button.rs +++ b/crates/copilot_button/src/copilot_button.rs @@ -366,7 +366,7 @@ async fn configure_disabled_globs( fn toggle_copilot_globally(fs: Arc, cx: &mut AppContext) { let show_copilot_suggestions = cx.global::().show_copilot_suggestions(None, None); - update_settings_file(fs, cx, move |file_contents| { + update_settings_file::(fs, cx, move |file_contents| { file_contents.editor.show_copilot_suggestions = Some((!show_copilot_suggestions).into()) }); } @@ -376,7 +376,7 @@ fn toggle_copilot_for_language(language: Arc, fs: Arc, cx: &mut App .global::() .show_copilot_suggestions(Some(&language), None); - update_settings_file(fs, cx, move |file_contents| { + update_settings_file::(fs, cx, move |file_contents| { file_contents.languages.insert( language, settings::EditorSettings { @@ -388,7 +388,7 @@ fn toggle_copilot_for_language(language: Arc, fs: Arc, cx: &mut App } fn hide_copilot(fs: Arc, cx: &mut AppContext) { - update_settings_file(fs, cx, move |file_contents| { + update_settings_file::(fs, cx, move |file_contents| { file_contents.features.copilot = Some(false) }); } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 964b98450b..abea2d8f3a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -22,7 +22,7 @@ pub mod test; use aho_corasick::AhoCorasick; use anyhow::{anyhow, Result}; use blink_manager::BlinkManager; -use client::ClickhouseEvent; +use client::{ClickhouseEvent, TelemetrySettings}; use clock::ReplicaId; use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; use copilot::Copilot; @@ -6873,7 +6873,7 @@ impl Editor { .untyped_user_settings() .get("vim_mode") == Some(&serde_json::Value::Bool(true)); - + let telemetry_settings = *settings::get_setting::(None, cx); let settings = cx.global::(); let extension = Path::new(file.file_name(cx)) @@ -6887,7 +6887,7 @@ impl Editor { _ => name, }, json!({ "File Extension": extension, "Vim Mode": vim_mode, "In Clickhouse": true }), - settings.telemetry(), + telemetry_settings, ); let event = ClickhouseEvent::Editor { file_extension: extension.map(ToString::to_string), @@ -6903,7 +6903,7 @@ impl Editor { .as_deref(), ), }; - telemetry.report_clickhouse_event(event, settings.telemetry()) + telemetry.report_clickhouse_event(event, telemetry_settings) } } diff --git a/crates/settings/Cargo.toml b/crates/settings/Cargo.toml index 6d6d303a82..ba9bc38b46 100644 --- a/crates/settings/Cargo.toml +++ b/crates/settings/Cargo.toml @@ -27,7 +27,7 @@ glob.workspace = true json_comments = "0.2" lazy_static.workspace = true postage.workspace = true -schemars = "0.8" +schemars.workspace = true serde.workspace = true serde_derive.workspace = true serde_json.workspace = true diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 5284d3a69a..82a4148c0d 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -58,8 +58,6 @@ pub struct Settings { pub language_overrides: HashMap, EditorSettings>, pub lsp: HashMap, LspSettings>, pub theme: Arc, - pub telemetry_defaults: TelemetrySettings, - pub telemetry_overrides: TelemetrySettings, pub base_keymap: BaseKeymap, } @@ -133,8 +131,6 @@ impl Setting for Settings { language_overrides: Default::default(), lsp: defaults.lsp.clone(), theme: themes.get(defaults.theme.as_ref().unwrap()).unwrap(), - telemetry_defaults: defaults.telemetry, - telemetry_overrides: Default::default(), base_keymap: Default::default(), features: Features { copilot: defaults.features.copilot.unwrap(), @@ -260,30 +256,6 @@ impl BaseKeymap { } } -#[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] -pub struct TelemetrySettings { - diagnostics: Option, - metrics: Option, -} - -impl TelemetrySettings { - pub fn metrics(&self) -> bool { - self.metrics.unwrap() - } - - pub fn diagnostics(&self) -> bool { - self.diagnostics.unwrap() - } - - pub fn set_metrics(&mut self, value: bool) { - self.metrics = Some(value); - } - - pub fn set_diagnostics(&mut self, value: bool) { - self.diagnostics = Some(value); - } -} - #[derive(Clone, Debug, Default)] pub struct CopilotSettings { pub disabled_globs: Vec, @@ -569,8 +541,6 @@ pub struct SettingsFileContent { #[serde(default)] pub theme: Option, #[serde(default)] - pub telemetry: TelemetrySettings, - #[serde(default)] pub base_keymap: Option, #[serde(default)] pub features: FeaturesContent, @@ -685,8 +655,6 @@ impl Settings { language_overrides: Default::default(), lsp: defaults.lsp.clone(), theme: themes.get(&defaults.theme.unwrap()).unwrap(), - telemetry_defaults: defaults.telemetry, - telemetry_overrides: Default::default(), base_keymap: Default::default(), features: Features { copilot: defaults.features.copilot.unwrap(), @@ -758,7 +726,6 @@ impl Settings { self.terminal_overrides.copy_on_select = data.terminal.copy_on_select; self.terminal_overrides = data.terminal; self.language_overrides = data.languages; - self.telemetry_overrides = data.telemetry; self.lsp = data.lsp; } @@ -869,27 +836,6 @@ impl Settings { }) } - pub fn telemetry(&self) -> TelemetrySettings { - TelemetrySettings { - diagnostics: Some(self.telemetry_diagnostics()), - metrics: Some(self.telemetry_metrics()), - } - } - - pub fn telemetry_diagnostics(&self) -> bool { - self.telemetry_overrides - .diagnostics - .or(self.telemetry_defaults.diagnostics) - .expect("missing default") - } - - pub fn telemetry_metrics(&self) -> bool { - self.telemetry_overrides - .metrics - .or(self.telemetry_defaults.metrics) - .expect("missing default") - } - fn terminal_setting(&self, f: F) -> R where F: Fn(&TerminalSettings) -> Option, @@ -963,11 +909,6 @@ impl Settings { language_overrides: Default::default(), lsp: Default::default(), theme: gpui::fonts::with_font_cache(cx.font_cache().clone(), Default::default), - telemetry_defaults: TelemetrySettings { - diagnostics: Some(true), - metrics: Some(true), - }, - telemetry_overrides: Default::default(), base_keymap: Default::default(), features: Features { copilot: true }, } diff --git a/crates/settings/src/settings_file.rs b/crates/settings/src/settings_file.rs index a07560307f..990ccf0249 100644 --- a/crates/settings/src/settings_file.rs +++ b/crates/settings/src/settings_file.rs @@ -1,6 +1,6 @@ use crate::{ settings_store::parse_json_with_comments, settings_store::SettingsStore, KeymapFileContent, - Setting, Settings, SettingsFileContent, DEFAULT_SETTINGS_ASSET_PATH, + Setting, Settings, DEFAULT_SETTINGS_ASSET_PATH, }; use anyhow::Result; use assets::Assets; @@ -158,10 +158,10 @@ async fn load_settings(fs: &Arc) -> Result { } } -pub fn update_settings_file( +pub fn update_settings_file( fs: Arc, cx: &mut AppContext, - update: impl 'static + Send + FnOnce(&mut SettingsFileContent), + update: impl 'static + Send + FnOnce(&mut T::FileContent), ) { cx.spawn(|cx| async move { let old_text = cx @@ -172,10 +172,7 @@ pub fn update_settings_file( }) .await?; - let edits = cx.read(|cx| { - cx.global::() - .update::(&old_text, update) - }); + let edits = cx.read(|cx| cx.global::().update::(&old_text, update)); let mut new_text = old_text; for (range, replacement) in edits.into_iter().rev() { diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index a35e546891..27c5a9ef4e 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -129,7 +129,7 @@ impl PickerDelegate for ThemeSelectorDelegate { self.selection_completed = true; let theme_name = cx.global::().theme.meta.name.clone(); - update_settings_file(self.fs.clone(), cx, |settings_content| { + update_settings_file::(self.fs.clone(), cx, |settings_content| { settings_content.theme = Some(theme_name); }); diff --git a/crates/welcome/Cargo.toml b/crates/welcome/Cargo.toml index 696a5c5e4e..82cd4ca35a 100644 --- a/crates/welcome/Cargo.toml +++ b/crates/welcome/Cargo.toml @@ -13,6 +13,7 @@ test-support = [] [dependencies] anyhow.workspace = true log.workspace = true +client = { path = "../client" } editor = { path = "../editor" } fs = { path = "../fs" } fuzzy = { path = "../fuzzy" } diff --git a/crates/welcome/src/base_keymap_picker.rs b/crates/welcome/src/base_keymap_picker.rs index c0e9e0a38d..24600d5b82 100644 --- a/crates/welcome/src/base_keymap_picker.rs +++ b/crates/welcome/src/base_keymap_picker.rs @@ -122,7 +122,7 @@ impl PickerDelegate for BaseKeymapSelectorDelegate { fn confirm(&mut self, cx: &mut ViewContext) { if let Some(selection) = self.matches.get(self.selected_index) { let base_keymap = BaseKeymap::from_names(&selection.string); - update_settings_file(self.fs.clone(), cx, move |settings| { + update_settings_file::(self.fs.clone(), cx, move |settings| { settings.base_keymap = Some(base_keymap) }); } diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index c2e65dc524..6b8fe7312d 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -2,6 +2,7 @@ mod base_keymap_picker; use std::{borrow::Cow, sync::Arc}; +use client::TelemetrySettings; use db::kvp::KEY_VALUE_STORE; use gpui::{ elements::{Flex, Label, ParentElement}, @@ -63,10 +64,7 @@ impl View for WelcomePage { let width = theme.welcome.page_width; - let (diagnostics, metrics) = { - let telemetry = settings.telemetry(); - (telemetry.diagnostics(), telemetry.metrics()) - }; + let telemetry_settings = *settings::get_setting::(None, cx); enum Metrics {} enum Diagnostics {} @@ -166,15 +164,17 @@ impl View for WelcomePage { .with_style(theme.welcome.usage_note.container), ), &theme.welcome.checkbox, - metrics, + telemetry_settings.metrics, 0, cx, |this, checked, cx| { if let Some(workspace) = this.workspace.upgrade(cx) { let fs = workspace.read(cx).app_state().fs.clone(); - update_settings_file(fs, cx, move |file| { - file.telemetry.set_metrics(checked) - }) + update_settings_file::( + fs, + cx, + move |setting| setting.metrics = Some(checked), + ) } }, ) @@ -185,15 +185,17 @@ impl View for WelcomePage { theme::ui::checkbox::( "Send crash reports", &theme.welcome.checkbox, - diagnostics, + telemetry_settings.diagnostics, 0, cx, |this, checked, cx| { if let Some(workspace) = this.workspace.upgrade(cx) { let fs = workspace.read(cx).app_state().fs.clone(); - update_settings_file(fs, cx, move |file| { - file.telemetry.set_diagnostics(checked) - }) + update_settings_file::( + fs, + cx, + move |setting| setting.diagnostics = Some(checked), + ) } }, ) diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 3d43109e6b..58c68c56d3 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -8,7 +8,7 @@ use cli::{ ipc::{self, IpcSender}, CliRequest, CliResponse, IpcHandshake, }; -use client::{self, UserStore, ZED_APP_VERSION, ZED_SECRET_CLIENT_TOKEN}; +use client::{self, TelemetrySettings, UserStore, ZED_APP_VERSION, ZED_SECRET_CLIENT_TOKEN}; use db::kvp::KEY_VALUE_STORE; use editor::Editor; use futures::{ @@ -187,7 +187,7 @@ fn main() { client.telemetry().report_mixpanel_event( "start app", Default::default(), - cx.global::().telemetry(), + *settings::get_setting::(None, cx), ); let app_state = Arc::new(AppState { @@ -407,7 +407,7 @@ fn init_panic_hook(app_version: String) { } fn upload_previous_panics(http: Arc, cx: &mut AppContext) { - let diagnostics_telemetry = cx.global::().telemetry_diagnostics(); + let telemetry_settings = *settings::get_setting::(None, cx); cx.background() .spawn({ @@ -437,7 +437,7 @@ fn upload_previous_panics(http: Arc, cx: &mut AppContext) { continue; }; - if diagnostics_telemetry { + if telemetry_settings.diagnostics { let panic_data_text = smol::fs::read_to_string(&child_path) .await .context("error reading panic file")?;