Repository: beatlabs/patron
Pull request #116 (2019-12-11 08:06:47)

Parse DSN to populate Connection Information
## Which problem is this PR solving?

This PR addresses #112  [Sql Client tracing does not populate the db name and the user ](https://github.com/beatlabs/patron/issues/112)

It introduces regex-based parsing for the Data Source Name connection string, so that we can populate the `ConnInfo` struct fields in `sql.go`

## Short description of the changes

* Introduce a regex-based parsing, based on an older [go-sql-driver](https://github.com/go-sql-driver/mysql/blob/f4bf8e8e0aa93d4ead0c6473503ca2f5d5eb65a8/utils.go#L34) commit.
* 100% test coverage on new code
* Introduce benchmarks and table-driven tests for new functionality

### TODO 
* Talk about if we should add support for ODBC-style DSNs (`param1=val1;param2=val2`)
* Talk about if we should return an error (personally I think not, since it's something optional)
* ~Talk about if we should *not* parse and store the password for safety reasons (it shouldn't just be laying around in memory after we connect to the database)~ Removed password field matching from the regex.
* Talk about setting defaults if the address or the protocol have not been set
* Talk about if we actually need the whole DSNInfo struct or if we should quickly return just the database and the username.

### Notes
The `sql.OpenDB` method's purpose is to [*"allow(ing) drivers to bypass a string based data source name"*](https://golang.org/pkg/database/sql/#OpenDB). I'm not sure how we could populate the `ConnInfo` struct in this case, (querying the DB itself?). The `driver.Connector`does present any useful methods.


Patch:
diff --git a/trace/sql/sql.go b/trace/sql/sql.go
index ea4b2530d..5ac508f43 100644
--- a/trace/sql/sql.go
+++ b/trace/sql/sql.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"database/sql/driver"
+	"regexp"
 	"time"
 
 	"github.com/beatlabs/patron/trace"
@@ -24,6 +25,16 @@ type Conn struct {
 	conn *sql.Conn
 }
 
+// DSNInfo contains information extracted from a valid
+// connection string. Additional parameters provided are discarded
+type DSNInfo struct {
+	Driver   string
+	DBName   string
+	Address  string
+	User     string
+	Protocol string
+}
+
 // BeginTx starts a transaction.
 func (c *Conn) BeginTx(ctx context.Context, opts *sql.TxOptions) (*Tx, error) {
 	sp, _ := c.startSpan(ctx, "conn.BeginTx", "")
@@ -116,7 +127,9 @@ func Open(driverName, dataSourceName string) (*DB, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &DB{db: db}, nil
+	info := parseDSN(dataSourceName)
+
+	return &DB{connInfo: connInfo{info.DBName, info.User}, db: db}, nil
 }
 
 // OpenDB opens a database.
@@ -373,3 +386,33 @@ func (tx *Tx) Stmt(ctx context.Context, stmt *Stmt) *Stmt {
 	defer trace.SpanSuccess(sp)
 	return &Stmt{stmt: tx.tx.StmtContext(ctx, stmt.stmt)}
 }
+
+func parseDSN(dsn string) DSNInfo {
+	res := DSNInfo{}
+
+	dsnPattern := regexp.MustCompile(
+		`^(?P<driver>.*:\/\/)?(?:(?P<username>.*?)(?::(.*))?@)?` + // [driver://][user[:password]@]
+			`(?:(?P<protocol>[^\(]*)(?:\((?P<address>[^\)]*)\))?)?` + // [net[(addr)]]
+			`\/(?P<dbname>.*?)` + // /dbname
+			`(?:\?(?P<params>[^\?]*))?$`) // [?param1=value1&paramN=valueN]
+
+	matches := dsnPattern.FindStringSubmatch(dsn)
+	fields := dsnPattern.SubexpNames()
+
+	for i, match := range matches {
+		switch fields[i] {
+		case "driver":
+			res.Driver = match
+		case "username":
+			res.User = match
+		case "protocol":
+			res.Protocol = match
+		case "address":
+			res.Address = match
+		case "dbname":
+			res.DBName = match
+		}
+	}
+
+	return res
+}


Test patch:
diff --git a/trace/sql/sql_test.go b/trace/sql/sql_test.go
new file mode 100644
index 000000000..9b3d9515a
--- /dev/null
+++ b/trace/sql/sql_test.go
@@ -0,0 +1,35 @@
+package sql
+
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestParseDSN(t *testing.T) {
+	tests := map[string]struct {
+		dsn  string
+		want DSNInfo
+	}{
+		"generic case":          {"username:password@protocol(address)/dbname?param=value", DSNInfo{"", "dbname", "address", "username", "protocol"}},
+		"empty DSN":             {"/", DSNInfo{"", "", "", "", ""}},
+		"dbname only":           {"/dbname", DSNInfo{"", "dbname", "", "", ""}},
+		"multiple @":            {"user:p@/ssword@/", DSNInfo{"", "", "", "user", ""}},
+		"driver and multiple @": {"postgresql://user:p@/ssword@/", DSNInfo{"postgresql://", "", "", "user", ""}},
+		"unix socket":           {"user@unix(/path/to/socket)/dbname?charset=utf8", DSNInfo{"", "dbname", "/path/to/socket", "user", "unix"}},
+		"params added":          {"user:password@/dbname?param1=val1&param2=val2&param3=val3", DSNInfo{"", "dbname", "", "user", ""}},
+		"IP as address":         {"bruce:hunter2@tcp(127.0.0.1)/arkhamdb?param=value", DSNInfo{"", "arkhamdb", "127.0.0.1", "bruce", "tcp"}},
+		"@ in path to socker":   {"user@unix(/path/to/mydir@/socket)/dbname?charset=utf8", DSNInfo{"", "dbname", "/path/to/mydir@/socket", "user", "unix"}},
+		"port in address":       {"user:password@tcp(localhost:5555)/dbname?charset=utf8&tls=true", DSNInfo{"", "dbname", "localhost:5555", "user", "tcp"}},
+		"multiple ':'":          {"us:er:name:password@memory(localhost:5555)/dbname?charset=utf8&tls=true", DSNInfo{"", "dbname", "localhost:5555", "us", "memory"}},
+		"IPv6 provided":         {"user:p@ss(word)@tcp([c023:9350:225b:671a:2cdd:3d83:7c19:ca42]:80)/dbname?loc=Local", DSNInfo{"", "dbname", "[c023:9350:225b:671a:2cdd:3d83:7c19:ca42]:80", "user", "tcp"}},
+		"empty string":          {"", DSNInfo{"", "", "", "", ""}},
+		"non-matching string":   {"rosebud", DSNInfo{"", "", "", "", ""}},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := parseDSN(tc.dsn)
+			assert.Equal(t, got, tc.want)
+		})
+	}
+}