diff --git a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs index 75374ccb9e..0bfbe618df 100644 --- a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs +++ b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeMap; use std::io::Write; use std::path::Path; use std::sync::Arc; @@ -179,7 +180,8 @@ struct MockResponsesState { allow_guardian_review: Notify, classification_completed: Notify, truncation_recorded: Notify, - sync_cost_recorded: Notify, + context_metric_bounds: Mutex>>, + context_metrics_recorded: Notify, luna_score: f64, invalid_classification: bool, review_outcome: ReviewOutcome, @@ -723,12 +725,22 @@ async fn guardian_v2_routes_scoped_tool_approvals( for metric in payload["resourceMetrics"].as_array().into_iter().flatten() .flat_map(|resource| resource["scopeMetrics"].as_array().into_iter().flatten()) .flat_map(|scope| scope["metrics"].as_array().into_iter().flatten()) - .filter(|metric| metric["name"] == "codex.guardian.context.request_tokens") + .filter(|metric| matches!(metric["name"].as_str(), Some("codex.guardian.context.request_tokens" | "codex.guardian.context.section_cost"))) { + let name = metric["name"].as_str().expect("context metric name"); for point in metric["histogram"]["dataPoints"].as_array().into_iter().flatten() { let attributes = point["attributes"].as_array().expect("metric attributes"); - if attributes.iter().any(|attr| attr["key"] == "target" && attr["value"]["stringValue"] == "sync") { - state.sync_cost_recorded.notify_one(); + for target in ["sync", "async"] { + if attributes.iter().any(|attr| attr["key"] == "target" && attr["value"]["stringValue"] == target) { + let mut bounds = state.context_metric_bounds.lock().expect("context metric bounds"); + bounds.insert( + (name.to_owned(), target.to_owned()), + point["explicitBounds"].as_array().and_then(|bounds| bounds.last()).and_then(Value::as_f64), + ); + if bounds.len() == 4 { + state.context_metrics_recorded.notify_one(); + } + } } } } @@ -1342,9 +1354,24 @@ async fn guardian_v2_routes_scoped_tool_approvals( } } if mixed_evidence { - timeout(TIMEOUT, responses_state.sync_cost_recorded.notified()) + timeout(TIMEOUT, responses_state.context_metrics_recorded.notified()) .await - .expect("sync request cost should be exported"); + .expect("sync and async request/section metrics should all be exported"); + let bounds = responses_state + .context_metric_bounds + .lock() + .expect("context metric bounds"); + for ((metric, target), bound) in bounds.iter() { + assert_eq!( + *bound, + Some(if metric == "codex.guardian.context.request_tokens" { + 2_000_000.0 + } else { + 16_777_216.0 + }), + "{metric} ({target}) must export its context metric buckets" + ); + } } if lifecycle.has_user_answer() { let reviews = responses_state diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index 43f5b83214..52ab173d3e 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -216,15 +216,19 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( let context = sections.compose(presentation, transcript)?; for (section, cost) in context.section_costs() { for (measurement, value) in cost.measurements() { - session.services.session_telemetry.histogram( - codex_guardian_context::SECTION_COST_METRIC, - i64::try_from(value).unwrap_or(i64::MAX), - &[ - ("target", "sync"), - ("section", section), - ("measurement", measurement), - ], - ); + session + .services + .session_telemetry + .histogram_with_boundaries( + codex_guardian_context::SECTION_COST_METRIC, + i64::try_from(value).unwrap_or(i64::MAX), + codex_guardian_context::SECTION_COST_BOUNDARIES, + &[ + ("target", "sync"), + ("section", section), + ("measurement", measurement), + ], + ); } } let items = context.into_user_inputs()?; diff --git a/codex-rs/core/src/guardian/request_budget.rs b/codex-rs/core/src/guardian/request_budget.rs index 50b356a435..ad53549845 100644 --- a/codex-rs/core/src/guardian/request_budget.rs +++ b/codex-rs/core/src/guardian/request_budget.rs @@ -1,6 +1,7 @@ //! Measures the complete synchronous request after image admission and wire //! prefix assembly. Includes reused history, tool definitions and output format. +use codex_guardian_context::REQUEST_TOKENS_BOUNDARIES; use codex_guardian_context::REQUEST_TOKENS_METRIC; use codex_protocol::protocol::TruncationPolicy; @@ -24,9 +25,10 @@ pub(crate) fn observe(telemetry: &SessionTelemetry, request: &ResponsesApiReques .saturating_add(metadata); // The assembled input already includes inherited history and the current // review. Do not report a guessed old/new split after context injection. - telemetry.histogram( + telemetry.histogram_with_boundaries( REQUEST_TOKENS_METRIC, i64::try_from(total).unwrap_or(i64::MAX), + REQUEST_TOKENS_BOUNDARIES, &[("target", "sync"), ("component", "total")], ); total diff --git a/codex-rs/core/src/session/extension_metrics.rs b/codex-rs/core/src/session/extension_metrics.rs index 13547c6bc2..bb4b271080 100644 --- a/codex-rs/core/src/session/extension_metrics.rs +++ b/codex-rs/core/src/session/extension_metrics.rs @@ -15,6 +15,17 @@ impl ExtensionMetrics for SessionTelemetryExtensionMetrics { fn histogram(&self, name: &str, value: i64, tags: &[(&str, &str)]) { self.session_telemetry.histogram(name, value, tags); } + + fn histogram_with_boundaries( + &self, + name: &str, + value: i64, + boundaries: &[f64], + tags: &[(&str, &str)], + ) { + self.session_telemetry + .histogram_with_boundaries(name, value, boundaries, tags); + } } pub(crate) fn from_session_telemetry( diff --git a/codex-rs/ext/extension-api/src/capabilities/metrics.rs b/codex-rs/ext/extension-api/src/capabilities/metrics.rs index b61b3d9616..67f395b480 100644 --- a/codex-rs/ext/extension-api/src/capabilities/metrics.rs +++ b/codex-rs/ext/extension-api/src/capabilities/metrics.rs @@ -8,4 +8,14 @@ pub trait ExtensionMetrics: Send + Sync { /// Records one histogram sample with optional extension-provided tags. fn histogram(&self, name: &str, value: i64, tags: &[(&str, &str)]); + + /// Records a histogram with explicit buckets, preserving host attribution. + /// All callers of the same metric name must use the same boundaries. + fn histogram_with_boundaries( + &self, + name: &str, + value: i64, + boundaries: &[f64], + tags: &[(&str, &str)], + ); } diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs b/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs index 54bdf39a79..6860cf52cb 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs @@ -603,9 +603,10 @@ impl GuardianV2Extension { if let Some(metrics) = metrics.as_deref() { for (section, cost) in transcript.section_costs() { for (measurement, value) in cost.measurements() { - metrics.histogram( + metrics.histogram_with_boundaries( codex_guardian_context::SECTION_COST_METRIC, i64::try_from(value).unwrap_or(i64::MAX), + codex_guardian_context::SECTION_COST_BOUNDARIES, &[ ("target", "async"), ("section", section), diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs b/codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs index c41025a8f6..85c130050c 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs @@ -358,6 +358,16 @@ impl RecordingMetrics { } impl ExtensionMetrics for RecordingMetrics { + fn histogram_with_boundaries( + &self, + name: &str, + value: i64, + _boundaries: &[f64], + tags: &[(&str, &str)], + ) { + self.histogram(name, value, tags); + } + fn counter(&self, name: &str, inc: i64, tags: &[(&str, &str)]) { self.0.lock().unwrap().push(RecordedMetric::Counter( name.to_owned(), diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/sampler.rs b/codex-rs/ext/guardian-v2/src/async_scorer/sampler.rs index 3474c2471b..2f68b94e49 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/sampler.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/sampler.rs @@ -302,9 +302,10 @@ impl LunaSampler { ("new_input", total_tokens), ("total", total_tokens), ] { - metrics.histogram( + metrics.histogram_with_boundaries( codex_guardian_context::REQUEST_TOKENS_METRIC, i64::try_from(tokens).unwrap_or(i64::MAX), + codex_guardian_context::REQUEST_TOKENS_BOUNDARIES, &[("target", "async"), ("component", component)], ); } diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/sampler_tests.rs b/codex-rs/ext/guardian-v2/src/async_scorer/sampler_tests.rs index c822c51944..6dc562c92c 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/sampler_tests.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/sampler_tests.rs @@ -239,6 +239,16 @@ type RecordedMetric = (String, i64, Vec<(String, String)>); struct RecordingMetrics(Mutex>); impl ExtensionMetrics for RecordingMetrics { + fn histogram_with_boundaries( + &self, + name: &str, + value: i64, + _boundaries: &[f64], + tags: &[(&str, &str)], + ) { + self.histogram(name, value, tags); + } + fn counter(&self, _name: &str, _inc: i64, _tags: &[(&str, &str)]) {} fn histogram(&self, name: &str, value: i64, tags: &[(&str, &str)]) { diff --git a/codex-rs/ext/skills/src/extension_tests.rs b/codex-rs/ext/skills/src/extension_tests.rs index 604ceaebf4..118812bc97 100644 --- a/codex-rs/ext/skills/src/extension_tests.rs +++ b/codex-rs/ext/skills/src/extension_tests.rs @@ -15,6 +15,16 @@ struct RecordingMetrics { } impl ExtensionMetrics for RecordingMetrics { + fn histogram_with_boundaries( + &self, + name: &str, + value: i64, + _boundaries: &[f64], + tags: &[(&str, &str)], + ) { + self.histogram(name, value, tags); + } + fn counter(&self, name: &str, _inc: i64, _tags: &[(&str, &str)]) { panic!("unexpected counter: {name}"); } diff --git a/codex-rs/ext/skills/src/render_observability_tests.rs b/codex-rs/ext/skills/src/render_observability_tests.rs index a3715f5891..f18ee7d08d 100644 --- a/codex-rs/ext/skills/src/render_observability_tests.rs +++ b/codex-rs/ext/skills/src/render_observability_tests.rs @@ -18,6 +18,16 @@ struct RecordingMetrics { } impl ExtensionMetrics for RecordingMetrics { + fn histogram_with_boundaries( + &self, + name: &str, + value: i64, + _boundaries: &[f64], + tags: &[(&str, &str)], + ) { + self.histogram(name, value, tags); + } + fn counter(&self, name: &str, _inc: i64, _tags: &[(&str, &str)]) { panic!("unexpected counter: {name}"); } diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index cf6857bb7d..ac9cc37781 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -2503,6 +2503,16 @@ impl RecordingMetrics { } impl ExtensionMetrics for RecordingMetrics { + fn histogram_with_boundaries( + &self, + name: &str, + value: i64, + _boundaries: &[f64], + tags: &[(&str, &str)], + ) { + self.histogram(name, value, tags); + } + fn counter(&self, name: &str, _inc: i64, _tags: &[(&str, &str)]) { panic!("unexpected counter: {name}"); } diff --git a/codex-rs/guardian-context/src/budget.rs b/codex-rs/guardian-context/src/budget.rs index a400c1967b..9a9d0d79a9 100644 --- a/codex-rs/guardian-context/src/budget.rs +++ b/codex-rs/guardian-context/src/budget.rs @@ -14,6 +14,41 @@ use crate::composition::SectionOutput; pub const SECTION_COST_METRIC: &str = "codex.guardian.context.section_cost"; pub const REQUEST_TOKENS_METRIC: &str = "codex.guardian.context.request_tokens"; +/// Fixed across reviewers and models so complete-request distributions align. +pub const REQUEST_TOKENS_BOUNDARIES: &[f64] = &[ + 1_000.0, + 4_000.0, + 16_000.0, + 32_000.0, + 64_000.0, + 128_000.0, + 256_000.0, + 400_000.0, + 512_000.0, + 1_000_000.0, + 2_000_000.0, +]; +/// One shared scale for the section metric's count, token and byte measurements. +/// Buckets belong to the metric name, not its measurement tag. +pub const SECTION_COST_BOUNDARIES: &[f64] = &[ + 0.0, + 1.0, + 2.0, + 4.0, + 8.0, + 16.0, + 64.0, + 256.0, + 1_024.0, + 4_096.0, + 16_384.0, + 65_536.0, + 262_144.0, + 1_048_576.0, + 4_194_304.0, + 8_388_608.0, + 16_777_216.0, +]; // A conservative reservation matching the existing original-image patch ceiling. const IMAGE_TOKEN_RESERVATION: usize = 10_000; /// Conservative input ceiling when the model catalog has no authoritative window. diff --git a/codex-rs/guardian-context/src/lib.rs b/codex-rs/guardian-context/src/lib.rs index df76d79600..0ba280b870 100644 --- a/codex-rs/guardian-context/src/lib.rs +++ b/codex-rs/guardian-context/src/lib.rs @@ -55,8 +55,10 @@ pub(crate) use enforcement::Retention; mod budget; mod composition; pub use budget::DEFAULT_MAX_INPUT_TOKENS; +pub use budget::REQUEST_TOKENS_BOUNDARIES; pub use budget::REQUEST_TOKENS_METRIC; pub use budget::RequestBudget; +pub use budget::SECTION_COST_BOUNDARIES; pub use budget::SECTION_COST_METRIC; pub use budget::SectionCost; pub use budget::effective_input_token_limit; diff --git a/codex-rs/otel/src/events/session_telemetry.rs b/codex-rs/otel/src/events/session_telemetry.rs index ddcd7fb2bb..5e8f7cebee 100644 --- a/codex-rs/otel/src/events/session_telemetry.rs +++ b/codex-rs/otel/src/events/session_telemetry.rs @@ -205,6 +205,29 @@ impl SessionTelemetry { } } + /// Records a histogram with explicit buckets and the usual session attribution. + /// All callers of the same metric name must use the same boundaries. + pub fn histogram_with_boundaries( + &self, + name: &str, + value: i64, + boundaries: &[f64], + tags: &[(&str, &str)], + ) { + let res: MetricsResult<()> = (|| { + let Some(metrics) = &self.metrics else { + return Ok(()); + }; + + let tags = self.tags_with_metadata(tags)?; + metrics.histogram_with_boundaries(name, value, boundaries, &tags) + })(); + + if let Err(e) = res { + tracing::warn!("metrics histogram [{name}] failed: {e}"); + } + } + pub fn record_duration(&self, name: &str, duration: Duration, tags: &[(&str, &str)]) { let res: MetricsResult<()> = (|| { let Some(metrics) = &self.metrics else { diff --git a/codex-rs/otel/tests/suite/manager_metrics.rs b/codex-rs/otel/tests/suite/manager_metrics.rs index e13de3f432..1a7a719d50 100644 --- a/codex-rs/otel/tests/suite/manager_metrics.rs +++ b/codex-rs/otel/tests/suite/manager_metrics.rs @@ -1,6 +1,7 @@ use crate::harness::attributes_to_map; use crate::harness::build_metrics_with_defaults; use crate::harness::find_metric; +use crate::harness::histogram_data; use crate::harness::latest_metrics; use codex_otel::PLUGIN_INSTALL_ELICITATION_SENT_METRIC; use codex_otel::PLUGIN_INSTALL_SUGGESTION_METRIC; @@ -37,9 +38,26 @@ fn manager_attaches_metadata_tags_to_metrics() -> Result<()> { /*inc*/ 1, &[("source", "tui")], ); + for tokens in [32_000, 256_000] { + manager.histogram_with_boundaries( + "codex.request_tokens", + tokens, + &[16_000.0, 128_000.0, 512_000.0], + &[("source", "tui")], + ); + } manager.shutdown_metrics()?; let resource_metrics = latest_metrics(&exporter); + assert_eq!( + histogram_data(&resource_metrics, "codex.request_tokens"), + ( + vec![16_000.0, 128_000.0, 512_000.0], + vec![0, 1, 1, 0], + 288_000.0, + 2, + ) + ); let metric = find_metric(&resource_metrics, "codex.session_started").expect("counter metric missing"); let attrs = match metric.data() {