Skip to content

Commit 87b9ace

Browse files
committed
fix(rails): move CaptureContext in the middleware stack
This fixes the issue while ensuring we're not adding Sentry overhead to static file-serving routes, which is why the middleware was moved historically.
1 parent 921ad17 commit 87b9ace

2 files changed

Lines changed: 18 additions & 3 deletions

File tree

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ module Sentry
99
class Railtie < ::Rails::Railtie
1010
# middlewares can't be injected after initialize
1111
initializer "sentry.use_rack_middleware" do |app|
12-
# placed first so anything logged before CaptureExceptions shares the same trace context
13-
app.config.middleware.unshift Sentry::Rails::CaptureContext
12+
# placed right after the app-request boundary: early enough that anything logged before
13+
# CaptureExceptions shares the same trace context, late enough that file-serving requests,
14+
# which never reach CaptureExceptions, do not pay for a hub clone they never use
15+
app.config.middleware.insert_after ActionDispatch::Executor, Sentry::Rails::CaptureContext
1416
# placed after all the file-sending middlewares so we can avoid unnecessary transactions
1517
app.config.middleware.insert_after ActionDispatch::ShowExceptions, Sentry::Rails::CaptureExceptions
1618
# need to place as close to DebugExceptions as possible to intercept most of the exceptions, including those raised by middlewares

sentry-rails/spec/sentry/rails_spec.rb

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,26 @@
2222

2323
it "inserts middleware to a correct position" do
2424
app = Rails.application
25-
expect(app.middleware.first).to eq(Sentry::Rails::CaptureContext)
2625
index_of_executor = app.middleware.find_index { |m| m == ActionDispatch::ShowExceptions }
2726
expect(app.middleware.find_index(Sentry::Rails::CaptureExceptions)).to eq(index_of_executor + 1)
2827
index_of_debug_exceptions = app.middleware.find_index { |m| m == ActionDispatch::DebugExceptions }
2928
expect(app.middleware.find_index(Sentry::Rails::RescuedExceptionInterceptor)).to eq(index_of_debug_exceptions + 1)
3029
end
3130

31+
it "establishes the trace context before the request is logged" do
32+
middleware = Rails.application.middleware
33+
34+
expect(middleware.find_index(Sentry::Rails::CaptureContext))
35+
.to be < middleware.find_index(Rails::Rack::Logger)
36+
end
37+
38+
it "leaves requests served above the app boundary untouched" do
39+
middleware = Rails.application.middleware
40+
41+
expect(middleware.find_index(Sentry::Rails::CaptureContext))
42+
.to be > middleware.find_index(ActionDispatch::Executor)
43+
end
44+
3245
it "propagates timezone to cron config" do
3346
# cron.default_timezone is set to nil by default
3447
expect(Sentry.configuration.cron.default_timezone).to eq("Etc/UTC")

0 commit comments

Comments
 (0)