diff --git a/app_server_tracing_design.md b/app_server_tracing_design.md index 11e6ae7449..7acdea9a9b 100644 --- a/app_server_tracing_design.md +++ b/app_server_tracing_design.md @@ -1,126 +1,127 @@ # App-server v2 tracing design -This document proposes a tracing design for `codex-rs/app-server` with these -goals: +This document proposes a simple, staged tracing design for +`codex-rs/app-server` with these goals: -- support true distributed tracing across client and server +- support distributed tracing from client-initiated app-server work into + app-server and `codex-core` - keep tracing consistent across the app-server v2 surface area - minimize tracing boilerplate in request handlers -- minimize OTEL-specific code in business logic -- support both unary and long-lived streaming APIs cleanly +- avoid introducing tracing-owned lifecycle state that duplicates existing + app-server runtime state -This design explicitly does **not** use a `RequestKind` classification as the -primary driver of tracing behavior. Instead, tracing behavior follows actual -runtime lifecycle events. +This design explicitly avoids a `RequestKind` taxonomy and avoids +app-server-owned long-lived lifecycle span registries. ## Summary The design has four major pieces: -1. A transport-level trace carrier on JSON-RPC envelopes. -2. A centralized app-server tracing layer that wraps inbound and outbound - messages. -3. A lifecycle tracing layer for long-lived app-server operations such as - active turns, plus lightweight thread and realtime session state with - lifecycle events/metrics. -4. An internal trace-context handoff through `codex_protocol::Submission` so - background work in `codex-core` runs under the correct parent span. +1. A transport-level W3C trace carrier on inbound JSON-RPC request envelopes. +2. A centralized app-server request tracing layer that wraps every inbound + request in the same request span. +3. An internal trace-context handoff through `codex_protocol::Submission` so + work that continues in `codex-core` inherits the inbound app-server request + ancestry. +4. A core-owned long-lived turn span for turn-producing operations such as + `turn/start` and `review/start`. Every inbound JSON-RPC request gets a standardized request span. -Long-lived operations create additional spans only when the actual business -logic starts those operations. For example: +When an app-server request submits work into core, the current span context is +captured into `Submission.trace`. Core then creates a short-lived dispatch span +parented from that carrier and, for turn-producing operations, creates a +long-lived turn span beneath it before continuing into its existing task and +model request tracing. -- `thread/start` creates a request span and emits thread lifecycle - events/metrics. -- `turn/start` creates a request span and then a long-lived turn span. -- `thread/list` creates only a request span because it does not create any - long-lived lifecycle. +Important: -Handlers do not construct OTEL spans directly. They make small calls into a -central tracing module to announce lifecycle transitions. +- request spans stay short-lived +- long-lived turn spans are owned by core, not app-server +- the design does not add app-server-owned long-lived thread or realtime spans ## Design goals - **Distributed tracing first** - Clients should be able to send trace context to app-server. - - App-server should propagate trace context on all outbound JSON-RPC - messages. - - Core model requests should continue propagating the active span context to - upstream HTTP/WebSocket requests. + - App-server should preserve that trace ancestry across the async handoff into + core. + - Existing core model request tracing should continue to inherit from the + active core span once the handoff occurs. -- **Consistent instrumentation** - - Every request should produce a standardized request span with the same base - attributes. - - Every response, notification, and server-initiated request should inject - trace context the same way. +- **Consistent request instrumentation** + - Every inbound request should produce the same request span with the same + base attributes. + - Request tracing should be wired at the transport boundary, not repeated in + individual handlers. - **Minimal boilerplate** - - Request handlers should not repeat span construction or attribute assembly. - - Most endpoints should need no tracing-specific code beyond a lifecycle hook - when they create or finish a long-lived operation. + - Request handlers should not manually parse carriers or build request spans. + - Existing calls to `thread.submit(...)` and similar APIs should pick up trace + propagation automatically. - **Minimal business logic pollution** - - OTEL-specific conversion, carrier parsing, and propagation should live in - dedicated tracing modules. - - Business code should report lifecycle facts such as "thread started" or - "turn finished", not manipulate OTEL contexts directly. + - W3C parsing, OTEL conversion, and span-parenting rules should live in + tracing-specific modules. + - App-server business logic should stay focused on request handling, not span + management. -- **Good support for streaming APIs** - - Request spans should not stay open for minutes or hours. - - Streaming APIs should create separate long-lived lifecycle spans. +- **Incremental rollout** + - The first rollout should prove inbound request tracing and app-server -> + core propagation. + - Once propagation is in place, core should add a long-lived turn span so a + single span covers the actual duration of a turn. + - Thread and realtime lifecycle tracing should wait until there is a concrete + need. ## Non-goals -- This design does not try to encode request semantics in a type-level proof. -- This design does not attempt to make the durable rollout lifetime equal to a - tracing span lifetime across process restarts. +- This design does not attempt to make every loaded thread or realtime session + correspond to a long-lived tracing span. +- This design does not add tracing-owned thread or realtime state stores in the + initial design. - This design does not require every app-server v2 `*Params` type to carry trace metadata. +- This design does not require outbound JSON-RPC trace propagation in the + initial rollout. ## Why not `RequestKind` An earlier direction considered a central `RequestKind` taxonomy such as `Unary`, `TurnLifecycle`, or `RealtimeLifecycle`. -That is workable, but it makes tracing depend on a label that can drift from -real runtime behavior. The no-`RequestKind` design instead treats tracing as -behavior-driven: +That is workable, but it makes tracing depend on a classification that can +drift from runtime behavior. The simpler design instead treats tracing as two +generic mechanics: -- every request gets the same generic request span -- long-lived spans are created only when the implementation actually starts a - lifecycle -- long-lived spans are finished only when the implementation actually observes - lifecycle completion +- every inbound request gets the same request span +- any async work that crosses from app-server into core gets the current span + context attached to `Submission` -This reduces the risk of "the spec says unary but the method now streams" and -avoids turning tracing into a taxonomy maintenance problem. - -We can still keep an inventory of traced APIs in docs or tests, but that -inventory should not be the primary source of runtime behavior. +This keeps the initial implementation small and avoids turning tracing into a +taxonomy maintenance problem. ## Terminology - **Request span** - - A short-lived span for one inbound JSON-RPC request. + - A short-lived span for one inbound JSON-RPC request to app-server. -- **Thread residency tracker** - - Lightweight app-server state keyed by `thread_id` that records facts such - as load time so we can emit thread lifecycle events and metrics without - keeping an unbounded thread-level span open for days. +- **W3C trace context** + - A serializable representation of distributed trace context based on + `traceparent` and `tracestate`. + +- **Submission trace handoff** + - The optional serialized trace context attached to + `codex_protocol::Submission` so core can restore parentage after the + app-server request handler returns. + +- **Dispatch span** + - A short-lived core span created when the submission loop receives a + `Submission` with trace context. - **Turn span** - - A long-lived span representing an active turn that starts from - `turn/start` or an equivalent API and ends at `turn/completed` or abort. - -- **Realtime session tracker** - - Lightweight app-server state keyed by session identity that records facts - such as start time so we can emit realtime lifecycle events and metrics - without keeping an unbounded realtime-session span open. - -- **Trace carrier** - - A serializable representation of distributed trace context, based on - `traceparent` and `tracestate`. + - A long-lived core-owned span representing the actual runtime of a turn from + turn start until completion, interruption, or failure. ## High-level tracing model @@ -128,66 +129,65 @@ inventory should not be the primary source of runtime behavior. For every inbound JSON-RPC request: -1. parse an optional trace carrier from the envelope -2. create a request span with standardized attributes -3. parent that request span from the incoming carrier when present -4. process the request inside that request span +1. parse an optional W3C trace carrier from the JSON-RPC envelope +2. create a standardized request span +3. parent that span from the incoming carrier when present +4. process the request inside that span -This is true for every request, regardless of whether it is unary or -streaming. +This is true for every request, regardless of whether the API is unary or +starts work that continues later. -### 2. Outbound messages +### 2. Async handoff into core -For every outbound JSON-RPC message: - -- response -- notification -- server-initiated request -- error - -inject a trace carrier into the envelope from `Span::current()`. - -This keeps propagation centralized and consistent. - -### 3. Long-lived lifecycle spans - -Create long-lived spans only when runtime behavior warrants them. Do not create -unbounded thread-level or realtime-session spans just because something remains -open in memory. - -Examples: - -- `thread/start`, `thread/resume`, `thread/fork` - - record thread residency and emit thread lifecycle events/metrics -- `turn/start`, `review/start`, `thread/compact/start` if it runs as a turn - - create turn spans -- `thread/realtime/start` - - record realtime session state and emit realtime lifecycle events/metrics - -Turn spans are stored by stable IDs and can be re-entered later when sending -notifications or handling completion. Thread and realtime lifecycle use -lightweight session-state trackers rather than long-lived spans. - -### 4. Internal async handoff - -App-server often starts work that continues on background tasks after the -original request handler returns. The critical example is `turn/start`, which -submits `Op::UserInput` into the core session loop and then returns -immediately. +Some app-server requests submit work that continues in core after the original +request returns. The critical example is `turn/start`, but the mechanism should +be generic. To preserve trace ancestry: -- add an optional trace carrier to `codex_protocol::Submission` -- populate it when app-server submits work to core +- add an optional `W3cTraceContext` to `codex_protocol::Submission` +- have `CodexThread::submit()` capture the current span context into that field + automatically - have `codex-core` create a per-submission dispatch span parented from that carrier -This lets existing core spans naturally nest beneath the right app-server turn -or request span without scattering tracing logic throughout core tasks. +This gives a clean causal chain: + +- client span +- app-server request span +- core dispatch span +- core turn span for turn-producing operations +- existing core spans such as `run_turn`, sampling, and model request spans + +### 3. Core-owned turn spans + +For turn-producing operations such as `turn/start` and `review/start`: + +- app-server creates the inbound request span +- app-server propagates that request context through `Submission.trace` +- core creates a dispatch span when it receives the submission +- core then creates a long-lived turn span beneath that dispatch span +- existing core work such as `run_turn` and model request tracing runs beneath + the turn span + +This keeps long-lived span ownership with the layer that actually owns turn +execution and completion. + +### 4. Defer thread and realtime lifecycle-heavy tracing + +The design should not add: + +- app-server-owned thread residency stores +- app-server-owned realtime session stores + +App-server already maintains thread subscription and runtime state in existing +structures. If later tracing work needs thread loaded-duration or realtime +duration metrics, that data should extend those existing structures rather than +introducing a parallel tracing-only state machine. ## Span model by API shape -The runtime behavior determines which spans exist. +The initial implementation keeps the app-server side uniform. ### Unary request/response APIs @@ -204,60 +204,29 @@ Behavior: - create request span - return response -- no additional long-lived spans +- no additional app-server span state -### Thread lifecycle APIs - -Examples: - -- `thread/start` -- `thread/resume` -- `thread/fork` -- `thread/unsubscribe` when it unloads a thread - -Behavior: - -- create request span -- annotate the request span with `thread.id` when known -- emit thread lifecycle events and metrics on load/unload transitions -- update lightweight thread residency state if needed for unload-duration - metrics - -Thread residency is useful as metadata and metrics, but not as a long-lived -tracing span. A thread may remain loaded for minutes, hours, or days, and that -is not a single causal operation. - -### Turn lifecycle APIs +### Turn-producing APIs Examples: - `turn/start` - `review/start` -- `thread/compact/start` when it runs as a normal turn lifecycle +- `thread/compact/start` when it executes as a normal turn lifecycle Behavior: - create request span -- on success, create turn span -- keep the turn span alive until turn completion or abort +- submit work under that request span +- capture the current span context into `Submission.trace` +- let core create a dispatch span and then a long-lived turn span +- let the turn span remain open until the real core turn lifecycle ends -Important: request spans should not remain open until `turn/completed`. -Long-running work belongs on the turn span, not the request span. +Important: request spans should not stay open until eventual streamed +completion. The request span ends quickly; the core-owned turn span carries the +long-running work. -### Turn control APIs - -Examples: - -- `turn/steer` -- `turn/interrupt` - -Behavior: - -- create request span -- link or add events to the active turn span if present -- do not create a new turn span - -### Realtime lifecycle APIs +### Other APIs that submit work into core Examples: @@ -268,119 +237,142 @@ Examples: Behavior: -- `start` creates a request span and records realtime session state -- append/stop requests create request spans and use `thread.id` / `session.id` - attributes for correlation -- completion emits realtime lifecycle events/metrics and clears session state +- create request span +- submit work under that request span +- capture the current span context into `Submission.trace` +- let core continue tracing from there + +These APIs do not automatically imply a long-lived app-server or core lifecycle +span in the initial design. + +### Thread lifecycle APIs + +Examples: + +- `thread/start` +- `thread/resume` +- `thread/fork` +- `thread/unsubscribe` + +Behavior in the initial design: + +- create request span +- annotate with `thread.id` when known +- do not introduce separate app-server lifecycle spans or tracing-only state + +If later work needs thread loaded/unloaded metrics, it should reuse the existing +thread runtime state already maintained by app-server. ## Where the code should live -### New crate: trace carrier +### `codex-rs/protocol` -Add a small shared crate, tentatively `codex-trace-context`. +Add a small shared `W3cTraceContext` type to +[`codex-rs/protocol/src/protocol.rs`](/Users/owen/repos/codex3/codex-rs/protocol/src/protocol.rs). Responsibilities: -- define a serializable trace carrier type -- avoid direct dependence on OTEL types -- be usable from protocol crates and runtime crates +- define a serializable W3C trace context type +- avoid direct dependence on OTEL runtime types +- be usable from both protocol crates and runtime crates Suggested contents: -- `TraceCarrier` +- `W3cTraceContext` - `traceparent: Option` - `tracestate: Option` -- helper methods for validation that do not depend on OTEL runtime types -Reason: +Suggested `Submission` change: -- `codex-app-server-protocol` needs a serializable envelope type -- `codex-protocol` needs the same type on `Submission` -- `codex-otel` should own OTEL conversion logic, not the plain data type +- `Submission { id, op, trace: Option }` + +This is the only new internal async handoff needed for the initial rollout. ### `codex-rs/otel` -Add a small helper module, tentatively `trace_context.rs`. +Add a small helper module or extend existing tracing helpers so OTEL-specific +logic stays centralized. Responsibilities: -- convert `TraceCarrier` -> OTEL `Context` -- convert `Span::current()` -> `TraceCarrier` -- parent a tracing span from an explicit carrier -- centralize precedence rules: - - explicit carrier from transport +- convert `W3cTraceContext` -> OTEL `Context` +- convert the current tracing span context -> `W3cTraceContext` +- parent a tracing span from an explicit carrier when present +- apply precedence rules: + - explicit carrier from app-server transport or `Submission.trace` - fallback to env `TRACEPARENT` / `TRACESTATE` - otherwise root span -This keeps OTEL-specific conversion in one place. +Important: + +- keep this focused on carrier parsing and span parenting +- do not move app-server runtime state into `codex-otel` +- do not overload `OtelManager` with app-server lifecycle ownership in the + initial design ### `codex-rs/app-server-protocol` -Extend JSON-RPC envelopes in +Extend inbound JSON-RPC request envelopes in [`codex-rs/app-server-protocol/src/jsonrpc_lite.rs`](/Users/owen/repos/codex3/codex-rs/app-server-protocol/src/jsonrpc_lite.rs) -with an optional `meta` field. +with a dedicated optional trace carrier field. Suggested shape: -- `JSONRPCRequest { id, method, params, meta }` -- `JSONRPCNotification { method, params, meta }` -- `JSONRPCResponse { id, result, meta }` -- `JSONRPCError { error, id, meta }` +- `JSONRPCRequest { id, method, params, trace }` Where: -- `meta.trace: Option` +- `trace: Option` Important: -- trace metadata belongs on the JSON-RPC envelope, not inside business payloads -- this keeps tracing transport-level and method-agnostic - -### `codex-rs/protocol` - -Extend `Submission` in -[`codex-rs/protocol/src/protocol.rs`](/Users/owen/repos/codex3/codex-rs/protocol/src/protocol.rs) -with an optional trace carrier. - -Suggested shape: - -- `Submission { id, op, trace: Option }` - -This is the async trace handoff between app-server and core. +- use a dedicated tracing field, not a generic `meta` bag +- keep tracing transport-level and method-agnostic +- do not add trace fields to individual `*Params` business payloads ### `codex-rs/core` -Make a small change in the submission dispatch path in +Make small changes in the submission path in [`codex-rs/core/src/codex.rs`](/Users/owen/repos/codex3/codex-rs/core/src/codex.rs). Responsibilities: - read `Submission.trace` -- create a per-submission dispatch span -- parent that span from the carrier -- run existing op handling under that span +- create a per-submission dispatch span parented from that carrier +- run existing submission handling under that span -This is enough for existing core tracing to inherit the correct ancestry. -Core business logic should not need broad tracing changes. +This is enough for existing core tracing to inherit the correct ancestry, and +it is the right place to add the long-lived turn span required for turn +lifecycles. + +For turn-producing operations, core responsibilities should include: + +- read `Submission.trace` +- create a per-submission dispatch span parented from that carrier +- create a long-lived turn span beneath the dispatch span when the operation + actually starts a turn +- finish that turn span when the real core turn lifecycle completes, + interrupts, or fails ### `codex-rs/app-server` -Add a dedicated tracing module rather than spreading logic across existing -handlers. A likely shape is: +Add a small dedicated tracing module rather than spreading request tracing logic +across handlers. A likely shape is: - `app_server_tracing/mod.rs` - `app_server_tracing/request_spans.rs` -- `app_server_tracing/registry.rs` - `app_server_tracing/incoming.rs` -- `app_server_tracing/outgoing.rs` Responsibilities: -- extract incoming trace carriers +- extract incoming W3C trace carriers from JSON-RPC requests - build standardized request spans -- maintain turn span registries and lightweight thread/realtime session state -- inject outgoing trace carriers -- expose small lifecycle APIs for handlers +- provide a small API that wraps request handling in the correct span + +Non-responsibilities in the initial design: + +- no thread residency registry +- no realtime session registry ## Standardized request spans @@ -413,354 +405,287 @@ Important: - the span factory should be the only place that assembles these fields - handlers should not manually construct request-span attributes +- for the `initialize` request itself, read `clientInfo.name` and + `clientInfo.version` directly from the request params when present +- for later requests on the same connection, read client metadata from + per-connection session state populated during `initialize` -## Lifecycle registries and lightweight session state +## No app-server tracing registries -The tracing layer should own registries for true long-lived spans and a small -amount of lightweight thread/realtime session state. +The design should not introduce app-server-owned tracing registries for turns, +threads, or realtime sessions. -Suggested structures: +Why: -- `TurnSpanRegistry` - - keyed by `turn_id` -- `ThreadResidencyTracker` - - keyed by `ThreadId` - - stores load timestamp and any minimal metadata needed to emit unload - duration metrics or consistency warnings -- `RealtimeSessionTracker` - - keyed by `(thread_id, session_id)` or the best available stable runtime key - - stores session start timestamp and any minimal metadata needed to emit - close duration metrics or consistency warnings +- app-server already has thread subscription and runtime state +- core already owns the real task and turn lifecycle +- a second tracing-specific state machine adds more code and more ways for + lifecycle tracking to drift -Responsibilities: +Future guidance: -- create true long-lived spans where warranted -- store turn spans by ID -- re-enter turn spans later when notifications are emitted -- end or drop turn spans on lifecycle completion -- record thread and realtime session state without keeping unbounded spans open - -Suggested span names: - -- `app_server.turn` - -Suggested span attributes: - -- `thread.id` -- `turn.id` -- `session.id` -- `app_server.connection_id` when relevant -- `rpc.method` of the starting request - -Suggested thread lifecycle events/metrics: - -- `app_server.thread.loaded` -- `app_server.thread.unloaded` -- `app_server.thread.loaded_duration_ms` - -Suggested realtime lifecycle events/metrics: - -- `app_server.realtime.started` -- `app_server.realtime.closed` -- `app_server.realtime.duration_ms` +- if thread loaded/unloaded metrics become important, extend existing app-server + thread state +- keep long-lived turn spans in core +- if realtime lifecycle metrics become important, extend the existing realtime + runtime path rather than creating a parallel tracing store ## No direct span construction in handlers Request handlers should not call `info_span!`, `trace_span!`, `set_parent`, or -OTEL APIs directly for app-server lifecycle tracing. +OTEL APIs directly for app-server request tracing. -Instead, handlers should call small tracing APIs such as: +Instead: -- `tracing_state.on_thread_loaded(...)` -- `tracing_state.on_thread_unloaded(...)` -- `tracing_state.on_turn_started(...)` -- `tracing_state.on_turn_finished(...)` -- `tracing_state.on_realtime_session_started(...)` -- `tracing_state.on_realtime_session_closed(...)` +- `message_processor` should wrap inbound request handling through the + centralized request-span helper +- `CodexThread::submit()` should capture the current span context into + `Submission.trace` -Those calls express business facts without embedding OTEL mechanics in handler -logic. +That keeps request tracing transport-level and largely invisible to business +handlers. + +## Layering + +The intended call graph is: + +- `message_processor` -> `app_server_tracing` + - create and enter the standardized inbound request span +- `CodexThread::submit()` -> `codex-otel` trace-context helper + - snapshot the current span context into `Submission.trace` +- `codex-core` submission loop -> `codex-otel` trace-context helper + - create a dispatch span parented from `Submission.trace` + - create a long-lived turn span for turn-producing operations + +Important: + +- app-server owns inbound request tracing +- core owns execution after the async handoff +- core owns long-lived turn spans +- the design does not add app-server-owned long-lived thread or realtime spans ## Inbound flow in app-server The inbound request path should work like this: -1. Parse the JSON-RPC request envelope, including `meta.trace`. +1. Parse the JSON-RPC request envelope, including `trace`. 2. Use the tracing module to create a request span. 3. Process the request inside that span. -4. If the request starts a long-lived lifecycle, call the appropriate - lifecycle hook. -5. When submitting work to `codex-core`, attach a trace carrier to - `Submission`. +4. If the request submits work into core, let `CodexThread::submit()` capture + the active span context into `Submission.trace`. Integration point: - [`codex-rs/app-server/src/message_processor.rs`](/Users/owen/repos/codex3/codex-rs/app-server/src/message_processor.rs) -## Outbound flow in app-server - -The outbound message path should work like this: - -1. When constructing a response, notification, server request, or error, - capture `Span::current()`. -2. Convert the current span context into a `TraceCarrier`. -3. Attach it to `meta.trace` on the outgoing envelope. -4. Send the envelope through the existing outbound routing. - -Integration point: - -- [`codex-rs/app-server/src/outgoing_message.rs`](/Users/owen/repos/codex3/codex-rs/app-server/src/outgoing_message.rs) - -Important: - -- injection should happen when the outgoing envelope is created, not later in - the transport writer task -- this preserves the correct context from the current handler or event scope - ## Core handoff flow The `turn/start` and similar flows cross an async boundary: -- request handler submits `Op::UserInput` -- core session loop receives `Submission` +- app-server handler submits work +- core submission loop receives `Submission` - actual work continues later on different tasks To preserve parentage: -1. app-server creates a turn span -2. app-server attaches that span context to `Submission.trace` -3. core submission loop creates a dispatch span parented from - `Submission.trace` -4. existing core spans naturally nest under it +1. app-server request handling runs inside `app_server.request` +2. `CodexThread::submit()` captures that active context into `Submission.trace` +3. core submission loop creates a dispatch span parented from `Submission.trace` +4. if the submission starts a turn, core creates a long-lived turn span beneath + that dispatch span +5. existing core spans naturally nest under the turn span This lets: +- submission handling +- a single long-lived turn span for turn-producing APIs - `run_turn` -- sampling spans - model client request tracing -inherit the app-server turn trace without broad tracing changes across core. +inherit the app-server request trace without broad tracing changes across core. ## Behavior for key v2 APIs ### `thread/start` - create request span -- on successful thread creation: - - annotate request span with `thread.id` - - record thread residency - - emit thread lifecycle event/metric -- send response and `thread/started` with injected trace metadata +- annotate with `thread.id` once known +- send response and `thread/started` +- no separate thread lifecycle span in the initial design ### `thread/resume` - create request span -- on successful resume: - - annotate request span with `thread.id` - - record thread residency if not already tracked - - emit thread lifecycle event/metric when appropriate -- no long-lived thread span +- annotate with `thread.id` when known +- no separate lifecycle span ### `thread/fork` - create request span -- on successful fork: - - annotate request span with the new `thread.id` - - record thread residency for the new thread - - emit thread lifecycle event/metric +- annotate with the new `thread.id` +- no separate lifecycle span ### `thread/unsubscribe` - create request span -- when the last subscriber causes unload: - - use thread residency state to emit unload event/metric and loaded duration - when `thread/closed` or equivalent unload completion occurs +- no separate unload span +- if later thread unload metrics are needed, reuse existing thread state rather + than adding a tracing-only registry ### `turn/start` - create request span -- create turn span on successful start -- propagate turn span context through `Submission` -- emit later streamed notifications under the turn span -- finish turn span on `turn/completed` or abort +- submit work into core under that request span +- propagate the active span context through `Submission.trace` +- let core create a dispatch span and then a long-lived turn span +- let that turn span cover the full duration until completion, interruption, or + failure ### `turn/steer` - create request span -- if active turn exists, optionally add an event or link against the turn span -- do not create a new turn span +- if the request submits core work, propagate via `Submission.trace` +- otherwise request span only ### `turn/interrupt` - create request span -- add an event or link against the active turn span -- let the existing turn span end on abort/completion +- request span only unless core submission is involved ### `review/start` - treat like `turn/start` -- do not introduce a separate tracing architecture for review turns +- let core create the same kind of long-lived turn span -### `thread/realtime/start` +### `thread/realtime/start`, `appendAudio`, `appendText`, `stop` - create request span -- annotate request span with `thread.id` and `session.id` when known -- record realtime session state on success -- emit realtime lifecycle event/metric - -### `thread/realtime/appendAudio` and `appendText` - -- create request span -- annotate request span with `thread.id` and `session.id` -- do not create new long-lived spans - -### `thread/realtime/stop` - -- create request span -- emit realtime lifecycle event/metric and clear session state when the - realtime lifecycle actually ends +- if the API submits work into core, propagate via `Submission.trace` +- do not introduce separate realtime lifecycle spans in the initial design ### Unary methods such as `thread/list` - create request span only -- no lifecycle registry interaction -## Safeguards without `RequestKind` +## Runtime checks -Without a central `RequestKind` taxonomy, we still want safeguards so tracing -drift is visible. +Keep runtime checks narrowly scoped in the initial rollout: -### Runtime checks +- warn when an inbound trace carrier is present but invalid +- test that `Submission.trace` is set when work is submitted from a traced + request -Add warnings or `debug_assert!`s for cases like: +Do not add lifecycle consistency checks for tracing registries that do not +exist yet. -- a completion notification arrives for an unknown turn span -- a thread unloads but there is no tracked residency state for it -- a realtime close/error arrives but there is no tracked session state for it +## Tests -These are strong signals that lifecycle hooks are missing or out of order. +Add tests for the initial mechanics: -### Tests +- inbound request tracing accepts a valid W3C carrier +- invalid carriers are ignored cleanly +- unary methods create request spans without needing any extra handler changes +- `turn/start` propagates request ancestry through `Submission.trace` into core +- `turn/start` creates a long-lived core-owned turn span +- the turn span closes on completion, interruption, or failure +- existing core spans inherit from the propagated parent -Add tests for the nontrivial lifecycle APIs: - -- `thread/start` records thread lifecycle state and emits thread lifecycle - events/metrics correctly -- `turn/start` creates turn span, propagates context, and closes on completion -- `review/start` reuses turn lifecycle tracing -- realtime session start/stop uses the realtime session tracker and emits - lifecycle events/metrics correctly -- unary methods emit request spans but no lifecycle spans - -The goal is not to test OTEL internals exhaustively, but to verify that the -central tracing layer sees the expected lifecycle facts. +The goal is to verify the centralized propagation behavior, not to exhaustively +test OTEL internals. ## Suggested PR sequence -### PR 1: Foundation plus `thread/start` vertical slice +### PR 1: Foundation plus inbound request spans Scope: -1. Introduce a shared `TraceCarrier` crate. -2. Add `meta.trace` to JSON-RPC envelopes in app-server protocol. -3. Add `trace` to `Submission`. -4. Add OTEL conversion helpers in `codex-rs/otel`. -5. Add the centralized app-server tracing module: - - request span builder - - outgoing injector - - lifecycle registries -6. Wire inbound request spans in `message_processor.rs`. -7. Wire outbound trace injection in `outgoing_message.rs`. -8. Wire `thread/start` lifecycle tracing. +1. Introduce a shared `W3cTraceContext` type in `codex-protocol`. +2. Add `trace` to inbound JSON-RPC request envelopes in app-server protocol. +3. Add focused trace-context helpers in `codex-rs/otel`. +4. Add the centralized app-server request tracing module. +5. Wrap inbound request handling in `message_processor.rs`. Why this PR: -- shows the complete architecture in one reviewable vertical slice -- makes the protocol and tracing infrastructure easier to evaluate because - there is a concrete API using them end to end -- validates distributed trace propagation, request spans, outbound injection, - and thread lifecycle events/metrics without yet taking on the async - complexity of turns +- proves the transport and request-span shape with minimal scope +- gives all inbound app-server APIs consistent request tracing immediately +- avoids mixing lifecycle questions into the initial plumbing review -### PR 2: `turn/start` proof of concept +### PR 2: Async handoff into core via `Submission` Scope: -1. Wire `turn/start` lifecycle tracing. -2. Use `Submission` trace propagation into `codex-core`. +1. Add `trace` to `Submission`. +2. Have `CodexThread::submit()` capture the current span context automatically. +3. Have the core submission loop restore parentage with a dispatch span. +4. Validate the flow with `turn/start`. Why this PR: - validates the critical async handoff from app-server into core -- proves the design works for long-lived streamed APIs that complete much later -- exercises turn span creation, streamed notification scoping, and completion - handling +- proves that existing core tracing can inherit the app-server request ancestry +- keeps the behavior change focused on one boundary -### PR 3: Roll out to other long-lived v2 APIs +### PR 3: Core-owned long-lived turn spans Scope: -1. Extend lifecycle tracing to `review/start`. -2. Extend lifecycle tracing to realtime APIs. -3. Extend the same centralized machinery to the rest of the v2 surface. +1. Add a long-lived turn span in core for `turn/start`. +2. Reuse the same turn-span pattern for `review/start`. +3. Ensure the span closes on completion, interruption, or failure. Why this PR: -- applies the already-proven infrastructure to the remaining long-lived APIs -- keeps the initial proof-of-concept focused before broadening coverage -- lets unary APIs inherit request tracing automatically while long-lived APIs - opt into the appropriate lifecycle hooks +- completes the minimum useful tracing story for turn lifecycles +- keeps long-lived span ownership in the layer that actually owns the turn +- still builds on the simpler propagation model from PR 2 instead of mixing + everything into one change + +### PR 4: Optional follow-ups + +Possible follow-ups: + +1. Reuse existing app-server thread state to add thread loaded/unloaded duration + metrics if needed. +2. Reuse existing realtime runtime state to add realtime duration metrics if + needed. +3. Add outbound JSON-RPC trace propagation only if there is a concrete + client-side tracing use case. ## Rollout guidance Start with: -- `thread/start` -- `turn/start` +- inbound request spans for all app-server requests +- `turn/start` request -> core propagation +- a core-owned long-lived turn span for `turn/start` -Those two endpoints exercise all of the important mechanics: +Those pieces exercise the important mechanics: - inbound carrier extraction - request span creation -- thread lifecycle events/metrics and turn lifecycle span creation - async handoff into core -- outbound response/notification injection -- streamed completion +- inherited core tracing beneath the propagated parent +- a single span covering the full duration of a turn -Once those are stable, apply the same centralized machinery to: - -- `thread/resume` -- `thread/fork` -- `review/start` -- realtime APIs -- remaining unary endpoints - -## Open questions - -- Whether client->server and server->client JSON-RPC envelopes should expose - `meta` as a general extensibility bag or only `meta.trace`. -- Whether to use only parent-child relationships or also OTEL links for - control APIs such as `turn/interrupt` and `turn/steer`. -- Whether thread residency tracking should also record subscriber-count or - connection churn as lightweight lifecycle events/metrics. -- Whether some long-lived asynchronous utility APIs such as `command/exec` - should get their own lifecycle registries if they stream for meaningful - durations. +After that, only add more lifecycle-specific tracing if a real debugging or +observability gap remains. ## Bottom line -The recommended design is: +The recommended initial design is: -- trace context on JSON-RPC envelopes -- one standard request span for every inbound request -- centralized outgoing trace injection for every outbound message -- long-lived spans created from actual runtime lifecycle events -- internal propagation through `Submission` into core -- lifecycle hooks expressed in business terms, not OTEL terms +- trace context on inbound JSON-RPC request envelopes +- one standardized request span for every inbound request +- automatic propagation through `Submission` into core +- core-owned long-lived turn spans for turn-producing APIs +- OTEL conversion and carrier logic centralized in `codex-otel` +- no app-server-owned tracing registries for turns, threads, or realtime + sessions in the initial implementation This gives app-server distributed tracing that is: - consistent - low-boilerplate -- unobtrusive in business logic -- suitable for both unary and streaming v2 APIs +- modular +- aligned with the existing ownership boundaries in app-server and core