[codex] Record exec-server connection 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 👍 / 👎.
Summary
Stack
Validation
just test -p codex-exec-server --lib(112 passed)