Clean up telemetry: hoist shared computations, tighten visibility

- Compute sanitize_command, repository_identifier, and CI check once
  before the if/else branches to avoid duplicate git I/O
- Make should_track_for_level private (only used by _track_inner)
- Check tracks.is_empty() before credentials in upload_blocking for
  consistency with emit()

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-19 14:55:25 -04:00
parent 5844d0778d
commit eaf9c68050
No known key found for this signature in database
3 changed files with 15 additions and 13 deletions

View file

@ -386,23 +386,26 @@ async fn main() {
let duration_ms = start.elapsed().as_millis() as u64;
let is_error = result.is_err();
let command = fabro_telemetry::sanitize::sanitize_command(&raw_args, &command_name);
let repository = fabro_telemetry::git::repository_identifier();
let ci = std::env::var("CI").is_ok();
if is_error {
fabro_telemetry::track!("CLI Errored", {
"subcommand": command_name,
"command": fabro_telemetry::sanitize::sanitize_command(&raw_args, &command_name),
"command": command,
"durationMs": duration_ms,
"repository": fabro_telemetry::git::repository_identifier(),
"ci": std::env::var("CI").is_ok(),
"repository": repository,
"ci": ci,
"success": false,
"exitCode": 1,
}, error);
} else {
fabro_telemetry::track!("CLI Executed", {
"subcommand": command_name,
"command": fabro_telemetry::sanitize::sanitize_command(&raw_args, &command_name),
"command": command,
"durationMs": duration_ms,
"repository": fabro_telemetry::git::repository_identifier(),
"ci": std::env::var("CI").is_ok(),
"repository": repository,
"ci": ci,
"success": true,
"exitCode": 0,
});

View file

@ -124,8 +124,7 @@ pub fn shutdown() {
}
}
/// Check whether an event should be tracked given a telemetry level.
pub fn should_track_for_level(level: TelemetryLevel, is_error: bool) -> bool {
fn should_track_for_level(level: TelemetryLevel, is_error: bool) -> bool {
match level {
TelemetryLevel::Off => false,
TelemetryLevel::Errors => is_error,

View file

@ -79,13 +79,13 @@ fn build_segment_batch(content: &str) -> Option<serde_json::Value> {
/// on the background telemetry thread.
/// No-ops if `SEGMENT_WRITE_KEY` was not set at compile time or `tracks` is empty.
pub fn upload_blocking(tracks: &[Track]) -> anyhow::Result<()> {
let write_key = SEGMENT_WRITE_KEY
.ok_or_else(|| anyhow::anyhow!("SEGMENT_WRITE_KEY not set at compile time"))?;
if tracks.is_empty() {
return Ok(());
}
let write_key = SEGMENT_WRITE_KEY
.ok_or_else(|| anyhow::anyhow!("SEGMENT_WRITE_KEY not set at compile time"))?;
let lines: Vec<String> = tracks
.iter()
.filter_map(|t| serde_json::to_string(t).ok())
@ -268,9 +268,9 @@ mod tests {
#[test]
fn upload_blocking_noops_with_empty_tracks() {
// With no write key, but empty tracks should still error on write key check
// Empty tracks returns Ok without checking credentials
let result = upload_blocking(&[]);
assert!(result.is_err());
assert!(result.is_ok());
}
// -- Step 4: upload() tests --