Skip to content

Commit

Permalink
Merge pull request #43390 from jhawthorn/remove_notification_event_ch…
Browse files Browse the repository at this point in the history
…ildren

Remove child event tracking from ActiveSupport::Subscriber
  • Loading branch information
jhawthorn committed Feb 17, 2022
2 parents 139ef8a + 9c58a54 commit 3e2f9a6
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 85 deletions.
75 changes: 44 additions & 31 deletions actionview/lib/action_view/log_subscriber.rb
Expand Up @@ -50,28 +50,58 @@ def render_collection(event)
end
end

def start(name, id, payload)
log_rendering_start(payload, name)
module Utils # :nodoc:
def logger
ActionView::Base.logger
end

super
end
private
def from_rails_root(string)
string = string.sub(rails_root, "")
string.sub!(VIEWS_PATTERN, "")
string
end

def logger
ActionView::Base.logger
def rails_root # :doc:
@root ||= "#{Rails.root}/"
end
end

private
EMPTY = ""
def from_rails_root(string) # :doc:
string = string.sub(rails_root, EMPTY)
string.sub!(VIEWS_PATTERN, EMPTY)
string
include Utils

class Start # :nodoc:
include Utils

def start(name, id, payload)
return unless logger
logger.debug do
qualifier =
if name == "render_template.action_view"
""
elsif name == "render_layout.action_view"
"layout "
end

return unless qualifier

message = +" Rendering #{qualifier}#{from_rails_root(payload[:identifier])}"
message << " within #{from_rails_root(payload[:layout])}" if payload[:layout]
message
end
end

def finish(name, id, payload)
end
end

def rails_root # :doc:
@root ||= "#{Rails.root}/"
def self.attach_to(*)
ActiveSupport::Notifications.subscribe("render_template.action_view", ActionView::LogSubscriber::Start.new)
ActiveSupport::Notifications.subscribe("render_layout.action_view", ActionView::LogSubscriber::Start.new)

super
end

private
def render_count(payload) # :doc:
if payload[:cache_hits]
"[#{payload[:cache_hits]} / #{payload[:count]} cache hits]"
Expand All @@ -88,23 +118,6 @@ def cache_message(payload) # :doc:
"[cache miss]"
end
end

def log_rendering_start(payload, name)
debug do
qualifier =
if name == "render_template.action_view"
""
elsif name == "render_layout.action_view"
"layout "
end

return unless qualifier

message = +" Rendering #{qualifier}#{from_rails_root(payload[:identifier])}"
message << " within #{from_rails_root(payload[:layout])}" if payload[:layout]
message
end
end
end
end

Expand Down
2 changes: 2 additions & 0 deletions activesupport/CHANGELOG.md
@@ -1,3 +1,5 @@
* Deprecate `Notification::Event`'s `#children` and `#parent_of?`

* Change default serialization format of `MessageEncryptor` from `Marshal` to `JSON` for Rails 7.1.

Existing apps are provided with an upgrade path to migrate to `JSON` as described in `guides/source/upgrading_ruby_on_rails.md`
Expand Down
8 changes: 2 additions & 6 deletions activesupport/lib/active_support/log_subscriber.rb
Expand Up @@ -107,14 +107,10 @@ def logger
LogSubscriber.logger
end

def start(name, id, payload)
super if logger
end

def finish(name, id, payload)
def call(event)
super if logger
rescue => e
log_exception(name, e)
log_exception(event.name, e)
end

def publish_event(event)
Expand Down
28 changes: 18 additions & 10 deletions activesupport/lib/active_support/notifications/instrumenter.rb
Expand Up @@ -56,7 +56,7 @@ def unique_id
end

class Event
attr_reader :name, :time, :end, :transaction_id, :children
attr_reader :name, :time, :end, :transaction_id
attr_accessor :payload

def initialize(name, start, ending, transaction_id, payload)
Expand All @@ -65,7 +65,6 @@ def initialize(name, start, ending, transaction_id, payload)
@time = start ? start.to_f * 1_000.0 : start
@transaction_id = transaction_id
@end = ending ? ending.to_f * 1_000.0 : ending
@children = []
@cpu_time_start = 0.0
@cpu_time_finish = 0.0
@allocation_count_start = 0
Expand Down Expand Up @@ -117,6 +116,23 @@ def allocations
@allocation_count_finish - @allocation_count_start
end

def children # :nodoc:
ActiveSupport::Deprecation.warn <<~EOM
ActiveSupport::Notifications::Event#children is deprecated and will
be removed in Rails 7.2.
EOM
[]
end

def parent_of?(event) # :nodoc:
ActiveSupport::Deprecation.warn <<~EOM
ActiveSupport::Notifications::Event#parent_of? is deprecated and will
be removed in Rails 7.2.
EOM
start = (time - event.time) * 1000
start <= 0 && (start + duration >= event.duration)
end

# Returns the difference in milliseconds between when the execution of the
# event started and when it ended.
#
Expand All @@ -133,14 +149,6 @@ def duration
self.end - time
end

def <<(event)
@children << event
end

def parent_of?(event)
@children.include? event
end

private
def now
Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_millisecond)
Expand Down
24 changes: 2 additions & 22 deletions activesupport/lib/active_support/subscriber.rb
Expand Up @@ -126,38 +126,18 @@ def fetch_public_methods(subscriber, inherit_all)
attr_reader :patterns # :nodoc:

def initialize
@queue_key = [self.class.name, object_id].join "-"
@patterns = {}
super
end

def start(name, id, payload)
event = ActiveSupport::Notifications::Event.new(name, nil, nil, id, payload)
event.start!
parent = event_stack.last
parent << event if parent

event_stack.push event
end

def finish(name, id, payload)
event = event_stack.pop
event.finish!
event.payload.merge!(payload)

method = name.split(".").first
def call(event)
method = event.name.split(".").first
send(method, event)
end

def publish_event(event) # :nodoc:
method = event.name.split(".").first
send(method, event)
end

private
def event_stack
registry = ActiveSupport::IsolatedExecutionState[:active_support_subscriber_queue_registry] ||= {}
registry[@queue_key] ||= []
end
end
end
4 changes: 3 additions & 1 deletion activesupport/test/log_subscriber_test.rb
Expand Up @@ -77,7 +77,9 @@ def test_event_is_an_active_support_notifications_event

def test_event_attributes
ActiveSupport::LogSubscriber.attach_to :my_log_subscriber, @log_subscriber
instrument "some_event.my_log_subscriber"
instrument "some_event.my_log_subscriber" do
[] # Make an allocation
end
wait
event = @log_subscriber.event
if defined?(JRUBY_VERSION)
Expand Down
15 changes: 0 additions & 15 deletions activesupport/test/notifications_test.rb
Expand Up @@ -462,21 +462,6 @@ def test_events_consumes_information_given_as_payload
assert_equal Hash[payload: :bar], event.payload
end

def test_event_is_parent_based_on_children
time = Process.clock_gettime(Process::CLOCK_MONOTONIC)

parent = event(:foo, Process.clock_gettime(Process::CLOCK_MONOTONIC), Process.clock_gettime(Process::CLOCK_MONOTONIC) + 100, random_id, {})
child = event(:foo, time, time + 10, random_id, {})
not_child = event(:foo, time, time + 100, random_id, {})

parent.children << child

assert parent.parent_of?(child)
assert_not child.parent_of?(parent)
assert_not parent.parent_of?(not_child)
assert_not not_child.parent_of?(parent)
end

def test_subscribe_raises_error_on_non_supported_arguments
notifier = ActiveSupport::Notifications::Fanout.new

Expand Down

0 comments on commit 3e2f9a6

Please sign in to comment.