From 020cd4f510134ef933f6a8ff2eba6a6f5f0331b1 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 28 Apr 2021 17:38:48 +0100 Subject: [PATCH] FEATURE: Introduce `pp=async-flamegraph` for asynchronous flamegraphs (#494) Using `?pp=async-flamegraph` causes the flamegraph data to be placed in long-term storage, and made available via a link in the mini_profiler UI. Flamegraph data will also be recorded and stored for all AJAX requests with `?pp=async-flamegraph` in the `Referer` header. This is useful in a few situations: - You want to view flamegraphs for AJAX requests made by a Javascript application. By supplying `pp=async-flamegraph`, flamegraph links for every AJAX request will be made available in the mini-profiler UI. - You want to see the HTML result of a request, and view the flamegraph later. The existing `?pp=flamegraph` option hides the true output. - You are performing the request via a tool like `curl`, and would like to view the flamegraph later in the browser (you can extract the X-MiniProfiler-Ids header from the response, then view flamegraph in the browser) --- When the `pp=async-flamegraph` parameter is supplied, a new "flamegraph" link is added to the UI. Clicking the link will take you to a URL like `/mini-profiler-resources/flamegraph?id=t0x70kt7hy3cmitx7adx`, which displays the flamegraph UI. --- CHANGELOG.md | 4 + Gemfile | 5 +- README.md | 4 + lib/html/includes.js | 3 + lib/html/includes.tmpl | 3 + lib/html/vendor.js | 2 +- lib/mini_profiler/profiler.rb | 88 ++++++++++++------- lib/mini_profiler/timer_struct/page.rb | 12 ++- spec/integration/middleware_spec.rb | 10 +++ spec/integration/mini_profiler_spec.rb | 32 +++++++ .../timer_struct/page_timer_struct_spec.rb | 15 ++++ 11 files changed, 139 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a09ffe..575695a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # CHANGELOG +## Unreleased + +- [FEATURE] Introduce `pp=async-flamegraph` for asynchronous flamegraphs + ## 2.3.1 - 2021-01-29 - [FIX] compatability with Ruby 3.0 diff --git a/Gemfile b/Gemfile index ae6c60a1..2e332a64 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,10 @@ ruby '>= 2.4.0' gemspec -gem 'codecov', require: false, group: :test +group :test do + gem 'codecov', require: false + gem 'stackprof', require: false +end group :development do gem 'guard', platforms: [:mri_22, :mri_23] diff --git a/README.md b/README.md index cb833f3b..f22932e5 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,10 @@ To generate [flamegraphs](http://samsaffron.com/archive/2013/03/19/flame-graphs- * add the [**stackprof**](https://rubygems.org/gems/stackprof) gem to your Gemfile * visit a page in your app with `?pp=flamegraph` +To store flamegraph data for later viewing, append the `?pp=async-flamegraph` parameter. The request will return as normal. +Flamegraph data for this request, and all subsequent requests made by this page (based on the `REFERER` header) will be stored. +'flamegraph' links will appear for these requests in the MiniProfiler UI. + ### Memory Profiling Memory allocations can be measured (using the [memory_profiler](https://github.com/SamSaffron/memory_profiler) gem) diff --git a/lib/html/includes.js b/lib/html/includes.js index c3065747..f704b43b 100644 --- a/lib/html/includes.js +++ b/lib/html/includes.js @@ -1213,6 +1213,9 @@ var _MiniProfiler = (function() { shareUrl: function shareUrl(id) { return options.path + "results?id=" + id; }, + flamegraphUrl: function flamegrapgUrl(id) { + return options.path + "flamegraph?id=" + id; + }, moreUrl: function moreUrl(requestName) { var requestParts = requestName.split(" "); var linkSrc = diff --git a/lib/html/includes.tmpl b/lib/html/includes.tmpl index 17e1cf91..5a46d589 100644 --- a/lib/html/includes.tmpl +++ b/lib/html/includes.tmpl @@ -142,6 +142,9 @@ - - - HTML - [200, headers, [html]] - else - [200, headers, [graph]] - end + html = <<~HTML + + + + + + + + + + HTML + [200, headers, [html]] end def ids(env) @@ -824,6 +826,24 @@ def handle_snapshots_request(env) response.finish end + def serve_flamegraph(env) + request = Rack::Request.new(env) + id = request.params['id'] + page_struct = @storage.load(id) + + if !page_struct + id = ERB::Util.html_escape(id) + user_info = ERB::Util.html_escape(user(env)) + return [404, {}, ["Request not found: #{id} - user #{user_info}"]] + end + + if !page_struct[:flamegraph] + return [404, {}, ["No flamegraph available for #{ERB::Util.html_escape(id)}"]] + end + + self.flamegraph(page_struct[:flamegraph], page_struct[:request_path]) + end + def rails_route_from_path(path, method) if defined?(Rails) && defined?(ActionController::RoutingError) hash = Rails.application.routes.recognize_path(path, method: method) diff --git a/lib/mini_profiler/timer_struct/page.rb b/lib/mini_profiler/timer_struct/page.rb index dbf13df4..fecb0f46 100644 --- a/lib/mini_profiler/timer_struct/page.rb +++ b/lib/mini_profiler/timer_struct/page.rb @@ -87,7 +87,9 @@ def initialize(env) executed_non_queries: 0, custom_timing_names: [], custom_timing_stats: {}, - custom_fields: {} + custom_fields: {}, + has_flamegraph: false, + flamegraph: nil ) self[:request_method] = env['REQUEST_METHOD'] self[:request_path] = env['PATH_INFO'] @@ -111,12 +113,16 @@ def root @attributes[:root] end + def attributes_to_serialize + @attributes.keys - [:flamegraph] + end + def to_json(*a) - ::JSON.generate(@attributes.merge(self.extra_json)) + ::JSON.generate(@attributes.slice(*attributes_to_serialize).merge(extra_json)) end def as_json(options = nil) - super(options).merge!(extra_json) + super(options).slice(*attributes_to_serialize.map(&:to_s)).merge!(extra_json) end def extra_json diff --git a/spec/integration/middleware_spec.rb b/spec/integration/middleware_spec.rb index 93d7348e..dfd6e5e8 100644 --- a/spec/integration/middleware_spec.rb +++ b/spec/integration/middleware_spec.rb @@ -74,6 +74,16 @@ def app ) end end + + describe 'with async-flamegraph query' do + it 'should return stackprof error message' do + Rack::MiniProfiler.config.enable_advanced_debugging_tools = true + do_get(pp: 'async-flamegraph') + expect(last_response.body).to eq( + 'Please install the stackprof gem and require it: add gem \'stackprof\' to your Gemfile' + ) + end + end end describe 'with Rack::MiniProfiler before Rack::Deflater' do diff --git a/spec/integration/mini_profiler_spec.rb b/spec/integration/mini_profiler_spec.rb index f5d18931..9b44042f 100644 --- a/spec/integration/mini_profiler_spec.rb +++ b/spec/integration/mini_profiler_spec.rb @@ -141,6 +141,38 @@ def app end end + it 'works with async-flamegraph' do + pid = fork do # Avoid polluting main process with stackprof + require 'stackprof' + + # Should store flamegraph for ?pp=async-flamegraph + get '/html?pp=async-flamegraph' + expect(last_response).to be_ok + id = last_response.headers['X-MiniProfiler-Ids'].split(",")[0] + get "/mini-profiler-resources/flamegraph?id=#{id}" + expect(last_response).to be_ok + expect(last_response.body).to include("var graph = {") + + # Should store flamegraph based on REFERER + get '/html', nil, { "HTTP_REFERER" => "/origin?pp=async-flamegraph" } + expect(last_response).to be_ok + id = last_response.headers['X-MiniProfiler-Ids'].split(",")[0] + get "/mini-profiler-resources/flamegraph?id=#{id}" + expect(last_response).to be_ok + expect(last_response.body).to include("var graph = {") + + # Should not store/return flamegraph for regular requests + get '/html' + expect(last_response).to be_ok + id = last_response.headers['X-MiniProfiler-Ids'].split(",")[0] + get "/mini-profiler-resources/flamegraph?id=#{id}" + expect(last_response.status).to eq(404) + end + + Process.wait(pid) + expect($?.exitstatus).to eq(0) + end + describe 'with an implicit body tag' do before do diff --git a/spec/lib/timer_struct/page_timer_struct_spec.rb b/spec/lib/timer_struct/page_timer_struct_spec.rb index 2b1e4bf8..3a555fba 100644 --- a/spec/lib/timer_struct/page_timer_struct_spec.rb +++ b/spec/lib/timer_struct/page_timer_struct_spec.rb @@ -41,4 +41,19 @@ expect(page.to_json).to eq(from_json_page.to_json) end end + + describe '.to_json' do + it 'does not include the flamegraph itself' do + page = described_class.new({ + 'REQUEST_METHOD' => 'POST', + 'PATH_INFO' => '/some/path', + 'SERVER_NAME' => 'server001', + }) + page[:has_flamegraph] = true + page[:flamegraph] = { fake: "data" } + result = JSON.parse(page.to_json) + expect(result["flamegraph"]).to eq(nil) + expect(result["has_flamegraph"]).to eq(true) + end + end end