[codex-analytics] Track CodexErr details in turn analytics#25707
Conversation
1b568c2 to
e27c689
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e27c6892e6
ℹ️ 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".
| Self { | ||
| kind: error.into(), | ||
| subreason: match error { | ||
| CodexErr::InvalidRequest(message) => Some(message.clone()), |
There was a problem hiding this comment.
Bound invalid-request telemetry payloads
When a terminal CodexErr::InvalidRequest comes from an HTTP 400 response, codex-api/src/api_bridge.rs constructs it from the entire response body (CodexErr::InvalidRequest(body_text)), and this new code copies that string verbatim into codex_error_subreason. For provider/API 400s with a large or diagnostic response body, the turn analytics event can become unexpectedly large and may include raw upstream error contents; please cap and sanitize/classify this value before adding it to telemetry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
oh this is a good call out, let's put a cap on max size and truncate?
| Fatal, | ||
| Io, | ||
| Json, | ||
| #[cfg(target_os = "linux")] |
There was a problem hiding this comment.
we could probably drop these annotations? as in, we could define them for all OSes but only linux would ever emit it
I think we try to avoid these OS-specific annotations unless they're absolutely necessary
There was a problem hiding this comment.
hmm nvm, codex says we need these since these variants don't exist at all in macos/windows builds. disregard
6328cb4 to
34cde64
Compare
34cde64 to
4c9529f
Compare
Summary
CodexErrtelemetry tocodex_turn_eventwhile leaving existingturn_errorunchangedCodexErrfacts from core immediately before the existing turn error event is sentcodex_error_*fields for downstream analytics, including the rawCodexErr::InvalidRequest(String)message ascodex_error_subreasonValidation
just test -p codex-analyticsjust test -p codex-core, but the local run timed out across unrelated integration suites in this environment and is not being used as validation