Skip to content

[codex] trace MCP startup latency #28630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rphilizaire-openai merged 1 commit into from
Jun 24, 2026
+13 −1
Merged

[codex] trace MCP startup latency #28630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions codex-rs/codex-mcp/src/codex_apps.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use serde::Deserialize;
use serde::Serialize;
use sha1::Digest;
use sha1::Sha1;
use tracing::instrument;

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct CodexAppsToolsCacheKey {
Expand Down Expand Up @@ -170,6 +171,7 @@ pub(crate) fn read_cached_codex_apps_tools(
}
}

#[instrument(level = "trace", skip_all)]
pub(crate) fn load_cached_codex_apps_tools(
cache_context: &CodexAppsToolsCacheContext,
) -> CachedCodexAppsToolsLoad {
Expand Down Expand Up @@ -210,6 +212,7 @@ pub(crate) fn write_cached_codex_apps_tools(
let _ = std::fs::write(cache_path, bytes);
}

#[instrument(level = "trace", skip_all)]
pub(crate) fn load_cached_codex_apps_server_info(
cache_context: &CodexAppsToolsCacheContext,
) -> Option<McpServerInfo> {
Expand Down
8 changes: 7 additions & 1 deletion codex-rs/codex-mcp/src/rmcp_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ use rmcp::model::JsonObject;
use rmcp::model::ProtocolVersion;
use rmcp::model::Tool as RmcpTool;
use tokio_util::sync::CancellationToken;
use tracing::Instrument;
use tracing::instrument;
use tracing::warn;

/// MCP server capability indicating that Codex should include [`SandboxState`]
Expand Down Expand Up @@ -141,6 +143,7 @@ pub(crate) struct AsyncManagedClient {
impl AsyncManagedClient {
// Keep this constructor flat so the startup inputs remain readable at the
// single call site instead of introducing a one-off params wrapper.
#[instrument(level = "trace", skip_all, fields(server_name = %server_name))]

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the session parent for startup spans

Because new is synchronous, this span ends after constructing the shared startup future below; the actual client construction/initialize/tool-list work is later polled from McpConnectionManager::new's background JoinSet task. For non-required MCP servers, or whenever that background task polls the shared future first, the new startup spans are detached from the session_init trace, so they still cannot explain which server made startup slow. Capture and instrument the startup future or spawned task with the current span so these definition-level spans remain under the session init trace.

Useful? React with 👍 / 👎.

@owenlin0 owenlin0 Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rphilizaire-openai this one's worth a look

#[allow(clippy::too_many_arguments)]
pub(crate) fn new(
server_name: String,
Expand Down Expand Up @@ -228,7 +231,7 @@ impl AsyncManagedClient {
startup_complete_for_fut.store(true, Ordering::Release);
outcome
};
let client = fut.boxed().shared();
let client = fut.in_current_span().boxed().shared();
if cached_tool_info_snapshot.is_some() {
let startup_task = client.clone();
tokio::spawn(async move {
Expand Down Expand Up @@ -306,6 +309,7 @@ impl From<anyhow::Error> for StartupOutcomeError {
}
}

#[instrument(level = "trace", skip_all, fields(server_name = %server_name))]
pub(crate) async fn list_tools_for_client_uncached(
server_name: &str,
is_codex_apps_mcp_server: bool,
Expand Down Expand Up @@ -529,6 +533,7 @@ fn validate_mcp_server_name(server_name: &str) -> Result<()> {
Ok(())
}

#[instrument(level = "trace", skip_all, fields(server_name = %server_name))]
async fn start_server_task(
server_name: String,
client: Arc<RmcpClient>,
Expand Down Expand Up @@ -658,6 +663,7 @@ struct StartServerTaskParams {
supports_openai_form_elicitation: bool,
}

#[instrument(level = "trace", skip_all, fields(server_name = %server_name))]
async fn make_rmcp_client(
server_name: &str,
server: EffectiveMcpServer,
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/rmcp-client/src/rmcp_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ use tokio::sync::Mutex;
use tokio::sync::Semaphore;
use tokio::sync::watch;
use tokio::time;
use tracing::instrument;
use tracing::warn;

use crate::elicitation_client_service::ElicitationClientService;
Expand Down Expand Up @@ -420,6 +421,7 @@ impl RmcpClient {

/// Perform the initialization handshake with the MCP server.
/// https://modelcontextprotocol.io/specification/2025-06-18/basic/lifecycle#initialization
#[instrument(level = "trace", skip_all)]
pub async fn initialize(
&self,
params: InitializeRequestParams,
Expand Down Expand Up @@ -501,6 +503,7 @@ impl RmcpClient {
Ok(result)
}

#[instrument(level = "trace", skip_all)]
pub async fn list_tools_with_connector_ids(
&self,
params: Option<PaginatedRequestParams>,
Expand Down
Loading