[No QA][Sentry] Add activeSpans logs #80645
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Waiting for Adam's work to be done and merged. We've already confirmed that adding logs before Onyx is initialized causes a lot of side effects so the best course of action is to wait for Onyx's fix |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddf6fa8276
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Code Review - 2 Issues FoundOverall this is a clean, well-scoped PR. The parameter whitelist and 1.
|
|
Answer to automatic code review: Ad 1.: this is correct. I've removed spanExists |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
@sosek108 This PR not only adds the |
|
@truph01 Yes. We added we disabled default sending console logs to sentry so only our |
| }); | ||
|
|
||
| const consoleIntegration = Sentry.consoleLoggingIntegration({ | ||
| levels: ['error'], |
There was a problem hiding this comment.
Don't we still want to send error logs to Sentry?
There was a problem hiding this comment.
should we filter the onyx ones and still send the others?
|
PR doesn’t need product input as a tooling PR. Unassigning and unsubscribing myself. |
| }); | ||
|
|
||
| const consoleIntegration = Sentry.consoleLoggingIntegration({ | ||
| levels: ['error'], |
There was a problem hiding this comment.
should we filter the onyx ones and still send the others?
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.26-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.26-8 🚀
|

Explanation of Change
This PR adds Sentry logs around
activeSpansfunctions to track Spans that are not correctly collected. (eg. infinite).Fixed Issues
$ #73322
PROPOSAL:
Tests
[Sentry]are presentOffline tests
N/A
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
No QA.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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