Optimize setting parameters for transactions

With the recent refactoring of the Rack middleware, I changed how we set
the request parameters on the transaction. Previously, we read the
parameters from the request object in the transaction when we sampled
it. Not always.

Now that we use the currently active transaction when a transaction is
already active, I needed to update how we set parameters using the
`set_params`/`set_params_if_nil` helpers. These helper methods require
us to pass in the parsed parameters, potentially making every request
slower because it has to fetch and parse the request parameters.

Add a block argument to the `set_params`/`set_params_if_nil` helpers to
fetch and parse the parameters only when we sample the transaction. This
change should reduce the AppSignal overhead for non-sampled
transactions.

I have no specific preference if the argument or block is leading. I
chose the argument to be leading.

.changesets/add-block-argument-to-set_params.md

@@ -0,0 +1,13 @@
+---
+bump: patch
+type: add
+---
+
+Add a block argument to the `Appsignal.set_params` and `Appsignal::Transaction#set_params` helpers. When `set_params` is called with a block argument, the block is executed when the parameters are stored on the Transaction. This block is only called when the Transaction is sampled. Use this block argument to avoid having to parse parameters for every transaction, even when it's not sampled.
+
+```ruby
+Appsignal.set_params do
+  # Some slow code to parse parameters
+  JSON.parse('{"param1": "value1"}')
+end
+```

lib/appsignal/helpers/instrumentation.rb

@@ -532,20 +532,41 @@ def tag_request(tags = {})
       # When no parameters are set this way, the transaction will look for
       # parameters in its request environment.
       #
-      # @example
+      # A block can be given to this method to defer the fetching and parsing
+      # of the parameters until and only when the transaction is sampled.
+      #
+      # When both the `given_params` and a block is given to this method, the
+      # `given_params` argument is leading and the block will _not_ be called.
+      #
+      # @example Set parameters
       #   Appsignal.set_params("param1" => "value1")
       #
-      # @example
+      # @example Calling `set_params` multiple times will only keep the last call
       #   Appsignal.set_params("param1" => "value1")
       #   Appsignal.set_params("param2" => "value2")
-      #   # Parameters are: { "param2" => "value2" }
+      #   # The parameters are: { "param2" => "value2" }
+      #
+      # @example Calling `set_params` with a block
+      #   Appsignal.set_params do
+      #     # Some slow code to parse parameters
+      #     JSON.parse('{"param1": "value1"}')
+      #   end
+      #   # The parameters are: { "param1" => "value1" }
+      #
+      # @example Calling `set_params` with a parameter and a block
+      #   Appsignal.set_params("argument" => "argument value") do
+      #     # Some slow code to parse parameters
+      #     JSON.parse('{"param1": "value1"}')
+      #   end
+      #   # The parameters are: { "argument" => "argument value" }
       #
       # @since 3.10.0
       # @param given_params [Hash] The parameters to set on the transaction.
       # @see https://docs.appsignal.com/guides/custom-data/sample-data.html
       #   Sample data guide
       # @see https://docs.appsignal.com/guides/filter-data/filter-parameters.html
       #   Parameter filtering guide
+      # @see Transaction#set_params
       # @return [void]
       def set_params(params)
         return unless active?

lib/appsignal/rack/abstract_middleware.rb

@@ -129,7 +129,7 @@ def add_transaction_metadata_after(transaction, request)
         request_method = request_method_for(request)
         transaction.set_metadata("method", request_method) if request_method
 
-        transaction.set_params_if_nil(params_for(request))
+        transaction.set_params_if_nil { params_for(request) }
         transaction.set_http_or_background_queue_start
       end
 

lib/appsignal/transaction.rb

@@ -88,6 +88,7 @@ def initialize(transaction_id, namespace, request, options = {})
       @store = Hash.new({})
       @options = options
       @options[:params_method] ||= :params
+      @params = nil
 
       @ext = Appsignal::Extension.start_transaction(
         @transaction_id,
@@ -139,9 +140,13 @@ def store(key)
     end
 
     def params
-      return @params if defined?(@params)
+      parameters = @params || request_params
 
-      request_params
+      if parameters.respond_to? :call
+        parameters.call
+      else
+        parameters
+      end
     end
 
     # Set parameters on the transaction.
@@ -152,10 +157,17 @@ def params
     # The parameters set using {#set_params} are leading over those extracted
     # from a request's environment.
     #
+    # When both the `given_params` and a block is given to this method, the
+    # `given_params` argument is leading and the block will _not_ be called.
+    #
     # @since 3.9.1
     # @param given_params [Hash] The parameters to set on the transaction.
+    # @yield This block is called when the transaction is sampled. The block's
+    #   return value will become the new parameters.
     # @return [void]
-    def set_params(given_params)
+    # @see {Helpers::Instrumentation#set_params}
+    def set_params(given_params = nil, &block)
+      @params = block if block
       @params = given_params if given_params
     end
 
@@ -175,9 +187,12 @@ def params=(given_params)
     #
     # @since 3.9.1
     # @param given_params [Hash] The parameters to set on the transaction if none are already set.
+    # @yield This block is called when the transaction is sampled. The block's
+    #   return value will become the new parameters.
     # @return [void]
-    def set_params_if_nil(given_params)
-      set_params(given_params) unless @params
+    # @see {Helpers::Instrumentation#set_params_if_nil}
+    def set_params_if_nil(given_params = nil, &block)
+      set_params(given_params, &block) unless @params
     end
 
     # Set tags on the transaction.

spec/lib/appsignal/transaction_spec.rb

@@ -326,6 +326,16 @@ def create_transaction(id = transaction_id)
         it "returns custom parameters" do
           expect(subject).to eq(:foo => "bar")
         end
+
+        context "when params is a callable object" do
+          it "calls the params object and sets the return value as parametesr" do
+            transaction.set_params { { "param1" => "value1" } }
+
+            expect(transaction.params).to eq(
+              "param1" => "value1"
+            )
+          end
+        end
       end
 
       context "without custom params set on transaction" do
@@ -821,6 +831,17 @@ def to_s
           kind_of(Integer)
         )
       end
+
+      context "when params is a callable object" do
+        it "calls the params object and sets the return value as parametesr" do
+          transaction.set_params { { "param1" => "value1" } }
+
+          transaction.sample_data
+          expect(transaction).to include_params(
+            "param1" => "value1"
+          )
+        end
+      end
     end
 
     describe "#set_error" do