Skip to content

Commit

Permalink
Add system_id (#18040)
Browse files Browse the repository at this point in the history
This PR adds `system_id` to telemetry, which is contained within a new
`global` database (accessible by any release channel of Zed on a single
system). This will help us get a more accurate understanding of user
count, instead of relying on `installationd_id`, which is different per
release channel. This doesn't solve the problem of a user with multiple
machines, but it gets us closer.

Release Notes:

- N/A
  • Loading branch information
JosephTLyons authored Sep 19, 2024
1 parent 5e6d181 commit ca4980d
Show file tree
Hide file tree
Showing 8 changed files with 184 additions and 62 deletions.
17 changes: 12 additions & 5 deletions crates/client/src/telemetry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ pub struct Telemetry {

struct TelemetryState {
settings: TelemetrySettings,
metrics_id: Option<Arc<str>>, // Per logged-in user
system_id: Option<Arc<str>>, // Per system
installation_id: Option<Arc<str>>, // Per app installation (different for dev, nightly, preview, and stable)
session_id: Option<String>, // Per app launch
metrics_id: Option<Arc<str>>, // Per logged-in user
release_channel: Option<&'static str>,
architecture: &'static str,
events_queue: Vec<EventWrapper>,
Expand Down Expand Up @@ -191,9 +192,10 @@ impl Telemetry {
settings: *TelemetrySettings::get_global(cx),
architecture: env::consts::ARCH,
release_channel,
system_id: None,
installation_id: None,
metrics_id: None,
session_id: None,
metrics_id: None,
events_queue: Vec::new(),
flush_events_task: None,
log_file: None,
Expand Down Expand Up @@ -283,11 +285,13 @@ impl Telemetry {

pub fn start(
self: &Arc<Self>,
system_id: Option<String>,
installation_id: Option<String>,
session_id: String,
cx: &mut AppContext,
) {
let mut state = self.state.lock();
state.system_id = system_id.map(|id| id.into());
state.installation_id = installation_id.map(|id| id.into());
state.session_id = Some(session_id);
state.app_version = release_channel::AppVersion::global(cx).to_string();
Expand Down Expand Up @@ -637,9 +641,10 @@ impl Telemetry {
let state = this.state.lock();

let request_body = EventRequestBody {
system_id: state.system_id.as_deref().map(Into::into),
installation_id: state.installation_id.as_deref().map(Into::into),
metrics_id: state.metrics_id.as_deref().map(Into::into),
session_id: state.session_id.clone(),
metrics_id: state.metrics_id.as_deref().map(Into::into),
is_staff: state.is_staff,
app_version: state.app_version.clone(),
os_name: state.os_name.clone(),
Expand Down Expand Up @@ -711,14 +716,15 @@ mod tests {
Utc.with_ymd_and_hms(1990, 4, 12, 12, 0, 0).unwrap(),
));
let http = FakeHttpClient::with_200_response();
let system_id = Some("system_id".to_string());
let installation_id = Some("installation_id".to_string());
let session_id = "session_id".to_string();

cx.update(|cx| {
let telemetry = Telemetry::new(clock.clone(), http, cx);

telemetry.state.lock().max_queue_size = 4;
telemetry.start(installation_id, session_id, cx);
telemetry.start(system_id, installation_id, session_id, cx);

assert!(is_empty_state(&telemetry));

Expand Down Expand Up @@ -796,13 +802,14 @@ mod tests {
Utc.with_ymd_and_hms(1990, 4, 12, 12, 0, 0).unwrap(),
));
let http = FakeHttpClient::with_200_response();
let system_id = Some("system_id".to_string());
let installation_id = Some("installation_id".to_string());
let session_id = "session_id".to_string();

cx.update(|cx| {
let telemetry = Telemetry::new(clock.clone(), http, cx);
telemetry.state.lock().max_queue_size = 4;
telemetry.start(installation_id, session_id, cx);
telemetry.start(system_id, installation_id, session_id, cx);

assert!(is_empty_state(&telemetry));

Expand Down
25 changes: 20 additions & 5 deletions crates/collab/src/api/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ pub async fn post_crash(
installation_id = %installation_id,
description = %description,
backtrace = %summary,
"crash report");
"crash report"
);

if let Some(slack_panics_webhook) = app.config.slack_panics_webhook.clone() {
let payload = slack::WebhookBody::new(|w| {
Expand Down Expand Up @@ -627,7 +628,9 @@ where

#[derive(Serialize, Debug, clickhouse::Row)]
pub struct EditorEventRow {
system_id: String,
installation_id: String,
session_id: Option<String>,
metrics_id: String,
operation: String,
app_version: String,
Expand All @@ -647,7 +650,6 @@ pub struct EditorEventRow {
historical_event: bool,
architecture: String,
is_staff: Option<bool>,
session_id: Option<String>,
major: Option<i32>,
minor: Option<i32>,
patch: Option<i32>,
Expand Down Expand Up @@ -677,9 +679,10 @@ impl EditorEventRow {
os_name: body.os_name.clone(),
os_version: body.os_version.clone().unwrap_or_default(),
architecture: body.architecture.clone(),
system_id: body.system_id.clone().unwrap_or_default(),
installation_id: body.installation_id.clone().unwrap_or_default(),
metrics_id: body.metrics_id.clone().unwrap_or_default(),
session_id: body.session_id.clone(),
metrics_id: body.metrics_id.clone().unwrap_or_default(),
is_staff: body.is_staff,
time: time.timestamp_millis(),
operation: event.operation,
Expand All @@ -699,6 +702,7 @@ impl EditorEventRow {
#[derive(Serialize, Debug, clickhouse::Row)]
pub struct InlineCompletionEventRow {
installation_id: String,
session_id: Option<String>,
provider: String,
suggestion_accepted: bool,
app_version: String,
Expand All @@ -713,7 +717,6 @@ pub struct InlineCompletionEventRow {
city: String,
time: i64,
is_staff: Option<bool>,
session_id: Option<String>,
major: Option<i32>,
minor: Option<i32>,
patch: Option<i32>,
Expand Down Expand Up @@ -879,7 +882,9 @@ impl AssistantEventRow {

#[derive(Debug, clickhouse::Row, Serialize)]
pub struct CpuEventRow {
system_id: Option<String>,
installation_id: Option<String>,
session_id: Option<String>,
is_staff: Option<bool>,
usage_as_percentage: f32,
core_count: u32,
Expand All @@ -888,7 +893,6 @@ pub struct CpuEventRow {
os_name: String,
os_version: String,
time: i64,
session_id: Option<String>,
// pub normalized_cpu_usage: f64, MATERIALIZED
major: Option<i32>,
minor: Option<i32>,
Expand Down Expand Up @@ -917,6 +921,7 @@ impl CpuEventRow {
release_channel: body.release_channel.clone().unwrap_or_default(),
os_name: body.os_name.clone(),
os_version: body.os_version.clone().unwrap_or_default(),
system_id: body.system_id.clone(),
installation_id: body.installation_id.clone(),
session_id: body.session_id.clone(),
is_staff: body.is_staff,
Expand All @@ -940,6 +945,7 @@ pub struct MemoryEventRow {
os_version: String,

// ClientEventBase
system_id: Option<String>,
installation_id: Option<String>,
session_id: Option<String>,
is_staff: Option<bool>,
Expand Down Expand Up @@ -971,6 +977,7 @@ impl MemoryEventRow {
release_channel: body.release_channel.clone().unwrap_or_default(),
os_name: body.os_name.clone(),
os_version: body.os_version.clone().unwrap_or_default(),
system_id: body.system_id.clone(),
installation_id: body.installation_id.clone(),
session_id: body.session_id.clone(),
is_staff: body.is_staff,
Expand All @@ -994,6 +1001,7 @@ pub struct AppEventRow {
os_version: String,

// ClientEventBase
system_id: Option<String>,
installation_id: Option<String>,
session_id: Option<String>,
is_staff: Option<bool>,
Expand Down Expand Up @@ -1024,6 +1032,7 @@ impl AppEventRow {
release_channel: body.release_channel.clone().unwrap_or_default(),
os_name: body.os_name.clone(),
os_version: body.os_version.clone().unwrap_or_default(),
system_id: body.system_id.clone(),
installation_id: body.installation_id.clone(),
session_id: body.session_id.clone(),
is_staff: body.is_staff,
Expand All @@ -1046,6 +1055,7 @@ pub struct SettingEventRow {
os_version: String,

// ClientEventBase
system_id: Option<String>,
installation_id: Option<String>,
session_id: Option<String>,
is_staff: Option<bool>,
Expand Down Expand Up @@ -1076,6 +1086,7 @@ impl SettingEventRow {
release_channel: body.release_channel.clone().unwrap_or_default(),
os_name: body.os_name.clone(),
os_version: body.os_version.clone().unwrap_or_default(),
system_id: body.system_id.clone(),
installation_id: body.installation_id.clone(),
session_id: body.session_id.clone(),
is_staff: body.is_staff,
Expand All @@ -1099,6 +1110,7 @@ pub struct ExtensionEventRow {
os_version: String,

// ClientEventBase
system_id: Option<String>,
installation_id: Option<String>,
session_id: Option<String>,
is_staff: Option<bool>,
Expand Down Expand Up @@ -1134,6 +1146,7 @@ impl ExtensionEventRow {
release_channel: body.release_channel.clone().unwrap_or_default(),
os_name: body.os_name.clone(),
os_version: body.os_version.clone().unwrap_or_default(),
system_id: body.system_id.clone(),
installation_id: body.installation_id.clone(),
session_id: body.session_id.clone(),
is_staff: body.is_staff,
Expand Down Expand Up @@ -1224,6 +1237,7 @@ pub struct EditEventRow {
os_version: String,

// ClientEventBase
system_id: Option<String>,
installation_id: Option<String>,
// Note: This column name has a typo in the ClickHouse table.
#[serde(rename = "sesssion_id")]
Expand Down Expand Up @@ -1261,6 +1275,7 @@ impl EditEventRow {
release_channel: body.release_channel.clone().unwrap_or_default(),
os_name: body.os_name.clone(),
os_version: body.os_version.clone().unwrap_or_default(),
system_id: body.system_id.clone(),
installation_id: body.installation_id.clone(),
session_id: body.session_id.clone(),
is_staff: body.is_staff,
Expand Down
68 changes: 43 additions & 25 deletions crates/db/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,14 @@ pub use smol;
pub use sqlez;
pub use sqlez_macros;

use release_channel::ReleaseChannel;
pub use release_channel::RELEASE_CHANNEL;
use sqlez::domain::Migrator;
use sqlez::thread_safe_connection::ThreadSafeConnection;
use sqlez_macros::sql;
use std::env;
use std::future::Future;
use std::path::Path;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::LazyLock;
use std::sync::{atomic::Ordering, LazyLock};
use std::{env, sync::atomic::AtomicBool};
use util::{maybe, ResultExt};

const CONNECTION_INITIALIZE_QUERY: &str = sql!(
Expand All @@ -47,16 +45,12 @@ pub static ALL_FILE_DB_FAILED: LazyLock<AtomicBool> = LazyLock::new(|| AtomicBoo
/// This will retry a couple times if there are failures. If opening fails once, the db directory
/// is moved to a backup folder and a new one is created. If that fails, a shared in memory db is created.
/// In either case, static variables are set so that the user can be notified.
pub async fn open_db<M: Migrator + 'static>(
db_dir: &Path,
release_channel: &ReleaseChannel,
) -> ThreadSafeConnection<M> {
pub async fn open_db<M: Migrator + 'static>(db_dir: &Path, scope: &str) -> ThreadSafeConnection<M> {
if *ZED_STATELESS {
return open_fallback_db().await;
}

let release_channel_name = release_channel.dev_name();
let main_db_dir = db_dir.join(Path::new(&format!("0-{}", release_channel_name)));
let main_db_dir = db_dir.join(format!("0-{}", scope));

let connection = maybe!(async {
smol::fs::create_dir_all(&main_db_dir)
Expand Down Expand Up @@ -118,7 +112,7 @@ pub async fn open_test_db<M: Migrator>(db_name: &str) -> ThreadSafeConnection<M>
/// Implements a basic DB wrapper for a given domain
#[macro_export]
macro_rules! define_connection {
(pub static ref $id:ident: $t:ident<()> = $migrations:expr;) => {
(pub static ref $id:ident: $t:ident<()> = $migrations:expr; $($global:ident)?) => {
pub struct $t($crate::sqlez::thread_safe_connection::ThreadSafeConnection<$t>);

impl ::std::ops::Deref for $t {
Expand All @@ -139,18 +133,23 @@ macro_rules! define_connection {
}
}

use std::sync::LazyLock;
#[cfg(any(test, feature = "test-support"))]
pub static $id: LazyLock<$t> = LazyLock::new(|| {
pub static $id: std::sync::LazyLock<$t> = std::sync::LazyLock::new(|| {
$t($crate::smol::block_on($crate::open_test_db(stringify!($id))))
});

#[cfg(not(any(test, feature = "test-support")))]
pub static $id: LazyLock<$t> = LazyLock::new(|| {
$t($crate::smol::block_on($crate::open_db($crate::database_dir(), &$crate::RELEASE_CHANNEL)))
pub static $id: std::sync::LazyLock<$t> = std::sync::LazyLock::new(|| {
let db_dir = $crate::database_dir();
let scope = if false $(|| stringify!($global) == "global")? {
"global"
} else {
$crate::RELEASE_CHANNEL.dev_name()
};
$t($crate::smol::block_on($crate::open_db(db_dir, scope)))
});
};
(pub static ref $id:ident: $t:ident<$($d:ty),+> = $migrations:expr;) => {
(pub static ref $id:ident: $t:ident<$($d:ty),+> = $migrations:expr; $($global:ident)?) => {
pub struct $t($crate::sqlez::thread_safe_connection::ThreadSafeConnection<( $($d),+, $t )>);

impl ::std::ops::Deref for $t {
Expand Down Expand Up @@ -178,7 +177,13 @@ macro_rules! define_connection {

#[cfg(not(any(test, feature = "test-support")))]
pub static $id: std::sync::LazyLock<$t> = std::sync::LazyLock::new(|| {
$t($crate::smol::block_on($crate::open_db($crate::database_dir(), &$crate::RELEASE_CHANNEL)))
let db_dir = $crate::database_dir();
let scope = if false $(|| stringify!($global) == "global")? {
"global"
} else {
$crate::RELEASE_CHANNEL.dev_name()
};
$t($crate::smol::block_on($crate::open_db(db_dir, scope)))
});
};
}
Expand Down Expand Up @@ -225,7 +230,11 @@ mod tests {
.prefix("DbTests")
.tempdir()
.unwrap();
let _bad_db = open_db::<BadDB>(tempdir.path(), &release_channel::ReleaseChannel::Dev).await;
let _bad_db = open_db::<BadDB>(
tempdir.path(),
&release_channel::ReleaseChannel::Dev.dev_name(),
)
.await;
}

/// Test that DB exists but corrupted (causing recreate)
Expand Down Expand Up @@ -262,13 +271,19 @@ mod tests {
.tempdir()
.unwrap();
{
let corrupt_db =
open_db::<CorruptedDB>(tempdir.path(), &release_channel::ReleaseChannel::Dev).await;
let corrupt_db = open_db::<CorruptedDB>(
tempdir.path(),
&release_channel::ReleaseChannel::Dev.dev_name(),
)
.await;
assert!(corrupt_db.persistent());
}

let good_db =
open_db::<GoodDB>(tempdir.path(), &release_channel::ReleaseChannel::Dev).await;
let good_db = open_db::<GoodDB>(
tempdir.path(),
&release_channel::ReleaseChannel::Dev.dev_name(),
)
.await;
assert!(
good_db.select_row::<usize>("SELECT * FROM test2").unwrap()()
.unwrap()
Expand Down Expand Up @@ -311,8 +326,11 @@ mod tests {
.unwrap();
{
// Setup the bad database
let corrupt_db =
open_db::<CorruptedDB>(tempdir.path(), &release_channel::ReleaseChannel::Dev).await;
let corrupt_db = open_db::<CorruptedDB>(
tempdir.path(),
&release_channel::ReleaseChannel::Dev.dev_name(),
)
.await;
assert!(corrupt_db.persistent());
}

Expand All @@ -323,7 +341,7 @@ mod tests {
let guard = thread::spawn(move || {
let good_db = smol::block_on(open_db::<GoodDB>(
tmp_path.as_path(),
&release_channel::ReleaseChannel::Dev,
&release_channel::ReleaseChannel::Dev.dev_name(),
));
assert!(
good_db.select_row::<usize>("SELECT * FROM test2").unwrap()()
Expand Down
Loading

0 comments on commit ca4980d

Please sign in to comment.