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 <[email protected]>

.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.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.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.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

lib/app_profiler/viewer/base_middleware.rb

@@ -0,0 +1,141 @@
+# frozen_string_literal: true
+
+gem "rails-html-sanitizer", ">= 1.6.0"
+require "rails-html-sanitizer"
+
+module AppProfiler
+  module Viewer
+    class BaseMiddleware
+      class Sanitizer < Rails::HTML::Sanitizer.best_supported_vendor.safe_list_sanitizer
+        self.allowed_tags = Set.new([
+          "strong",
+          "em",
+          "b",
+          "i",
+          "p",
+          "code",
+          "pre",
+          "tt",
+          "samp",
+          "kbd",
+          "var",
+          "sub",
+          "sup",
+          "dfn",
+          "cite",
+          "big",
+          "small",
+          "address",
+          "hr",
+          "br",
+          "div",
+          "span",
+          "h1",
+          "h2",
+          "h3",
+          "h4",
+          "h5",
+          "h6",
+          "ul",
+          "ol",
+          "li",
+          "dl",
+          "dt",
+          "dd",
+          "abbr",
+          "acronym",
+          "a",
+          "img",
+          "blockquote",
+          "del",
+          "ins",
+          "script",
+        ])
+      end
+
+      private_constant(:Sanitizer)
+
+      class << self
+        def id(file)
+          file.basename.to_s
+        end
+      end
+
+      def initialize(app)
+        @app = app
+      end
+
+      def call(env)
+        request = Rack::Request.new(env)
+
+        return index(env) if %r(\A/app_profiler/?\z).match?(request.path_info)
+
+        @app.call(env)
+      end
+
+      protected
+
+      def id(file)
+        self.class.id(file)
+      end
+
+      def profile_files
+        AppProfiler.profile_root.glob("**/*.json")
+      end
+
+      def render(html)
+        [
+          200,
+          { "Content-Type" => "text/html" },
+          [
+            +<<~HTML,
+              <!doctype html>
+              <html>
+                <head>
+                  <title>App Profiler</title>
+                </head>
+                <body>
+                  #{sanitizer.sanitize(html)}
+                </body>
+              </html>
+            HTML
+          ],
+        ]
+      end
+
+      def sanitizer
+        @sanitizer ||= Sanitizer.new
+      end
+
+      def viewer(_env, path)
+        raise NotImplementedError
+      end
+
+      def show(env, id)
+        raise NotImplementedError
+      end
+
+      def index(_env)
+        render(
+          (+"").tap do |content|
+            content << "<h1>Profiles</h1>"
+            profile_files.each do |file|
+              viewer = if file.to_s.end_with?(AppProfiler::VernierProfile::FILE_EXTENSION)
+                AppProfiler::Viewer::FirefoxRemoteViewer::NAME
+              else
+                AppProfiler::Viewer::SpeedscopeRemoteViewer::NAME
+              end
+              content << <<~HTML
+                <p>
+                  <a href="/app_profiler/#{viewer}/viewer/#{id(file)}">
+                    #{id(file)}
+                  </a>
+                </p>
+              HTML
+            end
+          end,
+        )
+      end
+    end
+  end
+end

lib/app_profiler/viewer/base_viewer.rb

@@ -7,10 +7,10 @@ class << self
         def view(profile, params = {})
           new(profile).view(**params)
         end
-      end
 
-      def view(_params = {})
-        raise NotImplementedError
+        def remote?
+          false
+        end
       end
     end
   end