mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Attribute app-server analytics by thread originator (#29935)
## Why Desktop Work threads and regular Codex threads can share the same app-server connection. App-server analytics currently copy `product_client_id` from connection metadata for every thread-scoped event, so Work thread activity is attributed to the Desktop connection instead of the thread's resolved originator. This prevents analytics from distinguishing the two products on a shared connection. ## What changed - Publish the resolved originator after a thread is materialized, covering new, resumed, forked, and subagent threads. - Store that originator in the analytics reducer's existing per-thread state. - Override only `app_server_client.product_client_id` for thread, turn, tool, review, goal, guardian, and compaction events while preserving the connection's client name, version, and transport metadata. - Fall back to the connection-wide product client ID when a thread has no originator override. - Preserve persisted originators in thread initialization analytics for resume and fork flows. ## Validation - `just test -p codex-analytics thread_originator_overrides_shared_connection_across_thread_events subagent_events_keep_thread_originator_with_explicit_turn_connection` - `just test -p codex-app-server turn_start_tracks_thread_originator_in_analytics thread_start_tracks_thread_initialized_analytics thread_fork_tracks_thread_initialized_analytics thread_resume_tracks_thread_initialized_analytics` - `just test -p codex-core thread_manager`
This commit is contained in:
@@ -504,13 +504,36 @@ impl OutgoingMessageSender {
|
||||
where
|
||||
T: Into<ClientResponsePayload>,
|
||||
{
|
||||
self.send_response_as(request_id, response.into()).await;
|
||||
self.send_response_as_inner(request_id, response.into(), /*thread_originator*/ None)
|
||||
.await;
|
||||
}
|
||||
|
||||
pub(crate) async fn send_response_with_thread_originator<T>(
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
response: T,
|
||||
thread_originator: String,
|
||||
) where
|
||||
T: Into<ClientResponsePayload>,
|
||||
{
|
||||
self.send_response_as_inner(request_id, response.into(), Some(thread_originator))
|
||||
.await;
|
||||
}
|
||||
|
||||
pub(crate) async fn send_response_as(
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
response: ClientResponsePayload,
|
||||
) {
|
||||
self.send_response_as_inner(request_id, response, /*thread_originator*/ None)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn send_response_as_inner(
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
response: ClientResponsePayload,
|
||||
thread_originator: Option<String>,
|
||||
) {
|
||||
let connection_id = request_id.connection_id;
|
||||
let request_id_for_analytics = request_id.request_id.clone();
|
||||
@@ -518,11 +541,24 @@ impl OutgoingMessageSender {
|
||||
.into_jsonrpc_parts_and_payload(request_id.request_id.clone())
|
||||
.map(|(id, result, response)| {
|
||||
if let Some(response) = response {
|
||||
self.analytics_events_client.track_response(
|
||||
connection_id.0,
|
||||
request_id_for_analytics,
|
||||
response,
|
||||
);
|
||||
match thread_originator {
|
||||
Some(thread_originator) => {
|
||||
self.analytics_events_client
|
||||
.track_response_with_thread_originator(
|
||||
connection_id.0,
|
||||
request_id_for_analytics,
|
||||
response,
|
||||
thread_originator,
|
||||
);
|
||||
}
|
||||
None => {
|
||||
self.analytics_events_client.track_response(
|
||||
connection_id.0,
|
||||
request_id_for_analytics,
|
||||
response,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
(id, result)
|
||||
});
|
||||
|
||||
@@ -640,6 +640,7 @@ pub(super) async fn handle_pending_thread_resume_request(
|
||||
active_permission_profile,
|
||||
workspace_roots,
|
||||
reasoning_effort,
|
||||
originator,
|
||||
..
|
||||
} = config_snapshot;
|
||||
let instruction_sources = pending.instruction_sources;
|
||||
@@ -665,7 +666,9 @@ pub(super) async fn handle_pending_thread_resume_request(
|
||||
multi_agent_mode: MultiAgentMode::ExplicitRequestOnly,
|
||||
initial_turns_page,
|
||||
};
|
||||
outgoing.send_response(request_id, response).await;
|
||||
outgoing
|
||||
.send_response_with_thread_originator(request_id, response, originator)
|
||||
.await;
|
||||
// Match cold resume: metadata-only resume should attach the listener without
|
||||
// paying the cost of turn reconstruction for historical usage replay.
|
||||
if let Some(token_usage_thread) = token_usage_thread {
|
||||
|
||||
@@ -1268,6 +1268,7 @@ impl ThreadRequestProcessor {
|
||||
let cwd = config_snapshot.cwd().clone();
|
||||
let active_permission_profile =
|
||||
thread_response_active_permission_profile(config_snapshot.active_permission_profile);
|
||||
let thread_originator = config_snapshot.originator.clone();
|
||||
|
||||
let response = ThreadStartResponse {
|
||||
thread: thread.clone(),
|
||||
@@ -1287,7 +1288,7 @@ impl ThreadRequestProcessor {
|
||||
let notif = thread_started_notification(thread);
|
||||
listener_task_context
|
||||
.outgoing
|
||||
.send_response(request_id, response)
|
||||
.send_response_with_thread_originator(request_id, response, thread_originator)
|
||||
.instrument(tracing::info_span!(
|
||||
"app_server.thread_start.send_response",
|
||||
otel.name = "app_server.thread_start.send_response",
|
||||
@@ -2863,6 +2864,7 @@ impl ThreadRequestProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
let thread_originator = config_snapshot.originator.clone();
|
||||
let response = ThreadResumeResponse {
|
||||
thread,
|
||||
model: session_configured.model,
|
||||
@@ -2881,7 +2883,9 @@ impl ThreadRequestProcessor {
|
||||
};
|
||||
|
||||
let connection_id = request_id.connection_id;
|
||||
self.outgoing.send_response(request_id, response).await;
|
||||
self.outgoing
|
||||
.send_response_with_thread_originator(request_id, response, thread_originator)
|
||||
.await;
|
||||
// `excludeTurns` is explicitly the cheap resume path, so avoid
|
||||
// rebuilding history only to attribute a replayed usage update.
|
||||
if let Some(token_usage_thread) = token_usage_thread {
|
||||
@@ -3582,6 +3586,7 @@ impl ThreadRequestProcessor {
|
||||
);
|
||||
let active_permission_profile =
|
||||
thread_response_active_permission_profile(config_snapshot.active_permission_profile);
|
||||
let thread_originator = config_snapshot.originator.clone();
|
||||
|
||||
let response = ThreadForkResponse {
|
||||
thread: thread.clone(),
|
||||
@@ -3602,7 +3607,9 @@ impl ThreadRequestProcessor {
|
||||
let notif = thread_started_notification(thread);
|
||||
let connection_id = request_id.connection_id;
|
||||
let token_usage_thread = include_turns.then(|| response.thread.clone());
|
||||
self.outgoing.send_response(request_id, response).await;
|
||||
self.outgoing
|
||||
.send_response_with_thread_originator(request_id, response, thread_originator)
|
||||
.await;
|
||||
// `excludeTurns` is the cheap fork path, so skip restored usage replay
|
||||
// instead of rebuilding history only to attribute a historical update.
|
||||
if let Some(token_usage_thread) = token_usage_thread {
|
||||
|
||||
Reference in New Issue
Block a user