fix: add back key for cache notification payloads (#600)

* fix: add back key for cache notification payloads

This time we call `ActiveSupport::Cache.expand_cache_key` on the value
for a more usable key for querying.

* Handle multi operations differently

* The hits data also contain keys, also add specs.

* Use a mock instead of copying the logic

Author: roelbondoc

lib/honeybadger/notification_subscriber.rb

@@ -41,13 +41,33 @@ def format_payload(payload)
 
   class ActionControllerCacheSubscriber < NotificationSubscriber
     def format_payload(payload)
-      payload.except(:key)
+      payload[:key] = ::ActiveSupport::Cache.expand_cache_key(payload[:key]) if payload[:key]
+      payload
     end
   end
 
   class ActiveSupportCacheSubscriber < NotificationSubscriber
     def format_payload(payload)
-      payload.except(:key)
+      payload[:key] = ::ActiveSupport::Cache.expand_cache_key(payload[:key]) if payload[:key]
+      payload
+    end
+  end
+
+  class ActiveSupportCacheMultiSubscriber < NotificationSubscriber
+    def format_payload(payload)
+      payload[:key] = expand_cache_keys_from_payload(payload[:key])
+      payload[:hits] = expand_cache_keys_from_payload(payload[:hits])
+      payload
+    end
+
+    def expand_cache_keys_from_payload(data)
+      return unless data
+
+      data = data.keys if data.is_a?(Hash)
+
+      Array(data).map do |k|
+        ::ActiveSupport::Cache.expand_cache_key(k)
+      end
     end
   end
 

lib/honeybadger/plugins/rails.rb

@@ -77,7 +77,8 @@ def self.source_ignored?(source)
           if config.load_plugin_insights?(:rails)
             ::ActiveSupport::Notifications.subscribe(/(process_action|send_file|redirect_to|halted_callback|unpermitted_parameters)\.action_controller/, Honeybadger::ActionControllerSubscriber.new)
             ::ActiveSupport::Notifications.subscribe(/(write_fragment|read_fragment|expire_fragment|exist_fragment\?)\.action_controller/, Honeybadger::ActionControllerCacheSubscriber.new)
-            ::ActiveSupport::Notifications.subscribe(/cache_(read|read_multi|generate|fetch_hit|write|write_multi|increment|decrement|delete|delete_multi|cleanup|prune|exist\?)\.active_support/, Honeybadger::ActiveSupportCacheSubscriber.new)
+            ::ActiveSupport::Notifications.subscribe(/cache_(read|generate|fetch_hit|write|increment|decrement|delete|cleanup|prune|exist\?)\.active_support/, Honeybadger::ActiveSupportCacheSubscriber.new)
+            ::ActiveSupport::Notifications.subscribe(/cache_(read_multi|write_multi|delete_multi\?)\.active_support/, Honeybadger::ActiveSupportCacheMultiSubscriber.new)
             ::ActiveSupport::Notifications.subscribe(/^render_(template|partial|collection)\.action_view/, Honeybadger::ActionViewSubscriber.new)
             ::ActiveSupport::Notifications.subscribe("sql.active_record", Honeybadger::ActiveRecordSubscriber.new)
             ::ActiveSupport::Notifications.subscribe("process.action_mailer", Honeybadger::ActionMailerSubscriber.new)

spec/unit/honeybadger/notification_subscriber_spec.rb

@@ -0,0 +1,59 @@
+# encoding: utf-8
+
+require 'honeybadger/notification_subscriber'
+
+describe Honeybadger::ActiveSupportCacheMultiSubscriber do
+  module ActiveSupport
+    module Cache; end
+  end
+
+  context "with a cache_write_multi.active_support payload" do
+    let(:payload) do
+      obj = Object.new
+      {
+        key: {'one' => 'data', 'object.cache_key' => obj },
+        store: 'cache-store-name'
+      }
+    end
+
+    before do
+      allow(::ActiveSupport::Cache).to receive(:expand_cache_key).with(payload[:key].keys[0]).and_return('one')
+      allow(::ActiveSupport::Cache).to receive(:expand_cache_key).with(payload[:key].keys[1]).and_return('foo/bar')
+    end
+
+    subject { described_class.new.format_payload(payload) }
+
+    it "returns a payload with all keys expanded and without cache values" do
+      expect(subject).to be_a(Hash)
+      expect(subject[:key]).to eq(%w(one foo/bar))
+      expect(subject[:store]).to eq('cache-store-name')
+    end
+  end
+
+  context "with a cache_read_multi.active_support payload" do
+    let(:payload) do
+      {
+        key: ['one', Object.new],
+        hits: ['one'],
+        store: 'cache-store-name',
+        super_operation: :fetch_multi
+      }
+    end
+
+    before do
+      allow(::ActiveSupport::Cache).to receive(:expand_cache_key).with(payload[:key][0]).and_return('one')
+      allow(::ActiveSupport::Cache).to receive(:expand_cache_key).with(payload[:key][1]).and_return('foo/bar')
+      allow(::ActiveSupport::Cache).to receive(:expand_cache_key).with(payload[:hits][0]).and_return('one')
+    end
+
+    subject { described_class.new.format_payload(payload) }
+
+    it "returns a payload with all keys expanded" do
+      expect(subject).to be_a(Hash)
+      expect(subject[:key]).to eq(%w(one foo/bar))
+      expect(subject[:hits]).to eq(%w(one))
+      expect(subject[:store]).to eq('cache-store-name')
+      expect(subject[:super_operation]).to eq(:fetch_multi)
+    end
+  end
+end