Add client-perf-diagnosis skill#5599
Conversation
A mode-based diagnostic guide for the client performance telemetry: how to
turn a field complaint ("user XYZ said their card was slow to load", "my tab
froze") into a measured, user-attributed story from the boxel:client-perf Loki
stream and the Client Performance Grafana dashboard.
Leads with the user-attributed drill-down (pivot to a matrix_user_id, pin the
session, find the slow card-load, then spider into the sibling events that
explain it), documents each of the six event types and the LogQL that reads
them, frames realm-event as the causal upstream of rebuild/card-load churn,
and cross-references indexing-diagnostics for the server-side join via the
logging correlation id.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Corrects several descriptions to match the instrument exactly: - `loading_ms`/`settle_ms` are both measured from the card-load window open and differ by one event-loop turn — neither is a first-paint metric; drop the "first content vs fully loaded" framing. - `env` is a hidden per-deployment constant baked at apply time, not an operator-facing drill control; only `matrix_user_id`/`session_id` are. - The wedge filter drops backgrounded-tab gaps outright and screens a foreground gap against an OS sleep/suspend wake by requiring a corroborating LoAF/longtask entry across a short grace window — not "timer coalescing". - The instrument beacons six signal event types plus a keepalive. - The By-user row surfaces signal-event volume, not a retry count. - `retried` is a reauth retry past a transient 401, not exponential backoff. - `profiler_stacks` is also absent where the self-profiler API/policy is unavailable, and `realm`/`env` are JSON fields too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
burieberry
left a comment
There was a problem hiding this comment.
Code review
Verified the skill's claims against the telemetry implementation on the base branch — the doc is accurate throughout except for two points, both of which would mislead a diagnostician in the direction of a false negative. Neither is a code change; both are fixes to SKILL.md.
1. Mode B's primary "find the wedges" query drops longtask-only freezes
The query filters top_frame_function!="":
... | event_type="wedge" | top_frame_function!="" [$__range]
But #emitWedge emits top_frame_function="" whenever the wedge was corroborated by a longtask with no LoAF script attribution — the fallback path (scripts is only populated from loafInWindow, so topFrame is undefined → top_frame_function ?? ''). The field's own comment says as much: "Empty when no LoAF attribution is available (e.g. the longtask fallback)."
The wedge is still recorded — it passed corroboration precisely because a longtask was present. So on any browser/window where a real freeze produced only a longtask entry, this query returns nothing, and Mode B reports "no freeze" on exactly the symptom it exists to diagnose. Suggest dropping the top_frame_function!="" filter (or grouping with a coalesce/fallback label) so longtask-only wedges still surface — the top_frames breadcrumb and longtask_count are still readable on those lines.
2. The retried="true" sentence mis-describes duration_ms
Line ~133 says a retried server-request "re-attempted the fetch after a transient auth (401) refresh, folding the extra round-trip into duration_ms."
The implementation does the opposite. The timing middleware is innermost, so it times each attempt separately (start = now() … now() - start per attempt). A retried event's duration_ms is the final successful attempt's own duration, and recordServerRequestTiming additionally splices the buffered 401 event out of the ring buffer. So the reauth/401 round-trip is excluded, not folded in. As written, the doc would lead a diagnostician to over-attribute a retried request's duration_ms to network/server latency that actually omits the reauth cost. Suggest rewording to: the reauth round-trip is not included — duration_ms covers only the final attempt, and the transient 401 is dropped from the stream.
🤖 Review generated with Claude Code
- Mode B's wedge query filtered `top_frame_function!=""`, which drops a real freeze corroborated only by a longtask (no LoAF attribution leaves the frame empty). Remove the filter so longtask-only wedges surface, and note they group under the blank frame with top_frames/longtask_count still readable. - The retried-request note claimed the reauth round-trip is folded into `duration_ms`. It is the opposite: each attempt is timed separately and the transient 401 is dropped from the stream, so `duration_ms` is the final attempt only and excludes the reauth cost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
[Claude Code 🤖] Both correct — verified against the emit path and fixed:
|
Background
The client performance telemetry adds an always-on browser instrument that beacons six event types to Loki and a Grafana dashboard, so real client behavior is measurable and attributable to a specific user or session. This adds the operator-facing counterpart: a diagnostic skill that turns a field complaint into a measured investigation against that telemetry.
The motivating shape of the complaint is concrete — "user XYZ said the app was slow loading their card" — and the whole reason attribution is in the pipeline is to answer it. So the skill leads with the user-attributed drill-down: pivot to the account's
matrix_user_id, pin the session, find the slowcard-load(the "Loading card…" window) and itssettle_ms, then spider into the sibling events in that same session that explain the cost.What the skill covers
A mode per investigation, all sharing the
matrix_user_id/session_iddrill-down:wedgeevents and theirfn @ url:charblocking-frame breadcrumb (source-map-resolvable for minified builds), plus profiler stacks where sampled.server-requestlatency by endpoint, joined to the realm-server's ownrealm:requests/realm:search-timingstage breakdown via the logging correlation id.It documents the pipeline, the Loki label-vs-JSON-field split, the six event schemas, the working LogQL for each mode (lifted from the dashboard so they match the instrument), the local-dev bring-up, and the seam to
indexing-diagnosticsfor the server-side half of a request.Notes
mainonce the base merges.🤖 Generated with Claude Code