Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 25 additions & 30 deletions codex-rs/core/src/mcp_tool_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ use url::Url;

mod telemetry;

use telemetry::MCP_CALL_COUNT_METRIC;
use telemetry::McpCallMetricOutcome;
use telemetry::McpErrorCodeSource;
use telemetry::emit_mcp_call_metrics;
use telemetry::mcp_call_metric_outcome;
use telemetry::record_mcp_call_outcome_span_telemetry;
Expand Down Expand Up @@ -182,6 +180,13 @@ pub(crate) async fn handle_mcp_tool_call(
.await
};

let connector_id = metadata
.as_ref()
.and_then(|metadata| metadata.connector_id.clone());
let connector_name = metadata
.as_ref()
.and_then(|metadata| metadata.connector_name.clone());

if server == CODEX_APPS_MCP_SERVER_NAME && !app_tool_policy.enabled {
let result = notify_mcp_tool_call_skip(
sess.as_ref(),
Expand All @@ -194,24 +199,22 @@ pub(crate) async fn handle_mcp_tool_call(
)
.await;
let status = if result.is_ok() { "ok" } else { "error" };
turn_context.session_telemetry.counter(
MCP_CALL_COUNT_METRIC,
/*inc*/ 1,
&[("status", status)],
let outcome = McpCallMetricOutcome::from_status(status);
emit_mcp_call_metrics(
turn_context.as_ref(),
&outcome,
&server,
&tool_name,
connector_id.as_deref(),
connector_name.as_deref(),
/*duration*/ None,
);
return HandledMcpToolCall {
result: CallToolResult::from_result(result),
tool_input: arguments_value
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())),
};
}
let connector_id = metadata
.as_ref()
.and_then(|metadata| metadata.connector_id.clone());
let connector_name = metadata
.as_ref()
.and_then(|metadata| metadata.connector_name.clone());

notify_mcp_tool_call_started(
sess.as_ref(),
turn_context.as_ref(),
Expand Down Expand Up @@ -279,6 +282,7 @@ pub(crate) async fn handle_mcp_tool_call(
emit_mcp_call_metrics(
turn_context.as_ref(),
&outcome,
&server,
&tool_name,
connector_id.as_deref(),
connector_name.as_deref(),
Expand Down Expand Up @@ -348,17 +352,11 @@ async fn handle_approved_mcp_tool_call(
let arguments_value = invocation.arguments.clone();
let connector_id = metadata.and_then(|metadata| metadata.connector_id.as_deref());
let connector_name = metadata.and_then(|metadata| metadata.connector_name.as_deref());
let (server_origin, error_code_source) = {
let server_origin = {
let mcp_connection_manager = sess.services.mcp_connection_manager.load_full();
let server_origin = mcp_connection_manager
mcp_connection_manager
.server_origin(&server)
.map(str::to_string);
let error_code_source = if mcp_connection_manager.is_host_owned_codex_apps_server(&server) {
McpErrorCodeSource::HostedPluginService
} else {
McpErrorCodeSource::Untrusted
};
(server_origin, error_code_source)
.map(str::to_string)
};

let start = Instant::now();
Expand Down Expand Up @@ -392,7 +390,7 @@ async fn handle_approved_mcp_tool_call(
.await
}
.await;
record_mcp_result_span_telemetry(&Span::current(), &result, error_code_source);
record_mcp_result_span_telemetry(&Span::current(), &result);
result
}
.instrument(mcp_tool_call_span(
Expand Down Expand Up @@ -424,10 +422,11 @@ async fn handle_approved_mcp_tool_call(
.await;
maybe_track_codex_app_used(sess, turn_context, &server, &tool_name).await;

let outcome = mcp_call_metric_outcome(&result, error_code_source);
let outcome = mcp_call_metric_outcome(&result);
emit_mcp_call_metrics(
turn_context,
&outcome,
&server,
&tool_name,
connector_id,
connector_name,
Expand Down Expand Up @@ -501,12 +500,8 @@ fn record_server_fields(span: &Span, url: Option<&str>) {
}
}

fn record_mcp_result_span_telemetry(
span: &Span,
result: &Result<CallToolResult, String>,
error_code_source: McpErrorCodeSource,
) {
record_mcp_call_outcome_span_telemetry(span, result, error_code_source);
fn record_mcp_result_span_telemetry(span: &Span, result: &Result<CallToolResult, String>) {
record_mcp_call_outcome_span_telemetry(span, result);

let Some(span_telemetry) = result
.as_ref()
Expand Down
79 changes: 38 additions & 41 deletions codex-rs/core/src/mcp_tool_call/telemetry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use codex_protocol::mcp::CallToolResult;
use serde_json::Value as JsonValue;
use tracing::Span;

pub(super) const MCP_CALL_COUNT_METRIC: &str = "codex.mcp.call";
const MCP_CALL_COUNT_METRIC: &str = "codex.mcp.call";
const MCP_CALL_DURATION_METRIC: &str = "codex.mcp.call.duration_ms";
const MCP_CALL_ERROR_COUNT_METRIC: &str = "codex.mcp.call.error";
// No CallToolResult was received. This includes request setup, transport, timeout, protocol, and
Expand Down Expand Up @@ -37,21 +37,22 @@ impl McpCallMetricOutcome {
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(super) enum McpErrorCodeSource {
HostedPluginService,
Untrusted,
}

pub(super) fn emit_mcp_call_metrics(
turn_context: &TurnContext,
outcome: &McpCallMetricOutcome,
server_name: &str,
tool_name: &str,
connector_id: Option<&str>,
connector_name: Option<&str>,
duration: Option<Duration>,
) {
let tags = mcp_call_metric_tags(outcome.status, tool_name, connector_id, connector_name);
let tags = mcp_call_metric_tags(
outcome.status,
server_name,
tool_name,
connector_id,
connector_name,
);
let tag_refs: Vec<(&str, &str)> = tags
.iter()
.map(|(key, value)| (*key, value.as_str()))
Expand Down Expand Up @@ -87,12 +88,14 @@ pub(super) fn emit_mcp_call_metrics(

fn mcp_call_metric_tags(
status: &str,
server_name: &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)),
("server", sanitize_metric_tag_value(server_name)),
("tool", sanitize_metric_tag_value(tool_name)),
];
if let Some(connector_id) = connector_id.filter(|connector_id| !connector_id.is_empty()) {
Expand All @@ -107,40 +110,35 @@ fn mcp_call_metric_tags(

pub(super) fn mcp_call_metric_outcome(
result: &Result<CallToolResult, String>,
error_code_source: McpErrorCodeSource,
) -> McpCallMetricOutcome {
match result {
Ok(result) if result.is_error.unwrap_or(false) => {
let error_code = if error_code_source == McpErrorCodeSource::HostedPluginService {
result
.structured_content
.as_ref()
.and_then(JsonValue::as_object)
.and_then(|structured_content| structured_content.get("error_code"))
.and_then(JsonValue::as_str)
.filter(|error_code| !error_code.is_empty())
.or_else(|| {
result
.meta
.as_ref()
.and_then(JsonValue::as_object)
.and_then(|meta| meta.get(MCP_TOOL_CODEX_APPS_META_KEY))
.and_then(JsonValue::as_object)
.and_then(|codex_apps| codex_apps.get("connector_auth_failure"))
.and_then(JsonValue::as_object)
.filter(|auth_failure| {
auth_failure
.get("is_auth_failure")
.and_then(JsonValue::as_bool)
== Some(true)
})
.and_then(|auth_failure| auth_failure.get("error_code"))
.and_then(JsonValue::as_str)
.filter(|error_code| !error_code.is_empty())
})
} else {
None
};
let error_code = result
.structured_content
.as_ref()
.and_then(JsonValue::as_object)
.and_then(|structured_content| structured_content.get("error_code"))
.and_then(JsonValue::as_str)
.filter(|error_code| !error_code.is_empty())
.or_else(|| {
result
.meta
.as_ref()
.and_then(JsonValue::as_object)
.and_then(|meta| meta.get(MCP_TOOL_CODEX_APPS_META_KEY))
.and_then(JsonValue::as_object)
.and_then(|codex_apps| codex_apps.get("connector_auth_failure"))
.and_then(JsonValue::as_object)
.filter(|auth_failure| {
auth_failure
.get("is_auth_failure")
.and_then(JsonValue::as_bool)
== Some(true)
})
.and_then(|auth_failure| auth_failure.get("error_code"))
.and_then(JsonValue::as_str)
.filter(|error_code| !error_code.is_empty())
});
let error_code: String = error_code
.unwrap_or(MCP_CALL_ERROR_CODE_UNKNOWN)
.chars()
Expand All @@ -164,9 +162,8 @@ pub(super) fn mcp_call_metric_outcome(
pub(super) fn record_mcp_call_outcome_span_telemetry(
span: &Span,
result: &Result<CallToolResult, String>,
error_code_source: McpErrorCodeSource,
) {
let outcome = mcp_call_metric_outcome(result, error_code_source);
let outcome = mcp_call_metric_outcome(result);
let (Some(error_type), Some(error_code)) = (outcome.error_type, outcome.error_code) else {
return;
};
Expand Down
59 changes: 35 additions & 24 deletions codex-rs/core/src/mcp_tool_call/telemetry_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,39 +14,50 @@ fn metric_call_tool_result(
}

#[test]
fn mcp_call_metric_outcome_distinguishes_request_and_tool_errors() {
fn mcp_call_metric_tags_include_server_name() {
assert_eq!(
mcp_call_metric_outcome(
&Ok(metric_call_tool_result(
/*is_error*/ false, /*structured_content*/ None,
)),
McpErrorCodeSource::HostedPluginService,
mcp_call_metric_tags(
"error",
"docs server",
"search docs",
Some("connector/docs"),
Some("Docs connector"),
),
vec![
("status", "error".to_string()),
("server", "docs_server".to_string()),
("tool", "search_docs".to_string()),
("connector_id", "connector/docs".to_string()),
("connector_name", "Docs_connector".to_string()),
],
);
}

#[test]
fn mcp_call_metric_outcome_distinguishes_request_and_tool_errors() {
assert_eq!(
mcp_call_metric_outcome(&Ok(metric_call_tool_result(
/*is_error*/ false, /*structured_content*/ None,
)),),
McpCallMetricOutcome {
status: "ok",
error_type: None,
error_code: None,
}
);
assert_eq!(
mcp_call_metric_outcome(
&Ok(metric_call_tool_result(
/*is_error*/ true,
Some(serde_json::json!({"error_code": "RATE_LIMITED"})),
)),
McpErrorCodeSource::HostedPluginService,
),
mcp_call_metric_outcome(&Ok(metric_call_tool_result(
/*is_error*/ true,
Some(serde_json::json!({"error_code": "RATE_LIMITED"})),
)),),
McpCallMetricOutcome {
status: "error",
error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
error_code: Some("RATE_LIMITED".to_string()),
}
);
assert_eq!(
mcp_call_metric_outcome(
&Err("connection closed".to_string()),
McpErrorCodeSource::HostedPluginService,
),
mcp_call_metric_outcome(&Err("connection closed".to_string())),
McpCallMetricOutcome {
status: "error",
error_type: Some(MCP_CALL_ERROR_TYPE_MCP_REQUEST),
Expand All @@ -56,24 +67,24 @@ fn mcp_call_metric_outcome_distinguishes_request_and_tool_errors() {
}

#[test]
fn mcp_call_metric_outcome_ignores_untrusted_tool_error_codes() {
fn mcp_call_metric_outcome_reports_server_tool_error_codes() {
let result = Ok(metric_call_tool_result(
/*is_error*/ true,
Some(serde_json::json!({"error_code": "arbitrary-user-value"})),
));

assert_eq!(
mcp_call_metric_outcome(&result, McpErrorCodeSource::Untrusted),
mcp_call_metric_outcome(&result),
McpCallMetricOutcome {
status: "error",
error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
error_code: Some(MCP_CALL_ERROR_CODE_UNKNOWN.to_string()),
error_code: Some("arbitrary-user-value".to_string()),
}
);
}

#[test]
fn mcp_call_metric_outcome_reads_hosted_auth_error_code_from_meta() {
fn mcp_call_metric_outcome_reads_auth_error_code_from_meta() {
let result = CallToolResult {
content: Vec::new(),
structured_content: None,
Expand All @@ -89,7 +100,7 @@ fn mcp_call_metric_outcome_reads_hosted_auth_error_code_from_meta() {
};

assert_eq!(
mcp_call_metric_outcome(&Ok(result), McpErrorCodeSource::HostedPluginService),
mcp_call_metric_outcome(&Ok(result)),
McpCallMetricOutcome {
status: "error",
error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
Expand All @@ -99,15 +110,15 @@ fn mcp_call_metric_outcome_reads_hosted_auth_error_code_from_meta() {
}

#[test]
fn mcp_call_metric_outcome_bounds_and_sanitizes_hosted_error_code() {
fn mcp_call_metric_outcome_bounds_and_sanitizes_error_code() {
let raw_error_code = format!("BAD CODE {}", "x".repeat(300));
let result = Ok(metric_call_tool_result(
/*is_error*/ true,
Some(serde_json::json!({"error_code": raw_error_code})),
));

assert_eq!(
mcp_call_metric_outcome(&result, McpErrorCodeSource::HostedPluginService),
mcp_call_metric_outcome(&result),
McpCallMetricOutcome {
status: "error",
error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
Expand Down
Loading
Loading