[NO QA] Filter high-volume Sentry spans: browser resources, Google Ads/Analytics, and Ping - #101464
Conversation
…nalytics requests, and the Ping heartbeat Co-authored-by: Rodrigo Lino da Costa <rlinoz@users.noreply.github.com>
… resource.script spans Co-authored-by: Rodrigo Lino da Costa <rlinoz@users.noreply.github.com>
|
The failing What's failing
Root cause Two Already being fixed Fix typecheck: pass movedFromReportName in ModifiedExpenseMessageTest (draft, by Evidence it's repo-wide, not this PR
What you need to do: nothing on this branch. Wait for #101467, then re-run |
|
Status update — no change on this branch, so still nothing for me to push. Details are in my earlier comment: the failing What moved since then: [NoQA] Fix typecheck: pass movedFromReportName in ModifiedExpenseMessageTest is now out of draft and its own I re-verified that Every other check on this PR passes. I'm deliberately not duplicating the 2-line fix onto this branch — it would conflict with #101467 and leave |
|
🤖 Web test steps verification results: Recovered sign-in by pressing Enter in the email field instead of clicking Continue, reaching the signed-in Home/LHN state with no error screen or blank page (step 1 passed). Steps 2-4 could not be completed because this browser session's accessibility bridge reported zero elements with resolvable bounds (
Two things worth knowing: sign-in was handed over already broken (CI's prestart failed at the Continue step), and clicking never worked in this session at all — keyboard-driven form submission was the only input that landed. So the documented console check is unverified on web rather than failing. |
…rting @sentry/react @sentry/react is a transitive dependency of @sentry/react-native, so knip reports it as unlisted when a file imports it by name. The dynamic import in the test added a new knip finding; reading the mock from a hoisted variable keeps the assertion and drops the import. Co-authored-by: Rodrigo Lino da Costa <rlinoz@users.noreply.github.com>
|
Pushed a fix for the one check that was actually this PR's fault. The two still red are both breaks on Fixed and pushed — My new test did Still red — nothing I can fix from this branch
Both are semantic merge conflicts — each PR was green against the
Evidence both are repo-wide, not this PRThis PR touches only
AI tests run locally on the fix
What's left for you: nothing on this branch for |
…anVolume Co-authored-by: Rodrigo Lino da Costa <rlinoz@users.noreply.github.com>
|
Fixed — merged What was failing
Both were semantic merge conflicts on Evidence and verificationThe failures were repo-wide, not this PR. This PR only touches Merge, not rebase — original commits and authorship are intact; merge commit is Run locally before pushing:
CI after the push: typecheck, ESLint, oxfmt, spellcheck, Jest (8 shards), Bun, Storybook, React Compiler, knip, unused styles, and both Android/iOS debug builds all pass. The PR body's |
|
Resolved — nothing left for me to fix here. [NoQA] Fix typecheck: pass movedFromReportName in ModifiedExpenseMessageTest merged at 21:13 UTC, and The one remaining red mark is The PR body's |
Co-authored-by: Rodrigo Lino da Costa <rlinoz@users.noreply.github.com>
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
Applied all three of your suggestions verbatim and pushed as
The filter list now reads uniformly — every phrase bare, matching the seven that were already there — and the AI tests run locally on this change
The PR body's |
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
@ahmedGaber93 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
🚧 rlinoz has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.4.90-0 🚀
|
|
🤖 No help site changes are required for this PR. This PR only changes internal Sentry tracing configuration — which spans the SDK creates for browser resources, Google Ads/Analytics requests, and What I checkedFiles changed:
Help site search: A case-insensitive search for Not a docs-worthy change under Because no changes are required, I did not create a draft help site PR. @rlinoz, please confirm you agree that no help site updates are needed here. If you think a specific article is affected, reply with |






Explanation of Change
A Sentry health check found that a handful of auto-instrumented span sources dominated our tracing volume (~820M spans over 30 days) while offering little debugging value. This PR filters three of them, which map to items 1, 3 and 4 of the linked issue. Item 2 (
OnyxDerivedCompute) is deliberately out of scope — it is our own instrumentation and needs a duration threshold rather than a filter, so it is better handled in its own PR.browserTracingIntegrationnow passesignoreResourceSpans: ['resource.link', 'resource.script'], so the SDK never creates a span per stylesheet/font or per script on pageload and navigation. Web vitals (LCP, FCP, CLS, INP) are unaffected because they are measurements on the pageload span rather than resource spans. The trade-off worth knowing: these spans are what feeds Sentry's Insights → Assets module for CSS/JS, so that module loses data for those two types.ccm/collect,rmkt/collect,pagead/form-dataandccm/form-dataare added to the existingshouldCreateSpanForRequestfilter. These are third-party requests we cannot act on. Trace propagation is left alone, since scopingtracePropagationTargetsis a separate, riskier change that affects headers sent to our own API./api/Pingis added to the same filter. NetInfo polls this URL on a timer for every client, so it is pure heartbeat volume — the same reason/api/Logis already filtered.All three cut spans at creation time rather than dropping them in
beforeSendTransaction, so there is no client-side work and no payload for the filtered requests.Fixed Issues
$ #101449
PROPOSAL: #101449 (comment)
Tests
Offline tests
QA Steps
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