mirror of
https://github.com/openai/codex.git
synced 2026-09-08 15:50:34 +00:00
Keep tool-call metrics out of Statsig exports (#36049)
## What changed - Treat `codex.tool.call` and `codex.tool.call.duration_ms` as runtime-only metrics when using the built-in Statsig exporter. - Continue exporting both metrics through explicitly configured OTLP exporters. ## Testing - Verify the Statsig configuration omits the tool-call metrics while retaining unrelated metrics. - Extend the OTLP HTTP loopback test to cover the tool-call counter and duration histogram. GitOrigin-RevId: 46dd2642063b34e9a4c69b67d58b45721609fcf2
This commit is contained in:
@@ -106,6 +106,7 @@ struct MetricsClientInner {
|
||||
histograms: Mutex<HashMap<String, Histogram<f64>>>,
|
||||
duration_histograms: Mutex<HashMap<InstrumentKey, Histogram<f64>>>,
|
||||
runtime_reader: Option<Arc<ManualReader>>,
|
||||
runtime_only_metrics: &'static [&'static str],
|
||||
default_tags: BTreeMap<String, String>,
|
||||
}
|
||||
|
||||
@@ -126,6 +127,10 @@ impl MetricsClientInner {
|
||||
}
|
||||
let attributes = self.attributes(tags)?;
|
||||
|
||||
if self.runtime_only_metrics.contains(&name) {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let mut counters = self
|
||||
.counters
|
||||
.lock()
|
||||
@@ -221,6 +226,10 @@ impl MetricsClientInner {
|
||||
validate_metric_name(name)?;
|
||||
let attributes = self.attributes(tags)?;
|
||||
|
||||
if self.runtime_only_metrics.contains(&name) {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let mut histograms = self
|
||||
.duration_histograms
|
||||
.lock()
|
||||
@@ -290,6 +299,7 @@ impl MetricsClient {
|
||||
exporter,
|
||||
export_interval,
|
||||
runtime_reader,
|
||||
runtime_only_metrics,
|
||||
default_tags,
|
||||
} = config;
|
||||
|
||||
@@ -334,6 +344,7 @@ impl MetricsClient {
|
||||
histograms: Mutex::new(HashMap::new()),
|
||||
duration_histograms: Mutex::new(HashMap::new()),
|
||||
runtime_reader,
|
||||
runtime_only_metrics,
|
||||
default_tags,
|
||||
})))
|
||||
}
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
use crate::config::OtelExporter;
|
||||
use crate::metrics::Result;
|
||||
use crate::metrics::names::TOOL_CALL_COUNT_METRIC;
|
||||
use crate::metrics::names::TOOL_CALL_DURATION_METRIC;
|
||||
use crate::metrics::validation::validate_tag_key;
|
||||
use crate::metrics::validation::validate_tag_value;
|
||||
use opentelemetry_sdk::metrics::InMemoryMetricExporter;
|
||||
use std::collections::BTreeMap;
|
||||
use std::time::Duration;
|
||||
|
||||
const RUNTIME_ONLY_METRICS: &[&str] = &[TOOL_CALL_COUNT_METRIC, TOOL_CALL_DURATION_METRIC];
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub enum MetricsExporter {
|
||||
Otlp(OtelExporter),
|
||||
@@ -20,6 +24,7 @@ pub struct MetricsConfig {
|
||||
pub(crate) exporter: MetricsExporter,
|
||||
pub(crate) export_interval: Option<Duration>,
|
||||
pub(crate) runtime_reader: bool,
|
||||
pub(crate) runtime_only_metrics: &'static [&'static str],
|
||||
pub(crate) default_tags: BTreeMap<String, String>,
|
||||
}
|
||||
|
||||
@@ -30,6 +35,11 @@ impl MetricsConfig {
|
||||
service_version: impl Into<String>,
|
||||
exporter: OtelExporter,
|
||||
) -> Self {
|
||||
let runtime_only_metrics = if matches!(exporter, OtelExporter::Statsig) {
|
||||
RUNTIME_ONLY_METRICS
|
||||
} else {
|
||||
&[]
|
||||
};
|
||||
Self {
|
||||
environment: environment.into(),
|
||||
service_name: service_name.into(),
|
||||
@@ -37,6 +47,7 @@ impl MetricsConfig {
|
||||
exporter: MetricsExporter::Otlp(exporter),
|
||||
export_interval: None,
|
||||
runtime_reader: false,
|
||||
runtime_only_metrics,
|
||||
default_tags: BTreeMap::new(),
|
||||
}
|
||||
}
|
||||
@@ -55,6 +66,7 @@ impl MetricsConfig {
|
||||
exporter: MetricsExporter::InMemory(exporter),
|
||||
export_interval: None,
|
||||
runtime_reader: false,
|
||||
runtime_only_metrics: &[],
|
||||
default_tags: BTreeMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -111,7 +111,7 @@ impl OtelProvider {
|
||||
settings.environment.clone(),
|
||||
settings.service_name.clone(),
|
||||
settings.service_version.clone(),
|
||||
metric_exporter,
|
||||
settings.metrics_exporter.clone(),
|
||||
);
|
||||
if settings.runtime_metrics {
|
||||
config = config.with_runtime_reader();
|
||||
@@ -462,6 +462,10 @@ mod shutdown_tests;
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::metrics::MetricsExporter;
|
||||
use crate::metrics::TOOL_CALL_COUNT_METRIC;
|
||||
use crate::metrics::TOOL_CALL_DURATION_METRIC;
|
||||
use opentelemetry_sdk::metrics::InMemoryMetricExporter;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::PathBuf;
|
||||
|
||||
@@ -528,6 +532,37 @@ mod tests {
|
||||
assert!(!is_trace_safe_target("codex_otel.network_proxy"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn statsig_runtime_only_metrics_are_not_exported() -> Result<(), Box<dyn Error>> {
|
||||
let exporter = InMemoryMetricExporter::default();
|
||||
let mut config = MetricsConfig::otlp(
|
||||
"test",
|
||||
"codex-cli",
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
OtelExporter::Statsig,
|
||||
);
|
||||
config.exporter = MetricsExporter::InMemory(exporter.clone());
|
||||
let metrics = MetricsClient::new(config)?;
|
||||
|
||||
metrics.counter(TOOL_CALL_COUNT_METRIC, /*inc*/ 1, &[])?;
|
||||
metrics.record_duration(TOOL_CALL_DURATION_METRIC, Duration::from_millis(25), &[])?;
|
||||
metrics.counter("codex.turns", /*inc*/ 1, &[])?;
|
||||
metrics.shutdown()?;
|
||||
|
||||
let exported_metrics = exporter.get_finished_metrics()?;
|
||||
let mut names: Vec<_> = exported_metrics
|
||||
.iter()
|
||||
.flat_map(opentelemetry_sdk::metrics::data::ResourceMetrics::scope_metrics)
|
||||
.flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics)
|
||||
.map(opentelemetry_sdk::metrics::data::Metric::name)
|
||||
.collect();
|
||||
names.sort_unstable();
|
||||
names.dedup();
|
||||
assert_eq!(names, vec!["codex.turns"]);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn test_otel_settings() -> OtelSettings {
|
||||
OtelSettings {
|
||||
environment: "test".to_string(),
|
||||
|
||||
@@ -186,6 +186,12 @@ fn otlp_http_exporter_sends_metrics_to_collector() -> Result<()> {
|
||||
))?;
|
||||
|
||||
metrics.counter("codex.turns", /*inc*/ 1, &[("source", "test")])?;
|
||||
metrics.counter("codex.tool.call", /*inc*/ 1, &[("tool", "test")])?;
|
||||
metrics.record_duration(
|
||||
"codex.tool.call.duration_ms",
|
||||
Duration::from_millis(42),
|
||||
&[("tool", "test")],
|
||||
)?;
|
||||
metrics.gauge_with_description(
|
||||
"codex.active",
|
||||
"Number of active Codex operations.",
|
||||
@@ -221,6 +227,16 @@ fn otlp_http_exporter_sends_metrics_to_collector() -> Result<()> {
|
||||
"expected gauge not found; body prefix: {}",
|
||||
&body.chars().take(2000).collect::<String>()
|
||||
);
|
||||
assert!(
|
||||
body.contains("\"codex.tool.call\""),
|
||||
"expected tool-call counter not found; body prefix: {}",
|
||||
&body.chars().take(2000).collect::<String>()
|
||||
);
|
||||
assert!(
|
||||
body.contains("\"codex.tool.call.duration_ms\""),
|
||||
"expected tool-call duration not found; body prefix: {}",
|
||||
&body.chars().take(2000).collect::<String>()
|
||||
);
|
||||
assert!(
|
||||
body.contains("component") && body.contains("test"),
|
||||
"expected gauge tag not found; body prefix: {}",
|
||||
|
||||
Reference in New Issue
Block a user