Conversation
# Conflicts: # codex-rs/core/tests/chat_completions_payload.rs # codex-rs/core/tests/chat_completions_sse.rs # codex-rs/core/tests/responses_headers.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
# Conflicts: # codex-rs/core/src/codex.rs
# Conflicts: # codex-rs/core/Cargo.toml
# Conflicts: # codex-rs/otel/src/traces/otel_provider.rs # docs/config.md # docs/example-config.md
codex-rs/otel/src/lib.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub fn counter(&self, name: &str, inc: i64, tags: &[(&str, &str)]) -> MetricsResult<()> { |
There was a problem hiding this comment.
we almost always ignore the return value of this method. Is it better to log internally and not force consumers to let _ =..
There was a problem hiding this comment.
I can make a wrapper yep
| - `auth_mode`: `swic` (AuthMode::ChatGPT) | `api` (AuthMode::ApiKey) | `unknown`. | ||
| - `model`: name of the model used. | ||
|
|
||
| ## Metrics catalog |
There was a problem hiding this comment.
I guess it is nice to have these in one place -- is there something we can do to prevent it getting stale?
There was a problem hiding this comment.
Yeah I was thinking of that... I don't think this is trivial but we can probably build a build time checker
Will think on this...
| | `feature.duration_ms` | histogram | `feature`, `status` | End-to-end feature latency. | | ||
| | `feature.used` | counter | `feature` | Feature usage through `/` (e.g., `/undo`, `/review`, ...). | |
There was a problem hiding this comment.
Can you derive counts from a histogram? Is there a reason to have both?
There was a problem hiding this comment.
It's quite standard actually
Those are ingested "as-is" by statsig
# Conflicts: # codex-rs/core/src/client.rs # codex-rs/core/tests/chat_completions_payload.rs # codex-rs/core/tests/chat_completions_sse.rs # codex-rs/core/tests/responses_headers.rs # codex-rs/core/tests/suite/client.rs # codex-rs/otel/src/traces/otel_manager.rs
3 participants
Add metrics capabilities to Codex. The
README.mdis up to date.This will not be merged with the metrics before this PR of course: #8350