feat(browser): opt-in long animation frame and slow interaction spans - #1158
Conversation
- tracing.longFrames spans every frame of 100ms or more as longAnimationFrame, attributed to the script that ran longest (code.file.path, code.function.name, maple.browser.script.invoker and duration) plus the frame's blocking duration. Browsers without the Long Animation Frames API get longtask spans instead. - tracing.slowInteractions spans every interaction of 200ms or more as interaction <event>, named after the event whose handlers ran longest (the click, not its pointerdown), with input delay, processing and presentation times and the target selector. - Both use buffered PerformanceObservers in the deferred chunk, so jank from before the SDK finished loading is reported too, and nest under the open navigation span. They follow tracing.sampleRate. The session sink's short selector helper is exported for the interaction target. Deferred budget 12 -> 14 kB; eager is unchanged.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Maple reviewConfidence 4/5 · likely safe to merge Adds opt-in
FindingsNote · F1 ·
|
| Change | Kind | Observable | Evidence |
|---|---|---|---|
| long frame / long task observer (PerformanceObserver) | background work | yes | onLongFrame ends a longAnimationFrame/longtask span per entry, parented to the open navigation (perf.ts:37-39, perf.ts:73-88) |
| slow interaction observer (PerformanceObserver) | background work | yes | spanInteraction ends one interaction <event> span per interaction id (perf.ts:110-121, perf.ts:151-154) |
Copy all findings (1)
Findings from an automated review of commit 46da4bed1bd0f67de813588ae3beda50439d11db. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.
---
F1 · Note · maintainability · packages/browser/src/config.ts:91-101
`captureHeaders` JSDoc block now documents `longFrames`
The `captureHeaders` doc block (`config.ts:85-90`) is left directly above the two new options, so in the published typings and editor tooltips `longFrames` is described as recording request/response headers while `captureHeaders` documents nothing. Move the new options below `captureHeaders` so each option keeps its own comment.
Replace those lines with:
readonly captureHeaders?: {
readonly request?: ReadonlyArray<string>
readonly response?: ReadonlyArray<string>
}
/**
* Span main-thread frames of 100ms or more (`longAnimationFrame`, with the
* script that ran longest; `longtask` where that API is missing). Default false.
*/
readonly longFrames?: boolean
/** Span interactions of 200ms or more (`interaction click`, ...), split into input delay, processing and presentation. Default false. */
readonly slowInteractions?: boolean
46da4be · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple to ask about one.
|
Note A newer push replaced |
Maple reviewConfidence 5/5 · safe to merge Adds two opt-in jank span sources to the browser SDK:
Still open from earlier reviews
What was checked
Observability coverage: 2 of 2 changes observable
|
Maple reviewConfidence 3/5 · needs attention Adds opt-in long-frame and slow-interaction spans to the browser SDK's deferred chunk, plus the
FindingsWarning · F2 ·
|
| Change | Kind | Observable | Evidence |
|---|---|---|---|
Long-frame spans (longAnimationFrame / longtask fallback) with script attribution |
span | yes | Spans go through liveMapleTracer (perf.ts:35-39), so they export over OTLP like other browser spans |
Slow-interaction spans (interaction <event>) under the open navigation |
span | yes | Same span() helper, attributes maple.browser.interaction.* (perf.ts:110-121) |
Copy all findings (1)
Findings from an automated review of commit a9b49cbc4712f605cfbb534e6c04c4a5ae70fe56. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.
---
F2 · Warning · correctness · packages/browser/src/deferred/perf.ts:49
`scriptsOf` reads script attributes with `Object.entries`, so they are always undefined
A real `PerformanceScriptTiming` keeps `invoker`, `sourceURL` and `sourceFunctionName` as IDL accessors on its prototype, so `Object.entries(script)` returns no own enumerable keys and `text()` returns `undefined` for all three: production `longAnimationFrame` spans ship without `code.file.path`, `code.function.name` and `maple.browser.script.invoker`, only `duration` (read directly) survives. The test passes a plain object literal, whose keys are own and enumerable, so it cannot catch this. Read the attribute off the object instead (`Reflect.get(script, key)` / `(script as Record<string, unknown>)[key]`).
Replace those lines with:
const text = (key: string): string | undefined => {
const value = Reflect.get(script, key)
return typeof value === "string" && value !== "" ? value : undefined
}
a9b49cb · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple to ask about one.
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
PerformanceScriptTiming keeps invoker, sourceURL and sourceFunctionName as prototype getters, which Object.entries never sees, so real long frames shipped without script attribution. Fields are read directly now, and the test's script timings are getter-backed like the real ones. Also moves the new tracing options below captureHeaders so each keeps its own doc comment.
Maple reviewConfidence 4/5 · likely safe to merge Adds two opt-in browser tracing sources:
Fixed since the last review
What was checked
Observability coverage: 2 of 2 changes observable
|
1 participant
Part 12 of the browser SDK stack. Based on the headers/bodies/canvas PR.
What changes
tracing.longFramesspans every frame of 100ms or more aslongAnimationFrame, attributed to the script that ran longest (code.file.path,code.function.name,maple.browser.script.invoker,maple.browser.script.duration_ms), plusmaple.browser.frame.blocking_duration_ms. Browsers without the Long Animation Frames API getlongtaskspans instead.tracing.slowInteractionsspans every interaction of 200ms or more (INP's "needs improvement" line) asinteraction <event>. The name comes from the event whose handlers ran longest, so it's theclick, not thepointerdownof the same interaction. Attributes: input delay, processing, presentation and the target selector.PerformanceObservers in the deferred chunk, so jank from before the SDK finished loading is reported too. They nest under the open navigation span and followtracing.sampleRate.instrumentation-user-interactionisn't used: it needs zone.js for useful async context, and event timing already carries the breakdown.The session sink's short selector helper is exported for the interaction target. Deferred budget 12 → 14 kB; eager is unchanged.
Testing
Browser tests:
userEventbecomes oneinteraction clickspan with the target and processing time;longtaskfallback with LoAF support hidden (headless Chromium lists LoAF but renders no frames to report);Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit