Merge pull request #1842 from zed-industries/telemetry-corrections

Joseph T. Lyons created

Telemetry corrections

Change summary

.github/workflows/ci.yml                  |   1 
.github/workflows/release_actions.yml     |  12 -
crates/client/src/amplitude_telemetry.rs  | 282 -------------------------
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(+), 351 deletions(-)

Detailed changes

.github/workflows/ci.yml 🔗

@@ -60,7 +60,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

.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 }}

crates/client/src/amplitude_telemetry.rs 🔗

@@ -1,282 +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 settings::ReleaseChannel;
-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<dyn HttpClient>,
-    executor: Arc<Background>,
-    session_id: u128,
-    state: Mutex<AmplitudeTelemetryState>,
-}
-
-#[derive(Default)]
-struct AmplitudeTelemetryState {
-    metrics_id: Option<Arc<str>>,
-    device_id: Option<Arc<str>>,
-    app_version: Option<Arc<str>>,
-    release_channel: Option<&'static str>,
-    os_version: Option<Arc<str>>,
-    os_name: &'static str,
-    queue: Vec<AmplitudeEvent>,
-    next_event_id: usize,
-    flush_task: Option<Task<()>>,
-    log_file: Option<NamedTempFile>,
-}
-
-const AMPLITUDE_EVENTS_URL: &'static str = "https://api2.amplitude.com/batch";
-
-lazy_static! {
-    static ref AMPLITUDE_API_KEY: Option<String> = 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<AmplitudeEvent>,
-}
-
-#[derive(Serialize)]
-struct AmplitudeEvent {
-    #[serde(skip_serializing_if = "Option::is_none")]
-    user_id: Option<Arc<str>>,
-    device_id: Option<Arc<str>>,
-    event_type: String,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    event_properties: Option<Map<String, Value>>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    user_properties: Option<Map<String, Value>>,
-    os_name: &'static str,
-    os_version: Option<Arc<str>>,
-    app_version: Option<Arc<str>>,
-    #[serde(rename = "App")]
-    app: &'static str,
-    release_channel: Option<&'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<dyn HttpClient>, cx: &AppContext) -> Arc<Self> {
-        let platform = cx.platform();
-        let release_channel = if cx.has_global::<ReleaseChannel>() {
-            Some(cx.global::<ReleaseChannel>().name())
-        } else {
-            None
-        };
-        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()),
-                release_channel,
-                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<PathBuf> {
-        Some(self.state.lock().log_file.as_ref()?.path().to_path_buf())
-    }
-
-    pub fn start(self: &Arc<Self>, 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<Self>,
-        metrics_id: Option<String>,
-        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<Self>, kind: &str, properties: Value) {
-        self.report_event_with_user_properties(kind, properties, Default::default());
-    }
-
-    fn report_event_with_user_properties(
-        self: &Arc<Self>,
-        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(),
-            release_channel: state.release_channel,
-            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<Self>) {
-        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();
-        }
-    }
-}

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<Peer>,
     http: Arc<dyn HttpClient>,
     telemetry: Arc<Telemetry>,
-    amplitude_telemetry: Arc<AmplitudeTelemetry>,
     state: RwLock<ClientState>,
 
     #[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();
             }
             _ => {}
@@ -1032,7 +1026,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
@@ -1117,7 +1110,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()?,
@@ -1230,16 +1222,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<PathBuf> {
-        self.amplitude_telemetry.log_file_path();
         self.telemetry.log_file_path()
     }
 }

crates/client/src/telemetry.rs 🔗

@@ -32,6 +32,7 @@ pub struct Telemetry {
 struct TelemetryState {
     metrics_id: Option<Arc<str>>,
     device_id: Option<Arc<str>>,
+    app: &'static str,
     app_version: Option<Arc<str>>,
     release_channel: Option<&'static str>,
     os_version: Option<Arc<str>>,
@@ -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);

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();
                             }
                         }

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}");
     }

script/amplitude_release/main.py 🔗

@@ -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()