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.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
ccharly
force-pushed
the
cc/feat/perf-logging
branch
from
e1e894a to
a452037
Compare
Co-authored-by: Gustavo Antunes <17601467+gantunesr@users.noreply.github.com>
2 participants
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
Changes wrap and propagate the tracing callback through
MultichainAccountService/MultichainAccountWalletand add new trace events around account creation/alignment; while largely observational, it touches core wallet/provider flows and timeout handling, so regressions could affect account operations.Overview
Adds debug-only local performance tracing by introducing
analytics/perf(plustimer) and conditionally wrapping the configuredTraceCallbackwhenDEBUGenables the package logger.Expands tracing coverage and structure: new
TraceNameentries and helpers (toProviderDataTraces,toCreateAccountsV2DataTraces), traces around wallet alignment and group creation, and provider-side traces for SnapcreateAccountv1 vs batchedcreateAccountsv2.Refactors
withTimeoutto accept a thunk (() => Promise) and updates all call sites accordingly; updates/extends tests to validate trace propagation/wrapping and deferred alignment behavior, and removes the oldconstants/traces.tsin favor ofanalytics/traces.Written by Cursor Bugbot for commit 750e8aa. This will update automatically on new commits. Configure here.