mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
Add optional process metrics sampling for code-mode hosts (#57841)
## What changed - Add `GetProcessMetrics` and provider sampling APIs for process ID, start time, cumulative CPU time, and resident memory, exposed through `ThreadManager` without creating a thread or session. - Distinguish unsupported collection, a locally absent host, and sampling failures. Process-owned providers never start or replace a host when sampling. - Bound gRPC sampling to one second without retries or explicit connection resets. Run optional collectors on a detached worker with at most one collection in flight, so stalled collection cannot block the HTTP/2 driver or runtime shutdown. ## Testing Add coverage for snapshots, collector errors and panics, deadlines, overload, and shutdown with a stuck collector. Extend lifecycle tests to check that sampling preserves session state and keepalive behavior without starting or restarting local hosts. GitOrigin-RevId: eeafe275891d0464636f28a29345a87aed1a12dc
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
//! Owns the local gRPC code-mode client and host process. Sessions share a lazy
|
||||
//! HTTP/2 connection. Only session admission can replace a dead host; other RPCs
|
||||
//! use the current generation. Dropping its I/O kills and reaps the child.
|
||||
//! HTTP/2 connection. Only session admission can replace a dead host; metrics
|
||||
//! use the current generation and cannot reconnect. Dropping its I/O reaps the child.
|
||||
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
@@ -17,9 +17,11 @@ use std::task::Context;
|
||||
use std::task::Poll;
|
||||
use std::time::Duration;
|
||||
|
||||
use codex_code_mode::CodeModeProcessMetricsSample;
|
||||
use codex_code_mode::CodeModeSessionCellExecutionLimits;
|
||||
use codex_code_mode::CodeModeSessionProvider;
|
||||
use codex_code_mode::CodeModeSessionProviderFuture;
|
||||
use codex_code_mode::CodeModeSessionResultFuture;
|
||||
use codex_code_mode::GrpcCodeModeSessionProvider;
|
||||
use codex_code_mode::ProcessOwnedCodeModeSessionProvider;
|
||||
use codex_protocol::shell_environment::scrub_non_inheritable_env_vars;
|
||||
@@ -63,6 +65,26 @@ impl CodeModeSessionProvider for ProcessOwnedGrpcCodeModeSessionProvider {
|
||||
.availability()
|
||||
}
|
||||
|
||||
fn sample_process_metrics(
|
||||
&self,
|
||||
) -> CodeModeSessionResultFuture<'_, CodeModeProcessMetricsSample> {
|
||||
// Absence is local lifecycle state, not a meaning assigned to any remote
|
||||
// gRPC error. A child dying after this observation produces a failed sample.
|
||||
if !self
|
||||
.current
|
||||
.lock()
|
||||
.unwrap_or_else(PoisonError::into_inner)
|
||||
.as_ref()
|
||||
.is_some_and(|host| host.alive.load(Ordering::Acquire))
|
||||
{
|
||||
return Box::pin(async { Ok(CodeModeProcessMetricsSample::NotRunning) });
|
||||
}
|
||||
match self.provider.get() {
|
||||
Some(provider) => provider.sample_process_metrics(),
|
||||
None => Box::pin(async { Ok(CodeModeProcessMetricsSample::NotRunning) }),
|
||||
}
|
||||
}
|
||||
|
||||
fn create_session(&self) -> CodeModeSessionProviderFuture<'_> {
|
||||
self.create_session_with_limits(CodeModeSessionCellExecutionLimits::default())
|
||||
}
|
||||
@@ -89,8 +111,8 @@ impl CodeModeSessionProvider for ProcessOwnedGrpcCodeModeSessionProvider {
|
||||
}
|
||||
|
||||
// Every channel gets exactly one child. Tonic's internal reconnect loop cannot
|
||||
// spawn a process, including if the child dies between channel selection and
|
||||
// dispatch. Session admission selects a new generation during recovery.
|
||||
// spawn a process, including if the child dies between a metrics liveness check
|
||||
// and dispatch. Session admission selects a new generation during recovery.
|
||||
struct HostTransport {
|
||||
host_program: PathBuf,
|
||||
current: Arc<Mutex<Option<HostChannel>>>,
|
||||
@@ -133,7 +155,7 @@ impl HostTransport {
|
||||
.map_err(|error| Status::unavailable(error.to_string()))?;
|
||||
let alive = connection.alive.clone();
|
||||
let connection = Arc::new(Mutex::new(Some(connection)));
|
||||
// Detect a live but unresponsive child even while the connection is idle.
|
||||
// Preserve execution liveness detection independently of metrics cadence.
|
||||
let channel = Endpoint::from_static("http://code-mode-host")
|
||||
.http2_keep_alive_interval(Duration::from_secs(/*secs*/ 30))
|
||||
.keep_alive_timeout(Duration::from_secs(/*secs*/ 10))
|
||||
|
||||
@@ -4,11 +4,13 @@
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_code_mode::CodeModeProcessMetricsSample;
|
||||
use codex_code_mode::CodeModeSessionProvider;
|
||||
use codex_code_mode::FunctionCallOutputContentItem;
|
||||
use codex_code_mode::GrpcCodeModeSessionProvider;
|
||||
@@ -18,6 +20,7 @@ use pretty_assertions::assert_eq;
|
||||
use tokio::sync::Notify;
|
||||
use tokio::sync::Semaphore;
|
||||
use tokio::time::timeout;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tonic::Status;
|
||||
use tonic::body::Body;
|
||||
use tower::ServiceExt;
|
||||
@@ -105,6 +108,25 @@ async fn child_exit_after_channel_selection_does_not_replay_execution() -> Resul
|
||||
)
|
||||
.await?
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
let samples = Arc::new(AtomicUsize::new(0));
|
||||
let stop = CancellationToken::new();
|
||||
let sampler = tokio::spawn({
|
||||
let provider = provider.clone();
|
||||
let samples = samples.clone();
|
||||
let stop = stop.clone();
|
||||
async move {
|
||||
let mut interval = tokio::time::interval(Duration::from_millis(/*millis*/ 5));
|
||||
loop {
|
||||
tokio::select! {
|
||||
_ = stop.cancelled() => break,
|
||||
_ = interval.tick() => {
|
||||
let _ = provider.sample_process_metrics().await;
|
||||
samples.fetch_add(1, Ordering::SeqCst);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
armed.store(true, Ordering::SeqCst);
|
||||
let pending = tokio::spawn({
|
||||
let session = sessions[0].clone();
|
||||
@@ -123,6 +145,8 @@ async fn child_exit_after_channel_selection_does_not_replay_execution() -> Resul
|
||||
})
|
||||
.await
|
||||
.context("child was not reaped")?;
|
||||
let absent = provider.sample_process_metrics().await.unwrap_err();
|
||||
assert!(absent.contains("host is not running"), "{absent}");
|
||||
assert_eq!(recorder.invocations()?.len(), 1);
|
||||
dispatch.add_permits(1);
|
||||
assert!(timeout(TEST_TIMEOUT, pending).await??.is_err());
|
||||
@@ -147,6 +171,16 @@ async fn child_exit_after_channel_selection_does_not_replay_execution() -> Resul
|
||||
Ok::<_, anyhow::Error>(())
|
||||
}));
|
||||
timeout(TEST_TIMEOUT, recovery).await??;
|
||||
assert_eq!(
|
||||
provider
|
||||
.sample_process_metrics()
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?,
|
||||
CodeModeProcessMetricsSample::Unsupported
|
||||
);
|
||||
stop.cancel();
|
||||
timeout(TEST_TIMEOUT, sampler).await??;
|
||||
assert!(samples.load(Ordering::SeqCst) > 0);
|
||||
assert_eq!(recorder.invocations()?.len(), 2);
|
||||
for session in sessions {
|
||||
session.shutdown().await.map_err(anyhow::Error::msg)?;
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_code_mode::CodeModeProcessMetricsSample;
|
||||
use codex_code_mode::CodeModeSession;
|
||||
use codex_code_mode::CodeModeSessionProvider;
|
||||
use codex_code_mode::ExecuteRequest;
|
||||
@@ -68,8 +69,24 @@ async fn grpc_stdio_sessions_share_a_host_and_isolate_state() -> Result<()> {
|
||||
#[cfg(not(unix))]
|
||||
let host_program = codex_utils_cargo_bin::cargo_bin("codex-code-mode-host")?;
|
||||
let provider = ProcessOwnedGrpcCodeModeSessionProvider::with_host_program(host_program);
|
||||
assert_eq!(
|
||||
provider
|
||||
.sample_process_metrics()
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?,
|
||||
CodeModeProcessMetricsSample::NotRunning
|
||||
);
|
||||
#[cfg(unix)]
|
||||
assert!(recorder.invocations()?.is_empty());
|
||||
let (first, second) = tokio::try_join!(provider.create_session(), provider.create_session())
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
assert_eq!(
|
||||
provider
|
||||
.sample_process_metrics()
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?,
|
||||
CodeModeProcessMetricsSample::Unsupported
|
||||
);
|
||||
assert_eq!(
|
||||
execute(&first, "store(\"stored\", 42); text(load(\"stored\"));").await?,
|
||||
vec![FunctionCallOutputContentItem::InputText {
|
||||
@@ -111,7 +128,7 @@ async fn grpc_stdio_sessions_share_a_host_and_isolate_state() -> Result<()> {
|
||||
}
|
||||
})
|
||||
.await
|
||||
.context("host was not reaped after provider teardown")?;
|
||||
.context("metrics retained the host after provider teardown")?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -143,6 +160,16 @@ async fn hung_grpc_stdio_host_is_reaped_and_the_session_recovers() -> Result<()>
|
||||
if unsafe { libc::kill(pid as libc::pid_t, libc::SIGSTOP) } != 0 {
|
||||
return Err(std::io::Error::last_os_error().into());
|
||||
}
|
||||
// A metrics deadline cancels only its stream. It must neither reap the
|
||||
// suspended child nor advance the independent 30s/10s keepalive deadline.
|
||||
assert!(provider.sample_process_metrics().await.is_err());
|
||||
assert_eq!(
|
||||
unsafe {
|
||||
libc::kill(pid as libc::pid_t, /*sig*/ 0)
|
||||
},
|
||||
0
|
||||
);
|
||||
assert_eq!(recorder.invocations()?.len(), 1);
|
||||
timeout(Duration::from_secs(/*secs*/ 55), async {
|
||||
while unsafe {
|
||||
libc::kill(pid as libc::pid_t, /*sig*/ 0)
|
||||
@@ -153,6 +180,18 @@ async fn hung_grpc_stdio_host_is_reaped_and_the_session_recovers() -> Result<()>
|
||||
})
|
||||
.await
|
||||
.context("unresponsive host was not killed and reaped")?;
|
||||
assert_eq!(
|
||||
provider
|
||||
.sample_process_metrics()
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?,
|
||||
CodeModeProcessMetricsSample::NotRunning
|
||||
);
|
||||
assert_eq!(
|
||||
recorder.invocations()?.len(),
|
||||
1,
|
||||
"metrics must not restart a dead host"
|
||||
);
|
||||
let reset = execute(&session, r#"store("recovery", "must not run");"#)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
@@ -715,6 +715,18 @@ impl ThreadManager {
|
||||
self
|
||||
}
|
||||
|
||||
/// Samples host metrics without creating a Codex thread or code-mode session.
|
||||
/// Process-owned providers observe existing children; custom providers retain
|
||||
/// their transport's connection policy.
|
||||
pub async fn sample_code_mode_process_metrics(
|
||||
&self,
|
||||
) -> Result<codex_code_mode::CodeModeProcessMetricsSample, String> {
|
||||
self.state
|
||||
.code_mode_session_provider
|
||||
.sample_process_metrics()
|
||||
.await
|
||||
}
|
||||
|
||||
/// Replaces the process-wide provider before this manager is shared with threads.
|
||||
pub fn with_code_mode_session_provider(
|
||||
mut self,
|
||||
|
||||
Reference in New Issue
Block a user