mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] add latency tracing spans (#27710)
## Why We have some large gaps in our thread start, resume, and pre-sampling traces that make it hard to tell where latency is coming from. ## What Changed - Added coarse spans around thread start/resume, turn context construction, rollout reconstruction, skill/plugin loading, and tool preparation. - Added a breakdown of discoverable-tool preparation across connector loading, plugin discovery, and local plugin details. ## Testing - `cargo check -p codex-app-server -p codex-core -p codex-core-skills -p codex-core-plugins` - Built the app-server locally and exercised thread start, first turn, follow-up turn, server restart, thread resume, and a resumed turn.
This commit is contained in:
committed by
GitHub
Unverified
parent
1460b509f4
commit
bacfc5e4c0
@@ -43,6 +43,10 @@ In the codex-rs folder where the rust code lives:
|
||||
directory reads, update the crate's `BUILD.bazel` (`compile_data`, `build_script_data`, or test
|
||||
data) or Bazel may fail even when Cargo passes.
|
||||
- Do not create small helper methods that are referenced only once.
|
||||
- For tracing async work, instrument the function or method definition with
|
||||
`#[tracing::instrument(...)]` instead of attaching spans to futures with
|
||||
`.instrument(...)` at call sites. Before adding instrumentation, check whether the callee—or
|
||||
the implementation method it immediately delegates to—is already instrumented.
|
||||
- Avoid large modules:
|
||||
- Prefer adding new modules instead of growing existing ones.
|
||||
- Target Rust modules under 500 LoC, excluding tests.
|
||||
|
||||
@@ -21,6 +21,7 @@ use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
use toml::Value as TomlValue;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
|
||||
/// Shared app-server entry point for loading effective Codex configuration.
|
||||
@@ -212,6 +213,7 @@ impl ConfigManager {
|
||||
.await
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub(crate) async fn load_with_cli_overrides(
|
||||
&self,
|
||||
cli_overrides: &[(String, TomlValue)],
|
||||
|
||||
@@ -2809,6 +2809,7 @@ impl ThreadRequestProcessor {
|
||||
Some(persisted_metadata)
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "trace", skip_all)]
|
||||
async fn resume_running_thread(
|
||||
&self,
|
||||
request_id: &ConnectionRequestId,
|
||||
@@ -2995,6 +2996,7 @@ impl ThreadRequestProcessor {
|
||||
Ok(RunningThreadResumeResult::NotRunning(None))
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "trace", skip_all)]
|
||||
async fn resume_thread_from_history(
|
||||
&self,
|
||||
history: &[ResponseItem],
|
||||
@@ -3011,6 +3013,7 @@ impl ThreadRequestProcessor {
|
||||
))
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "trace", skip_all)]
|
||||
async fn resume_thread_from_rollout(
|
||||
&self,
|
||||
thread_id: &str,
|
||||
@@ -3065,6 +3068,7 @@ impl ThreadRequestProcessor {
|
||||
Ok(stored_thread)
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "trace", skip_all)]
|
||||
async fn stored_thread_to_initial_history(
|
||||
&self,
|
||||
stored_thread: &StoredThread,
|
||||
|
||||
@@ -46,6 +46,7 @@ use std::path::Path;
|
||||
use std::process::Command;
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
|
||||
const DEFAULT_SKILLS_DIR_NAME: &str = "skills";
|
||||
@@ -111,6 +112,7 @@ struct PluginAppConfig {
|
||||
category: Option<String>,
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub async fn load_plugins_from_layer_stack(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
extra_plugins: HashMap<String, PluginConfig>,
|
||||
|
||||
@@ -77,6 +77,7 @@ use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Instant;
|
||||
use tokio::sync::Semaphore;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
|
||||
static CURATED_REPO_SYNC_STARTED: AtomicBool = AtomicBool::new(false);
|
||||
@@ -432,6 +433,11 @@ impl PluginsManager {
|
||||
.await
|
||||
}
|
||||
|
||||
#[instrument(
|
||||
level = "trace",
|
||||
skip_all,
|
||||
fields(force_reload, plugins_enabled = config.plugins_enabled)
|
||||
)]
|
||||
pub(crate) async fn plugins_for_config_with_force_reload(
|
||||
&self,
|
||||
config: &PluginsConfigInput,
|
||||
@@ -1188,6 +1194,7 @@ impl PluginsManager {
|
||||
})
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub async fn read_plugin_detail_for_marketplace_plugin(
|
||||
&self,
|
||||
config: &PluginsConfigInput,
|
||||
|
||||
@@ -10,6 +10,7 @@ use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use tracing::info;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::SkillLoadOutcome;
|
||||
@@ -100,6 +101,7 @@ impl SkillsManager {
|
||||
/// This path uses a cache keyed by the effective skill-relevant config state rather than just
|
||||
/// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen
|
||||
/// to share a directory.
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub async fn skills_for_config(
|
||||
&self,
|
||||
input: &SkillsLoadInput,
|
||||
@@ -177,6 +179,7 @@ impl SkillsManager {
|
||||
outcome
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
async fn build_skill_outcome(
|
||||
&self,
|
||||
roots: Vec<SkillRoot>,
|
||||
|
||||
@@ -18,6 +18,7 @@ use codex_tools::DiscoverableTool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use serde::Deserialize;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::config::Config;
|
||||
@@ -460,6 +461,7 @@ fn tool_suggest_connector_ids(
|
||||
connector_ids
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
async fn cached_directory_connectors_for_tool_suggest_with_auth(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
|
||||
@@ -5,7 +5,9 @@ use codex_core_plugins::ToolSuggestPluginDiscoveryInput;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_tools::DiscoverablePluginInfo;
|
||||
use std::collections::HashSet;
|
||||
use tracing::instrument;
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub(crate) async fn list_tool_suggest_discoverable_plugins(
|
||||
config: &Config,
|
||||
plugins_manager: &PluginsManager,
|
||||
|
||||
@@ -1308,6 +1308,14 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
#[instrument(
|
||||
level = "trace",
|
||||
skip_all,
|
||||
fields(
|
||||
thread_id = %self.thread_id(),
|
||||
rollout_item_count = rollout_items.len()
|
||||
)
|
||||
)]
|
||||
async fn apply_rollout_reconstruction(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
|
||||
@@ -20,6 +20,7 @@ use codex_sandboxing::policy_transforms::effective_network_sandbox_policy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
use tracing::instrument;
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub(crate) struct TurnSkillsContext {
|
||||
@@ -724,6 +725,7 @@ impl Session {
|
||||
.await
|
||||
}
|
||||
|
||||
#[instrument(name = "turn_context.build", level = "trace", skip_all)]
|
||||
async fn new_turn_context_from_configuration(
|
||||
&self,
|
||||
sub_id: String,
|
||||
|
||||
@@ -78,6 +78,7 @@ use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::RwLock;
|
||||
use tokio::sync::broadcast;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
|
||||
const THREAD_CREATED_CHANNEL_CAPACITY: usize = 1024;
|
||||
@@ -702,6 +703,7 @@ impl ThreadManager {
|
||||
.await
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub async fn resume_thread_with_history(
|
||||
&self,
|
||||
config: Config,
|
||||
|
||||
Reference in New Issue
Block a user