#4594 Add methods for including start events with notified actions - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4594 open

Add methods for including start events with notified actions

Reported by Justin George | May 13th, 2010 @ 11:37 PM | in 3.1

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.

Comments and changes to this ticket

You flagged this item as spam.
Justin George May 13th, 2010 @ 11:40 PM
Patch attached
- instrument_with_start.diff 3.8 KB
You flagged this item as spam.
Justin George May 13th, 2010 @ 11:40 PM
- Tag changed from “3.0, notifications” to “3.0, notifications, patch”
Rizwan Reza May 17th, 2010 @ 08:08 PM
- Milestone cleared.
- State changed from “new” to “verified”
+1 Verified. This patch applies cleanly and all tests pass.
Jeremy Kemper May 17th, 2010 @ 08:47 PM
Justin, we should always fire start events. Could you merge this with instrument rather than introduce a new API?
You flagged this item as spam.
Justin George May 17th, 2010 @ 09:40 PM
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.
Repository May 21st, 2010 @ 08:43 PM
- 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...
Jeremy Kemper May 25th, 2010 @ 10:19 PM
- State changed from “resolved” to “open”
Incorrectly resolved.
You flagged this item as spam.
Justin George May 26th, 2010 @ 01:19 AM
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
Rohit Arondekar June 17th, 2010 @ 07:16 AM
Justin, this patch doesn't apply any more to Rails master. Can you fix it?
Jeremy Kemper August 30th, 2010 @ 04:10 AM
- Milestone cleared.
- Importance changed from “” to “Low”
[bulk edit]
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
[bulk edit]
Santiago Pastorino November 15th, 2010 @ 09:15 PM
- Milestone cleared.
[bulk edit]
You flagged this item as spam.
Roger Leite January 4th, 2011 @ 07:08 PM
Hi, I made a fix to this patch.
Can you please check if it's ok ?
- add_start_events_3.diff 5.3 KB
Dan Pickett February 4th, 2011 @ 10:43 PM
Robert's patch currently applies cleanly to master (despite a trailing whitespace warning), with all tests passing
Santiago Pastorino February 7th, 2011 @ 04:08 PM
- Milestone set to “3.1”