From 148aa55349a27773e4bb5d876fb856ba229699bb Mon Sep 17 00:00:00 2001 From: Dale Hamel Date: Mon, 22 Jan 2024 15:06:06 -0500 Subject: [PATCH] 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 --- .rubocop.yml | 3 + README.md | 9 +- lib/app_profiler.rb | 47 ++++-- .../{profile.rb => base_profile.rb} | 6 - lib/app_profiler/middleware.rb | 1 - lib/app_profiler/railtie.rb | 11 +- lib/app_profiler/sampler.rb | 1 + lib/app_profiler/sampler/config.rb | 3 +- .../stackprof.rb => stackprof_profile.rb} | 4 +- .../vernier.rb => vernier_profile.rb} | 4 +- lib/app_profiler/viewer/base_middleware.rb | 141 +++++++++++++++++ lib/app_profiler/viewer/base_viewer.rb | 6 +- .../viewer/firefox_remote_viewer.rb | 33 ++++ .../firefox_remote_viewer/middleware.rb | 66 ++++++++ .../viewer/speedscope_remote_viewer.rb | 16 +- .../base_middleware.rb | 142 ------------------ .../speedscope_remote_viewer/middleware.rb | 31 ++-- lib/app_profiler/viewer/speedscope_viewer.rb | 6 - lib/app_profiler/yarn/command.rb | 16 +- .../yarn/with_firefox_profiler.rb | 73 +++++++++ .../app_profiler_configuration_test.rb | 1 + test/app_profiler/sampler/config_test.rb | 6 +- .../storage/google_cloud_storage_test.rb | 2 +- .../viewer/remote/firefox_viewer_test.rb | 42 ++++++ .../middleware/firefox_middleware_test.rb | 117 +++++++++++++++ .../middleware/speedscope_middleware_test.rb} | 45 ++---- .../speedscope_viewer_test.rb} | 2 +- .../viewer/speedscope_viewer_test.rb | 19 +-- test/app_profiler/yarn/command_test.rb | 4 +- test/test_helper.rb | 8 +- 30 files changed, 610 insertions(+), 255 deletions(-) rename lib/app_profiler/{profile.rb => base_profile.rb} (90%) rename lib/app_profiler/{profile/stackprof.rb => stackprof_profile.rb} (73%) rename lib/app_profiler/{profile/vernier.rb => vernier_profile.rb} (74%) create mode 100644 lib/app_profiler/viewer/base_middleware.rb create mode 100644 lib/app_profiler/viewer/firefox_remote_viewer.rb create mode 100644 lib/app_profiler/viewer/firefox_remote_viewer/middleware.rb delete mode 100644 lib/app_profiler/viewer/speedscope_remote_viewer/base_middleware.rb create mode 100644 lib/app_profiler/yarn/with_firefox_profiler.rb create mode 100644 test/app_profiler/viewer/remote/firefox_viewer_test.rb create mode 100644 test/app_profiler/viewer/remote/middleware/firefox_middleware_test.rb rename test/app_profiler/viewer/{speedscope_remote_viewer/middleware_test.rb => remote/middleware/speedscope_middleware_test.rb} (65%) rename test/app_profiler/viewer/{speedscope_remote_viewer_test.rb => remote/speedscope_viewer_test.rb} (92%) diff --git a/.rubocop.yml b/.rubocop.yml index 120e5ec1..d9bcb596 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,3 +5,6 @@ require: rubocop-performance Style/StringLiterals: EnforcedStyle: double_quotes + +AllCops: + SuggestExtensions: false diff --git a/README.md b/README.md index b983227b..c16d5ba5 100644 --- a/README.md +++ b/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 diff --git a/lib/app_profiler.rb b/lib/app_profiler.rb index 8da43197..262c9074 100644 --- a/lib/app_profiler.rb +++ b/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 diff --git a/lib/app_profiler/profile.rb b/lib/app_profiler/base_profile.rb similarity index 90% rename from lib/app_profiler/profile.rb rename to lib/app_profiler/base_profile.rb index a033dad0..5a148f6e 100644 --- a/lib/app_profiler/profile.rb +++ b/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 diff --git a/lib/app_profiler/middleware.rb b/lib/app_profiler/middleware.rb index bd5a63ee..5e37254e 100644 --- a/lib/app_profiler/middleware.rb +++ b/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 diff --git a/lib/app_profiler/railtie.rb b/lib/app_profiler/railtie.rb index 7e616b94..eac79900 100644 --- a/lib/app_profiler/railtie.rb +++ b/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 diff --git a/lib/app_profiler/sampler.rb b/lib/app_profiler/sampler.rb index 6ab509e1..82dead47 100644 --- a/lib/app_profiler/sampler.rb +++ b/lib/app_profiler/sampler.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "app_profiler/sampler/config" + module AppProfiler module Sampler @excluded_cache = {} diff --git a/lib/app_profiler/sampler/config.rb b/lib/app_profiler/sampler/config.rb index ee3abe3c..3676b6cd 100644 --- a/lib/app_profiler/sampler/config.rb +++ b/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 diff --git a/lib/app_profiler/profile/stackprof.rb b/lib/app_profiler/stackprof_profile.rb similarity index 73% rename from lib/app_profiler/profile/stackprof.rb rename to lib/app_profiler/stackprof_profile.rb index 182eba6a..9b040089 100644 --- a/lib/app_profiler/profile/stackprof.rb +++ b/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 diff --git a/lib/app_profiler/profile/vernier.rb b/lib/app_profiler/vernier_profile.rb similarity index 74% rename from lib/app_profiler/profile/vernier.rb rename to lib/app_profiler/vernier_profile.rb index d889e27f..4f0c760b 100644 --- a/lib/app_profiler/profile/vernier.rb +++ b/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 diff --git a/lib/app_profiler/viewer/base_middleware.rb b/lib/app_profiler/viewer/base_middleware.rb new file mode 100644 index 00000000..7f9c3949 --- /dev/null +++ b/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, + + + + App Profiler + + + #{sanitizer.sanitize(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 << "

Profiles

" + 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 +

+ + #{id(file)} + +

+ HTML + end + end, + ) + end + end + end +end diff --git a/lib/app_profiler/viewer/base_viewer.rb b/lib/app_profiler/viewer/base_viewer.rb index 98f06618..1bb541f4 100644 --- a/lib/app_profiler/viewer/base_viewer.rb +++ b/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 diff --git a/lib/app_profiler/viewer/firefox_remote_viewer.rb b/lib/app_profiler/viewer/firefox_remote_viewer.rb new file mode 100644 index 00000000..f8b0bf83 --- /dev/null +++ b/lib/app_profiler/viewer/firefox_remote_viewer.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "app_profiler/viewer/firefox_remote_viewer/middleware" + +module AppProfiler + module Viewer + class FirefoxRemoteViewer < BaseViewer + NAME = "firefox" + + class << self + def remote? + true + end + end + + def initialize(profile) + super() + @profile = profile + end + + def view(response: nil, autoredirect: nil, async: false) + id = Middleware.id(@profile.file) + + if response && response[0].to_i < 500 + response[1]["Location"] = "/app_profiler/#{NAME}/viewer/#{id}" + response[0] = 303 + else + AppProfiler.logger.info("[Profiler] Profile available at /app_profiler/#{id}\n") + end + end + end + end +end diff --git a/lib/app_profiler/viewer/firefox_remote_viewer/middleware.rb b/lib/app_profiler/viewer/firefox_remote_viewer/middleware.rb new file mode 100644 index 00000000..5a14bb3d --- /dev/null +++ b/lib/app_profiler/viewer/firefox_remote_viewer/middleware.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require "app_profiler/yarn/command" +require "app_profiler/yarn/with_firefox_profiler" + +module AppProfiler + module Viewer + class FirefoxRemoteViewer < BaseViewer + class Middleware < AppProfiler::Viewer::BaseMiddleware + include Yarn::WithFirefoxProfiler + + def initialize(app) + super + @firefox_profiler = Rack::File.new( + File.join(AppProfiler.root, "node_modules/firefox-profiler/dist"), + ) + end + + def call(env) + request = Rack::Request.new(env) + # Firefox profiler *really* doesn't like for /from-url/ to be at any other mount point + # so with this enabled, we take over both /app_profiler and /from-url in the app in development. + return from(env, Regexp.last_match(1)) if request.path_info =~ %r(\A/from-url/(.*)\z) + return viewer(env, Regexp.last_match(1)) if request.path_info =~ %r(\A/app_profiler/firefox/viewer/(.*)\z) + return show(env, Regexp.last_match(1)) if request.path_info =~ %r(\A/app_profiler/firefox/(.*)\z) + + super + end + + protected + + attr_reader(:firefox_profiler) + + def viewer(env, path) + setup_yarn unless yarn_setup + + if path.end_with?(AppProfiler::VernierProfile::FILE_EXTENSION) + proto = env["rack.url_scheme"] + host = env["HTTP_HOST"] + source = "#{proto}://#{host}/app_profiler/firefox/#{path}" + target = "/from-url/#{CGI.escape(source)}" + + [302, { "Location" => target }, [""]] + else + env[Rack::PATH_INFO] = path.delete_prefix("/app_profiler") + firefox_profiler.call(env) + end + end + + def from(env, path) + setup_yarn unless yarn_setup + index = File.read(File.join(AppProfiler.root, "node_modules/firefox-profiler/dist/index.html")) + [200, { "Content-Type" => "text/html" }, [index]] + end + + def show(_env, name) + profile = profile_files.find do |file| + id(file) == name + end || raise(ArgumentError) + + [200, { "Content-Type" => "application/json" }, [profile.read]] + end + end + end + end +end diff --git a/lib/app_profiler/viewer/speedscope_remote_viewer.rb b/lib/app_profiler/viewer/speedscope_remote_viewer.rb index ced8bbce..0ff3e25f 100644 --- a/lib/app_profiler/viewer/speedscope_remote_viewer.rb +++ b/lib/app_profiler/viewer/speedscope_remote_viewer.rb @@ -1,14 +1,22 @@ # frozen_string_literal: true -require "app_profiler/viewer/speedscope_remote_viewer/base_middleware" +require "active_support/deprecation/constant_accessor" require "app_profiler/viewer/speedscope_remote_viewer/middleware" module AppProfiler module Viewer class SpeedscopeRemoteViewer < BaseViewer + include ActiveSupport::Deprecation::DeprecatedConstantAccessor + deprecate_constant( + "BaseMiddleware", + "AppProfiler::Viewer::BaseMiddleware", + deprecator: AppProfiler.deprecator, + ) + NAME = "speedscope" + class << self - def view(profile, params = {}) - new(profile).view(**params) + def remote? + true end end @@ -21,7 +29,7 @@ def view(response: nil, autoredirect: nil, async: false) id = Middleware.id(@profile.file) if response && response[0].to_i < 500 - response[1]["Location"] = "/app_profiler/#{id}" + response[1]["Location"] = "/app_profiler/#{NAME}/viewer/#{id}" response[0] = 303 else AppProfiler.logger.info("[Profiler] Profile available at /app_profiler/#{id}\n") diff --git a/lib/app_profiler/viewer/speedscope_remote_viewer/base_middleware.rb b/lib/app_profiler/viewer/speedscope_remote_viewer/base_middleware.rb deleted file mode 100644 index 4ee7a28a..00000000 --- a/lib/app_profiler/viewer/speedscope_remote_viewer/base_middleware.rb +++ /dev/null @@ -1,142 +0,0 @@ -# frozen_string_literal: true - -gem "rails-html-sanitizer", ">= 1.6.0" -require "rails-html-sanitizer" - -module AppProfiler - module Viewer - class SpeedscopeRemoteViewer < BaseViewer - 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.delete_suffix(".json") - end - end - - def initialize(app) - @app = app - end - - def call(env) - request = Rack::Request.new(env) - - return index(env) if request.path_info =~ %r(\A/app_profiler/?\z) - return viewer(env, Regexp.last_match(1)) if request.path_info =~ %r(\A/app_profiler/viewer/(.*)\z) - return show(env, Regexp.last_match(1)) if request.path_info =~ %r(\A/app_profiler/(.*)\z) - - @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, - - - - App Profiler - - - #{sanitizer.sanitize(html)} - - - HTML - ], - ] - end - - def sanitizer - @sanitizer ||= Sanitizer.new - end - - def viewer(_env, path) - raise NotImplementedError - end - - def index(_env) - render( - (+"").tap do |content| - content << "

Profiles

" - profile_files.each do |file| - content << <<~HTML -

- - #{id(file)} - -

- HTML - end - end, - ) - end - - def show(env, id) - raise NotImplementedError - end - end - - private_constant(:BaseMiddleware) - end - end -end diff --git a/lib/app_profiler/viewer/speedscope_remote_viewer/middleware.rb b/lib/app_profiler/viewer/speedscope_remote_viewer/middleware.rb index 75fb4227..07ef418d 100644 --- a/lib/app_profiler/viewer/speedscope_remote_viewer/middleware.rb +++ b/lib/app_profiler/viewer/speedscope_remote_viewer/middleware.rb @@ -16,14 +16,23 @@ def initialize(app) ) end + def call(env) + request = Rack::Request.new(env) + + return viewer(env, Regexp.last_match(1)) if request.path_info =~ %r(\A/app_profiler/speedscope/viewer/(.*)\z) + return show(env, Regexp.last_match(1)) if request.path_info =~ %r(\A/app_profiler/speedscope/(.*)\z) + + super + end + protected attr_reader(:speedscope) def viewer(env, path) setup_yarn unless yarn_setup - env[Rack::PATH_INFO] = path.delete_prefix("/app_profiler") + env[Rack::PATH_INFO] = path.delete_prefix("/app_profiler/speedscope") speedscope.call(env) end @@ -32,25 +41,7 @@ def show(_env, name) id(file) == name end || raise(ArgumentError) - render( - <<~HTML, - - HTML - ) + [200, { "Content-Type" => "application/json" }, [profile.read]] end end end diff --git a/lib/app_profiler/viewer/speedscope_viewer.rb b/lib/app_profiler/viewer/speedscope_viewer.rb index 5d758cfb..3914e44c 100644 --- a/lib/app_profiler/viewer/speedscope_viewer.rb +++ b/lib/app_profiler/viewer/speedscope_viewer.rb @@ -8,12 +8,6 @@ module Viewer class SpeedscopeViewer < BaseViewer include Yarn::WithSpeedscope - class << self - def view(profile, params = {}) - new(profile).view(**params) - end - end - def initialize(profile) super() @profile = profile diff --git a/lib/app_profiler/yarn/command.rb b/lib/app_profiler/yarn/command.rb index 8fe4d1b6..2002758b 100644 --- a/lib/app_profiler/yarn/command.rb +++ b/lib/app_profiler/yarn/command.rb @@ -10,10 +10,12 @@ class YarnError < StandardError; end ["yarn", "init", "--yes"], ["yarn", "add", "speedscope", "--dev", "--ignore-workspace-root-check"], ["yarn", "run", "speedscope", /.*\.json/], + ["yarn", "add", "--dev", %r{.*/firefox-profiler}], + ["yarn", "--cwd", %r{.*/firefox-profiler}], + ["yarn", "--cwd", %r{.*/firefox-profiler}, "build-prod"], ] private_constant(:VALID_COMMANDS) - mattr_accessor(:yarn_setup, default: false) def yarn(command, *options) setup_yarn unless yarn_setup @@ -29,6 +31,14 @@ def setup_yarn yarn("init", "--yes") unless package_json_exists? end + def yarn_setup + @yarn_initialized || false + end + + def yarn_setup=(state) + @yarn_initialized = state + end + private def ensure_command_valid(command) @@ -39,6 +49,8 @@ def ensure_command_valid(command) def valid_command?(command) VALID_COMMANDS.any? do |valid_command| + next unless valid_command.size == command.size + valid_command.zip(command).all? do |valid_part, part| part.match?(valid_part) end @@ -55,7 +67,7 @@ def ensure_yarn_installed MSG ) end - self.yarn_setup = true + @yarn_initialized = true end def package_json_exists? diff --git a/lib/app_profiler/yarn/with_firefox_profiler.rb b/lib/app_profiler/yarn/with_firefox_profiler.rb new file mode 100644 index 00000000..e1e6f6fe --- /dev/null +++ b/lib/app_profiler/yarn/with_firefox_profiler.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module AppProfiler + module Yarn + module WithFirefoxProfiler + include Command + + PACKAGE = "https://github.com/tenderlove/profiler#v0.0.2" + private_constant(:PACKAGE) + + def setup_yarn + super + return if firefox_profiler_added? + + fetch_firefox_profiler + end + + private + + def firefox_profiler_added? + AppProfiler.root.join("node_modules/firefox-profiler/dist").exist? + end + + def fetch_firefox_profiler + repo, branch = PACKAGE.to_s.split("#") + + dir = "./tmp" + FileUtils.mkdir_p(dir) + Dir.chdir(dir) do + clone_args = ["git", "clone", repo, "firefox-profiler"] + clone_args.push("--branch=#{branch}") unless branch.nil? || branch&.empty? + system(*clone_args) + package_contents = File.read("firefox-profiler/package.json") + package_json = JSON.parse(package_contents) + package_json["name"] ||= "firefox-profiler" + package_json["version"] ||= "0.0.1" + File.write("firefox-profiler/package.json", package_json.to_json) + end + yarn("--cwd", "#{dir}/firefox-profiler") + + patch_firefox_profiler(dir) + yarn("--cwd", "#{dir}/firefox-profiler", "build-prod") + patch_file( + "#{dir}/firefox-profiler/dist/index.html", + 'href="locales/en-US/app.ftl"', + 'href="/app_profiler/firefox/viewer/locales/en-US/app.ftl"', + ) + + yarn("add", "--dev", "#{dir}/firefox-profiler") + end + + def patch_firefox_profiler(dir) + # Patch the publicPath so that the app can be "mounted" at the right location + patch_file( + "#{dir}/firefox-profiler/webpack.config.js", + "publicPath: '/'", + "publicPath: '/app_profiler/firefox/viewer/'", + ) + patch_file( + "#{dir}/firefox-profiler/src/app-logic/l10n.js", + "fetch(`/locales/", + "fetch(`/app_profiler/firefox/viewer/locales/", + ) + end + + def patch_file(file, find, replace) + contents = File.read(file) + new_contents = contents.gsub(find, replace) + File.write(file, new_contents) + end + end + end +end diff --git a/test/app_profiler/app_profiler_configuration_test.rb b/test/app_profiler/app_profiler_configuration_test.rb index b20d8271..ef47dd34 100644 --- a/test/app_profiler/app_profiler_configuration_test.rb +++ b/test/app_profiler/app_profiler_configuration_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "test_helper" + module AppProfiler class ConfigurationTest < TestCase test "unexpected handler profile_enqueue_success raises ArgumentError " do diff --git a/test/app_profiler/sampler/config_test.rb b/test/app_profiler/sampler/config_test.rb index b0eb8d11..19805324 100644 --- a/test/app_profiler/sampler/config_test.rb +++ b/test/app_profiler/sampler/config_test.rb @@ -28,8 +28,10 @@ class ConfigTest < TestCase end test "allows passing paths" do - config = Config.new(paths: ["/"]) - assert_equal(["/"], config.targets) + assert_deprecated(/passing paths is deprecated/, AppProfiler.deprecator) do + config = Config.new(paths: ["/"]) + assert_equal(["/"], config.targets) + end end end end diff --git a/test/app_profiler/storage/google_cloud_storage_test.rb b/test/app_profiler/storage/google_cloud_storage_test.rb index 7a70de50..29c683c2 100644 --- a/test/app_profiler/storage/google_cloud_storage_test.rb +++ b/test/app_profiler/storage/google_cloud_storage_test.rb @@ -162,7 +162,7 @@ def with_mock_gcs_bucket(profile) assert_equal(filename, GoogleCloudStorage.send(:gcs_filename, profile)) assert_equal(data[:content_type], "application/json") assert_equal(data[:content_encoding], "gzip") - assert_equal(data[:metadata], metadata) + assert_equal(data[:metadata].to_s, metadata.to_s) end.returns(file) GoogleCloudStorage.stubs(:bucket).returns(bucket) diff --git a/test/app_profiler/viewer/remote/firefox_viewer_test.rb b/test/app_profiler/viewer/remote/firefox_viewer_test.rb new file mode 100644 index 00000000..2a92fd24 --- /dev/null +++ b/test/app_profiler/viewer/remote/firefox_viewer_test.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require "test_helper" + +module AppProfiler + module Viewer + class FirefoxRemoteViewerTest < TestCase + test ".view initializes and calls #view" do + FirefoxRemoteViewer.any_instance.expects(:view) + + profile = VernierProfile.new(vernier_profile) + FirefoxRemoteViewer.view(profile) + end + + test "#view logs middleware URL" do + profile = VernierProfile.new(vernier_profile) + + viewer = FirefoxRemoteViewer.new(profile) + id = FirefoxRemoteViewer::Middleware.id(profile.file) + + AppProfiler.logger.expects(:info).with( + "[Profiler] Profile available at /app_profiler/#{id}\n", + ) + + viewer.view + end + + test "#view with response redirects to URL" do + response = [200, {}, ["OK"]] + profile = VernierProfile.new(vernier_profile) + + viewer = FirefoxRemoteViewer.new(profile) + id = FirefoxRemoteViewer::Middleware.id(profile.file) + + viewer.view(response: response) + + assert_equal(303, response[0]) + assert_equal("/app_profiler/firefox/viewer/#{id}", response[1]["Location"]) + end + end + end +end diff --git a/test/app_profiler/viewer/remote/middleware/firefox_middleware_test.rb b/test/app_profiler/viewer/remote/middleware/firefox_middleware_test.rb new file mode 100644 index 00000000..e57ed48a --- /dev/null +++ b/test/app_profiler/viewer/remote/middleware/firefox_middleware_test.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require "test_helper" + +module AppProfiler + module Viewer + class FirefoxRemoteViewer + class MiddlewareTest < TestCase + setup do + @app = Middleware.new( + proc { [200, { "Content-Type" => "text/plain" }, ["Hello world!"]] }, + ) + end + + test ".id" do + profile = VernierProfile.new(vernier_profile) + profile_id = profile.file.basename.to_s + + assert_equal(profile_id, Middleware.id(profile.file)) + end + + test "#call index" do + profiles = Array.new(3) { VernierProfile.new(vernier_profile).tap(&:file) } + + code, content_type, html = @app.call({ "PATH_INFO" => "/app_profiler" }) + html = html.first + + assert_equal(200, code) + assert_equal({ "Content-Type" => "text/html" }, content_type) + assert_match(%r(App Profiler), html) + profiles.each do |profile| + id = Middleware.id(profile.file) + assert_match( + %r(), html + ) + end + end + + test "#call index with slash" do + profiles = Array.new(3) { VernierProfile.new(vernier_profile).tap(&:file) } + + code, content_type, html = @app.call({ "PATH_INFO" => "/app_profiler/" }) + html = html.first + + assert_equal(200, code) + assert_equal({ "Content-Type" => "text/html" }, content_type) + assert_match(%r(App Profiler), html) + profiles.each do |profile| + id = Middleware.id(profile.file) + assert_match( + %r(), html + ) + end + end + + test "#call show" do + profile = VernierProfile.new(vernier_profile) + id = Middleware.id(profile.file) + + code, content_type, body = @app.call({ "PATH_INFO" => "/app_profiler/firefox/#{id}" }) + + assert_equal(200, code) + assert_equal({ "Content-Type" => "application/json" }, content_type) + assert_equal(JSON.dump(profile.to_h), body.first) + end + + test "#call viewer sets up yarn" do + @app.expects(:system).with("which", "yarn", out: File::NULL).returns(true) + @app.expects(:system).with("yarn", "init", "--yes").returns(true) + + url = "https://github.com/tenderlove/profiler" + branch = "v0.0.2" + @app.expects(:system).with("git", "clone", url, "firefox-profiler", "--branch=#{branch}").returns(true) + + File.expects(:read).returns("{}") + File.expects(:write).returns(true) + + dir = "./tmp" + + @app.expects(:system).with("yarn", "--cwd", "#{dir}/firefox-profiler").returns(true) + + File.expects(:read).with("#{dir}/firefox-profiler/webpack.config.js").returns("") + File.expects(:write).with("#{dir}/firefox-profiler/webpack.config.js", "").returns(true) + + File.expects(:read).with("#{dir}/firefox-profiler/src/app-logic/l10n.js").returns("") + File.expects(:write).with("#{dir}/firefox-profiler/src/app-logic/l10n.js", "").returns(true) + + @app.expects(:system).with("yarn", "--cwd", "#{dir}/firefox-profiler", "build-prod").returns(true) + + File.expects(:read).with("#{dir}/firefox-profiler/dist/index.html").returns("") + File.expects(:write).with("#{dir}/firefox-profiler/dist/index.html", "").returns(true) + + @app.expects(:system).with("yarn", "add", "--dev", "#{dir}/firefox-profiler").returns(true) + @app.call({ "PATH_INFO" => "/app_profiler/firefox/viewer/index.html" }) + + assert_predicate(@app, :yarn_setup) + end + + test "#call viewer" do + with_yarn_setup(@app) do + @app.expects(:firefox_profiler).returns(proc { [200, { "Content-Type" => "text/plain" }, ["Firefox"]] }) + + response = @app.call({ "PATH_INFO" => "/app_profiler/firefox/viewer/index.html" }) + + assert_equal([200, { "Content-Type" => "text/plain" }, ["Firefox"]], response) + end + end + + test "#call" do + response = @app.call({ "PATH_INFO" => "/app_level_route" }) + + assert_equal([200, { "Content-Type" => "text/plain" }, ["Hello world!"]], response) + end + end + end + end +end diff --git a/test/app_profiler/viewer/speedscope_remote_viewer/middleware_test.rb b/test/app_profiler/viewer/remote/middleware/speedscope_middleware_test.rb similarity index 65% rename from test/app_profiler/viewer/speedscope_remote_viewer/middleware_test.rb rename to test/app_profiler/viewer/remote/middleware/speedscope_middleware_test.rb index 346fa042..7e150e81 100644 --- a/test/app_profiler/viewer/speedscope_remote_viewer/middleware_test.rb +++ b/test/app_profiler/viewer/remote/middleware/speedscope_middleware_test.rb @@ -14,7 +14,7 @@ class MiddlewareTest < TestCase test ".id" do profile = BaseProfile.from_stackprof(stackprof_profile) - profile_id = profile.file.basename.to_s.delete_suffix(".json") + profile_id = profile.file.basename.to_s assert_equal(profile_id, Middleware.id(profile.file)) end @@ -29,7 +29,10 @@ class MiddlewareTest < TestCase assert_equal({ "Content-Type" => "text/html" }, content_type) assert_match(%r(App Profiler), html) profiles.each do |profile| - assert_match(%r(), html) + id = Middleware.id(profile.file) + assert_match( + %r(), html + ) end end @@ -43,7 +46,10 @@ class MiddlewareTest < TestCase assert_equal({ "Content-Type" => "text/html" }, content_type) assert_match(%r(App Profiler), html) profiles.each do |profile| - assert_match(%r(), html) + id = Middleware.id(profile.file) + assert_match( + %r(), html + ) end end @@ -51,28 +57,11 @@ class MiddlewareTest < TestCase profile = BaseProfile.from_stackprof(stackprof_profile) id = Middleware.id(profile.file) - code, content_type, html = @app.call({ "PATH_INFO" => "/app_profiler/#{id}" }) - html = html.first + code, content_type, body = @app.call({ "PATH_INFO" => "/app_profiler/speedscope/#{id}" }) assert_equal(200, code) - assert_equal({ "Content-Type" => "text/html" }, content_type) - assert_match(%r(App Profiler), html) - assert_match(%r(}, - html[-200..-1], - message: "The generated HTML was incomplete", - ) + assert_equal({ "Content-Type" => "application/json" }, content_type) + assert_equal(JSON.dump(profile.to_h), body.first) end test "#call viewer sets up yarn" do @@ -82,18 +71,16 @@ class MiddlewareTest < TestCase "yarn", "add", "speedscope", "--dev", "--ignore-workspace-root-check" ).returns(true) - @app.call({ "PATH_INFO" => "/app_profiler/viewer/index.html" }) + @app.call({ "PATH_INFO" => "/app_profiler/speedscope/viewer/index.html" }) - assert_predicate(Yarn::Command, :yarn_setup) - ensure - Yarn::Command.yarn_setup = false + assert_predicate(@app, :yarn_setup) end test "#call viewer" do - with_yarn_setup do + with_yarn_setup(@app) do @app.expects(:speedscope).returns(proc { [200, { "Content-Type" => "text/plain" }, ["Speedscope"]] }) - response = @app.call({ "PATH_INFO" => "/app_profiler/viewer/index.html" }) + response = @app.call({ "PATH_INFO" => "/app_profiler/speedscope/viewer/index.html" }) assert_equal([200, { "Content-Type" => "text/plain" }, ["Speedscope"]], response) end diff --git a/test/app_profiler/viewer/speedscope_remote_viewer_test.rb b/test/app_profiler/viewer/remote/speedscope_viewer_test.rb similarity index 92% rename from test/app_profiler/viewer/speedscope_remote_viewer_test.rb rename to test/app_profiler/viewer/remote/speedscope_viewer_test.rb index a9d3b1be..50e74c84 100644 --- a/test/app_profiler/viewer/speedscope_remote_viewer_test.rb +++ b/test/app_profiler/viewer/remote/speedscope_viewer_test.rb @@ -35,7 +35,7 @@ class SpeedscopeRemoteViewerTest < TestCase viewer.view(response: response) assert_equal(303, response[0]) - assert_equal("/app_profiler/#{id}", response[1]["Location"]) + assert_equal("/app_profiler/speedscope/viewer/#{id}", response[1]["Location"]) end end end diff --git a/test/app_profiler/viewer/speedscope_viewer_test.rb b/test/app_profiler/viewer/speedscope_viewer_test.rb index 24419abb..fa2fcabf 100644 --- a/test/app_profiler/viewer/speedscope_viewer_test.rb +++ b/test/app_profiler/viewer/speedscope_viewer_test.rb @@ -25,9 +25,7 @@ class SpeedscopeViewerTest < TestCase viewer.view - assert_predicate(Yarn::Command, :yarn_setup) - ensure - Yarn::Command.yarn_setup = false + assert_predicate(viewer, :yarn_setup) end test "#view doesn't init when package.json exists" do @@ -45,9 +43,8 @@ class SpeedscopeViewerTest < TestCase viewer.view - assert_predicate(Yarn::Command, :yarn_setup) + assert_predicate(viewer, :yarn_setup) ensure - Yarn::Command.yarn_setup = false AppProfiler.root.rmtree end @@ -64,21 +61,19 @@ class SpeedscopeViewerTest < TestCase viewer.view - assert_predicate(Yarn::Command, :yarn_setup) + assert_predicate(viewer, :yarn_setup) ensure - Yarn::Command.yarn_setup = false AppProfiler.root.rmtree end test "#view only opens profile in speedscope if yarn is already setup" do profile = BaseProfile.from_stackprof(stackprof_profile) - with_yarn_setup do - viewer = SpeedscopeViewer.new(profile) - viewer.expects(:system).with("yarn", "run", "speedscope", profile.file.to_s).returns(true) + viewer = SpeedscopeViewer.new(profile) + viewer.yarn_setup = true + viewer.expects(:system).with("yarn", "run", "speedscope", profile.file.to_s).returns(true) - viewer.view - end + viewer.view end test "#view raises YarnError when yarn command fails" do diff --git a/test/app_profiler/yarn/command_test.rb b/test/app_profiler/yarn/command_test.rb index 652dd224..11792536 100644 --- a/test/app_profiler/yarn/command_test.rb +++ b/test/app_profiler/yarn/command_test.rb @@ -8,11 +8,11 @@ class CommandTest < TestCase include(Command) setup do - Command.yarn_setup = true + @yarn_initialized = true end teardown do - Command.yarn_setup = false + @yarn_initialized = false end test "#yarn allows add speedscope" do diff --git a/test/test_helper.rb b/test/test_helper.rb index 789c3987..4a0af4d8 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -87,12 +87,12 @@ def vernier_params(params = {}) { mode: :wall, interval: 1000, metadata: { id: "foo" } }.merge(params) end - def with_yarn_setup - old_yarn_setup = Yarn::Command.yarn_setup - Yarn::Command.yarn_setup = true + def with_yarn_setup(app) + old_yarn_setup = app.yarn_setup + app.instance_variable_set(:@yarn_initialized, true) yield ensure - Yarn::Command.yarn_setup = old_yarn_setup + app.instance_variable_set(:@yarn_initialized, old_yarn_setup) end end end