Conversation
|
Fixed the failing
The checklist check should pass on the next run. |
This removes the SendPerformanceTiming API command from the App, which was sending performance timing data to Graphite/StatsD via the Web-Expensify backend. Each app version created new metric paths (e.g. global.web.ios.sidebar_loaded.9-0-47-0), leading to unbounded disk growth on the stats server. Firebase performance tracing, console.debug logging, and max execution time warnings are preserved. Co-authored-by: Nathan Metcalf <nathanmetcalf@users.noreply.github.com>
ce10a84 to
f762f5d
Compare
|
Fixed the failing checks by rebasing onto current Root cause: The PR branch was based on an old version of
The PR's own code changes (removing |
|
@shubham1206agra 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] |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-02-10.at.6.30.00.PM.mov |
|
🎯 @shubham1206agra, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #81979. |
mountiny
left a comment
There was a problem hiding this comment.
@adhorodyski @rlinoz do you also want to review?
| delete timestampData[eventName]; | ||
|
|
||
| if (Environment.isDevelopment()) { | ||
| // Don't create traces on dev as this will mess up the accuracy of data in release builds of the app |
There was a problem hiding this comment.
Not sure if we need to remove this comment
There was a problem hiding this comment.
Good catch — restored the original comment. It still applies since the early return prevents max execution time warnings from firing in dev, which could skew debugging. Pushed in 827fd11.
Addresses review feedback from mountiny to keep the comment explaining why we skip performance logging in development. Co-authored-by: Nathan Metcalf <nathanmetcalf@users.noreply.github.com>
|
Hmm we just merged this https://github.com/Expensify/App/pull/81691/changes#diff-59c248214d6ef3a5069053fda015678ec089bf113c50e8add1c75c2688344bd6 that deletes the whole Timing.ts |
|
That explains the Timings @MelvinBot can you sync with main and confirm if any more work is needed? |
|
I synced with Here's the file-by-file status on current
No additional work is needed. This PR can be closed as the work has been superseded by #81691. |
Explanation of Change
Removes the
SendPerformanceTimingAPI command from the App. This command was sending performance timing data to Graphite/StatsD via the Web-Expensify backend on every timed event. Each app version created new metric paths (e.g.global.web.ios.sidebar_loaded.9-0-47-0), leading to unbounded whisper file growth on the stats server disk.What's removed:
SendPerformanceTimingAPI call inTiming.end()(no more graphite stats)SendPerformanceTimingParamstype definition and exportsSEND_PERFORMANCE_TIMINGread command registration/api/SendPerformanceTiming(no longer needed since the request won't be made)What's preserved:
Firebase.startTrace/stopTrace)console.debugtiming outputLog.warnfor max execution time exceededTiming.start()/Timing.end()are unchangedFixed Issues
$ https://github.com/Expensify/Expensify/issues/594718
Tests
Timing:debug messages still appear in the consoleSendPerformanceTimingnetwork requests are madeOffline tests
No offline behavior changes. The
SendPerformanceTimingAPI call was a fire-and-forget read command that had no impact on offline behavior.QA Steps
[No QA] - This is an internal infrastructure change that removes stats reporting to Graphite. No user-facing behavior changes. Firebase performance tracing continues to work.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyx— N/A, no Onyx changestoggleReportand notonIconClick) — N/A, only deletionssrc/languages/*files and using the translation method — N/A, no user-facing copySTYLE.md) were followedStyleUtils.getBackgroundAndBorderStyle(theme.componentBG)) — N/Anpm run compress-svg) — N/AAvataris modified, I verified thatAvataris working as expected in all cases) — N/ADesignlabel and/or tagged@Expensify/designso the design team can review the changes. — N/A, no UI changesScrollViewcomponent to make it scrollable when more elements are added to the page. — N/Amainbranch 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
N/A - No UI changes
Android: mWeb Chrome
N/A - No UI changes
iOS: Native
N/A - No UI changes
iOS: mWeb Safari
N/A - No UI changes
MacOS: Chrome / Safari
N/A - No UI changes