contrib/database/sql: Fix sample code in document to prevent panic on OpenDB #1635
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fix document to prevent to misuse Register function.
Issue: #1634
Motivation
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
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
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
However when driver implement driver.DriverContext, user must care whether type is pointer or not when calling
Register
.Driver in
lib/pq
currently not implementdriver.DriverContext
but soon it will implement. see: lib/pq#900Ideally the way look up should be better, but at least document should be fixed.
Describe how to test/QA your changes
The problem can be reproduced by code below.
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.
This print
err with &mysql.MySQLDriver{} <nil>
.Reviewer's Checklist
Triage
milestone is set.