Un-sample asset/static file requests in a less-hacky way

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

@@ -7,6 +7,10 @@ def initialize(app)
         if defined?(::Sprockets::Rails)
           @assets_regex = %r(\A/{0,2}#{::Rails.application.config.assets.prefix})
         end
+
+        if ::Rails.application.config.public_file_server.enabled
+          @public_file_server_enabled = true
+        end
       end
 
       private
@@ -31,15 +35,43 @@ 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 skip_sampling?(env)
+          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
+
+      def skip_sampling?(env)
+        for_sprockets_assets?(env) || for_static_file?(env)
+      end
+
+      def for_sprockets_assets?(env)
+        path = env["PATH_INFO"]
+        @assets_regex && path.match?(@assets_regex)
+      end
+
+      def for_static_file?(env)
+        if @public_file_server_enabled
+          static_middleware = ::Rails.application.config.middleware.detect { |m| m == ::ActionDispatch::Static }
+
+          return false unless static_middleware
+
+          static_middleware = static_middleware.build(@app)
+          file_handler = static_middleware.instance_variable_get(:@file_handler)
+          request = ::Rack::Request.new env
+
+          if file_handler.respond_to?(:find_file, true)
+            !!file_handler.send(:find_file, request.path_info, accept_encoding: request.accept_encoding)
+          else
+            path = request.path_info.chomp("/")
+            !!file_handler.match?(path)
+          end
+        end
       end
     end
   end

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

@@ -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