From e272b5bade3de46c89bb6aa701f8e2b6ddbfae37 Mon Sep 17 00:00:00 2001 From: alexsong-oai Date: Wed, 25 Mar 2026 16:37:17 -0700 Subject: [PATCH] update --- codex-rs/core/src/analytics_client.rs | 123 +++++++------------- codex-rs/core/src/analytics_client_tests.rs | 60 +++++----- 2 files changed, 69 insertions(+), 114 deletions(-) diff --git a/codex-rs/core/src/analytics_client.rs b/codex-rs/core/src/analytics_client.rs index b38fab2402..1917523be7 100644 --- a/codex-rs/core/src/analytics_client.rs +++ b/codex-rs/core/src/analytics_client.rs @@ -51,15 +51,6 @@ pub(crate) enum InvocationType { Implicit, } -impl InvocationType { - fn tag_value(self) -> &'static str { - match self { - Self::Explicit => "explicit", - Self::Implicit => "implicit", - } - } -} - pub(crate) struct AppInvocation { pub(crate) connector_id: Option, pub(crate) app_name: Option, @@ -125,9 +116,11 @@ impl AnalyticsEventsQueue { TrySendError::Full(job) => ("queue_full", job), TrySendError::Closed(job) => ("queue_closed", job), }; - let job_type = job.job_type(); - emit_analytics_events_failure_metric(reason, job_type, job.invoke_type_tag(), &[]); - tracing::warn!("dropping analytics events job: reason={reason} job_type={job_type}"); + emit_analytics_events_failure_counters(reason, job.job_type(), job.event_count(), &[]); + tracing::warn!( + "dropping analytics events job: reason={reason} job_type={}", + job.job_type() + ); } } @@ -274,39 +267,49 @@ impl TrackEventsJob { } } - fn invoke_type_tag(&self) -> Option<&'static str> { + fn event_count(&self) -> usize { match self { - Self::SkillInvocations(job) => job - .invocations - .first() - .map(|invocation| invocation.invocation_type.tag_value()), - Self::AppMentioned(job) => job - .mentions - .first() - .and_then(|mention| mention.invocation_type.map(InvocationType::tag_value)), - Self::AppUsed(job) => job.app.invocation_type.map(InvocationType::tag_value), - Self::PluginUsed(_) + Self::SkillInvocations(job) => job.invocations.len(), + Self::AppMentioned(job) => job.mentions.len(), + Self::AppUsed(_) + | Self::PluginUsed(_) | Self::PluginInstalled(_) | Self::PluginUninstalled(_) | Self::PluginEnabled(_) - | Self::PluginDisabled(_) => None, + | Self::PluginDisabled(_) => 1, } } } -fn emit_analytics_events_failure_metric<'a>( +fn job_type_for_events(events: &[TrackEventRequest]) -> &'static str { + match events.first() { + Some(TrackEventRequest::SkillInvocation(_)) => "skill_invocations", + Some(TrackEventRequest::AppMentioned(_)) => "app_mentioned", + Some(TrackEventRequest::AppUsed(_)) => "app_used", + Some(TrackEventRequest::PluginUsed(_)) => "plugin_used", + Some(TrackEventRequest::PluginInstalled(_)) => "plugin_installed", + Some(TrackEventRequest::PluginUninstalled(_)) => "plugin_uninstalled", + Some(TrackEventRequest::PluginEnabled(_)) => "plugin_enabled", + Some(TrackEventRequest::PluginDisabled(_)) => "plugin_disabled", + None => unreachable!("events should be non-empty"), + } +} + +fn emit_analytics_events_failure_counters<'a>( reason: &'static str, job_type: &'static str, - invoke_type: Option<&'static str>, + event_count: usize, extra_tags: &[(&'a str, &'a str)], ) { if let Some(metrics) = codex_otel::metrics::global() { let mut tags = vec![("reason", reason), ("job_type", job_type)]; - if let Some(invoke_type) = invoke_type { - tags.push(("invoke_type", invoke_type)); - } tags.extend(extra_tags.iter().copied()); let _ = metrics.counter("codex.analytics_events.emit.failure", /*inc*/ 1, &tags); + let _ = metrics.counter( + "codex.analytics_events.emit.failure_events", + i64::try_from(event_count).unwrap_or(i64::MAX), + &tags, + ); } } @@ -743,52 +746,25 @@ async fn send_track_events( if events.is_empty() { return; } - let (job_type, invoke_type) = match events.first() { - Some(TrackEventRequest::SkillInvocation(event)) => ( - "skill_invocations", - event - .event_params - .invoke_type - .map(InvocationType::tag_value), - ), - Some(TrackEventRequest::AppMentioned(event)) => ( - "app_mentioned", - event - .event_params - .invoke_type - .map(InvocationType::tag_value), - ), - Some(TrackEventRequest::AppUsed(event)) => ( - "app_used", - event - .event_params - .invoke_type - .map(InvocationType::tag_value), - ), - Some(TrackEventRequest::PluginUsed(_)) => ("plugin_used", None), - Some(TrackEventRequest::PluginInstalled(_)) => ("plugin_installed", None), - Some(TrackEventRequest::PluginUninstalled(_)) => ("plugin_uninstalled", None), - Some(TrackEventRequest::PluginEnabled(_)) => ("plugin_enabled", None), - Some(TrackEventRequest::PluginDisabled(_)) => ("plugin_disabled", None), - None => unreachable!("events should be non-empty"), - }; + let event_count = events.len(); + let job_type = job_type_for_events(&events); let Some(auth) = auth_manager.auth().await else { - emit_analytics_events_failure_metric("auth_missing", job_type, invoke_type, &[]); + emit_analytics_events_failure_counters("auth_missing", job_type, event_count, &[]); return; }; if !auth.is_chatgpt_auth() { - emit_analytics_events_failure_metric("non_chatgpt_auth", job_type, invoke_type, &[]); + emit_analytics_events_failure_counters("non_chatgpt_auth", job_type, event_count, &[]); return; } let access_token = match auth.get_token() { Ok(token) => token, Err(_) => { - emit_analytics_events_failure_metric("token_error", job_type, invoke_type, &[]); + emit_analytics_events_failure_counters("token_error", job_type, event_count, &[]); return; } }; let Some(account_id) = auth.get_account_id() else { - emit_analytics_events_failure_metric("account_id_missing", job_type, invoke_type, &[]); + emit_analytics_events_failure_counters("account_id_missing", job_type, event_count, &[]); return; }; @@ -810,17 +786,17 @@ async fn send_track_events( Ok(response) if response.status().is_success() => {} Ok(response) => { let status = response.status(); - emit_analytics_events_failure_metric( + emit_analytics_events_failure_counters( "http_status", job_type, - invoke_type, + event_count, &[("status_code", status.as_str())], ); let body = response.text().await.unwrap_or_default(); tracing::warn!("events failed with status {status}: {body}"); } Err(err) => { - emit_analytics_events_failure_metric("request_error", job_type, invoke_type, &[]); + emit_analytics_events_failure_counters("request_error", job_type, event_count, &[]); tracing::warn!("failed to send events request: {err}"); } } @@ -832,13 +808,6 @@ pub(crate) fn skill_id_for_local_skill( skill_path: &Path, skill_name: &str, ) -> String { - tracing::info!( - ?repo_url, - ?repo_root, - skill_path = %skill_path.display(), - skill_name, - "building analytics skill id for local skill" - ); let path = normalize_path_for_skill_id(repo_url, repo_root, skill_path); let prefix = if let Some(url) = repo_url { format!("repo_{url}") @@ -848,15 +817,7 @@ pub(crate) fn skill_id_for_local_skill( let raw_id = format!("{prefix}_{path}_{skill_name}"); let mut hasher = Sha1::new(); hasher.update(raw_id.as_bytes()); - let skill_id = format!("{:x}", hasher.finalize()); - tracing::info!( - normalized_path = path, - prefix, - raw_id, - skill_id, - "built analytics skill id for local skill" - ); - skill_id + format!("{:x}", hasher.finalize()) } /// Returns a normalized path for skill ID construction. diff --git a/codex-rs/core/src/analytics_client_tests.rs b/codex-rs/core/src/analytics_client_tests.rs index 6037f66563..824dcaaeb0 100644 --- a/codex-rs/core/src/analytics_client_tests.rs +++ b/codex-rs/core/src/analytics_client_tests.rs @@ -361,7 +361,7 @@ fn track_events_job_type_uses_expected_tag_values() { } #[test] -fn track_events_job_invoke_type_tag_uses_first_available_value() { +fn track_events_job_event_count_matches_underlying_payloads() { let codex_home = TempDir::new().expect("tempdir should create"); let config = Arc::new(load_test_config(codex_home.path())); let tracking = TrackEventsContext { @@ -369,32 +369,31 @@ fn track_events_job_invoke_type_tag_uses_first_available_value() { thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), }; - let explicit_skill_job = TrackEventsJob::SkillInvocations(TrackSkillInvocationsJob { + let skill_job = TrackEventsJob::SkillInvocations(TrackSkillInvocationsJob { config: Arc::clone(&config), tracking: tracking.clone(), - invocations: vec![super::SkillInvocation { - skill_name: "doc".to_string(), - skill_scope: codex_protocol::protocol::SkillScope::Repo, - skill_path: codex_home.path().join("SKILL.md"), - invocation_type: InvocationType::Explicit, - }], + invocations: vec![ + super::SkillInvocation { + skill_name: "doc".to_string(), + skill_scope: codex_protocol::protocol::SkillScope::Repo, + skill_path: codex_home.path().join("SKILL.md"), + invocation_type: InvocationType::Explicit, + }, + super::SkillInvocation { + skill_name: "doc-2".to_string(), + skill_scope: codex_protocol::protocol::SkillScope::Repo, + skill_path: codex_home.path().join("SKILL-2.md"), + invocation_type: InvocationType::Implicit, + }, + ], }); - let implicit_app_job = TrackEventsJob::AppUsed(TrackAppUsedJob { - config: Arc::clone(&config), - tracking: tracking.clone(), - app: AppInvocation { - connector_id: Some("drive".to_string()), - app_name: Some("Google Drive".to_string()), - invocation_type: Some(InvocationType::Implicit), - }, - }); - let first_value_app_mentioned_job = TrackEventsJob::AppMentioned(TrackAppMentionedJob { + let app_mentioned_job = TrackEventsJob::AppMentioned(TrackAppMentionedJob { config: Arc::clone(&config), tracking: tracking.clone(), mentions: vec![ AppInvocation { connector_id: Some("drive".to_string()), - app_name: Some("Google Drive".to_string()), + app_name: Some("Drive".to_string()), invocation_type: Some(InvocationType::Explicit), }, AppInvocation { @@ -402,27 +401,22 @@ fn track_events_job_invoke_type_tag_uses_first_available_value() { app_name: Some("Calendar".to_string()), invocation_type: Some(InvocationType::Implicit), }, + AppInvocation { + connector_id: Some("gmail".to_string()), + app_name: Some("Gmail".to_string()), + invocation_type: None, + }, ], }); - let empty_skill_job = TrackEventsJob::SkillInvocations(TrackSkillInvocationsJob { - config: Arc::clone(&config), - tracking: tracking.clone(), - invocations: Vec::new(), - }); - let no_invoke_type_job = TrackEventsJob::PluginUsed(TrackPluginUsedJob { + let plugin_job = TrackEventsJob::PluginUsed(TrackPluginUsedJob { config, tracking, plugin: sample_plugin_metadata(), }); - assert_eq!(explicit_skill_job.invoke_type_tag(), Some("explicit")); - assert_eq!(implicit_app_job.invoke_type_tag(), Some("implicit")); - assert_eq!( - first_value_app_mentioned_job.invoke_type_tag(), - Some("explicit") - ); - assert_eq!(empty_skill_job.invoke_type_tag(), None); - assert_eq!(no_invoke_type_job.invoke_type_tag(), None); + assert_eq!(skill_job.event_count(), 2); + assert_eq!(app_mentioned_job.event_count(), 3); + assert_eq!(plugin_job.event_count(), 1); } fn sample_plugin_metadata() -> PluginTelemetryMetadata {