diff --git a/sentry-rails/lib/sentry/rails/capture_exceptions.rb b/sentry-rails/lib/sentry/rails/capture_exceptions.rb index f1f2ae297..e9986a849 100644 --- a/sentry-rails/lib/sentry/rails/capture_exceptions.rb +++ b/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 diff --git a/sentry-rails/lib/sentry/rails/overrides/file_handler.rb b/sentry-rails/lib/sentry/rails/overrides/file_handler.rb deleted file mode 100644 index 5abed4474..000000000 --- a/sentry-rails/lib/sentry/rails/overrides/file_handler.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Sentry - module Rails - module Overrides - module FileHandler - def serve(*args) - if Sentry.initialized? && current_transaction = Sentry.get_current_scope.span - # we don't want to expose a setter for @sampled just for this case - current_transaction.instance_variable_set(:@sampled, false) - end - - super - end - end - end - end -end diff --git a/sentry-rails/lib/sentry/rails/railtie.rb b/sentry-rails/lib/sentry/rails/railtie.rb index 3567f639d..0265b43c7 100644 --- a/sentry-rails/lib/sentry/rails/railtie.rb +++ b/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 diff --git a/sentry-rails/spec/sentry/rails/tracing_spec.rb b/sentry-rails/spec/sentry/rails/tracing_spec.rb index 48de91047..2e76e5f9f 100644 --- a/sentry-rails/spec/sentry/rails/tracing_spec.rb +++ b/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 /) 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 /) end it "doesn't get messed up by previous exception" do