MetaMask / MetaMask/metamask-extension
[P1] Benchmark outputs still describe mocked runs as live, on the PR comment and in Sentry
- Dominant language
- TypeScript
- Stars
- 13.2k
- Forks
- 5.6k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 451
Description
**Parent Epic:** [#7588 Gate Verdicts and Output](https://github.com/MetaMask/MetaMask-planning/issues/7588) → [#7586 Quality Gates: Reliability](https://github.com/MetaMask/MetaMask-planning/issues/7586)
**Files:** `test/e2e/benchmarks/utils/mock-config.ts`, `development/metamaskbot-build-announce/`, `.github/workflows/run-benchmarks.yml`
**Size:** S for the label, M for the Sentry decision
---
## Problem
[#45679](https://github.com/MetaMask/metamask-extension/pull/45679) merged 2026-08-24 and made `shouldUseMockedRequests()` return `true` unconditionally. Every benchmark on every ref now runs against mocks, `main` and `release/*` included.
Two outputs still describe those runs as live, and both are read by people making decisions.
**The PR comment says `real API`.** The population label was not updated alongside the code, so every `main` and `release/*` benchmark comment tells its reader the numbers came from live infrastructure. [#45267](https://github.com/MetaMask/metamask-extension/issues/45267) reported this shape and was closed 2026-08-11 as a false report — correctly, at the time. The code moved under the closure thirteen days later.
**The Sentry release-performance series changed meaning on the same date.** `run-benchmarks.yml:88-96` and `:178-186` still send `main` and `release/*` results to Sentry gated on `MAIN_OR_RELEASE`. [#7201](https://github.com/MetaMask/MetaMask-planning/issues/7201) describes those as the canonical release-performance figures, tracked so Sentry reflects real-world latency. After 2026-08-24 they are mocked timings under an unchanged series name.
Verification that this was a change rather than a standing condition: at `main` (`0350a01878e0d5e0eb1553b18ca7d653b4157cbb`) the function body is a bare `return true`. At `fb38a10e` — the sha [#45446](https://github.com/MetaMask/metamask-extension/issues/45446)'s own permalink pins — the branch-dependent form is still present.
---
## Why it matters
A wrong population label is worse than a missing one. A reader who knows the population is unstated will go and check; a reader told `real API` has no reason to.
For Sentry the cost compounds with time. A dashboard that silently changes what it measures produces a step at the changeover that reads as a performance event, and every later comparison against pre-2026-08-24 data is comparing two populations. Nobody has decided whether that series should still receive these numbers, whether it needs a population dimension, or whether the history is now discontinuous at that date.
---
## Solution
1. Derive the PR comment's population label from `shouldUseMockedRequests()` rather than from the ref, so the label cannot drift from the code again.
2. Decide what the Sentry series should do. Three options, none obviously right: keep sending with a population tag and mark the discontinuity; stop sending until a live population exists again; or keep sending untagged and accept the break. This is a decision, not an implementation, and it wants whoever reads that dashboard.
3. Resolve the dead branch-dependent path. `mock-config.ts` carries `TODO: Add a CI workflow that uses unmocked requests, or delete this dead code`, and `getTestSpecificMock`'s docblock (lines 17-29) still documents behavior that no longer happens.
---
## Acceptance Criteria
- [ ] No benchmark output describes a mocked run as live, on any surface
- [ ] The population label is computed from the same source of truth the runs use
- [ ] A decision on the Sentry series is recorded, with its date, whichever way it goes
- [ ] `getTestSpecificMock`'s docblock describes what the code does
- [ ] The dead live-request path is deleted, or a workflow exercises it
---
## Dependencies
**Related:** [#45267 release PRs report `real API` while mocked](https://github.com/MetaMask/metamask-extension/issues/45267) — closed as a false report before the code changed; this is the same surface after it did
**Related:** [#45472 no stable real-world measure](https://github.com/MetaMask/metamask-extension/issues/45472) and [#45446 live timings dominated by CDN cache state](https://github.com/MetaMask/metamask-extension/issues/45446) — where a live population would come back from, if one does
---
## Labels
`team-extension-platform`, `area-CI`
Contributor guide
Research direction
Start with test/e2e/benchmarks/utils/mock-config.ts, development/metamaskbot-build-announce/, and the MAIN_OR_RELEASE sections at lines 88-96 and 178-186 of .github/workflows/run-benchmarks.yml. Trace shouldUseMockedRequests() into the PR comment label, then review the Sentry series context and the getTestSpecificMock docblock. Done means every output identifies its population, the Sentry decision is recorded, and the dead path or its workflow is addressed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github-actions, typescript
- Domain
- ci-cd, observability
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100