From 577a3bcc336c5e9aa122a9dab9ecfd5b0bf8b031 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 2 Nov 2022 12:36:48 -0400 Subject: [PATCH] Merge pull request #1842 from zed-industries/telemetry-corrections Telemetry corrections --- .github/workflows/ci.yml | 1 - .github/workflows/release_actions.yml | 12 - crates/client/src/amplitude_telemetry.rs | 272 ---------------------- crates/client/src/client.rs | 11 - crates/client/src/telemetry.rs | 4 +- crates/client/src/user.rs | 10 - crates/zed/build.rs | 3 - script/amplitude_release/main.py | 30 --- script/amplitude_release/requirements.txt | 1 - 9 files changed, 3 insertions(+), 341 deletions(-) delete mode 100644 crates/client/src/amplitude_telemetry.rs delete mode 100644 script/amplitude_release/main.py delete mode 100644 script/amplitude_release/requirements.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cb15cd2c2d..6b62ed3e1a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,7 +57,6 @@ jobs: MACOS_CERTIFICATE_PASSWORD: ${{ secrets.MACOS_CERTIFICATE_PASSWORD }} APPLE_NOTARIZATION_USERNAME: ${{ secrets.APPLE_NOTARIZATION_USERNAME }} APPLE_NOTARIZATION_PASSWORD: ${{ secrets.APPLE_NOTARIZATION_PASSWORD }} - ZED_AMPLITUDE_API_KEY: ${{ secrets.ZED_AMPLITUDE_API_KEY }} ZED_MIXPANEL_TOKEN: ${{ secrets.ZED_MIXPANEL_TOKEN }} steps: - name: Install Rust diff --git a/.github/workflows/release_actions.yml b/.github/workflows/release_actions.yml index 7328ea3fd1..8ee8e4121c 100644 --- a/.github/workflows/release_actions.yml +++ b/.github/workflows/release_actions.yml @@ -20,15 +20,3 @@ jobs: ### Changelog ${{ github.event.release.body }} - ``` - amplitude_release: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-python@v4 - with: - python-version: "3.10.5" - architecture: "x64" - cache: "pip" - - run: pip install -r script/amplitude_release/requirements.txt - - run: python script/amplitude_release/main.py ${{ github.event.release.tag_name }} ${{ secrets.ZED_AMPLITUDE_API_KEY }} ${{ secrets.ZED_AMPLITUDE_SECRET_KEY }} diff --git a/crates/client/src/amplitude_telemetry.rs b/crates/client/src/amplitude_telemetry.rs deleted file mode 100644 index f463dbf078..0000000000 --- a/crates/client/src/amplitude_telemetry.rs +++ /dev/null @@ -1,272 +0,0 @@ -use crate::http::HttpClient; -use db::Db; -use gpui::{ - executor::Background, - serde_json::{self, value::Map, Value}, - AppContext, Task, -}; -use isahc::Request; -use lazy_static::lazy_static; -use parking_lot::Mutex; -use serde::Serialize; -use serde_json::json; -use std::{ - io::Write, - mem, - path::PathBuf, - sync::Arc, - time::{Duration, SystemTime, UNIX_EPOCH}, -}; -use tempfile::NamedTempFile; -use util::{post_inc, ResultExt, TryFutureExt}; -use uuid::Uuid; - -pub struct AmplitudeTelemetry { - http_client: Arc, - executor: Arc, - session_id: u128, - state: Mutex, -} - -#[derive(Default)] -struct AmplitudeTelemetryState { - metrics_id: Option>, - device_id: Option>, - app_version: Option>, - os_version: Option>, - os_name: &'static str, - queue: Vec, - next_event_id: usize, - flush_task: Option>, - log_file: Option, -} - -const AMPLITUDE_EVENTS_URL: &'static str = "https://api2.amplitude.com/batch"; - -lazy_static! { - static ref AMPLITUDE_API_KEY: Option = std::env::var("ZED_AMPLITUDE_API_KEY") - .ok() - .or_else(|| option_env!("ZED_AMPLITUDE_API_KEY").map(|key| key.to_string())); -} - -#[derive(Serialize)] -struct AmplitudeEventBatch { - api_key: &'static str, - events: Vec, -} - -#[derive(Serialize)] -struct AmplitudeEvent { - #[serde(skip_serializing_if = "Option::is_none")] - user_id: Option>, - device_id: Option>, - event_type: String, - #[serde(skip_serializing_if = "Option::is_none")] - event_properties: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - user_properties: Option>, - os_name: &'static str, - os_version: Option>, - app_version: Option>, - #[serde(rename = "App")] - app: &'static str, - event_id: usize, - session_id: u128, - time: u128, -} - -#[cfg(debug_assertions)] -const MAX_QUEUE_LEN: usize = 1; - -#[cfg(not(debug_assertions))] -const MAX_QUEUE_LEN: usize = 10; - -#[cfg(debug_assertions)] -const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(1); - -#[cfg(not(debug_assertions))] -const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); - -impl AmplitudeTelemetry { - pub fn new(client: Arc, cx: &AppContext) -> Arc { - let platform = cx.platform(); - let this = Arc::new(Self { - http_client: client, - executor: cx.background().clone(), - session_id: SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_millis(), - state: Mutex::new(AmplitudeTelemetryState { - os_version: platform.os_version().ok().map(|v| v.to_string().into()), - os_name: platform.os_name().into(), - app_version: platform.app_version().ok().map(|v| v.to_string().into()), - device_id: None, - queue: Default::default(), - flush_task: Default::default(), - next_event_id: 0, - log_file: None, - metrics_id: None, - }), - }); - - if AMPLITUDE_API_KEY.is_some() { - this.executor - .spawn({ - let this = this.clone(); - async move { - if let Some(tempfile) = NamedTempFile::new().log_err() { - this.state.lock().log_file = Some(tempfile); - } - } - }) - .detach(); - } - - this - } - - pub fn log_file_path(&self) -> Option { - Some(self.state.lock().log_file.as_ref()?.path().to_path_buf()) - } - - pub fn start(self: &Arc, db: Db) { - let this = self.clone(); - self.executor - .spawn( - async move { - let device_id = if let Ok(Some(device_id)) = db.read_kvp("device_id") { - device_id - } else { - let device_id = Uuid::new_v4().to_string(); - db.write_kvp("device_id", &device_id)?; - device_id - }; - - let device_id = Some(Arc::from(device_id)); - let mut state = this.state.lock(); - state.device_id = device_id.clone(); - for event in &mut state.queue { - event.device_id = device_id.clone(); - } - if !state.queue.is_empty() { - drop(state); - this.flush(); - } - - anyhow::Ok(()) - } - .log_err(), - ) - .detach(); - } - - pub fn set_authenticated_user_info( - self: &Arc, - metrics_id: Option, - is_staff: bool, - ) { - let is_signed_in = metrics_id.is_some(); - self.state.lock().metrics_id = metrics_id.map(|s| s.into()); - if is_signed_in { - self.report_event_with_user_properties( - "$identify", - Default::default(), - json!({ "$set": { "staff": is_staff } }), - ) - } - } - - pub fn report_event(self: &Arc, kind: &str, properties: Value) { - self.report_event_with_user_properties(kind, properties, Default::default()); - } - - fn report_event_with_user_properties( - self: &Arc, - kind: &str, - properties: Value, - user_properties: Value, - ) { - if AMPLITUDE_API_KEY.is_none() { - return; - } - - let mut state = self.state.lock(); - let event = AmplitudeEvent { - event_type: kind.to_string(), - time: SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_millis(), - session_id: self.session_id, - event_properties: if let Value::Object(properties) = properties { - Some(properties) - } else { - None - }, - user_properties: if let Value::Object(user_properties) = user_properties { - Some(user_properties) - } else { - None - }, - user_id: state.metrics_id.clone(), - device_id: state.device_id.clone(), - os_name: state.os_name, - app: "Zed", - os_version: state.os_version.clone(), - app_version: state.app_version.clone(), - event_id: post_inc(&mut state.next_event_id), - }; - state.queue.push(event); - if state.device_id.is_some() { - if state.queue.len() >= MAX_QUEUE_LEN { - drop(state); - self.flush(); - } else { - let this = self.clone(); - let executor = self.executor.clone(); - state.flush_task = Some(self.executor.spawn(async move { - executor.timer(DEBOUNCE_INTERVAL).await; - this.flush(); - })); - } - } - } - - fn flush(self: &Arc) { - let mut state = self.state.lock(); - let events = mem::take(&mut state.queue); - state.flush_task.take(); - drop(state); - - if let Some(api_key) = AMPLITUDE_API_KEY.as_ref() { - let this = self.clone(); - self.executor - .spawn( - async move { - let mut json_bytes = Vec::new(); - - if let Some(file) = &mut this.state.lock().log_file { - let file = file.as_file_mut(); - for event in &events { - json_bytes.clear(); - serde_json::to_writer(&mut json_bytes, event)?; - file.write_all(&json_bytes)?; - file.write(b"\n")?; - } - } - - let batch = AmplitudeEventBatch { api_key, events }; - json_bytes.clear(); - serde_json::to_writer(&mut json_bytes, &batch)?; - let request = - Request::post(AMPLITUDE_EVENTS_URL).body(json_bytes.into())?; - this.http_client.send(request).await?; - Ok(()) - } - .log_err(), - ) - .detach(); - } - } -} diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index c4a3167f83..c55d5f1681 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -1,13 +1,11 @@ #[cfg(any(test, feature = "test-support"))] pub mod test; -pub mod amplitude_telemetry; pub mod channel; pub mod http; pub mod telemetry; pub mod user; -use amplitude_telemetry::AmplitudeTelemetry; use anyhow::{anyhow, Context, Result}; use async_recursion::async_recursion; use async_tungstenite::tungstenite::{ @@ -85,7 +83,6 @@ pub struct Client { peer: Arc, http: Arc, telemetry: Arc, - amplitude_telemetry: Arc, state: RwLock, #[allow(clippy::type_complexity)] @@ -265,7 +262,6 @@ impl Client { id: 0, peer: Peer::new(), telemetry: Telemetry::new(http.clone(), cx), - amplitude_telemetry: AmplitudeTelemetry::new(http.clone(), cx), http, state: Default::default(), @@ -378,8 +374,6 @@ impl Client { } Status::SignedOut | Status::UpgradeRequired => { self.telemetry.set_authenticated_user_info(None, false); - self.amplitude_telemetry - .set_authenticated_user_info(None, false); state._reconnect_task.take(); } _ => {} @@ -1029,7 +1023,6 @@ impl Client { let platform = cx.platform(); let executor = cx.background(); let telemetry = self.telemetry.clone(); - let amplitude_telemetry = self.amplitude_telemetry.clone(); let http = self.http.clone(); executor.clone().spawn(async move { // Generate a pair of asymmetric encryption keys. The public key will be used by the @@ -1114,7 +1107,6 @@ impl Client { platform.activate(true); telemetry.report_event("authenticate with browser", Default::default()); - amplitude_telemetry.report_event("authenticate with browser", Default::default()); Ok(Credentials { user_id: user_id.parse()?, @@ -1227,16 +1219,13 @@ impl Client { pub fn start_telemetry(&self, db: Db) { self.telemetry.start(db.clone()); - self.amplitude_telemetry.start(db); } pub fn report_event(&self, kind: &str, properties: Value) { self.telemetry.report_event(kind, properties.clone()); - self.amplitude_telemetry.report_event(kind, properties); } pub fn telemetry_log_file_path(&self) -> Option { - self.amplitude_telemetry.log_file_path(); self.telemetry.log_file_path() } } diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 01adeccb50..1a0238cd0f 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -32,6 +32,7 @@ pub struct Telemetry { struct TelemetryState { metrics_id: Option>, device_id: Option>, + app: &'static str, app_version: Option>, release_channel: Option<&'static str>, os_version: Option>, @@ -119,6 +120,7 @@ impl Telemetry { state: Mutex::new(TelemetryState { os_version: platform.os_version().ok().map(|v| v.to_string().into()), os_name: platform.os_name().into(), + app: "Zed", app_version: platform.app_version().ok().map(|v| v.to_string().into()), release_channel, device_id: None, @@ -239,7 +241,7 @@ impl Telemetry { release_channel: state.release_channel, app_version: state.app_version.clone(), signed_in: state.metrics_id.is_some(), - app: "Zed", + app: state.app, }, }; state.queue.push(event); diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs index d06a6682c5..11b9ef6117 100644 --- a/crates/client/src/user.rs +++ b/crates/client/src/user.rs @@ -146,21 +146,11 @@ impl UserStore { Some(info.metrics_id.clone()), info.staff, ); - client.amplitude_telemetry.set_authenticated_user_info( - Some(info.metrics_id), - info.staff, - ); } else { client.telemetry.set_authenticated_user_info(None, false); - client - .amplitude_telemetry - .set_authenticated_user_info(None, false); } client.telemetry.report_event("sign in", Default::default()); - client - .amplitude_telemetry - .report_event("sign in", Default::default()); current_user_tx.send(user).await.ok(); } } diff --git a/crates/zed/build.rs b/crates/zed/build.rs index 1f30e7ded2..8622842c44 100644 --- a/crates/zed/build.rs +++ b/crates/zed/build.rs @@ -4,9 +4,6 @@ fn main() { if let Ok(value) = std::env::var("ZED_MIXPANEL_TOKEN") { println!("cargo:rustc-env=ZED_MIXPANEL_TOKEN={value}"); } - if let Ok(value) = std::env::var("ZED_AMPLITUDE_API_KEY") { - println!("cargo:rustc-env=ZED_AMPLITUDE_API_KEY={value}"); - } if let Ok(value) = std::env::var("ZED_PREVIEW_CHANNEL") { println!("cargo:rustc-env=ZED_PREVIEW_CHANNEL={value}"); } diff --git a/script/amplitude_release/main.py b/script/amplitude_release/main.py deleted file mode 100644 index 160e40b66c..0000000000 --- a/script/amplitude_release/main.py +++ /dev/null @@ -1,30 +0,0 @@ -import datetime -import sys - -from amplitude_python_sdk.v2.clients.releases_client import ReleasesAPIClient -from amplitude_python_sdk.v2.models.releases import Release - - -def main(): - version = sys.argv[1] - version = version.removeprefix("v") - - api_key = sys.argv[2] - secret_key = sys.argv[3] - - current_datetime = datetime.datetime.now(datetime.timezone.utc) - current_datetime = current_datetime.strftime("%Y-%m-%d %H:%M:%S") - - release = Release( - title=version, - version=version, - release_start=current_datetime, - created_by="GitHub Release Workflow", - chart_visibility=True - ) - - ReleasesAPIClient(api_key=api_key, secret_key=secret_key).create(release) - - -if __name__ == "__main__": - main() \ No newline at end of file diff --git a/script/amplitude_release/requirements.txt b/script/amplitude_release/requirements.txt deleted file mode 100644 index 7ed3ea6515..0000000000 --- a/script/amplitude_release/requirements.txt +++ /dev/null @@ -1 +0,0 @@ -amplitude-python-sdk==0.2.0 \ No newline at end of file