mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Preserve MCP trace context when tracing is disabled (#60506)
## Why MCP requests need to retain inbound W3C trace context even when no OpenTelemetry tracing layer is installed. ## What changed - Carry validated request trace context through app-server MCP tasks, connection startup, and resource listing tasks, while preferring active span context. - Forward fallback `traceparent` and `tracestate` headers for MCP HTTP requests, including discovery and initialization. - Use HTTP trace headers as a fallback for exec-server RPC calls when active span context is unavailable. ## Testing Add an app-server integration test that disables trace exporting and verifies that `mcpServer/resource/read` forwards the inbound `traceparent` and `tracestate` to the MCP server. GitOrigin-RevId: c886a55ce0a3992f1b2e04614efec80a20cb4b49
This commit is contained in:
@@ -10,6 +10,7 @@ use codex_mcp::ema_auth_scope;
|
||||
use codex_mcp::resolve_oauth_callback;
|
||||
use codex_otel::auth_storage::AuthStorageOriginator;
|
||||
use codex_rmcp_client::EnterpriseOAuthLoginRequest;
|
||||
use codex_rmcp_client::McpRequestTrace;
|
||||
use std::future::Future;
|
||||
|
||||
use crate::thread_state::ThreadStateManager;
|
||||
@@ -394,7 +395,7 @@ impl McpRequestProcessor {
|
||||
.await;
|
||||
outgoing.send_result(request, result).await;
|
||||
};
|
||||
tokio::spawn(AuthStorageOriginator::current().scope(task));
|
||||
self.spawn_mcp_request(request_id, task).await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -683,7 +684,7 @@ impl McpRequestProcessor {
|
||||
|
||||
if let Some(thread_id) = thread_id {
|
||||
let (_, thread) = self.load_thread(&thread_id).await?;
|
||||
let request_id = request_id.clone();
|
||||
let response_request_id = request_id.clone();
|
||||
|
||||
let task = async move {
|
||||
let origin_call_id = origin_call_id.filter(|_| is_codex_apps);
|
||||
@@ -695,10 +696,15 @@ impl McpRequestProcessor {
|
||||
}
|
||||
None => thread.read_mcp_resource(&server, resource_params).await,
|
||||
};
|
||||
Self::send_mcp_resource_read_response(outgoing, request_id, result, origin_call_id)
|
||||
.await;
|
||||
Self::send_mcp_resource_read_response(
|
||||
outgoing,
|
||||
response_request_id,
|
||||
result,
|
||||
origin_call_id,
|
||||
)
|
||||
.await;
|
||||
};
|
||||
tokio::spawn(AuthStorageOriginator::current().scope(task));
|
||||
self.spawn_mcp_request(request_id, task).await;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -722,7 +728,7 @@ impl McpRequestProcessor {
|
||||
// environment stdio MCPs must declare their own absolute cwd.
|
||||
let runtime_context =
|
||||
McpRuntimeContext::new(Arc::clone(&environment_manager), config.cwd.to_path_buf());
|
||||
let request_id = request_id.clone();
|
||||
let response_request_id = request_id.clone();
|
||||
|
||||
let task = async move {
|
||||
let result = read_mcp_resource_without_thread(
|
||||
@@ -738,11 +744,14 @@ impl McpRequestProcessor {
|
||||
.await
|
||||
.and_then(|result| serde_json::to_value(result).map_err(anyhow::Error::from));
|
||||
Self::send_mcp_resource_read_response(
|
||||
outgoing, request_id, result, /*origin_call_id*/ None,
|
||||
outgoing,
|
||||
response_request_id,
|
||||
result,
|
||||
/*origin_call_id*/ None,
|
||||
)
|
||||
.await;
|
||||
};
|
||||
tokio::spawn(AuthStorageOriginator::current().scope(task));
|
||||
self.spawn_mcp_request(request_id, task).await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -778,7 +787,7 @@ impl McpRequestProcessor {
|
||||
let (_, thread) = self.load_thread(&thread_id).await?;
|
||||
ensure_direct_input_allowed(thread.as_ref()).await?;
|
||||
let meta = with_mcp_tool_call_thread_id_meta(params.meta, &thread_id);
|
||||
let request_id = request_id.clone();
|
||||
let response_request_id = request_id.clone();
|
||||
|
||||
let task = async move {
|
||||
let result = thread
|
||||
@@ -786,11 +795,25 @@ impl McpRequestProcessor {
|
||||
.await
|
||||
.map(McpServerToolCallResponse::from)
|
||||
.map_err(mcp_operation_error);
|
||||
outgoing.send_result(request_id, result).await;
|
||||
outgoing.send_result(response_request_id, result).await;
|
||||
};
|
||||
tokio::spawn(AuthStorageOriginator::current().scope(task));
|
||||
self.spawn_mcp_request(request_id, task).await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn spawn_mcp_request<'a>(
|
||||
&'a self,
|
||||
request_id: &'a ConnectionRequestId,
|
||||
task: impl Future<Output = ()> + Send + 'static,
|
||||
) -> impl Future<Output = ()> + 'a {
|
||||
// Bound the dispatcher future before awaiting the request trace lookup.
|
||||
let task = Box::pin(task);
|
||||
async move {
|
||||
let trace =
|
||||
McpRequestTrace::from_w3c(self.outgoing.request_trace_context(request_id).await);
|
||||
tokio::spawn(AuthStorageOriginator::current().scope(trace.scope(task)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn mcp_operation_error(error: anyhow::Error) -> JSONRPCErrorError {
|
||||
|
||||
@@ -1651,7 +1651,7 @@ impl TestAppServer {
|
||||
.await
|
||||
}
|
||||
|
||||
async fn send_jsonrpc_message(&mut self, message: JSONRPCMessage) -> anyhow::Result<()> {
|
||||
pub async fn send_jsonrpc_message(&mut self, message: JSONRPCMessage) -> anyhow::Result<()> {
|
||||
eprintln!("writing message to stdin: {message:?}");
|
||||
let Some(stdin) = self.stdin.as_mut() else {
|
||||
anyhow::bail!("mcp stdin closed");
|
||||
|
||||
@@ -1,9 +1,50 @@
|
||||
//! Public JSON-RPC coverage for caller metadata on a thread's stdio resource read.
|
||||
//! Public JSON-RPC coverage for caller metadata on MCP resource reads.
|
||||
|
||||
use super::*;
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use codex_app_server_protocol::JSONRPCRequest;
|
||||
use codex_protocol::protocol::W3cTraceContext;
|
||||
use core_test_support::stdio_server_bin;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn mcp_resource_read_forwards_rpc_trace_context() -> Result<()> {
|
||||
let responses = responses::start_mock_server().await;
|
||||
let (url, calls, server) = start_resource_apps_mcp_server().await?;
|
||||
let (_home, mut mcp) = start_resource_test_app_server_with_extra_config(
|
||||
&url,
|
||||
&responses.uri(),
|
||||
"[otel]\ntrace_exporter = \"none\"",
|
||||
ResourceTestEnvironment::Auto,
|
||||
)
|
||||
.await?;
|
||||
let traceparent = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01";
|
||||
let tracestate = "vendor=origin";
|
||||
let id = RequestId::String("traced-read".to_string());
|
||||
mcp.send_jsonrpc_message(JSONRPCMessage::Request(JSONRPCRequest {
|
||||
id: id.clone(),
|
||||
method: "mcpServer/resource/read".to_string(),
|
||||
params: Some(json!({"server": "codex_apps", "uri": TEST_RESOURCE_URI})),
|
||||
trace: Some(W3cTraceContext {
|
||||
traceparent: Some(traceparent.to_string()),
|
||||
tracestate: Some(tracestate.to_string()),
|
||||
}),
|
||||
}))
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(id),
|
||||
)
|
||||
.await??;
|
||||
server.abort();
|
||||
|
||||
let reads = calls.read_metadata.lock().expect("resource read metadata");
|
||||
let read = reads.last().expect("resource read reached the MCP server");
|
||||
assert_eq!(read.get("traceparent"), Some(&json!(traceparent)));
|
||||
assert_eq!(read.get("tracestate"), Some(&json!(tracestate)));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn stdio_resource_receives_caller_metadata() -> Result<()> {
|
||||
let responses_server = responses::start_mock_server().await;
|
||||
|
||||
Reference in New Issue
Block a user