diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c2b657..273bbf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +### master +* Add support to instrument all controller actions, e.g., `instrument_action :all`. + ### Version 1.3.0 * Add support for configurable metric suites diff --git a/README.md b/README.md index 55271a4..4633595 100644 --- a/README.md +++ b/README.md @@ -269,6 +269,22 @@ class CommentController < ApplicationController end ``` +Optionally, you can instrument all controller actions: + +```ruby +class ArticlesController < ApplicationController + instrument_action :all + + def create + # ... + end + + def show + # ... + end +end +``` + Once you instrument an action, `librato-rails` will start reporting a set of metrics specific to that action including: * rails.action.request.total (# of requests) diff --git a/lib/librato/rails/helpers/controller.rb b/lib/librato/rails/helpers/controller.rb index 31844ac..88ad38d 100644 --- a/lib/librato/rails/helpers/controller.rb +++ b/lib/librato/rails/helpers/controller.rb @@ -10,7 +10,12 @@ def instrument_action(*actions) end end + def inherited(other) + super + Subscribers.inherit_watches(self.to_s, other.to_s) + end + end end end -end \ No newline at end of file +end diff --git a/lib/librato/rails/railtie.rb b/lib/librato/rails/railtie.rb index d92dfa7..19aace1 100644 --- a/lib/librato/rails/railtie.rb +++ b/lib/librato/rails/railtie.rb @@ -5,7 +5,7 @@ class Railtie < ::Rails::Railtie # don't have any custom http vars anymore, check if hostname is UUID on_heroku = Socket.gethostname =~ /[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/i - config.before_configuration do + config.before_configuration do # make configuration proxy for config inside Rails config.librato_rails = Configuration.new diff --git a/lib/librato/rails/subscribers.rb b/lib/librato/rails/subscribers.rb index c7b11df..12c2fa8 100644 --- a/lib/librato/rails/subscribers.rb +++ b/lib/librato/rails/subscribers.rb @@ -13,7 +13,20 @@ def self.collector def self.watch_controller_action(controller, action) @watches ||= [] - @watches << "#{controller}##{action}".freeze + + watch = + if action == :all + "#{controller}".freeze + else + "#{controller}##{action}".freeze + end + + @watches << watch + end + + def self.inherit_watches(base, descendant) + @watches ||= [] + @watches << descendant.freeze if @watches.include?(base) end end end diff --git a/lib/librato/rails/subscribers/action.rb b/lib/librato/rails/subscribers/action.rb index 3c3c9d3..380e9a8 100644 --- a/lib/librato/rails/subscribers/action.rb +++ b/lib/librato/rails/subscribers/action.rb @@ -14,7 +14,7 @@ module Subscribers format = "all" if format == "*/*" exception = event.payload[:exception] - if @watches && @watches.index("#{controller}##{action}") + if @watches && (@watches.index(controller) || @watches.index("#{controller}##{action}")) source = "#{controller}.#{action}.#{format}" collector.group 'rails.action.request' do |r| diff --git a/test/dummy/app/controllers/application_controller.rb b/test/dummy/app/controllers/application_controller.rb index 245f328..508b77a 100644 --- a/test/dummy/app/controllers/application_controller.rb +++ b/test/dummy/app/controllers/application_controller.rb @@ -1,8 +1,8 @@ class ApplicationController < ActionController::Base protect_from_forgery - + instrument_action :all #after_filter :flush_metrics_rails - + # manually flush per request def flush_metrics_rails Librato::Rails.flush diff --git a/test/dummy/app/controllers/base_controller.rb b/test/dummy/app/controllers/base_controller.rb new file mode 100644 index 0000000..9dbd0c5 --- /dev/null +++ b/test/dummy/app/controllers/base_controller.rb @@ -0,0 +1,11 @@ +class BaseController < ApplicationController + instrument_action :all + + def action_1 + render nothing: true + end + + def action_2 + render nothing: true + end +end diff --git a/test/dummy/app/controllers/derived_controller.rb b/test/dummy/app/controllers/derived_controller.rb new file mode 100644 index 0000000..1a63ea7 --- /dev/null +++ b/test/dummy/app/controllers/derived_controller.rb @@ -0,0 +1,9 @@ +class DerivedController < IntermediateController + def action_1 + render nothing: true + end + + def action_2 + render nothing: true + end +end diff --git a/test/dummy/app/controllers/instrument_action_controller.rb b/test/dummy/app/controllers/instrument_action_controller.rb index d29b640..aaf8ead 100644 --- a/test/dummy/app/controllers/instrument_action_controller.rb +++ b/test/dummy/app/controllers/instrument_action_controller.rb @@ -10,7 +10,7 @@ def inst end end - def not_instrumented + def not render nothing: true end diff --git a/test/dummy/app/controllers/intermediate_controller.rb b/test/dummy/app/controllers/intermediate_controller.rb new file mode 100644 index 0000000..9da04ba --- /dev/null +++ b/test/dummy/app/controllers/intermediate_controller.rb @@ -0,0 +1,5 @@ +class IntermediateController < BaseController + def action_1 + render nothing: true + end +end diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 6c7801a..27dbfb2 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -20,6 +20,12 @@ get 'instrument/inst' => 'instrument_action#inst', :as => :instrument_action get 'instrument/not' => 'instrument_action#not', :as => :not_instrumented + get 'base/action_1' => 'base#action_1', :as => :base_action_1 + get 'base/action_2' => 'base#action_2', :as => :base_action_2 + get 'intermediate/action_1' => 'intermediate#action_1', :as => :intermediate_action_1 + get 'derived/action_1' => 'derived#action_1', :as => :derived_action_1 + get 'derived/action_2' => 'derived#action_2', :as => :derived_action_2 + # The priority is based upon order of creation: # first created -> highest priority. diff --git a/test/integration/instrument_action_test.rb b/test/integration/instrument_action_test.rb index c3a80fa..9ed4450 100644 --- a/test/integration/instrument_action_test.rb +++ b/test/integration/instrument_action_test.rb @@ -19,4 +19,34 @@ class InstrumentActionTest < ActiveSupport::IntegrationCase assert_equal 1, counters.fetch("#{base}.total", source: source) end + test 'instrument all controller actions' do + visit base_action_1_path + visit base_action_2_path + + metric = 'rails.action.request.time' + + assert_equal 1, aggregate.fetch(metric, source: 'BaseController.action_1.html')[:count] + assert_equal 1, aggregate.fetch(metric, source: 'BaseController.action_2.html')[:count] + end + + test 'instrument all controller actions for inherited controllers' do + visit intermediate_action_1_path + visit derived_action_1_path + visit derived_action_2_path + + metric = 'rails.action.request.time' + + assert_equal 1, aggregate.fetch(metric, source: 'IntermediateController.action_1.html')[:count] + assert_equal 1, aggregate.fetch(metric, source: 'DerivedController.action_1.html')[:count] + assert_equal 1, aggregate.fetch(metric, source: 'DerivedController.action_2.html')[:count] + end + + test 'instrument all controller actions for all controllers' do + visit not_instrumented_path + + metric = 'rails.action.request.time' + + assert_equal 1, aggregate.fetch(metric, source: 'InstrumentActionController.not.html')[:count] + end + end