mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Add memory tool call metrics to memories extension (#24583)
## Why The memories extension now receives a metrics exporter, but the useful extension-owned signal is the memory tool call itself: which operation ran, which memory area it touched, whether the backend call succeeded, and whether the result was truncated. ## What changed - Added the `codex.memories.tool.call` counter in `ext/memories/src/metrics.rs`. - Emit that counter from `memories/add_ad_hoc_note`, `memories/list`, `memories/read`, and `memories/search` after backend execution. - Tag each call with `tool`, `operation`, `scope`, `status`, and `truncated`. - Pass the existing `MetricsClient` through the memories extension into the tool executors; tests use `None`. ## Verification - `just test -p codex-memories-extension`
This commit is contained in:
1 parent
b77be36896
commit
01a8bf0ae3
9 files changed
+154
-22
No files matched your search
@@ -20,14 +20,12 @@ use crate::tools;
|
|||||||
/// Contributes Codex memory read-path prompt context and memory read tools.
|
/// Contributes Codex memory read-path prompt context and memory read tools.
|
||||||
#[derive(Clone, Default)]
|
#[derive(Clone, Default)]
|
||||||
pub(crate) struct MemoriesExtension {
|
pub(crate) struct MemoriesExtension {
|
||||||
_metrics_client: Option<MetricsClient>,
|
metrics_client: Option<MetricsClient>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl MemoriesExtension {
|
impl MemoriesExtension {
|
||||||
fn new(metrics_client: Option<MetricsClient>) -> Self {
|
fn new(metrics_client: Option<MetricsClient>) -> Self {
|
||||||
Self {
|
Self { metrics_client }
|
||||||
_metrics_client: metrics_client,
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -103,7 +101,10 @@ impl ToolContributor for MemoriesExtension {
|
|||||||
return Vec::new();
|
return Vec::new();
|
||||||
}
|
}
|
||||||
|
|
||||||
tools::memory_tools(LocalMemoriesBackend::from_codex_home(&config.codex_home))
|
tools::memory_tools(
|
||||||
|
LocalMemoriesBackend::from_codex_home(&config.codex_home),
|
||||||
|
self.metrics_client.clone(),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
mod backend;
|
mod backend;
|
||||||
mod extension;
|
mod extension;
|
||||||
mod local;
|
mod local;
|
||||||
|
mod metrics;
|
||||||
mod prompts;
|
mod prompts;
|
||||||
mod schema;
|
mod schema;
|
||||||
mod tools;
|
mod tools;
|
||||||
|
|||||||
@@ -0,0 +1,69 @@
|
|||||||
|
use codex_otel::MetricsClient;
|
||||||
|
|
||||||
|
use crate::MEMORY_TOOLS_NAMESPACE;
|
||||||
|
|
||||||
|
pub(crate) const MEMORIES_TOOL_CALL_METRIC: &str = "codex.memories.tool.call";
|
||||||
|
|
||||||
|
pub(crate) fn record_tool_call(
|
||||||
|
metrics_client: Option<&MetricsClient>,
|
||||||
|
operation: &str,
|
||||||
|
scope: &str,
|
||||||
|
success: bool,
|
||||||
|
truncated: &str,
|
||||||
|
) {
|
||||||
|
let Some(metrics_client) = metrics_client else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
let tool = format!("{MEMORY_TOOLS_NAMESPACE}{operation}");
|
||||||
|
let _ = metrics_client.counter(
|
||||||
|
MEMORIES_TOOL_CALL_METRIC,
|
||||||
|
/*inc*/ 1,
|
||||||
|
&[
|
||||||
|
("tool", tool.as_str()),
|
||||||
|
("operation", operation),
|
||||||
|
("scope", scope),
|
||||||
|
("status", status_tag(success)),
|
||||||
|
("truncated", truncated),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn scope_from_path(path: &str) -> &'static str {
|
||||||
|
let path = path.trim_matches('/');
|
||||||
|
let path = path.strip_prefix("./").unwrap_or(path);
|
||||||
|
|
||||||
|
if path.is_empty() {
|
||||||
|
"root"
|
||||||
|
} else if path == "MEMORY.md" {
|
||||||
|
"memory_md"
|
||||||
|
} else if path == "memory_summary.md" {
|
||||||
|
"memory_summary"
|
||||||
|
} else if path == "raw_memories.md" {
|
||||||
|
"raw_memories"
|
||||||
|
} else if path == "rollout_summaries" || path.starts_with("rollout_summaries/") {
|
||||||
|
"rollout_summaries"
|
||||||
|
} else if path == "skills" || path.starts_with("skills/") {
|
||||||
|
"skills"
|
||||||
|
} else if path == "extensions/ad_hoc/notes" || path.starts_with("extensions/ad_hoc/notes/") {
|
||||||
|
"ad_hoc_notes"
|
||||||
|
} else {
|
||||||
|
"other"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn scope_from_optional_path(path: Option<&str>, default: &'static str) -> &'static str {
|
||||||
|
path.map_or(default, scope_from_path)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn truncated_tag(truncated: Option<bool>) -> &'static str {
|
||||||
|
match truncated {
|
||||||
|
Some(true) => "true",
|
||||||
|
Some(false) => "false",
|
||||||
|
None => "unknown",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn status_tag(success: bool) -> &'static str {
|
||||||
|
if success { "succeeded" } else { "failed" }
|
||||||
|
}
|
||||||
@@ -415,10 +415,13 @@ async fn search_tool_rejects_legacy_single_query() {
|
|||||||
|
|
||||||
fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc<dyn ToolExecutor<ToolCall>> {
|
fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc<dyn ToolExecutor<ToolCall>> {
|
||||||
let expected_tool_name = memory_tool_name(tool_name);
|
let expected_tool_name = memory_tool_name(tool_name);
|
||||||
crate::tools::memory_tools(LocalMemoriesBackend::from_memory_root(memory_root))
|
crate::tools::memory_tools(
|
||||||
.into_iter()
|
LocalMemoriesBackend::from_memory_root(memory_root),
|
||||||
.find(|tool| tool.tool_name() == expected_tool_name)
|
/*metrics_client*/ None,
|
||||||
.unwrap_or_else(|| panic!("{tool_name} tool should be registered"))
|
)
|
||||||
|
.into_iter()
|
||||||
|
.find(|tool| tool.tool_name() == expected_tool_name)
|
||||||
|
.unwrap_or_else(|| panic!("{tool_name} tool should be registered"))
|
||||||
}
|
}
|
||||||
|
|
||||||
fn memory_tool_name(tool_name: &str) -> ToolName {
|
fn memory_tool_name(tool_name: &str) -> ToolName {
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ use codex_extension_api::ToolCall;
|
|||||||
use codex_extension_api::ToolExecutor;
|
use codex_extension_api::ToolExecutor;
|
||||||
use codex_extension_api::ToolName;
|
use codex_extension_api::ToolName;
|
||||||
use codex_extension_api::ToolSpec;
|
use codex_extension_api::ToolSpec;
|
||||||
|
use codex_otel::MetricsClient;
|
||||||
use schemars::JsonSchema;
|
use schemars::JsonSchema;
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
@@ -11,6 +12,7 @@ use crate::ADD_AD_HOC_NOTE_TOOL_NAME;
|
|||||||
use crate::backend::AddAdHocMemoryNoteRequest;
|
use crate::backend::AddAdHocMemoryNoteRequest;
|
||||||
use crate::backend::AddAdHocMemoryNoteResponse;
|
use crate::backend::AddAdHocMemoryNoteResponse;
|
||||||
use crate::backend::MemoriesBackend;
|
use crate::backend::MemoriesBackend;
|
||||||
|
use crate::metrics::record_tool_call;
|
||||||
|
|
||||||
use super::backend_error_to_function_call;
|
use super::backend_error_to_function_call;
|
||||||
use super::memory_function_tool;
|
use super::memory_function_tool;
|
||||||
@@ -36,6 +38,7 @@ struct AddAdHocNoteArgs {
|
|||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
pub(super) struct AddAdHocNoteTool<B> {
|
pub(super) struct AddAdHocNoteTool<B> {
|
||||||
pub(super) backend: B,
|
pub(super) backend: B,
|
||||||
|
pub(super) metrics_client: Option<MetricsClient>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[async_trait::async_trait]
|
#[async_trait::async_trait]
|
||||||
@@ -66,8 +69,15 @@ where
|
|||||||
filename: args.filename,
|
filename: args.filename,
|
||||||
note: args.note,
|
note: args.note,
|
||||||
})
|
})
|
||||||
.await
|
.await;
|
||||||
.map_err(backend_error_to_function_call)?;
|
record_tool_call(
|
||||||
|
self.metrics_client.as_ref(),
|
||||||
|
ADD_AD_HOC_NOTE_TOOL_NAME,
|
||||||
|
"ad_hoc_notes",
|
||||||
|
response.is_ok(),
|
||||||
|
"not_applicable",
|
||||||
|
);
|
||||||
|
let response = response.map_err(backend_error_to_function_call)?;
|
||||||
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -3,6 +3,7 @@ use codex_extension_api::ToolCall;
|
|||||||
use codex_extension_api::ToolExecutor;
|
use codex_extension_api::ToolExecutor;
|
||||||
use codex_extension_api::ToolName;
|
use codex_extension_api::ToolName;
|
||||||
use codex_extension_api::ToolSpec;
|
use codex_extension_api::ToolSpec;
|
||||||
|
use codex_otel::MetricsClient;
|
||||||
use schemars::JsonSchema;
|
use schemars::JsonSchema;
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
@@ -13,6 +14,9 @@ use crate::MAX_LIST_RESULTS;
|
|||||||
use crate::backend::ListMemoriesRequest;
|
use crate::backend::ListMemoriesRequest;
|
||||||
use crate::backend::ListMemoriesResponse;
|
use crate::backend::ListMemoriesResponse;
|
||||||
use crate::backend::MemoriesBackend;
|
use crate::backend::MemoriesBackend;
|
||||||
|
use crate::metrics::record_tool_call;
|
||||||
|
use crate::metrics::scope_from_optional_path;
|
||||||
|
use crate::metrics::truncated_tag;
|
||||||
|
|
||||||
use super::backend_error_to_function_call;
|
use super::backend_error_to_function_call;
|
||||||
use super::clamp_max_results;
|
use super::clamp_max_results;
|
||||||
@@ -32,6 +36,7 @@ struct ListArgs {
|
|||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
pub(super) struct ListTool<B> {
|
pub(super) struct ListTool<B> {
|
||||||
pub(super) backend: B,
|
pub(super) backend: B,
|
||||||
|
pub(super) metrics_client: Option<MetricsClient>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[async_trait::async_trait]
|
#[async_trait::async_trait]
|
||||||
@@ -57,6 +62,7 @@ where
|
|||||||
{
|
{
|
||||||
let backend = self.backend.clone();
|
let backend = self.backend.clone();
|
||||||
let args: ListArgs = parse_args(&call)?;
|
let args: ListArgs = parse_args(&call)?;
|
||||||
|
let scope = scope_from_optional_path(args.path.as_deref(), "root");
|
||||||
let response = backend
|
let response = backend
|
||||||
.list(ListMemoriesRequest {
|
.list(ListMemoriesRequest {
|
||||||
path: args.path,
|
path: args.path,
|
||||||
@@ -67,8 +73,15 @@ where
|
|||||||
MAX_LIST_RESULTS,
|
MAX_LIST_RESULTS,
|
||||||
),
|
),
|
||||||
})
|
})
|
||||||
.await
|
.await;
|
||||||
.map_err(backend_error_to_function_call)?;
|
record_tool_call(
|
||||||
|
self.metrics_client.as_ref(),
|
||||||
|
LIST_TOOL_NAME,
|
||||||
|
scope,
|
||||||
|
response.is_ok(),
|
||||||
|
truncated_tag(response.as_ref().ok().map(|response| response.truncated)),
|
||||||
|
);
|
||||||
|
let response = response.map_err(backend_error_to_function_call)?;
|
||||||
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -7,6 +7,7 @@ use codex_extension_api::ToolExecutor;
|
|||||||
use codex_extension_api::ToolName;
|
use codex_extension_api::ToolName;
|
||||||
use codex_extension_api::ToolSpec;
|
use codex_extension_api::ToolSpec;
|
||||||
use codex_extension_api::parse_tool_input_schema;
|
use codex_extension_api::parse_tool_input_schema;
|
||||||
|
use codex_otel::MetricsClient;
|
||||||
use codex_tools::ResponsesApiNamespace;
|
use codex_tools::ResponsesApiNamespace;
|
||||||
use codex_tools::ResponsesApiNamespaceTool;
|
use codex_tools::ResponsesApiNamespaceTool;
|
||||||
use codex_tools::default_namespace_description;
|
use codex_tools::default_namespace_description;
|
||||||
@@ -24,21 +25,30 @@ mod list;
|
|||||||
mod read;
|
mod read;
|
||||||
mod search;
|
mod search;
|
||||||
|
|
||||||
pub(crate) fn memory_tools<B>(backend: B) -> Vec<Arc<dyn ToolExecutor<ToolCall>>>
|
pub(crate) fn memory_tools<B>(
|
||||||
|
backend: B,
|
||||||
|
metrics_client: Option<MetricsClient>,
|
||||||
|
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>>
|
||||||
where
|
where
|
||||||
B: MemoriesBackend,
|
B: MemoriesBackend,
|
||||||
{
|
{
|
||||||
vec![
|
vec![
|
||||||
Arc::new(ad_hoc_note::AddAdHocNoteTool {
|
Arc::new(ad_hoc_note::AddAdHocNoteTool {
|
||||||
backend: backend.clone(),
|
backend: backend.clone(),
|
||||||
|
metrics_client: metrics_client.clone(),
|
||||||
}),
|
}),
|
||||||
Arc::new(list::ListTool {
|
Arc::new(list::ListTool {
|
||||||
backend: backend.clone(),
|
backend: backend.clone(),
|
||||||
|
metrics_client: metrics_client.clone(),
|
||||||
}),
|
}),
|
||||||
Arc::new(read::ReadTool {
|
Arc::new(read::ReadTool {
|
||||||
backend: backend.clone(),
|
backend: backend.clone(),
|
||||||
|
metrics_client: metrics_client.clone(),
|
||||||
|
}),
|
||||||
|
Arc::new(search::SearchTool {
|
||||||
|
backend,
|
||||||
|
metrics_client,
|
||||||
}),
|
}),
|
||||||
Arc::new(search::SearchTool { backend }),
|
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ use codex_extension_api::ToolCall;
|
|||||||
use codex_extension_api::ToolExecutor;
|
use codex_extension_api::ToolExecutor;
|
||||||
use codex_extension_api::ToolName;
|
use codex_extension_api::ToolName;
|
||||||
use codex_extension_api::ToolSpec;
|
use codex_extension_api::ToolSpec;
|
||||||
|
use codex_otel::MetricsClient;
|
||||||
use schemars::JsonSchema;
|
use schemars::JsonSchema;
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
@@ -12,6 +13,9 @@ use crate::READ_TOOL_NAME;
|
|||||||
use crate::backend::MemoriesBackend;
|
use crate::backend::MemoriesBackend;
|
||||||
use crate::backend::ReadMemoryRequest;
|
use crate::backend::ReadMemoryRequest;
|
||||||
use crate::backend::ReadMemoryResponse;
|
use crate::backend::ReadMemoryResponse;
|
||||||
|
use crate::metrics::record_tool_call;
|
||||||
|
use crate::metrics::scope_from_path;
|
||||||
|
use crate::metrics::truncated_tag;
|
||||||
|
|
||||||
use super::backend_error_to_function_call;
|
use super::backend_error_to_function_call;
|
||||||
use super::memory_function_tool;
|
use super::memory_function_tool;
|
||||||
@@ -31,6 +35,7 @@ struct ReadArgs {
|
|||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
pub(super) struct ReadTool<B> {
|
pub(super) struct ReadTool<B> {
|
||||||
pub(super) backend: B,
|
pub(super) backend: B,
|
||||||
|
pub(super) metrics_client: Option<MetricsClient>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[async_trait::async_trait]
|
#[async_trait::async_trait]
|
||||||
@@ -56,15 +61,24 @@ where
|
|||||||
{
|
{
|
||||||
let backend = self.backend.clone();
|
let backend = self.backend.clone();
|
||||||
let args: ReadArgs = parse_args(&call)?;
|
let args: ReadArgs = parse_args(&call)?;
|
||||||
|
let path = args.path;
|
||||||
|
let scope = scope_from_path(path.as_str());
|
||||||
let response = backend
|
let response = backend
|
||||||
.read(ReadMemoryRequest {
|
.read(ReadMemoryRequest {
|
||||||
path: args.path,
|
path: path.clone(),
|
||||||
line_offset: args.line_offset.unwrap_or(1),
|
line_offset: args.line_offset.unwrap_or(1),
|
||||||
max_lines: args.max_lines,
|
max_lines: args.max_lines,
|
||||||
max_tokens: DEFAULT_READ_MAX_TOKENS,
|
max_tokens: DEFAULT_READ_MAX_TOKENS,
|
||||||
})
|
})
|
||||||
.await
|
.await;
|
||||||
.map_err(backend_error_to_function_call)?;
|
record_tool_call(
|
||||||
|
self.metrics_client.as_ref(),
|
||||||
|
READ_TOOL_NAME,
|
||||||
|
scope,
|
||||||
|
response.is_ok(),
|
||||||
|
truncated_tag(response.as_ref().ok().map(|response| response.truncated)),
|
||||||
|
);
|
||||||
|
let response = response.map_err(backend_error_to_function_call)?;
|
||||||
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -3,6 +3,7 @@ use codex_extension_api::ToolCall;
|
|||||||
use codex_extension_api::ToolExecutor;
|
use codex_extension_api::ToolExecutor;
|
||||||
use codex_extension_api::ToolName;
|
use codex_extension_api::ToolName;
|
||||||
use codex_extension_api::ToolSpec;
|
use codex_extension_api::ToolSpec;
|
||||||
|
use codex_otel::MetricsClient;
|
||||||
use schemars::JsonSchema;
|
use schemars::JsonSchema;
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
@@ -14,6 +15,9 @@ use crate::backend::MemoriesBackend;
|
|||||||
use crate::backend::SearchMatchMode;
|
use crate::backend::SearchMatchMode;
|
||||||
use crate::backend::SearchMemoriesRequest;
|
use crate::backend::SearchMemoriesRequest;
|
||||||
use crate::backend::SearchMemoriesResponse;
|
use crate::backend::SearchMemoriesResponse;
|
||||||
|
use crate::metrics::record_tool_call;
|
||||||
|
use crate::metrics::scope_from_optional_path;
|
||||||
|
use crate::metrics::truncated_tag;
|
||||||
|
|
||||||
use super::backend_error_to_function_call;
|
use super::backend_error_to_function_call;
|
||||||
use super::clamp_max_results;
|
use super::clamp_max_results;
|
||||||
@@ -40,6 +44,7 @@ struct SearchArgs {
|
|||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
pub(super) struct SearchTool<B> {
|
pub(super) struct SearchTool<B> {
|
||||||
pub(super) backend: B,
|
pub(super) backend: B,
|
||||||
|
pub(super) metrics_client: Option<MetricsClient>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[async_trait::async_trait]
|
#[async_trait::async_trait]
|
||||||
@@ -65,10 +70,16 @@ where
|
|||||||
{
|
{
|
||||||
let backend = self.backend.clone();
|
let backend = self.backend.clone();
|
||||||
let args: SearchArgs = parse_args(&call)?;
|
let args: SearchArgs = parse_args(&call)?;
|
||||||
let response = backend
|
let scope = scope_from_optional_path(args.path.as_deref(), "all");
|
||||||
.search(args.into_request())
|
let response = backend.search(args.into_request()).await;
|
||||||
.await
|
record_tool_call(
|
||||||
.map_err(backend_error_to_function_call)?;
|
self.metrics_client.as_ref(),
|
||||||
|
SEARCH_TOOL_NAME,
|
||||||
|
scope,
|
||||||
|
response.is_ok(),
|
||||||
|
truncated_tag(response.as_ref().ok().map(|response| response.truncated)),
|
||||||
|
);
|
||||||
|
let response = response.map_err(backend_error_to_function_call)?;
|
||||||
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
Ok(Box::new(JsonToolOutput::new(json!(response))))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user