ci: Sentry reporting only on develop branch, with Git message overrides by HowardBraham · Pull Request #27412 · MetaMask/metamask-extension

HowardBraham · 2024-09-26T01:33:23Z

Description

Vaibhav asked for new Sentry reporting rates from CircleCI:

10 times as frequent from the develop branch
Never from other branches

I also put in an override, such that if your Git message includes [flags.sentry.tracesSampleRate: x.xx] (a decimal number from 0.00 to 1.00), it will set tracesSampleRate to that fraction.

Moved what used to be at _flags.doNotForceSentryForThisTest to _flags.sentry.doNotForceSentryForThisTest

Related issues

Improves on: #26588

Manual testing steps

You shouldn't see any Sentry reporting from this branch
Try a commit on top of this with a Git message including [flags.sentry.tracesSampleRate: 1.00], and it should report to Sentry

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
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 format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-09-26T01:33:36Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2024-09-26T02:31:49Z

Builds ready [abd82c5]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1976 ± 116 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	314	2530	1876	426	204
		domContentLoaded	1539	2387	1941	226	108
		load	1547	2538	1976	242	116
		domInteractive	21	109	57	21	10

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 225 Bytes (0.00%)

sonarqubecloud · 2024-09-26T02:33:35Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

metamaskbot · 2024-09-26T02:58:46Z

Builds ready [75ab8e0]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1820 ± 85 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1554	2409	1811	177	85
		domContentLoaded	1510	2360	1786	170	82
		load	1556	2415	1820	178	85
		domInteractive	17	172	54	36	17

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 225 Bytes (0.00%)

v-goyal

LGTM

matthewwalsh0 · 2024-09-27T21:33:41Z

app/scripts/lib/manifestFlags.ts

-  doNotForceSentryForThisTest?: boolean;
+  sentry?: {
+    tracesSampleRate?: number;
+    doNotForceSentryForThisTest?: boolean;


Minor, could we remove the Sentry from this property as it's now implied by the parent property?

matthewwalsh0 · 2024-09-27T21:34:53Z

app/scripts/lib/setupSentry.js

+    // Report very frequently on develop branch, and never on other branches
+    // (Unless you do a [flags.sentry.tracesSampleRate: x.xx] override)
+    if (flags.circleci.branch === 'develop') {
+      return 0.03;


Minor, is it worth some constants for all these values for easier maintenance? Such as TRACES_SAMPLE_RATE_CIRCLE_CI_DEVELOP, TRACES_SAMPLE_RATE_CIRCLE_CI_OTHER, TRACES_SAMPLE_RATE_DEBUG etc?

matthewwalsh0 · 2024-09-27T21:54:16Z

test/e2e/set-manifest-flags.ts

 }

+// Grab the tracesSampleRate from the git message if it's set
+function getTracesSampleRateFromGitMessage(): number | undefined {


If the goal is to support this for specific PRs, is it possible to read this from the PR title or description via CircleCI?

@matthewwalsh0 This would require network calls to the GitHub API, which if we don't want to do it hundreds of times, is best done in get-changed-files-with-git-diff

Let's see if anyone even wants to use this system before we spend a lot of additional time on it.

matthewwalsh0 · 2024-09-27T21:54:46Z

test/e2e/set-manifest-flags.ts

+
+  // Search gitMessage for `[flags.sentry.tracesSampleRate: 0.000 to 1.000]`
+  const tracesSampleRateMatch = gitMessage.match(
+    /\[flags\.sentry\.tracesSampleRate: (0*(\.\d+)?|1(\.0*)?)\]/u,


Minor, is it worth abstracting this into a getConfigFromGitMessage(flag: string): string so this can be used in future?

HowardBraham added the team-tiger-deprecated DEPRECATED: team no longer exists label Sep 26, 2024

HowardBraham self-assigned this Sep 26, 2024

github-actions bot added the team-contributor-experience label Sep 26, 2024

HowardBraham removed the team-contributor-experience label Sep 26, 2024

HowardBraham changed the title ~~ci: sentry reporting only on develop branch, with git message overrides~~ ci: Sentry reporting only on develop branch, with Git message overrides Sep 26, 2024

HowardBraham requested review from davidmurdoch, matthewwalsh0 and v-goyal September 26, 2024 02:12

ci: sentry reporting only on develop branch, with git message overrides

75ab8e0

HowardBraham force-pushed the sentry-reporting-develop branch from abd82c5 to 75ab8e0 Compare September 26, 2024 02:21

HowardBraham marked this pull request as ready for review September 26, 2024 02:21

HowardBraham requested a review from a team as a code owner September 26, 2024 02:21

v-goyal approved these changes Sep 26, 2024

View reviewed changes

matthewwalsh0 approved these changes Sep 27, 2024

View reviewed changes

HowardBraham merged commit d1b778c into develop Sep 28, 2024

HowardBraham deleted the sentry-reporting-develop branch September 28, 2024 00:34

github-actions bot locked and limited conversation to collaborators Sep 28, 2024

metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: Sentry reporting only on develop branch, with Git message overrides#27412

ci: Sentry reporting only on develop branch, with Git message overrides#27412
HowardBraham merged 1 commit intodevelopfrom
sentry-reporting-develop

HowardBraham commented Sep 26, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Sep 26, 2024

Uh oh!

metamaskbot commented Sep 26, 2024

Uh oh!

sonarqubecloud bot commented Sep 26, 2024

Uh oh!

metamaskbot commented Sep 26, 2024

Uh oh!

v-goyal left a comment

Uh oh!

matthewwalsh0 Sep 27, 2024

Uh oh!

matthewwalsh0 Sep 27, 2024

Uh oh!

matthewwalsh0 Sep 27, 2024

Uh oh!

HowardBraham Sep 30, 2024

Uh oh!

matthewwalsh0 Sep 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

HowardBraham commented Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Sep 26, 2024

Uh oh!

metamaskbot commented Sep 26, 2024

Uh oh!

sonarqubecloud bot commented Sep 26, 2024

Quality Gate failed

Uh oh!

metamaskbot commented Sep 26, 2024

Uh oh!

v-goyal left a comment

Choose a reason for hiding this comment

Uh oh!

matthewwalsh0 Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

matthewwalsh0 Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

matthewwalsh0 Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

HowardBraham Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

matthewwalsh0 Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HowardBraham commented Sep 26, 2024 •

edited

Loading