[codex] Record exec-server lifecycle metrics#27467
Conversation
Codex Cloud Agents (CCA) couldn't complete this review. The original Codex Review is unaffected. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6b943d92c
ℹ️ 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".
| let active = inner | ||
| .connection_counter(transport) | ||
| .fetch_add(1, Ordering::AcqRel) | ||
| + 1; | ||
| inner.gauge( | ||
| CONNECTIONS_ACTIVE_METRIC, | ||
| CONNECTIONS_ACTIVE_DESCRIPTION, | ||
| active, | ||
| &[("transport", transport.metric_tag())], |
There was a problem hiding this comment.
Serialize active connection gauge updates
When two connections of the same transport start or finish concurrently, the atomic counter update and the gauge recording can be reordered: e.g. one disconnect computes active = 1, another computes and records 0, then the first records stale 1, leaving exec_server_connections_active overreported until another event. Please make the active gauge observe the atomic value at collection time or protect the update+record sequence so the emitted value cannot go backwards after a newer count was recorded.
Useful? React with 👍 / 👎.
d6b943d to
7a28c0a
Compare
e06c4e5 to
f3958b2
Compare
f3958b2 to
1ef0691
Compare
5a94cf1 to
1128e27
Compare
1ef0691 to
eb4572e
Compare
1128e27 to
2d7885f
Compare
Summary
codex exec-serversubprocess.Stack
Review and land this stack in order:
Validation
just test -p codex-exec-server --lib(147 passed)just test -p codex-cli --test exec_server(3 passed)just argument-comment-lintjust bazel-lock-checkjust fix -p codex-exec-server -p codex-clijust fmt