Subscribe to additional Action View topic #1748
Conversation
…512_subscribe_actionview
Refactor ActionViewSubscriber methods `start` and `finish` (which now inherit from NotificationsSubscriber to `start_segment` and `finish_segment`.
hannahramadan
changed the title
render_layout.action_view
hannahramadan
changed the title
render_layout.action_view
hannahramadan
changed the title
hannahramadan
changed the base branch from
dev
to
action_controller_subscriber_additional_topic
| def test_records_metrics_for_simple_layout | ||
| params = {:identifier => '/root/app/views/model/_layout.html.erb'} | ||
| nr_freeze_process_time | ||
| in_transaction do | ||
| @subscriber.start('render_layout.action_view', :id, params) | ||
| @subscriber.start('!render_layout.action_view', :id, | ||
| :virtual_path => 'model/_layout') | ||
| advance_process_time(2.0) | ||
| @subscriber.finish('!render_layout.action_view', :id, | ||
| :virtual_path => 'model/_layout') | ||
| @subscriber.finish('render_layout.action_view', :id, params) | ||
| end | ||
| expected = {:call_count => 1, :total_call_time => 2.0} | ||
|
|
||
| assert_metrics_recorded('View/model/_layout.html.erb/Layout' => expected) | ||
| end |
There was a problem hiding this comment.
Could you walk me through this test? What I think I'm seeing is:
- pause the clock
- start two similar events, but one with a false name (because it leads with a
!), for the same params - advance the clock
- finish both events
- make sure only one of the events is recorded
Is that right, or is there something else going on too? Are only metrics supposed to be recorded for render_layout, not segments?
(It looks like a great test, I just want to make sure I'm reading it correctly!)
There was a problem hiding this comment.
We chatted about this and the description above matches my understanding, except the events that start with ! are the ones we want recorded! Descriptive comment available in NewRelic::Agent::ActionViewSubscriber#recordable?
And add tests!
SimpleCov Report
|
|
|
||
| def metric_action(name) | ||
| case name | ||
| when /#{RENDER_TEMPLATE_EVENT_NAME}$/o then 'Rendering' | ||
| when RENDER_PARTIAL_EVENT_NAME then 'Partial' | ||
| when RENDER_COLLECTION_EVENT_NAME then 'Partial' | ||
| when RENDER_LAYOUT_EVENT_NAME then 'Layout' | ||
| else 'Unknown' |
There was a problem hiding this comment.
There are places where "Unknown" (capital U) are used, but for the subscriber stuff we've been consistently using "unknown" (lower case u). Perhaps this should be changed?
There was a problem hiding this comment.
Great catch. I wonder if we should define the UNKNOWN constant in NotificationsSubscriber so we have the same value everywhere?
render_layout.action_viewrender_layout.action_viewstartandfinishto accommodate for newstartandfinishmethods inherited from NotificationsSubscribercloses #1512