Add mysql driver support for Sqlmetrics #14
Conversation
| func storeConnInfo(conn *proxy.Conn, dsn string) error { | ||
| info, err := parseDSN(dsn) | ||
| func storeConnInfo(conn *proxy.Conn, driverName, dsn string) error { | ||
| info, err := parseDSN(driverName, dsn) |
There was a problem hiding this comment.
Use dburl package to derive this. Every dsn that connects already has this information. If it doesn't. It may not connect eirher.
| @@ -51,8 +51,29 @@ func loadConnInfo(conn *proxy.Conn) *connInfo { | |||
| return info.(*connInfo) | |||
| } | |||
|
|
|||
| func parseDSN(dsn string) (*connInfo, error) { | |||
| u, err := dburl.Parse(dsn) | |||
| // Note: the DB Url library expects a scheme in the DSN for | |||
There was a problem hiding this comment.
This is not a correct way to handle this. Sql package supports magnitudes of queriersz we cannot switch case for all. Let the expected dan contain the scheme.
| func getStatementName(driver, q string) string { | ||
|
|
||
| switch strings.Split(driver, ":")[0] { | ||
| case "postgres": |
There was a problem hiding this comment.
Promote these strings as module global constants. Strings don't auto complete, variables do.
| @@ -52,21 +52,23 @@ func TestPq(t *testing.T) { | |||
| srv := tests.MakeServer(mux) | |||
| defer srv.Close() | |||
|
|
|||
| driverName := "postgres" | |||
There was a problem hiding this comment.
Use the same module level constants here.
|
Btw you might hve to change the harness too since you changed the wait_postgres to wait_db |
| @@ -145,7 +145,7 @@ func RegisterDriverWithLabelMaker(d Options, fn LabelMaker) (string, error) { | |||
| dc := ctx.(*dbCtx) | |||
|
|
|||
| if err := emitDuration( | |||
| fn(dc.query).Merge( | |||
| fn(name, dc.query).Merge( | |||
There was a problem hiding this comment.
Not a scalable design you see. Imagine from the consumer who is overriding this. You may have to take a switch based on driver name. Others may do so on other basis. Best way is to return the Options struct itself in the function argument.
Also, don't use d.GetName() for switch case, use d.Name which is the original unmangled name.
statementof the query to emitted labelssnapshot of sample app with sqlmetrics emitted with the above changes