Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add memory-profiler and add ability to selectively enable rack-mini-profiler #24022

Merged
merged 3 commits into from
Aug 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,13 @@ gem 'launchy' # Peer dependency of Google::APIClient::InstalledAppFlow
# CSRF protection for Sinatra.
gem 'rack_csrf'

# Allow profiling in all environments (including production). It will only be enabled when
# CDO.rack_mini_profiler_enabled is set. See dashboard/config/initializers/mini_profiler.rb
gem 'memory_profiler'
gem 'rack-mini-profiler'

group :development do
gem 'annotate'
gem 'rack-mini-profiler'
gem 'ruby-progressbar', require: false
gem 'thin'
gem 'web-console'
Expand Down
4 changes: 3 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ GEM
mini_mime (>= 0.1.1)
marked-rails (0.3.2.0)
memoist (0.15.0)
memory_profiler (0.9.10)
metaclass (0.0.4)
method_source (0.9.0)
mime-types (3.1)
Expand Down Expand Up @@ -617,7 +618,7 @@ GEM
rack (2.0.5)
rack-cache (1.6.1)
rack (>= 0.4)
rack-mini-profiler (0.10.1)
rack-mini-profiler (1.0.0)
rack (>= 1.2.0)
rack-oauth2 (1.5.1)
activesupport (>= 2.3)
Expand Down Expand Up @@ -927,6 +928,7 @@ DEPENDENCIES
lograge
loofah (~> 2.2.1)
marked-rails
memory_profiler
mini_magick
minitest (~> 5.5)
minitest-around
Expand Down
1 change: 1 addition & 0 deletions cookbooks/cdo-varnish/libraries/http_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def self.config(env)
'callouts_seen',
'rack.session',
'remember_user_token',
'__profilin', # Used by rack-mini-profiler
session_key,
storage_id,
].concat(default_cookies)
Expand Down
20 changes: 11 additions & 9 deletions dashboard/app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,20 @@ def fix_crawlers_with_bad_accept_headers
end
end

# Configure development only filters.
if Rails.env.development?
# Enable or disable the rack mini-profiler if the 'pp' query string parameter is set.
# pp='disabled' will disable it; any other value will enable it.
before_action :maybe_enable_profiler
def maybe_enable_profiler
pp = params['pp']
if pp
ENV['RACK_MINI_PROFILER'] = (pp == 'disabled') ? 'off' : 'on'
if CDO.rack_mini_profiler_enabled
before_action :check_profiler
def check_profiler
# Authorize the mini profiler when the rack_mini_profiler_enabled setting is enabled and
# the ?pp query param is present, and this is development or a signed-in admin user
# in production (or another environment)
if CDO.rack_mini_profiler_enabled && params.key?(:pp) && (Rails.env.development? || current_user&.admin?)
Rack::MiniProfiler.authorize_request
end
end
end

# Configure development only filters.
if Rails.env.development?
before_action :configure_web_console
# Enable the Rails web console if params['dbg'] is set, or disable it
# if params['dbg'] is 'off'.
Expand Down
35 changes: 21 additions & 14 deletions dashboard/config/initializers/mini_profiler.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
if Rails.env.development?
require 'rack-mini-profiler'
require 'rack-mini-profiler'

# Configure the rack mini-profiler, which displays a on-page breakdown of the
# time spent rendering the page including details on the SQL queries run.
# Configure the rack mini-profiler, which displays a on-page breakdown of the
# time spent rendering the page including details on the SQL queries run,
# and can run memory, GC, and stack analysis.

# Display the mini-profiler on the right side of the page so that it is less
# likely to overlap important content.
Rack::MiniProfiler.config.position = 'right'
# Display the mini-profiler on the right side of the page so that it is less
# likely to overlap important content.
Rack::MiniProfiler.config.position = 'right'

# This callback is run for each page to determine whether to enable the
# mini-profiler. It is enabled only in development and only if the
# RACK_MINI_PROFILER environment variable is 'on'. This can be done either by
# setting the environment variable prior to running the server, or by hitting
# a page with a "pp=enabled" query string parameter and then refreshing the page.
Rack::MiniProfiler.config.pre_authorize_cb = lambda {|_env| ENV['RACK_MINI_PROFILER'] == 'on'}
end
# Only pre-authorize the min profiler when this setting is enabled (in locals.yml).
# See ApplicationController.check_profiler for more details on when the profiler
# is actually enabled per-page and user in different environments
Rack::MiniProfiler.config.pre_authorize_cb = proc {CDO.rack_mini_profiler_enabled}

# See https://github.com/MiniProfiler/rack-mini-profiler#storage
# Rack::MiniProfiler.config.storage = Rack::MiniProfiler::FileStore
Rack::MiniProfiler.config.storage = Rack::MiniProfiler::FileStore

# Only allow when it's explicitly allowed based on the rules in ApplicationController.check_profiler,
# across all environments for consistency.
# By default, this would be :allow_all for dev and test, :whitelist for other environments.
# The whitelist mode means we must explicitly call authorize_request on a given request.
Rack::MiniProfiler.config.authorization_mode = :whitelist