mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Add structured telemetry for response retries (#38452)
## What changed - Emit trace-safe `codex.retry` events immediately before retry delays, including the attempt number, selected delay, retry layer, and operation. - Cover HTTP requests, sampling streams, remote compaction, and sampling connection recovery. Track connection-recovery attempts separately from the stream retry budget. - Add integration coverage for retry timing and terminal behavior across HTTP, SSE, WebSocket, and remote-compaction paths, including rate limits, overloads, connection failures, and `Retry-After` inputs. GitOrigin-RevId: dae38900a0579cf8ba062c3f3d90ee61851c1c99
This commit is contained in:
committed by
copyberry
parent
813dc5f08d
commit
1b4ea8b3be
2
codex-rs/Cargo.lock
generated
2
codex-rs/Cargo.lock
generated
@@ -2463,6 +2463,7 @@ dependencies = [
|
||||
"http 1.4.0",
|
||||
"rand 0.9.3",
|
||||
"tokio",
|
||||
"tracing",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -2746,6 +2747,7 @@ dependencies = [
|
||||
"codex-app-server-protocol",
|
||||
"codex-apply-patch",
|
||||
"codex-async-utils",
|
||||
"codex-client",
|
||||
"codex-code-mode",
|
||||
"codex-config",
|
||||
"codex-connectors",
|
||||
|
||||
@@ -11,6 +11,7 @@ futures = { workspace = true }
|
||||
http = { workspace = true }
|
||||
rand = { workspace = true }
|
||||
tokio = { workspace = true, features = ["macros", "rt", "time", "sync"] }
|
||||
tracing = { workspace = true }
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
@@ -3,6 +3,7 @@ mod sse;
|
||||
mod telemetry;
|
||||
|
||||
pub use crate::retry::RetryOn;
|
||||
pub use crate::retry::RetryOperation;
|
||||
pub use crate::retry::RetryPolicy;
|
||||
pub use crate::retry::backoff;
|
||||
pub use crate::retry::run_with_retry;
|
||||
|
||||
@@ -3,7 +3,6 @@ use codex_http_client::TransportError;
|
||||
use rand::Rng;
|
||||
use std::future::Future;
|
||||
use std::time::Duration;
|
||||
use tokio::time::sleep;
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct RetryPolicy {
|
||||
@@ -48,6 +47,36 @@ pub fn backoff(base: Duration, attempt: u64) -> Duration {
|
||||
Duration::from_millis((raw as f64 * jitter) as u64)
|
||||
}
|
||||
|
||||
/// Identifies a retry path and its associated trace-event layer.
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub enum RetryOperation {
|
||||
HttpRequest,
|
||||
Sampling,
|
||||
RemoteCompactionV2,
|
||||
}
|
||||
|
||||
/// Emits retry telemetry at the caller's source location without adding it to normal OTEL logs.
|
||||
#[macro_export]
|
||||
macro_rules! record_retry {
|
||||
($attempt:expr, $delay:expr, $operation:expr $(,)?) => {{
|
||||
let (layer, operation) = match $operation {
|
||||
$crate::RetryOperation::HttpRequest => ("http", "request"),
|
||||
$crate::RetryOperation::Sampling => ("stream", "sampling"),
|
||||
$crate::RetryOperation::RemoteCompactionV2 => ("stream", "remote_compaction_v2"),
|
||||
};
|
||||
|
||||
::tracing::event!(
|
||||
target: "codex_otel.trace_safe",
|
||||
::tracing::Level::TRACE,
|
||||
event.name = "codex.retry",
|
||||
retry.attempt = $attempt,
|
||||
retry.delay_ms = ($delay).as_millis() as u64,
|
||||
retry.layer = layer,
|
||||
retry.operation = operation,
|
||||
);
|
||||
}};
|
||||
}
|
||||
|
||||
pub async fn run_with_retry<T, F, Fut>(
|
||||
policy: RetryPolicy,
|
||||
mut make_req: impl FnMut() -> Request,
|
||||
@@ -66,7 +95,10 @@ where
|
||||
.retry_on
|
||||
.should_retry(&err, attempt, policy.max_attempts) =>
|
||||
{
|
||||
sleep(backoff(policy.base_delay, attempt + 1)).await;
|
||||
let retry_attempt = attempt + 1;
|
||||
let delay = backoff(policy.base_delay, retry_attempt);
|
||||
crate::record_retry!(retry_attempt, delay, RetryOperation::HttpRequest);
|
||||
tokio::time::sleep(delay).await;
|
||||
}
|
||||
Err(err) => return Err(err),
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ codex-api = { workspace = true }
|
||||
codex-app-server-protocol = { workspace = true }
|
||||
codex-apply-patch = { workspace = true }
|
||||
codex-async-utils = { workspace = true }
|
||||
codex-client = { workspace = true }
|
||||
codex-code-mode = { workspace = true }
|
||||
codex-connectors = { workspace = true }
|
||||
codex-context-fragments = { workspace = true }
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::client::ModelClientSession;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::util::backoff;
|
||||
use codex_client::RetryOperation;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::CodexErrorDetails;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -23,6 +24,7 @@ pub(crate) enum ResponsesStreamRequest {
|
||||
|
||||
pub(crate) struct ResponsesStreamRetryState {
|
||||
retries: u64,
|
||||
connection_retries: u64,
|
||||
connection_retry_delay: Duration,
|
||||
}
|
||||
|
||||
@@ -30,6 +32,7 @@ impl Default for ResponsesStreamRetryState {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
retries: 0,
|
||||
connection_retries: 0,
|
||||
connection_retry_delay: INITIAL_CONNECTION_RETRY_DELAY,
|
||||
}
|
||||
}
|
||||
@@ -46,6 +49,11 @@ pub(crate) async fn handle_retryable_response_stream_error(
|
||||
turn_context: &TurnContext,
|
||||
request: ResponsesStreamRequest,
|
||||
) -> Result<(), CodexErr> {
|
||||
let operation = match request {
|
||||
ResponsesStreamRequest::Sampling => RetryOperation::Sampling,
|
||||
ResponsesStreamRequest::RemoteCompactionV2 => RetryOperation::RemoteCompactionV2,
|
||||
};
|
||||
|
||||
if matches!(request, ResponsesStreamRequest::Sampling)
|
||||
&& matches!(err.details(), CodexErrorDetails::ConnectionFailed(_))
|
||||
&& !turn_context.session_source.is_internal()
|
||||
@@ -60,6 +68,8 @@ pub(crate) async fn handle_retryable_response_stream_error(
|
||||
);
|
||||
sess.notify_stream_error(turn_context, "Reconnecting... waiting for network", err)
|
||||
.await;
|
||||
retry_state.connection_retries = retry_state.connection_retries.saturating_add(1);
|
||||
codex_client::record_retry!(retry_state.connection_retries, retry_delay, operation);
|
||||
tokio::time::sleep(retry_delay).await;
|
||||
retry_state.connection_retry_delay = retry_delay
|
||||
.saturating_mul(2)
|
||||
@@ -105,6 +115,7 @@ pub(crate) async fn handle_retryable_response_stream_error(
|
||||
)
|
||||
.await;
|
||||
}
|
||||
codex_client::record_retry!(retry_count, delay, operation);
|
||||
tokio::time::sleep(delay).await;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -125,6 +125,7 @@ mod responses_lite;
|
||||
mod responses_system_proxy;
|
||||
mod resume;
|
||||
mod resume_warning;
|
||||
mod retry_after;
|
||||
mod review;
|
||||
mod rmcp_client;
|
||||
mod rollout_budget;
|
||||
|
||||
1728
codex-rs/core/tests/suite/retry_after.rs
Normal file
1728
codex-rs/core/tests/suite/retry_after.rs
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user