fix: propagate tracing_ids to all route handlers#146
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates TracingIds into various API routes (such as catch-all, completions, and passthrough endpoints) by extracting them via Axum's Extension and passing them down to the proxy options. The feedback identifies two instances in catch_all.rs and passthrough.rs where tracing_ids is unnecessarily cloned. Since the conditional branches are mutually exclusive, tracing_ids can be moved directly without cloning, which avoids redundant heap allocations.
| backend_guard: Some(backend_guard), | ||
| response_shape: ResponseShape::ChatCompletion, | ||
| tracing_ids: None, | ||
| tracing_ids: Some(tracing_ids.clone()), |
There was a problem hiding this comment.
The .clone() here is unnecessary. Since the if and else if branches are mutually exclusive, Rust's borrow checker allows moving tracing_ids into both branches without cloning, avoiding redundant heap allocations of the TracingIds strings on the hot path.
| tracing_ids: Some(tracing_ids.clone()), | |
| tracing_ids: Some(tracing_ids), |
| backend_guard: None, | ||
| response_shape: ResponseShape::ChatCompletion, | ||
| tracing_ids: None, | ||
| tracing_ids: Some(tracing_ids.clone()), |
There was a problem hiding this comment.
The .clone() here is unnecessary. Since the Some and None branches of the match url_override are mutually exclusive, Rust's borrow checker allows moving tracing_ids into both branches without cloning, avoiding redundant heap allocations of the TracingIds strings on the hot path.
| tracing_ids: Some(tracing_ids.clone()), | |
| tracing_ids: Some(tracing_ids), |
… logs Two fixes for e2e tracing: 1. Propagate TracingIds to all route handlers (not just chat_completions): - completions.rs: extract Extension<TracingIds> - catch_all.rs: extract Extension<TracingIds> - passthrough.rs: extract in all handlers, add param to helper Previously these routes had tracing_ids: None, making org_id, request_id, workspace_id invisible in Datadog dashboards. 2. Lowercase model name in structured logs: The proxy logged model=opts.model_name (original case, e.g. 'Qwen/Qwen3.5-122B-A10B') but Datadog lowercases ALL metric tags and Docker label tags (e.g. 'qwen/qwen3.5-122b-a10b'). This caused the same traffic to appear under two different model names depending on which facet you query (@fields.model vs model tag), breaking joins with cloud_api metrics which also use lowercase model tags. Now logs emit lowercase model names consistent with all other sources.
lloydmak99
force-pushed
the
fix/tracing-ids-all-routes
branch
from
b8d6a1f to
1627f0e
Compare
1 participant
Problem
The e2e tracing PR (#130) only wired
TracingIds(request_id,org_id,workspace_id) into the/v1/chat/completionshandler. All other routes hadtracing_ids: None, meaning requests to those endpoints were logged withoutrequest_id,org_id, orworkspace_id— making them invisible in the Datadog e2e tracing dashboard (which filters@fields.org_id:*).Affected routes:
/v1/completions/v1/embeddings/v1/rerank/v1/score/v1/images/generations/v1/images/edits/v1/audio/transcriptionsFix
Extract
Extension<TracingIds>in all route handlers and passSome(tracing_ids)through toProxyOpts:completions.rs: extract and propagateTracingIdscatch_all.rs: extract and propagate in both streaming and JSON branchespassthrough.rs: extract in all 5 handler fns, addtracing_idsparam tojson_passthrough_encryptedhelper, propagate in allProxyOpts(bothurl_overrideandbackend_poolbranches)Not changed:
tokenizeroute — usesproxy::proxy_simplewhich has noProxyOpts(no signing, no usage, no structured logging).Testing
cargo check— cleancargo clippy --all-targets— cleancargo test— 134/134 passed