feat(logs): expose request timing breakdown - #6755
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR adds request-timing capture for supported relay requests. It records lifecycle phases, persists snapshots in logs, and displays normalized timing breakdowns in usage-log interfaces. ChangesRequest timing observability
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RequestTimingMiddleware
participant RelayInfo
participant Upstream
participant LogStore
participant UsageLogUI
Client->>RequestTimingMiddleware: Send supported POST request
RequestTimingMiddleware->>RelayInfo: Attach RequestTimingSession
RelayInfo->>Upstream: Mark attempt and send request
Upstream-->>RelayInfo: Return response or first data
RelayInfo->>RequestTimingMiddleware: Record client writes
RequestTimingMiddleware->>LogStore: Persist request_timing snapshot
LogStore-->>UsageLogUI: Provide request_timing metadata
UsageLogUI-->>Client: Render timing phases
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@middleware/request_timing_test.go`:
- Around line 46-70: In TestRequestTimingMiddlewareIgnoresKeepaliveWrites,
remove the exact equality assertion on the dereferenced FirstDataToClientMs
value while retaining the existing non-nil assertion. Keep the test focused on
verifying that the keepalive PING is not treated as the first client write.
In `@relay/channel/cloudflare/relay_cloudflare.go`:
- Around line 64-68: In the response loop, keep the first-data marker before
helper.ObjectData while moving info.SetFirstResponseTime() to after
helper.ObjectData completes, preserving the existing FirstResponseTime boundary
that includes rendering and write time. Split the isFirst handling as needed
without changing the existing error flow.
In
`@web/src/features/usage-logs/components/__tests__/request-timing-display.test.tsx`:
- Around line 1-195: Rewrite the request timing display suite to use Vitest and
React Testing Library: replace node:test, happy-dom global setup, createRoot,
act, and manual cleanup with RTL render and screen queries. Update assertions to
use user-facing queries such as screen.getByRole or getByLabelText, and use
userEvent to exercise the timing breakdown trigger interaction rather than
inspecting querySelector results or tagName. Preserve coverage for stream
phases, timing headlines, and non-admin details rendering.
In `@web/src/features/usage-logs/components/timing-metrics-cell.tsx`:
- Around line 161-175: Update the TooltipTrigger accessibility in the timing
metrics component so its accessible name retains the visible duration labels and
values rendered by content. Replace the aria-label usage with an
aria-describedby hint for “Timing breakdown,” or otherwise include the visible
text in the accessible name, and update the associated test assertion to match
the chosen ARIA behavior.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: d5f2b4bc-6f9d-4743-abe0-21bdac5701c8
📒 Files selected for processing (33)
common/request_timing.gocommon/request_timing_test.gomiddleware/request_timing.gomiddleware/request_timing_test.gomodel/log.gomodel/log_format_test.gomodel/log_request_timing_test.gorelay/channel/cloudflare/relay_cloudflare.gorelay/channel/cohere/relay-cohere.gorelay/chat_completions_via_responses.gorelay/claude_handler.gorelay/common/relay_info.gorelay/common/relay_info_test.gorelay/compatible_handler.gorelay/gemini_handler.gorelay/responses_handler.gorouter/relay-router.goweb/src/features/usage-logs/components/__tests__/request-timing-display.test.tsxweb/src/features/usage-logs/components/columns/common-logs-columns.tsxweb/src/features/usage-logs/components/dialogs/details-dialog.tsxweb/src/features/usage-logs/components/request-timing-breakdown.tsxweb/src/features/usage-logs/components/timing-metrics-cell.tsxweb/src/features/usage-logs/components/usage-logs-mobile-card.tsxweb/src/features/usage-logs/lib/__tests__/request-timing.test.tsweb/src/features/usage-logs/lib/request-timing.tsweb/src/features/usage-logs/types.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.json
7cf44ea to
a7f2455
Compare
|
@coderabbitai review |
|
|
@coderabbitai review |
|
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 `@Dockerfile.dev`:
- Line 15: Replace the ADD instruction for relaykit/go.mod with COPY, preserving
the existing source and destination paths.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 96ef4b11-7f5d-46dc-bca1-4fcb8584f53d
📒 Files selected for processing (1)
Dockerfile.dev
|
@coderabbitai review |
|
51fdfc5 to
2b6f1df
Compare
Important
📝 变更描述 / Description
为选定的主流 LLM 请求入口记录端到端阶段耗时,并写入现有日志字段
other.request_timing。本次不新增数据库字段、迁移、配置项或日志接口。POST /v1/chat/completions、/v1/completions、/v1/responses、/v1/messages以及 GeminigenerateContent/streamGenerateContent。request_timing,敏感的admin_info过滤逻辑不变。total_ms及端到端首数据耗时,旧日志继续回退到use_time/frt。use_time、frt、性能指标和计费逻辑,不扩大到 embeddings、图片、音频、realtime、任务等接口。脱敏后的流式日志结构示例:
{ "request_timing": { "total_ms": 1320, "gateway_ms": 24, "upstream_first_data_ms": 411, "first_data_to_client_ms": 2, "client_stream_ms": 862, "finalize_ms": 21 } }无法可靠取得的里程碑会省略,合法的
0ms阶段会保留。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
核心测试矩阵:
request_timing,仍不可见admin_infouse_time/frt0ms阶段本地结果:
完整前端测试剩余 3 个失败均位于既有
api-key-group-cell.test.tsxAuto ring 断言;在未修改的origin/main基线上可复现。本 PR 新增和涉及的前端测试全部通过。Summary by CodeRabbit
New Features
Bug Fixes