Merge pull request rails#52723 from frederikspang/feature/instrument-memorystore

Instrument ActiveSupport::Cache::MemoryStore#increment and #decrement

Author: rafaelfranca

activesupport/lib/active_support/cache.rb

@@ -538,10 +538,11 @@ def read_multi(*names)
 
         options = names.extract_options!
         options = merged_options(options)
+        keys    = names.map { |name| normalize_key(name, options) }
 
-        instrument_multi :read_multi, names, options do |payload|
+        instrument_multi :read_multi, keys, options do |payload|
           read_multi_entries(names, **options, event: payload).tap do |results|
-            payload[:hits] = results.keys
+            payload[:hits] = results.keys.map { |name| normalize_key(name, options) }
           end
         end
       end
@@ -551,8 +552,9 @@ def write_multi(hash, options = nil)
         return hash if hash.empty?
 
         options = merged_options(options)
+        normalized_hash = hash.transform_keys { |key| normalize_key(key, options) }
 
-        instrument_multi :write_multi, hash, options do |payload|
+        instrument_multi :write_multi, normalized_hash, options do |payload|
           entries = hash.each_with_object({}) do |(name, value), memo|
             memo[normalize_key(name, options)] = Entry.new(value, **options.merge(version: normalize_version(name, options)))
           end
@@ -596,9 +598,9 @@ def fetch_multi(*names)
 
         options = names.extract_options!
         options = merged_options(options)
-
+        keys    = names.map { |name| normalize_key(name, options) }
         writes  = {}
-        ordered = instrument_multi :read_multi, names, options do |payload|
+        ordered = instrument_multi :read_multi, keys, options do |payload|
           if options[:force]
             reads = {}
           else
@@ -610,7 +612,7 @@ def fetch_multi(*names)
           end
           writes.compact! if options[:skip_nil]
 
-          payload[:hits] = reads.keys
+          payload[:hits] = reads.keys.map { |name| normalize_key(name, options) }
           payload[:super_operation] = :fetch_multi
 
           ordered

activesupport/lib/active_support/cache/file_store.rb

@@ -58,7 +58,12 @@ def cleanup(options = nil)
       #   cache.increment("baz") # => 6
       #
       def increment(name, amount = 1, options = nil)
-        modify_value(name, amount, options)
+        options = merged_options(options)
+        key = normalize_key(name, options)
+
+        instrument(:increment, key, amount: amount) do
+          modify_value(name, amount, options)
+        end
       end
 
       # Decrement a cached integer value. Returns the updated value.
@@ -73,7 +78,12 @@ def increment(name, amount = 1, options = nil)
       #   cache.decrement("baz") # => 4
       #
       def decrement(name, amount = 1, options = nil)
-        modify_value(name, -amount, options)
+        options = merged_options(options)
+        key = normalize_key(name, options)
+
+        instrument(:decrement, key, amount: amount) do
+          modify_value(name, -amount, options)
+        end
       end
 
       def delete_matched(matcher, options = nil)

activesupport/lib/active_support/cache/memory_store.rb

@@ -147,7 +147,9 @@ def pruning?
       #   cache.increment("baz") # => 6
       #
       def increment(name, amount = 1, options = nil)
-        modify_value(name, amount, options)
+        instrument(:increment, name, amount: amount) do
+          modify_value(name, amount, options)
+        end
       end
 
       # Decrement a cached integer value. Returns the updated value.
@@ -162,7 +164,9 @@ def increment(name, amount = 1, options = nil)
       #   cache.decrement("baz") # => 4
       #
       def decrement(name, amount = 1, options = nil)
-        modify_value(name, -amount, options)
+        instrument(:decrement, name, amount: amount) do
+          modify_value(name, -amount, options)
+        end
       end
 
       # Deletes cache entries if the cache key matches a given pattern.

activesupport/lib/active_support/cache/redis_cache_store.rb

@@ -173,9 +173,12 @@ def read_multi(*names)
         return {} if names.empty?
 
         options = names.extract_options!
-        instrument_multi(:read_multi, names, options) do |payload|
+        options = merged_options(options)
+        keys    = names.map { |name| normalize_key(name, options) }
+
+        instrument_multi(:read_multi, keys, options) do |payload|
           read_multi_entries(names, **options).tap do |results|
-            payload[:hits] = results.keys
+            payload[:hits] = results.keys.map { |name| normalize_key(name, options) }
           end
         end
       end

activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb

@@ -14,7 +14,7 @@ def test_write_multi_instrumentation
 
     assert_equal %w[ cache_write_multi.active_support ], events.map(&:name)
     assert_nil events[0].payload[:super_operation]
-    assert_equal({ key_1 => value_1, key_2 => value_2 }, events[0].payload[:key])
+    assert_equal({ normalized_key(key_1) => value_1, normalized_key(key_2) => value_2 }, events[0].payload[:key])
   end
 
   def test_instrumentation_with_fetch_multi_as_super_operation
@@ -29,8 +29,8 @@ def test_instrumentation_with_fetch_multi_as_super_operation
 
     assert_equal %w[ cache_read_multi.active_support ], events.map(&:name)
     assert_equal :fetch_multi, events[0].payload[:super_operation]
-    assert_equal [key_2, key_1], events[0].payload[:key]
-    assert_equal [key_1], events[0].payload[:hits]
+    assert_equal [normalized_key(key_2), normalized_key(key_1)], events[0].payload[:key]
+    assert_equal [normalized_key(key_1)], events[0].payload[:hits]
     assert_equal @cache.class.name, events[0].payload[:store]
   end
 
@@ -59,8 +59,35 @@ def test_read_multi_instrumentation
     end
 
     assert_equal %w[ cache_read_multi.active_support ], events.map(&:name)
-    assert_equal [key_2, key_1], events[0].payload[:key]
-    assert_equal [key_1], events[0].payload[:hits]
+    assert_equal [normalized_key(key_2), normalized_key(key_1)], events[0].payload[:key]
+    assert_equal [normalized_key(key_1)], events[0].payload[:hits]
+    assert_equal @cache.class.name, events[0].payload[:store]
+  end
+
+  def test_increment_instrumentation
+    key_1 = SecureRandom.uuid
+    @cache.write(key_1, 0)
+
+    events = with_instrumentation "increment" do
+      @cache.increment(key_1)
+    end
+
+    assert_equal %w[ cache_increment.active_support ], events.map(&:name)
+    assert_equal normalized_key(key_1), events[0].payload[:key]
+    assert_equal @cache.class.name, events[0].payload[:store]
+  end
+
+
+  def test_decrement_instrumentation
+    key_1 = SecureRandom.uuid
+    @cache.write(key_1, 0)
+
+    events = with_instrumentation "decrement" do
+      @cache.decrement(key_1)
+    end
+
+    assert_equal %w[ cache_decrement.active_support ], events.map(&:name)
+    assert_equal normalized_key(key_1), events[0].payload[:key]
     assert_equal @cache.class.name, events[0].payload[:store]
   end
 
@@ -75,4 +102,8 @@ def with_instrumentation(method)
     ensure
       ActiveSupport::Notifications.unsubscribe event_name
     end
+
+    def normalized_key(key)
+      @cache.send(:normalize_key, key, @cache.options)
+    end
 end

guides/source/active_support_instrumentation.md

@@ -599,9 +599,6 @@ Cache stores may add their own data as well.
 
 #### `cache_increment.active_support`
 
-This event is only emitted when using [`MemCacheStore`][ActiveSupport::Cache::MemCacheStore]
-or [`RedisCacheStore`][ActiveSupport::Cache::RedisCacheStore].
-
 | Key       | Value                   |
 | --------- | ----------------------- |
 | `:key`    | Key used in the store   |
@@ -618,8 +615,6 @@ or [`RedisCacheStore`][ActiveSupport::Cache::RedisCacheStore].
 
 #### `cache_decrement.active_support`
 
-This event is only emitted when using the Memcached or Redis cache stores.
-
 | Key       | Value                   |
 | --------- | ----------------------- |
 | `:key`    | Key used in the store   |