feat(multichain-account-service): add local perf tracing (debug) + more traces#8244
feat(multichain-account-service): add local perf tracing (debug) + more traces#8244
Conversation
ccharly
changed the title
| accounts: [[mockEvmAccount0, mockEvmAccount1, mockEvmAccount2]], | ||
| }); | ||
|
|
||
| jest.spyOn(wallet, 'alignAccounts').mockResolvedValue(undefined); |
There was a problem hiding this comment.
This actually had no effect!
| // We use a non-resolving mock for SOL provider because we want to verify | ||
| // that it's not called during group creation, but rather during the deferred alignment. | ||
| const { | ||
| promise: mockSolCreateAccountsPromise, | ||
| resolve: mockSolCreateAccountsResolve, | ||
| } = createDeferredPromise(); | ||
| jest | ||
| .spyOn(solProvider, 'createAccounts') | ||
| .mockImplementationOnce(() => mockSolCreateAccountsPromise); |
There was a problem hiding this comment.
Re-wrote this logic since non-EVM post alignment is scheduled asynchronously, so it's not always reliable to have determinist behavior when you change async calls.
This should future-proof it in a more reliable way!
| const mockTrace = jest.fn().mockImplementation(async (request, fn) => { | ||
| expect(request.name).toBe(TraceName.SnapDiscoverAccounts); | ||
| expect(request.data).toStrictEqual({ | ||
| provider: BTC_ACCOUNT_PROVIDER_NAME, | ||
| }); | ||
| return await fn(); | ||
| }); |
There was a problem hiding this comment.
We have new traces now for our providers, we cannot just expect this one.
Note
This comment applies to all our providers!
| */ | ||
| export async function withTimeout<T>( | ||
| promise: Promise<T>, | ||
| fn: () => Promise<T>, |
There was a problem hiding this comment.
Decided to use a callback now, making it much easier to wrap the actual content in a trace call
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/multichain-account-service/src/MultichainAccountWallet.ts
Outdated
Show resolved
Hide resolved
packages/multichain-account-service/src/providers/SolAccountProvider.ts
Outdated
Show resolved
Hide resolved
packages/multichain-account-service/src/MultichainAccountWallet.ts
Outdated
Show resolved
Hide resolved
| SnapDiscoverAccounts = 'Snap Discover Accounts', | ||
| EvmDiscoverAccounts = 'EVM Discover Accounts', |
There was a problem hiding this comment.
We could potentially wrap those existing traces in new ones to match the new "trace naming" like ProviderDiscoverAccounts and then use the data.provider to filter them out differently? 🤔
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
1 participant
Explanation
Adding more perf tracing + a way to wrap Sentry perf logs with local debbuging, making it easier to get feedback when developping.
References
N/A
Checklist
Note
Medium Risk
Adds pervasive tracing wrappers (including optional performance timing) across wallet/group alignment and Snap account creation flows, and changes
withTimeoutto accept a thunk, which can subtly alter async timing/side effects. Main functional behavior should be unchanged, but instrumentation now wraps key operations and touches multiple providers/tests.Overview
Adds local performance tracing (gated by
DEBUGlogger enablement) by introducinganalytics/perfand auto-wrapping the configuredtracecallback inMultichainAccountService, then passing this trace down intoMultichainAccountWalletand providers.Expands trace coverage by adding new
TraceNameentries and wrapping key wallet operations (group creation and alignment, including post-/discovery alignment tags) plus Snap provider account creation paths (v1 and v2/batched) with richer tracedata(provider set, group index/range).Refactors provider timeout usage by changing
withTimeoutto take a callback returning a promise (avoiding eagerly-started work), and updates EVM/SOL/BTC/TRX discovery and Snap account creation call sites accordingly; updates/extends tests and movesTraceNamefromconstants/tracesintoanalytics/traces.Written by Cursor Bugbot for commit a5709d7. This will update automatically on new commits. Configure here.