Track JS bundle parse time as a Sentry span#39
Open
elirangoshen wants to merge 8 commits intomainfrom
Open
Conversation
📝 WalkthroughWalkthroughAdds telemetry instrumentation for measuring JavaScript parse time and reporting tab navigation events. New span name constants are defined, the endSpan function is extended to support custom end times, and a PerformanceObserver monitors bundle parsing events to capture parse duration metrics. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
elirangoshen
changed the title
elirangoshen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/setup/telemetry/index.ts (1)
44-64: Consider defensive handling for entry ordering.The implementation correctly tracks JS bundle parse time using the PerformanceObserver API. However, there's a subtle edge case: if
runJsBundleEndappears beforerunJsBundleStartin the entries array (e.g., if entries aren't guaranteed to be chronologically ordered), the observer would disconnect without recording the span.While this is unlikely in practice since entries should arrive in order, you could add robustness by deferring the disconnect until both events are processed:
♻️ Optional defensive fix
let jsParseStartMs: number | undefined; + let jsParseEndMs: number | undefined; const observer = new PerformanceObserver((list) => { const entries = list.getEntries(); for (const entry of entries) { if (entry.name === 'runJsBundleStart' && jsParseStartMs === undefined) { jsParseStartMs = entry.startTime; startSpan(CONST.TELEMETRY.SPAN_JS_PARSE_TIME, { name: CONST.TELEMETRY.SPAN_JS_PARSE_TIME, op: CONST.TELEMETRY.SPAN_JS_PARSE_TIME, startTime: jsParseStartMs / 1000, }); } - if (entry.name === 'runJsBundleEnd') { - if (jsParseStartMs !== undefined) { - endSpan(CONST.TELEMETRY.SPAN_JS_PARSE_TIME, entry.startTime / 1000); - } - observer.disconnect(); + if (entry.name === 'runJsBundleEnd' && jsParseEndMs === undefined) { + jsParseEndMs = entry.startTime; } } + if (jsParseStartMs !== undefined && jsParseEndMs !== undefined) { + endSpan(CONST.TELEMETRY.SPAN_JS_PARSE_TIME, jsParseEndMs / 1000); + observer.disconnect(); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/setup/telemetry/index.ts` around lines 44 - 64, The observer loop can disconnect before the start span is recorded if a runJsBundleEnd entry is seen before runJsBundleStart; update the handler in the PerformanceObserver callback to track both start and end events (e.g., use jsParseStartMs and a jsParseEndTime variable or a boolean jsParseEndSeen), on runJsBundleStart set jsParseStartMs and call startSpan, on runJsBundleEnd set the end time and only call endSpan and observer.disconnect() once jsParseStartMs is defined (or when both start and end flags/times are present), ensuring you still call endSpan(CONST.TELEMETRY.SPAN_JS_PARSE_TIME, ...) with the end timestamp and then observer.disconnect().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Mobile-Expensify`:
- Line 1: The PR contains a submodule pointer referencing the orphaned commit
ec6cc1176b78f7a7216c1bd4171b377d5da85e9f which is unreachable from the remote;
fix by either pushing the intended commit to the Mobile-Expensify remote so that
ec6cc11 becomes reachable and update the submodule pointer accordingly, or
change the submodule pointer (the gitlink entry for the Mobile-Expensify
submodule in the repository index/.gitmodules) to a commit that is present on a
tracked remote branch or tag; ensure the updated pointer references an auditable
commit on the remote before re-running the CI and merging.
---
Nitpick comments:
In `@src/setup/telemetry/index.ts`:
- Around line 44-64: The observer loop can disconnect before the start span is
recorded if a runJsBundleEnd entry is seen before runJsBundleStart; update the
handler in the PerformanceObserver callback to track both start and end events
(e.g., use jsParseStartMs and a jsParseEndTime variable or a boolean
jsParseEndSeen), on runJsBundleStart set jsParseStartMs and call startSpan, on
runJsBundleEnd set the end time and only call endSpan and observer.disconnect()
once jsParseStartMs is defined (or when both start and end flags/times are
present), ensuring you still call endSpan(CONST.TELEMETRY.SPAN_JS_PARSE_TIME,
...) with the end timestamp and then observer.disconnect().
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Mobile-Expensifysrc/CONST/index.tssrc/libs/telemetry/activeSpans.tssrc/setup/telemetry/index.ts
Uses native `runJsBundleStart`/`runJsBundleEnd` performance marks to create a `ManualJsParseTime` span parented to `ManualAppStartup`, making pre-JS startup time visible in Sentry traces. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
elirangoshen
force-pushed
the
eliran/sentry-js-parse-time
branch
from
543ade2 to
9726052
Compare
# Conflicts: # src/CONST/index.ts
This was referenced Feb 26, 2026
elirangoshen
changed the title
Author
|
I removed the dependancy of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Adds a Sentry span to track JS bundle parse time on Android HybridApp.
Start:
getBundleStartTimestampMs()fromSentry'stracing utils. This is a Sentry-internal function that already knows how to read the RN native global set before bundle execution — including handling the twopossible formats (epoch ms vs process-relative ms) across different RN versions. No declaration needed on our side.
End:
Date.now()captured at module load time.The result is a
SPAN_JS_PARSE_TIMEspan with accurate start and approximate end, with no native module dependency — only Sentry and standard RN globals.Fixed Issues
$Expensify#82975
Tests
Offline tests
N/A — telemetry only, no functional change.
QA Steps
Verify that no errors appear in the JS console
Install the HybridApp build
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Summary by CodeRabbit