From 04499da11ebd514cd8a8db13b20395532f6c179c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 10 Jul 2025 22:05:08 +0200 Subject: [PATCH] feat(telemetry): grab env and `distinct_id` from Sentry session (#9801) At present, our primary indicator as to whether telemetry is active is whether we have a Sentry session. For our analytics events however, we currently require passing in the Firezone ID and API url again. This makes it difficult to send analytics events from areas of the code that don't have this information available. To still allow for that, we integrate the `analytics` module more tightly with the Sentry session. This allows us to drop two parameters from the `$identify` event and also means we now respect the `NO_TELEMETRY` setting for these events except for `new_session`. This event is sent regardless because it allows us to track, how many on-prem installations of Firezone are out there. --- rust/apple-client-ffi/src/lib.rs | 7 +-- rust/client-ffi/src/lib.rs | 7 +-- rust/gateway/src/eventloop.rs | 10 +--- rust/gateway/src/main.rs | 11 +--- rust/gui-client/src-tauri/src/service.rs | 15 ++--- rust/headless-client/src/main.rs | 9 +-- rust/telemetry/src/analytics.rs | 71 ++++++++---------------- rust/telemetry/src/api_url.rs | 25 +++++++++ rust/telemetry/src/lib.rs | 31 ++++++++--- 9 files changed, 84 insertions(+), 102 deletions(-) create mode 100644 rust/telemetry/src/api_url.rs diff --git a/rust/apple-client-ffi/src/lib.rs b/rust/apple-client-ffi/src/lib.rs index a65b563eb..ba7d34fdc 100644 --- a/rust/apple-client-ffi/src/lib.rs +++ b/rust/apple-client-ffi/src/lib.rs @@ -264,12 +264,7 @@ impl WrappedSession { runtime.block_on(telemetry.start(&api_url, RELEASE, APPLE_DSN, device_id.clone())); Telemetry::set_account_slug(account_slug.clone()); - analytics::identify( - device_id.clone(), - api_url.to_string(), - RELEASE.to_owned(), - Some(account_slug), - ); + analytics::identify(RELEASE.to_owned(), Some(account_slug)); init_logging(log_dir.into(), log_filter)?; install_rustls_crypto_provider(); diff --git a/rust/client-ffi/src/lib.rs b/rust/client-ffi/src/lib.rs index a3e14cdc9..9cbe05d1e 100644 --- a/rust/client-ffi/src/lib.rs +++ b/rust/client-ffi/src/lib.rs @@ -237,12 +237,7 @@ fn connect( runtime.block_on(telemetry.start(&api_url, RELEASE, platform::DSN, device_id.clone())); Telemetry::set_account_slug(account_slug.clone()); - analytics::identify( - device_id.clone(), - api_url.to_string(), - RELEASE.to_owned(), - Some(account_slug), - ); + analytics::identify(RELEASE.to_owned(), Some(account_slug)); init_logging(&PathBuf::from(log_dir), log_filter)?; install_rustls_crypto_provider(); diff --git a/rust/gateway/src/eventloop.rs b/rust/gateway/src/eventloop.rs index 08cde5ae5..908147411 100644 --- a/rust/gateway/src/eventloop.rs +++ b/rust/gateway/src/eventloop.rs @@ -52,7 +52,6 @@ pub struct Eventloop { tunnel: GatewayTunnel, portal: PhoenixChannel<(), IngressMessages, (), PublicKeyParam>, tun_device_manager: Arc>, - firezone_id: String, resolve_tasks: futures_bounded::FuturesTupleSet, Arc>, ResolveTrigger>, @@ -68,14 +67,12 @@ impl Eventloop { tunnel: GatewayTunnel, mut portal: PhoenixChannel<(), IngressMessages, (), PublicKeyParam>, tun_device_manager: TunDeviceManager, - firezone_id: String, ) -> Self { portal.connect(PublicKeyParam(tunnel.public_key().to_bytes())); Self { tunnel, portal, - firezone_id, tun_device_manager: Arc::new(Mutex::new(tun_device_manager)), resolve_tasks: futures_bounded::FuturesTupleSet::new(DNS_RESOLUTION_TIMEOUT, 1000), set_interface_tasks: futures_bounded::FuturesSet::new(Duration::from_secs(5), 10), @@ -385,12 +382,7 @@ impl Eventloop { if let Some(account_slug) = init.account_slug { Telemetry::set_account_slug(account_slug.clone()); - analytics::identify( - self.firezone_id.clone(), - self.portal.url(), - RELEASE.to_owned(), - Some(account_slug), - ) + analytics::identify(RELEASE.to_owned(), Some(account_slug)) } self.tunnel.state_mut().update_relays( diff --git a/rust/gateway/src/main.rs b/rust/gateway/src/main.rs index 7a993ad15..a09a43487 100644 --- a/rust/gateway/src/main.rs +++ b/rust/gateway/src/main.rs @@ -134,13 +134,8 @@ async fn try_main(cli: Cli, telemetry: &mut Telemetry) -> Result<()> { opentelemetry::global::set_meter_provider(provider); } - let login = LoginUrl::gateway( - cli.api_url, - &cli.token, - firezone_id.clone(), - cli.firezone_name, - ) - .context("Failed to construct URL for logging into portal")?; + let login = LoginUrl::gateway(cli.api_url, &cli.token, firezone_id, cli.firezone_name) + .context("Failed to construct URL for logging into portal")?; let resolv_conf = resolv_conf::Config::parse( std::fs::read_to_string("/etc/resolv.conf").context("Failed to read /etc/resolv.conf")?, @@ -184,7 +179,7 @@ async fn try_main(cli: Cli, telemetry: &mut Telemetry) -> Result<()> { } let eventloop = future::poll_fn({ - let mut eventloop = Eventloop::new(tunnel, portal, tun_device_manager, firezone_id); + let mut eventloop = Eventloop::new(tunnel, portal, tun_device_manager); move |cx| eventloop.poll(cx) }); diff --git a/rust/gui-client/src-tauri/src/service.rs b/rust/gui-client/src-tauri/src/service.rs index 7cf8a0ac4..aa39a28e8 100644 --- a/rust/gui-client/src-tauri/src/service.rs +++ b/rust/gui-client/src-tauri/src/service.rs @@ -587,18 +587,13 @@ impl<'a> Handler<'a> { firezone_telemetry::GUI_DSN, self.device_id.id.clone(), ) - .await - } + .await; - if let Some(account_slug) = account_slug { - Telemetry::set_account_slug(account_slug.clone()); + if let Some(account_slug) = account_slug { + Telemetry::set_account_slug(account_slug.clone()); - analytics::identify( - self.device_id.id.clone(), - environment, - release, - Some(account_slug), - ); + analytics::identify(release, Some(account_slug)); + } } } } diff --git a/rust/headless-client/src/main.rs b/rust/headless-client/src/main.rs index 3f4fc5412..b2454b4f6 100644 --- a/rust/headless-client/src/main.rs +++ b/rust/headless-client/src/main.rs @@ -179,13 +179,6 @@ fn main() -> Result<()> { 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, }; - analytics::identify( - firezone_id.clone(), - cli.api_url.to_string(), - RELEASE.to_owned(), - None, - ); - let mut telemetry = Telemetry::default(); if cli.is_telemetry_allowed() { rt.block_on(telemetry.start( @@ -194,6 +187,8 @@ fn main() -> Result<()> { firezone_telemetry::HEADLESS_DSN, firezone_id.clone(), )); + + analytics::identify(RELEASE.to_owned(), None); } tracing::info!(arch = std::env::consts::ARCH, version = VERSION); diff --git a/rust/telemetry/src/analytics.rs b/rust/telemetry/src/analytics.rs index 3c21e3c5b..1c165d037 100644 --- a/rust/telemetry/src/analytics.rs +++ b/rust/telemetry/src/analytics.rs @@ -4,8 +4,11 @@ use anyhow::{Context as _, Result, bail}; use serde::Serialize; use sha2::Digest as _; -use crate::{Env, posthog::RUNTIME}; +use crate::{ApiUrl, Env, Telemetry, posthog::RUNTIME}; +/// Records a `new_session` event for a particular user and API url. +/// +/// This purposely does not use the existing telemetry session because we also want to capture sessions from self-hosted users. pub fn new_session(maybe_legacy_id: String, api_url: String) { let distinct_id = if uuid::Uuid::from_str(&maybe_legacy_id).is_ok() { hex::encode(sha2::Sha256::digest(&maybe_legacy_id)) @@ -17,8 +20,10 @@ pub fn new_session(maybe_legacy_id: String, api_url: String) { if let Err(e) = capture( "new_session", distinct_id, - api_url.clone(), - NewSessionProperties { api_url }, + ApiUrl::new(&api_url), + NewSessionProperties { + api_url: api_url.clone(), + }, ) .await { @@ -27,30 +32,23 @@ pub fn new_session(maybe_legacy_id: String, api_url: String) { }); } -/// Associate several properties with a particular "distinct_id" in PostHog. -pub fn identify( - maybe_legacy_id: String, - api_url: String, - release: String, - account_slug: Option, -) { - let is_legacy_id = uuid::Uuid::from_str(&maybe_legacy_id).is_ok(); - - let distinct_id = if is_legacy_id { - hex::encode(sha2::Sha256::digest(&maybe_legacy_id)) - } else { - maybe_legacy_id.clone() +/// Associate several properties with the current telemetry user. +pub fn identify(release: String, account_slug: Option) { + let Some(env) = Telemetry::current_env() else { + tracing::debug!("Cannot send $identify: Unknown env"); + return; + }; + let Some(distinct_id) = Telemetry::current_user() else { + tracing::debug!("Cannot send $identify: Unknown user"); + return; }; RUNTIME.spawn({ - let distinct_id = distinct_id.clone(); - let api_url = api_url.clone(); - async move { if let Err(e) = capture( "$identify", distinct_id, - api_url, + env, IdentifyProperties { set: PersonProperties { release, @@ -65,39 +63,20 @@ pub fn identify( } } }); - - // Create an alias ID for the user so we can also find them under the "external ID" used in the portal. - if is_legacy_id { - RUNTIME.spawn(async move { - if let Err(e) = capture( - "$create_alias", - distinct_id.clone(), - api_url, - CreateAliasProperties { - alias: maybe_legacy_id, - distinct_id, - }, - ) - .await - { - tracing::debug!("Failed to log `$create_alias` event: {e:#}"); - } - }); - } } async fn capture

( event: impl Into, distinct_id: String, - api_url: String, + env: impl Into, properties: P, ) -> Result<()> where P: Serialize, { let event = event.into(); + let env = env.into(); - let env = Env::from_api_url(&api_url); let Some(api_key) = crate::posthog::api_key_for_env(env) else { tracing::debug!(%event, %env, "Not sending event because we don't have an API key"); @@ -111,7 +90,7 @@ where .json(&CaptureRequest { api_key: api_key.to_string(), distinct_id, - event, + event: event.clone(), properties, }) .send() @@ -126,6 +105,8 @@ where bail!("Failed to capture event; status={status}, body={body}") } + tracing::debug!(%event); + Ok(()) } @@ -156,9 +137,3 @@ struct PersonProperties { #[serde(rename = "$os")] os: String, } - -#[derive(serde::Serialize)] -struct CreateAliasProperties { - distinct_id: String, - alias: String, -} diff --git a/rust/telemetry/src/api_url.rs b/rust/telemetry/src/api_url.rs new file mode 100644 index 000000000..f2160d7c8 --- /dev/null +++ b/rust/telemetry/src/api_url.rs @@ -0,0 +1,25 @@ +#[derive(Debug, PartialEq)] +pub(crate) struct ApiUrl<'a>(&'a str); + +impl ApiUrl<'static> { + pub(crate) const PROD: Self = ApiUrl("wss://api.firezone.dev"); + pub(crate) const STAGING: Self = ApiUrl("wss://api.firez.one"); + pub(crate) const DOCKER_COMPOSE: Self = ApiUrl("ws://api:8081"); + pub(crate) const LOCALHOST: Self = ApiUrl("ws://localhost:8081"); +} + +impl<'a> ApiUrl<'a> { + pub(crate) fn new(url: &'a str) -> Self { + Self(url.trim_end_matches("/")) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn trailing_slash_is_trimmed() { + assert_eq!(ApiUrl::new("wss://api.firezone.dev/"), ApiUrl::PROD) + } +} diff --git a/rust/telemetry/src/lib.rs b/rust/telemetry/src/lib.rs index 4907a6e2f..d77326bd2 100644 --- a/rust/telemetry/src/lib.rs +++ b/rust/telemetry/src/lib.rs @@ -3,6 +3,7 @@ use std::{borrow::Cow, collections::BTreeMap, fmt, str::FromStr, sync::Arc, time::Duration}; use anyhow::{Result, bail}; +use api_url::ApiUrl; use sentry::{ BeforeCallback, User, protocol::{Event, Log, LogAttribute, SessionStatus}, @@ -13,6 +14,7 @@ pub mod analytics; pub mod feature_flags; pub mod otel; +mod api_url; mod posthog; pub struct Dsn(&'static str); @@ -53,17 +55,18 @@ pub(crate) enum Env { OnPrem, } -impl Env { - pub(crate) fn from_api_url(api_url: &str) -> Self { - match api_url.trim_end_matches('/') { - "wss://api.firezone.dev" => Self::Production, - "wss://api.firez.one" => Self::Staging, - "ws://api:8081" => Self::DockerCompose, - "ws://localhost:8081" => Self::DockerCompose, +impl From> for Env { + fn from(value: ApiUrl) -> Self { + match value { + ApiUrl::PROD => Self::Production, + ApiUrl::STAGING => Self::Staging, + ApiUrl::DOCKER_COMPOSE | ApiUrl::LOCALHOST => Self::DockerCompose, _ => Self::OnPrem, } } +} +impl Env { pub(crate) fn as_str(&self) -> &'static str { match self { Env::Production => "production", @@ -116,7 +119,7 @@ impl Telemetry { pub async fn start(&mut self, api_url: &str, release: &str, dsn: Dsn, firezone_id: String) { // Can't use URLs as `environment` directly, because Sentry doesn't allow slashes in environments. // - let environment = Env::from_api_url(api_url); + let environment = Env::from(ApiUrl::new(api_url)); if self .inner @@ -213,6 +216,18 @@ impl Telemetry { user.other.insert("account_slug".to_owned(), slug.into()); }); } + + pub(crate) fn current_env() -> Option { + let client = sentry::Hub::main().client()?; + let env = client.options().environment.as_deref()?; + let env = Env::from_str(env).ok()?; + + Some(env) + } + + pub(crate) fn current_user() -> Option { + sentry::Hub::main().configure_scope(|s| s.user()?.id.clone()) + } } /// Computes the [`User`] scope based on the contents of `firezone_id`.