diff --git a/CHANGELOG.md b/CHANGELOG.md index f2ad4314f..af9c6bbf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ Airbrake Changelog ### master +* Rails: added support for Curb for HTTP performance breakdown + ([#957](https://github.com/airbrake/airbrake/pull/957)) + ### [v9.1.0][v9.1.0] (April 17, 2019) * Rails: added HTTP performance breakdown (`Net::HTTP` support only) diff --git a/airbrake.gemspec b/airbrake.gemspec index 025aa8caa..434c2b5e1 100644 --- a/airbrake.gemspec +++ b/airbrake.gemspec @@ -61,4 +61,6 @@ DESC if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.2.2') s.add_development_dependency 'sidekiq', '~> 5' end + + s.add_development_dependency 'curb', '~> 0.9' if RUBY_ENGINE == 'ruby' end diff --git a/lib/airbrake/rack.rb b/lib/airbrake/rack.rb index 544012767..0be1c7f0c 100644 --- a/lib/airbrake/rack.rb +++ b/lib/airbrake/rack.rb @@ -8,3 +8,27 @@ require 'airbrake/rack/route_filter' require 'airbrake/rack/middleware' require 'airbrake/rack/request_store' + +module Airbrake + # Rack is a namespace for all Rack-related code. + module Rack + # @api private + # @since 9.2.0 + def self.capture_http_performance + routes = Airbrake::Rack::RequestStore[:routes] + if !routes || routes.none? + response = yield + else + elapsed = Airbrake::Benchmark.measure do + response = yield + end + + routes.each do |_route_path, params| + params[:groups][:http] = elapsed + end + end + + response + end + end +end diff --git a/lib/airbrake/rails.rb b/lib/airbrake/rails.rb index b78bc42df..32edfd77f 100644 --- a/lib/airbrake/rails.rb +++ b/lib/airbrake/rails.rb @@ -63,19 +63,22 @@ class Railtie < ::Rails::Railtie 'process_action.action_controller', Airbrake::Rails::ActionControllerNotifySubscriber.new ) - - # Send performance breakdown: where a request spends its time. - require 'airbrake/rails/action_controller_performance_breakdown_subscriber' - ActiveSupport::Notifications.subscribe( - 'process_action.action_controller', - Airbrake::Rails::ActionControllerPerformanceBreakdownSubscriber.new - ) - - require 'airbrake/rails/net_http' end end end + initializer('airbrake.http_breakdown') do + # Send performance breakdown: where a request spends its time. + require 'airbrake/rails/action_controller_performance_breakdown_subscriber' + ActiveSupport::Notifications.subscribe( + 'process_action.action_controller', + Airbrake::Rails::ActionControllerPerformanceBreakdownSubscriber.new + ) + + require 'airbrake/rails/net_http' if defined?(Net) && defined?(Net::HTTP) + require 'airbrake/rails/curb' if defined?(Curl) && defined?(Curl::CURB_VERSION) + end + initializer('airbrake.active_record') do ActiveSupport.on_load(:active_record) do # Reports exceptions occurring in some bugged ActiveRecord callbacks. diff --git a/lib/airbrake/rails/curb.rb b/lib/airbrake/rails/curb.rb new file mode 100644 index 000000000..24eee765a --- /dev/null +++ b/lib/airbrake/rails/curb.rb @@ -0,0 +1,35 @@ +module Curl + # Monkey-patch to measure request timing. + class Easy + alias http_without_airbrake http + + def http(verb) + Airbrake::Rack.capture_http_performance do + http_without_airbrake(verb) + end + end + + alias perform_without_airbrake perform + + def perform(&block) + Airbrake::Rack.capture_http_performance do + perform_without_airbrake(&block) + end + end + end +end + +module Curl + # Monkey-patch to measure request timing. + class Multi + class << self + alias http_without_airbrake http + + def http(urls_with_config, multi_options = {}, &block) + Airbrake::Rack.capture_http_performance do + http_without_airbrake(urls_with_config, multi_options, &block) + end + end + end + end +end diff --git a/lib/airbrake/rails/net_http.rb b/lib/airbrake/rails/net_http.rb index 186addd63..53a2e2786 100644 --- a/lib/airbrake/rails/net_http.rb +++ b/lib/airbrake/rails/net_http.rb @@ -1,23 +1,10 @@ -require 'net/http' - # Monkey-patch Net::HTTP to benchmark it. Net::HTTP.class_eval do alias_method :request_without_airbrake, :request def request(request, *args, &block) - routes = Airbrake::Rack::RequestStore[:routes] - if !routes || routes.none? - response = request_without_airbrake(request, *args, &block) - else - elapsed = Airbrake::Benchmark.measure do - response = request_without_airbrake(request, *args, &block) - end - - routes.each do |route_path, _params| - Airbrake::Rack::RequestStore[:routes][route_path][:groups][:http] = elapsed - end + Airbrake::Rack.capture_http_performance do + request_without_airbrake(request, *args, &block) end - - response end end diff --git a/spec/apps/rails/dummy_app.rb b/spec/apps/rails/dummy_app.rb index 615441663..dabed40f7 100644 --- a/spec/apps/rails/dummy_app.rb +++ b/spec/apps/rails/dummy_app.rb @@ -1,3 +1,5 @@ +require 'curb' unless Airbrake::JRUBY + class DummyApp < Rails::Application # Rails requires these two keys. config.session_store :cookie_store, key: 'jiez4Mielu1AiHugog3shiiPhe3lai3faer' @@ -33,6 +35,9 @@ class DummyApp < Rails::Application get '/breakdown' => 'dummy#breakdown' get '/breakdown_view_only' => 'dummy#breakdown_view_only' get '/breakdown_http' => 'dummy#breakdown_http' + get '/breakdown_curl_http' => 'dummy#breakdown_curl_http' + get '/breakdown_curl_http_easy' => 'dummy#breakdown_curl_http_easy' + get '/breakdown_curl_http_multi' => 'dummy#breakdown_curl_http_multi' get '/notify_airbrake_helper' => 'dummy#notify_airbrake_helper' get '/notify_airbrake_sync_helper' => 'dummy#notify_airbrake_sync_helper' get '/active_record_after_commit' => 'dummy#active_record_after_commit' @@ -106,7 +111,10 @@ class DummyController < ActionController::Base 'dummy/delayed_job.html.erb' => 'delayed_job', 'dummy/breakdown.html.erb' => 'breakdown', 'dummy/breakdown_view_only.html.erb' => 'breakdown_view_only', - 'dummy/breakdown_http.html.erb' => 'breakdown_http' + 'dummy/breakdown_http.html.erb' => 'breakdown_http', + 'dummy/breakdown_curl_http.html.erb' => 'breakdown_curl_http', + 'dummy/breakdown_curl_http_easy.html.erb' => 'breakdown_curl_http_easy', + 'dummy/breakdown_curl_http_multi.html.erb' => 'breakdown_curl_http_multi' ) ] @@ -131,6 +139,21 @@ def breakdown_http render 'dummy/breakdown_http.html.erb' end + def breakdown_curl_http + Curl.get('example.com') + render 'dummy/breakdown_curl_http.html.erb' + end + + def breakdown_curl_http_easy + Curl::Easy.perform('example.com') + render 'dummy/breakdown_curl_http_easy.html.erb' + end + + def breakdown_curl_http_multi + Curl::Multi.get(['example.com']) + render 'dummy/breakdown_curl_http_multi.html.erb' + end + def notify_airbrake_helper notify_airbrake(AirbrakeTestError.new) end diff --git a/spec/integration/rails/rails_spec.rb b/spec/integration/rails/rails_spec.rb index 58da1c302..604f6a85a 100644 --- a/spec/integration/rails/rails_spec.rb +++ b/spec/integration/rails/rails_spec.rb @@ -322,7 +322,7 @@ end end - context "when an action performs a HTTP request" do + context "when an action performs a Net::HTTP request" do let!(:example_request) do stub_request(:get, 'http://example.com').to_return(body: '') end @@ -334,5 +334,45 @@ expect(example_request).to have_been_made end end + + context "when an action performs a Curl request" do + let!(:example_request) do + stub_request(:get, 'http://example.com').to_return(body: '') + end + + before { skip("JRuby doesn't support Curb") if Airbrake::JRUBY } + + it "includes the http breakdown" do + expect(Airbrake).to receive(:notify_performance_breakdown) + .with(hash_including(groups: { view: be > 0, http: be > 0 })) + get '/breakdown_curl_http' + expect(example_request).to have_been_made + end + end + + context "when an action performs a Curl::Easy request" do + let!(:example_request) do + stub_request(:get, 'http://example.com').to_return(body: '') + end + + before { skip("JRuby doesn't support Curb") if Airbrake::JRUBY } + + it "includes the http breakdown" do + expect(Airbrake).to receive(:notify_performance_breakdown) + .with(hash_including(groups: { view: be > 0, http: be > 0 })) + get '/breakdown_curl_http_easy' + expect(example_request).to have_been_made + end + end + + context "when an action performs a Curl::Multi request" do + before { skip("JRuby doesn't support Curb") if Airbrake::JRUBY } + + it "includes the http breakdown" do + expect(Airbrake).to receive(:notify_performance_breakdown) + .with(hash_including(groups: { view: be > 0, http: be > 0 })) + get '/breakdown_curl_http_multi' + end + end end end diff --git a/spec/unit/rack/rack_spec.rb b/spec/unit/rack/rack_spec.rb new file mode 100644 index 000000000..168967f4e --- /dev/null +++ b/spec/unit/rack/rack_spec.rb @@ -0,0 +1,37 @@ +RSpec.describe Airbrake::Rack do + describe ".capture_http_performance" do + context "when request store doesn't have any routes" do + before { Airbrake::Rack::RequestStore.clear } + + it "doesn't store http time" do + described_class.capture_http_performance {} + expect(Airbrake::Rack::RequestStore.store).to be_empty + end + + it "returns what the given block returns" do + expect(described_class.capture_http_performance { 1 }).to eq(1) + end + end + + context "when request store has a route" do + let(:routes) { Airbrake::Rack::RequestStore[:routes] } + + before do + Airbrake::Rack::RequestStore[:routes] = { + '/about' => { + method: 'GET', + response_type: :html, + groups: {} + } + } + end + + after { Airbrake::Rack::RequestStore.clear } + + it "attaches http timing to the route" do + described_class.capture_http_performance {} + expect(routes['/about'][:groups][:http]).to be > 0 + end + end + end +end