Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use same logging rules as ActiveRecord #76

Merged
merged 7 commits into from
Apr 27, 2015
Merged

Use same logging rules as ActiveRecord #76

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5267eb5
Use same logging rules as ActiveRecord
ankane Mar 31, 2015
ed3a789
Use super
ankane Apr 1, 2015
456f77f
Better flow
ankane Apr 1, 2015
94c8655
Add failing test to demonstrate 'SET' matching bug
righi Apr 26, 2015
445fcf2
Add test for case-insensitive matching on 'SET'
righi Apr 26, 2015
35b8f4d
Improve match of 'SET' statements
righi Apr 26, 2015
2f9ae53
Remove unnecessary caret in SQL_ALL_MATCHERS
righi Apr 26, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions lib/makara/logging/subscriber.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,17 @@ module Makara
module Logging

module Subscriber
IGNORE_PAYLOAD_NAMES = ["SCHEMA", "EXPLAIN"]

def sql(event)
name = event.payload[:name]
name = [current_wrapper_name(event), name].compact.join(' ')
event.payload[:name] = name
super(event)
if IGNORE_PAYLOAD_NAMES.include?(name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just use ActiveRecord::LogSubscriber::IGNORE_PAYLOAD_NAMES directly if possible. Are there AR version semantics?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I originally did, but IGNORE_PAYLOAD_NAMES isn't present before ActiveRecord 4 and could disappear at any time, so it's a bit safer to define it ourselves. I don't feel too strongly either way.

self.class.runtime += event.duration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't you just want to call super here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.

else
name = [current_wrapper_name(event), name].compact.join(' ')
event.payload[:name] = name
super(event)
end
end

protected
Expand Down