From 3b5d940fe5868af0f31989054e8fb7d2d9d11ec9 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Mon, 5 Feb 2018 18:46:55 +0200 Subject: Add missing instrumentation to RedisCacheStore#read_multi --- .../lib/active_support/cache/redis_cache_store.rb | 6 ++- activesupport/test/cache/behaviors.rb | 1 + .../behaviors/cache_instrumentation_behavior.rb | 60 +++++++++++++++++++++ .../test/cache/cache_store_write_multi_test.rb | 62 ---------------------- activesupport/test/cache/stores/file_store_test.rb | 1 + .../test/cache/stores/mem_cache_store_test.rb | 1 + .../test/cache/stores/memory_store_test.rb | 1 + .../test/cache/stores/redis_cache_store_test.rb | 1 + 8 files changed, 70 insertions(+), 63 deletions(-) create mode 100644 activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb delete mode 100644 activesupport/test/cache/cache_store_write_multi_test.rb (limited to 'activesupport') diff --git a/activesupport/lib/active_support/cache/redis_cache_store.rb b/activesupport/lib/active_support/cache/redis_cache_store.rb index 596541087f..c21ade8670 100644 --- a/activesupport/lib/active_support/cache/redis_cache_store.rb +++ b/activesupport/lib/active_support/cache/redis_cache_store.rb @@ -187,7 +187,11 @@ module ActiveSupport # fetched values. def read_multi(*names) if mget_capable? - read_multi_mget(*names) + instrument(:read_multi, names, options) do |payload| + read_multi_mget(*names).tap do |results| + payload[:hits] = results.keys + end + end else super end diff --git a/activesupport/test/cache/behaviors.rb b/activesupport/test/cache/behaviors.rb index 745dc09e2c..2d39976be3 100644 --- a/activesupport/test/cache/behaviors.rb +++ b/activesupport/test/cache/behaviors.rb @@ -3,6 +3,7 @@ require_relative "behaviors/autoloading_cache_behavior" require_relative "behaviors/cache_delete_matched_behavior" require_relative "behaviors/cache_increment_decrement_behavior" +require_relative "behaviors/cache_instrumentation_behavior" require_relative "behaviors/cache_store_behavior" require_relative "behaviors/cache_store_version_behavior" require_relative "behaviors/connection_pool_behavior" diff --git a/activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb b/activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb new file mode 100644 index 0000000000..4e8ff60eb3 --- /dev/null +++ b/activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module CacheInstrumentationBehavior + def test_fetch_multi_uses_write_multi_entries_store_provider_interface + assert_called_with(@cache, :write_multi_entries) do + @cache.fetch_multi "a", "b", "c" do |key| + key * 2 + end + end + end + + def test_write_multi_instrumentation + writes = { "a" => "aa", "b" => "bb" } + + events = with_instrumentation "write_multi" do + @cache.write_multi(writes) + end + + assert_equal %w[ cache_write_multi.active_support ], events.map(&:name) + assert_nil events[0].payload[:super_operation] + assert_equal({ "a" => "aa", "b" => "bb" }, events[0].payload[:key]) + end + + def test_instrumentation_with_fetch_multi_as_super_operation + @cache.write("b", "bb") + + events = with_instrumentation "read_multi" do + @cache.fetch_multi("a", "b") { |key| key * 2 } + end + + assert_equal %w[ cache_read_multi.active_support ], events.map(&:name) + assert_equal :fetch_multi, events[0].payload[:super_operation] + assert_equal ["b"], events[0].payload[:hits] + end + + def test_read_multi_instrumentation + @cache.write("b", "bb") + + events = with_instrumentation "read_multi" do + @cache.read_multi("a", "b") { |key| key * 2 } + end + + assert_equal %w[ cache_read_multi.active_support ], events.map(&:name) + assert_equal ["b"], events[0].payload[:hits] + end + + private + def with_instrumentation(method) + event_name = "cache_#{method}.active_support" + + [].tap do |events| + ActiveSupport::Notifications.subscribe event_name do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end + yield + end + ensure + ActiveSupport::Notifications.unsubscribe event_name + end +end diff --git a/activesupport/test/cache/cache_store_write_multi_test.rb b/activesupport/test/cache/cache_store_write_multi_test.rb deleted file mode 100644 index 7d606e3f7b..0000000000 --- a/activesupport/test/cache/cache_store_write_multi_test.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -require "abstract_unit" -require "active_support/cache" - -class CacheStoreWriteMultiEntriesStoreProviderInterfaceTest < ActiveSupport::TestCase - setup do - @cache = ActiveSupport::Cache.lookup_store(:null_store) - end - - test "fetch_multi uses write_multi_entries store provider interface" do - assert_called_with(@cache, :write_multi_entries) do - @cache.fetch_multi "a", "b", "c" do |key| - key * 2 - end - end - end -end - -class CacheStoreWriteMultiInstrumentationTest < ActiveSupport::TestCase - setup do - @cache = ActiveSupport::Cache.lookup_store(:memory_store) - end - - test "instrumentation" do - writes = { "a" => "aa", "b" => "bb" } - - events = with_instrumentation "write_multi" do - @cache.write_multi(writes) - end - - assert_equal %w[ cache_write_multi.active_support ], events.map(&:name) - assert_nil events[0].payload[:super_operation] - assert_equal({ "a" => "aa", "b" => "bb" }, events[0].payload[:key]) - end - - test "instrumentation with fetch_multi as super operation" do - @cache.write("b", "bb") - - events = with_instrumentation "read_multi" do - @cache.fetch_multi("a", "b") { |key| key * 2 } - end - - assert_equal %w[ cache_read_multi.active_support ], events.map(&:name) - assert_equal :fetch_multi, events[0].payload[:super_operation] - assert_equal ["b"], events[0].payload[:hits] - end - - private - def with_instrumentation(method) - event_name = "cache_#{method}.active_support" - - [].tap do |events| - ActiveSupport::Notifications.subscribe event_name do |*args| - events << ActiveSupport::Notifications::Event.new(*args) - end - yield - end - ensure - ActiveSupport::Notifications.unsubscribe event_name - end -end diff --git a/activesupport/test/cache/stores/file_store_test.rb b/activesupport/test/cache/stores/file_store_test.rb index 67eff9b94f..c3c35a7bcc 100644 --- a/activesupport/test/cache/stores/file_store_test.rb +++ b/activesupport/test/cache/stores/file_store_test.rb @@ -30,6 +30,7 @@ class FileStoreTest < ActiveSupport::TestCase include LocalCacheBehavior include CacheDeleteMatchedBehavior include CacheIncrementDecrementBehavior + include CacheInstrumentationBehavior include AutoloadingCacheBehavior def test_clear diff --git a/activesupport/test/cache/stores/mem_cache_store_test.rb b/activesupport/test/cache/stores/mem_cache_store_test.rb index 3e2316f217..f426a37c66 100644 --- a/activesupport/test/cache/stores/mem_cache_store_test.rb +++ b/activesupport/test/cache/stores/mem_cache_store_test.rb @@ -49,6 +49,7 @@ class MemCacheStoreTest < ActiveSupport::TestCase include CacheStoreVersionBehavior include LocalCacheBehavior include CacheIncrementDecrementBehavior + include CacheInstrumentationBehavior include EncodedKeyCacheBehavior include AutoloadingCacheBehavior include ConnectionPoolBehavior diff --git a/activesupport/test/cache/stores/memory_store_test.rb b/activesupport/test/cache/stores/memory_store_test.rb index 3981f05331..72fafc187b 100644 --- a/activesupport/test/cache/stores/memory_store_test.rb +++ b/activesupport/test/cache/stores/memory_store_test.rb @@ -14,6 +14,7 @@ class MemoryStoreTest < ActiveSupport::TestCase include CacheStoreVersionBehavior include CacheDeleteMatchedBehavior include CacheIncrementDecrementBehavior + include CacheInstrumentationBehavior def test_prune_size @cache.write(1, "aaaaaaaaaa") && sleep(0.001) diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index 62752d2c65..22b519daf1 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -107,6 +107,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests include CacheStoreVersionBehavior include LocalCacheBehavior include CacheIncrementDecrementBehavior + include CacheInstrumentationBehavior include AutoloadingCacheBehavior end -- cgit v1.2.3