From 5226f2fb4bcbc4aa3ec1863b6ba0bd2c4ddb0486 Mon Sep 17 00:00:00 2001 From: Alex Kira Date: Sat, 7 Nov 2026 22:25:09 +0000 Subject: [PATCH] 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 --- .../context/world_state/top_level_tools.rs | 41 +++++++- .../world_state/top_level_tools_tests.rs | 97 ++++++++++++++++++- codex-rs/otel/src/metrics/names.rs | 7 ++ 3 files changed, 141 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/context/world_state/top_level_tools.rs b/codex-rs/core/src/context/world_state/top_level_tools.rs index 772df01768..93b1b42010 100644 --- a/codex-rs/core/src/context/world_state/top_level_tools.rs +++ b/codex-rs/core/src/context/world_state/top_level_tools.rs @@ -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::>(); 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::>(); 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(), ); diff --git a/codex-rs/core/src/context/world_state/top_level_tools_tests.rs b/codex-rs/core/src/context/world_state/top_level_tools_tests.rs index b182b6226b..a0474f9b57 100644 --- a/codex-rs/core/src/context/world_state/top_level_tools_tests.rs +++ b/codex-rs/core/src/context/world_state/top_level_tools_tests.rs @@ -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::>(); + assert_eq!(tags.len(), 1); + (tags["action"].clone(), point.value()) + }) + .collect::>(), + [ + ("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![(0, 1, update_bytes as f64)], + ); metrics.shutdown().unwrap(); } diff --git a/codex-rs/otel/src/metrics/names.rs b/codex-rs/otel/src/metrics/names.rs index 999f89eee8..aa6dbf66ac 100644 --- a/codex-rs/otel/src/metrics/names.rs +++ b/codex-rs/otel/src/metrics/names.rs @@ -121,6 +121,13 @@ pub const TOOL_SPEC_DESCRIPTION_BYTES_METRIC: &str = "codex.tools.spec_descripti pub static TOOL_SPEC_DESCRIPTION_BYTES_METRIC_BUCKETS: LazyLock<[f64; 511]> = LazyLock::new(|| context_log_buckets(/*max_exponent*/ 20.0)); +/// Rendered exec entry deltas (tools and namespace instructions), by action. +/// Excludes full declarations; schema_changed includes any entry replacement. +pub const EXEC_TOOL_ENTRY_UPDATES_METRIC: &str = "codex.tools.exec_entry_updates"; +/// UTF-8 bytes per rendered added/changed exec batch, including hint and framing. +/// Excludes full declarations and removal notices. +pub const EXEC_TOOL_UPDATE_BYTES_METRIC: &str = "codex.tools.exec_update_bytes"; + pub static THREAD_SKILLS_COUNT_METRIC_BUCKETS: LazyLock<[f64; 513]> = LazyLock::new(|| std::array::from_fn(|index| index as f64));