diff --git a/codex-rs/core/src/session/extension_metrics.rs b/codex-rs/core/src/session/extension_metrics.rs new file mode 100644 index 0000000000..d4a30b7717 --- /dev/null +++ b/codex-rs/core/src/session/extension_metrics.rs @@ -0,0 +1,20 @@ +use std::sync::Arc; + +use codex_extension_api::ExtensionMetrics; +use codex_otel::SessionTelemetry; + +struct SessionTelemetryExtensionMetrics { + session_telemetry: SessionTelemetry, +} + +impl ExtensionMetrics for SessionTelemetryExtensionMetrics { + fn histogram(&self, name: &str, value: i64, tags: &[(&str, &str)]) { + self.session_telemetry.histogram(name, value, tags); + } +} + +pub(super) fn from_session_telemetry( + session_telemetry: SessionTelemetry, +) -> Arc { + Arc::new(SessionTelemetryExtensionMetrics { session_telemetry }) +} diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 122534185e..450a1a3765 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -206,6 +206,7 @@ use codex_protocol::exec_output::StreamOutput; mod code_mode_warning; mod config_lock; pub(crate) mod context_window; +mod extension_metrics; mod handlers; mod inject; mod input_queue; @@ -3352,12 +3353,17 @@ impl Session { .extension_data .get::() .is_some(); + let side_effects = if host_catalog_in_world_state { + SkillRenderSideEffects::None + } else { + SkillRenderSideEffects::ThreadStart { + session_telemetry: &self.services.session_telemetry, + } + }; let available_skills = build_available_skills( turn_context.turn_skills.snapshot.outcome(), default_skill_metadata_budget(turn_context.model_info.context_window), - SkillRenderSideEffects::ThreadStart { - session_telemetry: &self.services.session_telemetry, - }, + side_effects, ); if let Some(available_skills) = available_skills { let warning_message = available_skills.warning_message.clone(); diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index cf0d43c3b4..4f38ae9754 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1065,6 +1065,8 @@ impl Session { let session_extension_data = codex_extension_api::ExtensionData::new(session_id.to_string()); let mcp_resource_client = Arc::new(McpResourceClient::new(Arc::clone(&mcp_runtime))); + let extension_metrics = + extension_metrics::from_session_telemetry(session_telemetry.clone()); for contributor in extensions.thread_lifecycle_contributors() { contributor.on_thread_start(codex_extension_api::ThreadStartInput { config: config.as_ref(), @@ -1072,6 +1074,7 @@ impl Session { persistent_thread_state_available: state_db_ctx.is_some(), environments: session_configuration.environment_selections(), mcp_resource_client: Some(Arc::clone(&mcp_resource_client)), + extension_metrics: Some(Arc::clone(&extension_metrics)), session_store: &session_extension_data, thread_store: &thread_extension_data, }).await; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index ec3107081d..444a7d0bee 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -354,6 +354,155 @@ fn histogram_sum(resource_metrics: &ResourceMetrics, name: &str) -> u64 { } } +fn single_histogram_attributes( + resource_metrics: &ResourceMetrics, + name: &str, +) -> BTreeMap { + let metric = find_metric(resource_metrics, name); + let AggregatedMetrics::F64(data) = metric.data() else { + panic!("expected floating-point histogram"); + }; + let MetricData::Histogram(histogram) = data else { + panic!("expected histogram"); + }; + let points = histogram.data_points().collect::>(); + assert_eq!(points.len(), 1); + points[0] + .attributes() + .map(|attribute| { + ( + attribute.key.as_str().to_string(), + attribute.value.as_str().to_string(), + ) + }) + .collect() +} + +#[test] +fn extension_metrics_preserve_session_metadata_tags() { + let metrics = MetricsClient::new( + MetricsConfig::in_memory( + "test", + "codex-core", + env!("CARGO_PKG_VERSION"), + InMemoryMetricExporter::default(), + ) + .with_runtime_reader(), + ) + .expect("in-memory metrics client"); + let session_telemetry = SessionTelemetry::new( + ThreadId::new(), + "gpt-5.4", + "gpt-5.4", + /*account_id*/ None, + /*account_email*/ None, + Some(TelemetryAuthMode::Chatgpt), + "test_originator".to_string(), + /*log_user_prompts*/ false, + "tty".to_string(), + SessionSource::Cli, + ) + .with_metrics_service_name("test_service") + .with_metrics(metrics.clone()); + let extension_metrics = super::extension_metrics::from_session_telemetry(session_telemetry); + + extension_metrics.histogram( + "codex.test.extension", + /*value*/ 7, + &[ + ("component", "skills"), + ("app.version", "extension-version"), + ("auth_mode", "extension-auth"), + ("model", "extension-model"), + ("originator", "extension-originator"), + ("service_name", "extension-service"), + ("session_source", "extension-source"), + ], + ); + + let snapshot = metrics.snapshot().expect("metrics snapshot"); + let attributes = single_histogram_attributes(&snapshot, "codex.test.extension"); + assert_eq!( + attributes, + BTreeMap::from([ + ( + "app.version".to_string(), + env!("CARGO_PKG_VERSION").to_string(), + ), + ( + "auth_mode".to_string(), + TelemetryAuthMode::Chatgpt.to_string(), + ), + ("component".to_string(), "skills".to_string()), + ("model".to_string(), "gpt-5.4".to_string()), + ("originator".to_string(), "test_originator".to_string()), + ("service_name".to_string(), "test_service".to_string()), + ("session_source".to_string(), "cli".to_string()), + ]) + ); +} + +#[tokio::test] +async fn world_state_extension_metrics_follow_turn_model_switch() { + struct WorldStateMetricsRecorder; + + impl codex_extension_api::ContextContributor for WorldStateMetricsRecorder { + fn contribute_world_state<'a>( + &'a self, + input: codex_extension_api::WorldStateContributionInput<'a>, + ) -> codex_extension_api::ExtensionFuture< + 'a, + Vec, + > { + Box::pin(async move { + input + .extension_metrics + .expect("turn metrics should be available") + .histogram("codex.test.extension.turn", /*value*/ 1, &[]); + Vec::new() + }) + } + } + + let metrics = MetricsClient::new( + MetricsConfig::in_memory( + "test", + "codex-core", + env!("CARGO_PKG_VERSION"), + InMemoryMetricExporter::default(), + ) + .with_runtime_reader(), + ) + .expect("in-memory metrics client"); + let (mut session, mut turn_context) = make_session_and_context().await; + turn_context.session_telemetry = turn_context + .session_telemetry + .clone() + .with_metrics(metrics.clone()); + let next_model = if turn_context.model_info.slug == "gpt-5.4" { + "gpt-5.2" + } else { + "gpt-5.4" + }; + let turn_context = Arc::new( + turn_context + .with_model(next_model.to_string(), &session.services.models_manager) + .await, + ); + let mut builder = codex_extension_api::ExtensionRegistryBuilder::::new(); + builder.prompt_contributor(Arc::new(WorldStateMetricsRecorder)); + session.services.extensions = Arc::new(builder.build()); + + let _world_state = build_world_state_from_turn_context(&session, &turn_context).await; + + let snapshot = metrics.snapshot().expect("metrics snapshot"); + let attributes = single_histogram_attributes(&snapshot, "codex.test.extension.turn"); + assert_eq!( + attributes.get("model").map(String::as_str), + Some(next_model) + ); +} + fn skill_message(text: &str) -> ResponseItem { ResponseItem::Message { id: None, diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 5d82afc620..cb38541de2 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -760,12 +760,15 @@ async fn build_extension_turn_input_items( user_input: user_input.to_vec(), environments, }; + let extension_metrics = + super::extension_metrics::from_session_telemetry(turn_context.session_telemetry.clone()); let mut items = Vec::new(); for contributor in contributors { let contributed_fragments = contributor .contribute( input.clone(), + Some(Arc::clone(&extension_metrics)), &sess.services.session_extension_data, &sess.services.thread_extension_data, turn_context.extension_data.as_ref(), diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index 8d2b63d041..c5c4556dbd 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use super::session::Session; use super::step_context::StepContext; use crate::connectors; @@ -165,6 +167,9 @@ impl Session { .iter() .map(|root| root.selected_root().clone()) .collect::>(); + let extension_metrics = super::extension_metrics::from_session_telemetry( + turn_context.session_telemetry.clone(), + ); for contributor in self.services.extensions.context_contributors() { for section in contributor .contribute_world_state(WorldStateContributionInput { @@ -175,6 +180,7 @@ impl Session { executor_capability_discovery: step_context .executor_capability_discovery .as_deref(), + extension_metrics: Some(Arc::clone(&extension_metrics)), session_store: &self.services.session_extension_data, thread_store: &self.services.thread_extension_data, turn_store: turn_context.extension_data.as_ref(), diff --git a/codex-rs/ext/extension-api/src/capabilities/metrics.rs b/codex-rs/ext/extension-api/src/capabilities/metrics.rs new file mode 100644 index 0000000000..756519ed33 --- /dev/null +++ b/codex-rs/ext/extension-api/src/capabilities/metrics.rs @@ -0,0 +1,8 @@ +/// Host-provided metrics capability for extension-owned behavior. +/// +/// Implementations are expected to attach the host's session attribution before +/// forwarding samples to the configured metrics backend. +pub trait ExtensionMetrics: Send + Sync { + /// Records one histogram sample with optional extension-provided tags. + fn histogram(&self, name: &str, value: i64, tags: &[(&str, &str)]); +} diff --git a/codex-rs/ext/extension-api/src/capabilities/mod.rs b/codex-rs/ext/extension-api/src/capabilities/mod.rs index 39340c255c..ede4ddebe3 100644 --- a/codex-rs/ext/extension-api/src/capabilities/mod.rs +++ b/codex-rs/ext/extension-api/src/capabilities/mod.rs @@ -1,5 +1,6 @@ mod agent; mod events; +mod metrics; mod response_items; pub use agent::AgentSpawnFuture; @@ -7,6 +8,7 @@ pub use agent::AgentSpawner; pub use events::ExtensionEventSink; pub use events::ExtensionWarning; pub use events::NoopExtensionEventSink; +pub use metrics::ExtensionMetrics; pub use response_items::NoopResponseItemInjector; pub use response_items::ResponseItemInjectionFuture; pub use response_items::ResponseItemInjector; diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index 8bcaaf3d4c..14e0aa32dc 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -10,6 +10,7 @@ use codex_tools::ToolCall; use codex_tools::ToolExecutor; use crate::ExtensionData; +use crate::ExtensionMetrics; mod context; mod mcp; @@ -205,10 +206,12 @@ pub trait TurnLifecycleContributor: Send + Sync { /// host-specific dependencies belong on the extension value installed by the /// host, not in this input. pub trait TurnInputContributor: Send + Sync { - /// Returns additional contextual fragments for one submitted turn. + /// Returns additional contextual fragments for one submitted turn. The optional metrics + /// capability is bound to the effective model for that turn. fn contribute<'a>( &'a self, input: TurnInputContext, + extension_metrics: Option>, session_store: &'a ExtensionData, thread_store: &'a ExtensionData, turn_store: &'a ExtensionData, diff --git a/codex-rs/ext/extension-api/src/contributors/thread_lifecycle.rs b/codex-rs/ext/extension-api/src/contributors/thread_lifecycle.rs index 4fa6e6721c..bef595ee19 100644 --- a/codex-rs/ext/extension-api/src/contributors/thread_lifecycle.rs +++ b/codex-rs/ext/extension-api/src/contributors/thread_lifecycle.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use crate::ExtensionData; +use crate::ExtensionMetrics; use codex_mcp::McpResourceClient; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::TurnEnvironmentSelection; @@ -25,6 +26,8 @@ pub struct ThreadStartInput<'a, C> { pub environments: &'a [TurnEnvironmentSelection], /// MCP resource access supplied by the host for this session. pub mcp_resource_client: Option>, + /// Session-attributed metrics supplied by the host. + pub extension_metrics: Option>, /// Store scoped to the host session runtime. pub session_store: &'a ExtensionData, /// Store scoped to this thread runtime. diff --git a/codex-rs/ext/extension-api/src/contributors/world_state.rs b/codex-rs/ext/extension-api/src/contributors/world_state.rs index 75fae6d4a8..3aba6b9418 100644 --- a/codex-rs/ext/extension-api/src/contributors/world_state.rs +++ b/codex-rs/ext/extension-api/src/contributors/world_state.rs @@ -7,6 +7,7 @@ use codex_protocol::protocol::TurnEnvironmentSelection; use serde_json::Value; use crate::ExtensionData; +use crate::ExtensionMetrics; /// Host state available while an extension contributes one sampling step's World State. pub struct WorldStateContributionInput<'a> { @@ -17,6 +18,8 @@ pub struct WorldStateContributionInput<'a> { pub ready_selected_capability_roots: &'a [SelectedCapabilityRoot], /// Executor-materialized capability files shared by all consumers in this exact step. pub executor_capability_discovery: Option<&'a ExecutorCapabilityDiscoverySnapshot>, + /// Metrics bound to the effective model for this turn. + pub extension_metrics: Option>, pub session_store: &'a ExtensionData, pub thread_store: &'a ExtensionData, pub turn_store: &'a ExtensionData, diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index cdfb0b4cbd..8cae0e752b 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -7,6 +7,7 @@ mod user_instructions; pub use capabilities::AgentSpawnFuture; pub use capabilities::AgentSpawner; pub use capabilities::ExtensionEventSink; +pub use capabilities::ExtensionMetrics; pub use capabilities::ExtensionWarning; pub use capabilities::NoopExtensionEventSink; pub use capabilities::NoopResponseItemInjector; diff --git a/codex-rs/ext/extension-api/tests/registry.rs b/codex-rs/ext/extension-api/tests/registry.rs index 31a8805b78..6761a239f5 100644 --- a/codex-rs/ext/extension-api/tests/registry.rs +++ b/codex-rs/ext/extension-api/tests/registry.rs @@ -10,6 +10,7 @@ use codex_extension_api::ContextualUserFragment; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionFuture; +use codex_extension_api::ExtensionMetrics; use codex_extension_api::ExtensionRegistryBuilder; use codex_extension_api::ExtensionWarning; use codex_extension_api::PromptFragment; @@ -61,6 +62,7 @@ impl TurnInputContributor for AllContributors { fn contribute<'a>( &'a self, input: TurnInputContext, + _extension_metrics: Option>, _session_store: &'a ExtensionData, _thread_store: &'a ExtensionData, _turn_store: &'a ExtensionData, diff --git a/codex-rs/ext/goal/tests/goal_extension_backend.rs b/codex-rs/ext/goal/tests/goal_extension_backend.rs index de1615bb64..34fd5a7ade 100644 --- a/codex-rs/ext/goal/tests/goal_extension_backend.rs +++ b/codex-rs/ext/goal/tests/goal_extension_backend.rs @@ -1137,6 +1137,7 @@ async fn installed_tools_with_start( persistent_thread_state_available, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -1191,6 +1192,7 @@ impl GoalExtensionHarness { persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index 6c58b36c7e..cec0333d71 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -11,6 +11,7 @@ use codex_extension_api::ContextualUserFragment; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionFuture; +use codex_extension_api::ExtensionMetrics; use codex_extension_api::ExtensionRegistryBuilder; use codex_extension_api::ExtensionWarning; use codex_extension_api::PromptFragment; @@ -49,6 +50,8 @@ use crate::render::capped_skill_metadata_budget; use crate::render::render_available_skills; use crate::render::truncate_main_prompt_contents; use crate::render::truncate_utf8_to_bytes; +use crate::render_observability::CatalogSurface; +use crate::render_observability::record_catalog_render; use crate::selection::collect_explicit_skill_mentions; use crate::shadow_selection_experiment::ShadowSelectionExperiment; use crate::sources::SkillProviders; @@ -76,6 +79,8 @@ struct RenderedCatalog { } fn render_catalog( + extension_metrics: Option<&dyn ExtensionMetrics>, + catalog_surface: CatalogSurface, catalog: &SkillCatalog, include_skills_usage_instructions: bool, policy: SkillCatalogRenderPolicy, @@ -84,6 +89,7 @@ fn render_catalog( let Some(rendered) = render_available_skills(catalog, policy, budget) else { return RenderedCatalog::default(); }; + record_catalog_render(extension_metrics, catalog_surface, budget, &rendered.report); let warning_message = rendered.report.warning_message(); let fragment = rendered.into_fragment(include_skills_usage_instructions); RenderedCatalog { @@ -100,6 +106,7 @@ where Box::pin(async move { input.session_store.insert(SkillsSessionState { mcp_resources: input.mcp_resource_client.clone(), + extension_metrics: input.extension_metrics.clone(), }); let orchestrator_skills_available = !input .environments @@ -178,7 +185,12 @@ where let include_usage = thread_store .get::() .is_some_and(|model_info| model_info.include_skills_usage_instructions); + let extension_metrics = session_store + .get::() + .and_then(|state| state.extension_metrics.clone()); let rendered = render_catalog( + extension_metrics.as_deref(), + CatalogSurface::ThreadContext, &catalog, include_usage, SkillCatalogRenderPolicy::ExtensionCompatible, @@ -234,8 +246,11 @@ where .as_deref() .and_then(ModelInfo::resolved_context_window); let metadata_budget = capped_skill_metadata_budget(context_window); + let extension_metrics = input.extension_metrics.clone(); let rendered = if config.include_instructions { render_catalog( + extension_metrics.as_deref(), + CatalogSurface::ExecutorWorldState, &catalog, include_usage, SkillCatalogRenderPolicy::ExtensionCompatible, @@ -266,6 +281,7 @@ where config.include_instructions, include_usage, metadata_budget, + extension_metrics, )); } sections @@ -346,6 +362,7 @@ where fn contribute<'a>( &'a self, input: TurnInputContext, + extension_metrics: Option>, session_store: &'a ExtensionData, thread_store: &'a ExtensionData, turn_store: &'a ExtensionData, @@ -416,6 +433,8 @@ where .and_then(ModelInfo::resolved_context_window); let metadata_budget = capped_skill_metadata_budget(context_window); let rendered = render_catalog( + extension_metrics.as_deref(), + CatalogSurface::TurnInput, &turn_catalog, include_usage, SkillCatalogRenderPolicy::ExtensionCompatible, diff --git a/codex-rs/ext/skills/src/lib.rs b/codex-rs/ext/skills/src/lib.rs index 3e129db169..65955c8d85 100644 --- a/codex-rs/ext/skills/src/lib.rs +++ b/codex-rs/ext/skills/src/lib.rs @@ -5,6 +5,7 @@ mod extension; mod fragments; pub mod provider; mod render; +mod render_observability; mod selection; mod shadow_selection_experiment; mod sources; diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index a16492a386..f17f56e618 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -121,7 +121,7 @@ impl SkillRenderReport { .then(|| SKILL_DESCRIPTION_TRUNCATED_WARNING.to_string()) } - fn average_truncated_description_chars(&self) -> usize { + pub(crate) fn average_truncated_description_chars(&self) -> usize { if self.total_count == 0 || self.truncated_description_chars == 0 { return 0; } @@ -158,7 +158,7 @@ fn metadata_line_cost(budget: SkillMetadataBudget, line: &str) -> usize { } impl SkillMetadataBudget { - fn limit(self) -> usize { + pub(crate) fn limit(self) -> usize { match self { Self::Tokens(limit) | Self::Characters(limit) => limit, } diff --git a/codex-rs/ext/skills/src/render_observability.rs b/codex-rs/ext/skills/src/render_observability.rs new file mode 100644 index 0000000000..224797a82b --- /dev/null +++ b/codex-rs/ext/skills/src/render_observability.rs @@ -0,0 +1,94 @@ +use codex_extension_api::ExtensionMetrics; +use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC; +use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC; + +use crate::render::SkillMetadataBudget; +use crate::render::SkillRenderReport; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum CatalogSurface { + ThreadContext, + ExecutorWorldState, + HostWorldState, + TurnInput, +} + +impl CatalogSurface { + fn as_str(self) -> &'static str { + match self { + Self::ThreadContext => "thread_context", + Self::ExecutorWorldState => "executor_world_state", + Self::HostWorldState => "host_world_state", + Self::TurnInput => "turn_input", + } + } +} + +pub(crate) fn record_catalog_render( + extension_metrics: Option<&dyn ExtensionMetrics>, + catalog_surface: CatalogSurface, + budget: SkillMetadataBudget, + report: &SkillRenderReport, +) { + record_catalog_metrics( + extension_metrics, + catalog_surface, + report.total_count, + report.included_count, + report.omitted_count, + report.truncated_description_chars, + ); + + if report.omitted_count > 0 || report.truncated_description_chars > 0 { + tracing::info!( + budget_limit = budget.limit(), + total_skills = report.total_count, + included_skills = report.included_count, + omitted_skills = report.omitted_count, + truncated_description_chars_per_skill = report.average_truncated_description_chars(), + truncated_skill_descriptions = report.truncated_description_count, + "truncated skill metadata to fit skills context budget" + ); + } +} + +pub(crate) fn record_catalog_metrics( + extension_metrics: Option<&dyn ExtensionMetrics>, + catalog_surface: CatalogSurface, + total_count: usize, + included_count: usize, + omitted_count: usize, + truncated_description_chars: usize, +) { + let Some(extension_metrics) = extension_metrics else { + return; + }; + let samples = [ + ( + THREAD_SKILLS_ENABLED_TOTAL_METRIC, + i64::try_from(total_count).unwrap_or(i64::MAX), + ), + ( + THREAD_SKILLS_KEPT_TOTAL_METRIC, + i64::try_from(included_count).unwrap_or(i64::MAX), + ), + ( + THREAD_SKILLS_TRUNCATED_METRIC, + if omitted_count > 0 { 1 } else { 0 }, + ), + ( + THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC, + i64::try_from(truncated_description_chars).unwrap_or(i64::MAX), + ), + ]; + let tags = [("catalog_surface", catalog_surface.as_str())]; + for (name, value) in samples { + extension_metrics.histogram(name, value, &tags); + } +} + +#[cfg(test)] +#[path = "render_observability_tests.rs"] +mod tests; diff --git a/codex-rs/ext/skills/src/render_observability_tests.rs b/codex-rs/ext/skills/src/render_observability_tests.rs new file mode 100644 index 0000000000..8e8f7383f7 --- /dev/null +++ b/codex-rs/ext/skills/src/render_observability_tests.rs @@ -0,0 +1,82 @@ +use std::sync::Mutex; + +use codex_extension_api::ExtensionMetrics; +use pretty_assertions::assert_eq; + +use super::*; + +#[derive(Debug, Eq, PartialEq)] +struct RecordedHistogram { + name: String, + value: i64, + tags: Vec<(String, String)>, +} + +#[derive(Default)] +struct RecordingMetrics { + samples: Mutex>, +} + +impl ExtensionMetrics for RecordingMetrics { + fn histogram(&self, name: &str, value: i64, tags: &[(&str, &str)]) { + self.samples + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push(RecordedHistogram { + name: name.to_string(), + value, + tags: tags + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect(), + }); + } +} + +#[test] +fn records_core_equivalent_catalog_render_metrics_with_surface() { + let metrics = RecordingMetrics::default(); + let report = SkillRenderReport { + total_count: 5, + included_count: 3, + omitted_count: 2, + truncated_description_chars: 700, + truncated_description_count: 4, + }; + + record_catalog_render( + Some(&metrics), + CatalogSurface::TurnInput, + SkillMetadataBudget::Tokens(400), + &report, + ); + + assert_eq!( + *metrics + .samples + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner), + vec![ + RecordedHistogram { + name: THREAD_SKILLS_ENABLED_TOTAL_METRIC.to_string(), + value: 5, + tags: vec![("catalog_surface".to_string(), "turn_input".to_string())], + }, + RecordedHistogram { + name: THREAD_SKILLS_KEPT_TOTAL_METRIC.to_string(), + value: 3, + tags: vec![("catalog_surface".to_string(), "turn_input".to_string())], + }, + RecordedHistogram { + name: THREAD_SKILLS_TRUNCATED_METRIC.to_string(), + value: 1, + tags: vec![("catalog_surface".to_string(), "turn_input".to_string())], + }, + RecordedHistogram { + name: THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC.to_string(), + value: 700, + tags: vec![("catalog_surface".to_string(), "turn_input".to_string())], + }, + ] + ); +} diff --git a/codex-rs/ext/skills/src/state.rs b/codex-rs/ext/skills/src/state.rs index 8e39ad5395..6ba735c158 100644 --- a/codex-rs/ext/skills/src/state.rs +++ b/codex-rs/ext/skills/src/state.rs @@ -4,6 +4,7 @@ use std::future::Future; use std::sync::Arc; use std::sync::Mutex; +use codex_extension_api::ExtensionMetrics; use codex_mcp::McpResourceClient; use codex_mcp::McpResourceClientCacheKey; use codex_protocol::capabilities::SelectedCapabilityRoot; @@ -29,6 +30,7 @@ const MAX_CACHED_ORCHESTRATOR_CONTENT_BYTES: usize = 8 * 1024 * 1024; pub(crate) struct SkillsSessionState { pub(crate) mcp_resources: Option>, + pub(crate) extension_metrics: Option>, } pub(crate) struct SkillsThreadState { diff --git a/codex-rs/ext/skills/src/world_state.rs b/codex-rs/ext/skills/src/world_state.rs index 274f527198..4bb6bc88db 100644 --- a/codex-rs/ext/skills/src/world_state.rs +++ b/codex-rs/ext/skills/src/world_state.rs @@ -1,7 +1,10 @@ +use std::sync::Arc; + use codex_core_skills::HostSkillsSnapshot; use codex_core_skills::build_available_skills; use codex_core_skills::render::SkillRenderSideEffects; use codex_extension_api::ContextualUserFragment; +use codex_extension_api::ExtensionMetrics; use codex_extension_api::PreviousWorldStateSection; use codex_extension_api::RenderedWorldStateFragment; use codex_extension_api::WorldStateSectionContribution; @@ -11,6 +14,8 @@ use serde_json::json; use crate::fragments::AvailableSkillsInstructions; use crate::render::SkillMetadataBudget; +use crate::render_observability::CatalogSurface; +use crate::render_observability::record_catalog_metrics; pub(crate) const SKILLS_WORLD_STATE_ID: &str = "skills"; pub(crate) const HOST_SKILLS_WORLD_STATE_ID: &str = "host_skills"; @@ -78,6 +83,7 @@ pub(crate) fn host_skills_world_state_section( include_instructions: bool, include_skills_usage_instructions: bool, metadata_budget: SkillMetadataBudget, + extension_metrics: Option>, ) -> WorldStateSectionContribution { let outcome = host_snapshot.outcome(); let metadata_budget = match metadata_budget { @@ -91,6 +97,7 @@ pub(crate) fn host_skills_world_state_section( } else { None }; + let render_report = available.as_ref().map(|available| available.report.clone()); let body = available.map(|available| { AvailableSkillsInstructions::from_available_skills( available, @@ -121,6 +128,16 @@ pub(crate) fn host_skills_world_state_section( } } + if let Some(report) = &render_report { + record_catalog_metrics( + extension_metrics.as_deref(), + CatalogSurface::HostWorldState, + report.total_count, + report.included_count, + report.omitted_count, + report.truncated_description_chars, + ); + } let body = match body.as_deref() { Some(body) => body, None if previous_is_absent => return None, diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index c3f92b614a..5bf255e527 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -15,6 +15,7 @@ use codex_exec_server::LOCAL_FS; use codex_extension_api::ConversationHistory; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionEventSink; +use codex_extension_api::ExtensionMetrics; use codex_extension_api::ExtensionRegistryBuilder; use codex_extension_api::ExtensionWarning; use codex_extension_api::NoopTurnItemEmitter; @@ -25,6 +26,10 @@ use codex_extension_api::ToolPayload; use codex_extension_api::TurnInputContext; use codex_extension_api::WorldStateContributionInput; use codex_models_manager::model_info::model_info_from_slug; +use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC; +use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::protocol::Event; @@ -91,6 +96,7 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult { persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -126,6 +132,7 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult { }], environments: Vec::new(), }, + /*extension_metrics*/ None, &session_store, &thread_store, &turn_store, @@ -154,6 +161,142 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult { Ok(()) } +#[tokio::test] +async fn host_world_state_records_catalog_metrics_on_publish_and_change() -> TestResult { + let mut builder = ExtensionRegistryBuilder::new(); + install(&mut builder, skills_extension_config); + let registry = builder.build(); + let startup_metrics = Arc::new(RecordingMetrics::default()); + let turn_metrics = Arc::new(RecordingMetrics::default()); + let session_store = ExtensionData::new("session"); + let thread_store = ExtensionData::new("thread"); + let config = default_config(); + registry.thread_lifecycle_contributors()[0] + .on_thread_start(ThreadStartInput { + config: &config, + session_source: &SessionSource::Cli, + persistent_thread_state_available: true, + environments: &[], + mcp_resource_client: None, + extension_metrics: Some(startup_metrics.clone()), + session_store: &session_store, + thread_store: &thread_store, + }) + .await; + + let skill_path = AbsolutePathBuf::try_from(test_codex_home().join("skills/demo/SKILL.md"))?; + let mut outcome = SkillLoadOutcome::default(); + outcome.skills.push(SkillMetadata { + name: "demo".to_string(), + description: "Demo skill.".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: skill_path, + scope: SkillScope::User, + plugin_id: None, + }); + let turn_store = ExtensionData::new("turn-1"); + turn_store.insert(HostSkillsSnapshot::new(Arc::new(outcome.clone()))); + + let sections = registry.context_contributors()[0] + .contribute_world_state(WorldStateContributionInput { + thread_id: codex_protocol::ThreadId::new(), + turn_id: "turn-1", + environments: &[], + ready_selected_capability_roots: &[], + executor_capability_discovery: None, + extension_metrics: Some(turn_metrics.clone()), + session_store: &session_store, + thread_store: &thread_store, + turn_store: &turn_store, + }) + .await; + + assert_eq!(sections.len(), 2); + let host_section = §ions[1]; + let published_snapshot = host_section.snapshot().clone(); + assert!( + host_section + .render_diff(PreviousWorldStateSection::Absent) + .is_some() + ); + let expected_samples = |count| { + vec![ + (THREAD_SKILLS_ENABLED_TOTAL_METRIC.to_string(), count), + (THREAD_SKILLS_KEPT_TOTAL_METRIC.to_string(), count), + (THREAD_SKILLS_TRUNCATED_METRIC.to_string(), 0), + ( + THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC.to_string(), + 0, + ), + ] + }; + assert!(startup_metrics.samples().is_empty()); + assert_eq!(turn_metrics.samples(), expected_samples(1)); + + let sections = registry.context_contributors()[0] + .contribute_world_state(WorldStateContributionInput { + thread_id: codex_protocol::ThreadId::new(), + turn_id: "turn-1", + environments: &[], + ready_selected_capability_roots: &[], + executor_capability_discovery: None, + extension_metrics: Some(turn_metrics.clone()), + session_store: &session_store, + thread_store: &thread_store, + turn_store: &turn_store, + }) + .await; + assert!( + sections[1] + .render_diff(PreviousWorldStateSection::Known(&published_snapshot)) + .is_none() + ); + assert!(startup_metrics.samples().is_empty()); + assert_eq!(turn_metrics.samples(), expected_samples(1)); + + let second_skill_path = + AbsolutePathBuf::try_from(test_codex_home().join("skills/other/SKILL.md"))?; + outcome.skills.push(SkillMetadata { + name: "other".to_string(), + description: "Other skill.".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: second_skill_path, + scope: SkillScope::User, + plugin_id: None, + }); + turn_store.insert(HostSkillsSnapshot::new(Arc::new(outcome))); + let sections = registry.context_contributors()[0] + .contribute_world_state(WorldStateContributionInput { + thread_id: codex_protocol::ThreadId::new(), + turn_id: "turn-1", + environments: &[], + ready_selected_capability_roots: &[], + executor_capability_discovery: None, + extension_metrics: Some(turn_metrics.clone()), + session_store: &session_store, + thread_store: &thread_store, + turn_store: &turn_store, + }) + .await; + assert!( + sections[1] + .render_diff(PreviousWorldStateSection::Known(&published_snapshot)) + .is_some() + ); + let mut published_and_changed_samples = expected_samples(1); + published_and_changed_samples.extend(expected_samples(2)); + assert!(startup_metrics.samples().is_empty()); + assert_eq!(turn_metrics.samples(), published_and_changed_samples); + + Ok(()) +} + #[tokio::test] async fn selected_executor_catalog_follows_step_availability_and_reuses_its_cache() -> TestResult { let read_requests = Arc::new(Mutex::new(Vec::new())); @@ -195,6 +338,7 @@ async fn selected_executor_catalog_follows_step_availability_and_reuses_its_cach persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -218,6 +362,7 @@ async fn selected_executor_catalog_follows_step_availability_and_reuses_its_cach environments: std::slice::from_ref(&turn_environment), ready_selected_capability_roots: &selected_roots, executor_capability_discovery: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, turn_store: &turn_store, @@ -245,6 +390,7 @@ async fn selected_executor_catalog_follows_step_availability_and_reuses_its_cach }], environments: Vec::new(), }, + /*extension_metrics*/ None, &session_store, &thread_store, &turn_store, @@ -271,6 +417,7 @@ async fn selected_executor_catalog_follows_step_availability_and_reuses_its_cach environments: &[], ready_selected_capability_roots: &[], executor_capability_discovery: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, turn_store: &unavailable_turn_store, @@ -294,6 +441,7 @@ async fn selected_executor_catalog_follows_step_availability_and_reuses_its_cach environments: &[turn_environment], ready_selected_capability_roots: &selected_roots, executor_capability_discovery: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, turn_store: &restored_turn_store, @@ -322,6 +470,7 @@ async fn selected_executor_catalog_follows_step_availability_and_reuses_its_cach environments: &[], ready_selected_capability_roots: &selected_roots, executor_capability_discovery: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, turn_store: &listing_disabled_turn_store, @@ -384,6 +533,7 @@ async fn default_context_truncates_catalog_descriptions() -> TestResult { persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -441,6 +591,7 @@ async fn moderate_budget_pressure_keeps_every_catalog_entry() -> TestResult { persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -525,6 +676,7 @@ async fn extreme_budget_pressure_removes_descriptions_before_omitting_entries() persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -604,6 +756,7 @@ async fn skills_list_only_returns_model_visible_bounded_metadata() -> TestResult persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -681,6 +834,7 @@ async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult { persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -709,6 +863,7 @@ async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult { }], environments: Vec::new(), }, + /*extension_metrics*/ None, &session_store, &thread_store, &ExtensionData::new(turn_id), @@ -779,6 +934,7 @@ async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> Te persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -796,6 +952,7 @@ async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> Te }], ready_selected_capability_roots: &selected_roots, executor_capability_discovery: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, turn_store: &turn_store, @@ -811,6 +968,7 @@ async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> Te }], environments: Vec::new(), }, + /*extension_metrics*/ None, &session_store, &thread_store, &turn_store, @@ -888,6 +1046,7 @@ async fn model_context_window_scales_executor_catalog_but_not_thread_catalog() - persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -922,6 +1081,7 @@ async fn model_context_window_scales_executor_catalog_but_not_thread_catalog() - environments: &[], ready_selected_capability_roots: &selected_roots, executor_capability_discovery: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, turn_store: &turn_store, @@ -935,6 +1095,7 @@ async fn model_context_window_scales_executor_catalog_but_not_thread_catalog() - environments: &[], ready_selected_capability_roots: &selected_roots, executor_capability_discovery: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, turn_store: &turn_store, @@ -997,6 +1158,7 @@ async fn executor_catalog_emits_at_most_four_warnings() -> TestResult { persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -1017,6 +1179,7 @@ async fn executor_catalog_emits_at_most_four_warnings() -> TestResult { environments: &[], ready_selected_capability_roots: &selected_roots, executor_capability_discovery: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, turn_store: &turn_store, @@ -1029,6 +1192,7 @@ async fn executor_catalog_emits_at_most_four_warnings() -> TestResult { user_input: Vec::new(), environments: Vec::new(), }, + /*extension_metrics*/ None, &session_store, &thread_store, &turn_store, @@ -1096,6 +1260,7 @@ async fn host_catalog_compacts_shared_paths_under_budget_pressure() -> TestResul persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -1116,6 +1281,7 @@ async fn host_catalog_compacts_shared_paths_under_budget_pressure() -> TestResul }], environments: Vec::new(), }, + /*extension_metrics*/ None, &session_store, &thread_store, &turn_store, @@ -1180,6 +1346,7 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult { persistent_thread_state_available: true, environments: &[], mcp_resource_client: None, + extension_metrics: None, session_store: &session_store, thread_store: &thread_store, }) @@ -1195,6 +1362,7 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult { }], environments: Vec::new(), }, + /*extension_metrics*/ None, &session_store, &thread_store, &ExtensionData::new("turn-1"), @@ -1252,6 +1420,29 @@ impl ExtensionEventSink for ChannelEventSink { } } +#[derive(Default)] +struct RecordingMetrics { + samples: Mutex>, +} + +impl RecordingMetrics { + fn samples(&self) -> Vec<(String, i64)> { + self.samples + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + } +} + +impl ExtensionMetrics for RecordingMetrics { + fn histogram(&self, name: &str, value: i64, _tags: &[(&str, &str)]) { + self.samples + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push((name.to_string(), value)); + } +} + impl SkillProvider for StaticSkillProvider { fn list(&self, _query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> { let list_call = self diff --git a/codex-rs/otel/src/events/session_telemetry.rs b/codex-rs/otel/src/events/session_telemetry.rs index c74f29008b..0e5c94bae1 100644 --- a/codex-rs/otel/src/events/session_telemetry.rs +++ b/codex-rs/otel/src/events/session_telemetry.rs @@ -373,8 +373,8 @@ impl SessionTelemetry { &'a self, tags: &'a [(&'a str, &'a str)], ) -> MetricsResult> { - let mut merged = self.metadata_tag_refs()?; - merged.extend(tags.iter().copied()); + let mut merged = tags.to_vec(); + merged.extend(self.metadata_tag_refs()?); Ok(merged) }