From 2ca1fe0831ead72e2d5b366ce02712454240c200 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Tue, 16 Jul 2024 16:47:17 -0700 Subject: [PATCH] Remove imageworks-specific sentry configuration Relocate the `Sentry` config type to the spfs crate so it can be reused. Expect sentry configuration to exist in both the spfs and spk configs (the sentry configuration can be different DSNs between spfs and spk). Signed-off-by: J Robert Ray --- Cargo.lock | 1 + crates/spfs-cli/common/src/args.rs | 26 ++++++++++++++++---------- crates/spfs/src/config.rs | 21 +++++++++++++++++++++ crates/spfs/src/lib.rs | 1 + crates/spk-cli/common/src/env.rs | 5 +++++ crates/spk-config/Cargo.toml | 1 + crates/spk-config/src/config.rs | 17 +---------------- 7 files changed, 46 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6a122858f6..b767a7fc5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4164,6 +4164,7 @@ dependencies = [ "rstest 0.15.0", "serde", "serde_json", + "spfs", "thiserror", "tokio", "whoami", diff --git a/crates/spfs-cli/common/src/args.rs b/crates/spfs-cli/common/src/args.rs index cc2b87e641..eeac6a35e8 100644 --- a/crates/spfs-cli/common/src/args.rs +++ b/crates/spfs-cli/common/src/args.rs @@ -178,6 +178,10 @@ pub fn configure_sentry( use sentry::IntoDsn; + let Ok(config) = spfs::get_config() else { + return None; + }; + // SENTRY_USERNAME_OVERRIDE_VAR should hold the name of another // environment variable that can hold a username. If it does and // the other environment variable exists, its value will be used @@ -199,14 +203,13 @@ pub fn configure_sentry( let sentry_init_result = catch_unwind(|| { let mut opts = sentry::ClientOptions { - dsn: "http://3dd72e3b4b9a4032947304fabf29966e@sentry.spimageworks.com/4" - .into_dsn() - .unwrap_or(None), - environment: Some( - std::env::var("SENTRY_ENVIRONMENT") - .unwrap_or_else(|_| "production".to_string()) - .into(), - ), + dsn: config.sentry.dsn.as_str().into_dsn().unwrap_or_default(), + environment: config + .sentry + .environment + .as_ref() + .map(ToOwned::to_owned) + .map(std::borrow::Cow::Owned), // spdev follows sentry recommendation of using the release // tag as the name of the release in sentry release: Some(format!("v{}", spfs::VERSION).into()), @@ -251,8 +254,11 @@ pub fn configure_sentry( sentry::configure_scope(|scope| { scope.set_user(Some(sentry::protocol::User { - // TODO: make this configurable in future - email: Some(format!("{}@imageworks.com", &username)), + email: config + .sentry + .email_domain + .as_ref() + .map(|domain| format!("{username}@{domain}")), username: Some(username), ..Default::default() })); diff --git a/crates/spfs/src/config.rs b/crates/spfs/src/config.rs index 2414a91427..9b1e3e3742 100644 --- a/crates/spfs/src/config.rs +++ b/crates/spfs/src/config.rs @@ -403,6 +403,26 @@ impl Default for Monitor { } } +#[derive(Clone, Default, Debug, Deserialize, Serialize)] +#[serde(default)] +pub struct Sentry { + /// Sentry DSN + pub dsn: String, + + /// Sentry environment + pub environment: Option, + + /// Environment variable name to use as sentry username, if set. + /// + /// This is useful in CI if the CI system has a variable that contains + /// the username of the person who triggered the build. + pub username_override_var: Option, + + /// Set the email address domain used to generate email addresses for + /// sentry events. + pub email_domain: Option, +} + #[derive(Clone, Debug, Default, Deserialize, Serialize)] #[serde(default)] pub struct Config { @@ -412,6 +432,7 @@ pub struct Config { pub remote: std::collections::HashMap, pub fuse: Fuse, pub monitor: Monitor, + pub sentry: Sentry, } impl Config { diff --git a/crates/spfs/src/lib.rs b/crates/spfs/src/lib.rs index ede0f011bf..c62466f5c0 100644 --- a/crates/spfs/src/lib.rs +++ b/crates/spfs/src/lib.rs @@ -83,4 +83,5 @@ pub use self::config::{ Config, RemoteAddress, RemoteConfig, + Sentry, }; diff --git a/crates/spk-cli/common/src/env.rs b/crates/spk-cli/common/src/env.rs index b7a2c28a85..39b8f005b7 100644 --- a/crates/spk-cli/common/src/env.rs +++ b/crates/spk-cli/common/src/env.rs @@ -234,6 +234,11 @@ pub fn configure_sentry() -> Option { sentry::configure_scope(|scope| { scope.set_user(Some(sentry::User { + email: config + .sentry + .email_domain + .as_ref() + .map(|domain| format!("{username}@{domain}")), username: Some(username), ..Default::default() })); diff --git a/crates/spk-config/Cargo.toml b/crates/spk-config/Cargo.toml index c41ac8e5b4..691234f565 100644 --- a/crates/spk-config/Cargo.toml +++ b/crates/spk-config/Cargo.toml @@ -15,6 +15,7 @@ config = { workspace = true } dirs = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +spfs = { workspace = true } once_cell = { workspace = true } thiserror = { workspace = true } miette = { workspace = true } diff --git a/crates/spk-config/src/config.rs b/crates/spk-config/src/config.rs index 9671bb6fbb..bc30463f61 100644 --- a/crates/spk-config/src/config.rs +++ b/crates/spk-config/src/config.rs @@ -7,6 +7,7 @@ use std::sync::{Arc, RwLock}; use config::Environment; use once_cell::sync::OnceCell; use serde::{Deserialize, Serialize}; +use spfs::Sentry; use crate::Result; @@ -16,22 +17,6 @@ mod config_test; static CONFIG: OnceCell>> = OnceCell::new(); -#[derive(Clone, Default, Debug, Deserialize, Serialize)] -#[serde(default)] -pub struct Sentry { - /// Sentry DSN - pub dsn: String, - - /// Sentry environment - pub environment: Option, - - /// Environment variable name to use as sentry username, if set. - /// - /// This is useful in CI if the CI system has a variable that contains - /// the username of the person who triggered the build. - pub username_override_var: Option, -} - #[derive(Clone, Default, Debug, Deserialize, Serialize)] #[serde(default)] pub struct Metadata {