Re-position CaptureExceptions middleware to reduce tracing noise

sentry-rails/CHANGELOG.md

@@ -22,6 +22,7 @@ config.rails.tracing_subscribers = [MySubscriber]
 
 - Report exceptions from the interceptor middleware for exceptions app [#1379](https://github.com/getsentry/sentry-ruby/pull/1379)
   - Fixes [#1371](https://github.com/getsentry/sentry-ruby/issues/1371)
+- Re-position CaptureExceptions middleware to reduce tracing noise [#1405](https://github.com/getsentry/sentry-ruby/pull/1405) 
 
 ## 4.3.4
 

sentry-rails/lib/sentry/rails/capture_exceptions.rb

@@ -31,15 +31,15 @@ def capture_exception(exception)
       end
 
       def start_transaction(env, scope)
-        transaction = super
+        sentry_trace = env["HTTP_SENTRY_TRACE"]
+        options = { name: scope.transaction_name, op: transaction_op }
 
-        return unless transaction
-
-        if @assets_regex && transaction.name.match?(@assets_regex)
-          transaction.instance_variable_set(:@sampled, false)
+        if @assets_regex && scope.transaction_name.match?(@assets_regex)
+          options.merge!(sampled: false)
         end
 
-        transaction
+        transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace
+        Sentry.start_transaction(transaction: transaction, **options)
       end
     end
   end

sentry-rails/lib/sentry/rails/railtie.rb

@@ -6,8 +6,8 @@ module Sentry
   class Railtie < ::Rails::Railtie
     # middlewares can't be injected after initialize
     initializer "sentry.use_rack_middleware" do |app|
-      # need to be placed at first to capture as many errors as possible
-      app.config.middleware.insert 0, Sentry::Rails::CaptureExceptions
+      # placed after all the file-sending middlewares so we can avoid unnecessary transactions
+      app.config.middleware.insert_after ActionDispatch::Executor, Sentry::Rails::CaptureExceptions
       # need to be placed at last to smuggle app exceptions via env
       app.config.middleware.use(Sentry::Rails::RescuedExceptionInterceptor)
     end
@@ -21,7 +21,6 @@ class Railtie < ::Rails::Railtie
       extend_active_job if defined?(ActiveJob)
       patch_background_worker if defined?(ActiveRecord)
       override_streaming_reporter if defined?(ActionView)
-      override_file_handler if defined?(ActionDispatch) && app.config.public_file_server.enabled
       setup_backtrace_cleanup_callback
       inject_breadcrumbs_logger
       activate_tracing
@@ -79,14 +78,6 @@ def override_streaming_reporter
       end
     end
 
-    def override_file_handler
-      require "sentry/rails/overrides/file_handler"
-
-      ActiveSupport.on_load :action_controller do
-        ActionDispatch::FileHandler.send(:prepend, Sentry::Rails::Overrides::FileHandler)
-      end
-    end
-
     def activate_tracing
       if Sentry.configuration.tracing_enabled?
         subscribers = Sentry.configuration.rails.tracing_subscribers

sentry-rails/spec/sentry/rails/tracing_spec.rb

@@ -87,12 +87,18 @@
   end
 
   context "with sprockets-rails" do
+    let(:string_io) { StringIO.new }
+    let(:logger) do
+      ::Logger.new(string_io)
+    end
+
     before do
       require "sprockets/railtie"
 
       make_basic_app do |config, app|
         app.config.public_file_server.enabled = true
         config.traces_sample_rate = 1.0
+        config.logger = logger
       end
     end
 
@@ -101,14 +107,21 @@
 
       expect(response).to have_http_status(:not_found)
       expect(transport.events).to be_empty
+      expect(string_io.string).not_to match(/\[Tracing\] Starting <rails\.request>/)
     end
   end
 
   context "with config.public_file_server.enabled = true" do
+    let(:string_io) { StringIO.new }
+    let(:logger) do
+      ::Logger.new(string_io)
+    end
+
     before do
       make_basic_app do |config, app|
         app.config.public_file_server.enabled = true
         config.traces_sample_rate = 1.0
+        config.logger = logger
       end
     end
 
@@ -117,6 +130,7 @@
 
       expect(response).to have_http_status(:ok)
       expect(transport.events).to be_empty
+      expect(string_io.string).not_to match(/\[Tracing\] Starting <rails\.request>/)
     end
 
     it "doesn't get messed up by previous exception" do

sentry-rails/spec/sentry/rails_spec.rb

@@ -24,7 +24,8 @@
 
     it "inserts middleware to a correct position" do
       app = Rails.application
-      expect(app.middleware.find_index(Sentry::Rails::CaptureExceptions)).to eq(0)
+      index_of_executor = app.middleware.find_index { |m| m == ActionDispatch::Executor }
+      expect(app.middleware.find_index(Sentry::Rails::CaptureExceptions)).to eq(index_of_executor + 1)
       expect(app.middleware.find_index(Sentry::Rails::RescuedExceptionInterceptor)).to eq(app.middleware.count - 1)
     end