diff --git a/.github/workflows/_build_artifacts.yml b/.github/workflows/_build_artifacts.yml index 0e470e850..18ba32bd4 100644 --- a/.github/workflows/_build_artifacts.yml +++ b/.github/workflows/_build_artifacts.yml @@ -173,6 +173,7 @@ jobs: image_name: http-test-server env: BINARY_DEST_PATH: ${{ matrix.name.artifact }}_${{ matrix.name.version }}_${{ matrix.arch.shortname }} + SENTRY_ENVIRONMENT: "production" outputs: client_image: ${{ steps.image-name.outputs.client_image }} relay_image: ${{ steps.image-name.outputs.relay_image }} diff --git a/.github/workflows/_tauri.yml b/.github/workflows/_tauri.yml index 8d41ef711..5997e140a 100644 --- a/.github/workflows/_tauri.yml +++ b/.github/workflows/_tauri.yml @@ -20,6 +20,7 @@ defaults: env: RUSTFLAGS: "-Dwarnings" RUSTDOCFLAGS: "-D warnings" + SENTRY_ENVIRONMENT: "production" jobs: build-gui: diff --git a/rust/gui-client/src-common/src/controller.rs b/rust/gui-client/src-common/src/controller.rs index 7cac429fb..92d0c2194 100644 --- a/rust/gui-client/src-common/src/controller.rs +++ b/rust/gui-client/src-common/src/controller.rs @@ -191,8 +191,11 @@ impl Controller { // Start telemetry { let environment = self.advanced_settings.api_url.to_string(); - self.telemetry - .start(environment.clone(), firezone_telemetry::GUI_DSN); + self.telemetry.start( + &environment, + firezone_bin_shared::git_version!("gui-client-*"), + firezone_telemetry::GUI_DSN, + ); self.ipc_client .send_msg(&IpcClientMsg::StartTelemetry { environment }) .await?; diff --git a/rust/gui-client/src-common/src/settings.rs b/rust/gui-client/src-common/src/settings.rs index 9a54275ba..2a9b4d0ad 100644 --- a/rust/gui-client/src-common/src/settings.rs +++ b/rust/gui-client/src-common/src/settings.rs @@ -23,14 +23,14 @@ pub struct AdvancedSettings { #[cfg(debug_assertions)] mod defaults { pub(crate) const AUTH_BASE_URL: &str = "https://app.firez.one"; - pub(crate) const API_URL: &str = "wss://api.firez.one"; + pub(crate) const API_URL: &str = "wss://api.firez.one/"; pub(crate) const LOG_FILTER: &str = "firezone_gui_client=debug,info"; } #[cfg(not(debug_assertions))] mod defaults { pub(crate) const AUTH_BASE_URL: &str = "https://app.firezone.dev"; - pub(crate) const API_URL: &str = "wss://api.firezone.dev"; + pub(crate) const API_URL: &str = "wss://api.firezone.dev/"; pub(crate) const LOG_FILTER: &str = "info"; } diff --git a/rust/gui-client/src-tauri/src/client.rs b/rust/gui-client/src-tauri/src/client.rs index cbc04a972..408d240ce 100644 --- a/rust/gui-client/src-tauri/src/client.rs +++ b/rust/gui-client/src-tauri/src/client.rs @@ -60,7 +60,11 @@ pub(crate) fn run() -> Result<()> { // Can't check elevation here because the Windows CI is always elevated let settings = common::settings::load_advanced_settings().unwrap_or_default(); let telemetry = telemetry::Telemetry::default(); - telemetry.start(settings.api_url.to_string(), telemetry::GUI_DSN); + telemetry.start( + settings.api_url.as_ref(), + firezone_bin_shared::git_version!("gui-client-*"), + telemetry::GUI_DSN, + ); // Don't fix the log filter for smoke tests let common::logging::Handles { logger: _logger, @@ -88,7 +92,11 @@ fn run_gui(cli: Cli) -> Result<()> { let mut settings = common::settings::load_advanced_settings().unwrap_or_default(); let telemetry = 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.to_string(), telemetry::GUI_DSN); + telemetry.start( + settings.api_url.as_ref(), + firezone_bin_shared::git_version!("gui-client-*"), + telemetry::GUI_DSN, + ); fix_log_filter(&mut settings)?; let common::logging::Handles { logger: _logger, diff --git a/rust/headless-client/src/ipc_service.rs b/rust/headless-client/src/ipc_service.rs index efd08f6e1..34731851d 100644 --- a/rust/headless-client/src/ipc_service.rs +++ b/rust/headless-client/src/ipc_service.rs @@ -498,9 +498,11 @@ impl<'a> Handler<'a> { session.connlib.set_disabled_resources(disabled_resources); } - ClientMsg::StartTelemetry { environment } => self - .telemetry - .start(environment, firezone_telemetry::IPC_SERVICE_DSN), + ClientMsg::StartTelemetry { environment } => self.telemetry.start( + &environment, + firezone_bin_shared::git_version!("gui-client-*"), + firezone_telemetry::IPC_SERVICE_DSN, + ), ClientMsg::StopTelemetry => self.telemetry.stop(), } Ok(()) diff --git a/rust/headless-client/src/main.rs b/rust/headless-client/src/main.rs index 4aef0af75..04b396888 100644 --- a/rust/headless-client/src/main.rs +++ b/rust/headless-client/src/main.rs @@ -52,7 +52,7 @@ struct Cli { long, hide = true, env = "FIREZONE_API_URL", - default_value = "wss://api.firezone.dev" + default_value = "wss://api.firezone.dev/" )] api_url: url::Url, @@ -116,7 +116,11 @@ fn main() -> Result<()> { let token_env_var = cli.token.take().map(SecretString::from); let cli = cli; let telemetry = Telemetry::default(); - telemetry.start(cli.api_url.to_string(), firezone_telemetry::HEADLESS_DSN); + telemetry.start( + cli.api_url.as_ref(), + firezone_bin_shared::git_version!("headless-client-*"), + firezone_telemetry::HEADLESS_DSN, + ); // Docs indicate that `remove_var` should actually be marked unsafe // SAFETY: We haven't spawned any other threads, this code should be the first @@ -382,10 +386,10 @@ mod tests { fn cli() { let exe_name = "firezone-headless-client"; - let actual = Cli::try_parse_from([exe_name, "--api-url", "wss://api.firez.one"]).unwrap(); + let actual = Cli::try_parse_from([exe_name, "--api-url", "wss://api.firez.one/"]).unwrap(); assert_eq!( actual.api_url, - Url::parse("wss://api.firez.one").expect("Hard-coded URL should always be parsable") + Url::parse("wss://api.firez.one/").expect("Hard-coded URL should always be parsable") ); assert!(!actual.check); diff --git a/rust/telemetry/src/lib.rs b/rust/telemetry/src/lib.rs index 0e87a35eb..6dfc343ee 100644 --- a/rust/telemetry/src/lib.rs +++ b/rust/telemetry/src/lib.rs @@ -42,22 +42,33 @@ impl Clone for Telemetry { } impl Telemetry { - pub fn start(&self, environment: String, dsn: Dsn) { + pub fn start(&self, api_url: &str, release: &'static str, dsn: Dsn) { // Since it's `arc_swap` and not `Option`, there is a TOCTOU here, // but in practice it should never trigger if self.inner.load().is_some() { return; } + + // Can't use URLs as `environment` directly, because Sentry doesn't allow slashes in environments. + // + let environment = match api_url { + "wss://api.firezone.dev/" => "production", + "wss://api.firez.one/" => "staging", + _ => "self-hosted", + }; + tracing::info!("Starting telemetry"); let inner = sentry::init(( dsn.0, sentry::ClientOptions { environment: Some(environment.into()), - release: sentry::release_name!(), + // We can't get the release number ourselves because we don't know if we're embedded in a GUI Client or a Headless Client. + release: Some(release.into()), traces_sample_rate: 1.0, ..Default::default() }, )); + sentry::configure_scope(|scope| scope.set_tag("api_url", api_url)); self.inner.swap(Some(inner.into())); sentry::start_session(); } @@ -83,6 +94,8 @@ impl Telemetry { mod tests { use super::*; + const ENV: &str = "unit test"; + // To avoid problems with global mutable state, we run unrelated tests in the same test case. #[test] fn sentry() { @@ -110,7 +123,7 @@ mod tests { // Expect no telemetry because the telemetry module needs to be enabled before it can do anything negative_error("X7X4CKH3"); - tele.start("test".to_string(), HEADLESS_DSN); + tele.start("test", ENV, HEADLESS_DSN); // Expect telemetry because the user opted in. sentry::add_breadcrumb(sentry::Breadcrumb { ty: "test_crumb".into(), @@ -123,7 +136,7 @@ mod tests { // Expect no telemetry because the user opted back out. negative_error("2RSIYAPX"); - tele.start("test".to_string(), HEADLESS_DSN); + tele.start("test", ENV, HEADLESS_DSN); // Cycle one more time to be sure. error("S672IOBZ"); tele.stop(); @@ -137,12 +150,12 @@ mod tests { { let tele = Telemetry::default(); negative_error("4H7HFTNX"); - tele.start("test".to_string(), HEADLESS_DSN); + tele.start("test", ENV, HEADLESS_DSN); } { negative_error("GF46D6IL"); let tele = Telemetry::default(); - tele.start("test".to_string(), HEADLESS_DSN); + tele.start("test", ENV, HEADLESS_DSN); error("OKOEUKSW"); } }