Fix session data for Action Cable callbacks (#1314)

In issue #1313 it was reported that session data was not reported for
Action Cable callbacks. This broke after PR #1209 where we changed the
way we handle sample data by allowing merging, which required more
strict type checks.

By calling `to_h`, ensure the value is a Hash, which is a valid sample
data type.

Fixes #1313

.changesets/fix-session-data-for-action-cable-callbacks.md

@@ -0,0 +1,6 @@
+---
+bump: patch
+type: fix
+---
+
+Fix session data reporting for Action Cable actions.

lib/appsignal/hooks/action_cable.rb

@@ -52,7 +52,7 @@ def install_subscribe_callback
             transaction.set_metadata("method", "websocket")
             transaction.add_params_if_nil { request.params }
             transaction.add_headers_if_nil { request.env }
-            transaction.add_session_data { request.session if request.respond_to? :session }
+            transaction.add_session_data { request.session.to_h if request.respond_to? :session }
             transaction.add_tags(:request_id => request_id) if request_id
             Appsignal::Transaction.complete_current!
           end
@@ -88,7 +88,7 @@ def install_unsubscribe_callback
             transaction.set_metadata("method", "websocket")
             transaction.add_params_if_nil { request.params }
             transaction.add_headers_if_nil { request.env }
-            transaction.add_session_data { request.session if request.respond_to? :session }
+            transaction.add_session_data { request.session.to_h if request.respond_to? :session }
             transaction.add_tags(:request_id => request_id) if request_id
             Appsignal::Transaction.complete_current!
           end

lib/appsignal/integrations/action_cable.rb

@@ -21,6 +21,7 @@ def perform_action(*args, &block)
         ensure
           transaction.set_action_if_nil("#{self.class}##{args.first["action"]}")
           transaction.add_params_if_nil(args.first)
+          transaction.add_session_data { request.session.to_h if request.respond_to? :session }
           transaction.set_metadata("path", request.path)
           transaction.set_metadata("method", "websocket")
           transaction.add_tags(:request_id => request_id) if request_id

spec/lib/appsignal/hooks/action_cable_spec.rb

@@ -37,7 +37,16 @@ def self.to_s
             :with_queue_start => true
           )
         end
-        let(:connection) { ActionCable::Connection::Base.new(server, env) }
+        let(:request) do
+          ActionDispatch::Request.new(env).tap do |req|
+            set_rails_session_data(
+              req,
+              "user_id" => "123",
+              "session" => "yes"
+            )
+          end
+        end
+        let(:connection) { ActionCable::Connection::Base.new(server, request.env) }
         let(:identifier) { { :channel => "MyChannel" }.to_json }
         let(:params) { {} }
         let(:request_id) { SecureRandom.uuid }
@@ -75,6 +84,10 @@ def self.to_s
               "action" => "speak",
               "message" => "foo"
             )
+            expect(transaction).to include_session_data(
+              "user_id" => "123",
+              "session" => "yes"
+            )
             expect(transaction).to include_tags("request_id" => request_id)
             expect(transaction).to_not have_queue_start
             expect(transaction).to be_completed
@@ -152,6 +165,10 @@ def self.to_s
               "name" => "subscribed.action_cable",
               "title" => ""
             )
+            expect(transaction).to include_session_data(
+              "user_id" => "123",
+              "session" => "yes"
+            )
             expect(transaction).to include_tags("request_id" => request_id)
             expect(transaction).to_not have_queue_start
             expect(transaction).to be_completed
@@ -194,6 +211,10 @@ def self.to_s
                 "path" => "/blog"
               )
               expect(transaction).to include_params("internal" => "true")
+              expect(transaction).to include_session_data(
+                "user_id" => "123",
+                "session" => "yes"
+              )
               expect(transaction).to_not have_queue_start
               expect(transaction).to be_completed
             end
@@ -246,6 +267,10 @@ def self.to_s
               "path" => "/blog"
             )
             expect(transaction).to include_params("internal" => "true")
+            expect(transaction).to include_session_data(
+              "user_id" => "123",
+              "session" => "yes"
+            )
             expect(transaction).to include_event(
               "body" => "",
               "body_format" => Appsignal::EventFormatter::DEFAULT,
@@ -294,6 +319,10 @@ def self.to_s
                 "path" => "/blog"
               )
               expect(transaction).to include_params("internal" => "true")
+              expect(transaction).to include_session_data(
+                "user_id" => "123",
+                "session" => "yes"
+              )
               expect(transaction).to_not have_queue_start
               expect(transaction).to be_completed
             end

spec/support/helpers/env_helpers.rb

@@ -38,4 +38,32 @@ def background_env_with_data(args = {})
       :queue_start => fixed_time
     }.merge(args)
   end
+
+  def set_rails_session_data(request, data)
+    ActionDispatch::Request::Session.create(
+      rails_session_store(data),
+      request,
+      {}
+    )
+  end
+
+  def rails_session_store(data)
+    Class.new do
+      def initialize(data)
+        @data = data
+      end
+
+      def load_session(_env)
+        [1, @data]
+      end
+
+      def session_exists?(_env)
+        true
+      end
+
+      def delete_session(_env, _id, _options)
+        123
+      end
+    end.new(data)
+  end
 end