[codex] Observe remote exec-server lifecycle (#27470)

## Summary

- Record bounded duration and outcome metrics for remote environment
registration and Noise rendezvous connection attempts.
- Count reconnects by bounded reason: disconnect, connection failure, or
rejected registration.
- Trace registration at the owning client boundary without exporting raw
environment or registration identifiers.
- Replace the stale pre-Noise WebSocket observability design with the
current remote transport model.

## Stack

Review and land this stack in order:

1. #27466 — trace exec-server JSON-RPC requests
2. #27467 — record bounded connection, request, and process lifecycle
metrics
3. #27470 — observe remote registration and Noise rendezvous lifecycle
**(this PR)**

## Validation

- `just test -p codex-exec-server --lib` (149 passed)
- `just test -p codex-cli --test exec_server` (4 passed)
- `just argument-comment-lint`
- `just bazel-lock-check`
- `just fix -p codex-exec-server -p codex-cli`
- `just fmt`
This commit is contained in:
richardopenai
2026-06-25 13:42:40 -07:00
committed by GitHub
parent 3b78f58fb2
commit 3b22498f69
9 changed files with 381 additions and 24 deletions
+66
View File
@@ -1,3 +1,5 @@
use std::future::Future;
use tracing_subscriber::EnvFilter;
use tracing_subscriber::prelude::*;
@@ -43,6 +45,70 @@ pub(crate) fn init(
(otel, telemetry)
}
pub(crate) async fn run_until_shutdown<F, E>(run: F) -> Result<(), E>
where
F: Future<Output = Result<(), E>>,
{
let shutdown_signal = match shutdown_signal() {
Ok(signal) => Some(signal),
Err(error) => {
eprintln!("Could not listen for exec-server shutdown signal: {error}");
None
}
};
tokio::pin!(run);
if let Some(shutdown_signal) = shutdown_signal {
tokio::select! {
result = &mut run => result,
signal = wait_for_shutdown_signal(shutdown_signal) => {
match signal {
Ok(()) => Ok(()),
Err(error) => {
eprintln!("Could not listen for exec-server shutdown signal: {error}");
run.await
}
}
}
}
} else {
run.await
}
}
#[cfg(unix)]
struct ShutdownSignal {
terminate: tokio::signal::unix::Signal,
}
#[cfg(unix)]
fn shutdown_signal() -> std::io::Result<ShutdownSignal> {
Ok(ShutdownSignal {
terminate: tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate())?,
})
}
#[cfg(unix)]
async fn wait_for_shutdown_signal(mut shutdown_signal: ShutdownSignal) -> std::io::Result<()> {
tokio::select! {
result = tokio::signal::ctrl_c() => result,
_ = shutdown_signal.terminate.recv() => Ok(()),
}
}
#[cfg(not(unix))]
struct ShutdownSignal;
#[cfg(not(unix))]
fn shutdown_signal() -> std::io::Result<ShutdownSignal> {
Ok(ShutdownSignal)
}
#[cfg(not(unix))]
async fn wait_for_shutdown_signal(_: ShutdownSignal) -> std::io::Result<()> {
tokio::signal::ctrl_c().await
}
fn stderr_env_filter() -> EnvFilter {
EnvFilter::try_from_default_env()
.or_else(|_| EnvFilter::try_new(DEFAULT_LOG_FILTER))
+10 -6
View File
@@ -1706,7 +1706,10 @@ async fn run_exec_server_command(
remote_config.name = name;
}
let remote_config = remote_config.with_telemetry(telemetry);
codex_exec_server::run_remote_environment(remote_config, runtime_paths).await?;
exec_server_telemetry::run_until_shutdown(async move {
codex_exec_server::run_remote_environment(remote_config, runtime_paths).await
})
.await?;
Ok(())
} else {
let config_result = load_exec_server_config(root_config_overrides, strict_config).await;
@@ -1718,11 +1721,12 @@ async fn run_exec_server_command(
let (_otel, telemetry) = exec_server_telemetry::init(config.as_ref());
let listen_url = cmd
.listen
.as_deref()
.unwrap_or(codex_exec_server::DEFAULT_LISTEN_URL);
codex_exec_server::run_main_with_telemetry(listen_url, runtime_paths, telemetry)
.await
.map_err(anyhow::Error::from_boxed)
.unwrap_or_else(|| codex_exec_server::DEFAULT_LISTEN_URL.to_string());
exec_server_telemetry::run_until_shutdown(async move {
codex_exec_server::run_main_with_telemetry(&listen_url, runtime_paths, telemetry).await
})
.await
.map_err(anyhow::Error::from_boxed)
}
}
+62
View File
@@ -1,6 +1,20 @@
#[cfg(unix)]
use std::io::BufRead as _;
#[cfg(unix)]
use std::io::BufReader as StdBufReader;
#[cfg(unix)]
use std::io::Read as _;
#[cfg(unix)]
use std::io::Write as _;
#[cfg(unix)]
use std::net::TcpStream;
use std::path::Path;
use std::process::Stdio;
#[cfg(unix)]
use std::thread;
use std::time::Duration;
#[cfg(unix)]
use std::time::Instant;
use anyhow::Result;
use predicates::prelude::PredicateBooleanExt;
@@ -220,6 +234,54 @@ async fn send_json_line(
Ok(())
}
#[cfg(unix)]
#[test]
fn local_exec_server_exits_successfully_on_sigterm() -> Result<()> {
let codex_home = TempDir::new()?;
let mut child = std::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?)
.env("CODEX_HOME", codex_home.path())
.args(["exec-server", "--listen", "ws://127.0.0.1:0"])
.stdout(Stdio::piped())
.spawn()?;
let mut listen_url = String::new();
StdBufReader::new(child.stdout.take().expect("child stdout")).read_line(&mut listen_url)?;
assert!(listen_url.starts_with("ws://127.0.0.1:"), "{listen_url}");
let listen_addr = listen_url
.trim()
.strip_prefix("ws://")
.expect("listen URL should use ws://")
.parse()?;
let deadline = Instant::now() + Duration::from_secs(5);
let mut ready = false;
while let Some(remaining) = deadline.checked_duration_since(Instant::now()) {
if let Ok(mut stream) =
TcpStream::connect_timeout(&listen_addr, remaining.min(Duration::from_millis(100)))
{
let _ = stream.set_read_timeout(Some(Duration::from_secs(1)));
let request =
format!("GET /readyz HTTP/1.1\r\nHost: {listen_addr}\r\nConnection: close\r\n\r\n");
let mut response = String::new();
if stream.write_all(request.as_bytes()).is_ok()
&& stream.read_to_string(&mut response).is_ok()
&& response.starts_with("HTTP/1.1 200")
{
ready = true;
break;
}
}
thread::sleep(Duration::from_millis(10));
}
assert!(ready, "exec-server did not become ready at {listen_url}");
// SAFETY: `child.id()` is the live process spawned above.
let result = unsafe { libc::kill(child.id() as libc::pid_t, libc::SIGTERM) };
assert_eq!(result, 0);
let status = child.wait()?;
assert!(status.success(), "{status}");
Ok(())
}
async fn wait_for_response(
stdout: &mut (impl tokio::io::AsyncBufRead + Unpin),
expected_id: i64,
+21 -1
View File
@@ -8,6 +8,7 @@ use tokio::process::Command;
use tokio::time::timeout;
use tokio_tungstenite::connect_async;
use tokio_tungstenite::connect_async_with_config;
use tokio_tungstenite::tungstenite::client::IntoClientRequest;
use tracing::debug;
use tracing::warn;
@@ -30,6 +31,7 @@ use crate::noise_relay::NoiseHarnessConnectionArgs;
use crate::noise_relay::noise_harness_connection_from_websocket;
use crate::noise_relay::noise_relay_websocket_config;
use crate::relay::harness_connection_from_websocket;
use crate::trace_context::current_trace_context_headers;
const ENVIRONMENT_CLIENT_NAME: &str = "codex-environment";
@@ -216,6 +218,14 @@ impl ExecServerClient {
/// only ciphertext after that. Environment-managed connections use a
/// retained [`NoiseRendezvousConnectProvider`] so recovery can fetch a fresh
/// bundle for each reconnect.
#[tracing::instrument(
name = "codex.exec_server.remote.harness.connect",
skip_all,
fields(
otel.kind = "client",
otel.name = "codex.exec_server.remote.harness.connect",
)
)]
pub async fn connect_noise_rendezvous(
args: NoiseRendezvousConnectArgs,
) -> Result<Self, ExecServerError> {
@@ -249,10 +259,20 @@ impl ExecServerClient {
.next()
.unwrap_or(websocket_url.as_str())
.to_string();
let mut request = websocket_url
.as_str()
.into_client_request()
.map_err(|source| ExecServerError::WebSocketConnect {
url: diagnostic_url.clone(),
source,
})?;
request
.headers_mut()
.extend(current_trace_context_headers());
let (stream, _) = timeout(
connect_timeout,
connect_async_with_config(
websocket_url.as_str(),
request,
Some(noise_relay_websocket_config()),
/*disable_nagle*/ false,
),
+1
View File
@@ -28,6 +28,7 @@ mod runtime_paths;
mod sandboxed_file_system;
mod server;
mod telemetry;
mod trace_context;
use codex_exec_server_protocol as protocol;
+112 -17
View File
@@ -1,5 +1,6 @@
use std::sync::Arc;
use std::time::Duration;
use std::time::Instant;
use codex_api::AuthProvider;
use codex_api::SharedAuthProvider;
@@ -10,7 +11,10 @@ use http::HeaderValue;
use reqwest::StatusCode;
use serde::Deserialize;
use tokio::time::sleep;
use tokio_tungstenite::MaybeTlsStream;
use tokio_tungstenite::WebSocketStream;
use tokio_tungstenite::connect_async_with_config;
use tokio_tungstenite::tungstenite::client::IntoClientRequest;
use tracing::debug;
use tracing::info;
use tracing::warn;
@@ -35,6 +39,7 @@ use crate::noise_relay::noise_relay_websocket_config;
use crate::relay::HarnessKeyValidator;
use crate::relay::run_multiplexed_environment;
use crate::server::ConnectionProcessor;
use crate::trace_context::current_trace_context_headers;
const ERROR_BODY_PREVIEW_BYTES: usize = 4096;
const NOISE_RELAY_SECURITY_PROFILE: &str = "noise_hybrid_ik_v1";
@@ -45,6 +50,7 @@ struct EnvironmentRegistryClient {
auth_provider: SharedAuthProvider,
http: reqwest::Client,
connect_timeout: Duration,
telemetry: ExecServerTelemetry,
}
impl std::fmt::Debug for EnvironmentRegistryClient {
@@ -57,7 +63,16 @@ impl std::fmt::Debug for EnvironmentRegistryClient {
}
impl EnvironmentRegistryClient {
#[cfg(test)]
fn new(base_url: String, auth_provider: SharedAuthProvider) -> Result<Self, ExecServerError> {
Self::new_with_telemetry(base_url, auth_provider, ExecServerTelemetry::default())
}
fn new_with_telemetry(
base_url: String,
auth_provider: SharedAuthProvider,
telemetry: ExecServerTelemetry,
) -> Result<Self, ExecServerError> {
let base_url = normalize_base_url(base_url)?;
Ok(Self {
base_url,
@@ -66,15 +81,41 @@ impl EnvironmentRegistryClient {
.redirect(reqwest::redirect::Policy::none())
.build()?,
connect_timeout: DEFAULT_REMOTE_EXEC_SERVER_CONNECT_TIMEOUT,
telemetry,
})
}
/// Register the executor public key and obtain the rendezvous allocation.
/// The returned registration ID is included in each stream's Noise prologue.
#[tracing::instrument(
name = "codex.exec_server.remote.register",
skip_all,
fields(
otel.kind = "client",
otel.name = "codex.exec_server.remote.register",
result = tracing::field::Empty,
)
)]
async fn register_environment(
&self,
environment_id: &str,
executor_public_key: &NoiseChannelPublicKey,
) -> Result<EnvironmentRegistryRegistrationResponse, ExecServerError> {
let started_at = Instant::now();
let response = self
.register_environment_inner(environment_id, executor_public_key)
.await;
let result = if response.is_ok() { "success" } else { "error" };
tracing::Span::current().record("result", result);
self.telemetry
.remote_registration_completed(result, started_at.elapsed());
response
}
async fn register_environment_inner(
&self,
environment_id: &str,
executor_public_key: &NoiseChannelPublicKey,
) -> Result<EnvironmentRegistryRegistrationResponse, ExecServerError> {
let response = self
.http
@@ -83,6 +124,7 @@ impl EnvironmentRegistryClient {
&format!("/cloud/environment/{environment_id}/register"),
))
.headers(self.auth_provider.to_auth_headers())
.headers(current_trace_context_headers())
.json(&EnvironmentRegistryRegistrationRequest {
security_profile: NOISE_RELAY_SECURITY_PROFILE.to_string(),
executor_public_key: executor_public_key.clone(),
@@ -269,7 +311,11 @@ impl NoiseRendezvousEnvironmentConfig {
) -> Result<Self, ExecServerError> {
let environment_id = normalize_environment_id(environment_id)?;
let auth_provider = static_bearer_auth_provider(bearer_token, chatgpt_account_id)?;
let client = EnvironmentRegistryClient::new(base_url, auth_provider)?;
let client = EnvironmentRegistryClient::new_with_telemetry(
base_url,
auth_provider,
ExecServerTelemetry::default(),
)?;
Ok(Self {
provider: Arc::new(EnvironmentRegistryNoiseConnectProvider {
client,
@@ -416,18 +462,16 @@ impl RemoteEnvironmentConfig {
/// reconnects. The registration and rendezvous URL are also reused until
/// rendezvous rejects the URL, at which point the next attempt registers again.
/// The websocket carries cleartext routing metadata and encrypted payloads.
#[tracing::instrument(
name = "codex.exec_server",
skip_all,
fields(otel.kind = "internal")
)]
pub async fn run_remote_environment(
config: RemoteEnvironmentConfig,
runtime_paths: ExecServerRuntimePaths,
) -> Result<(), ExecServerError> {
ensure_rustls_crypto_provider();
let client =
EnvironmentRegistryClient::new(config.base_url.clone(), config.auth_provider.clone())?;
let client = EnvironmentRegistryClient::new_with_telemetry(
config.base_url.clone(),
config.auth_provider.clone(),
config.telemetry.clone(),
)?;
let processor =
ConnectionProcessor::new_with_telemetry(runtime_paths, config.telemetry.clone());
let identity = NoiseChannelIdentity::generate().map_err(|error| {
@@ -439,14 +483,8 @@ pub async fn run_remote_environment(
.await?;
loop {
match connect_async_with_config(
response.url.as_str(),
Some(noise_relay_websocket_config()),
/*disable_nagle*/ false,
)
.await
{
Ok((websocket, _)) => {
match connect_rendezvous(&response.url, &config.telemetry).await {
Ok(websocket) => {
backoff = Duration::from_secs(1);
let executor_registration_id = response.executor_registration_id.clone();
info!(
@@ -467,6 +505,7 @@ pub async fn run_remote_environment(
},
)
.await;
config.telemetry.remote_reconnect("disconnected");
}
Err(error) => {
let registration_rejected = matches!(
@@ -482,9 +521,12 @@ pub async fn run_remote_environment(
);
debug!(error = %error, "Noise executor rendezvous connection error");
if registration_rejected {
config.telemetry.remote_reconnect("registration_rejected");
response = client
.register_environment(&config.environment_id, &identity.public_key())
.await?;
} else {
config.telemetry.remote_reconnect("connect_failed");
}
}
}
@@ -494,6 +536,43 @@ pub async fn run_remote_environment(
}
}
#[tracing::instrument(
name = "codex.exec_server.remote.rendezvous.connect",
skip_all,
fields(
otel.kind = "client",
otel.name = "codex.exec_server.remote.rendezvous.connect",
result = tracing::field::Empty,
)
)]
async fn connect_rendezvous(
url: &str,
telemetry: &ExecServerTelemetry,
) -> Result<
WebSocketStream<MaybeTlsStream<tokio::net::TcpStream>>,
tokio_tungstenite::tungstenite::Error,
> {
let started_at = Instant::now();
let result = async {
let mut request = url.into_client_request()?;
request
.headers_mut()
.extend(current_trace_context_headers());
connect_async_with_config(
request,
Some(noise_relay_websocket_config()),
/*disable_nagle*/ false,
)
.await
.map(|(websocket, _)| websocket)
}
.await;
let result_name = if result.is_ok() { "success" } else { "error" };
tracing::Span::current().record("result", result_name);
telemetry.remote_rendezvous_completed(result_name, started_at.elapsed());
result
}
fn normalize_environment_id(environment_id: String) -> Result<String, ExecServerError> {
let environment_id = environment_id.trim().to_string();
if environment_id.is_empty() {
@@ -585,12 +664,17 @@ mod tests {
use codex_api::AuthProvider;
use http::HeaderMap;
use http::HeaderValue;
use opentelemetry::trace::TracerProvider as _;
use opentelemetry_sdk::trace::SdkTracerProvider;
use pretty_assertions::assert_eq;
use tracing::Instrument;
use tracing_subscriber::prelude::*;
use wiremock::Mock;
use wiremock::MockServer;
use wiremock::ResponseTemplate;
use wiremock::matchers::body_partial_json;
use wiremock::matchers::header;
use wiremock::matchers::header_regex;
use wiremock::matchers::method;
use wiremock::matchers::path;
@@ -616,8 +700,14 @@ mod tests {
Arc::new(StaticRegistryAuthProvider)
}
#[tokio::test]
#[tokio::test(flavor = "current_thread")]
async fn register_environment_posts_with_auth_provider_headers() {
let provider = SdkTracerProvider::builder().build();
let tracer = provider.tracer("exec-server-test");
let subscriber =
tracing_subscriber::registry().with(tracing_opentelemetry::layer().with_tracer(tracer));
let _guard = subscriber.set_default();
tracing::callsite::rebuild_interest_cache();
let server = MockServer::start().await;
let executor_public_key = NoiseChannelIdentity::generate()
.expect("identity")
@@ -626,6 +716,10 @@ mod tests {
.and(path("/cloud/environment/environment-requested/register"))
.and(header("authorization", "Bearer registry-token"))
.and(header("chatgpt-account-id", "workspace-123"))
.and(header_regex(
"traceparent",
"^00-[0-9a-f]{32}-[0-9a-f]{16}-0[01]$",
))
.and(body_partial_json(serde_json::json!({
"security_profile": NOISE_RELAY_SECURITY_PROFILE,
"executor_public_key": executor_public_key.clone(),
@@ -643,6 +737,7 @@ mod tests {
let response = client
.register_environment("environment-requested", &executor_public_key)
.instrument(tracing::info_span!("remote-operation"))
.await
.expect("register environment");
+58
View File
@@ -21,6 +21,28 @@ const PROCESSES_FINISHED_TOTAL_DESCRIPTION: &str =
"Total number of finished exec-server processes.";
const PROCESS_DURATION_METRIC: &str = "exec_server_process_duration_seconds";
const PROCESS_DURATION_DESCRIPTION: &str = "Duration of exec-server processes in seconds.";
const REMOTE_REGISTRATION_METRICS: OperationMetrics = OperationMetrics {
total_name: "exec_server_remote_registration_total",
total_description: "Total number of remote exec-server registration attempts.",
duration_name: "exec_server_remote_registration_duration_seconds",
duration_description: "Duration of remote exec-server registration attempts in seconds.",
};
const REMOTE_RENDEZVOUS_METRICS: OperationMetrics = OperationMetrics {
total_name: "exec_server_remote_rendezvous_connect_total",
total_description: "Total number of remote exec-server rendezvous connection attempts.",
duration_name: "exec_server_remote_rendezvous_connect_duration_seconds",
duration_description: "Duration of remote exec-server rendezvous connection attempts in seconds.",
};
const REMOTE_RECONNECTS_TOTAL_METRIC: &str = "exec_server_remote_reconnects_total";
const REMOTE_RECONNECTS_TOTAL_DESCRIPTION: &str = "Total number of remote exec-server reconnects.";
#[derive(Clone, Copy)]
struct OperationMetrics {
total_name: &'static str,
total_description: &'static str,
duration_name: &'static str,
duration_description: &'static str,
}
#[derive(Clone, Copy)]
pub(crate) enum ConnectionTransport {
@@ -123,6 +145,24 @@ impl ExecServerTelemetry {
});
}
pub(crate) fn remote_registration_completed(&self, result: &'static str, duration: Duration) {
self.record_operation(REMOTE_REGISTRATION_METRICS, result, duration);
}
pub(crate) fn remote_rendezvous_completed(&self, result: &'static str, duration: Duration) {
self.record_operation(REMOTE_RENDEZVOUS_METRICS, result, duration);
}
pub(crate) fn remote_reconnect(&self, reason: &'static str) {
self.with_inner(|inner| {
inner.counter(
REMOTE_RECONNECTS_TOTAL_METRIC,
REMOTE_RECONNECTS_TOTAL_DESCRIPTION,
&[("reason", reason)],
);
});
}
pub(crate) fn process_started(&self) -> ProcessMetricGuard {
self.with_inner(|inner| {
inner.adjust_process_count(/*delta*/ 1);
@@ -162,6 +202,24 @@ impl ExecServerTelemetry {
emit(inner);
}
}
fn record_operation(
&self,
metrics: OperationMetrics,
result: &'static str,
duration: Duration,
) {
self.with_inner(|inner| {
let tags = [("result", result)];
inner.counter(metrics.total_name, metrics.total_description, &tags);
inner.duration(
metrics.duration_name,
metrics.duration_description,
duration,
&tags,
);
});
}
}
impl Drop for ConnectionMetricGuard {
+24
View File
@@ -0,0 +1,24 @@
use reqwest::header::HeaderMap;
use reqwest::header::HeaderValue;
pub(crate) fn current_trace_context_headers() -> HeaderMap {
let mut headers = HeaderMap::new();
let Some(trace) = codex_otel::current_span_w3c_trace_context() else {
return headers;
};
if let Some(traceparent) = trace.traceparent
&& let Ok(value) = HeaderValue::try_from(traceparent)
{
headers.insert("traceparent", value);
}
if let Some(tracestate) = trace.tracestate
&& let Ok(value) = HeaderValue::try_from(tracestate)
{
headers.insert("tracestate", value);
}
headers
}
#[cfg(test)]
#[path = "trace_context_tests.rs"]
mod tests;
@@ -0,0 +1,27 @@
use opentelemetry::trace::TracerProvider as _;
use opentelemetry_sdk::trace::SdkTracerProvider;
use tracing_subscriber::prelude::*;
use super::current_trace_context_headers;
#[test]
fn creates_traceparent_header_from_current_span() {
let provider = SdkTracerProvider::builder().build();
let tracer = provider.tracer("exec-server-test");
let subscriber =
tracing_subscriber::registry().with(tracing_opentelemetry::layer().with_tracer(tracer));
let _guard = subscriber.set_default();
tracing::callsite::rebuild_interest_cache();
let span = tracing::info_span!("outbound-request");
let _entered = span.enter();
let headers = current_trace_context_headers();
let traceparent = headers
.get("traceparent")
.expect("traceparent header")
.to_str()
.expect("valid traceparent header");
assert!(traceparent.starts_with("00-"));
assert_eq!(traceparent.len(), 55);
}