[CP Staging] Add Scan flow Camera init telemetry#83275
Conversation
Extends SPAN_OPEN_CREATE_EXPENSE to include camera initialization time by deferring the span end until the native camera is ready. Adds a new child span SPAN_CAMERA_INIT for granular tracking.
Web doesn't need camera init tracking - the file picker loads instantly on desktop, and mobile web camera telemetry isn't a priority.
Tracks time from tapping the camera shutter to when the confirmation page renders, complementing existing camera init and submit expense spans.
Fixes a race condition where onInitialized could fire before the span was started, and adds missing early return when camera.current is null to prevent continuing photo capture flow.
marcaaron
changed the title
melvin-bot
bot
requested review from
trjExpensify
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a431adc7b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (transactionRequestType === CONST.IOU.REQUEST_TYPE.SCAN) { | ||
| return; |
There was a problem hiding this comment.
IOURequestStartPage now skips endSpan(SPAN_OPEN_CREATE_EXPENSE) for every scan request type, but only the native scan screen ends that span on camera init. I checked the web scan implementation (src/pages/iou/request/step/IOURequestStepScan/index.tsx) and it has no corresponding endSpan/cancelSpan for SPAN_OPEN_CREATE_EXPENSE, so web scan sessions will keep this span open until some later unrelated end/cancel path, inflating or dropping create-expense timing telemetry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
App/src/pages/iou/request/step/IOURequestStepScan/index.tsx
Lines 83 to 86 in 52386bd
Cancel SPAN_OPEN_CREATE_EXPENSE when permission is DENIED (not just BLOCKED/UNAVAILABLE), and always cancel the span on unmount if camera never initialized.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
|
||
| // Wraps a camera that will only be active when the tab is focused or as soon as it starts to become focused. | ||
| function Camera({cameraTabIndex, ref, forceInactive = false, ...props}: NavigationAwareCameraNativeProps) { | ||
| function Camera({cameraTabIndex, ref, forceInactive = false, onInitialized, ...props}: NavigationAwareCameraNativeProps) { |
There was a problem hiding this comment.
onInitialized is already part of CameraProps, and NavigationAwareCameraNativeProps is Omit<CameraProps, 'isActive'> & {...} , so it's already included in the ...props spread.
| }) | ||
| .catch((error: string) => { | ||
| setDidCapturePhoto(false); | ||
| if (!isMultiScanEnabled) { |
There was a problem hiding this comment.
This pattern is repeated 5 times, we could extract a small helper to reduce the repetition:
const cancelShutterSpan = useCallback(() => {
if (!isMultiScanEnabled) {
cancelSpan(CONST.TELEMETRY.SPAN_SHUTTER_TO_CONFIRMATION);
}
}, [isMultiScanEnabled]);
| } | ||
| endSpan(CONST.TELEMETRY.SPAN_OPEN_CREATE_EXPENSE); | ||
| }, []); | ||
| }, [transactionRequestType]); |
There was a problem hiding this comment.
Changing the dep array from []to [transactionRequestType] means this effect re-runs when the user switches tabs . Maybe we should keep [] with an eslint-disable?
There was a problem hiding this comment.
Oh right, startMoneyRequest() does not happen when we switch tabs. 👍
marcaaron
changed the title
| return; | ||
| } | ||
| startSpan(CONST.TELEMETRY.SPAN_CAMERA_INIT, { | ||
| name: 'camera-init', |
There was a problem hiding this comment.
use const
| name: 'camera-init', | |
| name: CONST.TELEMETRY.SPAN_CAMERA_INIT, |
| if (cameraInitSpanStarted.current || cameraPermissionStatus !== RESULTS.GRANTED || device == null) { | ||
| return; | ||
| } | ||
| startSpan(CONST.TELEMETRY.SPAN_CAMERA_INIT, { |
There was a problem hiding this comment.
NAB A minor missing thing is tracking the speed of getting cameraPermissionStatus. But there's probably not much gain to be had there and it might be tricky to exclude tracking spans where user needs to approve permission.
There was a problem hiding this comment.
Nevermind, i see you cancelling these spans. In that case would it be possible to start tracking earlier to include the permission check time?
There was a problem hiding this comment.
Yeah I think that might be something good to follow up on. My goal with this PR is to hopefully give a sketch around some of the native time that's showing up as "missing instrumentation" in sampled profiles. If we see that the camera readiness is super fast then maybe we look at permissions next?
There was a problem hiding this comment.
In that case would it be possible to start tracking earlier to include the permission check time?
I kind of don't want to do this because it's user dependent. I'm trying to instrument a specific flow which is:
- user already gave us permissions
- camera inits
- receipt is scanned
I would think that if they haven't given us permission that metric will be "however long it takes them to approve or deny".
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
PR doesn’t need product input as a performance metric related PR. Unassigning and unsubscribing myself. |
|
🚧 @marcaaron 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/marcaaron in version: 9.3.26-0 🚀
|
|
Hi @marcaaron Any QA steps for this? Thanks |
|
@izarutskaya Nope! |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.26-8 🚀
|
7 participants
Adds telemetry to help us understand performance of the native scan flow for creating expenses.
What we're tracking
Camera initialization - How long it takes from opening the scan screen until the camera is actually ready to capture. On native, we defer ending the "open create expense" span until the camera finishes initializing so we capture the full user-perceived load time.
Shutter to confirmation - For single-scan flows, tracks how long it takes from tapping the shutter button to reaching the confirmation screen.
Fixed Issues
$ #81849
Tests
Offline tests
N/A - telemetry only
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.