Skip to content

Commit

Permalink
Rails fixes
Browse files Browse the repository at this point in the history
These Rails fixes were driven by extensive testing of the newly released Rails
version 7.1, but some may be relevant to older Rails versions as well.

*lib/*
- When adding new segment attributes, anticipate and ignore numeric parameter
  keys
- For `Transaction#finish`, don't attempt to invoke methods on a non-existent
  initial segment

*test/*
- Inline controller method rendering with a proc was a thing in early Rails
  versions that was no longer supported in Rails 3. We were defining our own
  `proc_render` controller method to simulate the old style behavior and it
  finally caused issues for us in Rails 7. Given that we don't even support
  Rails versions older than 4 now, let's just stop testing the Rails 2 and
  below proc functionality.
- `ActiveJobTest`: a single test needed an additional `expect`
- Remove the temporary `before_suite.rb` based hacks for the Rails multiverse
  suite.
- Modify the `ActionController::Live` RUM test to set a `last-modified` header
  on the streaming response. This prevents Rack v3 from attempting to call
  `to_ary` on the streaming response object returned. In the future we could
  author a better streaming test that properly chunks data and possibly even
  uses a layout. But for now, this header fix gives us Rack 3 compatibility with
  the old test.

resolves #2254
  • Loading branch information
fallwith committed Oct 18, 2023
1 parent 10e75d1 commit 281dc52
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 41 deletions.
4 changes: 3 additions & 1 deletion lib/new_relic/agent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,9 @@ def add_custom_attributes(params) # THREAD_LOCAL_ACCESS
def add_new_segment_attributes(params, segment)
# Make sure not to override existing segment-level custom attributes
segment_custom_keys = segment.attributes.custom_attributes.keys.map(&:to_sym)
segment.add_custom_attributes(params.reject { |k, _v| segment_custom_keys.include?(k.to_sym) })
segment.add_custom_attributes(params.reject do |k, _v|
segment_custom_keys.include?(k.to_sym) if k.respond_to?(:to_sym) # param keys can be integers
end)
end

# Add custom attributes to the span event for the current span. Attributes will be visible on spans in the
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ def needs_middleware_summary_metrics?(name)
end

def finish
return unless state.is_execution_traced?
return unless state.is_execution_traced? && initial_segment

@end_time = Process.clock_gettime(Process::CLOCK_REALTIME)
@duration = @end_time - @start_time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def brains
end

def brain_stream
headers['last-modified'] = Time.now # tell Rack not to call #to_ary on the response object
render(:inline => RESPONSE_BODY, :stream => true)
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/multiverse/suites/rails/activejob_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def test_code_information_recorded_with_new_transaction
(NewRelic::Agent::Instrumentation::ActiveJobSubscriber::PAYLOAD_KEYS.size + 1).times do
segment.expect(:params, {}, [])
end
3.times do
4.times do
segment.expect(:finish, [])
end
segment.expect(:record_scoped_metric=, nil, [false])
Expand Down
23 changes: 0 additions & 23 deletions test/multiverse/suites/rails/before_suite.rb

This file was deleted.

17 changes: 2 additions & 15 deletions test/multiverse/suites/rails/view_instrumentation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,27 +65,14 @@ def collection_render
render((1..3).map { |x| Foo.new })
end

# proc rendering isn't available in rails 3 but you can do nonsense like this
# and assign an enumerable object to the response body.
def proc_render
streamer = Class.new do
def each
10_000.times do |i|
yield("This is line #{i}\n")
end
end
end
self.response_body = streamer.new
end

def raise_render
raise 'this is an uncaught RuntimeError'
end
end

class ViewInstrumentationTest < ActionDispatch::IntegrationTest
include MultiverseHelpers
RENDERING_OPTIONS = [:js_render, :xml_render, :proc_render, :json_render]
RENDERING_OPTIONS = [:js_render, :xml_render, :json_render]

setup_and_teardown_agent do
# ActiveSupport testing keeps blowing away my subscribers on
Expand All @@ -98,7 +85,7 @@ class ViewInstrumentationTest < ActionDispatch::IntegrationTest
end
end

(ViewsController.action_methods - %w[raise_render collection_render haml_render proc_render]).each do |method|
(ViewsController.action_methods - %w[raise_render collection_render haml_render]).each do |method|
define_method("test_sanity_#{method}") do
get "/views/#{method}"

Expand Down

0 comments on commit 281dc52

Please sign in to comment.