Improve nested Sinatra app instrumentation (#1097)

Sinatra apps, mounted in Rails app, would run into the issue that the
Rails middleware has already created a transaction for the request. It
could "force" a new transaction to be made, which loses information from
everything that happened before it.

Previously, before PR #1089, it would also leave a transaction that was
not closed properly. Even with that change, for one request, now two
transactions are created, one for Rails and one for the nested Sinatra
app.

This change reads if there's a current transaction from the request env,
and uses that instead of creating a new one. Some logic in the
Transaction class would read from the request object given to it on
`Transaction.create` to set metadata like parameters, so these need to
be set manually now.

It will also make sure not to close the transaction if one existed
already before this middleware was called.

Part of #329, the Rack middleware refactor.

.changesets/improve-mounted-sinatra-apps-in-rails-apps-instrumentation.md

@@ -0,0 +1,6 @@
+---
+bump: patch
+type: change
+---
+
+Improve instrumentation for mounted Sinatra apps in Rails apps. The sample reported for the Sinatra request will now include the time spent in Rails and its middleware.

lib/appsignal/rack/sinatra_instrumentation.rb

@@ -46,17 +46,26 @@ def call(env)
         end
       end
 
+      private
+
       def call_with_appsignal_monitoring(env)
-        options = { :force => @options.include?(:force) && @options[:force] }
-        options.merge!(:params_method => @options[:params_method]) if @options[:params_method]
         request = @options.fetch(:request_class, Sinatra::Request).new(env)
-        transaction = Appsignal::Transaction.create(
-          SecureRandom.uuid,
-          Appsignal::Transaction::HTTP_REQUEST,
-          request,
-          options
-        )
+        has_parent_instrumentation = env.key?(Appsignal::Rack::APPSIGNAL_TRANSACTION)
+        transaction =
+          if has_parent_instrumentation
+            env[Appsignal::Rack::APPSIGNAL_TRANSACTION]
+          else
+            Appsignal::Transaction.create(
+              SecureRandom.uuid,
+              Appsignal::Transaction::HTTP_REQUEST,
+              request
+            )
+          end
+
         begin
+          params = fetch_params(request, @options.fetch(:params_method, :params))
+          transaction.params = params if params
+
           Appsignal.instrument("process_action.sinatra") do
             @app.call(env)
           end
@@ -73,7 +82,9 @@ def call_with_appsignal_monitoring(env)
           transaction.set_metadata("path", request.path)
           transaction.set_metadata("method", request.request_method)
           transaction.set_http_or_background_queue_start
-          Appsignal::Transaction.complete_current!
+
+          # Only close if this middleware created the instrumentation
+          Appsignal::Transaction.complete_current! unless has_parent_instrumentation
         end
       end
 
@@ -88,7 +99,15 @@ def action_name(env)
         end
       end
 
-      private
+      def fetch_params(request, params_method)
+        return unless request.respond_to?(params_method)
+
+        request.send(params_method)
+      rescue => error
+        # Getting params from the request has been know to fail.
+        Appsignal.internal_logger.debug "Exception while getting Sinatra params: #{error}"
+        nil
+      end
 
       def raise_errors?(app)
         app.respond_to?(:settings) &&