diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 643cfffcff..453885b614 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -938,7 +938,9 @@ dependencies = [ "opentelemetry-semantic-conventions", "opentelemetry_sdk", "reqwest", + "serde", "serde_json", + "strum_macros 0.27.2", "tempfile", "time", "tokio", diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 534bb0e126..6135bf9f61 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -122,7 +122,7 @@ use crate::unified_exec::UnifiedExecSessionManager; use crate::user_instructions::UserInstructions; use crate::user_notification::UserNotification; use crate::util::backoff; -use codex_otel::trace_manager::TraceManager; +use codex_otel::trace_manager::{ToolDecisionOutcome, ToolDecisionSource, TraceManager}; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_protocol::custom_prompts::CustomPrompt; @@ -2303,6 +2303,7 @@ async fn handle_response_item( sess, turn_context, turn_diff_tracker, + "local_shell", sub_id.to_string(), effective_call_id, ) @@ -2449,6 +2450,7 @@ async fn handle_function_call( sess, turn_context, turn_diff_tracker, + name.as_str(), sub_id, call_id, ) @@ -2545,6 +2547,7 @@ async fn handle_function_call( sess, turn_context, turn_diff_tracker, + "apply_patch", sub_id, call_id, ) @@ -2649,6 +2652,7 @@ async fn handle_custom_tool_call( sess, turn_context, turn_diff_tracker, + "apply_patch", sub_id, call_id, ) @@ -2741,9 +2745,13 @@ async fn handle_container_exec_with_params( sess: &Session, turn_context: &TurnContext, turn_diff_tracker: &mut TurnDiffTracker, + tool_name: &str, sub_id: String, call_id: String, ) -> ResponseInputItem { + let trace_manager = turn_context.client.get_trace_manager(); + let mut auto_approved_via_user = false; + if params.with_escalated_permissions.unwrap_or(false) && !matches!(turn_context.approval_policy, AskForApproval::OnRequest) { @@ -2819,6 +2827,7 @@ async fn handle_container_exec_with_params( justification: params.justification.clone(), }; let safety = if *user_explicitly_approved_this_action { + auto_approved_via_user = true; SafetyCheck::AutoApprove { sandbox_type: SandboxType::None, } @@ -2852,7 +2861,21 @@ async fn handle_container_exec_with_params( }; let sandbox_type = match safety { - SafetyCheck::AutoApprove { sandbox_type } => sandbox_type, + SafetyCheck::AutoApprove { sandbox_type } => { + let source = if auto_approved_via_user { + ToolDecisionSource::UserTemporary + } else { + ToolDecisionSource::Config + }; + + trace_manager.tool_decision( + tool_name, + ToolDecisionOutcome::Accept, + source, + ); + + sandbox_type + } SafetyCheck::AskUser => { let rx_approve = sess .request_command_approval( @@ -2864,11 +2887,27 @@ async fn handle_container_exec_with_params( ) .await; match rx_approve.await.unwrap_or_default() { - ReviewDecision::Approved => (), + ReviewDecision::Approved => { + trace_manager.tool_decision( + tool_name, + ToolDecisionOutcome::Accept, + ToolDecisionSource::UserTemporary, + ); + } ReviewDecision::ApprovedForSession => { + trace_manager.tool_decision( + tool_name, + ToolDecisionOutcome::Accept, + ToolDecisionSource::UserForSession, + ); sess.add_approved_command(params.command.clone()).await; } - ReviewDecision::Denied | ReviewDecision::Abort => { + ReviewDecision::Denied => { + trace_manager.tool_decision( + tool_name, + ToolDecisionOutcome::Reject, + ToolDecisionSource::UserReject, + ); return ResponseInputItem::FunctionCallOutput { call_id, output: FunctionCallOutputPayload { @@ -2877,6 +2916,20 @@ async fn handle_container_exec_with_params( }, }; } + ReviewDecision::Abort => { + trace_manager.tool_decision( + tool_name, + ToolDecisionOutcome::Reject, + ToolDecisionSource::UserAbort, + ); + return ResponseInputItem::FunctionCallOutput { + call_id, + output: FunctionCallOutputPayload { + content: "exec command aborted by user".to_string(), + success: None, + }, + }; + } } // No sandboxing is applied because the user has given // explicit approval. Often, we end up in this case because @@ -2885,6 +2938,11 @@ async fn handle_container_exec_with_params( SandboxType::None } SafetyCheck::Reject { reason } => { + trace_manager.tool_decision( + tool_name, + ToolDecisionOutcome::Reject, + ToolDecisionSource::Config, + ); return ResponseInputItem::FunctionCallOutput { call_id, output: FunctionCallOutputPayload { diff --git a/codex-rs/otel/Cargo.toml b/codex-rs/otel/Cargo.toml index e86495a64e..5e794aa051 100644 --- a/codex-rs/otel/Cargo.toml +++ b/codex-rs/otel/Cargo.toml @@ -41,7 +41,9 @@ opentelemetry-otlp = { version = "0.30.0", features = ["grpc-tonic", "http-proto opentelemetry-proto = { version = "0.30.0", features = ["gen-tonic"], optional = true } tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros"], optional = true } tonic = { version = "0.13.1", optional = true } +serde = { version = "1", features = ["derive"] } serde_json = { version = "1", optional = true } +strum_macros = "0.27.2" uuid = { version = "1.18.1", features = ["v4"] } time = { version = "0.3", features = ["formatting", "parsing", "local-offset", "macros"] } walkdir = "2.5.0" diff --git a/codex-rs/otel/src/trace_manager.rs b/codex-rs/otel/src/trace_manager.rs index 7ce3e7928a..3038cb3e7d 100644 --- a/codex-rs/otel/src/trace_manager.rs +++ b/codex-rs/otel/src/trace_manager.rs @@ -9,9 +9,11 @@ use opentelemetry_http::HeaderInjector; use opentelemetry_sdk::propagation::TraceContextPropagator; use reqwest::StatusCode; use reqwest::header::HeaderMap; +use serde::Serialize; use tracing::Span; use tracing::info_span; use tracing_opentelemetry::OpenTelemetrySpanExt; +use strum_macros::Display; pub struct RequestSpan(pub(crate) Span); @@ -133,6 +135,56 @@ impl SSESpan { } } +#[derive(Debug, Clone, Serialize, Display)] +#[serde(rename_all = "snake_case")] +pub enum ToolDecisionOutcome { + Accept, + Reject, +} + +#[derive(Debug, Clone, Serialize, Display)] +#[serde(rename_all = "snake_case")] +pub enum ToolDecisionSource { + Config, + UserForSession, + UserTemporary, + UserAbort, + UserReject, +} + +pub struct ToolDecisionSpan(pub(crate) Span); + +impl ToolDecisionSpan { + pub fn new( + metadata: TraceMetadata, + tool_name: &str, + outcome: ToolDecisionOutcome, + source: ToolDecisionSource, + ) -> Self { + let span = info_span!( + "codex.tool_decision", + session.id = %metadata.conversation_id, + app.version = %metadata.app_version, + user.account_id = tracing::field::Empty, + terminal.type = %metadata.terminal_type, + event.timestamp = %timestamp(), + tool_name = %tool_name, + decision = outcome.to_string(), + source = source.to_string(), + ); + + if let Some(account_id) = &metadata.account_id { + span.record("user.account_id", account_id); + } + + ToolDecisionSpan(span) + } + + pub fn span(&self) -> Span { + self.0.clone() + } +} + pub struct UserPromptSpan(pub(crate) Span); impl UserPromptSpan { @@ -240,6 +292,10 @@ impl TraceManager { UserPromptSpan::new(self.metadata.clone(), prompt.as_ref()) } + + pub fn tool_decision(&self, tool_name: &str, outcome: ToolDecisionOutcome, source: ToolDecisionSource) -> ToolDecisionSpan { + ToolDecisionSpan::new(self.metadata.clone(), tool_name, outcome, source) + } } fn timestamp() -> String {