Skip to content
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.

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

refactor: add performance tracing infrastructure #26044

Merged
merged 46 commits into from
Aug 12, 2024

Conversation

matthewwalsh0
Copy link
Member

@matthewwalsh0 matthewwalsh0 commented Jul 23, 2024

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.

Open in GitHub Codespaces

Related issues

Fixes: #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

Sentry Developer Options

Pre-merge author checklist

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.

@matthewwalsh0 matthewwalsh0 added team-confirmations Push issues to confirmations team DO-NOT-MERGE Pull requests that should not be merged labels Jul 23, 2024
@DDDDDanica
Copy link
Contributor

Notes: to update the document for sentry debugging:
https://github.com/MetaMask/metamask-extension/blob/develop/development/README.md#debugging-in-sentry

Update README.
Rename type.
@matthewwalsh0 matthewwalsh0 removed the DO-NOT-MERGE Pull requests that should not be merged label Jul 31, 2024
@matthewwalsh0 matthewwalsh0 changed the title [DO NOT MERGE] refactor: add performance tracing infrastructure refactor: add performance tracing infrastructure Jul 31, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [3e09194]
Page Load Metrics (305 ± 285 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint5911485178
domContentLoaded94921115
load391950305593285
domInteractive84921115
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 6.32 KiB (0.09%)
  • common: 301 Bytes (0.00%)

Reduce sample rate.
Extend README.
Rename shared file.
Copy link

sonarcloud bot commented Aug 1, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [f1895b6]
Page Load Metrics (63 ± 8 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7214297178
domContentLoaded97723147
load4512263168
domInteractive97723147
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 6.46 KiB (0.09%)
  • common: 843 Bytes (0.01%)

@matthewwalsh0 matthewwalsh0 merged commit 1ccd235 into develop Aug 12, 2024
75 checks passed
@matthewwalsh0 matthewwalsh0 deleted the refactor/sentry-performance-infrastructure branch August 12, 2024 15:50
@github-actions github-actions bot locked and limited conversation to collaborators Aug 12, 2024
@metamaskbot metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Aug 12, 2024
@metamaskbot metamaskbot added release-12.2.0 Issue or pull request that will be included in release 12.2.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Aug 28, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-12.2.0 on PR. Adding release label release-12.2.0 on PR and removing other release labels(release-12.4.0), as PR was cherry-picked in branch 12.2.0.

@gauthierpetetin gauthierpetetin added release-12.1.0 Issue or pull request that will be included in release 12.1.0 and removed release-12.2.0 Issue or pull request that will be included in release 12.2.0 labels Sep 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.1.0 Issue or pull request that will be included in release 12.1.0 team-confirmations Push issues to confirmations team team-tiger Tiger team (for tech debt reduction + performance improvements)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants