Repository navigation
feat(gateway): derive backend request ids from rid and the middleware request id - #1960
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughAdds optional ChangesRequest ID propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GatewayMiddleware
participant RequestType
participant RequestBuilder
participant Backend
Client->>GatewayMiddleware: Send rid or x-request-id
GatewayMiddleware->>RequestType: Preserve request metadata
RequestType->>RequestBuilder: Provide protocol rid
RequestBuilder->>Backend: Send resolved backend request ID
Backend-->>Client: Return response id
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
… request id Backend engine ids were minted independently per endpoint, so gateway logs, engine logs, and response ids never correlated. All gRPC request-building stages now resolve ids through one helper: the protocol rid (added to chat/completions/messages, already on generate/embeddings/classify) is used verbatim outside PD, else the middleware request id (client-sent x-request-id or generated) with a per-execution suffix, else the endpoint-prefixed mint. HTTP mode passes the typed rid through natively. Closes #1874 Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
e48427b to
7191a7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/getting-started/logging.md`:
- Around line 359-364: Update the request-ID documentation to distinguish the
default gRPC `{request-id}-{uuid}` behavior from protocol-level `rid` handling:
outside PD mode, use `rid` verbatim, except batch completions return the shared
`rid` while dispatching each prompt with a per-prompt ID such as `{rid}-p{i}`.
State when response IDs match the shared or per-request ID rather than implying
the rule applies universally.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 850dc0b5-1734-4373-8083-5732154f294b
📒 Files selected for processing (21)
bindings/golang/client.gocrates/grpc_client/src/vllm_engine.rscrates/protocols/src/chat.rscrates/protocols/src/completion.rscrates/protocols/src/messages.rsdocs/getting-started/logging.mde2e_test/chat_completions/test_openai_server.pye2e_test/completions/test_basic.pymodel_gateway/benches/request_processing.rsmodel_gateway/src/middleware/request_id.rsmodel_gateway/src/middleware/tenant_resolution.rsmodel_gateway/src/routers/grpc/common/stages/helpers.rsmodel_gateway/src/routers/grpc/context.rsmodel_gateway/src/routers/grpc/harmony/stages/request_building.rsmodel_gateway/src/routers/grpc/regular/stages/chat/request_building.rsmodel_gateway/src/routers/grpc/regular/stages/completion/request_building.rsmodel_gateway/src/routers/grpc/regular/stages/embedding/request_building.rsmodel_gateway/src/routers/grpc/regular/stages/generate/request_building.rsmodel_gateway/src/routers/grpc/regular/stages/messages/request_building.rsmodel_gateway/src/routers/grpc/utils/message_utils.rsmodel_gateway/tests/routing/test_openai_routing.rs
Description
Problem
ridpassthrough existed on generate/embeddings/classify/rerank but not chat or completions (#1874) — and on gRPC it was only honored by/generate: embeddings/classify mintedembed-/classify-ids ignoring their ownridfields. Worse, every request-building stage minted its own id independently of the request-id middleware (middleware/request_id.rs), so three ids existed per request — the middleware id in gateway logs, the stage-minted id in engine logs and the response body, and whatever the client sent — none correlated.Solution
One resolver, every path.
helpers::resolve_request_idis now the single source of backend request ids for all gRPC request-building stages (chat, harmony chat/responses, messages, completions incl. batch, generate, embeddings, classify):rid(added to chat, completions, and messages; already on generate/embeddings/classify) — used verbatim outside PD; per-attempt-{uuid}suffix in PD (matching/generate's long-standing NIXL-lease rule).x-request-id,x-correlation-id, …) or middleware-generated; carried into the pipeline on the tenant request metadata (tenant resolution runs insideRequestIdLayerand copies the extension — zero router-signature changes). Always suffixed-{uuid}per execution, which keeps retries, PD attempts, and responses tool-loop iterations unique while staying grep-able by prefix.HTTP mode needs no mechanism: the typed
ridfields serialize through natively, and the middleware already echoesx-request-id.Net effect: gateway log id, engine log id, and response body
idare one lineage for every request —ridwhen given, the middleware id otherwise. Batched completions keep the clean shared id on the response and suffix per-sub engine ids.Behavior notes:
{middleware-id}-{uuid}(e.g.chatcmpl-Abc…-0198…); endpoint prefixes are preserved, including a newmsg_arm in the middleware generator so Anthropic message ids keep their shape.containsscans toends_withsuffix compares — sub-resource paths (e.g.GET /v1/responses/{id}) now generate the genericreq-log id instead ofresp-; those ids never reach response bodies.{request-id}-{uuid}per iteration, correlating engine work back to the originating request.Changes
crates/protocols:rid: Option<String>onChatCompletionRequest,CompletionRequest,CreateMessageRequest(typed field replaces theother-flatten passthrough on HTTP).grpc/common/stages/helpers.rs:resolve_request_id+middleware_request_id;RequestType::rid()accessor incontext.rs.{shared}-p{i}with the same uniqueness rules.middleware/tenant_resolution.rs: carriesRequestIdon the request meta;middleware/request_id.rs:msg_arm +ends_withmatching.RidonChatCompletionRequest.Test Plan
RequestId.cargo test -p smg -p openai-protocol -p smg-grpc-client: all green (smg lib 1233; api 106, routing 94, spec 96, security 50, …; protocols 84+126; grpc_client 65) — full-server api_tests exercise the new id lineage end-to-end.rid→response.id == rid; chatx-request-id: corr-abc→response.idstarts withcorr-abc-; completions scalar + batchrid→ response id equals rid.cargo clippy --all-targets -- -D warnings,cargo +nightly fmt --check,ruffclean; Go SDK package builds (go build .; examples needlibsmg_go, unchanged).Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
rid) support to chat, completion, messages, and other request types.ridis forwarded for backend log correlation and reflected in responseids.idformatting.