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

instrumentDriver() register wrapped driver per driver instead of per connection #725

Open
sio4 opened this issue Jun 1, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@sio4
Copy link
Member

sio4 commented Jun 1, 2022

The function instrumentDriver() registers a custom SQL driver with sql.Register() and it uses a decorated driver name as something like instrumented-sql-driver-sqlite3 which is based on the driver name but actually it used wrapper options per connection. So if there are two or more connections with the same driver, only one wrapped driver will be registered with the wrapper option of the first successful connection.

The purpose of the wrapped driver is to support logging, tracing, etc. and those actions can be applied to each connection with different settings.

func instrumentDriver(deets *ConnectionDetails, defaultDriverName string) (driverName, dialect string, err error) {

See this and the following comments for more details.

Related Issues or PRs

@sio4 sio4 added the bug Something isn't working label Jun 1, 2022
@sio4 sio4 self-assigned this Jun 1, 2022
@zepatrik
Copy link
Contributor

zepatrik commented Jun 1, 2022

I guess one option to consider would be to not register the instrumented drivers in the first place. After looking a bit through the code, it would be easily possible to just not register them. There is only two usages of sql.Open:

  1. db, err := sql.Open(nameSQLite3, ":memory:?cache=newSQLiteDriver_temporary")
    I think this is just a helper to determine whether the sqlite driver is registered
  2. con, err := sql.Open(driverName, dsn)
    which is the single point of opening a driver in the first place

Instead of using sql.Open, it would be possible to just get the driver, potentially wrap it, and then use sql.OpenDB instead (essentially re-implement what sql.Open does, with the driver as a parameter).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants