Skip to content

Commit 3b21958

Browse files
MarioAslautommasini
MarioAslau
and
tommasini
authored
feat: 1940 add custom sentry span (#11935)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> <img width="1052" alt="Screenshot 2024-10-11 at 20 29 04" src="https://github.com/user-attachments/assets/a5680804-7bc8-45c4-a27a-d790bd5f9a5d"> <img width="766" alt="Screenshot 2024-10-16 at 00 01 02" src="https://github.com/user-attachments/assets/aa4689f2-9ce0-4dc7-9b99-f24fd4db1b78"> <img width="699" alt="Screenshot 2024-10-15 at 17 57 12" src="https://github.com/user-attachments/assets/dc31684a-4dea-4f26-8b18-c5497679a0ce"> This task is for adding custom spans to track activities that happen between app start and wallet UI load. The screenshot below is an example of a trace for Wallet UI load that takes about a minute to load. During that time, we can see a large gap between app start spans and the initial http requests. The goal here is to isolate these areas and track them with custom spans. Once implemented, we can expect to see the custom spans appearing within the gap, which would inform us of the areas to optimize Issue: MetaMask/mobile-planning#1940 Technical Details * Added custom span for when the Login screen is mounted to when the login button is tapped * Added span for when the login button is tapped to when the wallet view is mounted * Added custom span for Engine initialization process * Added custom span for Store creation * Added custom span Storage rehydration * Added custom span fro Create New Wallet to Choose Password * Added custom span for Biometrics authentication ##**Engine Initialisation Bug**## While initally the engine init trace was preceded by an await, which would allow the grouping of all the calls that happen inside Engine to be under the Engine Initialisation process, that would cause a crash of the application in Production on Android. As a temporary fix until the issue that happens within Engine is found and fixed, the only noticeable difference will take place as bellow. API calls from within the Engine Initialization, will not all be grouped within the same span. <img width="1560" alt="Screenshot 2024-10-22 at 05 02 54" src="https://github.com/user-attachments/assets/4abc259f-be70-4f74-af2b-7ece47343678"> (left without await, right with await) **Bitrise Build:** https://app.bitrise.io/build/6ee0b625-d8b9-4d95-9fcc-a1d624202736?tab=artifacts ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: tommasini <tommasini15@gmail.com> Co-authored-by: tommasini <46944231+tommasini@users.noreply.github.com>
1 parent 8b7f955 commit 3b21958

File tree

9 files changed

+199
-47
lines changed

9 files changed

+199
-47
lines changed

‎app/components/Nav/App/index.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ import OptionsSheet from '../../UI/SelectOptionSheet/OptionsSheet';
131131
import FoxLoader from '../../../components/UI/FoxLoader';
132132
import { AppStateEventProcessor } from '../../../core/AppStateEventListener';
133133
import MultiRpcModal from '../../../components/Views/MultiRpcModal/MultiRpcModal';
134+
import { trace, TraceName, TraceOperation } from '../../../util/trace';
134135

135136
const clearStackNavigatorOptions = {
136137
headerShown: false,
@@ -579,7 +580,15 @@ const App = (props) => {
579580
setOnboarded(!!existingUser);
580581
try {
581582
if (existingUser) {
582-
await Authentication.appTriggeredAuth();
583+
await trace(
584+
{
585+
name: TraceName.BiometricAuthentication,
586+
op: TraceOperation.BiometricAuthentication,
587+
},
588+
async () => {
589+
await Authentication.appTriggeredAuth();
590+
},
591+
);
583592
// we need to reset the navigator here so that the user cannot go back to the login screen
584593
navigator.reset({ routes: [{ name: Routes.ONBOARDING.HOME_NAV }] });
585594
} else {

‎app/components/Views/LockScreen/index.js‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
import Routes from '../../../constants/navigation/Routes';
2323
import { CommonActions } from '@react-navigation/native';
2424
import trackErrorAsAnalytics from '../../../util/metrics/TrackError/trackErrorAsAnalytics';
25+
import { trace, TraceName, TraceOperation } from '../../../util/trace';
2526

2627
const LOGO_SIZE = 175;
2728
const createStyles = (colors) =>
@@ -134,10 +135,19 @@ class LockScreen extends PureComponent {
134135
// Retrieve the credentials
135136
Logger.log('Lockscreen::unlockKeychain - getting credentials');
136137

137-
await Authentication.appTriggeredAuth({
138-
bioStateMachineId,
139-
disableAutoLogout: true,
140-
});
138+
await trace(
139+
{
140+
name: TraceName.BiometricAuthentication,
141+
op: TraceOperation.BiometricAuthentication,
142+
},
143+
async () => {
144+
await Authentication.appTriggeredAuth({
145+
bioStateMachineId,
146+
disableAutoLogout: true,
147+
});
148+
},
149+
);
150+
141151
this.setState({ ready: true });
142152
Logger.log('Lockscreen::unlockKeychain - state: ready');
143153
} catch (error) {

‎app/components/Views/Login/index.js‎

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ import Text, {
1717
} from '../../../component-library/components/Texts/Text';
1818
import StorageWrapper from '../../../store/storage-wrapper';
1919
import { KeyboardAwareScrollView } from 'react-native-keyboard-aware-scroll-view';
20-
import Button, { ButtonSize, ButtonVariants, ButtonWidthTypes } from '../../../component-library/components/Buttons/Button';
20+
import Button, {
21+
ButtonSize,
22+
ButtonVariants,
23+
ButtonWidthTypes,
24+
} from '../../../component-library/components/Buttons/Button';
2125
import { fontStyles } from '../../../styles/common';
2226
import { strings } from '../../../../locales/i18n';
2327
import FadeOutOverlay from '../../UI/FadeOutOverlay';
@@ -55,9 +59,19 @@ import { LoginViewSelectors } from '../../../../e2e/selectors/LoginView.selector
5559
import { withMetricsAwareness } from '../../../components/hooks/useMetrics';
5660
import trackErrorAsAnalytics from '../../../util/metrics/TrackError/trackErrorAsAnalytics';
5761
import { downloadStateLogs } from '../../../util/logs';
58-
import TextField, { TextFieldSize } from '../../../component-library/components/Form/TextField';
62+
import {
63+
trace,
64+
endTrace,
65+
TraceName,
66+
TraceOperation,
67+
} from '../../../util/trace';
68+
import TextField, {
69+
TextFieldSize,
70+
} from '../../../component-library/components/Form/TextField';
5971
import Label from '../../../component-library/components/Form/Label';
60-
import HelpText, { HelpTextSeverity } from '../../../component-library/components/Form/HelpText';
72+
import HelpText, {
73+
HelpTextSeverity,
74+
} from '../../../component-library/components/Form/HelpText';
6175

6276
const deviceHeight = Device.getDeviceHeight();
6377
const breakPoint = deviceHeight < 700;
@@ -107,7 +121,7 @@ const createStyles = (colors) =>
107121
},
108122
footer: {
109123
marginVertical: 40,
110-
alignItems: 'center'
124+
alignItems: 'center',
111125
},
112126
goBack: {
113127
marginVertical: 14,
@@ -237,6 +251,10 @@ class Login extends PureComponent {
237251
fieldRef = React.createRef();
238252

239253
async componentDidMount() {
254+
trace({
255+
name: TraceName.LoginToPasswordEntry,
256+
op: TraceOperation.LoginToPasswordEntry,
257+
});
240258
this.props.metrics.trackEvent(MetaMetricsEvents.LOGIN_SCREEN_VIEWED);
241259
BackHandler.addEventListener('hardwareBackPress', this.handleBackPress);
242260

@@ -360,7 +378,15 @@ class Login extends PureComponent {
360378
);
361379

362380
try {
363-
await Authentication.userEntryAuth(password, authType);
381+
await trace(
382+
{
383+
name: TraceName.AuthenticateUser,
384+
op: TraceOperation.AuthenticateUser,
385+
},
386+
async () => {
387+
await Authentication.userEntryAuth(password, authType);
388+
},
389+
);
364390

365391
Keyboard.dismiss();
366392

@@ -428,7 +454,15 @@ class Login extends PureComponent {
428454
const { current: field } = this.fieldRef;
429455
field?.blur();
430456
try {
431-
await Authentication.appTriggeredAuth();
457+
await trace(
458+
{
459+
name: TraceName.BiometricAuthentication,
460+
op: TraceOperation.BiometricAuthentication,
461+
},
462+
async () => {
463+
await Authentication.appTriggeredAuth();
464+
},
465+
);
432466
const onboardingWizard = await StorageWrapper.getItem(ONBOARDING_WIZARD);
433467
if (!onboardingWizard) this.props.setOnboardingWizardStep(1);
434468
this.props.navigation.replace(Routes.ONBOARDING.HOME_NAV);
@@ -447,6 +481,7 @@ class Login extends PureComponent {
447481
};
448482

449483
triggerLogIn = () => {
484+
endTrace({ name: TraceName.LoginToPasswordEntry });
450485
this.onLogin();
451486
};
452487

@@ -529,10 +564,7 @@ class Login extends PureComponent {
529564
)}
530565
</TouchableOpacity>
531566

532-
<Text
533-
style={styles.title}
534-
testID={LoginViewSelectors.TITLE_ID}
535-
>
567+
<Text style={styles.title} testID={LoginViewSelectors.TITLE_ID}>
536568
{strings('login.title')}
537569
</Text>
538570
<View style={styles.field}>
@@ -582,19 +614,22 @@ class Login extends PureComponent {
582614
style={styles.ctaWrapper}
583615
testID={LoginViewSelectors.LOGIN_BUTTON_ID}
584616
>
585-
586-
<Button variant={ButtonVariants.Primary}
617+
<Button
618+
variant={ButtonVariants.Primary}
587619
width={ButtonWidthTypes.Full}
588620
size={ButtonSize.Lg}
589621
onPress={this.triggerLogIn}
590-
label={this.state.loading ? (
591-
<ActivityIndicator
592-
size="small"
593-
color={colors.primary.inverse}
594-
/>
595-
) : (
596-
strings('login.unlock_button')
597-
)}/>
622+
label={
623+
this.state.loading ? (
624+
<ActivityIndicator
625+
size="small"
626+
color={colors.primary.inverse}
627+
/>
628+
) : (
629+
strings('login.unlock_button')
630+
)
631+
}
632+
/>
598633
</View>
599634

600635
<View style={styles.footer}>

‎app/components/Views/Login/index.test.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
import React from 'react';
22
import Login from './';
33
import renderWithProvider from '../../../util/test/renderWithProvider';
4+
// eslint-disable-next-line import/no-namespace
5+
import * as traceObj from '../../../util/trace';
46

57
describe('Login', () => {
68
it('should render correctly', () => {
7-
const { toJSON } = renderWithProvider(
8-
<Login />
9-
);
9+
const spyFetch = jest
10+
.spyOn(traceObj, 'trace')
11+
.mockImplementation(() => undefined);
12+
const { toJSON } = renderWithProvider(<Login />);
1013
expect(toJSON()).toMatchSnapshot();
14+
expect(spyFetch).toHaveBeenCalledTimes(1);
1115
});
1216
});

‎app/components/Views/Onboarding/index.js‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import { OnboardingSelectorIDs } from '../../../../e2e/selectors/Onboarding/Onbo
4949
import Routes from '../../../constants/navigation/Routes';
5050
import { selectAccounts } from '../../../selectors/accountTrackerController';
5151
import trackOnboarding from '../../../util/metrics/TrackOnboarding/trackOnboarding';
52+
import { trace, TraceName, TraceOperation } from '../../../util/trace';
5253

5354
const createStyles = (colors) =>
5455
StyleSheet.create({
@@ -275,24 +276,33 @@ class Onboarding extends PureComponent {
275276
};
276277

277278
onPressCreate = () => {
278-
const action = async () => {
279-
const { metrics } = this.props;
280-
if (metrics.isEnabled()) {
281-
this.props.navigation.navigate('ChoosePassword', {
282-
[PREVIOUS_SCREEN]: ONBOARDING,
283-
});
284-
this.track(MetaMetricsEvents.WALLET_SETUP_STARTED);
285-
} else {
286-
this.props.navigation.navigate('OptinMetrics', {
287-
onContinue: () => {
288-
this.props.navigation.replace('ChoosePassword', {
279+
const action = () => {
280+
trace(
281+
{
282+
name: TraceName.CreateNewWalletToChoosePassword,
283+
op: TraceOperation.CreateNewWalletToChoosePassword,
284+
},
285+
() => {
286+
const { metrics } = this.props;
287+
if (metrics.isEnabled()) {
288+
this.props.navigation.navigate('ChoosePassword', {
289289
[PREVIOUS_SCREEN]: ONBOARDING,
290290
});
291291
this.track(MetaMetricsEvents.WALLET_SETUP_STARTED);
292-
},
293-
});
294-
}
292+
} else {
293+
this.props.navigation.navigate('OptinMetrics', {
294+
onContinue: () => {
295+
this.props.navigation.replace('ChoosePassword', {
296+
[PREVIOUS_SCREEN]: ONBOARDING,
297+
});
298+
this.track(MetaMetricsEvents.WALLET_SETUP_STARTED);
299+
},
300+
});
301+
}
302+
},
303+
);
295304
};
305+
296306
this.handleExistingUser(action);
297307
};
298308

‎app/components/Views/Onboarding/index.test.tsx‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { renderScreen } from '../../../util/test/renderWithProvider';
22
import Onboarding from './';
33
import { backgroundState } from '../../../util/test/initial-root-state';
4+
import { OnboardingSelectorIDs } from '../../../../e2e/selectors/Onboarding/Onboarding.selectors';
5+
import { fireEvent } from '@testing-library/react-native';
6+
// eslint-disable-next-line import/no-namespace
7+
import * as traceObj from '../../../util/trace';
48

59
const mockInitialState = {
610
engine: {
@@ -21,4 +25,20 @@ describe('Onboarding', () => {
2125
);
2226
expect(toJSON()).toMatchSnapshot();
2327
});
28+
it('must call trace when press start', () => {
29+
const spyFetch = jest
30+
.spyOn(traceObj, 'trace')
31+
.mockImplementation(() => undefined);
32+
const { getByTestId } = renderScreen(
33+
Onboarding,
34+
{ name: 'Onboarding' },
35+
{
36+
state: mockInitialState,
37+
},
38+
);
39+
40+
const startButton = getByTestId(OnboardingSelectorIDs.NEW_WALLET_BUTTON);
41+
fireEvent.press(startButton);
42+
expect(spyFetch).toHaveBeenCalledTimes(1);
43+
});
2444
});

‎app/components/Views/Wallet/index.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ import { ButtonVariants } from '../../../component-library/components/Buttons/Bu
106106
import { useListNotifications } from '../../../util/notifications/hooks/useNotifications';
107107
import { PortfolioBalance } from '../../UI/Tokens/TokenList/PortfolioBalance';
108108
import { isObject } from 'lodash';
109+
109110
const createStyles = ({ colors, typography }: Theme) =>
110111
StyleSheet.create({
111112
base: {

‎app/store/index.ts‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { Authentication } from '../core';
99
import LockManagerService from '../core/LockManagerService';
1010
import ReadOnlyNetworkStore from '../util/test/network-store';
1111
import { isE2E } from '../util/test/utils';
12+
import { trace, endTrace, TraceName, TraceOperation } from '../util/trace';
13+
1214
import thunk from 'redux-thunk';
1315

1416
import persistConfig from './persistConfig';
@@ -46,6 +48,11 @@ const createStoreAndPersistor = async () => {
4648
middlewares.push(createReduxFlipperDebugger());
4749
}
4850

51+
trace({
52+
name: TraceName.CreateStore,
53+
op: TraceOperation.CreateStore,
54+
});
55+
4956
store = configureStore({
5057
reducer: pReducer,
5158
middleware: middlewares,
@@ -54,10 +61,19 @@ const createStoreAndPersistor = async () => {
5461

5562
sagaMiddleware.run(rootSaga);
5663

64+
endTrace({ name: TraceName.CreateStore });
65+
66+
trace({
67+
name: TraceName.StorageRehydration,
68+
op: TraceOperation.StorageRehydration,
69+
});
70+
5771
/**
5872
* Initialize services after persist is completed
5973
*/
60-
const onPersistComplete = () => {
74+
const onPersistComplete = async () => {
75+
endTrace({ name: TraceName.StorageRehydration });
76+
6177
/**
6278
* EngineService.initalizeEngine(store) with SES/lockdown:
6379
* Requires ethjs nested patches (lib->src)
@@ -73,6 +89,7 @@ const createStoreAndPersistor = async () => {
7389
* - TypeError: undefined is not an object (evaluating 'TokenListController.tokenList')
7490
* - V8: SES_UNHANDLED_REJECTION
7591
*/
92+
7693
store.dispatch({
7794
type: 'TOGGLE_BASIC_FUNCTIONALITY',
7895
basicFunctionalityEnabled:
@@ -83,7 +100,16 @@ const createStoreAndPersistor = async () => {
83100
store.dispatch({
84101
type: 'FETCH_FEATURE_FLAGS',
85102
});
86-
EngineService.initalizeEngine(store);
103+
trace(
104+
{
105+
name: TraceName.EngineInitialization,
106+
op: TraceOperation.EngineInitialization,
107+
},
108+
() => {
109+
EngineService.initalizeEngine(store);
110+
},
111+
);
112+
87113
Authentication.init(store);
88114
AppStateEventProcessor.init(store);
89115
LockManagerService.init(store);
@@ -93,7 +119,13 @@ const createStoreAndPersistor = async () => {
93119
};
94120

95121
(async () => {
96-
await createStoreAndPersistor();
122+
await trace(
123+
{
124+
name: TraceName.UIStartup,
125+
op: TraceOperation.UIStartup,
126+
},
127+
async () => await createStoreAndPersistor(),
128+
);
97129
})();
98130

99131
export { store, persistor };

0 commit comments

Comments
 (0)