charkost / prosopite

:mag: Rails N+1 queries auto-detection with zero false positives / false negatives
Apache License 2.0
1.57k stars 46 forks source link

Not ready to work with multiple databases #48

Open Koilanetroc opened 2 years ago

Koilanetroc commented 2 years ago

As I can see in source prosopite uses ActiveRecord::Base to define database adapter. If you have multiple databases you probably have multiple successors of ApplicationRecord where each of them uses different database. When prosopite uses ActiveRecord::Base it will always get adapter of primary database which is not correct. As a result, gem just fails when encounters active record queries from not primary database and can't be used at all.

Not sure if it's easy fix, but maybe it is worth mentioning it in docs?

technicalpickles commented 1 year ago

Adding a permalink copy, since the line has changed: https://github.com/charkost/prosopite/blob/5d1fbc1624452a8262e31df5bf6e6c5e7c51c983/lib/prosopite.rb#L109

It looks like this code is only using it to determine if it's mysql or postgresql.

We do use prosopite with multiple database, but they are all the same kind (mysql).

technicalpickles commented 1 year ago

It looks like we use ActiveSupport::Notification to subscribe to sql.active_record:

https://github.com/charkost/prosopite/blob/5d1fbc1624452a8262e31df5bf6e6c5e7c51c983/lib/prosopite.rb#L215-L218

Pulling up the docs, it looks like one of the pieces of data that comes across is the Connection object. That means it should be feasible to save details about the connection's database type with the sql queries, but does require changing some of the way data is saved about the queries.

bf4 commented 1 year ago

test_prof uses ActiveRecord::Base.connection_handler.connection_pool_list which gets a list of the connection classes. might be a useful strategy here. we use multiple dbs but I don't think we've seen issues.

e.g. to get a list of all my connection classes I can

ActiveRecord::Base.connection_handler.connection_pool_list.map(&:connection_class).map(&:name).sort
jasonkarns commented 1 year ago

Is there any workaround for using prosopite with multiple databases that aren't all the same? We use both mysql and postgres and were hoping to start using prosopite.