Skip to content

Commit 330ae99

Browse files
OlimpiaZurekOlimpiaZurek
committed
address cr comments
1 parent 4cacb3d commit 330ae99

7 files changed

Lines changed: 169 additions & 92 deletions

File tree

‎src/libs/API/index.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import {resolveDuplicationConflictAction, resolveEnableFeatureConflicts} from '@
55
import type {AnyRequestMatcher, EnablePolicyFeatureCommand} from '@libs/actions/RequestConflictUtils';
66
import Log from '@libs/Log';
77
import {handleDeletedAccount, HandleUnusedOptimisticID, Logging, Pagination, Reauthentication, RecheckConnection, SaveResponseInOnyx, SupportalPermission} from '@libs/Middleware';
8-
import ExpenseServerTiming from '@libs/Middleware/ExpenseServerTiming';
98
import FraudMonitoring from '@libs/Middleware/FraudMonitoring';
9+
import SentryServerTiming from '@libs/Middleware/SentryServerTiming';
1010
import {isOffline} from '@libs/Network/NetworkStore';
1111
import {push as pushToSequentialQueue, waitForIdle as waitForSequentialQueueIdle} from '@libs/Network/SequentialQueue';
1212
import Pusher from '@libs/Pusher';
@@ -43,8 +43,8 @@ addMiddleware(HandleUnusedOptimisticID);
4343

4444
addMiddleware(Pagination);
4545

46-
// ExpenseServerTiming - Tracks server round-trip time for expense creation commands via Sentry spans.
47-
addMiddleware(ExpenseServerTiming);
46+
// SentryServerTiming - Tracks server round-trip time for configured command groups via Sentry spans.
47+
addMiddleware(SentryServerTiming);
4848

4949
// SaveResponseInOnyx - Merges either the successData or failureData (or finallyData, if included in place of the former two values) into Onyx depending on if the call was successful or not. This needs to be the LAST middleware we use, don't add any
5050
// middlewares after this, because the SequentialQueue depends on the result of this middleware to pause the queue (if needed) to bring the app to an up-to-date state.

‎src/libs/Middleware/ExpenseServerTiming.ts‎

Lines changed: 0 additions & 54 deletions
This file was deleted.

‎src/libs/Middleware/SentryServerTiming.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import {WRITE_COMMANDS} from '@libs/API/types';
2+
import {cancelSpan, endSpan, getSpan, startSpan} from '@libs/telemetry/activeSpans';
3+
import CONST from '@src/CONST';
4+
import type Middleware from './types';
5+
6+
/**
7+
* Configuration for a tracked command group.
8+
* Maps a set of write commands to a Sentry span operation name.
9+
*/
10+
type TrackedCommandGroup = {
11+
/** Set of write commands that should be instrumented */
12+
commands: Set<string>;
13+
/** Sentry span operation name */
14+
spanOp: string;
15+
/** Human-readable span name */
16+
spanName: string;
17+
};
18+
19+
/**
20+
* Registry of command groups to instrument with server round-trip timing.
21+
* Add new entries here to track additional flows without creating new middlewares.
22+
*/
23+
const TRACKED_COMMAND_GROUPS: TrackedCommandGroup[] = [
24+
{
25+
commands: new Set<string>([
26+
WRITE_COMMANDS.REQUEST_MONEY,
27+
WRITE_COMMANDS.CREATE_PER_DIEM_REQUEST,
28+
WRITE_COMMANDS.SPLIT_BILL,
29+
WRITE_COMMANDS.SPLIT_BILL_AND_OPEN_REPORT,
30+
WRITE_COMMANDS.START_SPLIT_BILL,
31+
WRITE_COMMANDS.CREATE_DISTANCE_REQUEST,
32+
WRITE_COMMANDS.TRACK_EXPENSE,
33+
WRITE_COMMANDS.SEND_INVOICE,
34+
]),
35+
spanOp: CONST.TELEMETRY.SPAN_EXPENSE_SERVER_RESPONSE,
36+
spanName: 'expense-server-response',
37+
},
38+
];
39+
40+
/**
41+
* Finds the tracked command group for a given command, if any.
42+
*/
43+
function findTrackedGroup(command: string): TrackedCommandGroup | undefined {
44+
return TRACKED_COMMAND_GROUPS.find((group) => group.commands.has(command));
45+
}
46+
47+
/**
48+
* Middleware that tracks server round-trip time for configured command groups via Sentry spans.
49+
* For non-tracked commands, this is a no-op pass-through.
50+
*
51+
* To add tracking for a new flow, add an entry to TRACKED_COMMAND_GROUPS above.
52+
*/
53+
const SentryServerTiming: Middleware = (response, request) => {
54+
const group = findTrackedGroup(request.command);
55+
if (!group) {
56+
return response;
57+
}
58+
59+
const spanId = `${group.spanOp}_${request.requestID}`;
60+
startSpan(spanId, {
61+
name: group.spanName,
62+
op: group.spanOp,
63+
attributes: {
64+
[CONST.TELEMETRY.ATTRIBUTE_COMMAND]: request.command,
65+
},
66+
});
67+
68+
return response
69+
.then((data) => {
70+
const span = getSpan(spanId);
71+
span?.setAttributes({
72+
[CONST.TELEMETRY.ATTRIBUTE_JSON_CODE]: data?.jsonCode,
73+
});
74+
endSpan(spanId);
75+
return data;
76+
})
77+
.catch((error) => {
78+
cancelSpan(spanId);
79+
throw error;
80+
});
81+
};
82+
83+
export default SentryServerTiming;

‎src/libs/telemetry/getSubmitExpenseScenario.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import type {IOUType} from '@src/CONST';
2+
import CONST from '@src/CONST';
3+
4+
type GetSubmitExpenseScenarioParams = {
5+
iouType: IOUType;
6+
isDistanceRequest: boolean;
7+
isMovingTransactionFromTrackExpense: boolean;
8+
isUnreported: boolean;
9+
isCategorizingTrackExpense: boolean;
10+
isSharingTrackExpense: boolean;
11+
isPerDiemRequest: boolean;
12+
isFromGlobalCreate: boolean;
13+
hasReceiptFiles: boolean;
14+
};
15+
16+
/**
17+
* Determines the telemetry scenario string for a submit expense action.
18+
*/
19+
function getSubmitExpenseScenario({
20+
iouType,
21+
isDistanceRequest,
22+
isMovingTransactionFromTrackExpense,
23+
isUnreported,
24+
isCategorizingTrackExpense,
25+
isSharingTrackExpense,
26+
isPerDiemRequest,
27+
isFromGlobalCreate,
28+
hasReceiptFiles,
29+
}: GetSubmitExpenseScenarioParams): string {
30+
const {SUBMIT_EXPENSE_SCENARIO} = CONST.TELEMETRY;
31+
32+
if (iouType !== CONST.IOU.TYPE.TRACK && isDistanceRequest && !isMovingTransactionFromTrackExpense && !isUnreported) {
33+
return SUBMIT_EXPENSE_SCENARIO.DISTANCE;
34+
}
35+
if (iouType === CONST.IOU.TYPE.SPLIT) {
36+
if (hasReceiptFiles) {
37+
return SUBMIT_EXPENSE_SCENARIO.SPLIT_RECEIPT;
38+
}
39+
if (isFromGlobalCreate) {
40+
return SUBMIT_EXPENSE_SCENARIO.SPLIT_GLOBAL;
41+
}
42+
return SUBMIT_EXPENSE_SCENARIO.SPLIT;
43+
}
44+
if (iouType === CONST.IOU.TYPE.INVOICE) {
45+
return SUBMIT_EXPENSE_SCENARIO.INVOICE;
46+
}
47+
if (iouType === CONST.IOU.TYPE.TRACK || isCategorizingTrackExpense || isSharingTrackExpense) {
48+
return SUBMIT_EXPENSE_SCENARIO.TRACK_EXPENSE;
49+
}
50+
if (isPerDiemRequest) {
51+
return SUBMIT_EXPENSE_SCENARIO.PER_DIEM;
52+
}
53+
if (hasReceiptFiles) {
54+
return SUBMIT_EXPENSE_SCENARIO.REQUEST_MONEY_SCAN;
55+
}
56+
57+
return SUBMIT_EXPENSE_SCENARIO.REQUEST_MONEY_MANUAL;
58+
}
59+
60+
export default getSubmitExpenseScenario;
61+
export type {GetSubmitExpenseScenarioParams};

‎src/libs/telemetry/markNavigateAfterExpenseCreateEnd.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import Performance from '@libs/Performance';
22
import CONST from '@src/CONST';
3-
import {endSpan} from './activeSpans';
3+
import {endSpan, getSpan} from './activeSpans';
44

55
/**
66
* Mark the post-submit navigation telemetry span as finished.
77
*/
88
function markNavigateAfterExpenseCreateEnd() {
9+
if (!getSpan(CONST.TELEMETRY.SPAN_NAVIGATE_AFTER_EXPENSE_CREATE)) {
10+
return;
11+
}
912
endSpan(CONST.TELEMETRY.SPAN_NAVIGATE_AFTER_EXPENSE_CREATE);
1013
Performance.markEnd(CONST.TIMING.NAVIGATE_AFTER_EXPENSE_CREATE);
1114
}

‎src/libs/telemetry/markSubmitExpenseEnd.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
import Performance from '@libs/Performance';
22
import CONST from '@src/CONST';
3-
import {endSpan} from './activeSpans';
3+
import {endSpan, getSpan} from './activeSpans';
44

55
/**
66
* Mark the submit expense telemetry span as finished.
77
* Called after all API.write() calls for the expense have been dispatched.
88
*/
99
function markSubmitExpenseEnd() {
10+
if (!getSpan(CONST.TELEMETRY.SPAN_SUBMIT_EXPENSE)) {
11+
return;
12+
}
1013
endSpan(CONST.TELEMETRY.SPAN_SUBMIT_EXPENSE);
1114
Performance.markEnd(CONST.TIMING.SUBMIT_EXPENSE);
1215
}

‎src/pages/iou/request/step/IOURequestStepConfirmation.tsx‎

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ import {
6161
isReportOutstanding,
6262
isSelectedManagerMcTest,
6363
} from '@libs/ReportUtils';
64-
import {cancelSpan, endSpan, startSpan} from '@libs/telemetry/activeSpans';
64+
import {endSpan, startSpan} from '@libs/telemetry/activeSpans';
65+
import getSubmitExpenseScenario from '@libs/telemetry/getSubmitExpenseScenario';
6566
import markSubmitExpenseEnd from '@libs/telemetry/markSubmitExpenseEnd';
6667
import {
6768
getAttendees,
@@ -326,16 +327,6 @@ function IOURequestStepConfirmation({
326327
Performance.markEnd(CONST.TIMING.OPEN_CREATE_EXPENSE_APPROVE);
327328
}, []);
328329

329-
useEffect(() => {
330-
return () => {
331-
// Cancel submit expense span if user abandons the confirmation page without submitting
332-
if (formHasBeenSubmitted.current) {
333-
return;
334-
}
335-
cancelSpan(CONST.TELEMETRY.SPAN_SUBMIT_EXPENSE);
336-
};
337-
}, []);
338-
339330
useEffect(() => {
340331
if (!isCreatingTrackExpense || policyID === undefined) {
341332
return;
@@ -988,31 +979,20 @@ function IOURequestStepConfirmation({
988979

989980
formHasBeenSubmitted.current = true;
990981

991-
// Determine scenario for telemetry before branching
992982
const hasReceiptFiles = Object.values(receiptFiles).some((receipt) => !!receipt);
993983
const isFromGlobalCreate = transaction?.isFromGlobalCreate ?? transaction?.isFromFloatingActionButton ?? false;
994984

995-
const {SUBMIT_EXPENSE_SCENARIO} = CONST.TELEMETRY;
996-
let scenario: string = SUBMIT_EXPENSE_SCENARIO.REQUEST_MONEY_MANUAL;
997-
if (iouType !== CONST.IOU.TYPE.TRACK && isDistanceRequest && !isMovingTransactionFromTrackExpense && !isUnreported) {
998-
scenario = SUBMIT_EXPENSE_SCENARIO.DISTANCE;
999-
} else if (iouType === CONST.IOU.TYPE.SPLIT) {
1000-
if (hasReceiptFiles) {
1001-
scenario = SUBMIT_EXPENSE_SCENARIO.SPLIT_RECEIPT;
1002-
} else if (isFromGlobalCreate) {
1003-
scenario = SUBMIT_EXPENSE_SCENARIO.SPLIT_GLOBAL;
1004-
} else {
1005-
scenario = SUBMIT_EXPENSE_SCENARIO.SPLIT;
1006-
}
1007-
} else if (iouType === CONST.IOU.TYPE.INVOICE) {
1008-
scenario = SUBMIT_EXPENSE_SCENARIO.INVOICE;
1009-
} else if (iouType === CONST.IOU.TYPE.TRACK || isCategorizingTrackExpense || isSharingTrackExpense) {
1010-
scenario = SUBMIT_EXPENSE_SCENARIO.TRACK_EXPENSE;
1011-
} else if (isPerDiemRequest) {
1012-
scenario = SUBMIT_EXPENSE_SCENARIO.PER_DIEM;
1013-
} else if (hasReceiptFiles) {
1014-
scenario = SUBMIT_EXPENSE_SCENARIO.REQUEST_MONEY_SCAN;
1015-
}
985+
const scenario = getSubmitExpenseScenario({
986+
iouType,
987+
isDistanceRequest,
988+
isMovingTransactionFromTrackExpense,
989+
isUnreported,
990+
isCategorizingTrackExpense,
991+
isSharingTrackExpense,
992+
isPerDiemRequest,
993+
isFromGlobalCreate,
994+
hasReceiptFiles,
995+
});
1016996

1017997
Performance.markStart(CONST.TIMING.SUBMIT_EXPENSE);
1018998
startSpan(CONST.TELEMETRY.SPAN_SUBMIT_EXPENSE, {
@@ -1029,6 +1009,7 @@ function IOURequestStepConfirmation({
10291009

10301010
// IMPORTANT: Every branch below must call markSubmitExpenseEnd() after dispatching the expense action.
10311011
// This ensures the telemetry span started above is always closed, including inside async getCurrentPosition callbacks.
1012+
// If missed, the impact is benign (an orphaned Sentry span), but it pollutes telemetry data.
10321013
if (iouType !== CONST.IOU.TYPE.TRACK && isDistanceRequest && !isMovingTransactionFromTrackExpense && !isUnreported) {
10331014
createDistanceRequest(iouType === CONST.IOU.TYPE.SPLIT ? splitParticipants : selectedParticipants, trimmedComment);
10341015
markSubmitExpenseEnd();

0 commit comments

Comments
 (0)