Include real OS info in metrics. (#10425)

calculated a hashed user ID from either auth user id or API key
Also correctly populates OS.

These will make our metrics more useful and powerful for analysis.
This commit is contained in:
iceweasel-oai 2026-02-05 06:30:31 -08:00 committed by GitHub
parent 040ecee715
commit f2ffc4e5d0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 88 additions and 32 deletions

5
codex-rs/Cargo.lock generated
View file

@ -1920,6 +1920,7 @@ dependencies = [
"codex-app-server-protocol",
"codex-protocol",
"codex-utils-absolute-path",
"codex-utils-string",
"eventsource-stream",
"http 1.4.0",
"opentelemetry",
@ -1927,6 +1928,7 @@ dependencies = [
"opentelemetry-otlp",
"opentelemetry-semantic-conventions",
"opentelemetry_sdk",
"os_info",
"pretty_assertions",
"reqwest",
"serde",
@ -2244,6 +2246,9 @@ dependencies = [
[[package]]
name = "codex-utils-string"
version = "0.0.0"
dependencies = [
"pretty_assertions",
]
[[package]]
name = "codex-windows-sandbox"

View file

@ -23,6 +23,7 @@ disable-default-metrics-exporter = []
chrono = { workspace = true }
codex-app-server-protocol = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-string = { workspace = true }
codex-api = { workspace = true }
codex-protocol = { workspace = true }
eventsource-stream = { workspace = true }
@ -50,6 +51,7 @@ opentelemetry_sdk = { workspace = true, features = [
"trace",
] }
http = { workspace = true }
os_info = { workspace = true }
reqwest = { workspace = true, features = ["blocking", "rustls-tls"] }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }

View file

@ -14,6 +14,7 @@ use crate::metrics::validation::validate_tag_key;
use crate::metrics::validation::validate_tag_value;
use crate::otel_provider::OtelProvider;
use codex_protocol::ThreadId;
pub use codex_utils_string::sanitize_metric_tag_value;
use opentelemetry_sdk::metrics::data::ResourceMetrics;
use serde::Serialize;
use std::time::Duration;
@ -188,7 +189,7 @@ impl OtelManager {
if !self.metrics_use_metadata_tags {
return Ok(Vec::new());
}
let mut tags = Vec::with_capacity(6);
let mut tags = Vec::with_capacity(5);
Self::push_metadata_tag(&mut tags, "auth_mode", self.metadata.auth_mode.as_deref())?;
Self::push_metadata_tag(
&mut tags,

View file

@ -9,6 +9,7 @@ use crate::metrics::validation::validate_metric_name;
use crate::metrics::validation::validate_tag_key;
use crate::metrics::validation::validate_tag_value;
use crate::metrics::validation::validate_tags;
use codex_utils_string::sanitize_metric_tag_value;
use opentelemetry::KeyValue;
use opentelemetry::metrics::Counter;
use opentelemetry::metrics::Histogram;
@ -197,12 +198,17 @@ impl MetricsClient {
validate_tags(&default_tags)?;
let mut resource_attributes = Vec::with_capacity(4);
resource_attributes.push(KeyValue::new(
semconv::attribute::SERVICE_VERSION,
service_version,
));
resource_attributes.push(KeyValue::new(ENV_ATTRIBUTE, environment));
resource_attributes.extend(os_resource_attributes());
let resource = Resource::builder()
.with_service_name(service_name)
.with_attributes(vec![
KeyValue::new(semconv::attribute::SERVICE_VERSION, service_version),
KeyValue::new(ENV_ATTRIBUTE, environment),
])
.with_attributes(resource_attributes)
.build();
let runtime_reader = runtime_reader.then(|| {
@ -284,6 +290,22 @@ impl MetricsClient {
}
}
fn os_resource_attributes() -> Vec<KeyValue> {
let os_info = os_info::get();
let os_type_raw = os_info.os_type().to_string();
let os_type = sanitize_metric_tag_value(os_type_raw.as_str());
let os_version_raw = os_info.version().to_string();
let os_version = sanitize_metric_tag_value(os_version_raw.as_str());
let mut attributes = Vec::new();
if os_type != "unspecified" {
attributes.push(KeyValue::new("os", os_type));
}
if os_version != "unspecified" {
attributes.push(KeyValue::new("os_version", os_version));
}
attributes
}
fn build_provider<E>(
resource: Resource,
exporter: E,

View file

@ -6,3 +6,6 @@ license.workspace = true
[lints]
workspace = true
[dev-dependencies]
pretty_assertions = { workspace = true }

View file

@ -36,3 +36,46 @@ pub fn take_last_bytes_at_char_boundary(s: &str, maxb: usize) -> &str {
}
&s[start..]
}
/// Sanitize a tag value to comply with metric tag validation rules:
/// only ASCII alphanumeric, '.', '_', '-', and '/' are allowed.
pub fn sanitize_metric_tag_value(value: &str) -> String {
const MAX_LEN: usize = 256;
let sanitized: String = value
.chars()
.map(|ch| {
if ch.is_ascii_alphanumeric() || matches!(ch, '.' | '_' | '-' | '/') {
ch
} else {
'_'
}
})
.collect();
let trimmed = sanitized.trim_matches('_');
if trimmed.is_empty() || trimmed.chars().all(|ch| !ch.is_ascii_alphanumeric()) {
return "unspecified".to_string();
}
if trimmed.len() <= MAX_LEN {
trimmed.to_string()
} else {
trimmed[..MAX_LEN].to_string()
}
}
#[cfg(test)]
mod tests {
use super::sanitize_metric_tag_value;
use pretty_assertions::assert_eq;
#[test]
fn sanitize_metric_tag_value_trims_and_fills_unspecified() {
let msg = "///";
assert_eq!(sanitize_metric_tag_value(msg), "unspecified");
}
#[test]
fn sanitize_metric_tag_value_replaces_invalid_chars() {
let msg = "bad value!";
assert_eq!(sanitize_metric_tag_value(msg), "bad_value");
}
}

View file

@ -91,7 +91,7 @@ pub use setup::SETUP_VERSION;
#[cfg(target_os = "windows")]
pub use setup_error::extract_failure as extract_setup_failure;
#[cfg(target_os = "windows")]
pub use setup_error::sanitize_tag_value as sanitize_setup_metric_tag_value;
pub use setup_error::sanitize_setup_metric_tag_value;
#[cfg(target_os = "windows")]
pub use setup_error::setup_error_path;
#[cfg(target_os = "windows")]

View file

@ -1,5 +1,6 @@
use anyhow::Context;
use anyhow::Result;
use codex_utils_string::sanitize_metric_tag_value;
use serde::Deserialize;
use serde::Serialize;
use std::fs;
@ -126,7 +127,7 @@ impl SetupFailure {
}
pub fn metric_message(&self) -> String {
sanitize_tag_value(&self.message)
sanitize_setup_metric_tag_value(&self.message)
}
}
@ -181,30 +182,9 @@ pub fn read_setup_error_report(codex_home: &Path) -> Result<Option<SetupErrorRep
Ok(Some(report))
}
/// Sanitize a tag value to comply with metric tag validation rules:
/// only ASCII alphanumeric, '.', '_', '-', and '/' are allowed.
pub fn sanitize_tag_value(value: &str) -> String {
const MAX_LEN: usize = 256;
let redacted = redact_home_paths(value);
let sanitized: String = redacted
.chars()
.map(|ch| {
if ch.is_ascii_alphanumeric() || matches!(ch, '.' | '_' | '-' | '/') {
ch
} else {
'_'
}
})
.collect();
let trimmed = sanitized.trim_matches('_');
if trimmed.is_empty() {
return "unspecified".to_string();
}
if trimmed.len() <= MAX_LEN {
trimmed.to_string()
} else {
trimmed[..MAX_LEN].to_string()
}
/// Sanitize a setup error message for use as a metric tag.
pub fn sanitize_setup_metric_tag_value(value: &str) -> String {
sanitize_metric_tag_value(redact_home_paths(value).as_str())
}
fn redact_home_paths(value: &str) -> String {
@ -267,7 +247,7 @@ fn redact_username_segments(value: &str, usernames: &[String]) -> String {
#[cfg(test)]
mod tests {
use super::*;
use super::redact_username_segments;
use pretty_assertions::assert_eq;
#[test]