From b65d6674c93afbc95e9cecee8c032e6949229aab Mon Sep 17 00:00:00 2001 From: Tom de Bruijn <tom@tomdebruijn.com> Date: Mon, 24 Jun 2024 15:06:04 +0200 Subject: [PATCH] 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. --- .changesets/remove-internal-deprecate-params--usage.md | 6 ++++++ lib/appsignal/helpers/instrumentation.rb | 4 ++-- lib/appsignal/hooks/active_job.rb | 3 ++- lib/appsignal/integrations/action_cable.rb | 2 +- lib/appsignal/integrations/hanami.rb | 2 +- lib/appsignal/integrations/railtie.rb | 2 +- lib/appsignal/integrations/resque.rb | 2 +- lib/appsignal/integrations/sidekiq.rb | 6 ++---- lib/appsignal/rack/rails_instrumentation.rb | 3 +-- spec/lib/appsignal/rack/rails_instrumentation_spec.rb | 2 +- spec/lib/appsignal/transaction_spec.rb | 6 +++--- 11 files changed, 21 insertions(+), 17 deletions(-) create mode 100644 .changesets/remove-internal-deprecate-params--usage.md diff --git a/.changesets/remove-internal-deprecate-params--usage.md b/.changesets/remove-internal-deprecate-params--usage.md new file mode 100644 index 000000000..76311de35 --- /dev/null +++ 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 index 103bd4655..0ffa7571b 100644 --- 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 index 8fae47eb9..e5d94fd10 100644 --- 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 index 8fff1695a..3a3c82db8 100644 --- 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 index 615300752..fc96935a9 100644 --- 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 index e1ea2255e..dbe89153d 100644 --- 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 index 5f50a064d..ca02927b0 100644 --- 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 index 5587dab77..f3242d2d5 100644 --- 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 index 53da7351b..5259e9a6a 100644 --- 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 index 4b1fbed0b..3088d7758 100644 --- 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 index b4006b77a..29731946b 100644 --- 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