Add support for vernier profile vieweing

Adds RemoteFirefoxViewer which uses the Firefox Profiler via middleware
to serve profiles.

- Fixes deprecation syntax
- Fixes several warnings
- Deprecates AppProfiler.viewer
- Removes deprecard AppProfiler::Profile constant
- Fixes directory structure to make files easier to find

Co-authored-by: Gannon McGibbon <gannon.mcgibbon@shopify.com>

Author: dalehamel

.rubocop.yml

@@ -5,3 +5,6 @@ require: rubocop-performance
 
 Style/StringLiterals:
   EnforcedStyle: double_quotes
+
+AllCops:
+    SuggestExtensions: false

README.md

@@ -290,11 +290,16 @@ report = AppProfiler.run(mode: :cpu) do
   # ...
 end
 
-report.view # opens the profile locally in speedscope.
+report.view # opens the profile locally in speedscope by default
 ```
 
 Profile files can be found locally in your rails app at `tmp/app_profiler/*.json`.
 
+**Note** In development, if using the `AppProfiler::Viewer::SpeedscopeRemoteViewer` for stackprof
+or if using Vernier, a route for `/app_profiler` will be added to the application.
+If using Vernier, a route for `/from-url` is also added. These will be handled
+in middlewares, before any application routing logic. There is a small chance
+that these could shadow existing routes in the application.
 
 ## Storage backends
 
@@ -336,6 +341,8 @@ Rails.application.config.app_profiler.backend = AppProfiler::StackprofBackend #
 
 By default, the stackprof backend will be used.
 
+In local development, changing the backend will change whether the profile is viewed in Speedscope or Firefox Profiler.
+
 
 ## Profile Sampling
 

lib/app_profiler.rb

@@ -30,16 +30,20 @@ module Storage
   module Viewer
     autoload :BaseViewer, "app_profiler/viewer/base_viewer"
     autoload :SpeedscopeViewer, "app_profiler/viewer/speedscope_viewer"
+    autoload :BaseMiddleware, "app_profiler/viewer/base_middleware"
     autoload :SpeedscopeRemoteViewer, "app_profiler/viewer/speedscope_remote_viewer"
+    autoload :FirefoxRemoteViewer, "app_profiler/viewer/firefox_remote_viewer"
   end
 
-  require "app_profiler/middleware"
-  require "app_profiler/parameters"
-  require "app_profiler/request_parameters"
-  require "app_profiler/profile"
-  require "app_profiler/backend"
-  require "app_profiler/server"
-  require "app_profiler/sampler"
+  autoload(:Middleware, "app_profiler/middleware")
+  autoload(:Parameters, "app_profiler/parameters")
+  autoload(:RequestParameters, "app_profiler/request_parameters")
+  autoload(:BaseProfile, "app_profiler/base_profile")
+  autoload :StackprofProfile, "app_profiler/stackprof_profile"
+  autoload :VernierProfile, "app_profiler/vernier_profile"
+  autoload(:Backend, "app_profiler/backend")
+  autoload(:Server, "app_profiler/server")
+  autoload(:Sampler, "app_profiler/sampler")
 
   mattr_accessor :logger, default: Logger.new($stdout)
   mattr_accessor :root
@@ -50,11 +54,10 @@ module Viewer
   mattr_reader   :profile_header, default: "X-Profile"
   mattr_accessor :profile_async_header, default: "X-Profile-Async"
   mattr_accessor :context, default: nil
-  mattr_reader   :profile_url_formatter,
-    default: DefaultProfileFormatter
-
+  mattr_reader   :profile_url_formatter, default: DefaultProfileFormatter
   mattr_accessor :storage, default: Storage::FileStorage
-  mattr_accessor :viewer, default: Viewer::SpeedscopeViewer
+  mattr_writer :stackprof_viewer, default: nil
+  mattr_writer :vernier_viewer, default: nil
   mattr_accessor :middleware, default: Middleware
   mattr_accessor :server, default: Server
   mattr_accessor :upload_queue_max_length, default: 10
@@ -68,6 +71,10 @@ module Viewer
   mattr_accessor :profile_sampler_config
 
   class << self
+    def deprecator # :nodoc:
+      @deprecator ||= ActiveSupport::Deprecation.new("in future releases", "app_profiler")
+    end
+
     def run(*args, backend: nil, **kwargs, &block)
       orig_backend = self.backend
       begin
@@ -117,6 +124,14 @@ def backend=(new_backend)
       @profiler_backend = new_profiler_backend
     end
 
+    def stackprof_viewer
+      @@stackprof_viewer ||= Viewer::SpeedscopeViewer # rubocop:disable Style/ClassVars
+    end
+
+    def vernier_viewer
+      @@vernier_viewer ||= Viewer::FirefoxRemoteViewer # rubocop:disable Style/ClassVars
+    end
+
     def profile_sampler_enabled=(value)
       if value.is_a?(Proc)
         raise ArgumentError,
@@ -207,6 +222,16 @@ def profile_url(upload)
       AppProfiler.profile_url_formatter.call(upload)
     end
 
+    def viewer
+      deprecator.warn("AppProfiler.viewer is deprecated, please use stackprof_viewer instead.")
+      stackprof_viewer
+    end
+
+    def viewer=(viewer)
+      deprecator.warn("AppProfiler.viewer= is deprecated, please use stackprof_viewer= instead.")
+      self.stackprof_viewer = viewer
+    end
+
     private
 
     def clear

lib/app_profiler/profile.rb lib/app_profiler/base_profile.rblib/app_profiler/profile.rb renamed to lib/app_profiler/base_profile.rb

@@ -3,9 +3,6 @@
 require "active_support/deprecation/constant_accessor"
 
 module AppProfiler
-  autoload :StackprofProfile, "app_profiler/profile/stackprof"
-  autoload :VernierProfile, "app_profiler/profile/vernier"
-
   class BaseProfile
     INTERNAL_METADATA_KEYS = [:id, :context]
     private_constant :INTERNAL_METADATA_KEYS
@@ -111,7 +108,4 @@ def path
       AppProfiler.profile_root.join(filename)
     end
   end
-
-  include ActiveSupport::Deprecation::DeprecatedConstantAccessor
-  deprecate_constant "Profile", "AppProfiler::BaseProfile", deprecator: ActiveSupport::Deprecation.new
 end

lib/app_profiler/middleware.rb

@@ -4,7 +4,6 @@
 require "app_profiler/middleware/base_action"
 require "app_profiler/middleware/upload_action"
 require "app_profiler/middleware/view_action"
-require "app_profiler/sampler/config"
 
 module AppProfiler
   class Middleware

lib/app_profiler/railtie.rb

@@ -11,7 +11,12 @@ class Railtie < Rails::Railtie
       AppProfiler.logger = app.config.app_profiler.logger || Rails.logger
       AppProfiler.root = app.config.app_profiler.root || Rails.root
       AppProfiler.storage = app.config.app_profiler.storage || Storage::FileStorage
-      AppProfiler.viewer = app.config.app_profiler.viewer || Viewer::SpeedscopeViewer
+      if app.config.app_profiler.stackprof_viewer
+        AppProfiler.stackprof_viewer = app.config.app_profiler.stackprof_viewer
+      end
+      if app.config.app_profiler.vernier_viewer
+        AppProfiler.vernier_viewer = app.config.app_profiler.vernier_viewer
+      end
       AppProfiler.storage.bucket_name = app.config.app_profiler.storage_bucket_name || "profiles"
       AppProfiler.storage.credentials = app.config.app_profiler.storage_credentials || {}
       AppProfiler.middleware = app.config.app_profiler.middleware || Middleware
@@ -49,8 +54,8 @@ class Railtie < Rails::Railtie
 
     initializer "app_profiler.add_middleware" do |app|
       unless AppProfiler.middleware.disabled
-        if AppProfiler.viewer == Viewer::SpeedscopeRemoteViewer
-          app.middleware.insert_before(0, Viewer::SpeedscopeRemoteViewer::Middleware)
+        if (Rails.env.development? || Rails.env.test?) && AppProfiler.stackprof_viewer.remote?
+          app.middleware.insert_before(0, AppProfiler.viewer::Middleware)
         end
         app.middleware.insert_before(0, AppProfiler.middleware)
       end

lib/app_profiler/sampler.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "app_profiler/sampler/config"
+
 module AppProfiler
   module Sampler
     @excluded_cache = {}

lib/app_profiler/sampler/config.rb

@@ -2,6 +2,7 @@
 
 require "app_profiler/sampler/stackprof_config"
 require "app_profiler/sampler/vernier_config"
+
 module AppProfiler
   module Sampler
     class Config
@@ -27,7 +28,7 @@ def initialize(sample_rate: SAMPLE_RATE,
 
         raise ArgumentError, "mode probabilities must sum to 1" unless backends_probability.values.sum == 1.0
 
-        ActiveSupport::Deprecation.new.warn("passing paths is deprecated, use targets instead") if paths
+        AppProfiler.deprecator.warn("passing paths is deprecated, use targets instead") if paths
 
         @sample_rate = sample_rate
         @targets = paths || targets

lib/app_profiler/profile/stackprof.rb lib/app_profiler/stackprof_profile.rblib/app_profiler/profile/stackprof.rb renamed to lib/app_profiler/stackprof_profile.rb

@@ -2,7 +2,7 @@
 
 module AppProfiler
   class StackprofProfile < BaseProfile
-    FILE_EXTENSION = ".json"
+    FILE_EXTENSION = ".stackprof.json"
 
     def mode
       @data[:mode]
@@ -17,7 +17,7 @@ def format
     end
 
     def view(params = {})
-      AppProfiler.viewer.view(self, **params)
+      AppProfiler.stackprof_viewer.view(self, **params)
     end
   end
 end

lib/app_profiler/profile/vernier.rb lib/app_profiler/vernier_profile.rblib/app_profiler/profile/vernier.rb renamed to lib/app_profiler/vernier_profile.rb

@@ -2,7 +2,7 @@
 
 module AppProfiler
   class VernierProfile < BaseProfile
-    FILE_EXTENSION = ".gecko.json"
+    FILE_EXTENSION = ".vernier.json"
 
     def mode
       @data[:meta][:mode]
@@ -17,7 +17,7 @@ def format
     end
 
     def view(params = {})
-      raise NotImplementedError
+      AppProfiler.vernier_viewer.view(self, **params)
     end
   end
 end