aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorJohn Hawthorn <john@hawthorn.email>2019-05-07 13:16:57 -0700
committerJohn Hawthorn <john@hawthorn.email>2019-05-08 13:30:41 -0700
commit6f549ce53f3398403d4c2a47f9ecba910be256dd (patch)
tree866b93d0aa56cf3fcca33ecf92b215f1ddc1393b /actionpack
parent52125dc0f8669d8dd497427c7b177d5d04106e0c (diff)
downloadrails-6f549ce53f3398403d4c2a47f9ecba910be256dd.tar.gz
rails-6f549ce53f3398403d4c2a47f9ecba910be256dd.tar.bz2
rails-6f549ce53f3398403d4c2a47f9ecba910be256dd.zip
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.
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/lib/action_dispatch/middleware/stack.rb15
-rw-r--r--actionpack/test/controller/show_exceptions_test.rb2
-rw-r--r--actionpack/test/dispatch/middleware_stack_test.rb6
3 files changed, 17 insertions, 6 deletions
diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb
index f0c869fba0..57e4adb457 100644
--- a/actionpack/lib/action_dispatch/middleware/stack.rb
+++ b/actionpack/lib/action_dispatch/middleware/stack.rb
@@ -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
diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb
index 8724f9bcdb..1d68a359dc 100644
--- a/actionpack/test/controller/show_exceptions_test.rb
+++ b/actionpack/test/controller/show_exceptions_test.rb
@@ -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
diff --git a/actionpack/test/dispatch/middleware_stack_test.rb b/actionpack/test/dispatch/middleware_stack_test.rb
index 90f2eccd19..c534e60c74 100644
--- a/actionpack/test/dispatch/middleware_stack_test.rb
+++ b/actionpack/test/dispatch/middleware_stack_test.rb
@@ -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