From 2d200792d33154d788b6b98dc6d5a8380595c573 Mon Sep 17 00:00:00 2001 From: Vishnu Chopra Date: Mon, 11 Aug 2025 18:35:30 +0100 Subject: [PATCH] Address review comments --- codex-rs/codex-telemetry/src/lib.rs | 11 ++--------- codex-rs/codex-telemetry/tests/otel_file_export.rs | 2 -- codex-rs/core/src/config.rs | 5 ++++- codex-rs/core/src/telemetry_init.rs | 3 --- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/codex-rs/codex-telemetry/src/lib.rs b/codex-rs/codex-telemetry/src/lib.rs index 911af61885..f947bdfb7c 100644 --- a/codex-rs/codex-telemetry/src/lib.rs +++ b/codex-rs/codex-telemetry/src/lib.rs @@ -32,7 +32,6 @@ mod imp { pub enum TelemetryExporter { None, OtlpFile { - path: PathBuf, rotate_mb: Option, }, OtlpGrpc { @@ -112,10 +111,7 @@ mod imp { global::set_tracer_provider(provider.clone()); return Some(TelemetryGuard { provider }); } - TelemetryExporter::OtlpFile { - path: _ignored, - rotate_mb, - } => { + TelemetryExporter::OtlpFile { rotate_mb } => { let final_path = determine_traces_file_path(settings.codex_home.as_ref()); if debug_enabled() { eprintln!( @@ -245,10 +241,7 @@ mod imp { let tracer = provider.tracer(settings.service_name.clone()); return Some((TelemetryGuard { provider }, tracer)); } - TelemetryExporter::OtlpFile { - path: _ignored, - rotate_mb, - } => { + TelemetryExporter::OtlpFile { rotate_mb } => { let final_path = determine_traces_file_path(settings.codex_home.as_ref()); if debug_enabled() { eprintln!( diff --git a/codex-rs/codex-telemetry/tests/otel_file_export.rs b/codex-rs/codex-telemetry/tests/otel_file_export.rs index 4fac79aa9e..5d26d4f0ea 100644 --- a/codex-rs/codex-telemetry/tests/otel_file_export.rs +++ b/codex-rs/codex-telemetry/tests/otel_file_export.rs @@ -31,7 +31,6 @@ async fn file_exporter_writes_span_json() { let (guard, tracer) = telemetry::build_layer(&telemetry::Settings { enabled: true, exporter: telemetry::Exporter::OtlpFile { - path: PathBuf::new(), rotate_mb: Some(100), }, service_name: "codex-test".to_string(), @@ -80,7 +79,6 @@ async fn session_span_flushes_on_shutdown() { let (guard, tracer) = telemetry::build_layer(&telemetry::Settings { enabled: true, exporter: telemetry::Exporter::OtlpFile { - path: PathBuf::new(), rotate_mb: Some(100), }, service_name: "codex-test".to_string(), diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 27443e7835..3e6c5c2615 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -48,6 +48,9 @@ pub(crate) const PROJECT_DOC_MAX_BYTES: usize = 32 * 1024; // 32 KiB pub(crate) const CONFIG_TOML_FILE: &str = "config.toml"; +/// Default rotation size for telemetry file exporter, in MiB. +pub(crate) const DEFAULT_TELEMETRY_ROTATE_MB: u64 = 100; + /// Application configuration loaded from disk and merged with overrides. #[derive(Debug, Clone, PartialEq)] pub struct Config { @@ -1068,7 +1071,7 @@ impl Config { let exporter = t.exporter.unwrap_or(TelemetryExporterKind::OtlpFile); let endpoint = t.endpoint; let headers = t.headers.unwrap_or_default(); - let rotate_mb = t.rotate_mb.or(Some(100)); + let rotate_mb = t.rotate_mb.or(Some(DEFAULT_TELEMETRY_ROTATE_MB)); TelemetryConfig { enabled, exporter, diff --git a/codex-rs/core/src/telemetry_init.rs b/codex-rs/core/src/telemetry_init.rs index 1d615c2c51..280a5875c8 100644 --- a/codex-rs/core/src/telemetry_init.rs +++ b/codex-rs/core/src/telemetry_init.rs @@ -1,5 +1,3 @@ -use std::path::PathBuf; - use opentelemetry_sdk::trace::Tracer; use crate::config::Config; @@ -17,7 +15,6 @@ pub fn build_otel_layer_from_config( let exporter = match config.telemetry.exporter { Kind::None => telemetry::Exporter::None, Kind::OtlpFile => telemetry::Exporter::OtlpFile { - path: PathBuf::new(), rotate_mb: config.telemetry.rotate_mb, }, Kind::OtlpHttp => telemetry::Exporter::OtlpHttp {