feat(agentsight): add LLM latency metrics API - #2578
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0e2aaa07c
ℹ️ 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".
|
@chengshuyi The automated review raised two points that I’d like to confirm with you before making further changes: PR size I’ll wait for your preference on these two points before making further changes. |
chengshuyi
left a comment
There was a problem hiding this comment.
LGTM. Clean design — derive metrics at query time, capture first-output timestamp atomically, good test coverage (~60% of diff).
A few minor nits (non-blocking):
-
TPS/TPOT convention: Keep N (total output_tokens) per the issue definition. Add a one-line comment in
stats.rsnoting this is by convention and the N vs N-1 difference is negligible for typical response lengths. -
first_output_timestamp_ns()in http2.rs: The per-frame re-parse is O(frames²) worst case. In practice this is fine (first token arrives early → early return; no-output responses have tiny bodies). A one-line comment explaining this trade-off would help future readers. -
stats.rsformatting: Missing blank line betweenget_latency_metricsclosing brace and the next doc comment (/// One bucket in a token time-series query.). -
PR size: Accepting the 998-line diff given it's test-heavy and the data path (capture → persist → query) needs to stay together for end-to-end testability.
Optional follow-ups (not blocking):
- Sync AGENTS.md API Endpoints table with
/api/metrics/latency response_sse_event_count()doc: note that count includes non-JSON events like[DONE](differs fromresponse_sse_json_array().len())
|
Follow-up: the AgentSight CI lint failures from this PR and the non-blocking review nits are addressed in #2583. |
Summary
Backend/API phase for #2339.
Refs #2339
This PR adds backend collection and querying for LLM latency and throughput metrics in AgentSight.
What changed
agent_namefilters.[DONE]-only and metadata-only SSE responses remain streaming calls.first_output_timestamp_nsasnullwhen no meaningful output is observable.nullfor non-SSE responses where first-token timing cannot be reliably observed.first_output_timestamp_nsasnullfor compressed or otherwise unobservable HTTP/2 SSE responses.schema_version.API
Adds latency metric aggregation with:
agent_namefilteringScope
This PR contains the backend/API phase only.
It does not include:
The Agent Panel UI will be handled separately.
Testing
cargo fmt --all -- --checkgit diff --checkcargo test --lib: 1483 passed / 0 failed / 1 ignored[DONE]-only, metadata-only, and regular JSON responsescargo testadditionally encounters an existing doctest failure insrc/genai/helpers.rs. The same failure is reproducible on the currentupstream/main, and that file is not modified by this PR.