mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Avoid noisy OTEL diagnostics in codex exec (#21107)
`codex exec` should not print OpenTelemetry exporter self-diagnostics to stderr by default. Suppress the SDK and OTLP exporter targets unless callers explicitly opt in with `RUST_LOG`. Also stop defaulting the trace exporter to the log exporter, since OTLP HTTP endpoints are signal-specific and a logs endpoint is not valid for spans. Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
346070a424
commit
f9063045e1
@@ -7113,6 +7113,39 @@ async fn metrics_exporter_defaults_to_statsig_when_missing() -> std::io::Result<
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn trace_exporter_defaults_to_none_when_log_exporter_is_set() -> std::io::Result<()> {
|
||||
let fixture = create_test_fixture()?;
|
||||
let mut cfg = fixture.cfg.clone();
|
||||
cfg.otel = Some(OtelConfigToml {
|
||||
exporter: Some(OtelExporterKind::OtlpHttp {
|
||||
endpoint: "http://localhost:14318/v1/logs".to_string(),
|
||||
headers: HashMap::new(),
|
||||
protocol: codex_config::types::OtelHttpProtocol::Binary,
|
||||
tls: None,
|
||||
}),
|
||||
metrics_exporter: Some(OtelExporterKind::None),
|
||||
..Default::default()
|
||||
});
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
cfg,
|
||||
ConfigOverrides {
|
||||
cwd: Some(fixture.cwd_path()),
|
||||
..Default::default()
|
||||
},
|
||||
fixture.codex_home(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(matches!(
|
||||
config.otel.exporter,
|
||||
OtelExporterKind::OtlpHttp { .. }
|
||||
));
|
||||
assert_eq!(config.otel.trace_exporter, OtelExporterKind::None);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_null_service_tier_override_sets_fast_default_opt_out() -> std::io::Result<()> {
|
||||
let fixture = create_test_fixture()?;
|
||||
|
||||
@@ -3209,7 +3209,10 @@ impl Config {
|
||||
.environment
|
||||
.unwrap_or(DEFAULT_OTEL_ENVIRONMENT.to_string());
|
||||
let exporter = t.exporter.unwrap_or(OtelExporterKind::None);
|
||||
let trace_exporter = t.trace_exporter.unwrap_or_else(|| exporter.clone());
|
||||
// OTLP HTTP endpoints are signal-specific in our config, so
|
||||
// enabling log export must not implicitly send spans to a
|
||||
// /v1/logs endpoint.
|
||||
let trace_exporter = t.trace_exporter.unwrap_or(OtelExporterKind::None);
|
||||
let metrics_exporter = t.metrics_exporter.unwrap_or(OtelExporterKind::Statsig);
|
||||
OtelConfig {
|
||||
log_user_prompt,
|
||||
|
||||
@@ -156,6 +156,7 @@ use crate::cli::Command as ExecCommand;
|
||||
use crate::event_processor::EventProcessor;
|
||||
|
||||
const DEFAULT_ANALYTICS_ENABLED: bool = true;
|
||||
const EXEC_DEFAULT_LOG_FILTER: &str = "error,opentelemetry_sdk=off,opentelemetry_otlp=off";
|
||||
|
||||
enum InitialOperation {
|
||||
UserTurn {
|
||||
@@ -222,6 +223,14 @@ fn exec_root_span() -> tracing::Span {
|
||||
)
|
||||
}
|
||||
|
||||
fn exec_stderr_env_filter() -> EnvFilter {
|
||||
// OTEL export is best-effort; keep exporter self-diagnostics out of
|
||||
// headless command output unless the caller opts in with RUST_LOG.
|
||||
EnvFilter::try_from_default_env()
|
||||
.or_else(|_| EnvFilter::try_new(EXEC_DEFAULT_LOG_FILTER))
|
||||
.unwrap_or_else(|_| EnvFilter::new("error"))
|
||||
}
|
||||
|
||||
pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
#[allow(clippy::print_stderr)]
|
||||
if let Some(message) = cli.removed_full_auto_warning() {
|
||||
@@ -268,18 +277,10 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
supports_color::on_cached(Stream::Stderr).is_some(),
|
||||
),
|
||||
};
|
||||
// Build fmt layer (existing logging) to compose with OTEL layer.
|
||||
let default_level = "error";
|
||||
|
||||
// Build env_filter separately and attach via with_filter.
|
||||
let env_filter = EnvFilter::try_from_default_env()
|
||||
.or_else(|_| EnvFilter::try_new(default_level))
|
||||
.unwrap_or_else(|_| EnvFilter::new(default_level));
|
||||
|
||||
let fmt_layer = tracing_subscriber::fmt::layer()
|
||||
.with_ansi(stderr_with_ansi)
|
||||
.with_writer(std::io::stderr)
|
||||
.with_filter(env_filter);
|
||||
.with_filter(exec_stderr_env_filter());
|
||||
|
||||
let sandbox_mode = if removed_full_auto {
|
||||
Some(SandboxMode::WorkspaceWrite)
|
||||
|
||||
@@ -8,6 +8,10 @@ use opentelemetry::trace::TraceId;
|
||||
use opentelemetry::trace::TracerProvider as _;
|
||||
use opentelemetry_sdk::trace::SdkTracerProvider;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::io;
|
||||
use std::io::Write;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use tempfile::tempdir;
|
||||
use tracing_opentelemetry::OpenTelemetrySpanExt;
|
||||
|
||||
@@ -22,6 +26,61 @@ fn exec_defaults_analytics_to_enabled() {
|
||||
assert_eq!(DEFAULT_ANALYTICS_ENABLED, true);
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct TestLogWriter {
|
||||
buffer: Arc<Mutex<Vec<u8>>>,
|
||||
}
|
||||
|
||||
struct TestLogSink {
|
||||
buffer: Arc<Mutex<Vec<u8>>>,
|
||||
}
|
||||
|
||||
impl<'a> tracing_subscriber::fmt::MakeWriter<'a> for TestLogWriter {
|
||||
type Writer = TestLogSink;
|
||||
|
||||
fn make_writer(&'a self) -> Self::Writer {
|
||||
TestLogSink {
|
||||
buffer: Arc::clone(&self.buffer),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Write for TestLogSink {
|
||||
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
|
||||
self.buffer.lock().expect("log buffer lock").extend(buf);
|
||||
Ok(buf.len())
|
||||
}
|
||||
|
||||
fn flush(&mut self) -> io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_default_stderr_filter_suppresses_otel_self_diagnostics() {
|
||||
let buffer = Arc::new(Mutex::new(Vec::new()));
|
||||
let writer = TestLogWriter {
|
||||
buffer: Arc::clone(&buffer),
|
||||
};
|
||||
let subscriber = tracing_subscriber::registry().with(
|
||||
tracing_subscriber::fmt::layer()
|
||||
.with_ansi(false)
|
||||
.with_writer(writer)
|
||||
.with_filter(EnvFilter::try_new(EXEC_DEFAULT_LOG_FILTER).expect("default filter")),
|
||||
);
|
||||
|
||||
tracing::subscriber::with_default(subscriber, || {
|
||||
tracing::error!(target: "opentelemetry_sdk", "telemetry export failed");
|
||||
tracing::error!(target: "opentelemetry_otlp", "telemetry request failed");
|
||||
tracing::error!(target: "codex_exec_test", "real exec error");
|
||||
});
|
||||
|
||||
let logs = String::from_utf8(buffer.lock().expect("log buffer lock").clone()).expect("utf8");
|
||||
assert!(!logs.contains("telemetry export failed"));
|
||||
assert!(!logs.contains("telemetry request failed"));
|
||||
assert!(logs.contains("real exec error"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_root_span_can_be_parented_from_trace_context() {
|
||||
let subscriber = test_tracing_subscriber();
|
||||
|
||||
Reference in New Issue
Block a user