Update Orchestra instrumentations and move part of logging to Orchestra.

4 files changed, 38 insertions, 15 deletions

diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb
index a415686020..dfd53462af 100644
--- a/activesupport/lib/active_support/cache.rb
+++ b/activesupport/lib/active_support/cache.rb
@@ -219,7 +219,6 @@ module ActiveSupport
       end
 
       def increment(key, amount = 1)
-        log("incrementing", key, amount)
         if num = read(key)
           write(key, num + amount)
         else
@@ -228,7 +227,6 @@ module ActiveSupport
       end
 
       def decrement(key, amount = 1)
-        log("decrementing", key, amount)
         if num = read(key)
           write(key, num - amount)
         else
@@ -247,13 +245,14 @@ module ActiveSupport
           payload = { :key => key }
           payload.merge!(options) if options.is_a?(Hash)
 
-          event = ActiveSupport::Orchestra.instrument(:"cache_#{operation}", payload, &block)
-          log("#{operation} (%.1fms)" % event.duration, key, options)
-          event.result
+          # Cache events should be logged or not?
+          # log(operation, key, options)
+          ActiveSupport::Orchestra.instrument(:"cache_#{operation}", payload, &block)
         end
 
         def log(operation, key, options)
-          logger.debug("Cache #{operation}: #{key}#{options ? " (#{options.inspect})" : ""}") if logger && !silence?
+          return unless logger && !silence?
+          logger.debug("Cache #{operation}: #{key}#{options ? " (#{options.inspect})" : ""}")
         end
     end
   end
diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb
index 516af99ce9..bec9de86ed 100644
--- a/activesupport/lib/active_support/cache/mem_cache_store.rb
+++ b/activesupport/lib/active_support/cache/mem_cache_store.rb
@@ -103,17 +103,20 @@ module ActiveSupport
       end
 
       def increment(key, amount = 1) # :nodoc:
-        log("incrementing", key, amount)
+        response = instrument(:increment, key, :amount => amount) do
+          @data.incr(key, amount)
+        end
 
-        response = @data.incr(key, amount)
         response == Response::NOT_FOUND ? nil : response
       rescue MemCache::MemCacheError
         nil
       end
 
       def decrement(key, amount = 1) # :nodoc:
-        log("decrement", key, amount)
-        response = @data.decr(key, amount)
+        response = instrument(:decrement, key, :amount => amount) do
+          @data.decr(key, amount)
+        end
+
         response == Response::NOT_FOUND ? nil : response
       rescue MemCache::MemCacheError
         nil
diff --git a/activesupport/lib/active_support/orchestra.rb b/activesupport/lib/active_support/orchestra.rb
index 7c9c3074e3..7d4c25669d 100644
--- a/activesupport/lib/active_support/orchestra.rb
+++ b/activesupport/lib/active_support/orchestra.rb
@@ -66,7 +66,7 @@ module ActiveSupport
       def instrument(name, payload={})
         payload[:time]      = Time.now
         payload[:thread_id] = Thread.current.object_id
-        payload[:result]    = yield
+        payload[:result]    = yield if block_given?
       ensure
         payload[:duration] = 1000 * (Time.now.to_f - payload[:time].to_f)
         @publisher.publish(name, payload)
@@ -147,7 +147,7 @@ module ActiveSupport
         end
 
         def publish(name, payload)
-          unless @pattern && name.to_s !~ @pattern
+          unless @pattern && !(@pattern === name.to_s)
             @queue << [name, payload]
           end
         end
diff --git a/activesupport/test/orchestra_test.rb b/activesupport/test/orchestra_test.rb
index 810d99ebeb..7a6e9208b4 100644
--- a/activesupport/test/orchestra_test.rb
+++ b/activesupport/test/orchestra_test.rb
@@ -90,18 +90,39 @@ class OrchestraMainTest < Test::Unit::TestCase
     assert_equal Hash[:payload => "orchestra"], @events.last.payload
   end
 
+  def test_event_is_pushed_even_without_block
+    ActiveSupport::Orchestra.instrument(:awesome, :payload => "orchestra")
+    sleep(0.1)
+
+    assert_equal 1, @events.size
+    assert_equal :awesome, @events.last.name
+    assert_equal Hash[:payload => "orchestra"], @events.last.payload
+  end
+
   def test_subscriber_with_pattern
     @another = []
-    ActiveSupport::Orchestra.subscribe(/cache/) { |event| @another << event }
+    ActiveSupport::Orchestra.subscribe("cache"){ |event| @another << event }
+    ActiveSupport::Orchestra.instrument(:cache){ 1 }
+
+    sleep(0.1)
+
+    assert_equal 1, @another.size
+    assert_equal :cache, @another.first.name
+    assert_equal 1, @another.first.result
+  end
+
+  def test_subscriber_with_pattern_as_regexp
+    @another = []
+    ActiveSupport::Orchestra.subscribe(/cache/){ |event| @another << event }
 
     ActiveSupport::Orchestra.instrument(:something){ 0 }
-    ActiveSupport::Orchestra.instrument(:cache){ 10 }
+    ActiveSupport::Orchestra.instrument(:cache){ 1 }
 
     sleep(0.1)
 
     assert_equal 1, @another.size
     assert_equal :cache, @another.first.name
-    assert_equal 10, @another.first.result
+    assert_equal 1, @another.first.result
   end
 
   def test_with_several_consumers_and_several_events