Skip to content

Commit 20a0ba4

Browse files
committed
Move ScoutApm instrumentation into an initializer
edd6330 added ScoutApm instrumentation to methods on the following: - ReactOnRails::Helper - ReactOnRails::ServerRenderingPool::RubyEmbeddedJavaScript Previously, this instrumentation was installed at gem load time using `defined?(ScoutApm)` guards. This caused two problems: 1. Gem ordering dependency: if scout_apm appeared after react_on_rails in the Gemfile, ScoutApm was not yet defined when react_on_rails loaded, so instrumentation was silently skipped. 2. Noisy log output: if scout_apm appeared before react_on_rails, instrumentation was installed before ScoutApm was initialized (via its "scout_apm.start" Railtie initializer), causing INFO-level messages to be written to stdout during boot. This commit moves the instrumentation into ReactOnRails::Engine with an initializer explicitly configured to run after ScoutApm initializes ("scout_apm.start"). This ensures instrumentation is properly installed regardless of gem ordering and only after ScoutApm is fully configured.
1 parent e5d7a97 commit 20a0ba4

File tree

5 files changed

+105
-13
lines changed

5 files changed

+105
-13
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ Changes since the last non-beta release.
2929

3030
#### Fixed
3131

32+
- **Fixed ScoutApm instrumentation depending on Gemfile ordering**. ScoutApm instrumentation for `react_component`, `react_component_hash`, and `exec_server_render_js` was previously installed at gem load time using `defined?(ScoutApm)` guards, which meant it was silently skipped if `scout_apm` appeared after `react_on_rails` in the Gemfile, and produced noisy INFO log messages if it appeared before (since ScoutApm wasn't yet initialized). Moved instrumentation into an initializer that runs after `scout_apm.start`, ensuring it works regardless of gem ordering and only after ScoutApm is fully configured. [PR 2442](https://github.com/shakacode/react_on_rails/pull/2442) by [tonyta](https://github.com/tonyta).
3233
- **Fixed string values interpolated into generated JS code without proper escaping**. All string values (component names, DOM IDs, Redux store names) embedded in server-rendering JavaScript now use `.to_json` instead of unescaped single-quoted interpolation, preventing potential JS breakage from special characters. [PR 2440](https://github.com/shakacode/react_on_rails/pull/2440) by [AbanoubGhadban](https://github.com/AbanoubGhadban).
3334

3435
#### Pro

react_on_rails/lib/react_on_rails/engine.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,23 @@ def self.package_json_missing?
8383
!File.exist?(VersionChecker::NodePackageVersion.package_json_path)
8484
end
8585

86+
# Install ScoutApm instrumentation after ScoutApm is configured via "scout_apm.start" initializer.
87+
# https://github.com/scoutapp/scout_apm_ruby/blob/v6.1.0/lib/scout_apm.rb#L221
88+
initializer "react_on_rails.scout_apm_instrumentation", after: "scout_apm.start" do
89+
next unless defined?(ScoutApm)
90+
91+
ReactOnRails::Helper.class_eval do
92+
include ScoutApm::Tracer
93+
instrument_method :react_component, type: "ReactOnRails", name: "react_component"
94+
instrument_method :react_component_hash, type: "ReactOnRails", name: "react_component_hash"
95+
end
96+
97+
ReactOnRails::ServerRenderingPool::RubyEmbeddedJavaScript.singleton_class.class_eval do
98+
include ScoutApm::Tracer
99+
instrument_method :exec_server_render_js, type: "ReactOnRails", name: "ExecJs React Server Rendering"
100+
end
101+
end
102+
86103
config.to_prepare do
87104
ReactOnRails::ServerRenderingPool.reset_pool
88105
end

react_on_rails/lib/react_on_rails/helper.rb

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -714,13 +714,6 @@ def in_mailer?
714714
controller.is_a?(ActionMailer::Base)
715715
end
716716

717-
if defined?(ScoutApm)
718-
include ScoutApm::Tracer
719-
720-
instrument_method :react_component, type: "ReactOnRails", name: "react_component"
721-
instrument_method :react_component_hash, type: "ReactOnRails", name: "react_component_hash"
722-
end
723-
724717
def raise_missing_autoloaded_bundle(react_component_name)
725718
raise ReactOnRails::SmartError.new(
726719
error_type: :missing_auto_loaded_bundle,

react_on_rails/lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rb

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,12 +211,6 @@ def console_polyfill
211211
JS
212212
end
213213

214-
if defined?(ScoutApm)
215-
include ScoutApm::Tracer
216-
217-
instrument_method :exec_server_render_js, type: "ReactOnRails", name: "ExecJs React Server Rendering"
218-
end
219-
220214
private
221215

222216
def file_url_to_string(url)

react_on_rails/spec/react_on_rails/engine_spec.rb

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,93 @@ module ReactOnRails
166166
end
167167
end
168168

169+
describe "ScoutApm instrumentation initializer" do
170+
subject(:initializer) { described_class.initializers.find { |i| i.name.include?("scout_apm") } }
171+
172+
it "defines a named Rails initializer to run after scout_apm.start" do
173+
expect(initializer.name).to eq "react_on_rails.scout_apm_instrumentation"
174+
expect(initializer.after).to eq "scout_apm.start"
175+
end
176+
177+
describe "react_on_rails.scout_apm_instrumentation" do
178+
let(:mock_scout_tracer) do
179+
#
180+
# Simplified mock of ScoutApm::Tracer that mirrors its real implementation.
181+
# https://github.com/scoutapp/scout_apm_ruby/blob/v6.1.0/lib/scout_apm/tracer.rb#L47-L70
182+
#
183+
Module.new do
184+
def self.included(base)
185+
base.define_singleton_method(:instrument_method) do |method_name, **|
186+
raise "method does not exist: #{method_name}" unless method_defined?(method_name)
187+
188+
instrumented_name = :"#{method_name}_with_test_instrument"
189+
uninstrumented_name = :"#{method_name}_without_test_instrument"
190+
191+
define_method(instrumented_name) do |*args, **kwargs, &block|
192+
send(uninstrumented_name, *args, **kwargs, &block)
193+
end
194+
195+
alias_method uninstrumented_name, method_name
196+
alias_method method_name, instrumented_name
197+
end
198+
end
199+
end
200+
end
201+
202+
let(:mock_helper) { Module.new.include(ReactOnRails::Helper) }
203+
let(:mock_rb_embedded_js) { Class.new(ReactOnRails::ServerRenderingPool::RubyEmbeddedJavaScript) }
204+
205+
before do
206+
stub_const("ReactOnRails::Helper", mock_helper)
207+
stub_const("ReactOnRails::ServerRenderingPool::RubyEmbeddedJavaScript", mock_rb_embedded_js)
208+
end
209+
210+
context "when ScoutApm is not defined" do
211+
before { hide_const("ScoutApm") }
212+
213+
it "does not instrument Helper#react_component" do
214+
initializer.run
215+
expect(mock_helper.instance_methods(false)).not_to include(:react_component_with_test_instrument)
216+
expect(mock_helper.instance_methods(false)).not_to include(:react_component_without_test_instrument)
217+
end
218+
219+
it "does not instrument Helper#react_component_hash" do
220+
initializer.run
221+
expect(mock_helper.instance_methods(false)).not_to include(:react_component_hash_with_test_instrument)
222+
expect(mock_helper.instance_methods(false)).not_to include(:react_component_hash_without_test_instrument)
223+
end
224+
225+
it "does not instrument RubyEmbeddedJavaScript.exec_server_render_js" do
226+
initializer.run
227+
expect(mock_rb_embedded_js.methods(false)).not_to include(:exec_server_render_js_with_test_instrument)
228+
expect(mock_rb_embedded_js.methods(false)).not_to include(:exec_server_render_js_without_test_instrument)
229+
end
230+
end
231+
232+
context "when ScoutApm is defined" do
233+
before { stub_const("ScoutApm::Tracer", mock_scout_tracer) }
234+
235+
it "instruments Helper#react_component" do
236+
initializer.run
237+
expect(mock_helper.instance_methods(false)).to include(:react_component_with_test_instrument)
238+
expect(mock_helper.instance_methods(false)).to include(:react_component_without_test_instrument)
239+
end
240+
241+
it "instruments Helper#react_component_hash" do
242+
initializer.run
243+
expect(mock_helper.instance_methods(false)).to include(:react_component_hash_with_test_instrument)
244+
expect(mock_helper.instance_methods(false)).to include(:react_component_hash_without_test_instrument)
245+
end
246+
247+
it "instruments RubyEmbeddedJavaScript.exec_server_render_js" do
248+
initializer.run
249+
expect(mock_rb_embedded_js.methods(false)).to include(:exec_server_render_js_with_test_instrument)
250+
expect(mock_rb_embedded_js.methods(false)).to include(:exec_server_render_js_without_test_instrument)
251+
end
252+
end
253+
end
254+
end
255+
169256
describe "automatic rake task loading" do
170257
# Rails::Engine automatically loads all .rake files from lib/tasks/
171258
# This test verifies that our rake tasks are loaded without needing

0 commit comments

Comments
 (0)