This project is archived and is in readonly mode.
Order notifications from most to least significant (ala jQuery event namespaces)
-
Justin George- Tag changed from notifications to feature, notifications
Here's a patch that implements my suggestions
-
namespace_switch.diff
16.6 KB
-
Justin George
- Tag changed from feature, notifications to feature, notifications, patch
-
Jeremy Kemper- Milestone cleared.
- State changed from new to open
- Assigned user set to José Valim
-
José Valim- State changed from open to wontfix
Sometimes you want to match all SQL queries, but sometimes you want to match all events that happens inside a framework.
This is exactly the reason why you can give a regular expression to subscriber:
ActiveSupport::Notifications.subscribe /^(\w+).sql$/
-
Justin George
José Valim:
Maybe you can help me figure out a better namespacing system then:
I need to add start events to most of the current notifications so I can build a stack of events for transaction tracing.
This is important because most events do not occur within Rails' notification framework - an example being Net::HTTP calls or raw Memcached reads.
I (New Relic) do this currently by method chaining - I think this is less than ideal, and would prefer to use notifications.
However, it's important that these start events don't show up in the standard notifications, or you end up getting double-logging and the like.
My thought was that a reversed namespace would allow me to exclude things listening to just sql.
-
José Valim
- State changed from wontfix to incomplete
Hey Justin,
Jeremy and Yehuda convinced me about this patch so let's apply it!
I would like to ask you to just change one thing: action_view.render_template! should be become !render_template.action_view.
Currently, all notifications that ends with ! means that you should not be listening to them. Putting them in the middle, as you did in your patch, makes them hard to catch with a regexp. So it seems that adding them at the beginning would be inline with your patch. What do you think?
-
Justin George
Sounds great, fixed patch attached.
Thanks for your feedback, I appreciate it.
Justin
-
namespace_switch_2.diff
16.6 KB
-
-
Justin George
Whoops, missed the test cases on this - yet another patch attached.
Thinking about this more, in the future we may want to decompose these method names (i.e. have render.template.action_view) but I will file separate tickets when I figure that stuff out.
Justin
-
namespace_switch_3.diff
16.6 KB
-
-
Repository
- State changed from incomplete to resolved
(from [731d4392e478ff5526b595074d9caa999da8bd0c]) Change event namespace ordering to most-significant first [#4504 Order notifications from most to least significant (ala jQuery event namespaces) state:resolved]
More work still needs to be done on some of these names
(render_template.action_view and render_template!.action_view particularly) but this allows (for example) /^sql/ to subscribe to all
the various ORMs without further modificationSigned-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/731d4392e478ff5526b595074d9caa... -