exec-server: standardize JSON-RPC request spans#31687
Conversation
anp-oai
changed the title
|
@codex review |
|
@codex review for matching app-server's OTEL shape |
![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.
Reviewed commit: 3b6a3e7004
ℹ️ 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".
anp-oai
force-pushed
the
codex/exec-server-rpc-span-fields
branch
from
3b6a3e7 to
c671b06
Compare
anp-oai
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ 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". |
Align client and server spans with app-server's RPC attributes, centralize client span creation in call_inner so timed calls are traced, and serialize tracing tests that rebuild global callsite interest.
anp-oai
force-pushed
the
codex/exec-server-rpc-span-fields
branch
from
c671b06 to
1ecebbc
Compare
| let _call_slot = self.acquire_regular_call_slot()?; | ||
| self.call_inner(method, params, RpcCallTimeout::None).await | ||
| let request_id = self.allocate_request_id(); | ||
| self.call_inner(request_id, method, params, RpcCallTimeout::None) |
There was a problem hiding this comment.
I think this drops telemetry for the two early admission failures: regular calls can return PendingRequestLimitExceeded before call_inner, and cleanup overflow closes the transport before reaching it too...
Both were inside the outer request spans before this change, so saturation is now exactly when this telemetry disappears
| otel.kind = "server", | ||
| otel.name = span_name, | ||
| rpc.system = "jsonrpc", | ||
| rpc.method = method, |
There was a problem hiding this comment.
NIT: should rpc.method use the bounded routed span_name here?
2 participants
Why
app-server OTEL spans for RPC methods include a standard set of fields; exec-server should match, and every client request path should share the same span boundary.
What
rpc.system,rpc.method, andrpc.request_idto client and server request spans.call_inner, covering normal, timed, and cleanup calls without duplicate nesting.Validation
just test -p codex-exec-server process_start_propagates_caller_trace_context_across_background_task(1 passed)bazel test //codex-rs/exec-server:exec-server-unit-tests --runs_per_test=30 --nocache_test_results --test_output=errors(the tracing test passed 30/30; 11 aggregate runs failed only in unrelated relay/recovery timing tests under 30-way load)just test -p codex-exec-server(306 passed, 2 skipped)just fix -p codex-exec-server(clean)just fmt(clean)Stack