Skip to content

Commit fcaaec5

Browse files
charley-openaicharley-openai
committed
Ingest node_repl stderr telemetry spans
1 parent 6bbd710 commit fcaaec5

12 files changed

Lines changed: 805 additions & 5 deletions

File tree

‎codex-rs/codex-mcp/src/mcp_connection_manager.rs‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ use codex_rmcp_client::LocalStdioServerLauncher;
5656
use codex_rmcp_client::RmcpClient;
5757
use codex_rmcp_client::SendElicitation;
5858
use codex_rmcp_client::StdioServerLauncher;
59+
use codex_rmcp_client::StdioServerTelemetry;
60+
use codex_rmcp_client::StdioServerTelemetrySink;
5961
use futures::future::BoxFuture;
6062
use futures::future::FutureExt;
6163
use futures::future::Shared;
@@ -111,6 +113,7 @@ const CODEX_APPS_TOOLS_CACHE_DIR: &str = "cache/codex_apps_tools";
111113
const MCP_TOOLS_LIST_DURATION_METRIC: &str = "codex.mcp.tools.list.duration_ms";
112114
const MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC: &str = "codex.mcp.tools.fetch_uncached.duration_ms";
113115
const MCP_TOOLS_CACHE_WRITE_DURATION_METRIC: &str = "codex.mcp.tools.cache_write.duration_ms";
116+
const NODE_REPL_MCP_SERVER_NAME: &str = "node_repl";
114117

115118
fn sha1_hex(s: &str) -> String {
116119
let mut hasher = Sha1::new();
@@ -1579,9 +1582,17 @@ async fn make_rmcp_client(
15791582
// `RmcpClient` always sees a launched MCP stdio server. The
15801583
// launcher hides whether that means a local child process or an
15811584
// executor process whose stdin/stdout bytes cross the process API.
1582-
RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd, launcher)
1583-
.await
1584-
.map_err(|err| StartupOutcomeError::from(anyhow!(err)))
1585+
RmcpClient::new_stdio_client(
1586+
command_os,
1587+
args_os,
1588+
env_os,
1589+
&env_vars,
1590+
cwd,
1591+
launcher,
1592+
node_repl_telemetry_sink(server_name),
1593+
)
1594+
.await
1595+
.map_err(|err| StartupOutcomeError::from(anyhow!(err)))
15851596
}
15861597
McpServerTransportConfig::StreamableHttp {
15871598
url,
@@ -1625,6 +1636,26 @@ async fn make_rmcp_client(
16251636
}
16261637
}
16271638

1639+
fn node_repl_telemetry_sink(server_name: &str) -> Option<StdioServerTelemetrySink> {
1640+
if server_name != NODE_REPL_MCP_SERVER_NAME {
1641+
return None;
1642+
}
1643+
1644+
Some(Arc::new(|telemetry: StdioServerTelemetry| {
1645+
if let Err(error) = codex_otel::emit_node_repl_stderr_span_telemetry(telemetry.payload) {
1646+
match error {
1647+
codex_otel::StderrSpanTelemetryError::UnsupportedVersion
1648+
| codex_otel::StderrSpanTelemetryError::UnsupportedType => {
1649+
tracing::debug!("ignoring unsupported node_repl stderr telemetry: {error}");
1650+
}
1651+
_ => {
1652+
warn!("ignoring invalid node_repl stderr telemetry: {error}");
1653+
}
1654+
}
1655+
}
1656+
}))
1657+
}
1658+
16281659
fn write_cached_codex_apps_tools_if_needed(
16291660
server_name: &str,
16301661
cache_context: Option<&CodexAppsToolsCacheContext>,

‎codex-rs/core/src/mcp_tool_call.rs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ async fn execute_mcp_tool_call(
477477
metadata.and_then(|metadata| metadata.openai_file_input_params.as_deref()),
478478
)
479479
.await?;
480+
let request_meta =
481+
augment_mcp_tool_request_meta_with_otel_stderr_spans(server, tool_name, request_meta);
480482
let request_meta =
481483
with_mcp_tool_call_thread_id_meta(request_meta, &sess.conversation_id.to_string());
482484
let request_meta =
@@ -663,6 +665,10 @@ const MCP_TOOL_CODEX_APPS_META_KEY: &str = "_codex_apps";
663665
const MCP_TOOL_OPENAI_OUTPUT_TEMPLATE_META_KEY: &str = "openai/outputTemplate";
664666
const MCP_TOOL_UI_RESOURCE_URI_META_KEY: &str = "ui/resourceUri";
665667
const MCP_TOOL_THREAD_ID_META_KEY: &str = "threadId";
668+
const MCP_TOOL_OTEL_STDERR_SPANS_META_KEY: &str = "x-codex-otel-stderr-spans";
669+
const MCP_TOOL_OTEL_STDERR_SPANS_VERSION: u64 = 1;
670+
const NODE_REPL_MCP_SERVER_NAME: &str = "node_repl";
671+
const NODE_REPL_JS_TOOL_NAME: &str = "js";
666672

667673
fn custom_mcp_tool_approval_mode(
668674
turn_context: &TurnContext,
@@ -717,6 +723,59 @@ fn build_mcp_tool_call_request_meta(
717723
(!request_meta.is_empty()).then_some(serde_json::Value::Object(request_meta))
718724
}
719725

726+
fn augment_mcp_tool_request_meta_with_otel_stderr_spans(
727+
server: &str,
728+
tool_name: &str,
729+
meta: Option<serde_json::Value>,
730+
) -> Option<serde_json::Value> {
731+
if server != NODE_REPL_MCP_SERVER_NAME || tool_name != NODE_REPL_JS_TOOL_NAME {
732+
return meta;
733+
}
734+
735+
let Some(trace_context) = codex_otel::current_span_w3c_trace_context() else {
736+
return meta;
737+
};
738+
let Some(traceparent) = trace_context.traceparent else {
739+
return meta;
740+
};
741+
742+
let mut telemetry_meta = serde_json::Map::new();
743+
telemetry_meta.insert("enabled".to_string(), serde_json::Value::Bool(true));
744+
telemetry_meta.insert(
745+
"v".to_string(),
746+
serde_json::Value::Number(MCP_TOOL_OTEL_STDERR_SPANS_VERSION.into()),
747+
);
748+
telemetry_meta.insert(
749+
"traceparent".to_string(),
750+
serde_json::Value::String(traceparent),
751+
);
752+
if let Some(tracestate) = trace_context.tracestate {
753+
telemetry_meta.insert(
754+
"tracestate".to_string(),
755+
serde_json::Value::String(tracestate),
756+
);
757+
}
758+
759+
match meta {
760+
Some(serde_json::Value::Object(mut map)) => {
761+
map.insert(
762+
MCP_TOOL_OTEL_STDERR_SPANS_META_KEY.to_string(),
763+
serde_json::Value::Object(telemetry_meta),
764+
);
765+
Some(serde_json::Value::Object(map))
766+
}
767+
None => {
768+
let mut map = serde_json::Map::new();
769+
map.insert(
770+
MCP_TOOL_OTEL_STDERR_SPANS_META_KEY.to_string(),
771+
serde_json::Value::Object(telemetry_meta),
772+
);
773+
Some(serde_json::Value::Object(map))
774+
}
775+
other => other,
776+
}
777+
}
778+
720779
fn with_mcp_tool_call_thread_id_meta(
721780
meta: Option<serde_json::Value>,
722781
thread_id: &str,

‎codex-rs/core/src/mcp_tool_call_tests.rs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,94 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
651651
);
652652
}
653653

654+
#[test]
655+
fn node_repl_js_request_meta_includes_otel_stderr_spans_when_trace_context_exists() {
656+
use opentelemetry::trace::TracerProvider as _;
657+
use opentelemetry_sdk::trace::SdkTracerProvider;
658+
use tracing_subscriber::prelude::*;
659+
660+
let provider = SdkTracerProvider::builder().build();
661+
let tracer = provider.tracer("codex-core-mcp-tool-call-tests");
662+
let subscriber =
663+
tracing_subscriber::registry().with(tracing_opentelemetry::layer().with_tracer(tracer));
664+
let _guard = tracing::subscriber::set_default(subscriber);
665+
666+
let span = tracing::info_span!("mcp.tools.call");
667+
let _entered = span.enter();
668+
669+
let meta = augment_mcp_tool_request_meta_with_otel_stderr_spans(
670+
"node_repl",
671+
"js",
672+
Some(serde_json::json!({
673+
"threadId": "thread-live",
674+
})),
675+
)
676+
.expect("meta");
677+
678+
let telemetry = meta
679+
.get(MCP_TOOL_OTEL_STDERR_SPANS_META_KEY)
680+
.expect("otel stderr span metadata");
681+
assert_eq!(
682+
telemetry.get("enabled"),
683+
Some(&serde_json::Value::Bool(true))
684+
);
685+
assert_eq!(
686+
telemetry.get("v"),
687+
Some(&serde_json::json!(MCP_TOOL_OTEL_STDERR_SPANS_VERSION))
688+
);
689+
assert!(
690+
telemetry
691+
.get("traceparent")
692+
.and_then(serde_json::Value::as_str)
693+
.is_some_and(|traceparent| traceparent.starts_with("00-"))
694+
);
695+
assert_eq!(
696+
meta.get("threadId"),
697+
Some(&serde_json::json!("thread-live"))
698+
);
699+
}
700+
701+
#[test]
702+
fn otel_stderr_spans_meta_is_only_added_for_node_repl_js_with_trace_context() {
703+
assert_eq!(
704+
augment_mcp_tool_request_meta_with_otel_stderr_spans(
705+
"node_repl",
706+
"js",
707+
Some(serde_json::json!({"threadId": "thread-live"})),
708+
),
709+
Some(serde_json::json!({"threadId": "thread-live"}))
710+
);
711+
712+
use opentelemetry::trace::TracerProvider as _;
713+
use opentelemetry_sdk::trace::SdkTracerProvider;
714+
use tracing_subscriber::prelude::*;
715+
716+
let provider = SdkTracerProvider::builder().build();
717+
let tracer = provider.tracer("codex-core-mcp-tool-call-tests");
718+
let subscriber =
719+
tracing_subscriber::registry().with(tracing_opentelemetry::layer().with_tracer(tracer));
720+
let _guard = tracing::subscriber::set_default(subscriber);
721+
let span = tracing::info_span!("mcp.tools.call");
722+
let _entered = span.enter();
723+
724+
assert_eq!(
725+
augment_mcp_tool_request_meta_with_otel_stderr_spans(
726+
"node_repl",
727+
"other",
728+
Some(serde_json::json!({"threadId": "thread-live"})),
729+
),
730+
Some(serde_json::json!({"threadId": "thread-live"}))
731+
);
732+
assert_eq!(
733+
augment_mcp_tool_request_meta_with_otel_stderr_spans(
734+
"other",
735+
"js",
736+
Some(serde_json::json!({"threadId": "thread-live"})),
737+
),
738+
Some(serde_json::json!({"threadId": "thread-live"}))
739+
);
740+
}
741+
654742
#[test]
655743
fn mcp_tool_call_thread_id_meta_is_added_to_request_meta() {
656744
assert_eq!(

‎codex-rs/core/src/tools/js_repl/mod.rs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ use tokio::sync::Mutex;
2727
use tokio::sync::Notify;
2828
use tokio::sync::OnceCell;
2929
use tokio_util::sync::CancellationToken;
30+
use tracing::Instrument;
31+
use tracing::Span;
3032
use tracing::info;
3133
use tracing::trace;
3234
use tracing::warn;
@@ -1676,6 +1678,7 @@ impl JsReplManager {
16761678
call_id: req.id.clone(),
16771679
payload,
16781680
};
1681+
let span = js_repl_tool_call_span(&exec.session, &exec.turn, &req, &call);
16791682

16801683
let session = Arc::clone(&exec.session);
16811684
let turn = Arc::clone(&exec.turn);
@@ -1691,6 +1694,7 @@ impl JsReplManager {
16911694
call,
16921695
crate::tools::router::ToolCallSource::JsRepl,
16931696
)
1697+
.instrument(span)
16941698
.await
16951699
{
16961700
Ok(result) => {
@@ -1838,6 +1842,78 @@ fn is_js_repl_internal_tool(name: &str) -> bool {
18381842
matches!(name, "js_repl" | "js_repl_reset")
18391843
}
18401844

1845+
fn js_repl_tool_call_span(
1846+
session: &Session,
1847+
turn: &TurnContext,
1848+
req: &RunToolRequest,
1849+
call: &crate::tools::router::ToolCall,
1850+
) -> Span {
1851+
let resolved_tool_name = call.tool_name.display();
1852+
let payload_kind = match &call.payload {
1853+
crate::tools::context::ToolPayload::Function { .. } => "function",
1854+
crate::tools::context::ToolPayload::ToolSearch { .. } => "tool_search",
1855+
crate::tools::context::ToolPayload::Custom { .. } => "custom",
1856+
crate::tools::context::ToolPayload::LocalShell { .. } => "local_shell",
1857+
crate::tools::context::ToolPayload::Mcp { .. } => "mcp",
1858+
};
1859+
let (mcp_server_name, mcp_tool_name) = match &call.payload {
1860+
crate::tools::context::ToolPayload::Mcp { server, tool, .. } => {
1861+
(server.as_str(), tool.as_str())
1862+
}
1863+
crate::tools::context::ToolPayload::Function { .. }
1864+
| crate::tools::context::ToolPayload::ToolSearch { .. }
1865+
| crate::tools::context::ToolPayload::Custom { .. }
1866+
| crate::tools::context::ToolPayload::LocalShell { .. } => ("", ""),
1867+
};
1868+
let browser_action = browser_action_name(req, call);
1869+
1870+
tracing::info_span!(
1871+
"js_repl.tool.call",
1872+
otel.kind = "internal",
1873+
tool.source = "js_repl",
1874+
tool.name = resolved_tool_name,
1875+
tool.requested_name = req.tool_name,
1876+
tool.call_id = req.id,
1877+
tool.payload.kind = payload_kind,
1878+
js_repl.exec_id = req.exec_id,
1879+
browser.action = browser_action.as_deref().unwrap_or(""),
1880+
mcp.server.name = mcp_server_name,
1881+
mcp.tool.name = mcp_tool_name,
1882+
conversation.id = %session.conversation_id,
1883+
session.id = %session.conversation_id,
1884+
turn.id = turn.sub_id.as_str(),
1885+
)
1886+
}
1887+
1888+
fn browser_action_name(
1889+
req: &RunToolRequest,
1890+
call: &crate::tools::router::ToolCall,
1891+
) -> Option<String> {
1892+
if let Some(action) = browser_action_name_from_parts(
1893+
call.tool_name.namespace.as_deref(),
1894+
call.tool_name.name.as_str(),
1895+
) {
1896+
return Some(action);
1897+
}
1898+
if let crate::tools::context::ToolPayload::Mcp { tool, .. } = &call.payload
1899+
&& let Some(action) = browser_action_name_from_parts(None, tool)
1900+
{
1901+
return Some(action);
1902+
}
1903+
browser_action_name_from_parts(None, &req.tool_name)
1904+
}
1905+
1906+
fn browser_action_name_from_parts(namespace: Option<&str>, name: &str) -> Option<String> {
1907+
let namespace = namespace.unwrap_or_default();
1908+
if namespace == "browser" || namespace == "browser/" || namespace == "browser_" {
1909+
return Some(name.trim_start_matches(['_', '/', '.']).to_string());
1910+
}
1911+
name.strip_prefix("browser_")
1912+
.or_else(|| name.strip_prefix("browser."))
1913+
.or_else(|| name.strip_prefix("browser/"))
1914+
.map(str::to_string)
1915+
}
1916+
18411917
#[derive(Clone, Debug, Deserialize)]
18421918
#[serde(tag = "type", rename_all = "snake_case")]
18431919
enum KernelToHost {

0 commit comments

Comments
 (0)