mirror of
https://github.com/openai/codex.git
synced 2026-04-28 02:11:08 +03:00
Add MCP connector metrics (#15805)
## Summary - enrich `codex.mcp.call` with `tool`, `connector_id`, and sanitized `connector_name` for actual MCP executions - record `codex.mcp.call.duration_ms` for actual MCP executions so connector-level latency is visible in metrics - keep skipped, blocked, declined, and cancelled paths on the plain status-only `codex.mcp.call` counter ## Included Changes - `codex-rs/core/src/mcp_tool_call.rs`: add connector-sliced MCP count and duration metrics only for executed tool calls, while leaving non-executed outcomes as status-only counts - `codex-rs/core/src/mcp_tool_call_tests.rs`: cover metric tag shaping, connector-name sanitization, and the new duration metric tags ## Testing - `cargo test -p codex-core` - `just fix -p codex-core` - `just fmt` ## Notes - `cargo test -p codex-core` still hits existing unrelated failures in approvals-reviewer config tests and the sandboxed JS REPL `mktemp` test - full workspace `cargo test` was not run --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
nicholasclark-openai
committed by
GitHub
GitHub
parent
0d44bd708e
commit
8d479f741c
@@ -38,6 +38,7 @@ use codex_analytics::AppInvocation;
|
||||
use codex_analytics::InvocationType;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_features::Feature;
|
||||
use codex_otel::sanitize_metric_tag_value;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -61,6 +62,9 @@ use tracing::Span;
|
||||
use tracing::field::Empty;
|
||||
use url::Url;
|
||||
|
||||
const MCP_CALL_COUNT_METRIC: &str = "codex.mcp.call";
|
||||
const MCP_CALL_DURATION_METRIC: &str = "codex.mcp.call.duration_ms";
|
||||
|
||||
/// Handles the specified tool call dispatches the appropriate
|
||||
/// `McpToolCallBegin` and `McpToolCallEnd` events to the `Session`.
|
||||
pub(crate) async fn handle_mcp_tool_call(
|
||||
@@ -128,7 +132,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
.await;
|
||||
let status = if result.is_ok() { "ok" } else { "error" };
|
||||
turn_context.session_telemetry.counter(
|
||||
"codex.mcp.call",
|
||||
MCP_CALL_COUNT_METRIC,
|
||||
/*inc*/ 1,
|
||||
&[("status", status)],
|
||||
);
|
||||
@@ -166,7 +170,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
)
|
||||
.await
|
||||
{
|
||||
let result = match decision {
|
||||
let (result, call_duration) = match decision {
|
||||
McpToolApprovalDecision::Accept
|
||||
| McpToolApprovalDecision::AcceptForSession
|
||||
| McpToolApprovalDecision::AcceptAndRemember => {
|
||||
@@ -206,10 +210,11 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
if let Err(error) = &result {
|
||||
tracing::warn!("MCP tool call error: {error:?}");
|
||||
}
|
||||
let duration = start.elapsed();
|
||||
let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent {
|
||||
call_id: call_id.clone(),
|
||||
invocation,
|
||||
duration: start.elapsed(),
|
||||
duration,
|
||||
result: result.clone(),
|
||||
});
|
||||
notify_mcp_tool_call_event(
|
||||
@@ -225,50 +230,62 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
&tool_name,
|
||||
)
|
||||
.await;
|
||||
result
|
||||
(result, Some(duration))
|
||||
}
|
||||
McpToolApprovalDecision::Decline => {
|
||||
let message = "user rejected MCP tool call".to_string();
|
||||
notify_mcp_tool_call_skip(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
message,
|
||||
/*already_started*/ true,
|
||||
(
|
||||
notify_mcp_tool_call_skip(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
message,
|
||||
/*already_started*/ true,
|
||||
)
|
||||
.await,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
McpToolApprovalDecision::Cancel => {
|
||||
let message = "user cancelled MCP tool call".to_string();
|
||||
notify_mcp_tool_call_skip(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
message,
|
||||
/*already_started*/ true,
|
||||
(
|
||||
notify_mcp_tool_call_skip(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
message,
|
||||
/*already_started*/ true,
|
||||
)
|
||||
.await,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
McpToolApprovalDecision::BlockedBySafetyMonitor(message) => {
|
||||
notify_mcp_tool_call_skip(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
message,
|
||||
/*already_started*/ true,
|
||||
(
|
||||
notify_mcp_tool_call_skip(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
message,
|
||||
/*already_started*/ true,
|
||||
)
|
||||
.await,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
};
|
||||
|
||||
let status = if result.is_ok() { "ok" } else { "error" };
|
||||
turn_context.session_telemetry.counter(
|
||||
"codex.mcp.call",
|
||||
/*inc*/ 1,
|
||||
&[("status", status)],
|
||||
emit_mcp_call_metrics(
|
||||
turn_context.as_ref(),
|
||||
status,
|
||||
&tool_name,
|
||||
connector_id.as_deref(),
|
||||
connector_name.as_deref(),
|
||||
call_duration,
|
||||
);
|
||||
|
||||
return CallToolResult::from_result(result);
|
||||
@@ -306,10 +323,11 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
if let Err(error) = &result {
|
||||
tracing::warn!("MCP tool call error: {error:?}");
|
||||
}
|
||||
let duration = start.elapsed();
|
||||
let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent {
|
||||
call_id: call_id.clone(),
|
||||
invocation,
|
||||
duration: start.elapsed(),
|
||||
duration,
|
||||
result: result.clone(),
|
||||
});
|
||||
|
||||
@@ -322,13 +340,63 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
maybe_track_codex_app_used(sess.as_ref(), turn_context.as_ref(), &server, &tool_name).await;
|
||||
|
||||
let status = if result.is_ok() { "ok" } else { "error" };
|
||||
turn_context
|
||||
.session_telemetry
|
||||
.counter("codex.mcp.call", /*inc*/ 1, &[("status", status)]);
|
||||
emit_mcp_call_metrics(
|
||||
turn_context.as_ref(),
|
||||
status,
|
||||
&tool_name,
|
||||
connector_id.as_deref(),
|
||||
connector_name.as_deref(),
|
||||
Some(duration),
|
||||
);
|
||||
|
||||
CallToolResult::from_result(result)
|
||||
}
|
||||
|
||||
fn emit_mcp_call_metrics(
|
||||
turn_context: &TurnContext,
|
||||
status: &str,
|
||||
tool_name: &str,
|
||||
connector_id: Option<&str>,
|
||||
connector_name: Option<&str>,
|
||||
duration: Option<Duration>,
|
||||
) {
|
||||
let tags = mcp_call_metric_tags(status, tool_name, connector_id, connector_name);
|
||||
let tag_refs: Vec<(&str, &str)> = tags
|
||||
.iter()
|
||||
.map(|(key, value)| (*key, value.as_str()))
|
||||
.collect();
|
||||
turn_context
|
||||
.session_telemetry
|
||||
.counter(MCP_CALL_COUNT_METRIC, /*inc*/ 1, &tag_refs);
|
||||
if let Some(duration) = duration {
|
||||
turn_context.session_telemetry.record_duration(
|
||||
MCP_CALL_DURATION_METRIC,
|
||||
duration,
|
||||
&tag_refs,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn mcp_call_metric_tags(
|
||||
status: &str,
|
||||
tool_name: &str,
|
||||
connector_id: Option<&str>,
|
||||
connector_name: Option<&str>,
|
||||
) -> Vec<(&'static str, String)> {
|
||||
let mut tags = vec![
|
||||
("status", sanitize_metric_tag_value(status)),
|
||||
("tool", sanitize_metric_tag_value(tool_name)),
|
||||
];
|
||||
if let Some(connector_id) = connector_id.filter(|connector_id| !connector_id.is_empty()) {
|
||||
tags.push(("connector_id", sanitize_metric_tag_value(connector_id)));
|
||||
}
|
||||
if let Some(connector_name) = connector_name.filter(|connector_name| !connector_name.is_empty())
|
||||
{
|
||||
tags.push(("connector_name", sanitize_metric_tag_value(connector_name)));
|
||||
}
|
||||
tags
|
||||
}
|
||||
|
||||
fn mcp_tool_call_span(
|
||||
session: &Session,
|
||||
turn_context: &TurnContext,
|
||||
|
||||
Reference in New Issue
Block a user