This project is archived and is in readonly mode.

#4504 ✓resolved
Justin George

Order notifications from most to least significant (ala jQuery event namespaces)

Reported by Justin George | April 29th, 2010 @ 10:57 PM | in 3.0.2

Currently, notification events are named with the framework, then the action:

'active_record.sql'

For ease of subscribing to multiple frameworks, I propose that the order be switched to:

'sql.active_record'

Comments and changes to this ticket

  • Justin George

    Justin George April 29th, 2010 @ 11:00 PM

    • Tag changed from notifications to feature, notifications

    Here's a patch that implements my suggestions

  • Justin George

    Justin George April 29th, 2010 @ 11:00 PM

    • Tag changed from feature, notifications to feature, notifications, patch
  • Jeremy Kemper

    Jeremy Kemper April 29th, 2010 @ 11:47 PM

    • Milestone cleared.
    • State changed from “new” to “open”
    • Assigned user set to “José Valim”
  • José Valim

    José Valim April 30th, 2010 @ 09:15 AM

    • 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

    Justin George April 30th, 2010 @ 07:42 PM

    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

    José Valim April 30th, 2010 @ 08:46 PM

    • 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

    Justin George April 30th, 2010 @ 09:47 PM

    Sounds great, fixed patch attached.

    Thanks for your feedback, I appreciate it.

    Justin

  • Justin George

    Justin George April 30th, 2010 @ 09:50 PM

    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

  • Repository

    Repository May 2nd, 2010 @ 09:51 PM

    • State changed from “incomplete” to “resolved”

    (from [731d4392e478ff5526b595074d9caa999da8bd0c]) Change event namespace ordering to most-significant first [#4504 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 modification

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/731d4392e478ff5526b595074d9caa...

  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2
    • Importance changed from “” to “Low”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages