mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
Add metrics for incremental exec catalog updates (#60177)
## What changed Count tool and namespace-instruction entry deltas in `codex.tools.exec_entry_updates`, tagged by `added`, `schema_changed`, or `removed`. Entry replacements count as `schema_changed`; full catalog declarations do not count as entry deltas. Record rendered UTF-8 batch sizes in `codex.tools.exec_update_bytes`, including hints and framing but excluding full declarations and removal notices. ## Testing Extend the namespace transition test to verify action counts, UTF-8 byte totals, and metric behavior when exec is hidden, restored, or unchanged. GitOrigin-RevId: 22070b84230296d78a67b83641cbc02157d84419
This commit is contained in:
@@ -14,6 +14,8 @@ use crate::context::DeveloperInstructions;
|
||||
use codex_code_mode::ExecDefinition;
|
||||
use codex_code_mode::PUBLIC_TOOL_NAME;
|
||||
use codex_extension_api::ExtensionMetrics;
|
||||
use codex_otel::EXEC_TOOL_ENTRY_UPDATES_METRIC;
|
||||
use codex_otel::EXEC_TOOL_UPDATE_BYTES_METRIC;
|
||||
use codex_otel::TOOL_INCREMENTAL_UPDATES_METRIC;
|
||||
use codex_otel::TOOL_SPEC_DESCRIPTION_BYTES_METRIC;
|
||||
use codex_otel::TOOL_SPEC_DESCRIPTION_BYTES_METRIC_BUCKETS;
|
||||
@@ -389,9 +391,35 @@ impl TopLevelToolsState {
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
if !changed.is_empty() {
|
||||
updates.push(
|
||||
WorldStateUpdate::fragment(render_exec_update(&changed)).standalone(),
|
||||
);
|
||||
let update = render_exec_update(&changed);
|
||||
if let Some(metrics) = self.metrics.as_deref() {
|
||||
let added = changed
|
||||
.iter()
|
||||
.filter(|entry| {
|
||||
!previous_exec
|
||||
.is_some_and(|previous| previous.contains_key(&entry.key))
|
||||
})
|
||||
.count();
|
||||
for (action, count) in [
|
||||
(ACTION_ADDED, added),
|
||||
(ACTION_SCHEMA_CHANGED, changed.len() - added),
|
||||
] {
|
||||
if count > 0 {
|
||||
metrics.counter(
|
||||
EXEC_TOOL_ENTRY_UPDATES_METRIC,
|
||||
i64::try_from(count).unwrap_or(i64::MAX),
|
||||
&[("action", action)],
|
||||
);
|
||||
}
|
||||
}
|
||||
metrics.histogram_with_boundaries(
|
||||
EXEC_TOOL_UPDATE_BYTES_METRIC,
|
||||
i64::try_from(update.0.len()).unwrap_or(i64::MAX),
|
||||
TOOL_SPEC_DESCRIPTION_BYTES_METRIC_BUCKETS.as_slice(),
|
||||
&[],
|
||||
);
|
||||
}
|
||||
updates.push(WorldStateUpdate::fragment(update).standalone());
|
||||
}
|
||||
} else if previous.is_some() {
|
||||
// Install the full catalog when enabling granular updates or restoring exec.
|
||||
@@ -422,6 +450,13 @@ impl TopLevelToolsState {
|
||||
.cloned()
|
||||
.collect::<Vec<_>>();
|
||||
if !removed.is_empty() {
|
||||
if let Some(metrics) = self.metrics.as_deref() {
|
||||
metrics.counter(
|
||||
EXEC_TOOL_ENTRY_UPDATES_METRIC,
|
||||
i64::try_from(removed.len()).unwrap_or(i64::MAX),
|
||||
&[("action", ACTION_REMOVED)],
|
||||
);
|
||||
}
|
||||
updates.push(
|
||||
WorldStateUpdate::fragment(RemovedExecDefinitions(removed)).standalone(),
|
||||
);
|
||||
|
||||
@@ -170,7 +170,7 @@ async fn snapshots_namespace_transitions() {
|
||||
"functions",
|
||||
vec![declaration("unchanged"), declaration("restored")],
|
||||
)],
|
||||
Some(telemetry),
|
||||
Some(Arc::clone(&telemetry)),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
@@ -259,6 +259,101 @@ async fn snapshots_namespace_transitions() {
|
||||
"Look up a value.".len() as f64
|
||||
)],
|
||||
);
|
||||
// Granular exec entries have separate metrics; full catalogs never count as entry deltas.
|
||||
let [exec_original, exec_updated] = [
|
||||
[
|
||||
("Tool: tools.a", "Original a."),
|
||||
("Tool: tools.b", "Original b."),
|
||||
("Namespace instructions: example", "Original guidance."),
|
||||
],
|
||||
[
|
||||
("Tool: tools.a", "Updated a: é."),
|
||||
("Tool: tools.c", "Added c."),
|
||||
("Namespace instructions: example", "Updated guidance."),
|
||||
],
|
||||
]
|
||||
.map(|entries| {
|
||||
TopLevelToolsState::new(
|
||||
/*messages*/ Default::default(),
|
||||
vec![exec_declaration("Full exec catalog.", "grammar")],
|
||||
Some(Arc::clone(&telemetry)),
|
||||
)
|
||||
.unwrap()
|
||||
.with_incremental_exec_tools(Some(Arc::new(
|
||||
entries
|
||||
.into_iter()
|
||||
.map(|(key, description)| ExecDefinition {
|
||||
key: key.to_string(),
|
||||
description: description.to_string(),
|
||||
})
|
||||
.collect(),
|
||||
)))
|
||||
});
|
||||
let initial = exec_original.render_diff(Absent).0.unwrap();
|
||||
let (updated, updates) = exec_updated.render_diff(Known(&initial));
|
||||
let rendered = merge_world_state_updates(updates);
|
||||
let ResponseItem::Message { content, .. } = &rendered[0] else {
|
||||
panic!("exec update must be a message");
|
||||
};
|
||||
let ContentItem::InputText { text } = &content[0] else {
|
||||
panic!("exec update must contain text");
|
||||
};
|
||||
let update_bytes = text.len();
|
||||
let hidden = TopLevelToolsState::new(
|
||||
/*messages*/ Default::default(),
|
||||
Vec::new(),
|
||||
Some(telemetry),
|
||||
)
|
||||
.unwrap()
|
||||
.with_incremental_exec_tools(/*exec_definitions*/ None);
|
||||
let hidden = hidden.render_diff(Known(&updated.unwrap())).0.unwrap();
|
||||
// Returning to the original catalog sends a full declaration and withdraws c.
|
||||
let restored = exec_original.render_diff(Known(&hidden)).0.unwrap();
|
||||
assert!(exec_original.render_diff(Known(&restored)).1.is_empty());
|
||||
|
||||
let snapshot = metrics.snapshot().unwrap();
|
||||
let counter = snapshot
|
||||
.scope_metrics()
|
||||
.flat_map(ScopeMetrics::metrics)
|
||||
.find(|metric| metric.name() == EXEC_TOOL_ENTRY_UPDATES_METRIC)
|
||||
.expect("exec entry update counter");
|
||||
let AggregatedMetrics::U64(MetricData::Sum(sum)) = counter.data() else {
|
||||
panic!("exec entry updates must be a counter");
|
||||
};
|
||||
assert_eq!(
|
||||
sum.data_points()
|
||||
.map(|point| {
|
||||
let tags = point
|
||||
.attributes()
|
||||
.map(|tag| (tag.key.as_str(), tag.value.to_string()))
|
||||
.collect::<BTreeMap<_, _>>();
|
||||
assert_eq!(tags.len(), 1);
|
||||
(tags["action"].clone(), point.value())
|
||||
})
|
||||
.collect::<BTreeMap<_, _>>(),
|
||||
[
|
||||
("added".to_string(), 1),
|
||||
("schema_changed".to_string(), 2),
|
||||
("removed".to_string(), 2),
|
||||
]
|
||||
.into_iter()
|
||||
.collect(),
|
||||
);
|
||||
let histogram = snapshot
|
||||
.scope_metrics()
|
||||
.flat_map(ScopeMetrics::metrics)
|
||||
.find(|metric| metric.name() == EXEC_TOOL_UPDATE_BYTES_METRIC)
|
||||
.expect("exec update byte histogram");
|
||||
let AggregatedMetrics::F64(MetricData::Histogram(histogram)) = histogram.data() else {
|
||||
panic!("exec update bytes must be a histogram");
|
||||
};
|
||||
assert_eq!(
|
||||
histogram
|
||||
.data_points()
|
||||
.map(|point| (point.attributes().count(), point.count(), point.sum()))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![(0, 1, update_bytes as f64)],
|
||||
);
|
||||
metrics.shutdown().unwrap();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user