fix(gui-client): update telemetry context on new session (#8152)

Every time we start a new session, our telemetry context potentially
changes, i.e. the user may sign into a new account. This should ensure
that both the IPC service and the GUI always use the most up-to-date
`account_slug` as part of Sentry events. In addition, this will also set
the `account_slug` for clients that just signed in. Previously, the
`account_slug` would only get populated on the next start of the client.
This commit is contained in:
Thomas Eizinger
2025-02-17 14:29:08 +11:00
committed by GitHub
parent 159e8e3dd5
commit 72782b8389
9 changed files with 52 additions and 50 deletions

View File

@@ -328,7 +328,7 @@ fn connect(
let mut telemetry = Telemetry::default();
telemetry.start(&api_url, RELEASE, ANDROID_DSN);
telemetry.set_firezone_id(device_id.clone());
Telemetry::set_firezone_id(device_id.clone());
init_logging(&PathBuf::from(log_dir), log_filter)?;
install_rustls_crypto_provider();

View File

@@ -239,8 +239,8 @@ impl WrappedSession {
) -> Result<Self> {
let mut telemetry = Telemetry::default();
telemetry.start(&api_url, RELEASE, APPLE_DSN);
telemetry.set_firezone_id(device_id.clone());
telemetry.set_account_slug(account_slug);
Telemetry::set_firezone_id(device_id.clone());
Telemetry::set_account_slug(account_slug);
init_logging(log_dir.into(), log_filter)?;
install_rustls_crypto_provider();

View File

@@ -61,7 +61,7 @@ fn main() -> ExitCode {
.expect("Failed to create tokio runtime");
match runtime
.block_on(try_main(cli, &mut telemetry))
.block_on(try_main(cli))
.context("Failed to start Gateway")
{
Ok(ExitCode::SUCCESS) => {
@@ -97,13 +97,13 @@ fn has_necessary_permissions() -> bool {
is_root || has_net_admin
}
async fn try_main(cli: Cli, telemetry: &mut Telemetry) -> Result<ExitCode> {
async fn try_main(cli: Cli) -> Result<ExitCode> {
firezone_logging::setup_global_subscriber(layer::Identity::default())
.context("Failed to set up logging")?;
let firezone_id = get_firezone_id(cli.firezone_id).await
.context("Couldn't read FIREZONE_ID or write it to disk: Please provide it through the env variable or provide rw access to /var/lib/firezone/")?;
telemetry.set_firezone_id(firezone_id.clone());
Telemetry::set_firezone_id(firezone_id.clone());
let login = LoginUrl::gateway(
cli.api_url,

View File

@@ -31,7 +31,7 @@ mod ran_before;
pub type CtlrTx = mpsc::Sender<ControllerRequest>;
pub struct Controller<'a, I: GuiIntegration> {
pub struct Controller<I: GuiIntegration> {
/// Debugging-only settings like API URL, auth URL, log filter
advanced_settings: AdvancedSettings,
// Sign-in state with the portal / deep links
@@ -46,7 +46,6 @@ pub struct Controller<'a, I: GuiIntegration> {
release: Option<updates::Release>,
rx: ReceiverStream<ControllerRequest>,
status: Status,
telemetry: &'a mut Telemetry,
updates_rx: ReceiverStream<Option<updates::Notification>>,
uptime: crate::uptime::Tracker,
@@ -179,14 +178,13 @@ enum EventloopTick {
UpdateNotification(Option<Option<updates::Notification>>),
}
impl<I: GuiIntegration> Controller<'_, I> {
impl<I: GuiIntegration> Controller<I> {
pub async fn start(
ctlr_tx: CtlrTx,
integration: I,
rx: mpsc::Receiver<ControllerRequest>,
advanced_settings: AdvancedSettings,
log_filter_reloader: LogFilterReloader,
telemetry: &mut Telemetry,
updates_rx: mpsc::Receiver<Option<updates::Notification>>,
) -> Result<(), Error> {
tracing::debug!("Starting new instance of `Controller`");
@@ -208,7 +206,6 @@ impl<I: GuiIntegration> Controller<'_, I> {
release: None,
rx: ReceiverStream::new(rx),
status: Default::default(),
telemetry,
updates_rx: ReceiverStream::new(updates_rx),
uptime: Default::default(),
dns_notifier,
@@ -221,23 +218,7 @@ impl<I: GuiIntegration> Controller<'_, I> {
}
pub async fn main_loop(mut self) -> Result<(), Error> {
let account_slug = self.auth.session().map(|s| s.account_slug.to_owned());
// Tell IPC service to start telemetry.
{
let environment = self.advanced_settings.api_url.to_string();
if let Some(account_slug) = account_slug.clone() {
self.telemetry.set_account_slug(account_slug);
}
self.ipc_client
.send_msg(&IpcClientMsg::StartTelemetry {
environment,
release: crate::RELEASE.to_string(),
account_slug,
})
.await?;
}
self.update_telemetry_context().await?;
if let Some(token) = self
.auth
@@ -371,6 +352,25 @@ impl<I: GuiIntegration> Controller<'_, I> {
Ok(())
}
async fn update_telemetry_context(&mut self) -> Result<()> {
let environment = self.advanced_settings.api_url.to_string();
let account_slug = self.auth.session().map(|s| s.account_slug.to_owned());
if let Some(account_slug) = account_slug.clone() {
Telemetry::set_account_slug(account_slug);
}
self.ipc_client
.send_msg(&IpcClientMsg::StartTelemetry {
environment,
release: crate::RELEASE.to_string(),
account_slug,
})
.await?;
Ok(())
}
async fn handle_deep_link(&mut self, url: &SecretString) -> Result<(), Error> {
let auth_response =
deep_link::parse_auth_callback(url).context("Couldn't parse scheme request")?;
@@ -382,7 +382,10 @@ impl<I: GuiIntegration> Controller<'_, I> {
.auth
.handle_response(auth_response)
.context("Couldn't handle auth response")?;
self.update_telemetry_context().await?;
self.start_session(token).await?;
Ok(())
}

View File

@@ -3,7 +3,7 @@ use clap::{Args, Parser};
use firezone_gui_client_common::{
self as common, controller::Failure, deep_link, settings::AdvancedSettings,
};
use firezone_telemetry as telemetry;
use firezone_telemetry::Telemetry;
use tracing::instrument;
use tracing_subscriber::EnvFilter;
@@ -60,11 +60,11 @@ pub(crate) fn run() -> Result<()> {
Some(Cmd::SmokeTest) => {
// Can't check elevation here because the Windows CI is always elevated
let settings = common::settings::load_advanced_settings().unwrap_or_default();
let mut telemetry = telemetry::Telemetry::default();
let mut telemetry = Telemetry::default();
telemetry.start(
settings.api_url.as_ref(),
firezone_gui_client_common::RELEASE,
telemetry::GUI_DSN,
firezone_telemetry::GUI_DSN,
);
// Don't fix the log filter for smoke tests
let common::logging::Handles {
@@ -91,17 +91,17 @@ pub(crate) fn run() -> Result<()> {
// Can't `instrument` this because logging isn't running when we enter it.
fn run_gui(cli: Cli) -> Result<()> {
let mut settings = common::settings::load_advanced_settings().unwrap_or_default();
let mut telemetry = telemetry::Telemetry::default();
let mut telemetry = Telemetry::default();
// In the future telemetry will be opt-in per organization, that's why this isn't just at the top of `main`
telemetry.start(
settings.api_url.as_ref(),
firezone_gui_client_common::RELEASE,
telemetry::GUI_DSN,
firezone_telemetry::GUI_DSN,
);
// Get the device ID before starting Tokio, so that all the worker threads will inherit the correct scope.
// Technically this means we can fail to get the device ID on a newly-installed system, since the IPC service may not have fully started up when the GUI process reaches this point, but in practice it's unlikely.
if let Ok(id) = firezone_headless_client::device_id::get() {
telemetry.set_firezone_id(id.id);
Telemetry::set_firezone_id(id.id);
}
fix_log_filter(&mut settings)?;
let common::logging::Handles {

View File

@@ -242,7 +242,6 @@ pub(crate) fn run(
ctlr_rx,
advanced_settings,
reloader,
&mut telemetry,
updates_rx,
)).catch_unwind().await;

View File

@@ -257,7 +257,7 @@ async fn ipc_listen(
.context("Failed to read / create device ID")?
.id;
telemetry.set_firezone_id(firezone_id);
Telemetry::set_firezone_id(firezone_id);
let mut server = IpcServer::new(ServiceId::Prod).await?;
let mut dns_controller = DnsController { dns_control_method };
@@ -564,7 +564,7 @@ impl<'a> Handler<'a> {
.start(&environment, &release, firezone_telemetry::GUI_DSN);
if let Some(account_slug) = account_slug {
self.telemetry.set_account_slug(account_slug);
Telemetry::set_account_slug(account_slug);
}
}
}
@@ -581,7 +581,7 @@ impl<'a> Handler<'a> {
assert!(self.session.is_none());
let device_id = device_id::get_or_create().context("Failed to get-or-create device ID")?;
self.telemetry.set_firezone_id(device_id.id.clone());
Telemetry::set_firezone_id(device_id.id.clone());
let url = LoginUrl::client(
Url::parse(api_url).context("Failed to parse URL")?,

View File

@@ -179,7 +179,7 @@ fn main() -> Result<()> {
Some(id) => id,
None => device_id::get_or_create().context("Could not get `firezone_id` from CLI, could not read it from disk, could not generate it and save it to disk")?.id,
};
telemetry.set_firezone_id(firezone_id.clone());
Telemetry::set_firezone_id(firezone_id.clone());
let url = LoginUrl::client(
cli.api_url,

View File

@@ -143,26 +143,26 @@ impl Telemetry {
.await;
}
pub fn set_account_slug(&mut self, slug: String) {
self.update_user(|user| {
pub fn set_account_slug(slug: String) {
update_user(|user| {
user.other.insert("account_slug".to_owned(), slug.into());
});
}
pub fn set_firezone_id(&mut self, id: String) {
self.update_user(|user| {
pub fn set_firezone_id(id: String) {
update_user(|user| {
user.id = Some(id);
});
}
}
fn update_user(&mut self, update: impl FnOnce(&mut sentry::User)) {
sentry::Hub::main().configure_scope(|scope| {
let mut user = scope.user().cloned().unwrap_or_default();
update(&mut user);
fn update_user(update: impl FnOnce(&mut sentry::User)) {
sentry::Hub::main().configure_scope(|scope| {
let mut user = scope.user().cloned().unwrap_or_default();
update(&mut user);
scope.set_user(Some(user));
});
}
scope.set_user(Some(user));
});
}
fn set_current_user(user: Option<sentry::User>) {