From c8d2db9cc06f6d972757973a9224eeaefb56c755 Mon Sep 17 00:00:00 2001 From: jif Date: Thu, 9 Jul 2026 14:36:15 +0100 Subject: [PATCH] Reduce MCP tool-list trace volume (#31790) ## Why Every MCP tool-list build emitted two normal-path TRACE events per configured server: one before waiting for tools and one after listing them. On active sessions this produced thousands of nearly identical SQLite rows while carrying little information beyond server readiness and tool counts. ## What changed - Remove the two normal-path per-server TRACE events. - Keep the existing per-server trace span for timing and remote trace context. - Emit per-server details only when a server's tools are unavailable. - Emit one bounded summary per tool-list build with available server, unavailable server, and tool counts. Successful builds retain the useful aggregate signal without repeating it for every server. Related to #28224. --- codex-rs/codex-mcp/src/connection_manager.rs | 30 ++++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 612b2ed442..c21a9f1d10 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -504,18 +504,14 @@ impl McpConnectionManager { #[instrument(level = "trace", skip_all, fields(mcp_server_count = self.clients.len()))] pub async fn list_all_tools(&self) -> Vec { let mut tools = Vec::new(); + let mut available_server_count = 0; + let mut unavailable_server_count = 0; for (server_name, managed_client) in &self.clients { managed_client.reconnect_failed_startup().await; let has_cached_tools = managed_client.has_cached_tools(); let startup_complete = managed_client .startup_complete .load(std::sync::atomic::Ordering::Acquire); - trace!( - server_name = %server_name, - has_cached_tools, - startup_complete, - "waiting for MCP server tools while building tool list" - ); let Some(server_tools) = managed_client .listed_tools() .instrument(trace_span!( @@ -526,20 +522,30 @@ impl McpConnectionManager { )) .await else { + unavailable_server_count += 1; + trace!( + server_name = %server_name, + has_cached_tools, + startup_complete, + "MCP server tools unavailable while building tool list" + ); continue; }; - trace!( - server_name = %server_name, - tool_count = server_tools.len(), - "listed MCP server tools while building tool list" - ); + available_server_count += 1; tools.extend( server_tools .into_iter() .map(|tool| self.with_server_metadata(tool)), ); } - normalize_tools_for_model_with_prefix(tools, self.prefix_mcp_tool_names) + let tools = normalize_tools_for_model_with_prefix(tools, self.prefix_mcp_tool_names); + trace!( + available_server_count, + unavailable_server_count, + tool_count = tools.len(), + "built MCP tool list" + ); + tools } /// Force-refresh codex apps tools by bypassing the in-process cache.