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.

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

Add configuration to filter out sql queries #60

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ Rails.application.configure do
end
```

### Filtering sql queries by name

You can filter out queries using the `query_name_denylist` configuration.
This takes an array of regular expressions to match against the query name. If the query name matches any of the regular expressions, it will be ignored. By default, `lograge-sql` ignores queries named `SCHEMA` and queries from the `SolidCable` namespace.
If you are using Solid Cable in your project, be careful when removing this default value as it will cause a [memory leak](https://github.com/iMacTia/lograge-sql/issues/59).

```ruby
# config/initializers/lograge.rb
Rails.application.configure do
# Defaults is [/\ASCHEMA\z/, /\ASolidCable::/]
config.lograge_sql.query_name_denylist << /\AEXACT NAME TO IGNORE\z/
end
```

### Output Customization

By default, the format is a string concatenation of the query name, the query duration and the query itself joined by `\n` newline:
Expand Down
2 changes: 1 addition & 1 deletion lib/lograge/active_record_log_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def filter_query(event)
end

def valid?(event)
return false if event.payload[:name] == 'SCHEMA'
return false if event.payload[:name]&.match?(Regexp.union(Lograge::Sql.query_name_denylist))

# Only store SQL events if `event.duration` is greater than the configured +min_duration+
# No need to check if +min_duration+ is present before as it defaults to 0
Expand Down
3 changes: 3 additions & 0 deletions lib/lograge/sql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@ class << self
attr_accessor :min_duration_ms
# Filter SQL query
attr_accessor :query_filter
# Filter wich SQL queries to store
attr_accessor :query_name_denylist

# Initialise configuration with fallback to default values
def setup(config)
Lograge::Sql.formatter = config.formatter || default_formatter
Lograge::Sql.extract_event = config.extract_event || default_extract_event
Lograge::Sql.min_duration_ms = config.min_duration_ms || 0
Lograge::Sql.query_filter = config.query_filter
Lograge::Sql.query_name_denylist = config.query_name_denylist || [/\ASCHEMA\z/, /\ASolidCable::/]
Copy link

Choose a reason for hiding this comment

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

@fdaugs @iMacTia
Perhaps this could be better for performance and avoiding runtime error.

Suggested change
Lograge::Sql.query_name_denylist = config.query_name_denylist || [/\ASCHEMA\z/, /\ASolidCable::/]
Lograge::Sql.query_name_denylist = Regexp.union(config.query_name_denylist || [/\ASCHEMA\z/, /\ASolidCable::/])

related: rails/rails#46452

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kakubin Thanks for the suggestion! #63


# Disable existing ActiveRecord logging
unsubscribe_log_subscribers unless config.keep_default_active_record_log
Expand Down
40 changes: 40 additions & 0 deletions spec/lograge/active_record_log_subscriber_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
stub_const('ActiveRecord::RuntimeRegistry', ar_runtime_registry)
Lograge::Sql.extract_event = proc {}
Lograge::Sql.store.clear
Lograge::Sql.query_name_denylist = [/\ASCHEMA\z/, /\ASolidCable::/]
end

context 'with keep_default_active_record_log not set' do
Expand Down Expand Up @@ -75,5 +76,44 @@
expect(query_filter).to have_received(:call).with('foo')
end
end

context 'with default name denylist' do
before do
allow(Rails).to receive_message_chain('application.config.lograge_sql.keep_default_active_record_log') { true }
end

context 'with SCHEMA as name' do
before do
allow(event).to receive(:payload).and_return({ name: 'SCHEMA' })
end

it 'does not store the event' do
log_subscriber.sql(event)
expect(Lograge::Sql.store[:lograge_sql_queries]).to be_nil
end
end

context 'with name starting with SCHEMA' do
before do
allow(event).to receive(:payload).and_return({ name: 'SCHEMA foo' })
end

it 'stores the event' do
log_subscriber.sql(event)
expect(Lograge::Sql.store[:lograge_sql_queries]).not_to be_nil
end
end

context 'with name starting with SolidCable::' do
before do
allow(event).to receive(:payload).and_return({ name: 'SolidCable::Message Maximum' })
end

it 'does not store the event' do
log_subscriber.sql(event)
expect(Lograge::Sql.store[:lograge_sql_queries]).to be_nil
end
end
end
end
end