fix(rust/client): set sentry release version and environment correctly (#6855)

Closes #6854 


- Sets release version from the GUI Client / Headless Client version
instead of the `firezone-telemetry` version
- Set environment to "production" and "staging" for well-known API URLs,
and "self-hosted" for others, since environments in Sentry can't have
slashes in them
- Sets API URL as a tag
- Sets release to `unit test` for unit testing `firezone-telemetry`
itself, since it has no good version number

<img width="398" alt="image"
src="https://github.com/user-attachments/assets/86f71193-2511-45c1-8304-413db8e5ef90">
This commit is contained in:
Reactor Scram
2024-09-30 11:24:39 -05:00
committed by GitHub
parent 9644b0c0b1
commit d2a8155ba7
8 changed files with 51 additions and 19 deletions

View File

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

View File

@@ -20,6 +20,7 @@ defaults:
env:
RUSTFLAGS: "-Dwarnings"
RUSTDOCFLAGS: "-D warnings"
SENTRY_ENVIRONMENT: "production"
jobs:
build-gui:

View File

@@ -191,8 +191,11 @@ impl<I: GuiIntegration> Controller<I> {
// 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?;

View File

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

View File

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

View File

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

View File

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

View File

@@ -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.
// <https://docs.sentry.io/platforms/rust/configuration/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");
}
}