mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Add OTEL counter descriptions (#26091)
## Why Metric descriptions should be declared with reusable OTEL instruments instead of being coupled to individual consumers. Counter descriptions are the smallest API primitive needed by the exec-server observability work. ## What changed - Adds `counter_with_description` while preserving the existing counter API. - Caches counters by name and description so instrument metadata remains part of the declaration identity. - Covers the exported description together with the existing value and attribute contract. This PR only adds counter descriptions. It does not add gauges, second-based durations, or exec-server adoption. ## Stack 1. **#26091: counter descriptions** 2. #27057: gauge instruments 3. #27058: second-based duration histograms Related independent coverage: #27059 tests OTLP HTTP log and trace event export. The `codex-exec-server` bounded service tag now stays with the exec-server adoption change instead of this reusable infrastructure stack. ## Validation - `just test -p codex-otel` - `just fix -p codex-otel` - `just fmt`
This commit is contained in:
committed by
GitHub
Unverified
parent
381f0de531
commit
feca160da4
@@ -45,6 +45,12 @@ const METER_NAME: &str = "codex";
|
||||
const DURATION_UNIT: &str = "ms";
|
||||
const DURATION_DESCRIPTION: &str = "Duration in milliseconds.";
|
||||
|
||||
#[derive(Debug, Eq, Hash, PartialEq)]
|
||||
struct InstrumentKey {
|
||||
name: String,
|
||||
description: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
struct SharedManualReader {
|
||||
inner: Arc<ManualReader>,
|
||||
@@ -82,7 +88,7 @@ impl MetricReader for SharedManualReader {
|
||||
struct MetricsClientInner {
|
||||
meter_provider: SdkMeterProvider,
|
||||
meter: Meter,
|
||||
counters: Mutex<HashMap<String, Counter<u64>>>,
|
||||
counters: Mutex<HashMap<InstrumentKey, Counter<u64>>>,
|
||||
histograms: Mutex<HashMap<String, Histogram<f64>>>,
|
||||
duration_histograms: Mutex<HashMap<String, Histogram<f64>>>,
|
||||
runtime_reader: Option<Arc<ManualReader>>,
|
||||
@@ -90,7 +96,13 @@ struct MetricsClientInner {
|
||||
}
|
||||
|
||||
impl MetricsClientInner {
|
||||
fn counter(&self, name: &str, inc: i64, tags: &[(&str, &str)]) -> Result<()> {
|
||||
fn counter(
|
||||
&self,
|
||||
name: &str,
|
||||
description: Option<&str>,
|
||||
inc: i64,
|
||||
tags: &[(&str, &str)],
|
||||
) -> Result<()> {
|
||||
validate_metric_name(name)?;
|
||||
if inc < 0 {
|
||||
return Err(MetricsError::NegativeCounterIncrement {
|
||||
@@ -104,9 +116,17 @@ impl MetricsClientInner {
|
||||
.counters
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
let counter = counters
|
||||
.entry(name.to_string())
|
||||
.or_insert_with(|| self.meter.u64_counter(name.to_string()).build());
|
||||
let key = InstrumentKey {
|
||||
name: name.to_string(),
|
||||
description: description.map(str::to_string),
|
||||
};
|
||||
let counter = counters.entry(key).or_insert_with(|| {
|
||||
let builder = self.meter.u64_counter(name.to_string());
|
||||
match description {
|
||||
Some(description) => builder.with_description(description.to_string()).build(),
|
||||
None => builder.build(),
|
||||
}
|
||||
});
|
||||
counter.add(inc as u64, &attributes);
|
||||
Ok(())
|
||||
}
|
||||
@@ -242,7 +262,18 @@ impl MetricsClient {
|
||||
|
||||
/// Send a single counter increment.
|
||||
pub fn counter(&self, name: &str, inc: i64, tags: &[(&str, &str)]) -> Result<()> {
|
||||
self.0.counter(name, inc, tags)
|
||||
self.0.counter(name, /*description*/ None, inc, tags)
|
||||
}
|
||||
|
||||
/// Send a single counter increment with an instrument description.
|
||||
pub fn counter_with_description(
|
||||
&self,
|
||||
name: &str,
|
||||
description: &str,
|
||||
inc: i64,
|
||||
tags: &[(&str, &str)],
|
||||
) -> Result<()> {
|
||||
self.0.counter(name, Some(description), inc, tags)
|
||||
}
|
||||
|
||||
/// Send a single histogram sample.
|
||||
|
||||
@@ -13,8 +13,9 @@ fn send_builds_payload_with_tags_and_histograms() -> Result<()> {
|
||||
let (metrics, exporter) =
|
||||
build_metrics_with_defaults(&[("service", "codex-cli"), ("env", "prod")])?;
|
||||
|
||||
metrics.counter(
|
||||
metrics.counter_with_description(
|
||||
"codex.turns",
|
||||
"Total number of Codex turns.",
|
||||
/*inc*/ 1,
|
||||
&[("model", "gpt-5.1"), ("env", "dev")],
|
||||
)?;
|
||||
@@ -28,6 +29,7 @@ fn send_builds_payload_with_tags_and_histograms() -> Result<()> {
|
||||
let resource_metrics = latest_metrics(&exporter);
|
||||
|
||||
let counter = find_metric(&resource_metrics, "codex.turns").expect("counter metric missing");
|
||||
assert_eq!(counter.description(), "Total number of Codex turns.");
|
||||
let counter_attributes = match counter.data() {
|
||||
opentelemetry_sdk::metrics::data::AggregatedMetrics::U64(data) => match data {
|
||||
opentelemetry_sdk::metrics::data::MetricData::Sum(sum) => {
|
||||
|
||||
Reference in New Issue
Block a user