[No QA][Sentry] activeSpans logs are displayed locally only#85353
[No QA][Sentry] activeSpans logs are displayed locally only#85353rlinoz merged 2 commits intoExpensify:mainfrom
Conversation
|
@mkhutornyi 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] |
|
@rlinoz I created new branch as previous had wrong base branch. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb Safari |
| * Ends a span only if it's currently active. Unlike `endSpan`, this silently no-ops | ||
| * when the span doesn't exist, making it safe for render paths where the span | ||
| * may or may not have been started. |
There was a problem hiding this comment.
NAB: Is this no longer needed? I think was good for optimization.
There was a problem hiding this comment.
Yeah @TMisiukiewicz added it very recently, should we keep it?
There was a problem hiding this comment.
It was added so we don't send too much of trying to end nonexistent span logs to /api/Log. Now when we don't send these logs at all this is unnecessary and only duplicates the code.
🤖 Code ReviewSummary: This PR replaces Changes Review
CI StatusAll checks pass (typecheck, ESLint, tests, perf-tests, etc.). VerdictClean, minimal change that achieves exactly what the linked issue asks for. No concerns. ✅ |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #84761 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
PR doesn’t need product input as a perf tooling PR. Unassigning and unsubscribing myself. |
|
🚧 @rlinoz 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/rlinoz in version: 9.3.39-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.39-3 🚀
|

Explanation of Change
This PR removes sending Sentry logs from
activeSpansto/api/Log. Only local debug logs will be displayed for debug purposes.Fixed Issues
$ #84761
PROPOSAL:
Tests
[Sentry]logs are displayed on the logs.expensify.com[Sentry]logs are displayed in Developer Tools consoleOffline tests
N/A
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
N/A
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