contrib/database/sql: panic: sqltrace.OpenDB: driver is not registered via sqltrace.Register

[tsbkw]

TL;DR

If we use some driver which implement driver.DriverContext interface, and call Register with driver struct(not a pointer to driver struct), it panic with panic: sqltrace.OpenDB: driver is not registered via sqltrace.Register.

It may be better to prevent error even if struct is passed when calling Register, but at least document should be fixed.

Details

In the example below, driver.Driver is passed as struct, and reflect.TypeOf(driver) is added to map.

dd-trace-go/contrib/database/sql/sql.go

Lines 12 to 13 in 2d25957

	// sqltrace.Register("pq", pq.Driver{})
	// db, err := sqltrace.Open("pq", "postgres://pqgotest:password@localhost...")

Later when user call OpenDB, reflect.TypeOf(driver) is looked up in d.keys, therefore driver added and looked up must have same type. For example, if added type is struct, looked up by struct. And if added type is pointer of struct, looked up by pointer of struct.

dd-trace-go/contrib/database/sql/sql.go

Line 76 in 2d25957

name, ok := d.keys[reflect.TypeOf(driver)]

This behavior is not a problem when driver doesn't implement driver.DriverContext interface, because in this case driver get from map driver is used when lookup and type of these always same.

dd-trace-go/contrib/database/sql/sql.go

Line 236 in 2d25957

return OpenDB(&dsnConnector{dsn: dataSourceName, driver: d}, opts...), nil

However when driver implement driver.DriverContext, user must care whether type is pointer or not when calling Register.

Driver in lib/pq currently not implement driver.DriverContext but soon it will implement. see: lib/pq#900

The problem can be reproduced by code below with github.com/go-sql-driver/mysql. (You can see full code at https://go.dev/play/p/4g6-qwh9TqF but you have to run it locally.)

	sqltrace.Register("mysql", mysql.MySQLDriver{})
	_, err := sqltrace.Open("mysql", "user:password@tcp(localhost:3306)/db")
	fmt.Printf("err with mysql.MySQLDriver{} %v\n", err)

This print panic: sqltrace.OpenDB: driver is not registered via sqltrace.Register with stacktrace.

The error can be avoided by passing by pointer like below.

	sqltrace.Register("mysql", &mysql.MySQLDriver{})
	_, err2 := sqltrace.Open("mysql", "user:password@tcp(localhost:3306)/dbe")
	fmt.Printf("err with &mysql.MySQLDriver{} %v\n", err2)

This print err with &mysql.MySQLDriver{} <nil>.

[zarirhamza]

PR was merged and issue should be fixed. Your contribution is much appreciated and if there are any ongoing problems, please open another issue. Thank you!