-
Notifications
You must be signed in to change notification settings - Fork 5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refactor: add performance tracing infrastructure (#26044) [cherry-pic…
…k] (#26624) Cherry-pick of (#26044) for v12.1.0. original description: ## **Description** Add initial tracing and performance metrics infrastructure. Specifically: - Add the `trace` method, to be used anywhere in the client to generate a Sentry trace to monitor and analyse performance metrics. - Add buttons to the developer settings as an easy mechanism to generate Sentry errors (in the UI or background) and traces. - Add the `Sentry.browserTracingIntegration` to automatically generate traces for each page navigation, including nested spans for any HTTP requests. - Update the Sentry debugging documentation in the `README`. Note that the previously used `browserProfilingIntegration` does not generate transactions or traces itself, but rather attempts to add profiling data to any generated transactions. This does not appear to be compatible given it requires the Self-Profiling API object which is only available in Chrome and in our case, the UI. In addition, it requires the `js-profiling` document policy which also appears to currently not be supported in browser extensions. Sentry also provides React specific automated instrumentation via `@sentry/react` and the `Sentry.reactRouterV5BrowserTracingInstrumentation` integration, but this should be pursued in a separate ticket if needed given the required further refactor. [](https://codespaces.new/MetaMask/metamask-extension/pull/26044?quickstart=1) ## **Related issues** Fixes: [#2711](MetaMask/MetaMask-planning#2711) ## **Manual testing steps** 1. Enable metrics in the settings. 2. Login to Sentry account. 3. Go to `Traces` or `Performance` section. 4. Note new transactions or traces named `popup.html` with nested spans including HTTP requests. ## **Screenshots/Recordings** ### **Before** ### **After** <img width="250" alt="Sentry Developer Options" src="https://github.com/user-attachments/assets/620cbfb8-ab4d-4ed0-b5f6-ba04ef975ddc"> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.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: Matthew Walsh <matthew.walsh@consensys.net>
](https://codespaces.new/MetaMask/metamask-extension/pull/26044?quickstart=1){kind=link}
- Loading branch information
1 parent
6927746
commit 42555d3
Showing
11 changed files
with
589 additions
and
42 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import { Span, startSpan, withIsolationScope } from '@sentry/browser'; | ||
| import { trace } from './trace'; | ||
|
|
||
| jest.mock('@sentry/browser', () => ({ | ||
| withIsolationScope: jest.fn(), | ||
| startSpan: jest.fn(), | ||
| })); | ||
|
|
||
| const NAME_MOCK = 'testTransaction'; | ||
| const PARENT_CONTEXT_MOCK = {} as Span; | ||
|
|
||
| const TAGS_MOCK = { | ||
| tag1: 'value1', | ||
| tag2: true, | ||
| tag3: 123, | ||
| }; | ||
|
|
||
| const DATA_MOCK = { | ||
| data1: 'value1', | ||
| data2: true, | ||
| data3: 123, | ||
| }; | ||
|
|
||
| function mockGetMetaMetricsEnabled(enabled: boolean) { | ||
| global.sentry = { | ||
| getMetaMetricsEnabled: () => Promise.resolve(enabled), | ||
| }; | ||
| } | ||
|
|
||
| describe('Trace', () => { | ||
| const startSpanMock = jest.mocked(startSpan); | ||
| const withIsolationScopeMock = jest.mocked(withIsolationScope); | ||
| const setTagsMock = jest.fn(); | ||
|
|
||
| beforeEach(() => { | ||
| jest.resetAllMocks(); | ||
|
|
||
| startSpanMock.mockImplementation((_, fn) => fn({} as Span)); | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| withIsolationScopeMock.mockImplementation((fn: any) => | ||
| fn({ setTags: setTagsMock }), | ||
| ); | ||
| }); | ||
|
|
||
| describe('trace', () => { | ||
| // @ts-expect-error This function is missing from the Mocha type definitions | ||
| it.each([ | ||
| ['enabled', true], | ||
| ['disabled', false], | ||
| ])( | ||
| 'executes callback if Sentry is %s', | ||
| async (_: string, sentryEnabled: boolean) => { | ||
| let callbackExecuted = false; | ||
|
|
||
| mockGetMetaMetricsEnabled(sentryEnabled); | ||
|
|
||
| await trace({ name: NAME_MOCK }, async () => { | ||
| callbackExecuted = true; | ||
| }); | ||
|
|
||
| expect(callbackExecuted).toBe(true); | ||
| }, | ||
| ); | ||
|
|
||
| // @ts-expect-error This function is missing from the Mocha type definitions | ||
| it.each([ | ||
| ['enabled', true], | ||
| ['disabled', false], | ||
| ])( | ||
| 'returns value from callback if Sentry is %s', | ||
| async (_: string, sentryEnabled: boolean) => { | ||
| mockGetMetaMetricsEnabled(sentryEnabled); | ||
|
|
||
| const result = await trace({ name: NAME_MOCK }, async () => { | ||
| return true; | ||
| }); | ||
|
|
||
| expect(result).toBe(true); | ||
| }, | ||
| ); | ||
|
|
||
| it('invokes Sentry if enabled', async () => { | ||
| mockGetMetaMetricsEnabled(true); | ||
|
|
||
| await trace( | ||
| { | ||
| name: NAME_MOCK, | ||
| tags: TAGS_MOCK, | ||
| data: DATA_MOCK, | ||
| parentContext: PARENT_CONTEXT_MOCK, | ||
| }, | ||
| async () => Promise.resolve(), | ||
| ); | ||
|
|
||
| expect(withIsolationScopeMock).toHaveBeenCalledTimes(1); | ||
|
|
||
| expect(startSpanMock).toHaveBeenCalledTimes(1); | ||
| expect(startSpanMock).toHaveBeenCalledWith( | ||
| { | ||
| name: NAME_MOCK, | ||
| parentSpan: PARENT_CONTEXT_MOCK, | ||
| attributes: DATA_MOCK, | ||
| }, | ||
| expect.any(Function), | ||
| ); | ||
|
|
||
| expect(setTagsMock).toHaveBeenCalledTimes(1); | ||
| expect(setTagsMock).toHaveBeenCalledWith(TAGS_MOCK); | ||
| }); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import * as Sentry from '@sentry/browser'; | ||
| import { Primitive } from '@sentry/types'; | ||
| import { createModuleLogger } from '@metamask/utils'; | ||
| import { log as sentryLogger } from '../../app/scripts/lib/setupSentry'; | ||
|
|
||
| const log = createModuleLogger(sentryLogger, 'trace'); | ||
|
|
||
| export type TraceRequest = { | ||
| data?: Record<string, number | string | boolean>; | ||
| name: string; | ||
| parentContext?: unknown; | ||
| tags?: Record<string, number | string | boolean>; | ||
| }; | ||
|
|
||
| export async function trace<T>( | ||
| request: TraceRequest, | ||
| fn: (context?: unknown) => Promise<T>, | ||
| ): Promise<T> { | ||
| const { data: attributes, name, parentContext, tags } = request; | ||
| const parentSpan = (parentContext ?? null) as Sentry.Span | null; | ||
|
|
||
| const isSentryEnabled = | ||
| (await globalThis.sentry.getMetaMetricsEnabled()) as boolean; | ||
|
|
||
| const callback = async (span: Sentry.Span | null) => { | ||
| log('Starting trace', name, request); | ||
|
|
||
| const start = Date.now(); | ||
| let error; | ||
|
|
||
| try { | ||
| return await fn(span); | ||
| } catch (currentError) { | ||
| error = currentError; | ||
| throw currentError; | ||
| } finally { | ||
| const end = Date.now(); | ||
| const duration = end - start; | ||
|
|
||
| log('Finished trace', name, duration, { error, request }); | ||
| } | ||
| }; | ||
|
|
||
| if (!isSentryEnabled) { | ||
| log('Skipping Sentry trace as metrics disabled', name, request); | ||
| return callback(null); | ||
| } | ||
|
|
||
| return await Sentry.withIsolationScope(async (scope) => { | ||
| scope.setTags(tags as Record<string, Primitive>); | ||
|
|
||
| return await Sentry.startSpan({ name, parentSpan, attributes }, callback); | ||
| }); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.