Skip to content

Commit

Permalink
Align CacheStore Instrumentation keys and output
Browse files Browse the repository at this point in the history
  • Loading branch information
frederikspang committed Aug 28, 2024
1 parent f0fd6ab commit 3835113
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 22 deletions.
14 changes: 8 additions & 6 deletions activesupport/lib/active_support/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 12 additions & 2 deletions activesupport/lib/active_support/cache/file_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions activesupport/lib/active_support/cache/memory_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions activesupport/lib/active_support/cache/redis_cache_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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
5 changes: 0 additions & 5 deletions guides/source/active_support_instrumentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down

0 comments on commit 3835113

Please sign in to comment.