feat(tracing): Replace println/eprintln with tracing#116
feat(tracing): Replace println/eprintln with tracing#116
Conversation
Applications embedding sentry-options as a dependency may have their own logging sinks (e.g. a Sentry SDK or structured log aggregator). Hard-coded eprintln/println bypasses those sinks entirely. By emitting events through the tracing crate instead, the host application decides how and where they are captured. The CLI gets a compact, time-free tracing-subscriber at INFO level so its own output is unaffected. Co-Authored-By: Claude <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 06ffd5d. Configure here.
|
|
||
| if let Err(e) = result { | ||
| eprintln!("{}", e); | ||
| tracing::error!(error = %e); |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 06ffd5d. Configure here.
There was a problem hiding this comment.
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.
| namespaces = ?namespaces, | ||
| elapsed_ms = reload_duration.as_millis(), | ||
| "Reloaded values", | ||
| ); |
There was a problem hiding this comment.
New INFO log added in library reload path
Low Severity
The tracing::info!("Reloaded values", ...) call is not replacing an existing println!/eprintln! — it's entirely new logging added to the success path of reload_values. The old code only had an eprintln for the error case. This fires every time the background file-watcher detects a change and successfully reloads values. For a library crate, this adds a recurring INFO-level event that host applications may not expect, going beyond the PR's stated scope of replacing existing print statements.
Reviewed by Cursor Bugbot for commit 06ffd5d. Configure here.
There was a problem hiding this comment.
I did this on purpose, as I found it useful to get an INFO-level log when this happens. Feel free to revert.
1 participant
Applications embedding
sentry-optionsas a dependency may have their own logging sinks — a Sentry SDK, a structured log aggregator, or similar. Hard-codedeprintln/printlnbypasses those sinks entirely.By emitting events through the
tracingcrate, the host application controls how and where they are captured. The CLI gets a compact, time-freetracing-subscriberat INFO level so its own output is unaffected.