Skip to content

[No QA] feat: add native span benchmark tooling - #98144

Merged
roryabraham merged 76 commits into
Expensify:mainfrom
margelo:codex/benchmark-sentry-spans
Sep 8, 2026
Merged

roryabraham merged 76 commits into
Expensify:mainfrom
margelo:codex/benchmark-sentry-spans

Conversation

@chrispader

@chrispader chrispader commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Release builds strip ordinary console logging and require different device-control paths on Android and iOS, so startup measurements have depended on one-off commands and results that are difficult to reproduce. Comparing two builds is especially unreliable when they run at different times on a changing device. This PR adds opt-in Sentry span output, repeatable startup benchmarks, and local bootstrap tooling for side-by-side Android and iOS release builds.

@roryabraham @mountiny @tgolen

Explanation of Change

  • Developers list exact span names in the git-ignored root .env with EXPO_PUBLIC_BENCHMARK_SENTRY_SPANS. Successful, non-canceled spans emit a machine-readable [EXPENSIFY_BENCHMARK] JSON event. iOS also writes one marker per span to the app container because CoreDevice does not expose an equivalent stream of release app logs.
  • benchmark-app-startup runs one warm-up followed by repeated process or true-cold launches. It can collect every configured span or one selected span, use a fixed collection window or stop after a chosen span, warn about incomplete samples, and write both raw samples and Average, P50, P75, P90, P95, P99, Min, and Max results as CSV.
  • The results command recalculates statistics from existing sample files. Alternating comparison mode interleaves two installed apps in A/B order and keeps their samples and summaries separate, which reduces bias from device temperature and other conditions that change during a run.
  • bootstrap-device prepares uniquely identified local builds that can coexist on one device. The Android adapter updates application IDs, local signing, Firebase clients, shortcuts, manifests, and launcher labels. The iOS adapter updates Xcode targets, signing teams, plist values, app-group entitlements, and launcher labels. Unless an identifier is supplied, the script asks gh for the current user. The GitHub CLI checks GH_TOKEN, then GITHUB_TOKEN, then its stored active account. If gh is not installed, the script uses Octokit with GH_TOKEN.
  • Thin Bun entrypoints are separate from CLI parsing, benchmark orchestration, statistics, and the Android and iOS adapters. The scripts use Bun's file, glob, write, and shell APIs directly, while retaining Node compatibility APIs only for path, temporary-directory, filesystem-removal, and readline behavior that Bun does not provide directly.
  • A scripts-specific tsconfig.json extends the repository's Bun configuration so editors load the same Bun globals and module types used by the tooling typecheck. Knip ignores the OpenSSL executable used to inspect provisioning profiles instead of reporting it as an undeclared package binary.
  • CoreDevice and Google Services JSON stays typed as JsonValue until the required fields pass runtime validation. The script files also document their responsibilities, and non-obvious helpers have JSDoc comments.
  • Span durations use one monotonic clock after translating explicit Sentry start timestamps. contributingGuides/BENCHMARKING.md documents configuration, side-by-side builds, GitHub username resolution, collection modes, comparisons, result exports, and reusable library entrypoints.

Fixed Issues

$ #98280
PROPOSAL: #98280

Tests

  1. Add at least two exact span names to the git-ignored root .env:
    EXPO_PUBLIC_BENCHMARK_SENTRY_SPANS=ManualAppStartup,ManualAppStartupNetworkRequest
  2. Run gh api user --jq .login to note the GitHub user selected by the CLI. Run nr bootstrap-device android --suffix candidate or nr bootstrap-device ios --suffix candidate, then verify the printed application identifier contains that username. Pass --github-username and verify the printed identifier uses the override instead. Build and install the release app from Mobile-Expensify.
  3. Verify the installed launcher name contains the suffix, for example Expensify (candidate), and that it can be installed beside the app without that suffix.
  4. Run the benchmark on the connected device with a fixed collection window:
    nr benchmark-app-startup ios 20 --device "Developer's iPhone" --wait-time 30
    Add --app-id with the identifier printed by the bootstrap script. On Android, use android and the device serial instead. Do not set --wait-until-span for this multi-span test, since the network span can finish after startup.
  5. Verify the script prints the resolved platform, device, application identifier, spans, mode, run count, wait settings, raw sample path, and results path before launching the app.
  6. Verify one unmeasured warm-up is followed by 20 measured launches. Confirm the final console table contains one row per configured span, its runs column equals the number of collected samples, and the matching *-results.csv file contains the displayed aggregate values. The raw sample CSV must contain only observed samples. Any metric with fewer than 20 samples must produce a warning showing collected/requested counts and the missing count. A metric with all 20 samples must not produce that warning.
  7. Verify intentional early stopping separately by rerunning with --span ManualAppStartup --wait-until-span ManualAppStartup. Confirm collection ends once startup is observed instead of waiting for the full window. This mode does not promise to collect spans that finish later.
  8. Test the zero-sample warning by temporarily adding BenchmarkNeverEmitted to the allowlist without adding an app span with that name. Rebuild and reinstall, then run two measured launches with --wait-time 1 and neither --span nor --wait-until-span. Confirm the warning includes BenchmarkNeverEmitted: 0/2 samples collected; 2 missing, its console and results CSV rows show runs=0 and N/A statistics, and the raw sample CSV contains no rows for it. Restore the original allowlist and rebuild afterward.
  9. Recreate a results table from the raw samples:
    nr benchmark-app-startup results --input-files .benchmarks/sample-a.csv,.benchmarks/sample-b.csv --results-output .benchmarks/results.csv
    Verify the command combines samples by span, prints their statistics, and writes the same values to .benchmarks/results.csv. Repeat without --results-output and verify it writes .benchmarks/results.csv by default.
  10. Run the results command with an invalid sample header and verify it exits with a clear error. Repeat a live benchmark with a --span value absent from the allowlist and verify it exits without launching the app.
  11. Install two bootstrapped builds with different identifiers. Run alternating mode with --app-id-a and --app-id-b, then verify the terminal alternates A and B for every cycle and produces separate raw and results CSV files for both binaries. Repeat the missing-span test and confirm each warning identifies binary A or B and uses that binary's sample count.
  12. Verify no unexpected errors appear in the JS, device, or benchmark console. The [EXPENSIFY_BENCHMARK] output and incomplete-sample warnings from the deliberate missing-span test are expected.
  • Verify that no unexpected errors appear in the JS, device, or benchmark console apart from the expected benchmark output and deliberately triggered incomplete-sample warnings.

Offline tests

Offline testing was not performed. The change adds local release-build logging and device automation without changing network behavior. Running a benchmark offline would measure the existing offline behavior of the selected span, not behavior introduced by this PR.

QA Steps

This PR cannot be tested in a standard staging or production build. The span allowlist, app identifiers, and signing changes are supplied while producing a local release build, and QA does not have access to that build workflow or the required command-line tooling.

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

No screenshots or videos are included because the benchmark has no in-app UI. The iOS launcher-name suffix was verified locally as part of the developer setup flow.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ This PR is possibly changing native code and/or updating libraries, it may cause problems with HybridApp. Please check if any patch updates are required in the HybridApp repo and run an AdHoc build to verify that HybridApp will not break. Ask Contributor Plus for help if you are not sure how to handle this. ⚠️

@codecov

codecov Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/libs/telemetry/activeSpans.ts 98.38% <100.00%> (+1.77%) ⬆️
src/libs/telemetry/logBenchmarkSpanEnd.ts 88.88% <88.88%> (ø)
src/libs/telemetry/writeBenchmarkLog/index.ios.ts 77.77% <77.77%> (ø)
src/libs/telemetry/writeBenchmarkLog/index.ts 0.00% <0.00%> (ø)
src/types/utils/JSONUtils.ts 0.00% <0.00%> (ø)
... and 227 files with indirect coverage changes

@chrispader
chrispader force-pushed the codex/benchmark-sentry-spans branch from 36666c0 to 24d3bc5 Compare August 12, 2026 15:11
@quinthar quinthar removed the #quality label Aug 13, 2026
@chrispader chrispader changed the title feat: add native span benchmark tooling [No QA] feat: add native span benchmark tooling Aug 24, 2026
@chrispader
chrispader marked this pull request as ready for review August 24, 2026 19:41
@chrispader
chrispader requested a review from a team as a code owner August 24, 2026 19:41
@chrispader

Copy link
Copy Markdown
Contributor Author

@roryabraham the PR is ready for review again, but we'll need to merge this one first, to make Knip work with Bun shell interpolation

@roryabraham

Copy link
Copy Markdown
Contributor

#100329 merged

roryabraham
roryabraham previously approved these changes Sep 4, 2026
debug: 'build-types/debug/res/xml-v25/shortcuts.xml',
adhoc: 'build-types/adhoc/res/xml-v25/shortcuts.xml',
} as const;
for (const buildType of ['release', 'debug', 'adhoc'] as const) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: Do we need to patch all build types?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm changing this, so that by default only release gets patched and other build types can be configured manually

Comment thread scripts/benchmarkAppStartup.ts Resolved
Comment thread scripts/bootstrapForDevice.ts Resolved
@roryabraham

Copy link
Copy Markdown
Contributor

didn't see any blockers. Let's merge main to ensure knip passes after the upgrade

@roryabraham

Copy link
Copy Markdown
Contributor

@chrispader knip is still failing

@chrispader

Copy link
Copy Markdown
Contributor Author

@roryabraham sorry forgot to push the main merge. Knip check is succeeding.

The PR is now ready for final review and merge!

@roryabraham
roryabraham merged commit dc956a9 into Expensify:main Sep 8, 2026
42 of 43 checks passed
@OSBotify

OSBotify commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify OSBotify mentioned this pull request Sep 9, 2026
83 tasks done
@OSBotify

OSBotify commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/roryabraham in version: 9.4.75-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

MelvinBot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

🤖 No help site changes required — no draft PR created.

This PR is developer-only tooling. Nothing in it changes behavior a customer can see, so there is nothing to document in App/docs/articles.

What I checked

The changed files are all internal:

Area Files Audience
Benchmark CLI scripts/benchmarkAppStartup.ts, scripts/lib/benchmark*, scripts/lib/nativeAppBenchmark/* Developers
Side-by-side build bootstrap scripts/bootstrapForDevice.ts, scripts/lib/bootstrapForDevice/* Developers
Opt-in Sentry span logging src/libs/telemetry/logBenchmarkSpanEnd.ts, src/libs/telemetry/writeBenchmarkLog/*, src/libs/telemetry/activeSpans.ts Developers
Config / tooling .env.example, .gitignore, knip.json, package.json, scripts/tsconfig.json Developers
Docs contributingGuides/BENCHMARKING.md Developers
Tests tests/tooling/*, tests/unit/* Developers

Why none of it belongs on the help site:

  • The span output is opt-in and gated behind EXPO_PUBLIC_BENCHMARK_SENTRY_SPANS in a git-ignored root .env. Customers never set it, and it produces no UI.
  • bootstrap-device only affects local release builds on a developer's own device.
  • The PR already documents itself in the right place: contributingGuides/BENCHMARKING.md, which is the contributor guide, not the help site.
  • The PR is marked [No QA] and its own QA Steps say it can't be tested in a staging or production build.

Confirmed the help site has no matching content: a case-insensitive search of App/docs/articles for sentry, benchmark, and telemetry returned zero matches. There is no existing article this change would make stale.

@chrispader, no help site PR was created because no article changes are needed. If you think a customer-facing behavior changed here that I missed, reply with @MelvinBot and the detail and I'll draft the docs PR.


view run · view recording

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.75-3 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 failure ❌
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.75-3 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 failure ❌
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

@daledah

daledah commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

@MelvinBot, please create a payment issue for me since I was the C+ reviewer for this PR.

@MelvinBot

Copy link
Copy Markdown
Contributor

I created the payment issue: [Due for payment 2026-09-17] C+ payment for reviewing https://github.com/Expensify/App/pull/98144.

You (daledah) and roryabraham, who merged this PR, are assigned. I confirmed that you completed the reviewer checklist and approved the PR. The PR reached production on 2026-09-10, so the 7-day regression period has already ended.


view run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants