Skip to content

Perf: Add diagnostic sub-spans to ManualOpenSearchRouter #83076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mountiny wants to merge 12 commits into
base: main
Choose a base branch
from
+79 −6
Open

Perf: Add diagnostic sub-spans to ManualOpenSearchRouter #83076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9a89d49
Perf: Add diagnostic sub-spans to ManualOpenSearchRouter
mountiny Feb 23, 2026
ef575b5
Merge remote-tracking branch 'origin/main' into vit-searchRouter-diag…
mountiny Feb 24, 2026
5f99d51
Link sub-spans as children of SPAN_OPEN_SEARCH_ROUTER via parentSpan
mountiny Feb 24, 2026
b1112c6
Merge branch 'main' into vit-searchRouter-diagnosticSubSpans
mountiny Feb 25, 2026
ff0acd5
Fix cold_start attribute timing and add span cleanup on unmount
mountiny Feb 25, 2026
8b8ae8d
Merge branch 'main' into vit-searchRouter-diagnosticSubSpans
mountiny Feb 26, 2026
a3d27e3
Fix compute span overlap and cold_start attribute accuracy
mountiny Feb 26, 2026
4016f14
Use CONST attribute names for cold_start and trigger
mountiny Feb 26, 2026
08ed61f
Gate compute span to non-loading render pass
mountiny Mar 2, 2026
2444370
Merge remote-tracking branch 'origin/main' into vit-searchRouter-diag…
mountiny Mar 2, 2026
61bb27a
Fix OptionsListContext rename after main merge
mountiny Mar 2, 2026
7fe536c
Merge remote-tracking branch 'origin/main' into vit-searchRouter-diag…
mountiny Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/CONST/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1790,6 +1790,10 @@ const CONST = {
SPAN_OD_ND_TRANSITION: 'ManualOdNdTransition',
SPAN_OD_ND_TRANSITION_LOGGED_OUT: 'ManualOdNdTransitionLoggedOut',
SPAN_OPEN_SEARCH_ROUTER: 'ManualOpenSearchRouter',
SPAN_SEARCH_ROUTER_MODAL_CLOSE_WAIT: 'SearchRouter.ModalCloseWait',
SPAN_SEARCH_ROUTER_OPTIONS_INIT: 'SearchRouter.OptionsInit',
SPAN_SEARCH_ROUTER_COMPUTE_OPTIONS: 'SearchRouter.ComputeOptions',
SPAN_SEARCH_ROUTER_LIST_RENDER: 'SearchRouter.ListRender',
Comment on lines +1793 to +1796
Copy link
Contributor

@JakubKorytko JakubKorytko Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are dots in values here?

Copy link
Contributor Author

@mountiny mountiny Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dots are intentional to visually distinguish these as diagnostic sub-spans from the top-level parent spans in Sentry's trace view. The existing parent spans use flat PascalCase (ManualOpenReport, ManualOpenSearchRouter, etc.) while these child spans use the SearchRouter. prefix to make it immediately clear they belong to the SearchRouter flow when browsing traces. It's a common Sentry convention for hierarchical span naming. Happy to change to flat PascalCase (e.g. SearchRouterModalCloseWait) if you feel consistency with the parent span naming is more important.

SPAN_OPEN_CREATE_EXPENSE: 'ManualOpenCreateExpense',
SPAN_CAMERA_INIT: 'ManualCameraInit',
SPAN_SHUTTER_TO_CONFIRMATION: 'ManualShutterToConfirmation',
Expand Down Expand Up @@ -1847,6 +1851,8 @@ const CONST = {
ATTRIBUTE_IS_FROM_GLOBAL_CREATE: 'is_from_global_create',
ATTRIBUTE_COMMAND: 'command',
ATTRIBUTE_JSON_CODE: 'json_code',
ATTRIBUTE_COLD_START: 'cold_start',
ATTRIBUTE_TRIGGER: 'trigger',
ATTRIBUTE_PLATFORM: 'platform',
ATTRIBUTE_IS_MULTI_SCAN: 'is_multi_scan',
SUBMIT_EXPENSE_SCENARIO: {
Expand Down
13 changes: 13 additions & 0 deletions src/components/OptionListContextProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import usePrivateIsArchivedMap from '@hooks/usePrivateIsArchivedMap';
import {createOptionFromReport, createOptionList, processReport, shallowOptionsListCompare} from '@libs/OptionsListUtils';
import type {OptionList, SearchOption} from '@libs/OptionsListUtils';
import {isSelfDM} from '@libs/ReportUtils';
import {endSpan, getSpan, startSpan} from '@libs/telemetry/activeSpans';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetails, Report} from '@src/types/onyx';
import {usePersonalDetails} from './OnyxListItemProvider';
Expand Down Expand Up @@ -267,8 +269,19 @@ function OptionsListContextProvider({children}: OptionsListProviderProps) {
}, [personalDetails]);

const initializeOptions = useCallback(() => {
const isSearchRouterSpanActive = !!getSpan(CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER);
if (isSearchRouterSpanActive) {
startSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_OPTIONS_INIT, {
name: CONST.TELEMETRY.SPAN_SEARCH_ROUTER_OPTIONS_INIT,
op: 'function',
parentSpan: getSpan(CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER),
});
}
loadOptions();
areOptionsInitialized.current = true;
if (isSearchRouterSpanActive) {
endSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_OPTIONS_INIT);
}
}, [loadOptions]);

const resetOptions = useCallback(() => {
Expand Down
53 changes: 49 additions & 4 deletions src/components/Search/SearchAutocompleteList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type {ForwardedRef, RefObject} from 'react';
import React, {useEffect, useRef, useState} from 'react';
import React, {useContext, useEffect, useRef, useState} from 'react';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import {useOptionsList} from '@components/OptionListContextProvider';
import {OptionsListStateContext, useOptionsList} from '@components/OptionListContextProvider';
import OptionsListSkeletonView from '@components/OptionsListSkeletonView';
import type {AnimatedTextInputRef} from '@components/RNTextInput';
import type {ListItem as NewListItem, UserListItemProps} from '@components/SelectionList/ListItem/types';
Expand Down Expand Up @@ -30,7 +30,7 @@
import {getReportOrDraftReport} from '@libs/ReportUtils';
import {buildSearchQueryJSON, buildUserReadableQueryString, getQueryWithoutFilters, shouldHighlight} from '@libs/SearchQueryUtils';
import StringUtils from '@libs/StringUtils';
import {endSpan} from '@libs/telemetry/activeSpans';
import {cancelSpan, endSpan, getSpan, startSpan} from '@libs/telemetry/activeSpans';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {CardFeeds, CardList, PersonalDetailsList, Policy, Report} from '@src/types/onyx';
Expand Down Expand Up @@ -163,6 +163,40 @@
const taxRates = getAllTaxRates(policies);

const {options, areOptionsInitialized} = useOptionsList();

const computeSpanStarted = useRef(false);
const spanHandoffDone = useRef(false);
const isRecentSearchesDataLoaded = !isLoadingOnyxValue(recentSearchesMetadata);
// eslint-disable-next-line react-hooks/refs -- intentional: telemetry span must start during render to measure computation time
if (!computeSpanStarted.current && areOptionsInitialized && isRecentSearchesDataLoaded && getSpan(CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER)) {
startSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_COMPUTE_OPTIONS, {
name: CONST.TELEMETRY.SPAN_SEARCH_ROUTER_COMPUTE_OPTIONS,
op: 'function',
parentSpan: getSpan(CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER),
});
computeSpanStarted.current = true;
}

useEffect(() => {
return () => {
cancelSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_COMPUTE_OPTIONS);
cancelSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_LIST_RENDER);
};
}, []);

const {areOptionsInitialized: contextAreOptionsInitialized} = useContext(OptionsListStateContext);
const coldStartAttributeSet = useRef(false);
useEffect(() => {
if (coldStartAttributeSet.current) {
return;
}
const parentSpan = getSpan(CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER);
if (parentSpan) {
parentSpan.setAttribute(CONST.TELEMETRY.ATTRIBUTE_COLD_START, !contextAreOptionsInitialized);
coldStartAttributeSet.current = true;
}
}, [contextAreOptionsInitialized]);

const searchOptions = (() => {
if (!areOptionsInitialized) {
return defaultListOptions;
Expand Down Expand Up @@ -321,7 +355,7 @@
}, [autocompleteQueryWithoutFilters, debounceHandleSearch]);

/* Sections generation */
const sections: Array<Section<AutocompleteListItem>> = [];

Check warning on line 358 in src/components/Search/SearchAutocompleteList.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

The 'sections' array makes the dependencies of useEffect Hook (at line 440) change on every render. To fix this, wrap the initialization of 'sections' in its own useMemo() Hook

Check warning on line 358 in src/components/Search/SearchAutocompleteList.tsx

View workflow job for this annotation

GitHub Actions / ESLint check 

The 'sections' array makes the dependencies of useEffect Hook (at line 440) change on every render. To fix this, wrap the initialization of 'sections' in its own useMemo() Hook
let sectionIndex = 0;

if (searchQueryItem) {
Expand Down Expand Up @@ -413,7 +447,6 @@
}
}, [autocompleteQueryValue, onHighlightFirstItem, normalizedReferenceText]);

const isRecentSearchesDataLoaded = !isLoadingOnyxValue(recentSearchesMetadata);
const isLoading = !isRecentSearchesDataLoaded || !areOptionsInitialized;

if (isLoading) {
Expand All @@ -426,6 +459,17 @@
);
}

// eslint-disable-next-line react-hooks/refs -- intentional: telemetry handoff must run during render to accurately mark compute→render transition
if (isInitialRender && computeSpanStarted.current && !spanHandoffDone.current) {
spanHandoffDone.current = true;
endSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_COMPUTE_OPTIONS);
startSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_LIST_RENDER, {
name: CONST.TELEMETRY.SPAN_SEARCH_ROUTER_LIST_RENDER,
op: 'ui.render',
parentSpan: getSpan(CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER),
});
}

return (
<SelectionListWithSections<AutocompleteListItem>
showLoadingPlaceholder
Expand All @@ -447,6 +491,7 @@
disableKeyboardShortcuts={!shouldSubscribeToArrowKeyEvents}
addBottomSafeAreaPadding
onLayout={() => {
endSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_LIST_RENDER);
setPerformanceTimersEnd();
setIsInitialRender(false);
innerListRef.current?.updateExternalTextInputFocus(textInputRef?.current?.isFocused() ?? false);
Expand Down
3 changes: 3 additions & 0 deletions src/components/Search/SearchRouter/SearchButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ function SearchButton({style, shouldUseAutoHitSlop = false}: SearchButtonProps)
startSpan(CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER, {
name: CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER,
op: CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER,
attributes: {
[CONST.TELEMETRY.ATTRIBUTE_TRIGGER]: 'button',
},
});

openSearchRouter();
Expand Down
10 changes: 8 additions & 2 deletions src/components/Search/SearchRouter/SearchRouterContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, {useContext, useEffect, useRef, useState} from 'react';
import type {AnimatedTextInputRef} from '@components/RNTextInput';
import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute';
import {navigationRef} from '@libs/Navigation/Navigation';
import {startSpan} from '@libs/telemetry/activeSpans';
import {endSpan, getSpan, startSpan} from '@libs/telemetry/activeSpans';
import {close} from '@userActions/Modal';
import CONST from '@src/CONST';
import NAVIGATORS from '@src/NAVIGATORS';
Expand Down Expand Up @@ -77,8 +77,14 @@ function SearchRouterContextProvider({children}: ChildrenProps) {
if (isBrowserWithHistory) {
window.history.pushState({isSearchModalOpen: true} satisfies HistoryState, '');
}
startSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_MODAL_CLOSE_WAIT, {
name: CONST.TELEMETRY.SPAN_SEARCH_ROUTER_MODAL_CLOSE_WAIT,
op: 'ui.modal.wait',
parentSpan: getSpan(CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER),
});
close(
() => {
endSpan(CONST.TELEMETRY.SPAN_SEARCH_ROUTER_MODAL_CLOSE_WAIT);
openSearch(setIsSearchRouterDisplayed);
searchRouterDisplayedRef.current = true;
},
Expand All @@ -103,7 +109,7 @@ function SearchRouterContextProvider({children}: ChildrenProps) {
name: CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER,
op: CONST.TELEMETRY.SPAN_OPEN_SEARCH_ROUTER,
attributes: {
trigger: 'keyboard',
[CONST.TELEMETRY.ATTRIBUTE_TRIGGER]: 'keyboard',
Copy link
Contributor

@JakubKorytko JakubKorytko Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should keyboard value be in the CONSTs as well?

Copy link
Contributor Author

@mountiny mountiny Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. The attribute keys (trigger, cold_start) are constants because they're the contract with Sentry and are referenced in multiple files. The attribute values ('keyboard', 'button') are each used exactly once and are self-documenting, so extracting them felt like over-abstracting. That said, it's a trivial change — happy to add TRIGGER_KEYBOARD and TRIGGER_BUTTON constants if you think it's worth it for consistency.

},
});
};
Expand Down
Loading