mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Only build middleware proxy when instrumentating
The instrumentation proxy adds three stack frames per-middleware, even when nothing is listening. This commit, when the middleware stack is built, only adds instrumentation when the `process_middleware.action_dispatch` event has already been subscribed to. The advantage to this is that we don't have any extra stack frames in apps which don't need middleware instrumentation. The disadvantage is that the subscriptions need to be in place when the middleware stack is built (during app boot). I think this is likely okay because temporary AS::Notifications subscriptions are strongly discouraged.
This commit is contained in:
parent
52125dc0f8
commit
6f549ce53f
3 changed files with 17 additions and 6 deletions
|
@ -34,7 +34,11 @@ module ActionDispatch
|
|||
end
|
||||
|
||||
def build(app)
|
||||
InstrumentationProxy.new(klass.new(app, *args, &block), inspect)
|
||||
klass.new(app, *args, &block)
|
||||
end
|
||||
|
||||
def build_instrumented(app)
|
||||
InstrumentationProxy.new(build(app), inspect)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -119,7 +123,14 @@ module ActionDispatch
|
|||
end
|
||||
|
||||
def build(app = nil, &block)
|
||||
middlewares.freeze.reverse.inject(app || block) { |a, e| e.build(a) }
|
||||
instrumenting = ActiveSupport::Notifications.notifier.listening?(InstrumentationProxy::EVENT_NAME)
|
||||
middlewares.freeze.reverse.inject(app || block) do |a, e|
|
||||
if instrumenting
|
||||
e.build_instrumented(a)
|
||||
else
|
||||
e.build(a)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
|
|
@ -99,7 +99,7 @@ module ShowExceptions
|
|||
class ShowFailsafeExceptionsTest < ActionDispatch::IntegrationTest
|
||||
def test_render_failsafe_exception
|
||||
@app = ShowExceptionsOverriddenController.action(:boom)
|
||||
middleware = @app.instance_variable_get(:@middleware)
|
||||
middleware = @app
|
||||
@exceptions_app = middleware.instance_variable_get(:@exceptions_app)
|
||||
middleware.instance_variable_set(:@exceptions_app, nil)
|
||||
$stderr = StringIO.new
|
||||
|
|
|
@ -121,9 +121,6 @@ class MiddlewareStackTest < ActiveSupport::TestCase
|
|||
end
|
||||
|
||||
test "instruments the execution of middlewares" do
|
||||
app = @stack.build(proc { |env| [200, {}, []] })
|
||||
env = {}
|
||||
|
||||
events = []
|
||||
|
||||
subscriber = proc do |*args|
|
||||
|
@ -131,6 +128,9 @@ class MiddlewareStackTest < ActiveSupport::TestCase
|
|||
end
|
||||
|
||||
ActiveSupport::Notifications.subscribed(subscriber, "process_middleware.action_dispatch") do
|
||||
app = @stack.build(proc { |env| [200, {}, []] })
|
||||
|
||||
env = {}
|
||||
app.call(env)
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in a new issue