app_profiler icon indicating copy to clipboard operation
app_profiler copied to clipboard
verified publisher icon Shopify

Add firefox viewer, simplify speedscope viewer options

Open dalehamel opened this issue 1 year ago • 2 comments

Depends on https://github.com/Shopify/app_profiler/pull/101

This adds firefox-profiler as an optional viewer, for viewing vernier's "gecko" profiles.

By default, it will use the upstream repo https://github.com/firefox-devtools/profiler, but it is possible to override this and use the ruby-specific fork fairly easily by setting:

config.app_profiler.gecko_viewer_package = "https://github.com/tenderlove/profiler#ruby"

The remote viewer no longer loads the profile via a blob in javascript, it uses the #profileURL parameter and encodes a path that hosts the profile. This lets the client handle fetching the profile

Both the firefox and speedscope remote middlewares are added, and both are lazily initialized. Note that the firefox viewer takes significantly longer to compile. Thus it is no longer necessary (or possible) to explicitly configure the viewer to use. The speedscope one is only added if the remote viewer is set.

The firefox viewer also cannot be as easily initialized, as the upstream project does not facilitate this by setting a name and version in their package.json. For this reason, the repo is instead cloned and the contents are patched. In a future PR, the option to fetch a pre-compiled blob probably makes sense to avoid this complexity which is likely to be error prone.

🎩

To test this, configure an app to have the vernier gem and take a profile by setting backend=vernier in the URL parameter. It should automatically redirect to firefox profiler, after lazily initializing it.

Likewise, browsing profiles at /app_profiler/ should open speedscope for stackprof profiles, and firefox-profiler for gecko profiles.

dalehamel avatar Jan 24 '24 20:01 dalehamel

notes from pairing:

  • [x] Bring back the local viewer
  • [x] Rename current speedscope viewer back to remote, corresponding name change for firefox-profiler also
  • [x] Documentation notice about /from-url path being taken for firefox-profiler
  • [x] Bring back AppProfiler.viewer, but rename and alias AppProfiler.speedscope_viewer and have deprecation notice for usage of AppProfiler.viewer

dalehamel avatar Feb 05 '24 19:02 dalehamel

@gmcgibbon this one should be good for another look too, I believe I addressed everything from your prior review and our pairing ❤️

dalehamel avatar Feb 09 '24 18:02 dalehamel