feat(otel, core): record turn TTFT and TTFM metrics in codex-core#13630
feat(otel, core): record turn TTFT and TTFM metrics in codex-core#13630
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/core/src/tasks/mod.rs
Lines 154 to 155 in d7afd1f
The task is spawned first, and turn state is marked started only later in register_new_active_task after an await on total_token_usage. A fast stream can emit first output before mark_turn_started, so record_turn_ttft/ttfm returns None and misses (or delays) the first-latency metric for that turn.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
d7afd1f to
98d86b9
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
pakrym-oai
left a comment
There was a problem hiding this comment.
Can we offload more logic from codex.rs?
Summary
This adds turn-level latency metrics for the first model output and the first completed agent message.
codex.turn.ttft.duration_msstarts at turn start and records on the first output signal we see from the model. That includes normal assistant text, reasoning deltas, and non-text outputs like tool-call items.codex.turn.ttfm.duration_msalso starts at turn start, but it records when the first agent message finishes streaming rather than when its first delta arrives.Implementation notes
The timing is tracked in codex-core, not app-server, so the definition stays consistent across CLI, TUI, and app-server clients.
I reused the existing turn lifecycle boundary that already drives
codex.turn.e2e_duration_ms, stored the turn start timestamp in turn state, and record each metric once per turn.I also wired the new metric names into the OTEL runtime metrics summary so they show up in the same in-memory/debug snapshot path as the existing timing metrics.