From cbebd439d48c85fce230fd86597f7aec59ebdae0 Mon Sep 17 00:00:00 2001 From: Justin George Date: Tue, 27 Apr 2010 21:16:06 -0700 Subject: [PATCH] Change event namespace ordering to most-significant first [#4504 state:resolved] More work still needs to be done on some of these names (render_template.action_view and render_template!.action_view particularly) but this allows (for example) /^sql/ to subscribe to all the various ORMs without further modification --- actionmailer/lib/action_mailer/base.rb | 4 ++-- .../lib/action_controller/caching/fragments.rb | 2 +- actionpack/lib/action_controller/caching/pages.rb | 2 +- .../lib/action_controller/metal/instrumentation.rb | 10 +++++----- actionpack/lib/action_controller/test_case.rb | 8 ++++---- actionpack/lib/action_view/render/partials.rb | 4 ++-- actionpack/lib/action_view/render/rendering.rb | 2 +- actionpack/lib/action_view/template.rb | 2 +- .../connection_adapters/abstract/query_cache.rb | 2 +- .../connection_adapters/abstract_adapter.rb | 2 +- activeresource/lib/active_resource/connection.rb | 2 +- activesupport/lib/active_support/cache.rb | 2 +- railties/lib/rails/log_subscriber.rb | 6 +++--- .../application/initializers/notifications_test.rb | 2 +- railties/test/log_subscriber_test.rb | 18 +++++++++--------- 15 files changed, 34 insertions(+), 34 deletions(-) diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index e566132..8be47b3 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -316,7 +316,7 @@ module ActionMailer #:nodoc: # end # end def receive(raw_mail) - ActiveSupport::Notifications.instrument("action_mailer.receive") do |payload| + ActiveSupport::Notifications.instrument("receive.action_mailer") do |payload| mail = Mail.new(raw_mail) set_payload_for_mail(payload, mail) new.receive(mail) @@ -328,7 +328,7 @@ module ActionMailer #:nodoc: # when you call :deliver on the Mail::Message, calling +deliver_mail+ directly # and passing a Mail::Message will do nothing except tell the logger you sent the email. def deliver_mail(mail) #:nodoc: - ActiveSupport::Notifications.instrument("action_mailer.deliver") do |payload| + ActiveSupport::Notifications.instrument("deliver.action_mailer") do |payload| self.set_payload_for_mail(payload, mail) yield # Let Mail do the delivery actions end diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 473a2fe..460273d 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -99,7 +99,7 @@ module ActionController #:nodoc: end def instrument_fragment_cache(name, key) - ActiveSupport::Notifications.instrument("action_controller.#{name}", :key => key){ yield } + ActiveSupport::Notifications.instrument("#{name}.action_controller", :key => key){ yield } end end end diff --git a/actionpack/lib/action_controller/caching/pages.rb b/actionpack/lib/action_controller/caching/pages.rb index cefd1f4..4f7a5d3 100644 --- a/actionpack/lib/action_controller/caching/pages.rb +++ b/actionpack/lib/action_controller/caching/pages.rb @@ -109,7 +109,7 @@ module ActionController #:nodoc: end def instrument_page_cache(name, path) - ActiveSupport::Notifications.instrument("action_controller.#{name}", :path => path){ yield } + ActiveSupport::Notifications.instrument("#{name}.action_controller", :path => path){ yield } end end diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index d69de65..ba38b18 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -23,9 +23,9 @@ module ActionController :path => (request.fullpath rescue "unknown") } - ActiveSupport::Notifications.instrument("action_controller.start_processing", raw_payload.dup) + ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) - ActiveSupport::Notifications.instrument("action_controller.process_action", raw_payload) do |payload| + ActiveSupport::Notifications.instrument("process_action.action_controller", raw_payload) do |payload| result = super payload[:status] = response.status append_info_to_payload(payload) @@ -42,20 +42,20 @@ module ActionController end def send_file(path, options={}) - ActiveSupport::Notifications.instrument("action_controller.send_file", + ActiveSupport::Notifications.instrument("send_file.action_controller", options.merge(:path => path)) do super end end def send_data(data, options = {}) - ActiveSupport::Notifications.instrument("action_controller.send_data", options) do + ActiveSupport::Notifications.instrument("send_data.action_controller", options) do super end end def redirect_to(*args) - ActiveSupport::Notifications.instrument("action_controller.redirect_to") do |payload| + ActiveSupport::Notifications.instrument("redirect_to.action_controller") do |payload| result = super payload[:status] = self.status payload[:location] = self.location diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 2b9b349..06ddc50 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -16,12 +16,12 @@ module ActionController @templates = Hash.new(0) @layouts = Hash.new(0) - ActiveSupport::Notifications.subscribe("action_view.render_template") do |name, start, finish, id, payload| + ActiveSupport::Notifications.subscribe("render_template.action_view") do |name, start, finish, id, payload| path = payload[:layout] @layouts[path] += 1 end - ActiveSupport::Notifications.subscribe("action_view.render_template!") do |name, start, finish, id, payload| + ActiveSupport::Notifications.subscribe("render_template!.action_view") do |name, start, finish, id, payload| path = payload[:virtual_path] next unless path partial = path =~ /^.*\/_[^\/]*$/ @@ -36,8 +36,8 @@ module ActionController end def teardown_subscriptions - ActiveSupport::Notifications.unsubscribe("action_view.render_template") - ActiveSupport::Notifications.unsubscribe("action_view.render_template!") + ActiveSupport::Notifications.unsubscribe("render_template.action_view") + ActiveSupport::Notifications.unsubscribe("render_template!.action_view") end # Asserts that the request was rendered with the appropriate template file or partials diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 4d23d55..9743456 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -211,12 +211,12 @@ module ActionView identifier = ((@template = find_template) ? @template.identifier : @path) if @collection - ActiveSupport::Notifications.instrument("action_view.render_collection", + ActiveSupport::Notifications.instrument("render_collection.action_view", :identifier => identifier || "collection", :count => @collection.size) do render_collection end else - content = ActiveSupport::Notifications.instrument("action_view.render_partial", + content = ActiveSupport::Notifications.instrument("render_partial.action_view", :identifier => identifier) do render_partial end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 4923269..4198013 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -52,7 +52,7 @@ module ActionView locals = options[:locals] || {} layout = find_layout(layout) if layout - ActiveSupport::Notifications.instrument("action_view.render_template", + ActiveSupport::Notifications.instrument("render_template.action_view", :identifier => template.identifier, :layout => layout.try(:virtual_path)) do content = template.render(self, locals) { |*name| _layout_for(*name) } diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 3c0cd35..09add48 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -41,7 +41,7 @@ module ActionView def render(view, locals, &block) # Notice that we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. - ActiveSupport::Notifications.instrument("action_view.render_template!", :virtual_path => @virtual_path) do + ActiveSupport::Notifications.instrument("render_template!.action_view", :virtual_path => @virtual_path) do method_name = compile(locals, view) view.send(method_name, locals, &block) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 3c56090..533a7bb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -76,7 +76,7 @@ module ActiveRecord def cache_sql(sql) result = if @query_cache.has_key?(sql) - ActiveSupport::Notifications.instrument("active_record.sql", + ActiveSupport::Notifications.instrument("sql.active_record", :sql => sql, :name => "CACHE", :connection_id => self.object_id) @query_cache[sql] else diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 5782970..28a59c1 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -197,7 +197,7 @@ module ActiveRecord def log(sql, name) name ||= "SQL" result = nil - ActiveSupport::Notifications.instrument("active_record.sql", + ActiveSupport::Notifications.instrument("sql.active_record", :sql => sql, :name => name, :connection_id => self.object_id) do @runtime += Benchmark.ms { result = yield } end diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index 2f0ccd7..b7befe1 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -106,7 +106,7 @@ module ActiveResource private # Makes a request to the remote service. def request(method, path, *arguments) - result = ActiveSupport::Notifications.instrument("active_resource.request") do |payload| + result = ActiveSupport::Notifications.instrument("request.active_resource") do |payload| payload[:method] = method payload[:request_uri] = "#{site.scheme}://#{site.host}:#{site.port}#{path}" payload[:result] = http.send(method, path, *arguments) diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index ec5007c..2605a3f 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -502,7 +502,7 @@ module ActiveSupport if self.class.instrument payload = { :key => key } payload.merge!(options) if options.is_a?(Hash) - ActiveSupport::Notifications.instrument("active_support.cache_#{operation}", payload){ yield } + ActiveSupport::Notifications.instrument("cache_#{operation}.active_support", payload){ yield } else yield end diff --git a/railties/lib/rails/log_subscriber.rb b/railties/lib/rails/log_subscriber.rb index 42697d2..145c7e0 100644 --- a/railties/lib/rails/log_subscriber.rb +++ b/railties/lib/rails/log_subscriber.rb @@ -22,7 +22,7 @@ module Rails # # Rails::LogSubscriber.add :active_record, ActiveRecord::Railtie::LogSubscriber.new # - # So whenever a "active_record.sql" notification arrive to Rails::LogSubscriber, + # So whenever a "sql.active_record" notification arrive to Rails::LogSubscriber, # it will properly dispatch the event (ActiveSupport::Notifications::Event) to # the sql method. # @@ -54,7 +54,7 @@ module Rails log_subscribers << log_subscriber log_subscriber.public_methods(false).each do |event| - notifier.subscribe("#{namespace}.#{event}") do |*args| + notifier.subscribe("#{event}.#{namespace}") do |*args| next if log_subscriber.logger.nil? begin @@ -105,4 +105,4 @@ module Rails "#{bold}#{color}#{text}#{CLEAR}" end end -end \ No newline at end of file +end diff --git a/railties/test/application/initializers/notifications_test.rb b/railties/test/application/initializers/notifications_test.rb index b99cf5b..276950c 100644 --- a/railties/test/application/initializers/notifications_test.rb +++ b/railties/test/application/initializers/notifications_test.rb @@ -38,7 +38,7 @@ module ApplicationTests ActiveRecord::Base.logger = logger = MockLogger.new # Mimic ActiveRecord notifications - instrument "active_record.sql", :name => "SQL", :sql => "SHOW tables" + instrument "sql.active_record", :name => "SQL", :sql => "SHOW tables" wait assert_equal 1, logger.logged.size diff --git a/railties/test/log_subscriber_test.rb b/railties/test/log_subscriber_test.rb index 49288cf..a3a755a 100644 --- a/railties/test/log_subscriber_test.rb +++ b/railties/test/log_subscriber_test.rb @@ -61,21 +61,21 @@ class SyncLogSubscriberTest < ActiveSupport::TestCase def test_event_is_sent_to_the_registered_class Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber - instrument "my_log_subscriber.some_event" + instrument "some_event.my_log_subscriber" wait - assert_equal %w(my_log_subscriber.some_event), @logger.logged(:info) + assert_equal %w(some_event.my_log_subscriber), @logger.logged(:info) end def test_event_is_an_active_support_notifications_event Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber - instrument "my_log_subscriber.some_event" + instrument "some_event.my_log_subscriber" wait assert_kind_of ActiveSupport::Notifications::Event, @log_subscriber.event end def test_does_not_send_the_event_if_it_doesnt_match_the_class Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber - instrument "my_log_subscriber.unknown_event" + instrument "unknown_event.my_log_subscriber" wait # If we get here, it means that NoMethodError was not raised. end @@ -84,7 +84,7 @@ class SyncLogSubscriberTest < ActiveSupport::TestCase Rails.logger = nil @log_subscriber.expects(:some_event).never Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber - instrument "my_log_subscriber.some_event" + instrument "some_event.my_log_subscriber" wait end @@ -110,14 +110,14 @@ class SyncLogSubscriberTest < ActiveSupport::TestCase def test_logging_does_not_die_on_failures Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber - instrument "my_log_subscriber.puke" - instrument "my_log_subscriber.some_event" + instrument "puke.my_log_subscriber" + instrument "some_event.my_log_subscriber" wait assert_equal 1, @logger.logged(:info).size - assert_equal 'my_log_subscriber.some_event', @logger.logged(:info).last + assert_equal 'some_event.my_log_subscriber', @logger.logged(:info).last assert_equal 1, @logger.logged(:error).size - assert_equal 'Could not log "my_log_subscriber.puke" event. RuntimeError: puke', @logger.logged(:error).last + assert_equal 'Could not log "puke.my_log_subscriber" event. RuntimeError: puke', @logger.logged(:error).last end end \ No newline at end of file -- 1.7.0.3