Fix Transaction#params= deprecation warning (#1110)

After PR #1109 our Ruby gem logs deprecation warnings about our internal `Transaction#params=` usage. Update it to `Transaction#set_params` to remove the deprecation messages.
appsignal · Jun 24, 2024 · b65d667 · b65d667
1 parent 6f23673
commit b65d667
Show file tree

Hide file tree

Showing 11 changed files with 21 additions and 17 deletions.
diff --git a/.changesets/remove-internal-deprecate-params--usage.md b/.changesets/remove-internal-deprecate-params--usage.md
@@ -0,0 +1,6 @@
+---
+bump: patch
+type: fix
+---
+
+Fix deprecation warnings about `Transacation.params=` usage by updating how we record parameters in our instrumentations.
diff --git a/lib/appsignal/helpers/instrumentation.rb b/lib/appsignal/helpers/instrumentation.rb
@@ -176,7 +176,7 @@ def listen_for_error(
       #
       # @example Add more metadata to transaction
       #   Appsignal.send_error(e) do |transaction|
-      #     transaction.params = { :search_query => params[:search_query] }
+      #     transaction.set_params(:search_query => params[:search_query])
       #     transaction.set_action("my_action_name")
       #     transaction.set_tags(:key => "value")
       #     transaction.set_namespace("my_namespace")
@@ -273,7 +273,7 @@ def send_error(
       #
       # @example Add more metadata to transaction
       #   Appsignal.set_error(e) do |transaction|
-      #     transaction.params = { :search_query => params[:search_query] }
+      #     transaction.set_params(:search_query => params[:search_query])
       #     transaction.set_action("my_action_name")
       #     transaction.set_tags(:key => "value")
       #     transaction.set_namespace("my_namespace")

diff --git a/lib/appsignal/hooks/active_job.rb b/lib/appsignal/hooks/active_job.rb
@@ -62,11 +62,12 @@ def execute(job)
             end
 
           if transaction
-            transaction.params =
+            transaction.set_params_if_nil(
               Appsignal::Utils::HashSanitizer.sanitize(
                 job["arguments"],
                 Appsignal.config[:filter_parameters]
               )
+            )
 
             transaction_tags = ActiveJobHelpers.transaction_tags_for(job)
             transaction_tags["active_job_id"] = job["job_id"]

diff --git a/lib/appsignal/integrations/action_cable.rb b/lib/appsignal/integrations/action_cable.rb
@@ -22,7 +22,7 @@ def perform_action(*args, &block)
           transaction.set_error(exception)
           raise exception
         ensure
-          transaction.params = args.first
+          transaction.set_params_if_nil(args.first)
           transaction.set_action_if_nil("#{self.class}##{args.first["action"]}")
           transaction.set_metadata("path", request.path)
           transaction.set_metadata("method", "websocket")

diff --git a/lib/appsignal/integrations/hanami.rb b/lib/appsignal/integrations/hanami.rb
@@ -55,7 +55,7 @@ def call(env)
       transaction.set_metadata("status", "500")
       raise error
     ensure
-      transaction.params = request.params.to_h
+      transaction.set_params_if_nil(request.params.to_h)
       transaction.set_action_if_nil(self.class.name)
       transaction.set_metadata("path", request.path)
       transaction.set_metadata("method", request.request_method)

diff --git a/lib/appsignal/integrations/railtie.rb b/lib/appsignal/integrations/railtie.rb
@@ -71,7 +71,7 @@ def report(error, handled:, severity:, context: {}, source: nil)
             transaction.set_action(action_name) if action_name
             transaction.set_metadata("path", path)
             transaction.set_metadata("method", method)
-            transaction.params = params
+            transaction.set_params_if_nil(params)
             transaction.set_sample_data("custom_data", custom_data) if custom_data
 
             tags[:severity] = severity

diff --git a/lib/appsignal/integrations/resque.rb b/lib/appsignal/integrations/resque.rb
@@ -25,7 +25,7 @@ def perform
               ResqueHelpers.arguments(payload),
               Appsignal.config[:filter_parameters]
             )
-          transaction.params = args if args
+          transaction.set_params_if_nil(args)
           transaction.set_tags("queue" => queue)
 
           Appsignal::Transaction.complete_current!

diff --git a/lib/appsignal/integrations/sidekiq.rb b/lib/appsignal/integrations/sidekiq.rb
@@ -45,7 +45,7 @@ def call(exception, sidekiq_context, _sidekiq_config = nil)
             )
           transaction.set_action_if_nil("SidekiqInternal")
           transaction.set_metadata("sidekiq_error", sidekiq_context[:context])
-          transaction.params = { :jobstr => sidekiq_context[:jobstr] }
+          transaction.set_params_if_nil(:jobstr => sidekiq_context[:jobstr])
           transaction.set_error(exception)
         end
 
@@ -83,9 +83,7 @@ def call(_worker, item, _queue, &block)
         raise exception
       ensure
         if transaction
-          params = filtered_arguments(item)
-          transaction.params = params if params
-
+          transaction.set_params_if_nil(filtered_arguments(item))
           transaction.set_http_or_background_queue_start
           Appsignal::Transaction.complete_current! unless exception
 

diff --git a/lib/appsignal/rack/rails_instrumentation.rb b/lib/appsignal/rack/rails_instrumentation.rb
@@ -28,8 +28,6 @@ def call_with_appsignal_monitoring(env)
         )
 
         begin
-          transaction.params = fetch_params(request)
-
           @app.call(env)
         rescue Exception => error # rubocop:disable Lint/RescueException
           transaction.set_error(error)
@@ -39,6 +37,7 @@ def call_with_appsignal_monitoring(env)
           if controller
             transaction.set_action_if_nil("#{controller.class}##{controller.action_name}")
           end
+          transaction.set_params_if_nil(fetch_params(request))
           request_id = fetch_request_id(env)
           transaction.set_tags(:request_id => request_id) if request_id
           transaction.set_metadata("path", request.path)

diff --git a/spec/lib/appsignal/rack/rails_instrumentation_spec.rb b/spec/lib/appsignal/rack/rails_instrumentation_spec.rb
@@ -115,7 +115,7 @@ def run
       context "with custom params" do
         let(:app) do
           lambda do |env|
-            env[Appsignal::Rack::APPSIGNAL_TRANSACTION].params = { "custom_param" => "yes" }
+            env[Appsignal::Rack::APPSIGNAL_TRANSACTION].set_params("custom_param" => "yes")
           end
         end
 

diff --git a/spec/lib/appsignal/transaction_spec.rb b/spec/lib/appsignal/transaction_spec.rb
@@ -328,7 +328,7 @@ def current_transaction
 
       context "with custom params set on transaction" do
         before do
-          transaction.params = { :foo => "bar" }
+          transaction.set_params(:foo => "bar")
         end
 
         it "returns custom parameters" do
@@ -352,7 +352,7 @@ def current_transaction
 
       it "sets params on the transaction" do
         params = { "foo" => "bar" }
-        transaction.params = params
+        silence { transaction.params = params }
 
         transaction.complete # Sample the data
         expect(transaction.params).to eq(params)
@@ -1241,7 +1241,7 @@ def to_s
 
       context "with custom params" do
         before do
-          transaction.params = { :foo => "bar", :baz => :bat }
+          transaction.set_params(:foo => "bar", :baz => :bat)
         end
 
         it "returns custom params" do