openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-31 19:28:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

codex: address app-server analytics review feedback (#25193)

Browse Source
This commit is contained in:
Adam Perry
2026-06-01 19:29:36 +00:00
parent 9441ce2b71
commit babf24a559
8 changed files with 167 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/analytics/src/events.rs
+1
View File
@@ -43,6 +43,7 @@ use serde::Serialize;
#[serde(rename_all = "snake_case")]
pub enum AppServerRpcTransport {
Stdio,
UnixSocket,
Websocket,
InProcess,
}
codex-rs/app-server/src/analytics_utils.rs
+6 -1
View File
@@ -7,10 +7,15 @@ use codex_login::AuthManager;
pub(crate) fn analytics_events_client_from_config(
auth_manager: Arc<AuthManager>,
config: &Config,
default_analytics_enabled: bool,
) -> AnalyticsEventsClient {
AnalyticsEventsClient::new(
auth_manager,
config.chatgpt_base_url.trim_end_matches('/').to_string(),
config.analytics_enabled,
Some(
config
.analytics_enabled
.unwrap_or(default_analytics_enabled),
),
)
}
codex-rs/app-server/src/in_process.rs
+17 -4
View File
@@ -382,8 +382,11 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult<InProcessClie
let auth_manager =
AuthManager::shared_from_config(args.config.as_ref(), args.enable_codex_api_key_env)
.await;
let analytics_events_client =
analytics_events_client_from_config(Arc::clone(&auth_manager), args.config.as_ref());
let analytics_events_client = analytics_events_client_from_config(
Arc::clone(&auth_manager),
args.config.as_ref(),
/*default_analytics_enabled*/ true,
);
let outgoing_message_sender = Arc::new(OutgoingMessageSender::new(
outgoing_tx,
analytics_events_client.clone(),
@@ -422,9 +425,8 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult<InProcessClie
args.thread_config_loader,
);
let (processor_tx, mut processor_rx) = mpsc::channel::<ProcessorCommand>(channel_capacity);
analytics_events_client
.track_app_server_started(AppServerRpcTransport::InProcess, startup_started.elapsed());
let mut processor_handle = tokio::spawn(async move {
let app_server_started_analytics_events_client = analytics_events_client.clone();
let processor = Arc::new(MessageProcessor::new(MessageProcessorArgs {
outgoing: Arc::clone(&processor_outgoing),
analytics_events_client,
@@ -446,6 +448,7 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult<InProcessClie
let mut thread_created_rx = processor.thread_created_receiver();
let session = Arc::new(ConnectionSessionState::new());
let mut listen_for_threads = true;
let mut app_server_started_tracked = false;
loop {
tokio::select! {
@@ -483,7 +486,17 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult<InProcessClie
}
}
Some(ProcessorCommand::Notification(notification)) => {
let initialized =
matches!(notification, ClientNotification::Initialized);
processor.process_client_notification(notification).await;
if initialized && !app_server_started_tracked {
app_server_started_analytics_events_client
.track_app_server_started(
AppServerRpcTransport::InProcess,
startup_started.elapsed(),
);
app_server_started_tracked = true;
}
}
None => {
break;
codex-rs/app-server/src/lib.rs
+25 -5
View File
@@ -789,8 +789,11 @@ pub async fn run_main_with_transport_options(
let processor_handle = tokio::spawn({
let auth_manager = Arc::clone(&auth_manager);
let analytics_events_client =
analytics_events_client_from_config(Arc::clone(&auth_manager), &config);
let analytics_events_client = analytics_events_client_from_config(
Arc::clone(&auth_manager),
&config,
default_analytics_enabled,
);
let rpc_transport = analytics_rpc_transport(&transport);
let outgoing_message_sender = Arc::new(OutgoingMessageSender::new(
outgoing_tx,
@@ -1090,15 +1093,19 @@ pub async fn run_main_with_transport_options(
fn analytics_rpc_transport(transport: &AppServerTransport) -> AppServerRpcTransport {
match transport {
AppServerTransport::Stdio => AppServerRpcTransport::Stdio,
AppServerTransport::UnixSocket { .. }
| AppServerTransport::WebSocket { .. }
| AppServerTransport::Off => AppServerRpcTransport::Websocket,
AppServerTransport::UnixSocket { .. } => AppServerRpcTransport::UnixSocket,
AppServerTransport::WebSocket { .. } | AppServerTransport::Off => {
AppServerRpcTransport::Websocket
}
}
}
#[cfg(test)]
mod tests {
use super::LogFormat;
use super::analytics_rpc_transport;
use crate::transport::AppServerTransport;
use codex_analytics::AppServerRpcTransport;
use pretty_assertions::assert_eq;
#[test]
@@ -1118,4 +1125,17 @@ mod tests {
assert_eq!(LogFormat::from_env_value(Some("text")), LogFormat::Default);
assert_eq!(LogFormat::from_env_value(Some("jsonl")), LogFormat::Default);
}
#[cfg(unix)]
#[test]
fn analytics_rpc_transport_preserves_unix_socket() {
let transport = "unix:///tmp/codex-app-server.sock"
.parse::<AppServerTransport>()
.expect("unix socket transport should parse");
assert!(matches!(
analytics_rpc_transport(&transport),
AppServerRpcTransport::UnixSocket
));
}
}
codex-rs/app-server/src/message_processor_tracing_tests.rs
+5 -2
View File
@@ -244,8 +244,11 @@ async fn build_test_processor(
Arg0DispatchPaths::default(),
Arc::new(codex_config::NoopThreadConfigLoader),
);
let analytics_events_client =
analytics_events_client_from_config(Arc::clone(&auth_manager), config.as_ref());
let analytics_events_client = analytics_events_client_from_config(
Arc::clone(&auth_manager),
config.as_ref(),
/*default_analytics_enabled*/ true,
);
let outgoing = Arc::new(OutgoingMessageSender::new(
outgoing_tx,
analytics_events_client.clone(),
codex-rs/app-server/tests/suite/v2/analytics.rs
+101 -13
View File
@@ -112,6 +112,24 @@ async fn standalone_app_server_startup_tracks_analytics_event() -> Result<()> {
Ok(())
}
#[tokio::test]
async fn standalone_app_server_startup_respects_default_disabled_analytics() -> Result<()> {
let server = MockServer::start().await;
let codex_home = TempDir::new()?;
write_mock_responses_config_toml_with_chatgpt_base_url(
codex_home.path(),
&server.uri(),
&server.uri(),
)?;
mount_analytics_endpoint(&server).await;
write_analytics_auth(codex_home.path())?;
let _mcp = McpProcess::new_without_managed_config(codex_home.path()).await?;
assert_analytics_event_not_received(&server, Duration::from_secs(1), "codex_app_server_started")
.await
}
#[tokio::test]
async fn embedded_app_server_startup_tracks_analytics_event() -> Result<()> {
let server = MockServer::start().await;
@@ -123,14 +141,53 @@ async fn embedded_app_server_startup_tracks_analytics_event() -> Result<()> {
)?;
mount_analytics_capture(&server, codex_home.path()).await?;
let client =
in_process::start(in_process_start_args(codex_home.path(), "codex-tui").await?).await?;
let event =
wait_for_analytics_event(&server, Duration::from_secs(10), "codex_app_server_started")
.await?;
assert_app_server_started_event(&event, "in_process");
client.shutdown().await?;
Ok(())
}
#[tokio::test]
async fn embedded_app_server_failed_initialize_does_not_track_startup_event() -> Result<()> {
let server = MockServer::start().await;
let codex_home = TempDir::new()?;
write_mock_responses_config_toml_with_chatgpt_base_url(
codex_home.path(),
&server.uri(),
&server.uri(),
)?;
mount_analytics_capture(&server, codex_home.path()).await?;
let result =
in_process::start(in_process_start_args(codex_home.path(), "bad\rname").await?).await;
let Err(error) = result else {
anyhow::bail!("in-process start should reject an invalid client name");
};
assert!(
error
.to_string()
.contains("in-process initialize failed: Invalid clientInfo.name")
);
assert_analytics_event_not_received(&server, Duration::from_secs(1), "codex_app_server_started")
.await
}
async fn in_process_start_args(codex_home: &Path, client_name: &str) -> Result<InProcessStartArgs> {
let loader_overrides = LoaderOverrides::without_managed_config_for_tests();
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.codex_home(codex_home.to_path_buf())
.fallback_cwd(Some(codex_home.to_path_buf()))
.loader_overrides(loader_overrides.clone())
.build()
.await?;
let client = in_process::start(InProcessStartArgs {
Ok(InProcessStartArgs {
arg0_paths: Arg0DispatchPaths::default(),
config: Arc::new(config),
cli_overrides: Vec::new(),
@@ -147,7 +204,7 @@ async fn embedded_app_server_startup_tracks_analytics_event() -> Result<()> {
enable_codex_api_key_env: false,
initialize: InitializeParams {
client_info: ClientInfo {
name: "codex-tui".to_string(),
name: client_name.to_string(),
title: None,
version: "0.1.0".to_string(),
},
@@ -155,15 +212,6 @@ async fn embedded_app_server_startup_tracks_analytics_event() -> Result<()> {
},
channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY,
})
.await?;
let event =
wait_for_analytics_event(&server, Duration::from_secs(10), "codex_app_server_started")
.await?;
assert_app_server_started_event(&event, "in_process");
client.shutdown().await?;
Ok(())
}
fn assert_app_server_started_event(event: &Value, rpc_transport: &str) {
@@ -178,12 +226,27 @@ fn assert_app_server_started_event(event: &Value, rpc_transport: &str) {
}
pub(crate) async fn mount_analytics_capture(server: &MockServer, codex_home: &Path) -> Result<()> {
mount_analytics_endpoint(server).await;
let config_path = codex_home.join("config.toml");
let config = std::fs::read_to_string(&config_path)?;
std::fs::write(
config_path,
format!("{config}\n[analytics]\nenabled = true\n"),
)?;
write_analytics_auth(codex_home)
}
async fn mount_analytics_endpoint(server: &MockServer) {
Mock::given(method("POST"))
.and(path("/codex/analytics-events/events"))
.respond_with(ResponseTemplate::new(200))
.mount(server)
.await;
}
fn write_analytics_auth(codex_home: &Path) -> Result<()> {
write_chatgpt_auth(
codex_home,
ChatGptAuthFixture::new("chatgpt-token")
@@ -196,6 +259,31 @@ pub(crate) async fn mount_analytics_capture(server: &MockServer, codex_home: &Pa
Ok(())
}
async fn assert_analytics_event_not_received(
server: &MockServer,
wait_duration: Duration,
event_type: &str,
) -> Result<()> {
tokio::time::sleep(wait_duration).await;
let Some(requests) = server.received_requests().await else {
return Ok(());
};
for request in &requests {
if request.method != "POST" || request.url.path() != "/codex/analytics-events/events" {
continue;
}
let payload: Value = serde_json::from_slice(&request.body)
.map_err(|err| anyhow::anyhow!("invalid analytics payload: {err}"))?;
if payload["events"]
.as_array()
.is_some_and(|events| events.iter().any(|event| event["event_type"] == event_type))
{
anyhow::bail!("received unexpected {event_type} analytics event");
}
}
Ok(())
}
pub(crate) async fn wait_for_thread_initialized_payload(
server: &MockServer,
read_timeout: Duration,
codex-rs/app-server/tests/suite/v2/plugin_install.rs
+11 -1
View File
@@ -763,6 +763,7 @@ async fn plugin_install_tracks_remote_plugin_analytics_event() -> Result<()> {
)
.await;
configure_remote_plugin_test(codex_home.path(), &server)?;
enable_analytics(codex_home.path())?;
mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await;
mount_empty_remote_installed_plugins(&server).await;
mount_remote_plugin_install(&server, REMOTE_PLUGIN_ID).await;
@@ -1333,7 +1334,16 @@ plugins = true
fn write_analytics_config(codex_home: &std::path::Path, base_url: &str) -> std::io::Result<()> {
std::fs::write(
codex_home.join("config.toml"),
format!("chatgpt_base_url = \"{base_url}\"\n"),
format!("chatgpt_base_url = \"{base_url}\"\n\n[analytics]\nenabled = true\n"),
)
}
fn enable_analytics(codex_home: &std::path::Path) -> std::io::Result<()> {
let config_path = codex_home.join("config.toml");
let config = std::fs::read_to_string(&config_path)?;
std::fs::write(
config_path,
format!("{config}\n[analytics]\nenabled = true\n"),
)
}
codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs
+1 -1
View File
@@ -87,7 +87,7 @@ async fn plugin_uninstall_tracks_analytics_event() -> Result<()> {
std::fs::write(
codex_home.path().join("config.toml"),
format!(
"chatgpt_base_url = \"{}\"\n\n[features]\nplugins = true\n\n[plugins.\"sample-plugin@debug\"]\nenabled = true\n",
"chatgpt_base_url = \"{}\"\n\n[features]\nplugins = true\n\n[analytics]\nenabled = true\n\n[plugins.\"sample-plugin@debug\"]\nenabled = true\n",
analytics_server.uri()
),
)?;