Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions .github/scripts/benchmark-stats-commit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,23 @@ CLONE_DIR="temp-benchmark-stats"
RAW_BRANCH="${BRANCH_NAME:-main}"
SAFE_BRANCH="${RAW_BRANCH//\//-}"

# Mirrors resolveBenchmarkMockMode() in shared/constants/benchmarks.ts, including
# its precedence: an explicit BENCHMARK_MOCK_MODE wins over the branch heuristic.
# The two must agree — the harness stamps the mode it measured under and this
# stamps the mode the entry is filed under, and a disagreement silently files a
# live measurement in the mocked series.
resolve_mock_mode() {
case "${BENCHMARK_MOCK_MODE:-}" in
mocked) echo 'mocked'; return ;;
live) echo 'live'; return ;;
esac
if [[ "${RAW_BRANCH}" == "main" || "${RAW_BRANCH}" == release/* ]]; then
echo 'live'
else
echo 'mocked'
fi
}

# Assemble the commit data based on mode
assemble_performance_data() {
local results_dir="${BENCHMARK_RESULTS_DIR:-benchmark-results}"
Expand Down Expand Up @@ -137,10 +154,8 @@ assemble_performance_data() {
# `live` run carries upstream latency that a `mocked` run does not, so
# blending them yields deltas that describe the internet rather than the
# commit. Mirrors resolveBenchmarkMockMode() in shared/constants/benchmarks.ts.
local mock_mode='mocked'
if [[ "${RAW_BRANCH}" == "main" || "${RAW_BRANCH}" == release/* ]]; then
mock_mode='live'
fi
local mock_mode
mock_mode="$(resolve_mock_mode)"

# presets_json can exceed ARG_MAX; pass it via stdin instead of as a jq argument
# (a too-large argv makes the kernel fail to exec jq with "Argument list too long").
Expand All @@ -153,7 +168,16 @@ assemble_performance_data() {
# Resolve stats file and assemble data
case "${DATA_TYPE}" in
performance)
STATS_FILE="stats/${SAFE_BRANCH}/performance_data.json"
# Separate file per population. Keying only by branch would put both
# series in one file now that main publishes mocked as well as live, and
# the consumer's filter would then be the only thing keeping them apart —
# one unstamped entry and a mocked baseline silently absorbs live latency.
# `live` keeps the historical path so the existing series is continuous.
if [[ "$(resolve_mock_mode)" == 'mocked' ]]; then
STATS_FILE="stats/${SAFE_BRANCH}/performance_data_mocked.json"
else
STATS_FILE="stats/${SAFE_BRANCH}/performance_data.json"
Comment thread
cursor[bot] marked this conversation as resolved.
fi
COMMIT_MESSAGE="Adding performance benchmark data for ${RAW_BRANCH} at commit: ${HEAD_COMMIT_HASH}"
echo "Mode: performance (branch: ${RAW_BRANCH})"
echo "Assembling benchmark data from directory..."
Expand Down
17 changes: 17 additions & 0 deletions .github/workflows/run-benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ on:
required: true
type: string
description: The run ID to get builds from
benchmark-mock-mode:
required: false
type: string
default: mocked
description: >-
Network population to measure: `mocked` or `live`. Overrides the branch
heuristic in resolveBenchmarkMockMode(). The per-commit path takes the
default so every commit is measured against one population; the
scheduled drift job passes `live`. Leave empty to fall back to the
branch heuristic.

permissions:
contents: read
Expand Down Expand Up @@ -49,6 +59,7 @@ jobs:
GITHUB_REF_NAME: ${{ github.head_ref || github.ref_name }}
# main or release/* only — for S3 / AWS CLI (same ref semantics as Sentry).
MAIN_OR_RELEASE: ${{ github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/') }}
BENCHMARK_MOCK_MODE: ${{ inputs.benchmark-mock-mode }}
steps:
- name: Checkout and setup environment
uses: MetaMask/action-checkout-and-setup@v3
Expand Down Expand Up @@ -134,6 +145,7 @@ jobs:
BENCHMARK_PAGE_LOADS: '10'
DAPP_BENCHMARK_JSON: benchmark-chrome-webpack-pageLoadBenchmark.json
MAIN_OR_RELEASE: ${{ github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/') }}
BENCHMARK_MOCK_MODE: ${{ inputs.benchmark-mock-mode }}
steps:
- name: Checkout and setup environment
uses: MetaMask/action-checkout-and-setup@v3
Expand Down Expand Up @@ -214,6 +226,10 @@ jobs:
}}
runs-on: ubuntu-latest
timeout-minutes: 10
env:
# The gate must resolve the same population the harness measured under,
# or it selects a baseline from the other series.
BENCHMARK_MOCK_MODE: ${{ inputs.benchmark-mock-mode }}
steps:
- name: Checkout quality-gate sources
uses: actions/checkout@v6
Expand Down Expand Up @@ -313,6 +329,7 @@ jobs:
HEAD_COMMIT_HASH: ${{ github.sha }}
OWNER: ${{ github.repository_owner }}
BRANCH_NAME: ${{ github.ref_name }}
BENCHMARK_MOCK_MODE: ${{ inputs.benchmark-mock-mode }}
steps:
- name: Checkout repository
uses: actions/checkout@v6
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,10 @@ async function main(): Promise<void> {

// Same branch-derived value the benchmark harness used to decide whether to
// mock, so the gate evaluates the population that was actually measured.
const mockMode = resolveBenchmarkMockMode(process.env.GITHUB_REF_NAME);
const mockMode = resolveBenchmarkMockMode(
process.env.GITHUB_REF_NAME,
process.env.BENCHMARK_MOCK_MODE,
);
const baseline = await loadBaseline(mockMode);

const result = runComparison(benchmarks, baseline, mockMode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,30 @@ describe('fetchHistoricalPerformanceDataFromMain', () => {
);
});

it('fetches the mocked series, not the live one, for a mocked run', async () => {
// The two populations are written to separate files. Filtering on the
// in-file `mockMode` is not enough — a mocked consumer pointed at the live
// file finds no matching entries and reports no baseline forever, which
// silently defeats publishing the mocked series at all.
mockFetch.mockReturnValueOnce(makeOkResponse(mockFile));

await fetchHistoricalPerformanceDataFromMain(BENCHMARK_MOCK_MODE.MOCKED);

expect(mockFetch).toHaveBeenCalledWith(
expect.stringContaining('stats/main/performance_data_mocked.json'),
);
});

it('keeps the live series on the unsuffixed path so the existing history stays continuous', async () => {
mockFetch.mockReturnValueOnce(makeOkResponse(mockFile));

await fetchHistoricalPerformanceDataFromMain(BENCHMARK_MOCK_MODE.LIVE);

const [url] = mockFetch.mock.calls[0];
expect(url).toContain('stats/main/performance_data.json');
expect(url).not.toContain('performance_data_mocked.json');
});

it('returns null when main has no data', async () => {
mockFetch.mockReturnValueOnce(makeNotFoundResponse());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
BenchmarkResults,
HistoricalBaselineMetrics,
} from '../../shared/constants/benchmarks';
import { EXTENSION_BENCHMARK_STATS_MAIN_PERFORMANCE_DATA_URL } from './utils';
import { getBenchmarkStatsUrl } from './utils';

type NestedPresetEntry = Record<string, Partial<BenchmarkResults>>;

Expand Down Expand Up @@ -68,9 +68,7 @@ export async function fetchHistoricalPerformanceDataFromMain(
mockMode: BenchmarkMockMode,
): Promise<HistoricalBaselineResult | null> {
try {
const response = await fetch(
EXTENSION_BENCHMARK_STATS_MAIN_PERFORMANCE_DATA_URL,
);
const response = await fetch(getBenchmarkStatsUrl(mockMode));
if (!response.ok) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import type {
} from './historical-comparison';
import { fetchHistoricalPerformanceDataFromMain } from './historical-comparison';
import {
EXTENSION_BENCHMARK_STATS_MAIN_PERFORMANCE_DATA_URL,
getBenchmarkStatsUrl,
resolveBaseline,
buildEntryKey,
buildCombo,
Expand Down Expand Up @@ -1243,7 +1243,10 @@ export async function buildPerformanceBenchmarksSection(
// same source the harness reads keeps that agreement a property of this call
// rather than of the trigger config.
fetchHistoricalPerformanceDataFromMain(
resolveBenchmarkMockMode(process.env.GITHUB_REF_NAME),
resolveBenchmarkMockMode(
process.env.GITHUB_REF_NAME,
process.env.BENCHMARK_MOCK_MODE,
),
),
]);

Expand Down Expand Up @@ -1323,7 +1326,14 @@ export async function buildPerformanceBenchmarksSection(
const pipelineLink = runUrl
? `<a href="${runUrl}">${benchmarkRunId}</a>`
: (benchmarkRunId ?? '');
const baselineLogsLink = `<a href="${EXTENSION_BENCHMARK_STATS_MAIN_PERFORMANCE_DATA_URL}">Baseline logs</a>`;
// Point at the series this run actually compared against, not always the
// live one — otherwise the link contradicts the numbers beside it.
const baselineLogsLink = `<a href="${getBenchmarkStatsUrl(
resolveBenchmarkMockMode(
process.env.GITHUB_REF_NAME,
process.env.BENCHMARK_MOCK_MODE,
),
)}">Baseline logs</a>`;
const commitInfo = `\n\n<p><strong>Baseline (latest main)</strong>: ${commitLink} | <strong>Date</strong>: ${commitDate} | <strong>Pipeline</strong>: ${pipelineLink} | ${baselineLogsLink}</p>\n\n`;

// Plain text only inside <summary> (no block elements like <p>).
Expand Down
24 changes: 24 additions & 0 deletions development/metamaskbot-build-announce/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,36 @@
import {
BENCHMARK_PLATFORMS,
BENCHMARK_BUILD_TYPES,
BENCHMARK_MOCK_MODE,
} from '../../shared/constants/benchmarks';
import type { BenchmarkMockMode } from '../../shared/constants/benchmarks';
import type { HistoricalBaselineReference } from './historical-comparison';

export const EXTENSION_BENCHMARK_STATS_MAIN_PERFORMANCE_DATA_URL =
'https://raw.githubusercontent.com/MetaMask/extension_benchmark_stats/main/stats/main/performance_data.json';

export const EXTENSION_BENCHMARK_STATS_MAIN_PERFORMANCE_DATA_MOCKED_URL =
'https://raw.githubusercontent.com/MetaMask/extension_benchmark_stats/main/stats/main/performance_data_mocked.json';

/**
* URL of the published series for a population.
*
* The two populations are written to separate files by
* `benchmark-stats-commit.sh`, so the reader has to select the same one the
* run measured. Selecting only with the in-file `mockMode` filter is not
* enough: a `mocked` consumer pointed at the live file finds no matching
* entries and reports no baseline forever, which silently defeats publishing
* the mocked series at all.
*
* @param mockMode - Population the consuming run measured.
* @returns Raw URL of that population's series.
*/
export function getBenchmarkStatsUrl(mockMode: BenchmarkMockMode): string {
return mockMode === BENCHMARK_MOCK_MODE.MOCKED
? EXTENSION_BENCHMARK_STATS_MAIN_PERFORMANCE_DATA_MOCKED_URL
: EXTENSION_BENCHMARK_STATS_MAIN_PERFORMANCE_DATA_URL;
}

/**
* Runs a section builder and returns its result, or a "data not available"
* message for `sectionName` if the builder returns null/undefined/'' or throws.
Expand Down
30 changes: 30 additions & 0 deletions shared/constants/benchmarks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,36 @@ describe('resolveBenchmarkMockMode', () => {
expect(resolveBenchmarkMockMode('main')).toBe(BENCHMARK_MOCK_MODE.LIVE);
});

it('lets an explicit override beat the branch heuristic on main', () => {
// The per-commit path relies on this: main measures mocked so the gate
// compares one population, and the scheduled drift job passes `live`.
expect(resolveBenchmarkMockMode('main', 'mocked')).toBe(
BENCHMARK_MOCK_MODE.MOCKED,
);
expect(resolveBenchmarkMockMode('release/13.44.0', 'mocked')).toBe(
BENCHMARK_MOCK_MODE.MOCKED,
);
});

it('lets an explicit override beat the branch heuristic on a PR ref', () => {
expect(resolveBenchmarkMockMode('45147/merge', 'live')).toBe(
BENCHMARK_MOCK_MODE.LIVE,
);
});

it('falls back to the branch heuristic when the override is absent or unrecognised', () => {
// An empty string is what an unset workflow input expands to, and a typo
// must not silently pick a population — both fall through to the branch.
for (const override of [undefined, '', 'MOCKED', 'mock', 'true']) {
expect(resolveBenchmarkMockMode('main', override)).toBe(
BENCHMARK_MOCK_MODE.LIVE,
);
expect(resolveBenchmarkMockMode('45147/merge', override)).toBe(
BENCHMARK_MOCK_MODE.MOCKED,
);
}
});

it('treats release branches as the live population', () => {
expect(resolveBenchmarkMockMode('release/13.42.0')).toBe(
BENCHMARK_MOCK_MODE.LIVE,
Expand Down
28 changes: 22 additions & 6 deletions shared/constants/benchmarks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,20 +224,36 @@ export type BenchmarkMockMode =
(typeof BENCHMARK_MOCK_MODE)[keyof typeof BENCHMARK_MOCK_MODE];

/**
* Resolves the network population for a run from its branch name.
* Resolves the network population for a run.
*
* `main` and `release/*` exercise real servers (introduced by #39587 to keep
* a real-world signal on the trunk); every other ref — PRs, local dev — runs
* against the mock suite for determinism.
* An explicit `override` decides it. Failing that, the branch heuristic
* applies: `main` and `release/*` exercise real servers (introduced by #39587
* to keep a real-world signal on the trunk); every other ref — PRs, local
* dev — runs against the mock suite for determinism.
*
* Pure function of the branch name so both the benchmark harness and the
* quality gate can derive the same answer without sharing runtime state.
* Pure function of its inputs so both the benchmark harness and the quality
* gate can derive the same answer without sharing runtime state.
*
* @param branch - Branch name, e.g. `process.env.GITHUB_REF_NAME`.
* @param override - Explicit population, `mocked` or `live`, e.g.
* `process.env.BENCHMARK_MOCK_MODE`. Any other value, including the empty
* string an unset workflow input expands to, falls through to the branch.
*/
export function resolveBenchmarkMockMode(
branch: string | undefined,
override?: string | undefined,
): BenchmarkMockMode {
// An explicit override wins over the branch heuristic. The per-commit path
// sets it to `mocked` on every branch including main, so the series the gate
// compares against is one population throughout; the scheduled drift job sets
// it to `live`. The branch heuristic remains the fallback for local runs and
// for any caller that does not set it.
if (override === BENCHMARK_MOCK_MODE.MOCKED) {
return BENCHMARK_MOCK_MODE.MOCKED;
}
if (override === BENCHMARK_MOCK_MODE.LIVE) {
return BENCHMARK_MOCK_MODE.LIVE;
}
const name = branch ?? '';
const isMainOrRelease = name === 'main' || name.startsWith('release/');
return isMainOrRelease
Expand Down
5 changes: 4 additions & 1 deletion test/e2e/benchmarks/utils/mock-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import {
* @returns The mock mode for the current branch.
*/
export function getBenchmarkMockMode(): BenchmarkMockMode {
return resolveBenchmarkMockMode(process.env.GITHUB_REF_NAME);
return resolveBenchmarkMockMode(
process.env.GITHUB_REF_NAME,
process.env.BENCHMARK_MOCK_MODE,
);
}

/**
Expand Down
Loading