From 0d2ff40a58dde63e5aa8be85b5a5f19f384c354c Mon Sep 17 00:00:00 2001 From: Colin Young Date: Tue, 17 Mar 2026 14:26:27 -0700 Subject: [PATCH] Add auth env observability (#14905) CXC-410 Emit Env Var Status with `/feedback` report Add more observability on top of #14611 [Unset](https://openai.sentry.io/issues/7340419168/?project=4510195390611458&query=019cfa8d-c1ba-7002-96fa-e35fc340551d&referrer=issue-stream) [Set](https://openai.sentry.io/issues/7340426331/?project=4510195390611458&query=019cfa91-aba1-7823-ab7e-762edfbc0ed4&referrer=issue-stream) image ###### Summary - Adds auth-env telemetry that records whether key auth-related env overrides were present on session start and request paths. - Threads those auth-env fields through `/responses`, websocket, and `/models` telemetry and feedback metadata. - Buckets custom provider `env_key` configuration to a safe `"configured"` value instead of emitting raw config text. - Keeps the slice observability-only: no raw token values or raw URLs are emitted. ###### Rationale (from spec findings) - 401 and auth-path debugging needs a way to distinguish env-driven auth paths from sessions with no auth env override. - Startup and model-refresh failures need the same auth-env diagnostics as normal request failures. - Feedback and Sentry tags need the same auth-env signal as OTel events so reports can be triaged consistently. - Custom provider config is user-controlled text, so the telemetry contract must stay presence-only / bucketed. ###### Scope - Adds a small `AuthEnvTelemetry` bundle for env presence collection and threads it through the main request/session telemetry paths. - Does not add endpoint/base-url/provider-header/geo routing attribution or broader telemetry API redesign. ###### Trade-offs - `provider_env_key_name` is bucketed to `"configured"` instead of preserving the literal configured env var name. - `/models` is included because startup/model-refresh auth failures need the same diagnostics, but broader parity work remains out of scope. - This slice keeps the existing telemetry APIs and layers auth-env fields onto them rather than redesigning the metadata model. ###### Client follow-up - Add the separate endpoint/base-url attribution slice if routing-source diagnosis is still needed. - Add provider-header or residency attribution only if auth-env presence proves insufficient in real reports. - Revisit whether any additional auth-related env inputs need safe bucketing after more 401 triage data. ###### Testing - `cargo test -p codex-core emit_feedback_request_tags -- --nocapture` - `cargo test -p codex-core collect_auth_env_telemetry_buckets_provider_env_key_name -- --nocapture` - `cargo test -p codex-core models_request_telemetry_emits_auth_env_feedback_tags_on_failure -- --nocapture` - `cargo test -p codex-otel otel_export_routing_policy_routes_api_request_auth_observability -- --nocapture` - `cargo test -p codex-otel otel_export_routing_policy_routes_websocket_connect_auth_observability -- --nocapture` - `cargo test -p codex-otel otel_export_routing_policy_routes_websocket_request_transport_observability -- --nocapture` - `cargo test -p codex-core --no-run --message-format short` - `cargo test -p codex-otel --no-run --message-format short` --------- Co-authored-by: Codex --- codex-rs/core/src/auth.rs | 4 + codex-rs/core/src/auth_env_telemetry.rs | 85 ++++++++ codex-rs/core/src/client.rs | 166 +++++++++------- codex-rs/core/src/codex.rs | 8 +- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/models_manager/manager.rs | 57 ++++-- .../core/src/models_manager/manager_tests.rs | 154 +++++++++++++++ codex-rs/core/src/util.rs | 128 ++++++++---- codex-rs/core/src/util_tests.rs | 186 ++++++++++++++---- codex-rs/otel/src/events/session_telemetry.rs | 41 ++++ codex-rs/otel/src/lib.rs | 1 + .../tests/suite/otel_export_routing_policy.rs | 100 +++++++++- 12 files changed, 770 insertions(+), 161 deletions(-) create mode 100644 codex-rs/core/src/auth_env_telemetry.rs diff --git a/codex-rs/core/src/auth.rs b/codex-rs/core/src/auth.rs index bafc3179d..90f0dcfda 100644 --- a/codex-rs/core/src/auth.rs +++ b/codex-rs/core/src/auth.rs @@ -1250,6 +1250,10 @@ impl AuthManager { .is_some_and(CodexAuth::is_external_chatgpt_tokens) } + pub fn codex_api_key_env_enabled(&self) -> bool { + self.enable_codex_api_key_env + } + /// Convenience constructor returning an `Arc` wrapper. pub fn shared( codex_home: PathBuf, diff --git a/codex-rs/core/src/auth_env_telemetry.rs b/codex-rs/core/src/auth_env_telemetry.rs new file mode 100644 index 000000000..be281e05a --- /dev/null +++ b/codex-rs/core/src/auth_env_telemetry.rs @@ -0,0 +1,85 @@ +use codex_otel::AuthEnvTelemetryMetadata; + +use crate::auth::CODEX_API_KEY_ENV_VAR; +use crate::auth::OPENAI_API_KEY_ENV_VAR; +use crate::auth::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; +use crate::model_provider_info::ModelProviderInfo; + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub(crate) struct AuthEnvTelemetry { + pub(crate) openai_api_key_env_present: bool, + pub(crate) codex_api_key_env_present: bool, + pub(crate) codex_api_key_env_enabled: bool, + pub(crate) provider_env_key_name: Option, + pub(crate) provider_env_key_present: Option, + pub(crate) refresh_token_url_override_present: bool, +} + +impl AuthEnvTelemetry { + pub(crate) fn to_otel_metadata(&self) -> AuthEnvTelemetryMetadata { + AuthEnvTelemetryMetadata { + openai_api_key_env_present: self.openai_api_key_env_present, + codex_api_key_env_present: self.codex_api_key_env_present, + codex_api_key_env_enabled: self.codex_api_key_env_enabled, + provider_env_key_name: self.provider_env_key_name.clone(), + provider_env_key_present: self.provider_env_key_present, + refresh_token_url_override_present: self.refresh_token_url_override_present, + } + } +} + +pub(crate) fn collect_auth_env_telemetry( + provider: &ModelProviderInfo, + codex_api_key_env_enabled: bool, +) -> AuthEnvTelemetry { + AuthEnvTelemetry { + openai_api_key_env_present: env_var_present(OPENAI_API_KEY_ENV_VAR), + codex_api_key_env_present: env_var_present(CODEX_API_KEY_ENV_VAR), + codex_api_key_env_enabled, + // Custom provider `env_key` is arbitrary config text, so emit only a safe bucket. + provider_env_key_name: provider.env_key.as_ref().map(|_| "configured".to_string()), + provider_env_key_present: provider.env_key.as_deref().map(env_var_present), + refresh_token_url_override_present: env_var_present(REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR), + } +} + +fn env_var_present(name: &str) -> bool { + match std::env::var(name) { + Ok(value) => !value.trim().is_empty(), + Err(std::env::VarError::NotUnicode(_)) => true, + Err(std::env::VarError::NotPresent) => false, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn collect_auth_env_telemetry_buckets_provider_env_key_name() { + let provider = ModelProviderInfo { + name: "Custom".to_string(), + base_url: None, + env_key: Some("sk-should-not-leak".to_string()), + env_key_instructions: None, + experimental_bearer_token: None, + wire_api: crate::model_provider_info::WireApi::Responses, + query_params: None, + http_headers: None, + env_http_headers: None, + request_max_retries: None, + stream_max_retries: None, + stream_idle_timeout_ms: None, + requires_openai_auth: false, + supports_websockets: false, + }; + + let telemetry = collect_auth_env_telemetry(&provider, false); + + assert_eq!( + telemetry.provider_env_key_name, + Some("configured".to_string()) + ); + } +} diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 79fec5dfa..48e8d90ec 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -34,6 +34,8 @@ use crate::api_bridge::CoreAuthProvider; use crate::api_bridge::auth_provider_from_auth; use crate::api_bridge::map_api_error; use crate::auth::UnauthorizedRecovery; +use crate::auth_env_telemetry::AuthEnvTelemetry; +use crate::auth_env_telemetry::collect_auth_env_telemetry; use codex_api::CompactClient as ApiCompactClient; use codex_api::CompactionInput as ApiCompactionInput; use codex_api::MemoriesClient as ApiMemoriesClient; @@ -106,7 +108,7 @@ use crate::response_debug_context::telemetry_transport_error_message; use crate::tools::spec::create_tools_json_for_responses_api; use crate::util::FeedbackRequestTags; use crate::util::emit_feedback_auth_recovery_tags; -use crate::util::emit_feedback_request_tags; +use crate::util::emit_feedback_request_tags_with_auth_env; pub const OPENAI_BETA_HEADER: &str = "OpenAI-Beta"; pub const X_CODEX_TURN_STATE_HEADER: &str = "x-codex-turn-state"; @@ -138,6 +140,7 @@ struct ModelClientState { auth_manager: Option>, conversation_id: ThreadId, provider: ModelProviderInfo, + auth_env_telemetry: AuthEnvTelemetry, session_source: SessionSource, model_verbosity: Option, responses_websockets_enabled_by_feature: bool, @@ -267,11 +270,16 @@ impl ModelClient { include_timing_metrics: bool, beta_features_header: Option, ) -> Self { + let codex_api_key_env_enabled = auth_manager + .as_ref() + .is_some_and(|manager| manager.codex_api_key_env_enabled()); + let auth_env_telemetry = collect_auth_env_telemetry(&provider, codex_api_key_env_enabled); Self { state: Arc::new(ModelClientState { auth_manager, conversation_id, provider, + auth_env_telemetry, session_source, model_verbosity, responses_websockets_enabled_by_feature, @@ -362,6 +370,7 @@ impl ModelClient { PendingUnauthorizedRetry::default(), ), RequestRouteTelemetry::for_endpoint(RESPONSES_COMPACT_ENDPOINT), + self.state.auth_env_telemetry.clone(), ); let client = ApiCompactClient::new(transport, client_setup.api_provider, client_setup.api_auth) @@ -430,6 +439,7 @@ impl ModelClient { PendingUnauthorizedRetry::default(), ), RequestRouteTelemetry::for_endpoint(MEMORIES_SUMMARIZE_ENDPOINT), + self.state.auth_env_telemetry.clone(), ); let client = ApiMemoriesClient::new(transport, client_setup.api_provider, client_setup.api_auth) @@ -474,11 +484,13 @@ impl ModelClient { session_telemetry: &SessionTelemetry, auth_context: AuthRequestTelemetryContext, request_route_telemetry: RequestRouteTelemetry, + auth_env_telemetry: AuthEnvTelemetry, ) -> Arc { let telemetry = Arc::new(ApiTelemetry::new( session_telemetry.clone(), auth_context, request_route_telemetry, + auth_env_telemetry, )); let request_telemetry: Arc = telemetry; request_telemetry @@ -561,6 +573,7 @@ impl ModelClient { session_telemetry, auth_context, request_route_telemetry, + self.state.auth_env_telemetry.clone(), ); let websocket_connect_timeout = self.state.provider.websocket_connect_timeout(); let start = Instant::now(); @@ -601,27 +614,30 @@ impl ModelClient { response_debug.auth_error.as_deref(), response_debug.auth_error_code.as_deref(), ); - emit_feedback_request_tags(&FeedbackRequestTags { - endpoint: request_route_telemetry.endpoint, - auth_header_attached: auth_context.auth_header_attached, - auth_header_name: auth_context.auth_header_name, - auth_mode: auth_context.auth_mode, - auth_retry_after_unauthorized: Some(auth_context.retry_after_unauthorized), - auth_recovery_mode: auth_context.recovery_mode, - auth_recovery_phase: auth_context.recovery_phase, - auth_connection_reused: Some(false), - auth_request_id: response_debug.request_id.as_deref(), - auth_cf_ray: response_debug.cf_ray.as_deref(), - auth_error: response_debug.auth_error.as_deref(), - auth_error_code: response_debug.auth_error_code.as_deref(), - auth_recovery_followup_success: auth_context - .retry_after_unauthorized - .then_some(result.is_ok()), - auth_recovery_followup_status: auth_context - .retry_after_unauthorized - .then_some(status) - .flatten(), - }); + emit_feedback_request_tags_with_auth_env( + &FeedbackRequestTags { + endpoint: request_route_telemetry.endpoint, + auth_header_attached: auth_context.auth_header_attached, + auth_header_name: auth_context.auth_header_name, + auth_mode: auth_context.auth_mode, + auth_retry_after_unauthorized: Some(auth_context.retry_after_unauthorized), + auth_recovery_mode: auth_context.recovery_mode, + auth_recovery_phase: auth_context.recovery_phase, + auth_connection_reused: Some(false), + auth_request_id: response_debug.request_id.as_deref(), + auth_cf_ray: response_debug.cf_ray.as_deref(), + auth_error: response_debug.auth_error.as_deref(), + auth_error_code: response_debug.auth_error_code.as_deref(), + auth_recovery_followup_success: auth_context + .retry_after_unauthorized + .then_some(result.is_ok()), + auth_recovery_followup_status: auth_context + .retry_after_unauthorized + .then_some(status) + .flatten(), + }, + &self.state.auth_env_telemetry, + ); result } @@ -1030,6 +1046,7 @@ impl ModelClientSession { session_telemetry, request_auth_context, RequestRouteTelemetry::for_endpoint(RESPONSES_ENDPOINT), + self.client.state.auth_env_telemetry.clone(), ); let compression = self.responses_request_compression(client_setup.auth.as_ref()); let options = self.build_responses_options(turn_metadata_header, compression); @@ -1190,11 +1207,13 @@ impl ModelClientSession { session_telemetry: &SessionTelemetry, auth_context: AuthRequestTelemetryContext, request_route_telemetry: RequestRouteTelemetry, + auth_env_telemetry: AuthEnvTelemetry, ) -> (Arc, Arc) { let telemetry = Arc::new(ApiTelemetry::new( session_telemetry.clone(), auth_context, request_route_telemetry, + auth_env_telemetry, )); let request_telemetry: Arc = telemetry.clone(); let sse_telemetry: Arc = telemetry; @@ -1206,11 +1225,13 @@ impl ModelClientSession { session_telemetry: &SessionTelemetry, auth_context: AuthRequestTelemetryContext, request_route_telemetry: RequestRouteTelemetry, + auth_env_telemetry: AuthEnvTelemetry, ) -> Arc { let telemetry = Arc::new(ApiTelemetry::new( session_telemetry.clone(), auth_context, request_route_telemetry, + auth_env_telemetry, )); let websocket_telemetry: Arc = telemetry; websocket_telemetry @@ -1663,6 +1684,7 @@ struct ApiTelemetry { session_telemetry: SessionTelemetry, auth_context: AuthRequestTelemetryContext, request_route_telemetry: RequestRouteTelemetry, + auth_env_telemetry: AuthEnvTelemetry, } impl ApiTelemetry { @@ -1670,11 +1692,13 @@ impl ApiTelemetry { session_telemetry: SessionTelemetry, auth_context: AuthRequestTelemetryContext, request_route_telemetry: RequestRouteTelemetry, + auth_env_telemetry: AuthEnvTelemetry, ) -> Self { Self { session_telemetry, auth_context, request_route_telemetry, + auth_env_telemetry, } } } @@ -1708,29 +1732,32 @@ impl RequestTelemetry for ApiTelemetry { debug.auth_error.as_deref(), debug.auth_error_code.as_deref(), ); - emit_feedback_request_tags(&FeedbackRequestTags { - endpoint: self.request_route_telemetry.endpoint, - auth_header_attached: self.auth_context.auth_header_attached, - auth_header_name: self.auth_context.auth_header_name, - auth_mode: self.auth_context.auth_mode, - auth_retry_after_unauthorized: Some(self.auth_context.retry_after_unauthorized), - auth_recovery_mode: self.auth_context.recovery_mode, - auth_recovery_phase: self.auth_context.recovery_phase, - auth_connection_reused: None, - auth_request_id: debug.request_id.as_deref(), - auth_cf_ray: debug.cf_ray.as_deref(), - auth_error: debug.auth_error.as_deref(), - auth_error_code: debug.auth_error_code.as_deref(), - auth_recovery_followup_success: self - .auth_context - .retry_after_unauthorized - .then_some(error.is_none()), - auth_recovery_followup_status: self - .auth_context - .retry_after_unauthorized - .then_some(status) - .flatten(), - }); + emit_feedback_request_tags_with_auth_env( + &FeedbackRequestTags { + endpoint: self.request_route_telemetry.endpoint, + auth_header_attached: self.auth_context.auth_header_attached, + auth_header_name: self.auth_context.auth_header_name, + auth_mode: self.auth_context.auth_mode, + auth_retry_after_unauthorized: Some(self.auth_context.retry_after_unauthorized), + auth_recovery_mode: self.auth_context.recovery_mode, + auth_recovery_phase: self.auth_context.recovery_phase, + auth_connection_reused: None, + auth_request_id: debug.request_id.as_deref(), + auth_cf_ray: debug.cf_ray.as_deref(), + auth_error: debug.auth_error.as_deref(), + auth_error_code: debug.auth_error_code.as_deref(), + auth_recovery_followup_success: self + .auth_context + .retry_after_unauthorized + .then_some(error.is_none()), + auth_recovery_followup_status: self + .auth_context + .retry_after_unauthorized + .then_some(status) + .flatten(), + }, + &self.auth_env_telemetry, + ); } } @@ -1759,29 +1786,32 @@ impl WebsocketTelemetry for ApiTelemetry { error_message.as_deref(), connection_reused, ); - emit_feedback_request_tags(&FeedbackRequestTags { - endpoint: self.request_route_telemetry.endpoint, - auth_header_attached: self.auth_context.auth_header_attached, - auth_header_name: self.auth_context.auth_header_name, - auth_mode: self.auth_context.auth_mode, - auth_retry_after_unauthorized: Some(self.auth_context.retry_after_unauthorized), - auth_recovery_mode: self.auth_context.recovery_mode, - auth_recovery_phase: self.auth_context.recovery_phase, - auth_connection_reused: Some(connection_reused), - auth_request_id: debug.request_id.as_deref(), - auth_cf_ray: debug.cf_ray.as_deref(), - auth_error: debug.auth_error.as_deref(), - auth_error_code: debug.auth_error_code.as_deref(), - auth_recovery_followup_success: self - .auth_context - .retry_after_unauthorized - .then_some(error.is_none()), - auth_recovery_followup_status: self - .auth_context - .retry_after_unauthorized - .then_some(status) - .flatten(), - }); + emit_feedback_request_tags_with_auth_env( + &FeedbackRequestTags { + endpoint: self.request_route_telemetry.endpoint, + auth_header_attached: self.auth_context.auth_header_attached, + auth_header_name: self.auth_context.auth_header_name, + auth_mode: self.auth_context.auth_mode, + auth_retry_after_unauthorized: Some(self.auth_context.retry_after_unauthorized), + auth_recovery_mode: self.auth_context.recovery_mode, + auth_recovery_phase: self.auth_context.recovery_phase, + auth_connection_reused: Some(connection_reused), + auth_request_id: debug.request_id.as_deref(), + auth_cf_ray: debug.cf_ray.as_deref(), + auth_error: debug.auth_error.as_deref(), + auth_error_code: debug.auth_error_code.as_deref(), + auth_recovery_followup_success: self + .auth_context + .retry_after_unauthorized + .then_some(error.is_none()), + auth_recovery_followup_status: self + .auth_context + .retry_after_unauthorized + .then_some(status) + .flatten(), + }, + &self.auth_env_telemetry, + ); } fn on_ws_event( diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 1b4a23fe1..9382401ba 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -17,6 +17,7 @@ use crate::analytics_client::AppInvocation; use crate::analytics_client::InvocationType; use crate::analytics_client::build_track_events_context; use crate::apps::render_apps_section; +use crate::auth_env_telemetry::collect_auth_env_telemetry; use crate::commit_attribution::commit_message_trailer_instruction; use crate::compact; use crate::compact::InitialContextInjection; @@ -1565,6 +1566,10 @@ impl Session { let originator = crate::default_client::originator().value; let terminal_type = terminal::user_agent(); let session_model = session_configuration.collaboration_mode.model().to_string(); + let auth_env_telemetry = collect_auth_env_telemetry( + &session_configuration.provider, + auth_manager.codex_api_key_env_enabled(), + ); let mut session_telemetry = SessionTelemetry::new( conversation_id, session_model.as_str(), @@ -1576,7 +1581,8 @@ impl Session { config.otel.log_user_prompt, terminal_type.clone(), session_configuration.session_source.clone(), - ); + ) + .with_auth_env(auth_env_telemetry.to_otel_metadata()); if let Some(service_name) = session_configuration.metrics_service_name.as_deref() { session_telemetry = session_telemetry.with_metrics_service_name(service_name); } diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e02a34654..51d9fcf8d 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -11,6 +11,7 @@ mod apply_patch; mod apps; mod arc_monitor; pub mod auth; +mod auth_env_telemetry; mod client; mod client_common; pub mod codex; diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index f974aa4c1..29a1a8576 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -4,6 +4,8 @@ use crate::api_bridge::map_api_error; use crate::auth::AuthManager; use crate::auth::AuthMode; use crate::auth::CodexAuth; +use crate::auth_env_telemetry::AuthEnvTelemetry; +use crate::auth_env_telemetry::collect_auth_env_telemetry; use crate::config::Config; use crate::default_client::build_reqwest_client; use crate::error::CodexErr; @@ -15,7 +17,7 @@ use crate::models_manager::model_info; use crate::response_debug_context::extract_response_debug_context; use crate::response_debug_context::telemetry_transport_error_message; use crate::util::FeedbackRequestTags; -use crate::util::emit_feedback_request_tags; +use crate::util::emit_feedback_request_tags_with_auth_env; use codex_api::ModelsClient; use codex_api::RequestTelemetry; use codex_api::ReqwestTransport; @@ -46,6 +48,7 @@ struct ModelsRequestTelemetry { auth_mode: Option, auth_header_attached: bool, auth_header_name: Option<&'static str>, + auth_env: AuthEnvTelemetry, } impl RequestTelemetry for ModelsRequestTelemetry { @@ -74,6 +77,12 @@ impl RequestTelemetry for ModelsRequestTelemetry { endpoint = MODELS_ENDPOINT, auth.header_attached = self.auth_header_attached, auth.header_name = self.auth_header_name, + auth.env_openai_api_key_present = self.auth_env.openai_api_key_env_present, + auth.env_codex_api_key_present = self.auth_env.codex_api_key_env_present, + auth.env_codex_api_key_enabled = self.auth_env.codex_api_key_env_enabled, + auth.env_provider_key_name = self.auth_env.provider_env_key_name.as_deref(), + auth.env_provider_key_present = self.auth_env.provider_env_key_present, + auth.env_refresh_token_url_override_present = self.auth_env.refresh_token_url_override_present, auth.request_id = response_debug.request_id.as_deref(), auth.cf_ray = response_debug.cf_ray.as_deref(), auth.error = response_debug.auth_error.as_deref(), @@ -92,28 +101,37 @@ impl RequestTelemetry for ModelsRequestTelemetry { endpoint = MODELS_ENDPOINT, auth.header_attached = self.auth_header_attached, auth.header_name = self.auth_header_name, + auth.env_openai_api_key_present = self.auth_env.openai_api_key_env_present, + auth.env_codex_api_key_present = self.auth_env.codex_api_key_env_present, + auth.env_codex_api_key_enabled = self.auth_env.codex_api_key_env_enabled, + auth.env_provider_key_name = self.auth_env.provider_env_key_name.as_deref(), + auth.env_provider_key_present = self.auth_env.provider_env_key_present, + auth.env_refresh_token_url_override_present = self.auth_env.refresh_token_url_override_present, auth.request_id = response_debug.request_id.as_deref(), auth.cf_ray = response_debug.cf_ray.as_deref(), auth.error = response_debug.auth_error.as_deref(), auth.error_code = response_debug.auth_error_code.as_deref(), auth.mode = self.auth_mode.as_deref(), ); - emit_feedback_request_tags(&FeedbackRequestTags { - endpoint: MODELS_ENDPOINT, - auth_header_attached: self.auth_header_attached, - auth_header_name: self.auth_header_name, - auth_mode: self.auth_mode.as_deref(), - auth_retry_after_unauthorized: None, - auth_recovery_mode: None, - auth_recovery_phase: None, - auth_connection_reused: None, - auth_request_id: response_debug.request_id.as_deref(), - auth_cf_ray: response_debug.cf_ray.as_deref(), - auth_error: response_debug.auth_error.as_deref(), - auth_error_code: response_debug.auth_error_code.as_deref(), - auth_recovery_followup_success: None, - auth_recovery_followup_status: None, - }); + emit_feedback_request_tags_with_auth_env( + &FeedbackRequestTags { + endpoint: MODELS_ENDPOINT, + auth_header_attached: self.auth_header_attached, + auth_header_name: self.auth_header_name, + auth_mode: self.auth_mode.as_deref(), + auth_retry_after_unauthorized: None, + auth_recovery_mode: None, + auth_recovery_phase: None, + auth_connection_reused: None, + auth_request_id: response_debug.request_id.as_deref(), + auth_cf_ray: response_debug.cf_ray.as_deref(), + auth_error: response_debug.auth_error.as_deref(), + auth_error_code: response_debug.auth_error_code.as_deref(), + auth_recovery_followup_success: None, + auth_recovery_followup_status: None, + }, + &self.auth_env, + ); } } @@ -417,11 +435,16 @@ impl ModelsManager { let auth_mode = auth.as_ref().map(CodexAuth::auth_mode); let api_provider = self.provider.to_api_provider(auth_mode)?; let api_auth = auth_provider_from_auth(auth.clone(), &self.provider)?; + let auth_env = collect_auth_env_telemetry( + &self.provider, + self.auth_manager.codex_api_key_env_enabled(), + ); let transport = ReqwestTransport::new(build_reqwest_client()); let request_telemetry: Arc = Arc::new(ModelsRequestTelemetry { auth_mode: auth_mode.map(|mode| TelemetryAuthMode::from(mode).to_string()), auth_header_attached: api_auth.auth_header_attached(), auth_header_name: api_auth.auth_header_name(), + auth_env, }); let client = ModelsClient::new(transport, api_provider, api_auth) .with_telemetry(Some(request_telemetry)); diff --git a/codex-rs/core/src/models_manager/manager_tests.rs b/codex-rs/core/src/models_manager/manager_tests.rs index da42f2c85..07cf4dc39 100644 --- a/codex-rs/core/src/models_manager/manager_tests.rs +++ b/codex-rs/core/src/models_manager/manager_tests.rs @@ -3,12 +3,27 @@ use crate::CodexAuth; use crate::auth::AuthCredentialsStoreMode; use crate::config::ConfigBuilder; use crate::model_provider_info::WireApi; +use base64::Engine as _; use chrono::Utc; +use codex_api::TransportError; use codex_protocol::openai_models::ModelsResponse; use core_test_support::responses::mount_models_once; +use http::HeaderMap; +use http::StatusCode; use pretty_assertions::assert_eq; use serde_json::json; +use std::collections::BTreeMap; +use std::sync::Arc; +use std::sync::Mutex; use tempfile::tempdir; +use tracing::Event; +use tracing::Subscriber; +use tracing::field::Visit; +use tracing_subscriber::Layer; +use tracing_subscriber::layer::Context; +use tracing_subscriber::layer::SubscriberExt; +use tracing_subscriber::registry::LookupSpan; +use tracing_subscriber::util::SubscriberInitExt; use wiremock::MockServer; fn remote_model(slug: &str, display: &str, priority: i32) -> ModelInfo { @@ -77,6 +92,47 @@ fn provider_for(base_url: String) -> ModelProviderInfo { } } +#[derive(Default)] +struct TagCollectorVisitor { + tags: BTreeMap, +} + +impl Visit for TagCollectorVisitor { + fn record_bool(&mut self, field: &tracing::field::Field, value: bool) { + self.tags + .insert(field.name().to_string(), value.to_string()); + } + + fn record_str(&mut self, field: &tracing::field::Field, value: &str) { + self.tags + .insert(field.name().to_string(), value.to_string()); + } + + fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) { + self.tags + .insert(field.name().to_string(), format!("{value:?}")); + } +} + +#[derive(Clone)] +struct TagCollectorLayer { + tags: Arc>>, +} + +impl Layer for TagCollectorLayer +where + S: Subscriber + for<'a> LookupSpan<'a>, +{ + fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) { + if event.metadata().target() != "feedback_tags" { + return; + } + let mut visitor = TagCollectorVisitor::default(); + event.record(&mut visitor); + self.tags.lock().unwrap().extend(visitor.tags); + } +} + #[tokio::test] async fn get_model_info_tracks_fallback_usage() { let codex_home = tempdir().expect("temp dir"); @@ -530,6 +586,104 @@ async fn refresh_available_models_skips_network_without_chatgpt_auth() { ); } +#[test] +fn models_request_telemetry_emits_auth_env_feedback_tags_on_failure() { + let tags = Arc::new(Mutex::new(BTreeMap::new())); + let _guard = tracing_subscriber::registry() + .with(TagCollectorLayer { tags: tags.clone() }) + .set_default(); + + let telemetry = ModelsRequestTelemetry { + auth_mode: Some(TelemetryAuthMode::Chatgpt.to_string()), + auth_header_attached: true, + auth_header_name: Some("authorization"), + auth_env: crate::auth_env_telemetry::AuthEnvTelemetry { + openai_api_key_env_present: false, + codex_api_key_env_present: false, + codex_api_key_env_enabled: false, + provider_env_key_name: Some("configured".to_string()), + provider_env_key_present: Some(false), + refresh_token_url_override_present: false, + }, + }; + let mut headers = HeaderMap::new(); + headers.insert("x-request-id", "req-models-401".parse().unwrap()); + headers.insert("cf-ray", "ray-models-401".parse().unwrap()); + headers.insert( + "x-openai-authorization-error", + "missing_authorization_header".parse().unwrap(), + ); + headers.insert( + "x-error-json", + base64::engine::general_purpose::STANDARD + .encode(r#"{"error":{"code":"token_expired"}}"#) + .parse() + .unwrap(), + ); + telemetry.on_request( + 1, + Some(StatusCode::UNAUTHORIZED), + Some(&TransportError::Http { + status: StatusCode::UNAUTHORIZED, + url: Some("https://example.test/models".to_string()), + headers: Some(headers), + body: Some("plain text error".to_string()), + }), + Duration::from_millis(17), + ); + + let tags = tags.lock().unwrap().clone(); + assert_eq!( + tags.get("endpoint").map(String::as_str), + Some("\"/models\"") + ); + assert_eq!( + tags.get("auth_mode").map(String::as_str), + Some("\"Chatgpt\"") + ); + assert_eq!( + tags.get("auth_request_id").map(String::as_str), + Some("\"req-models-401\"") + ); + assert_eq!( + tags.get("auth_error").map(String::as_str), + Some("\"missing_authorization_header\"") + ); + assert_eq!( + tags.get("auth_error_code").map(String::as_str), + Some("\"token_expired\"") + ); + assert_eq!( + tags.get("auth_env_openai_api_key_present") + .map(String::as_str), + Some("false") + ); + assert_eq!( + tags.get("auth_env_codex_api_key_present") + .map(String::as_str), + Some("false") + ); + assert_eq!( + tags.get("auth_env_codex_api_key_enabled") + .map(String::as_str), + Some("false") + ); + assert_eq!( + tags.get("auth_env_provider_key_name").map(String::as_str), + Some("\"configured\"") + ); + assert_eq!( + tags.get("auth_env_provider_key_present") + .map(String::as_str), + Some("\"false\"") + ); + assert_eq!( + tags.get("auth_env_refresh_token_url_override_present") + .map(String::as_str), + Some("false") + ); +} + #[test] fn build_available_models_picks_default_after_hiding_hidden_models() { let codex_home = tempdir().expect("temp dir"); diff --git a/codex-rs/core/src/util.rs b/codex-rs/core/src/util.rs index 43c6d8522..1dbd6a84f 100644 --- a/codex-rs/core/src/util.rs +++ b/codex-rs/core/src/util.rs @@ -7,6 +7,7 @@ use rand::Rng; use tracing::debug; use tracing::error; +use crate::auth_env_telemetry::AuthEnvTelemetry; use crate::parse_command::shlex_join; const INITIAL_DELAY_MS: u64 = 200; @@ -54,6 +55,23 @@ pub(crate) struct FeedbackRequestTags<'a> { pub auth_recovery_followup_status: Option, } +struct FeedbackRequestSnapshot<'a> { + endpoint: &'a str, + auth_header_attached: bool, + auth_header_name: &'a str, + auth_mode: &'a str, + auth_retry_after_unauthorized: String, + auth_recovery_mode: &'a str, + auth_recovery_phase: &'a str, + auth_connection_reused: String, + auth_request_id: &'a str, + auth_cf_ray: &'a str, + auth_error: &'a str, + auth_error_code: &'a str, + auth_recovery_followup_success: String, + auth_recovery_followup_status: String, +} + struct Auth401FeedbackSnapshot<'a> { request_id: &'a str, cf_ray: &'a str, @@ -77,42 +95,84 @@ impl<'a> Auth401FeedbackSnapshot<'a> { } } +impl<'a> FeedbackRequestSnapshot<'a> { + fn from_tags(tags: &'a FeedbackRequestTags<'a>) -> Self { + Self { + endpoint: tags.endpoint, + auth_header_attached: tags.auth_header_attached, + auth_header_name: tags.auth_header_name.unwrap_or(""), + auth_mode: tags.auth_mode.unwrap_or(""), + auth_retry_after_unauthorized: tags + .auth_retry_after_unauthorized + .map_or_else(String::new, |value| value.to_string()), + auth_recovery_mode: tags.auth_recovery_mode.unwrap_or(""), + auth_recovery_phase: tags.auth_recovery_phase.unwrap_or(""), + auth_connection_reused: tags + .auth_connection_reused + .map_or_else(String::new, |value| value.to_string()), + auth_request_id: tags.auth_request_id.unwrap_or(""), + auth_cf_ray: tags.auth_cf_ray.unwrap_or(""), + auth_error: tags.auth_error.unwrap_or(""), + auth_error_code: tags.auth_error_code.unwrap_or(""), + auth_recovery_followup_success: tags + .auth_recovery_followup_success + .map_or_else(String::new, |value| value.to_string()), + auth_recovery_followup_status: tags + .auth_recovery_followup_status + .map_or_else(String::new, |value| value.to_string()), + } + } +} + +#[cfg(test)] pub(crate) fn emit_feedback_request_tags(tags: &FeedbackRequestTags<'_>) { - let auth_header_name = tags.auth_header_name.unwrap_or(""); - let auth_mode = tags.auth_mode.unwrap_or(""); - let auth_retry_after_unauthorized = tags - .auth_retry_after_unauthorized - .map_or_else(String::new, |value| value.to_string()); - let auth_recovery_mode = tags.auth_recovery_mode.unwrap_or(""); - let auth_recovery_phase = tags.auth_recovery_phase.unwrap_or(""); - let auth_connection_reused = tags - .auth_connection_reused - .map_or_else(String::new, |value| value.to_string()); - let auth_request_id = tags.auth_request_id.unwrap_or(""); - let auth_cf_ray = tags.auth_cf_ray.unwrap_or(""); - let auth_error = tags.auth_error.unwrap_or(""); - let auth_error_code = tags.auth_error_code.unwrap_or(""); - let auth_recovery_followup_success = tags - .auth_recovery_followup_success - .map_or_else(String::new, |value| value.to_string()); - let auth_recovery_followup_status = tags - .auth_recovery_followup_status - .map_or_else(String::new, |value| value.to_string()); + let snapshot = FeedbackRequestSnapshot::from_tags(tags); feedback_tags!( - endpoint = tags.endpoint, - auth_header_attached = tags.auth_header_attached, - auth_header_name = auth_header_name, - auth_mode = auth_mode, - auth_retry_after_unauthorized = auth_retry_after_unauthorized, - auth_recovery_mode = auth_recovery_mode, - auth_recovery_phase = auth_recovery_phase, - auth_connection_reused = auth_connection_reused, - auth_request_id = auth_request_id, - auth_cf_ray = auth_cf_ray, - auth_error = auth_error, - auth_error_code = auth_error_code, - auth_recovery_followup_success = auth_recovery_followup_success, - auth_recovery_followup_status = auth_recovery_followup_status + endpoint = snapshot.endpoint, + auth_header_attached = snapshot.auth_header_attached, + auth_header_name = snapshot.auth_header_name, + auth_mode = snapshot.auth_mode, + auth_retry_after_unauthorized = snapshot.auth_retry_after_unauthorized, + auth_recovery_mode = snapshot.auth_recovery_mode, + auth_recovery_phase = snapshot.auth_recovery_phase, + auth_connection_reused = snapshot.auth_connection_reused, + auth_request_id = snapshot.auth_request_id, + auth_cf_ray = snapshot.auth_cf_ray, + auth_error = snapshot.auth_error, + auth_error_code = snapshot.auth_error_code, + auth_recovery_followup_success = snapshot.auth_recovery_followup_success, + auth_recovery_followup_status = snapshot.auth_recovery_followup_status + ); +} + +pub(crate) fn emit_feedback_request_tags_with_auth_env( + tags: &FeedbackRequestTags<'_>, + auth_env: &AuthEnvTelemetry, +) { + let snapshot = FeedbackRequestSnapshot::from_tags(tags); + feedback_tags!( + endpoint = snapshot.endpoint, + auth_header_attached = snapshot.auth_header_attached, + auth_header_name = snapshot.auth_header_name, + auth_mode = snapshot.auth_mode, + auth_retry_after_unauthorized = snapshot.auth_retry_after_unauthorized, + auth_recovery_mode = snapshot.auth_recovery_mode, + auth_recovery_phase = snapshot.auth_recovery_phase, + auth_connection_reused = snapshot.auth_connection_reused, + auth_request_id = snapshot.auth_request_id, + auth_cf_ray = snapshot.auth_cf_ray, + auth_error = snapshot.auth_error, + auth_error_code = snapshot.auth_error_code, + auth_recovery_followup_success = snapshot.auth_recovery_followup_success, + auth_recovery_followup_status = snapshot.auth_recovery_followup_status, + auth_env_openai_api_key_present = auth_env.openai_api_key_env_present, + auth_env_codex_api_key_present = auth_env.codex_api_key_env_present, + auth_env_codex_api_key_enabled = auth_env.codex_api_key_env_enabled, + auth_env_provider_key_name = auth_env.provider_env_key_name.as_deref().unwrap_or(""), + auth_env_provider_key_present = auth_env + .provider_env_key_present + .map_or_else(String::new, |value| value.to_string()), + auth_env_refresh_token_url_override_present = auth_env.refresh_token_url_override_present ); } diff --git a/codex-rs/core/src/util_tests.rs b/codex-rs/core/src/util_tests.rs index 9df8c67e8..0e9979309 100644 --- a/codex-rs/core/src/util_tests.rs +++ b/codex-rs/core/src/util_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::auth_env_telemetry::AuthEnvTelemetry; use std::collections::BTreeMap; use std::sync::Arc; use std::sync::Mutex; @@ -68,6 +69,7 @@ impl Visit for TagCollectorVisitor { #[derive(Clone)] struct TagCollectorLayer { tags: Arc>>, + event_count: Arc>, } impl Layer for TagCollectorLayer @@ -81,32 +83,49 @@ where let mut visitor = TagCollectorVisitor::default(); event.record(&mut visitor); self.tags.lock().unwrap().extend(visitor.tags); + *self.event_count.lock().unwrap() += 1; } } #[test] fn emit_feedback_request_tags_records_sentry_feedback_fields() { let tags = Arc::new(Mutex::new(BTreeMap::new())); + let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() - .with(TagCollectorLayer { tags: tags.clone() }) + .with(TagCollectorLayer { + tags: tags.clone(), + event_count: event_count.clone(), + }) .set_default(); - emit_feedback_request_tags(&FeedbackRequestTags { - endpoint: "/responses", - auth_header_attached: true, - auth_header_name: Some("authorization"), - auth_mode: Some("chatgpt"), - auth_retry_after_unauthorized: Some(false), - auth_recovery_mode: Some("managed"), - auth_recovery_phase: Some("refresh_token"), - auth_connection_reused: Some(true), - auth_request_id: Some("req-123"), - auth_cf_ray: Some("ray-123"), - auth_error: Some("missing_authorization_header"), - auth_error_code: Some("token_expired"), - auth_recovery_followup_success: Some(true), - auth_recovery_followup_status: Some(200), - }); + let auth_env = AuthEnvTelemetry { + openai_api_key_env_present: true, + codex_api_key_env_present: false, + codex_api_key_env_enabled: true, + provider_env_key_name: Some("configured".to_string()), + provider_env_key_present: Some(true), + refresh_token_url_override_present: true, + }; + + emit_feedback_request_tags_with_auth_env( + &FeedbackRequestTags { + endpoint: "/responses", + auth_header_attached: true, + auth_header_name: Some("authorization"), + auth_mode: Some("chatgpt"), + auth_retry_after_unauthorized: Some(false), + auth_recovery_mode: Some("managed"), + auth_recovery_phase: Some("refresh_token"), + auth_connection_reused: Some(true), + auth_request_id: Some("req-123"), + auth_cf_ray: Some("ray-123"), + auth_error: Some("missing_authorization_header"), + auth_error_code: Some("token_expired"), + auth_recovery_followup_success: Some(true), + auth_recovery_followup_status: Some(200), + }, + &auth_env, + ); let tags = tags.lock().unwrap().clone(); assert_eq!( @@ -121,6 +140,35 @@ fn emit_feedback_request_tags_records_sentry_feedback_fields() { tags.get("auth_header_name").map(String::as_str), Some("\"authorization\"") ); + assert_eq!( + tags.get("auth_env_openai_api_key_present") + .map(String::as_str), + Some("true") + ); + assert_eq!( + tags.get("auth_env_codex_api_key_present") + .map(String::as_str), + Some("false") + ); + assert_eq!( + tags.get("auth_env_codex_api_key_enabled") + .map(String::as_str), + Some("true") + ); + assert_eq!( + tags.get("auth_env_provider_key_name").map(String::as_str), + Some("\"configured\"") + ); + assert_eq!( + tags.get("auth_env_provider_key_present") + .map(String::as_str), + Some("\"true\"") + ); + assert_eq!( + tags.get("auth_env_refresh_token_url_override_present") + .map(String::as_str), + Some("true") + ); assert_eq!( tags.get("auth_request_id").map(String::as_str), Some("\"req-123\"") @@ -139,13 +187,18 @@ fn emit_feedback_request_tags_records_sentry_feedback_fields() { .map(String::as_str), Some("\"200\"") ); + assert_eq!(*event_count.lock().unwrap(), 1); } #[test] fn emit_feedback_auth_recovery_tags_preserves_401_specific_fields() { let tags = Arc::new(Mutex::new(BTreeMap::new())); + let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() - .with(TagCollectorLayer { tags: tags.clone() }) + .with(TagCollectorLayer { + tags: tags.clone(), + event_count: event_count.clone(), + }) .set_default(); emit_feedback_auth_recovery_tags( @@ -175,13 +228,18 @@ fn emit_feedback_auth_recovery_tags_preserves_401_specific_fields() { tags.get("auth_401_error_code").map(String::as_str), Some("\"token_expired\"") ); + assert_eq!(*event_count.lock().unwrap(), 1); } #[test] fn emit_feedback_auth_recovery_tags_clears_stale_401_fields() { let tags = Arc::new(Mutex::new(BTreeMap::new())); + let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() - .with(TagCollectorLayer { tags: tags.clone() }) + .with(TagCollectorLayer { + tags: tags.clone(), + event_count: event_count.clone(), + }) .set_default(); emit_feedback_auth_recovery_tags( @@ -217,13 +275,18 @@ fn emit_feedback_auth_recovery_tags_clears_stale_401_fields() { tags.get("auth_401_error_code").map(String::as_str), Some("\"\"") ); + assert_eq!(*event_count.lock().unwrap(), 2); } #[test] fn emit_feedback_request_tags_preserves_latest_auth_fields_after_unauthorized() { let tags = Arc::new(Mutex::new(BTreeMap::new())); + let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() - .with(TagCollectorLayer { tags: tags.clone() }) + .with(TagCollectorLayer { + tags: tags.clone(), + event_count: event_count.clone(), + }) .set_default(); emit_feedback_request_tags(&FeedbackRequestTags { @@ -265,31 +328,48 @@ fn emit_feedback_request_tags_preserves_latest_auth_fields_after_unauthorized() .map(String::as_str), Some("\"false\"") ); + assert_eq!(*event_count.lock().unwrap(), 1); } #[test] -fn emit_feedback_request_tags_clears_stale_latest_auth_fields() { +fn emit_feedback_request_tags_preserves_auth_env_fields_for_legacy_emitters() { let tags = Arc::new(Mutex::new(BTreeMap::new())); + let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() - .with(TagCollectorLayer { tags: tags.clone() }) + .with(TagCollectorLayer { + tags: tags.clone(), + event_count: event_count.clone(), + }) .set_default(); - emit_feedback_request_tags(&FeedbackRequestTags { - endpoint: "/responses", - auth_header_attached: true, - auth_header_name: Some("authorization"), - auth_mode: Some("chatgpt"), - auth_retry_after_unauthorized: Some(false), - auth_recovery_mode: Some("managed"), - auth_recovery_phase: Some("refresh_token"), - auth_connection_reused: Some(true), - auth_request_id: Some("req-123"), - auth_cf_ray: Some("ray-123"), - auth_error: Some("missing_authorization_header"), - auth_error_code: Some("token_expired"), - auth_recovery_followup_success: Some(true), - auth_recovery_followup_status: Some(200), - }); + let auth_env = AuthEnvTelemetry { + openai_api_key_env_present: true, + codex_api_key_env_present: true, + codex_api_key_env_enabled: true, + provider_env_key_name: Some("configured".to_string()), + provider_env_key_present: Some(true), + refresh_token_url_override_present: true, + }; + + emit_feedback_request_tags_with_auth_env( + &FeedbackRequestTags { + endpoint: "/responses", + auth_header_attached: true, + auth_header_name: Some("authorization"), + auth_mode: Some("chatgpt"), + auth_retry_after_unauthorized: Some(false), + auth_recovery_mode: Some("managed"), + auth_recovery_phase: Some("refresh_token"), + auth_connection_reused: Some(true), + auth_request_id: Some("req-123"), + auth_cf_ray: Some("ray-123"), + auth_error: Some("missing_authorization_header"), + auth_error_code: Some("token_expired"), + auth_recovery_followup_success: Some(true), + auth_recovery_followup_status: Some(200), + }, + &auth_env, + ); emit_feedback_request_tags(&FeedbackRequestTags { endpoint: "/responses", auth_header_attached: true, @@ -323,6 +403,35 @@ fn emit_feedback_request_tags_clears_stale_latest_auth_fields() { tags.get("auth_error_code").map(String::as_str), Some("\"\"") ); + assert_eq!( + tags.get("auth_env_openai_api_key_present") + .map(String::as_str), + Some("true") + ); + assert_eq!( + tags.get("auth_env_codex_api_key_present") + .map(String::as_str), + Some("true") + ); + assert_eq!( + tags.get("auth_env_codex_api_key_enabled") + .map(String::as_str), + Some("true") + ); + assert_eq!( + tags.get("auth_env_provider_key_name").map(String::as_str), + Some("\"configured\"") + ); + assert_eq!( + tags.get("auth_env_provider_key_present") + .map(String::as_str), + Some("\"true\"") + ); + assert_eq!( + tags.get("auth_env_refresh_token_url_override_present") + .map(String::as_str), + Some("true") + ); assert_eq!( tags.get("auth_recovery_followup_success") .map(String::as_str), @@ -333,6 +442,7 @@ fn emit_feedback_request_tags_clears_stale_latest_auth_fields() { .map(String::as_str), Some("\"\"") ); + assert_eq!(*event_count.lock().unwrap(), 2); } #[test] diff --git a/codex-rs/otel/src/events/session_telemetry.rs b/codex-rs/otel/src/events/session_telemetry.rs index aac10e2c1..e2c86a6e6 100644 --- a/codex-rs/otel/src/events/session_telemetry.rs +++ b/codex-rs/otel/src/events/session_telemetry.rs @@ -62,10 +62,21 @@ const RESPONSES_API_ENGINE_SERVICE_TTFT_FIELD: &str = "engine_service_ttft_total const RESPONSES_API_ENGINE_IAPI_TBT_FIELD: &str = "engine_iapi_tbt_across_engine_calls_ms"; const RESPONSES_API_ENGINE_SERVICE_TBT_FIELD: &str = "engine_service_tbt_across_engine_calls_ms"; +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct AuthEnvTelemetryMetadata { + pub openai_api_key_env_present: bool, + pub codex_api_key_env_present: bool, + pub codex_api_key_env_enabled: bool, + pub provider_env_key_name: Option, + pub provider_env_key_present: Option, + pub refresh_token_url_override_present: bool, +} + #[derive(Debug, Clone)] pub struct SessionTelemetryMetadata { pub(crate) conversation_id: ThreadId, pub(crate) auth_mode: Option, + pub(crate) auth_env: AuthEnvTelemetryMetadata, pub(crate) account_id: Option, pub(crate) account_email: Option, pub(crate) originator: String, @@ -86,6 +97,11 @@ pub struct SessionTelemetry { } impl SessionTelemetry { + pub fn with_auth_env(mut self, auth_env: AuthEnvTelemetryMetadata) -> Self { + self.metadata.auth_env = auth_env; + self + } + pub fn with_model(mut self, model: &str, slug: &str) -> Self { self.metadata.model = model.to_owned(); self.metadata.slug = slug.to_owned(); @@ -255,6 +271,7 @@ impl SessionTelemetry { metadata: SessionTelemetryMetadata { conversation_id, auth_mode: auth_mode.map(|m| m.to_string()), + auth_env: AuthEnvTelemetryMetadata::default(), account_id, account_email, originator: sanitize_metric_tag_value(originator.as_str()), @@ -309,6 +326,12 @@ impl SessionTelemetry { common: { event.name = "codex.conversation_starts", provider_name = %provider_name, + auth.env_openai_api_key_present = self.metadata.auth_env.openai_api_key_env_present, + auth.env_codex_api_key_present = self.metadata.auth_env.codex_api_key_env_present, + auth.env_codex_api_key_enabled = self.metadata.auth_env.codex_api_key_env_enabled, + auth.env_provider_key_name = self.metadata.auth_env.provider_env_key_name.as_deref(), + auth.env_provider_key_present = self.metadata.auth_env.provider_env_key_present, + auth.env_refresh_token_url_override_present = self.metadata.auth_env.refresh_token_url_override_present, reasoning_effort = reasoning_effort.map(|e| e.to_string()), reasoning_summary = %reasoning_summary, context_window = context_window, @@ -407,6 +430,12 @@ impl SessionTelemetry { auth.recovery_mode = recovery_mode, auth.recovery_phase = recovery_phase, endpoint = endpoint, + auth.env_openai_api_key_present = self.metadata.auth_env.openai_api_key_env_present, + auth.env_codex_api_key_present = self.metadata.auth_env.codex_api_key_env_present, + auth.env_codex_api_key_enabled = self.metadata.auth_env.codex_api_key_env_enabled, + auth.env_provider_key_name = self.metadata.auth_env.provider_env_key_name.as_deref(), + auth.env_provider_key_present = self.metadata.auth_env.provider_env_key_present, + auth.env_refresh_token_url_override_present = self.metadata.auth_env.refresh_token_url_override_present, auth.request_id = request_id, auth.cf_ray = cf_ray, auth.error = auth_error, @@ -454,6 +483,12 @@ impl SessionTelemetry { auth.recovery_mode = recovery_mode, auth.recovery_phase = recovery_phase, endpoint = endpoint, + auth.env_openai_api_key_present = self.metadata.auth_env.openai_api_key_env_present, + auth.env_codex_api_key_present = self.metadata.auth_env.codex_api_key_env_present, + auth.env_codex_api_key_enabled = self.metadata.auth_env.codex_api_key_env_enabled, + auth.env_provider_key_name = self.metadata.auth_env.provider_env_key_name.as_deref(), + auth.env_provider_key_present = self.metadata.auth_env.provider_env_key_present, + auth.env_refresh_token_url_override_present = self.metadata.auth_env.refresh_token_url_override_present, auth.connection_reused = connection_reused, auth.request_id = request_id, auth.cf_ray = cf_ray, @@ -489,6 +524,12 @@ impl SessionTelemetry { duration_ms = %duration.as_millis(), success = success_str, error.message = error, + auth.env_openai_api_key_present = self.metadata.auth_env.openai_api_key_env_present, + auth.env_codex_api_key_present = self.metadata.auth_env.codex_api_key_env_present, + auth.env_codex_api_key_enabled = self.metadata.auth_env.codex_api_key_env_enabled, + auth.env_provider_key_name = self.metadata.auth_env.provider_env_key_name.as_deref(), + auth.env_provider_key_present = self.metadata.auth_env.provider_env_key_present, + auth.env_refresh_token_url_override_present = self.metadata.auth_env.refresh_token_url_override_present, auth.connection_reused = connection_reused, }, log: {}, diff --git a/codex-rs/otel/src/lib.rs b/codex-rs/otel/src/lib.rs index cd1bbe5ce..353130976 100644 --- a/codex-rs/otel/src/lib.rs +++ b/codex-rs/otel/src/lib.rs @@ -12,6 +12,7 @@ use crate::metrics::Result as MetricsResult; use serde::Serialize; use strum_macros::Display; +pub use crate::events::session_telemetry::AuthEnvTelemetryMetadata; pub use crate::events::session_telemetry::SessionTelemetry; pub use crate::events::session_telemetry::SessionTelemetryMetadata; pub use crate::metrics::runtime_metrics::RuntimeMetricTotals; diff --git a/codex-rs/otel/tests/suite/otel_export_routing_policy.rs b/codex-rs/otel/tests/suite/otel_export_routing_policy.rs index df5d876b4..b7fce8614 100644 --- a/codex-rs/otel/tests/suite/otel_export_routing_policy.rs +++ b/codex-rs/otel/tests/suite/otel_export_routing_policy.rs @@ -1,3 +1,4 @@ +use codex_otel::AuthEnvTelemetryMetadata; use codex_otel::OtelProvider; use codex_otel::SessionTelemetry; use codex_otel::TelemetryAuthMode; @@ -18,6 +19,9 @@ use tracing_subscriber::filter::filter_fn; use tracing_subscriber::layer::SubscriberExt; use codex_protocol::ThreadId; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; @@ -76,6 +80,17 @@ fn find_span_event_by_name_attr<'a>( .unwrap_or_else(|| panic!("missing span event: {event_name}")) } +fn auth_env_metadata() -> AuthEnvTelemetryMetadata { + AuthEnvTelemetryMetadata { + openai_api_key_env_present: true, + codex_api_key_env_present: false, + codex_api_key_env_enabled: true, + provider_env_key_name: Some("configured".to_string()), + provider_env_key_present: Some(true), + refresh_token_url_override_present: true, + } +} + #[test] fn otel_export_routing_policy_routes_user_prompt_log_and_trace_events() { let log_exporter = InMemoryLogExporter::default(); @@ -482,9 +497,21 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() { true, "tty".to_string(), SessionSource::Cli, - ); + ) + .with_auth_env(auth_env_metadata()); let root_span = tracing::info_span!("root"); let _root_guard = root_span.enter(); + manager.conversation_starts( + "openai", + None, + ReasoningSummary::Auto, + None, + None, + AskForApproval::Never, + SandboxPolicy::DangerFullAccess, + Vec::new(), + None, + ); manager.record_api_request( 1, Some(401), @@ -507,6 +534,20 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() { tracer_provider.force_flush().expect("flush traces"); let logs = log_exporter.get_emitted_logs().expect("log export"); + let conversation_log = find_log_by_event_name(&logs, "codex.conversation_starts"); + let conversation_log_attrs = log_attributes(&conversation_log.record); + assert_eq!( + conversation_log_attrs + .get("auth.env_openai_api_key_present") + .map(String::as_str), + Some("true") + ); + assert_eq!( + conversation_log_attrs + .get("auth.env_provider_key_name") + .map(String::as_str), + Some("configured") + ); let request_log = find_log_by_event_name(&logs, "codex.api_request"); let request_log_attrs = log_attributes(&request_log.record); assert_eq!( @@ -547,8 +588,29 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() { request_log_attrs.get("auth.error").map(String::as_str), Some("missing_authorization_header") ); + assert_eq!( + request_log_attrs + .get("auth.env_codex_api_key_enabled") + .map(String::as_str), + Some("true") + ); + assert_eq!( + request_log_attrs + .get("auth.env_refresh_token_url_override_present") + .map(String::as_str), + Some("true") + ); let spans = span_exporter.get_finished_spans().expect("span export"); + let conversation_trace_event = + find_span_event_by_name_attr(&spans[0].events.events, "codex.conversation_starts"); + let conversation_trace_attrs = span_event_attributes(conversation_trace_event); + assert_eq!( + conversation_trace_attrs + .get("auth.env_provider_key_present") + .map(String::as_str), + Some("true") + ); let request_trace_event = find_span_event_by_name_attr(&spans[0].events.events, "codex.api_request"); let request_trace_attrs = span_event_attributes(request_trace_event); @@ -574,6 +636,12 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() { request_trace_attrs.get("endpoint").map(String::as_str), Some("/responses") ); + assert_eq!( + request_trace_attrs + .get("auth.env_openai_api_key_present") + .map(String::as_str), + Some("true") + ); } #[test] @@ -614,7 +682,8 @@ fn otel_export_routing_policy_routes_websocket_connect_auth_observability() { true, "tty".to_string(), SessionSource::Cli, - ); + ) + .with_auth_env(auth_env_metadata()); let root_span = tracing::info_span!("root"); let _root_guard = root_span.enter(); manager.record_websocket_connect( @@ -667,6 +736,12 @@ fn otel_export_routing_policy_routes_websocket_connect_auth_observability() { .map(String::as_str), Some("false") ); + assert_eq!( + connect_log_attrs + .get("auth.env_provider_key_name") + .map(String::as_str), + Some("configured") + ); let spans = span_exporter.get_finished_spans().expect("span export"); let connect_trace_event = @@ -678,6 +753,12 @@ fn otel_export_routing_policy_routes_websocket_connect_auth_observability() { .map(String::as_str), Some("reload") ); + assert_eq!( + connect_trace_attrs + .get("auth.env_refresh_token_url_override_present") + .map(String::as_str), + Some("true") + ); } #[test] @@ -718,7 +799,8 @@ fn otel_export_routing_policy_routes_websocket_request_transport_observability() true, "tty".to_string(), SessionSource::Cli, - ); + ) + .with_auth_env(auth_env_metadata()); let root_span = tracing::info_span!("root"); let _root_guard = root_span.enter(); manager.record_websocket_request( @@ -744,6 +826,12 @@ fn otel_export_routing_policy_routes_websocket_request_transport_observability() request_log_attrs.get("error.message").map(String::as_str), Some("stream error") ); + assert_eq!( + request_log_attrs + .get("auth.env_openai_api_key_present") + .map(String::as_str), + Some("true") + ); let spans = span_exporter.get_finished_spans().expect("span export"); let request_trace_event = @@ -755,4 +843,10 @@ fn otel_export_routing_policy_routes_websocket_request_transport_observability() .map(String::as_str), Some("true") ); + assert_eq!( + request_trace_attrs + .get("auth.env_provider_key_present") + .map(String::as_str), + Some("true") + ); }