Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ sentry = { version = "0.46", default-features = false }
chrono = "0.4"
openssl = { version = "0.10", features = ["vendored"] }
sha1 = "0.10"
tracing = "0.1.41"
1 change: 1 addition & 0 deletions clients/rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ serde_json.workspace = true
thiserror.workspace = true
sha1.workspace = true
num = "0.4.3"
tracing.workspace = true

[dev-dependencies]
tempfile.workspace = true
69 changes: 9 additions & 60 deletions clients/rust/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use num::bigint::{BigInt, Sign};
use std::cell::Cell;
use std::collections::HashMap;
use std::sync::OnceLock;

use serde_json::Value;
use sha1::{Digest, Sha1};
Expand Down Expand Up @@ -345,61 +344,6 @@ fn eval_equals(ctx_val: &Value, condition_val: &Value) -> bool {
}
}

#[derive(Debug, PartialEq)]
enum DebugLogLevel {
None,
Parse,
Match,
All,
}

static DEBUG_LOG_LEVEL: OnceLock<DebugLogLevel> = OnceLock::new();
static DEBUG_MATCH_SAMPLE_RATE: OnceLock<u64> = OnceLock::new();

fn debug_log_level() -> &'static DebugLogLevel {
DEBUG_LOG_LEVEL.get_or_init(|| {
match std::env::var("SENTRY_OPTIONS_FEATURE_DEBUG_LOG")
.as_deref()
.unwrap_or("")
{
"all" => DebugLogLevel::All,
"parse" => DebugLogLevel::Parse,
"match" => DebugLogLevel::Match,
_ => DebugLogLevel::None,
}
})
}

fn debug_match_sample_rate() -> u64 {
*DEBUG_MATCH_SAMPLE_RATE.get_or_init(|| {
std::env::var("SENTRY_OPTIONS_FEATURE_DEBUG_LOG_SAMPLE_RATE")
.ok()
.and_then(|v| v.parse::<f64>().ok())
.map(|r| (r.clamp(0.0, 1.0) * 1000.0) as u64)
.unwrap_or(1000)
})
}

fn debug_log_parse(msg: &str) {
match debug_log_level() {
DebugLogLevel::Parse | DebugLogLevel::All => eprintln!("[sentry-options/parse] {msg}"),
_ => {}
}
}

fn debug_log_match(feature: &str, result: bool, context_id: u64) {
match debug_log_level() {
DebugLogLevel::Match | DebugLogLevel::All => {
if context_id % 1000 < debug_match_sample_rate() {
eprintln!(
"[sentry-options/match] feature='{feature}' result={result} context_id={context_id}"
);
}
}
_ => {}
}
}

/// A handle for checking feature flags within a specific namespace.
pub struct FeatureChecker {
namespace: String,
Expand Down Expand Up @@ -427,24 +371,29 @@ impl FeatureChecker {
let feature_val = match opts.get(&self.namespace, &key) {
Ok(v) => v,
Err(e) => {
debug_log_parse(&format!("Failed to get feature '{key}': {e}"));
tracing::debug!(key = %key, error = %e, "Failed to get feature");
return false;
}
};

let feature = match Feature::from_json(&feature_val) {
Some(f) => {
debug_log_parse(&format!("Parsed feature '{key}'"));
tracing::debug!(key = %key, "Parsed feature");
f
}
None => {
debug_log_parse(&format!("Failed to parse feature '{key}'"));
tracing::debug!(key = %key, "Failed to parse feature");
return false;
}
};

let result = feature.matches(context);
debug_log_match(feature_name, result, context.id());
tracing::debug!(
feature = feature_name,
result,
context_id = context.id(),
"Feature match result"
);
result
}
}
Expand Down
2 changes: 2 additions & 0 deletions sentry-options-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ sentry-options-validation.workspace = true
thiserror.workspace = true
walkdir = "2.5.0"
tempfile.workspace = true
tracing.workspace = true
tracing-subscriber = { version = "0.3.19", features = ["fmt"] }

[dev-dependencies]
tempfile.workspace = true
29 changes: 18 additions & 11 deletions sentry-options-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ fn cli_validate_schema(schemas: String, quiet: bool) -> Result<()> {
SchemaRegistry::from_directory(Path::new(&schemas))?;

if !quiet {
println!("Schema validation successful");
tracing::info!("Schema validation successful");
}
Ok(())
}
Expand All @@ -229,7 +229,7 @@ fn cli_validate_values(schemas: String, root: String, quiet: bool) -> Result<()>
ensure_no_duplicate_keys(&grouped)?;

if !quiet {
println!("Values validation successful");
tracing::info!("Values validation successful");
}
Ok(())
}
Expand All @@ -252,7 +252,7 @@ fn cli_write(args: WriteArgs, quiet: bool) -> Result<()> {
write_json(PathBuf::from(&out_path), json_outputs)?;

if !quiet {
println!("Successfully wrote {} output files", num_files);
tracing::info!(num_files, "Successfully wrote output files");
}
}
OutputFormat::Configmap => {
Expand All @@ -276,11 +276,12 @@ fn cli_write(args: WriteArgs, quiet: bool) -> Result<()> {

if !quiet {
match out_path {
Some(path) => eprintln!("Successfully wrote ConfigMap to {}", path.display()),
None => eprintln!(
"Successfully generated ConfigMap: sentry-options-{}",
namespace
),
Some(path) => {
tracing::info!(path = %path.display(), "Successfully wrote ConfigMap")
}
None => {
tracing::info!(name = %format!("sentry-options-{namespace}"), "Successfully generated ConfigMap")
}
}
}
}
Expand All @@ -292,7 +293,7 @@ fn cli_fetch_schemas(config: String, out: String, quiet: bool) -> Result<()> {
let config = repo_schema_config::RepoSchemaConfigs::from_file(Path::new(&config))?;
schema_retriever::fetch_all_schemas(&config, Path::new(&out), quiet)?;
if !quiet {
println!("Successfully fetched schemas to {}", out);
tracing::info!(path = %out, "Successfully fetched schemas");
}
Ok(())
}
Expand Down Expand Up @@ -329,7 +330,7 @@ fn cli_validate_schema_changes(
)?;

if !quiet {
eprintln!("Schema validation passed");
tracing::info!("Schema validation passed");
}

Ok(())
Expand All @@ -342,6 +343,12 @@ fn cli_check_option_usage(deletions: String, root: String) -> Result<()> {
}

fn main() {
tracing_subscriber::fmt()
.compact()
.without_time()
.with_max_level(tracing::Level::INFO)
.init();

let cli = Cli::parse();

let result = match cli.command {
Expand All @@ -359,7 +366,7 @@ fn main() {
};

if let Err(e) = result {
eprintln!("{}", e);
tracing::error!(error = %e);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error event missing message string unlike all others

Medium Severity

tracing::error!(error = %e) is the only tracing call across the entire diff that omits a message string. Every other call follows the pattern tracing::level!(fields, "Human-readable message"). Without a message, the compact formatter produces output like ERROR error=I/O error: ... instead of something like ERROR Command failed error=I/O error: .... For a CLI's top-level error handler, this makes the output less user-friendly compared to the original eprintln!("{}", e) which printed just the error text.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 06ffd5d. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the error contains the message, I think this is fine here, but it may still make sense to add a message describing the context.

std::process::exit(1);
}
}
Expand Down
13 changes: 6 additions & 7 deletions sentry-options-cli/src/schema_evolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,19 +304,18 @@ pub fn detect_changes(
}

if !quiet {
eprintln!("Schema Changes:");
if changelog.is_empty() {
eprintln!("\tNo changes");
}
for change in &changelog {
eprintln!("\t{}", change);
tracing::info!("No schema changes");
} else {
for change in &changelog {
tracing::info!(%change, "Schema change detected");
}
}
}

if !errors.is_empty() {
println!("Errors:");
for error in &errors {
println!("\t{}", error);
tracing::error!(%error, "Schema validation error");
}
return Err(ValidationError::ValidationErrors(errors));
}
Expand Down
4 changes: 2 additions & 2 deletions sentry-options-cli/src/schema_retriever.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub fn fetch_all_schemas(config: &RepoSchemaConfigs, out_dir: &Path, quiet: bool
repo_names.sort();

if !quiet {
println!("Fetching schemas...");
tracing::info!("Fetching schemas");
}

// Fetch all repos in parallel, collecting results
Expand Down Expand Up @@ -58,7 +58,7 @@ pub fn fetch_all_schemas(config: &RepoSchemaConfigs, out_dir: &Path, quiet: bool
match result {
Ok(()) => {
if !quiet {
println!(" Fetched {}", repo_name);
tracing::info!(repo = repo_name, "Fetched schema");
}
}
Err(e) => errors.push(e),
Expand Down
Loading
Loading