This project is archived and is in readonly mode.

Add methods for including start events with notified actions

#4594

The use case here is essentially for callback-esque use cases - we want to note when a sql transaction has begun, and when it has ended, as seperate events.

A use case for this is to notice when sql queries are hanging, or for noticing which order queries are starting in without reference to their final length.

ActiveSupport::Notifications.subscribe(/sql.active_record/) do |name, *args|
  @events << name
end

ActiveSupport::Notifications.instrument_with_start('sql.active_record', {:some => 'payload'}) do
  # query logic
end

@events.inspect #=> ['sql.active_record.start', 'sql.active_record']

Open to suggestions on what to call it, and how it should work.

Reported by Justin George · May 13th, 2010 @ 11:37 PM

Activity

Justin George
Patch attached
- instrument_with_start.diff 3.79 KB
May 13th, 2010 @ 11:40 PM
Justin George
- Tag changed from 3.0, notifications to 3.0, notifications, patch
May 13th, 2010 @ 11:40 PM
Rizwan Reza
- Milestone cleared.
- State changed from new to verified
+1 Verified. This patch applies cleanly and all tests pass.

May 17th, 2010 @ 08:08 PM
Jeremy Kemper
Justin, we should always fire start events. Could you merge this with instrument rather than introduce a new API?

May 17th, 2010 @ 08:47 PM
Justin George
There's a couple situations where you don't want it, !render_template.action_view being the main one where 1000s of calls may be mode.

I will rename the old instrument method to be instrument_without_start, or something like that, yes? Or make it take an optional parameter? I'm not sure which is the proper way.
```
def instrument(event, payload={})
  ...
end

def instrument_without_start(event, payload={})
  ...
end
```
versus
```
def instrument(event, payload={}, start=true)
  @notifier.publish(...) if start
  ...
end
```
Thanks for the feedback, I was less sure about this change.
May 17th, 2010 @ 09:40 PM
Repository
- State changed from verified to resolved
(from [f3abc8ac36055afed9fcc902c33ee146e066d17a]) Use multibyte proxy class on 1.9, refactor Unicode.

Makes String#mb_chars on Ruby 1.9 return an instance of ActiveSupport::Multibyte::Chars to work around 1.9's lack of Unicode case folding.

Refactors class methods from ActiveSupport::Multibyte::Chars into new Unicode module, adding other related functionality for consistency.

[#4594 state:resolved]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/f3abc8ac36055afed9fcc902c33ee1...

May 21st, 2010 @ 08:43 PM
Jeremy Kemper
- State changed from resolved to open
Incorrectly resolved.

May 25th, 2010 @ 10:19 PM
Justin George
Okay, here we are, after a brief hiatus, a proper patch for this.

I think that the case I was worrying about before (!render_template) is not that big a deal - if it's a problem we can fix it later.
- add_start_events_2.diff 5.5 KB
May 26th, 2010 @ 01:19 AM
Rohit Arondekar
Justin, this patch doesn't apply any more to Rails master. Can you fix it?

June 17th, 2010 @ 07:16 AM
Jeremy Kemper
- Milestone cleared.
- Importance changed from to Low
[bulk edit]

August 30th, 2010 @ 04:10 AM
Jeremy Kemper
- Milestone set to 3.0.2
[bulk edit]

October 15th, 2010 @ 11:01 PM
Santiago Pastorino
- Milestone cleared.
[bulk edit]

November 15th, 2010 @ 09:15 PM
Roger Leite
Hi, I made a fix to this patch.
Can you please check if it's ok ?
- add_start_events_3.diff 5.31 KB
January 4th, 2011 @ 07:08 PM
Dan Pickett
Robert's patch currently applies cleanly to master (despite a trailing whitespace warning), with all tests passing

February 4th, 2011 @ 10:43 PM
Santiago Pastorino
- Milestone set to 3.1
February 7th, 2011 @ 04:08 PM

There was a problem

Add methods for including start events with notified actions

Activity