Track expense created events in Sentry per request type#90388
Track expense created events in Sentry per request type#90388Julesssss wants to merge 2 commits into
Conversation
melvin-bot
Bot
requested review from
JmillsExpensify and
NicolasBonet
and removed request for
a team
|
@NicolasBonet 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| endSpan(CONST.TELEMETRY.SPAN_OPEN_CREATE_EXPENSE); | ||
| }, []); | ||
|
|
||
| const handleTabFocused = useCallback((tabName: SelectedTabRequest) => { |
There was a problem hiding this comment.
❌ CLEAN-REACT-PATTERNS-0 (docs)
React Compiler is enabled and this file compiles successfully. The useCallback wrapping handleTabFocused is redundant because the compiler automatically memoizes closures based on their captured variables. Manual memoization adds maintenance overhead (dependency arrays) with no benefit.
Remove useCallback and define handleTabFocused as a plain function:
const handleTabFocused = (tabName: SelectedTabRequest) => {
Sentry.startInactiveSpan({
name: `${SCREENS.MONEY_REQUEST.DISTANCE_CREATE}.tab.${tabName}`,
op: 'ui.tab.focus',
forceTransaction: true,
})?.end();
};Reviewed at: 1de2933 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| } | ||
| Sentry.startInactiveSpan({ | ||
| name: `ExpenseCreated.${iouRequestType}`, | ||
| op: 'expense.created', |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The strings 'expense.created' (op) and the 'ExpenseCreated.' prefix (name) are magic strings. The existing telemetry codebase consistently defines span identifiers as named constants in CONST.TELEMETRY (e.g., CONST.TELEMETRY.SPAN_SUBMIT_EXPENSE, CONST.TELEMETRY.SPAN_APP_STARTUP). Using inline strings here diverges from that established pattern and makes it harder to find all telemetry span definitions in one place.
Define these as constants in CONST.TELEMETRY and reference them here:
// In CONST.ts under TELEMETRY
SPAN_EXPENSE_CREATED: 'ExpenseCreated',
OP_EXPENSE_CREATED: 'expense.created',Reviewed at: 1de2933 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| const handleTabFocused = useCallback((tabName: SelectedTabRequest) => { | ||
| Sentry.startInactiveSpan({ | ||
| name: `${SCREENS.MONEY_REQUEST.DISTANCE_CREATE}.tab.${tabName}`, | ||
| op: 'ui.tab.focus', |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The string 'ui.tab.focus' is a magic string used as a Sentry span operation name. The rest of the telemetry codebase defines span identifiers as named constants in CONST.TELEMETRY. Using an inline string here makes it harder to discover and maintain telemetry instrumentation points.
Define this as a constant in CONST.TELEMETRY and reference it here:
// In CONST.ts under TELEMETRY
OP_TAB_FOCUS: 'ui.tab.focus',Reviewed at: 1de2933 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
1 participant
Explanation of Change
Adds a per-request-type Sentry transaction whenever a distance expense is created, so we can answer "how many users used the odometer flow / each distance mode" without relying on screen-load proxies. Today, switching distance tabs inside
Money_Request_Distance_Createproduces no Sentry transaction (TopTab transitions aren't tracked byreactNavigationIntegration), and the deep-linkMoney_Request_Step_Distance_*routes only fire when reached from edit/split/receipt flows — neither captures the in-create-flow creation event.A new leaf helper
src/libs/telemetry/emitExpenseCreated.tsemits a forced Sentry transaction namedExpenseCreated.${iouRequestType}with opexpense.created. It's called from the non-split branch ofcreateDistanceRequestinsrc/libs/actions/IOU/Split.ts, right after the optimistic transaction is built. The split branch is structurally outside the call site, so split flows are not tracked.Once deployed, queries like the following give per-mode unique-user counts:
Per-mode names emerge automatically from
transaction.iouRequestType:distance-map,distance-manual,distance-gps,distance-odometer.The helper is intentionally generic — extending to per-diem / track / scan / manual single-expense flows in the future is one line at each
API.write(WRITE_COMMANDS.CREATE_*)site.Fixed Issues
$ N/A — telemetry follow-up to #87726
PROPOSAL: N/A
Tests
sentry.io), verify a transaction is emitted with nameExpenseCreated.distance-odometerand opexpense.created.ExpenseCreated.distance-*transaction.ExpenseCreated.*transaction is emitted (split branch intentionally skipped).Offline tests
Same as Tests. The emit fires before
deferOrExecuteWritequeues the API call, so transactions are recorded for offline creates as well. The Sentry transport will buffer envelopes until network returns.QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection above (N/A — pure telemetry)TestssectionOffline stepssectionQA stepssectionSentry.startInactiveSpanpattern used insrc/libs/OptionsListUtils/index.ts)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps