From 854cbb2fd43f513f73faa7a5f379292377a219c4 Mon Sep 17 00:00:00 2001 From: tongzhou wang Date: Thu, 20 Aug 2026 19:01:33 +0000 Subject: [PATCH] Make tool-result telemetry limits configurable (#39779) ## What changed - Add `otel.tool_result.max_bytes`, defaulting to 2048 bytes, to control the text included in `codex.tool_result` logs independently of model-visible output limits. - Apply truncation centrally when emitting telemetry, preserve UTF-8 boundaries, and report whether the logged output was truncated. - Record typed tool names and namespaces, agent names, and process-local result sequence numbers while keeping tool arguments and output out of trace events. ## Testing - Cover configuration propagation, UTF-8-safe truncation, raw tool-output logging, and log/trace event routing and fields. GitOrigin-RevId: 89ca76e55f1a09b729e7770ce01f2e4d98f9a22d --- codex-rs/config/src/types.rs | 5 + codex-rs/core/config.schema.json | 24 +++ codex-rs/core/src/config/config_tests.rs | 2 + codex-rs/core/src/config/otel.rs | 1 + codex-rs/core/src/session/session.rs | 3 +- codex-rs/core/src/tools/approvals.rs | 4 +- codex-rs/core/src/tools/context.rs | 99 ++++------- codex-rs/core/src/tools/context_tests.rs | 72 ++++---- .../core/src/tools/handlers/current_time.rs | 2 +- .../tools/handlers/get_context_remaining.rs | 2 +- .../handlers/multi_agents/close_agent.rs | 2 +- .../handlers/multi_agents/resume_agent.rs | 2 +- .../tools/handlers/multi_agents/send_input.rs | 2 +- .../src/tools/handlers/multi_agents/spawn.rs | 2 +- .../src/tools/handlers/multi_agents/wait.rs | 2 +- .../multi_agents_v2/interrupt_agent.rs | 2 +- .../handlers/multi_agents_v2/list_agents.rs | 2 +- .../tools/handlers/multi_agents_v2/spawn.rs | 2 +- .../tools/handlers/multi_agents_v2/wait.rs | 2 +- codex-rs/core/src/tools/handlers/plan.rs | 2 +- .../core/src/tools/handlers/view_image.rs | 4 +- codex-rs/core/src/tools/mod.rs | 6 - codex-rs/core/src/tools/network_approval.rs | 11 +- .../core/src/tools/network_approval_tests.rs | 6 +- codex-rs/core/src/tools/orchestrator.rs | 2 +- codex-rs/core/src/tools/registry.rs | 58 ++----- .../core/src/tools/runtimes/unified_exec.rs | 1 + codex-rs/core/tests/suite/otel.rs | 9 +- codex-rs/ext/image-generation/src/tool.rs | 2 +- codex-rs/ext/web-search/src/output.rs | 2 +- codex-rs/otel/src/events/session_telemetry.rs | 118 +++++++------ codex-rs/otel/src/events/shared.rs | 10 ++ codex-rs/otel/src/lib.rs | 1 + codex-rs/otel/src/tool_result.rs | 114 ++++++++++++ codex-rs/otel/src/tool_result_tests.rs | 67 +++++++ .../tests/suite/otel_export_routing_policy.rs | 163 ++++++++++++++++-- codex-rs/otel/tests/suite/runtime_summary.rs | 3 +- codex-rs/protocol/src/config_types.rs | 17 ++ codex-rs/tools/src/tool_output.rs | 65 ++----- 39 files changed, 583 insertions(+), 310 deletions(-) create mode 100644 codex-rs/otel/src/tool_result.rs create mode 100644 codex-rs/otel/src/tool_result_tests.rs diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 148fef00f7..4e01557be5 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -548,6 +548,9 @@ pub enum OtelExporterKind { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct OtelConfigToml { + /// Byte limit for tool-result log output; independent of model-visible output. + #[serde(default)] + pub tool_result: codex_protocol::config_types::ToolResultLogConfig, /// Log user prompt in traces pub log_user_prompt: Option, @@ -573,6 +576,7 @@ pub struct OtelConfigToml { /// Effective OTEL settings after defaults are applied. #[derive(Debug, Clone, PartialEq)] pub struct OtelConfig { + pub tool_result: codex_protocol::config_types::ToolResultLogConfig, pub log_user_prompt: bool, pub environment: String, pub exporter: OtelExporterKind, @@ -585,6 +589,7 @@ pub struct OtelConfig { impl Default for OtelConfig { fn default() -> Self { OtelConfig { + tool_result: Default::default(), log_user_prompt: false, environment: DEFAULT_OTEL_ENVIRONMENT.to_owned(), exporter: OtelExporterKind::None, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 51eefd8e0a..1a9f2611fd 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2561,6 +2561,17 @@ "description": "Attributes to add to every exported trace span.", "type": "object" }, + "tool_result": { + "allOf": [ + { + "$ref": "#/definitions/ToolResultLogConfig" + } + ], + "default": { + "max_bytes": 2048 + }, + "description": "Byte limit for tool-result log output; independent of model-visible output." + }, "trace_exporter": { "allOf": [ { @@ -3410,6 +3421,19 @@ }, "type": "object" }, + "ToolResultLogConfig": { + "description": "Limit for the text included in `codex.tool_result` log records. This does not affect model-visible output. Raising it can expose more tool data to logs.", + "properties": { + "max_bytes": { + "default": 2048, + "description": "Maximum UTF-8 bytes before the truncation notice. Defaults to 2048.", + "format": "uint", + "minimum": 0.0, + "type": "integer" + } + }, + "type": "object" + }, "ToolSuggestConfig": { "additionalProperties": false, "properties": { diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 57f96aaaa6..0749e3aed9 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -9554,6 +9554,7 @@ async fn trace_exporter_defaults_to_none_when_log_exporter_is_set() -> std::io:: let fixture = create_test_fixture()?; let mut cfg = fixture.cfg.clone(); cfg.otel = Some(OtelConfigToml { + tool_result: toml::from_str("max_bytes = 8192").expect("tool-result logging config"), exporter: Some(OtelExporterKind::OtlpHttp { endpoint: "http://localhost:14318/v1/logs".to_string(), headers: HashMap::new(), @@ -9574,6 +9575,7 @@ async fn trace_exporter_defaults_to_none_when_log_exporter_is_set() -> std::io:: ) .await?; + assert_eq!(config.otel.tool_result.max_bytes, 8192); assert!(matches!( config.otel.exporter, OtelExporterKind::OtlpHttp { .. } diff --git a/codex-rs/core/src/config/otel.rs b/codex-rs/core/src/config/otel.rs index cb65d304d1..56f3b79cf4 100644 --- a/codex-rs/core/src/config/otel.rs +++ b/codex-rs/core/src/config/otel.rs @@ -26,6 +26,7 @@ pub(crate) fn resolve_config( let tracestate = resolve_tracestate(config.tracestate, startup_warnings); OtelConfig { + tool_result: config.tool_result, log_user_prompt, environment, exporter, diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index c0b0407967..c185a62455 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1067,7 +1067,8 @@ impl Session { terminal_type.clone(), session_configuration.session_source.clone(), ) - .with_auth_env(auth_env_telemetry.to_otel_metadata()); + .with_auth_env(auth_env_telemetry.to_otel_metadata()) + .with_tool_result_log_config(config.otel.tool_result); 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/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 780a93b74e..e4f8ee21d9 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -15,7 +15,6 @@ use crate::mcp_tool_call::request_mcp_tool_user_approval; use crate::sandboxing::SandboxPermissions; use crate::session::session::Session; use crate::session::turn_context::TurnContext; -use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; use crate::tools::runtimes::apply_patch::ApplyPatchApprovalKey; use crate::tools::runtimes::unified_exec::UnifiedExecApprovalKey; @@ -778,9 +777,8 @@ fn record_resolution(ctx: &ApprovalContext, resolution: &ApprovalResolution) { ApprovalResolutionSource::Guardian => ToolDecisionSource::AutomatedReviewer, ApprovalResolutionSource::User => ToolDecisionSource::User, }; - let tool_name = flat_tool_name(&ctx.tool_name); ctx.review_context.turn().session_telemetry.tool_decision( - tool_name.as_ref(), + &ctx.tool_name, &ctx.call_id, &resolution.decision, Some(source), diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index d1b12b0544..dafb4a3dcc 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -3,9 +3,6 @@ use crate::original_image_detail::sanitize_original_image_detail; use crate::session::session::Session; use crate::session::step_context::StepContext; use crate::session::turn_context::TurnContext; -use crate::tools::TELEMETRY_PREVIEW_MAX_BYTES; -use crate::tools::TELEMETRY_PREVIEW_MAX_LINES; -use crate::tools::TELEMETRY_PREVIEW_TRUNCATION_NOTICE; use crate::turn_diff_tracker::TurnDiffTracker; use crate::unified_exec::format_output_omission_marker; use crate::unified_exec::resolve_max_tokens; @@ -21,7 +18,6 @@ use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::approx_token_count; use codex_utils_output_truncation::formatted_truncate_text; use codex_utils_output_truncation::truncate_text; -use codex_utils_string::take_bytes_at_char_boundary; use serde::Serialize; use serde_json::Value as JsonValue; use std::num::NonZeroUsize; @@ -80,13 +76,16 @@ pub struct McpToolOutput { } impl ToolOutput for McpToolOutput { - fn log_preview(&self) -> String { - let payload = self.response_payload(); - let preview = payload.body.to_text().unwrap_or_else(|| { - serde_json::to_string(&self.result.content) - .unwrap_or_else(|err| format!("failed to serialize mcp result: {err}")) - }); - telemetry_preview(&preview) + fn log_output(&self) -> String { + // Logging has its own budget; do not first apply the model-context budget. + let output = self.result.log_output(); + let wall_time_seconds = self.wall_time.as_secs_f64(); + let header = format!("Wall time: {wall_time_seconds:.4} seconds\nOutput:"); + if output.is_empty() { + header + } else { + format!("{header}\n{output}") + } } fn success_for_logging(&self) -> bool { @@ -152,7 +151,7 @@ pub struct ToolSearchOutput { } impl ToolOutput for ToolSearchOutput { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { let tools = self .tools .iter() @@ -162,7 +161,7 @@ impl ToolOutput for ToolSearchOutput { }) }) .collect(); - telemetry_preview(&JsonValue::Array(tools).to_string()) + JsonValue::Array(tools).to_string() } fn success_for_logging(&self) -> bool { @@ -219,10 +218,8 @@ impl FunctionToolOutput { } impl ToolOutput for FunctionToolOutput { - fn log_preview(&self) -> String { - telemetry_preview( - &function_call_output_content_items_to_text(&self.body).unwrap_or_default(), - ) + fn log_output(&self) -> String { + function_call_output_content_items_to_text(&self.body).unwrap_or_default() } fn success_for_logging(&self) -> bool { @@ -249,8 +246,8 @@ impl ApplyPatchToolOutput { } impl ToolOutput for ApplyPatchToolOutput { - fn log_preview(&self) -> String { - telemetry_preview(&self.text) + fn log_output(&self) -> String { + self.text.clone() } fn success_for_logging(&self) -> bool { @@ -282,8 +279,8 @@ pub struct AbortedToolOutput { } impl ToolOutput for AbortedToolOutput { - fn log_preview(&self) -> String { - telemetry_preview(&self.message) + fn log_output(&self) -> String { + self.message.clone() } fn success_for_logging(&self) -> bool { @@ -328,8 +325,16 @@ pub struct ExecCommandToolOutput { } impl ToolOutput for ExecCommandToolOutput { - fn log_preview(&self) -> String { - telemetry_preview(&self.response_text()) + fn log_output(&self) -> String { + // The telemetry budget must not inherit the model's output-token limit. + let mut output = String::from_utf8_lossy(&self.raw_output).into_owned(); + if let Some(omitted_bytes) = self.output_omitted_bytes { + let marker = format_output_omission_marker(omitted_bytes.get()); + if !output.contains(&marker) { + output = format!("{marker}\n{output}"); + } + } + format!("{}\n{output}", self.response_header()) } fn success_for_logging(&self) -> bool { @@ -447,7 +452,7 @@ impl ExecCommandToolOutput { ) } - fn response_text(&self) -> String { + fn response_header(&self) -> String { let mut sections = Vec::new(); if !self.chunk_id.is_empty() { @@ -470,7 +475,11 @@ impl ExecCommandToolOutput { } sections.push("Output:".to_string()); - let header = sections.join("\n"); + sections.join("\n") + } + + fn response_text(&self) -> String { + let header = self.response_header(); let output_budget = (self.truncation_policy * 1.2) .byte_budget() .saturating_sub(header.len().saturating_add(/*rhs*/ 1)); @@ -524,46 +533,6 @@ fn function_tool_response( } } -fn telemetry_preview(content: &str) -> String { - let truncated_slice = take_bytes_at_char_boundary(content, TELEMETRY_PREVIEW_MAX_BYTES); - let truncated_by_bytes = truncated_slice.len() < content.len(); - - let mut preview = String::new(); - let mut lines_iter = truncated_slice.lines(); - for idx in 0..TELEMETRY_PREVIEW_MAX_LINES { - match lines_iter.next() { - Some(line) => { - if idx > 0 { - preview.push('\n'); - } - preview.push_str(line); - } - None => break, - } - } - let truncated_by_lines = lines_iter.next().is_some(); - - if !truncated_by_bytes && !truncated_by_lines { - return content.to_string(); - } - - if preview.len() < truncated_slice.len() - && truncated_slice - .as_bytes() - .get(preview.len()) - .is_some_and(|byte| *byte == b'\n') - { - preview.push('\n'); - } - - if !preview.is_empty() && !preview.ends_with('\n') { - preview.push('\n'); - } - preview.push_str(TELEMETRY_PREVIEW_TRUNCATION_NOTICE); - - preview -} - #[cfg(test)] #[path = "context_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/context_tests.rs b/codex-rs/core/src/tools/context_tests.rs index a13527f5cf..60c6382b5c 100644 --- a/codex-rs/core/src/tools/context_tests.rs +++ b/codex-rs/core/src/tools/context_tests.rs @@ -153,6 +153,13 @@ fn mcp_tool_output_response_item_truncates_large_structured_content() { truncation_policy: TruncationPolicy::Bytes(128), }; + assert_eq!( + output.log_output(), + format!( + "Wall time: 1.2500 seconds\nOutput:\n{}", + json!({"items": "large structured value ".repeat(1_000)}) + ) + ); let response = output.to_response_item( "mcp-call-large", &ToolPayload::Function { @@ -384,51 +391,19 @@ fn log_preview_uses_content_items_when_plain_text_is_missing() { Some(true), ); - assert_eq!(output.log_preview(), "preview"); + assert_eq!(output.log_output(), "preview"); assert_eq!( function_call_output_content_items_to_text(&output.body), Some("preview".to_string()) ); } -#[test] -fn telemetry_preview_returns_original_within_limits() { - let content = "short output"; - assert_eq!(telemetry_preview(content), content); -} - -#[test] -fn telemetry_preview_truncates_by_bytes() { - let content = "x".repeat(TELEMETRY_PREVIEW_MAX_BYTES + 8); - let preview = telemetry_preview(&content); - - assert!(preview.contains(TELEMETRY_PREVIEW_TRUNCATION_NOTICE)); - assert!( - preview.len() - <= TELEMETRY_PREVIEW_MAX_BYTES + TELEMETRY_PREVIEW_TRUNCATION_NOTICE.len() + 1 - ); -} - -#[test] -fn telemetry_preview_truncates_by_lines() { - let content = (0..(TELEMETRY_PREVIEW_MAX_LINES + 5)) - .map(|idx| format!("line {idx}")) - .collect::>() - .join("\n"); - - let preview = telemetry_preview(&content); - let lines: Vec<&str> = preview.lines().collect(); - - assert!(lines.len() <= TELEMETRY_PREVIEW_MAX_LINES + 1); - assert_eq!(lines.last(), Some(&TELEMETRY_PREVIEW_TRUNCATION_NOTICE)); -} - #[test] fn exec_command_tool_output_formats_truncated_response() { let payload = ToolPayload::Function { arguments: "{}".to_string(), }; - let response = ExecCommandToolOutput { + let output = ExecCommandToolOutput { event_call_id: "call-42".to_string(), chunk_id: "abc123".to_string(), wall_time: std::time::Duration::from_millis(1250), @@ -440,8 +415,12 @@ fn exec_command_tool_output_formats_truncated_response() { original_token_count: Some(10), output_omitted_bytes: None, hook_command: None, - } - .to_response_item("call-42", &payload); + }; + assert_eq!( + output.log_output(), + "Chunk ID: abc123\nWall time: 1.2500 seconds\nProcess exited with code 0\nOriginal token count: 10\nOutput:\ntoken one token two token three token four token five" + ); + let response = output.to_response_item("call-42", &payload); match response { ResponseInputItem::FunctionCallOutput { call_id, output } => { @@ -525,7 +504,7 @@ fn exec_command_tool_output_preserves_omission_metadata_when_truncated() { "z".repeat(/*n*/ 100) ) .into_bytes(); - let response = ExecCommandToolOutput { + let mut output = ExecCommandToolOutput { event_call_id: "call-omitted".to_string(), chunk_id: "abc123".to_string(), wall_time: std::time::Duration::from_millis(/*millis*/ 1250), @@ -537,8 +516,23 @@ fn exec_command_tool_output_preserves_omission_metadata_when_truncated() { original_token_count: Some(42_000), output_omitted_bytes: NonZeroUsize::new(/*n*/ 123_456), hook_command: None, - } - .to_response_item("call-omitted", &payload); + }; + let expected_header = "Chunk ID: abc123\nWall time: 1.2500 seconds\nProcess exited with code 0\nOriginal token count: 42000\nOutput:\n"; + assert_eq!( + output.log_output(), + format!( + "{expected_header}{}", + String::from_utf8_lossy(&output.raw_output) + ) + ); + let response = output.to_response_item("call-omitted", &payload); + + // Collection may report omitted bytes without including the marker in its text. + output.raw_output = b"remaining output".to_vec(); + assert_eq!( + output.log_output(), + format!("{expected_header}{marker}\nremaining output") + ); let ResponseInputItem::FunctionCallOutput { output, .. } = response else { panic!("expected FunctionCallOutput"); diff --git a/codex-rs/core/src/tools/handlers/current_time.rs b/codex-rs/core/src/tools/handlers/current_time.rs index 983e33e23d..781f5b0731 100644 --- a/codex-rs/core/src/tools/handlers/current_time.rs +++ b/codex-rs/core/src/tools/handlers/current_time.rs @@ -25,7 +25,7 @@ const TOOL_NAME: &str = "curr_time"; struct CurrentTimeOutput(CurrentTimeReminder); impl ToolOutput for CurrentTimeOutput { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { self.0.body() } diff --git a/codex-rs/core/src/tools/handlers/get_context_remaining.rs b/codex-rs/core/src/tools/handlers/get_context_remaining.rs index 77f0429722..1bc4c33b34 100644 --- a/codex-rs/core/src/tools/handlers/get_context_remaining.rs +++ b/codex-rs/core/src/tools/handlers/get_context_remaining.rs @@ -36,7 +36,7 @@ impl GetContextRemainingOutput { } impl ToolOutput for GetContextRemainingOutput { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { self.fragment() } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs index 2ce3dd0c67..cdaf4233bb 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs @@ -141,7 +141,7 @@ pub(crate) struct CloseAgentResult { } impl ToolOutput for CloseAgentResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "close_agent") } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs index 8e97b2afbd..7f5b2e9c17 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs @@ -168,7 +168,7 @@ pub(crate) struct ResumeAgentResult { } impl ToolOutput for ResumeAgentResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "resume_agent") } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs b/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs index 87e4aba5ed..c5527d19c1 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs @@ -145,7 +145,7 @@ pub(crate) struct SendInputResult { } impl ToolOutput for SendInputResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "send_input") } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index 73443cdf42..d7a69f5a89 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -245,7 +245,7 @@ pub(crate) struct SpawnAgentResult { } impl ToolOutput for SpawnAgentResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "spawn_agent") } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/wait.rs b/codex-rs/core/src/tools/handlers/multi_agents/wait.rs index 1ccad426eb..84ebc35c1e 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/wait.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/wait.rs @@ -284,7 +284,7 @@ pub(crate) struct WaitAgentResult { } impl ToolOutput for WaitAgentResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "wait_agent") } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/interrupt_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/interrupt_agent.rs index a418dec4a8..7c22fc21eb 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/interrupt_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/interrupt_agent.rs @@ -113,7 +113,7 @@ pub(crate) struct InterruptAgentResult { } impl ToolOutput for InterruptAgentResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "interrupt_agent") } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs index 99e54e1ce9..d1348d804d 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs @@ -65,7 +65,7 @@ pub(crate) struct ListAgentsResult { } impl ToolOutput for ListAgentsResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "list_agents") } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 9d93d8d35f..5cbaa165d4 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -330,7 +330,7 @@ pub(crate) enum SpawnAgentResult { } impl ToolOutput for SpawnAgentResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "spawn_agent") } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs index 6a23e8012b..14715ad326 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs @@ -157,7 +157,7 @@ impl WaitAgentResult { } impl ToolOutput for WaitAgentResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { tool_output_json_text(self, "wait_agent") } diff --git a/codex-rs/core/src/tools/handlers/plan.rs b/codex-rs/core/src/tools/handlers/plan.rs index d962314752..2c21334a3f 100644 --- a/codex-rs/core/src/tools/handlers/plan.rs +++ b/codex-rs/core/src/tools/handlers/plan.rs @@ -22,7 +22,7 @@ pub struct PlanToolOutput; const PLAN_UPDATED_MESSAGE: &str = "Plan updated"; impl ToolOutput for PlanToolOutput { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { PLAN_UPDATED_MESSAGE.to_string() } diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 96152b20da..22ed341590 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -223,7 +223,7 @@ pub struct ViewImageOutput { } impl ToolOutput for ViewImageOutput { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { format!("", self.image_url.len()) } @@ -323,7 +323,7 @@ mod tests { unified_image_budget: false, }; - assert_eq!(output.log_preview(), ""); + assert_eq!(output.log_output(), ""); } #[test] diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 5c29628079..df6b5969a3 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -33,12 +33,6 @@ use codex_utils_output_truncation::truncate_text; pub(crate) use executed_tool_calls::ExecutedToolCallRecorder; pub use router::ToolRouter; -// Telemetry preview limits: keep log events smaller than model budgets. -pub(crate) const TELEMETRY_PREVIEW_MAX_BYTES: usize = 2 * 1024; // 2 KiB -pub(crate) const TELEMETRY_PREVIEW_MAX_LINES: usize = 64; // lines -pub(crate) const TELEMETRY_PREVIEW_TRUNCATION_NOTICE: &str = - "[... telemetry preview truncated ...]"; - /// Legacy boundaries such as hook payloads, telemetry tags, and Responses tool /// names still require a single flattened string. Keep comparisons and sorting /// on `ToolName` itself; use this only when crossing those boundaries. diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 203dd09efb..e776ae059e 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -49,6 +49,8 @@ const ABANDONED_NETWORK_APPROVAL_MESSAGE: &str = pub(crate) struct NetworkApprovalSpec { pub network: Option, pub trigger: GuardianNetworkAccessTrigger, + /// Preserve the typed identity independently of Guardian's display-name payload. + pub tool_name: ToolName, pub command: String, pub environment_id: String, pub permission_profile: PermissionProfile, @@ -229,6 +231,7 @@ struct ActiveNetworkApprovalCall { registration_id: String, turn_id: String, trigger: GuardianNetworkAccessTrigger, + tool_name: ToolName, command: String, environment_id: String, permission_profile: PermissionProfile, @@ -730,8 +733,8 @@ impl NetworkApprovalService { |call| call.trigger.call_id.clone(), ); let telemetry_tool_name = owner_call.as_ref().map_or_else( - || "network_access".to_string(), - |call| call.trigger.tool_name.clone(), + || ToolName::plain("network_access"), + |call| call.tool_name.clone(), ); let action = ApprovalAction::NetworkAccess { id: guardian_approval_id, @@ -750,7 +753,7 @@ impl NetworkApprovalService { let approval_context = ApprovalContext { review_context: GuardianReviewContext::from(&turn_context), call_id: approval_call_id, - tool_name: ToolName::plain(telemetry_tool_name.clone()), + tool_name: telemetry_tool_name.clone(), strict_auto_review, approval_reason: Some(prompt_reason), retry_reason: Some(policy_denial_message.clone()), @@ -1026,6 +1029,7 @@ pub(crate) async fn begin_network_approval( let NetworkApprovalSpec { network, trigger, + tool_name, command, environment_id, permission_profile, @@ -1057,6 +1061,7 @@ pub(crate) async fn begin_network_approval( registration_id: registration_id.clone(), turn_id: turn_id.to_string(), trigger, + tool_name, command, environment_id, permission_profile, diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index dceaac5b0a..95ac30a406 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -478,6 +478,7 @@ async fn register_call_with_default_shell_trigger( .register_call(ActiveNetworkApprovalCall { registration_id: registration_id.to_string(), turn_id: "turn-1".to_string(), + tool_name: ToolName::plain("exec_command"), trigger: GuardianNetworkAccessTrigger { call_id: "call-1".to_string(), tool_name: "exec_command".to_string(), @@ -500,9 +501,10 @@ async fn register_call_with_default_shell_trigger( #[tokio::test] async fn active_call_preserves_triggering_command_context() { let service = NetworkApprovalService::default(); + let tool_name = ToolName::namespaced("mcp__example", "exec_command"); let expected = GuardianNetworkAccessTrigger { call_id: "call-1".to_string(), - tool_name: "exec_command".to_string(), + tool_name: tool_name.to_string(), command: vec!["curl".to_string(), "https://example.com".to_string()], cwd: PathUri::parse("file:///C:/repo").expect("valid Windows path URI"), sandbox_permissions: SandboxPermissions::UseDefault, @@ -516,6 +518,7 @@ async fn active_call_preserves_triggering_command_context() { registration_id: "registration-1".to_string(), turn_id: "turn-1".to_string(), trigger: expected.clone(), + tool_name: tool_name.clone(), command: "curl https://example.com".to_string(), environment_id: "remote".to_string(), permission_profile: PermissionProfile::workspace_write(), @@ -529,6 +532,7 @@ async fn active_call_preserves_triggering_command_context() { .expect("single active call should resolve"); assert_eq!(&call.trigger, &expected); + assert_eq!(call.tool_name, tool_name); assert_eq!(call.command, "curl https://example.com"); assert_eq!(call.environment_id, "remote"); } diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 0a57d7a514..fe5272fe5d 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -178,7 +178,7 @@ impl ToolOrchestrator { already_approved = true; } else { otel.tool_decision( - &otel_tn, + &tool_ctx.tool_name, otel_ci, &ReviewDecision::Approved, Some(ToolDecisionSource::Config), diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 5ae1d9d054..eeba3b9455 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -215,8 +215,8 @@ struct PostToolUseFeedbackOutput { } impl ToolOutput for PostToolUseFeedbackOutput { - fn log_preview(&self) -> String { - self.original.log_preview() + fn log_output(&self) -> String { + self.original.log_output() } fn success_for_logging(&self) -> bool { @@ -482,7 +482,6 @@ impl ToolRegistry { terminal_outcome_reached: Option>, ) -> Result { let tool_name = invocation.tool_name.clone(); - let tool_name_flat = flat_tool_name(&tool_name); let call_id_owned = invocation.call_id.clone(); let otel = invocation.turn.session_telemetry.clone(); let permission_profile = invocation.turn.permission_profile(); @@ -520,7 +519,7 @@ impl ToolRegistry { let message = unsupported_tool_call_message(&invocation.payload, &tool_name); let log_payload = tool_log_payload(&invocation.payload, &invocation.source); otel.tool_result_with_tags( - tool_name_flat.as_ref(), + &tool_name, &call_id_owned, log_payload.as_ref(), Duration::ZERO, @@ -550,7 +549,7 @@ impl ToolRegistry { let message = format!("tool {tool_name} invoked with incompatible payload"); let log_payload = tool_log_payload(&invocation.payload, &invocation.source); otel.tool_result_with_tags( - tool_name_flat.as_ref(), + &tool_name, &call_id_owned, log_payload.as_ref(), Duration::ZERO, @@ -639,37 +638,26 @@ impl ToolRegistry { tool_result_tags.push(("command_category", category)); } - let response_cell = tokio::sync::Mutex::new(None); - let invocation_for_tool = invocation.clone(); let log_payload = tool_log_payload(&invocation.payload, &invocation.source); let result = otel .log_tool_result_with_tags( - tool_name_flat.as_ref(), + &tool_name, &call_id_owned, log_payload.as_ref(), &tool_result_tags, &extra_trace_fields, - || { - let tool = tool.clone(); - let response_cell = &response_cell; - async move { - match handle_any_tool(tool.as_ref(), invocation_for_tool).await { - Ok(result) => { - let preview = result.result.log_preview(); - let success = result.result.success_for_logging(); - let mut guard = response_cell.lock().await; - *guard = Some(result); - Ok((preview, success)) - } - Err(err) => Err(err), - } - } + || handle_any_tool(tool.as_ref(), invocation.clone()), + |result| { + ( + result.result.log_output(), + result.result.success_for_logging(), + ) }, ) .await; let success = match &result { - Ok((_, success)) => *success, + Ok(result) => result.result.success_for_logging(), Err(_) => false, }; if let Some(analytics) = control_tool_analytics.as_mut() { @@ -681,9 +669,9 @@ impl ToolRegistry { } emit_metric_for_tool_read(&invocation, success); let post_tool_use_payload = if success { - let guard = response_cell.lock().await; - guard + result .as_ref() + .ok() .and_then(|result| result.post_tool_use_payload.clone()) } else { None @@ -715,17 +703,7 @@ impl ToolRegistry { // A PostToolUse block rejects the result, not the already-completed tool execution. let lifecycle_outcome = match &result { - Ok(_) => { - let guard = response_cell.lock().await; - match guard.as_ref() { - Some(result) => ToolCallOutcome::Completed { - success: result.result.success_for_logging(), - }, - None => ToolCallOutcome::Failed { - handler_executed: true, - }, - } - } + Ok(_) => ToolCallOutcome::Completed { success }, Err(_) => ToolCallOutcome::Failed { handler_executed: true, }, @@ -738,11 +716,7 @@ impl ToolRegistry { .await; match result { - Ok(_) => { - let mut guard = response_cell.lock().await; - let mut result = guard.take().ok_or_else(|| { - FunctionCallError::Fatal("tool produced no output".to_string()) - })?; + Ok(mut result) => { if let Some(outcome) = post_tool_use_outcome { if outcome.should_block { let message = outcome.feedback_message.unwrap_or_else(|| { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 25a21b7990..eae1f0ba22 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -225,6 +225,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt managed_network_for_sandbox_permissions(req.network.as_ref(), sandbox_permissions)?; Some(NetworkApprovalSpec { network: Some(network.clone()), + tool_name: ctx.tool_name.clone(), trigger: GuardianNetworkAccessTrigger { call_id: ctx.call_id.clone(), tool_name: flat_tool_name(&ctx.tool_name).into_owned(), diff --git a/codex-rs/core/tests/suite/otel.rs b/codex-rs/core/tests/suite/otel.rs index b3489240d1..18b7bdfeca 100644 --- a/codex-rs/core/tests/suite/otel.rs +++ b/codex-rs/core/tests/suite/otel.rs @@ -4,6 +4,7 @@ use codex_features::Feature; use codex_otel::SessionTelemetry; use codex_otel::TelemetryAuthMode; use codex_protocol::ThreadId; +use codex_protocol::ToolName; use codex_protocol::approvals::NetworkPolicyAmendment; use codex_protocol::approvals::NetworkPolicyRuleAction; use codex_protocol::config_types::ServiceTier; @@ -1080,6 +1081,9 @@ fn tool_decision_assertion<'a>( if !lower.contains("tool_name=exec_command") { return Err("missing tool_name for exec_command".to_string()); } + if !lower.contains("tool_namespace=functions") { + return Err("missing default tool namespace".to_string()); + } if !lower.contains(&format!("decision={expected_decision}")) { return Err(format!("unexpected decision for {call_id}")); } @@ -1154,7 +1158,7 @@ fn network_policy_decisions_omit_source_and_destination() { ), ] { telemetry.tool_decision( - "exec_command", + &ToolName::namespaced("mcp__example", "exec_command"), call_id, &ReviewDecision::NetworkPolicyAmendment { network_policy_amendment: NetworkPolicyAmendment { @@ -1177,6 +1181,9 @@ fn network_policy_decisions_omit_source_and_destination() { if !line.contains("tool_name=exec_command") { return Err("missing triggering network tool name".to_string()); } + if !line.contains("tool_namespace=mcp__example") { + return Err("missing triggering network tool namespace".to_string()); + } if !line.contains(&format!("decision={expected_decision}")) { return Err(format!("unexpected network tool decision for {call_id}")); } diff --git a/codex-rs/ext/image-generation/src/tool.rs b/codex-rs/ext/image-generation/src/tool.rs index 1c916fc4d4..b4f53fbc97 100644 --- a/codex-rs/ext/image-generation/src/tool.rs +++ b/codex-rs/ext/image-generation/src/tool.rs @@ -641,7 +641,7 @@ struct GeneratedImageOutput { impl ToolOutput for GeneratedImageOutput { /// Avoids copying image bytes into tool-call telemetry. - fn log_preview(&self) -> String { + fn log_output(&self) -> String { "[generated image]".to_string() } diff --git a/codex-rs/ext/web-search/src/output.rs b/codex-rs/ext/web-search/src/output.rs index 543afdf7a9..bf305fc4ea 100644 --- a/codex-rs/ext/web-search/src/output.rs +++ b/codex-rs/ext/web-search/src/output.rs @@ -15,7 +15,7 @@ impl SearchOutput { } impl ToolOutput for SearchOutput { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { "[standalone web search output]".to_string() } diff --git a/codex-rs/otel/src/events/session_telemetry.rs b/codex-rs/otel/src/events/session_telemetry.rs index 2ffa061308..a91f18ae9d 100644 --- a/codex-rs/otel/src/events/session_telemetry.rs +++ b/codex-rs/otel/src/events/session_telemetry.rs @@ -32,11 +32,16 @@ use crate::metrics::runtime_metrics::RuntimeMetricsSummary; use crate::metrics::timer::Timer; use crate::provider::OtelProvider; use crate::sanitize_metric_tag_value; +use crate::tool_result::ToolResultEvent; +use crate::tool_result::emit_tool_result; use codex_api::AgentIdentityTelemetry; use codex_api::ApiError; use codex_api::ResponseEvent; +use codex_protocol::AgentPath; use codex_protocol::ThreadId; +use codex_protocol::ToolName; use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::ToolResultLogConfig; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; @@ -50,7 +55,6 @@ use eventsource_stream::EventStreamError as StreamError; use opentelemetry_sdk::metrics::data::ResourceMetrics; use reqwest::Error; use reqwest::Response; -use std::borrow::Cow; use std::future::Future; use std::time::Duration; use std::time::Instant; @@ -87,6 +91,9 @@ pub struct AuthEnvTelemetryMetadata { #[derive(Debug, Clone)] pub struct SessionTelemetryMetadata { pub(crate) conversation_id: ThreadId, + // Full canonical path when available; legacy nickname or thread ID otherwise. + // Keep this potentially user-chosen label on detailed logs, not trace-safe events. + pub(crate) agent_name: String, pub(crate) auth_mode: Option, pub(crate) auth_env: AuthEnvTelemetryMetadata, pub(crate) account_id: Option, @@ -105,12 +112,18 @@ pub struct SessionTelemetryMetadata { #[derive(Debug, Clone)] pub struct SessionTelemetry { + tool_result_log_config: ToolResultLogConfig, pub(crate) metadata: SessionTelemetryMetadata, pub(crate) metrics: Option, pub(crate) metrics_use_metadata_tags: bool, } impl SessionTelemetry { + pub fn with_tool_result_log_config(mut self, config: ToolResultLogConfig) -> Self { + self.tool_result_log_config = config; + self + } + pub fn with_auth_env(mut self, auth_env: AuthEnvTelemetryMetadata) -> Self { self.metadata.auth_env = auth_env; self @@ -425,9 +438,22 @@ impl SessionTelemetry { terminal_type: String, session_source: SessionSource, ) -> SessionTelemetry { + let agent_name = session_source + .get_agent_path() + .map(|path| path.to_string()) + .or_else(|| session_source.get_nickname()) + .unwrap_or_else(|| { + if session_source.is_non_root_agent() { + conversation_id.to_string() + } else { + AgentPath::ROOT.to_string() + } + }); Self { + tool_result_log_config: ToolResultLogConfig::default(), metadata: SessionTelemetryMetadata { conversation_id, + agent_name, auth_mode: auth_mode.map(|m| m.to_string()), auth_env: AuthEnvTelemetryMetadata::default(), account_id, @@ -1008,16 +1034,18 @@ impl SessionTelemetry { pub fn tool_decision( &self, - tool_name: &str, + tool_name: &ToolName, call_id: &str, decision: &ReviewDecision, source: Option, ) { + let tool_namespace = crate::events::shared::tool_namespace(tool_name); match source { Some(source) => log_event!( self, event.name = "codex.tool_decision", - tool_name = %tool_name, + tool_name = %tool_name.name, + tool_namespace = %tool_namespace, call_id = %call_id, decision = %decision.to_opaque_string(), source = %source.to_string(), @@ -1025,7 +1053,8 @@ impl SessionTelemetry { None => log_event!( self, event.name = "codex.tool_decision", - tool_name = %tool_name, + tool_name = %tool_name.name, + tool_namespace = %tool_namespace, call_id = %call_id, decision = %decision.to_opaque_string(), ), @@ -1063,19 +1092,22 @@ impl SessionTelemetry { ); } + /// Records a diagnostic representation and returns the original result. The temporary + /// output string is truncated and released before the caller resumes async work. #[allow(clippy::too_many_arguments)] - pub async fn log_tool_result_with_tags( + pub async fn log_tool_result_with_tags( &self, - tool_name: &str, + tool_name: &ToolName, call_id: &str, arguments: &str, extra_tags: &[(&str, &str)], extra_trace_fields: &[(&str, &str)], f: F, - ) -> Result<(String, bool), E> + log_output: impl FnOnce(&T) -> (String, bool), + ) -> Result where F: FnOnce() -> Fut, - Fut: Future>, + Fut: Future>, E: std::fmt::Display, { let start = Instant::now(); @@ -1083,8 +1115,8 @@ impl SessionTelemetry { let duration = start.elapsed(); let (output, success) = match &result { - Ok((preview, success)) => (Cow::Borrowed(preview.as_str()), *success), - Err(error) => (Cow::Owned(error.to_string()), false), + Ok(result) => log_output(result), + Err(error) => (error.to_string(), false), }; self.tool_result_with_tags( @@ -1093,7 +1125,7 @@ impl SessionTelemetry { arguments, duration, success, - output.as_ref(), + &output, extra_tags, extra_trace_fields, ); @@ -1101,34 +1133,10 @@ impl SessionTelemetry { result } - pub fn log_tool_failed(&self, tool_name: &str, error: &str) { - log_event!( - self, - event.name = "codex.tool_result", - tool_name = %tool_name, - duration_ms = %Duration::ZERO.as_millis(), - success = %false, - output = %error, - mcp_server = "", - mcp_server_origin = "", - ); - trace_event!( - self, - event.name = "codex.tool_result", - tool_name = %tool_name, - duration_ms = %Duration::ZERO.as_millis(), - success = %false, - output_length = error.len() as i64, - output_line_count = error.lines().count() as i64, - tool_origin = %"builtin", - error.message = %error, - ); - } - #[allow(clippy::too_many_arguments)] pub fn tool_result_with_tags( &self, - tool_name: &str, + tool_name: &ToolName, call_id: &str, arguments: &str, duration: Duration, @@ -1137,9 +1145,10 @@ impl SessionTelemetry { extra_tags: &[(&str, &str)], extra_trace_fields: &[(&str, &str)], ) { + let flat_tool_name = tool_name.to_string(); let success_str = if success { "true" } else { "false" }; let mut tags = Vec::with_capacity(2 + extra_tags.len()); - tags.push(("tool", tool_name)); + tags.push(("tool", flat_tool_name.as_str())); tags.push(("success", success_str)); tags.extend_from_slice(extra_tags); self.counter(TOOL_CALL_COUNT_METRIC, /*inc*/ 1, &tags); @@ -1147,30 +1156,19 @@ impl SessionTelemetry { let mcp_server = trace_field_value(extra_trace_fields, "mcp_server").unwrap_or(""); let mcp_server_origin = trace_field_value(extra_trace_fields, "mcp_server_origin").unwrap_or(""); - log_event!( + emit_tool_result( self, - event.name = "codex.tool_result", - tool_name = %tool_name, - call_id = %call_id, - arguments = %arguments, - duration_ms = %duration.as_millis(), - success = %success_str, - output = %output, - mcp_server = %mcp_server, - mcp_server_origin = %mcp_server_origin, - ); - trace_event!( - self, - event.name = "codex.tool_result", - tool_name = %tool_name, - call_id = %call_id, - duration_ms = %duration.as_millis(), - success = %success_str, - arguments_length = arguments.len() as i64, - output_length = output.len() as i64, - output_line_count = output.lines().count() as i64, - tool_origin = if mcp_server.is_empty() { "builtin" } else { "mcp" }, - mcp_tool = !mcp_server.is_empty(), + self.tool_result_log_config, + ToolResultEvent { + tool_name, + call_id, + arguments, + mcp_server, + mcp_server_origin, + duration, + success, + output, + }, ); } diff --git a/codex-rs/otel/src/events/shared.rs b/codex-rs/otel/src/events/shared.rs index 08e369316f..f962e828c4 100644 --- a/codex-rs/otel/src/events/shared.rs +++ b/codex-rs/otel/src/events/shared.rs @@ -1,5 +1,15 @@ use chrono::SecondsFormat; use chrono::Utc; +use codex_protocol::DEFAULT_FUNCTION_NAMESPACE; +use codex_protocol::ToolName; + +pub(crate) fn tool_namespace(tool_name: &ToolName) -> &str { + tool_name + .namespace + .as_deref() + .filter(|namespace| !namespace.is_empty()) + .unwrap_or(DEFAULT_FUNCTION_NAMESPACE) +} macro_rules! log_event { ($self:expr, $($fields:tt)*) => {{ diff --git a/codex-rs/otel/src/lib.rs b/codex-rs/otel/src/lib.rs index 65b034cc31..e75167d021 100644 --- a/codex-rs/otel/src/lib.rs +++ b/codex-rs/otel/src/lib.rs @@ -6,6 +6,7 @@ pub(crate) mod trace_context; mod otlp; mod targets; +mod tool_result; use crate::metrics::Result as MetricsResult; use codex_protocol::auth::AuthMode; diff --git a/codex-rs/otel/src/tool_result.rs b/codex-rs/otel/src/tool_result.rs new file mode 100644 index 0000000000..a7567a3b1e --- /dev/null +++ b/codex-rs/otel/src/tool_result.rs @@ -0,0 +1,114 @@ +use crate::SessionTelemetry; +use crate::events::shared::log_and_trace_event; +use crate::events::shared::log_event; +use crate::events::shared::tool_namespace; +use crate::events::shared::trace_event; +use codex_protocol::ToolName; +use codex_protocol::config_types::ToolResultLogConfig; +use codex_utils_string::take_bytes_at_char_boundary; +use std::borrow::Cow; +use std::sync::atomic::AtomicU64; +use std::sync::atomic::Ordering; +use std::time::Duration; + +const TRUNCATION_NOTICE: &str = "[... telemetry preview truncated ...]"; + +/// Result-recording order across all sessions in this process, not tool-start order. +/// Independent of the optional rollout-trace writer and reset on process restart. +fn next_tool_result_seq() -> u64 { + static NEXT_SEQ: AtomicU64 = AtomicU64::new(1); + NEXT_SEQ.fetch_add(/*val*/ 1, Ordering::Relaxed) +} + +pub(crate) struct ToolResultEvent<'a> { + pub tool_name: &'a ToolName, + pub call_id: &'a str, + pub arguments: &'a str, + pub mcp_server: &'a str, + pub mcp_server_origin: &'a str, + pub duration: Duration, + pub success: bool, + pub output: &'a str, +} + +/// Emits diagnostic logs and trace-safe metadata without changing tool-call metrics. +pub(crate) fn emit_tool_result( + telemetry: &SessionTelemetry, + limits: ToolResultLogConfig, + event: ToolResultEvent<'_>, +) { + let ToolResultEvent { + tool_name, + call_id, + arguments, + mcp_server, + mcp_server_origin, + duration, + success, + output, + } = event; + let tool_namespace = tool_namespace(tool_name); + let preview = telemetry_preview(output, limits); + let tool_result_seq = next_tool_result_seq(); + + log_and_trace_event!( + telemetry, + common: { + event.name = "codex.tool_result", + tool_result_seq = tool_result_seq, + tool_name = %tool_name.name, + tool_namespace = %tool_namespace, + call_id = %call_id, + duration_ms = %duration.as_millis(), + success = %success, + output_truncated = preview.truncated, + }, + log: { + agent_name = %telemetry.metadata.agent_name, + arguments = %arguments, + output = %preview.text, + mcp_server = %mcp_server, + mcp_server_origin = %mcp_server_origin, + }, + trace: { + arguments_length = arguments.len() as i64, + output_length = output.len() as i64, + output_line_count = output.lines().count() as i64, + tool_origin = if mcp_server.is_empty() { "builtin" } else { "mcp" }, + mcp_tool = !mcp_server.is_empty(), + }, + ); +} + +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct ToolResultPreview<'a> { + pub text: Cow<'a, str>, + /// Whether this telemetry limit removed text, not whether the tool result was lossless. + pub truncated: bool, +} + +pub(crate) fn telemetry_preview( + content: &str, + limits: ToolResultLogConfig, +) -> ToolResultPreview<'_> { + let prefix = take_bytes_at_char_boundary(content, limits.max_bytes); + if prefix.len() == content.len() { + return ToolResultPreview { + text: Cow::Borrowed(content), + truncated: false, + }; + } + let separator = if prefix.is_empty() || prefix.ends_with('\n') { + "" + } else { + "\n" + }; + ToolResultPreview { + text: Cow::Owned(format!("{prefix}{separator}{TRUNCATION_NOTICE}")), + truncated: true, + } +} + +#[cfg(test)] +#[path = "tool_result_tests.rs"] +mod tests; diff --git a/codex-rs/otel/src/tool_result_tests.rs b/codex-rs/otel/src/tool_result_tests.rs new file mode 100644 index 0000000000..a445b54299 --- /dev/null +++ b/codex-rs/otel/src/tool_result_tests.rs @@ -0,0 +1,67 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn telemetry_preview_returns_original_within_limits() { + // A literal marker in the tool's own output must not imply telemetry truncation. + for content in ["", "first\r\nsecond\r\n", TRUNCATION_NOTICE] { + let preview = telemetry_preview(content, ToolResultLogConfig::default()); + assert!(matches!(preview.text, Cow::Borrowed(_))); + assert_eq!( + preview, + ToolResultPreview { + text: Cow::Borrowed(content), + truncated: false + } + ); + } +} + +#[test] +fn telemetry_preview_truncates_by_bytes() { + let limits = ToolResultLogConfig::default(); + let content = "x".repeat(limits.max_bytes + 8); + let preview = telemetry_preview(&content, limits); + assert!(preview.truncated); + assert!(preview.text.contains(TRUNCATION_NOTICE)); + assert!(preview.text.len() <= limits.max_bytes + TRUNCATION_NOTICE.len() + 1); + assert_eq!( + telemetry_preview( + &content, + ToolResultLogConfig { + max_bytes: content.len(), + } + ), + ToolResultPreview { + text: Cow::Borrowed(&content), + truncated: false + } + ); + assert_eq!( + telemetry_preview("éé", ToolResultLogConfig { max_bytes: 3 }), + ToolResultPreview { + text: Cow::Owned(format!("é\n{TRUNCATION_NOTICE}")), + truncated: true + } + ); + for (content, max_bytes, expected, truncated) in [ + ("", 0, "".to_string(), false), + ("\nsecret", 0, TRUNCATION_NOTICE.to_string(), true), + ( + "first\r\nsecond", + 7, + format!("first\r\n{TRUNCATION_NOTICE}"), + true, + ), + ("first\r\n", 7, "first\r\n".to_string(), false), + ("\n\nsecret", 1, format!("\n{TRUNCATION_NOTICE}"), true), + ] { + assert_eq!( + telemetry_preview(content, ToolResultLogConfig { max_bytes }), + ToolResultPreview { + text: Cow::Owned(expected), + truncated + } + ); + } +} 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 17b6f764db..7605fab73e 100644 --- a/codex-rs/otel/tests/suite/otel_export_routing_policy.rs +++ b/codex-rs/otel/tests/suite/otel_export_routing_policy.rs @@ -19,11 +19,15 @@ use tracing_subscriber::Layer; use tracing_subscriber::filter::filter_fn; use tracing_subscriber::layer::SubscriberExt; +use codex_protocol::AgentPath; use codex_protocol::ThreadId; +use codex_protocol::ToolName; use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::ToolResultLogConfig; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; +use codex_protocol::protocol::SubAgentSource; use codex_protocol::user_input::UserInput; fn log_attributes(record: &SdkLogRecord) -> BTreeMap { @@ -205,6 +209,25 @@ fn otel_export_routing_policy_routes_user_prompt_log_and_trace_events() { #[test] fn otel_export_routing_policy_routes_tool_result_log_and_trace_events() { + let output = "secret output\nsecond line\n".repeat(100); + let root_id = ThreadId::new(); + let child_id = ThreadId::new(); + let legacy_id = ThreadId::new(); + let unnamed_id = ThreadId::new(); + let make_manager = |thread_id, source| { + SessionTelemetry::new( + thread_id, + "gpt-5.1", + "gpt-5.1", + Some("account-id".to_string()), + Some("engineer@example.com".to_string()), + Some(TelemetryAuthMode::ApiKey), + "codex_exec".to_string(), + /*log_user_prompts*/ true, + "tty".to_string(), + source, + ) + }; let log_exporter = InMemoryLogExporter::default(); let logger_provider = SdkLoggerProvider::builder() .with_simple_exporter(log_exporter.clone()) @@ -230,33 +253,72 @@ fn otel_export_routing_policy_routes_tool_result_log_and_trace_events() { tracing::subscriber::with_default(subscriber, || { tracing::callsite::rebuild_interest_cache(); - let manager = SessionTelemetry::new( - ThreadId::new(), - "gpt-5.1", - "gpt-5.1", - Some("account-id".to_string()), - Some("engineer@example.com".to_string()), - Some(TelemetryAuthMode::ApiKey), - "codex_exec".to_string(), - /*log_user_prompts*/ true, - "tty".to_string(), - SessionSource::Cli, - ); + let manager = make_manager(root_id, SessionSource::Cli); let root_span = tracing::info_span!("root"); let _root_guard = root_span.enter(); + let manager = manager.with_tool_result_log_config(ToolResultLogConfig { + max_bytes: output.len(), + }); manager.tool_result_with_tags( - "shell", + &ToolName::namespaced("mcp__example", "shell"), "call-1", "secret arguments", std::time::Duration::from_millis(42), /*success*/ true, - "secret output\nsecond line", + &output, &[], &[ ("mcp_server", "internal-mcp"), ("mcp_server_origin", "stdio"), ], ); + let child = make_manager( + child_id, + SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: root_id, + depth: 1, + agent_path: Some(AgentPath::try_from("/root/reviewer").expect("agent path")), + agent_nickname: Some("legacy nickname".to_string()), + agent_role: None, + }), + ); + child.tool_result_with_tags( + &ToolName::plain("shell"), + "call-2", + "{}", + std::time::Duration::ZERO, + /*success*/ true, + &output, + &[], + &[], + ); + let legacy = make_manager( + legacy_id, + SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: root_id, + depth: 1, + agent_path: None, + agent_nickname: Some("Mendel".to_string()), + agent_role: None, + }), + ); + let unnamed = make_manager(unnamed_id, SessionSource::SubAgent(SubAgentSource::Review)); + for (manager, name, error) in [ + (&manager, "root_failure", "failure"), + (&legacy, "legacy_failure", output.as_str()), + (&unnamed, "unnamed_failure", "failure"), + ] { + manager.tool_result_with_tags( + &ToolName::plain(name), + name, + "{}", + std::time::Duration::ZERO, + /*success*/ false, + error, + &[], + &[], + ); + } }); logger_provider.force_flush().expect("flush logs"); @@ -270,14 +332,71 @@ fn otel_export_routing_policy_routes_tool_result_log_and_trace_events() { let tool_log = find_log_by_event_name(&logs, "codex.tool_result"); let tool_log_attrs = log_attributes(&tool_log.record); + assert_eq!( + tool_log_attrs.get("tool_name").map(String::as_str), + Some("shell") + ); + assert_eq!( + tool_log_attrs.get("tool_namespace").map(String::as_str), + Some("mcp__example") + ); assert_eq!( tool_log_attrs.get("arguments").map(String::as_str), Some("secret arguments") ); assert_eq!( tool_log_attrs.get("output").map(String::as_str), - Some("secret output\nsecond line") + Some(output.as_str()) ); + let tool_logs: Vec<_> = logs + .iter() + .map(|log| log_attributes(&log.record)) + .filter(|attrs| attrs.get("event.name").map(String::as_str) == Some("codex.tool_result")) + .collect(); + assert_eq!( + tool_logs + .iter() + .map(|attrs| ( + attrs["conversation.id"].clone(), + attrs["agent_name"].clone(), + attrs["output_truncated"].clone(), + )) + .collect::>(), + vec![ + ( + root_id.to_string(), + "/root".to_string(), + "false".to_string() + ), + ( + child_id.to_string(), + "/root/reviewer".to_string(), + "true".to_string() + ), + ( + root_id.to_string(), + "/root".to_string(), + "false".to_string() + ), + ( + legacy_id.to_string(), + "Mendel".to_string(), + "true".to_string() + ), + ( + unnamed_id.to_string(), + unnamed_id.to_string(), + "false".to_string() + ), + ] + ); + assert!(tool_logs[1]["output"].ends_with("[... telemetry preview truncated ...]")); + let sequences: Vec = tool_logs + .iter() + .map(|attrs| attrs["tool_result_seq"].parse().expect("numeric sequence")) + .collect(); + assert!(sequences[0] > 0); + assert!(sequences.windows(2).all(|pair| pair[0] < pair[1])); assert_eq!( tool_log_attrs.get("mcp_server").map(String::as_str), Some("internal-mcp") @@ -290,7 +409,15 @@ fn otel_export_routing_policy_routes_tool_result_log_and_trace_events() { let spans = span_exporter.get_finished_spans().expect("span export"); assert_eq!(spans.len(), 1); let span_events = &spans[0].events.events; - assert_eq!(span_events.len(), 1); + assert_eq!(span_events.len(), tool_logs.len()); + for (event, log) in span_events.iter().zip(&tool_logs) { + let attrs = span_event_attributes(event); + assert_eq!(attrs.get("tool_name"), log.get("tool_name")); + assert_eq!(attrs.get("tool_namespace"), log.get("tool_namespace")); + assert_eq!(attrs.get("tool_result_seq"), log.get("tool_result_seq")); + assert_eq!(attrs.get("output_truncated"), log.get("output_truncated")); + assert!(!attrs.contains_key("agent_name")); + } let tool_trace_event = find_span_event_by_name_attr(span_events, "codex.tool_result"); let tool_trace_attrs = span_event_attributes(tool_trace_event); @@ -300,13 +427,13 @@ fn otel_export_routing_policy_routes_tool_result_log_and_trace_events() { ); assert_eq!( tool_trace_attrs.get("output_length").map(String::as_str), - Some("25") + Some(output.len().to_string().as_str()) ); assert_eq!( tool_trace_attrs .get("output_line_count") .map(String::as_str), - Some("2") + Some(output.lines().count().to_string().as_str()) ); assert!(!tool_trace_attrs.contains_key("arguments")); assert!(!tool_trace_attrs.contains_key("output")); diff --git a/codex-rs/otel/tests/suite/runtime_summary.rs b/codex-rs/otel/tests/suite/runtime_summary.rs index f4914fd98d..18722e278f 100644 --- a/codex-rs/otel/tests/suite/runtime_summary.rs +++ b/codex-rs/otel/tests/suite/runtime_summary.rs @@ -6,6 +6,7 @@ use codex_otel::RuntimeMetricsSummary; use codex_otel::SessionTelemetry; use codex_otel::TelemetryAuthMode; use codex_protocol::ThreadId; +use codex_protocol::ToolName; use codex_protocol::protocol::SessionSource; use eventsource_stream::Event as StreamEvent; use opentelemetry_sdk::metrics::InMemoryMetricExporter; @@ -37,7 +38,7 @@ fn runtime_metrics_summary_collects_tool_api_and_streaming_metrics() -> Result<( manager.reset_runtime_metrics(); manager.tool_result_with_tags( - "shell", + &ToolName::plain("shell"), "call-1", "{\"cmd\":\"echo\"}", Duration::from_millis(250), diff --git a/codex-rs/protocol/src/config_types.rs b/codex-rs/protocol/src/config_types.rs index 55dc633382..6db800f7f6 100644 --- a/codex-rs/protocol/src/config_types.rs +++ b/codex-rs/protocol/src/config_types.rs @@ -21,6 +21,23 @@ use wildmatch::WildMatchPattern; use crate::openai_models::ReasoningEffort; +/// Limit for the text included in `codex.tool_result` log records. +/// This does not affect model-visible output. Raising it can expose more tool data to logs. +#[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, JsonSchema)] +#[serde(default)] +pub struct ToolResultLogConfig { + /// Maximum UTF-8 bytes before the truncation notice. Defaults to 2048. + pub max_bytes: usize, +} + +impl Default for ToolResultLogConfig { + fn default() -> Self { + Self { + max_bytes: 2 * 1024, + } + } +} + /// Selects which part of the active context is charged against /// `model_auto_compact_token_limit`. #[derive( diff --git a/codex-rs/tools/src/tool_output.rs b/codex-rs/tools/src/tool_output.rs index a2e36db9e1..a8cfa569d3 100644 --- a/codex-rs/tools/src/tool_output.rs +++ b/codex-rs/tools/src/tool_output.rs @@ -3,18 +3,17 @@ use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; -use codex_utils_string::take_bytes_at_char_boundary; use serde_json::Value as JsonValue; use crate::ToolPayload; -const TELEMETRY_PREVIEW_MAX_BYTES: usize = 2 * 1024; -const TELEMETRY_PREVIEW_MAX_LINES: usize = 64; -const TELEMETRY_PREVIEW_TRUNCATION_NOTICE: &str = "[... telemetry preview truncated ...]"; - /// Model-facing output contract returned by executable tool runtimes. pub trait ToolOutput: Send { - fn log_preview(&self) -> String; + /// Returns a deliberately lossy diagnostic representation, before telemetry size limits. + /// Implementations may summarize results or omit media and encrypted content. This is not + /// the authoritative tool result; an untruncated log does not imply a complete result. + /// The logger owns the additional configurable byte limit. + fn log_output(&self) -> String; fn success_for_logging(&self) -> bool; @@ -56,8 +55,8 @@ impl ToolOutput for Box where T: ToolOutput + ?Sized, { - fn log_preview(&self) -> String { - (**self).log_preview() + fn log_output(&self) -> String { + (**self).log_output() } fn success_for_logging(&self) -> bool { @@ -120,8 +119,8 @@ impl JsonToolOutput { } impl ToolOutput for JsonToolOutput { - fn log_preview(&self) -> String { - telemetry_preview(&self.value.to_string()) + fn log_output(&self) -> String { + self.value.to_string() } fn success_for_logging(&self) -> bool { @@ -162,10 +161,10 @@ impl ToolOutput for JsonToolOutput { } impl ToolOutput for codex_protocol::mcp::CallToolResult { - fn log_preview(&self) -> String { + fn log_output(&self) -> String { let output = self.as_function_call_output_payload(); - let preview = output.body.to_text().unwrap_or_else(|| output.to_string()); - telemetry_preview(&preview) + // Do not fall back to serializing media or encrypted content into logs. + output.body.to_text().unwrap_or_default() } fn success_for_logging(&self) -> bool { @@ -255,43 +254,3 @@ fn content_items_to_code_mode_result(items: &[FunctionCallOutputContentItem]) -> .join("\n"), ) } - -fn telemetry_preview(content: &str) -> String { - let truncated_slice = take_bytes_at_char_boundary(content, TELEMETRY_PREVIEW_MAX_BYTES); - let truncated_by_bytes = truncated_slice.len() < content.len(); - - let mut preview = String::new(); - let mut lines_iter = truncated_slice.lines(); - for idx in 0..TELEMETRY_PREVIEW_MAX_LINES { - match lines_iter.next() { - Some(line) => { - if idx > 0 { - preview.push('\n'); - } - preview.push_str(line); - } - None => break, - } - } - let truncated_by_lines = lines_iter.next().is_some(); - - if !truncated_by_bytes && !truncated_by_lines { - return content.to_string(); - } - - if preview.len() < truncated_slice.len() - && truncated_slice - .as_bytes() - .get(preview.len()) - .is_some_and(|byte| *byte == b'\n') - { - preview.push('\n'); - } - - if !preview.is_empty() && !preview.ends_with('\n') { - preview.push('\n'); - } - preview.push_str(TELEMETRY_PREVIEW_TRUNCATION_NOTICE); - - preview -}