While looking at the databasesql instrumentation, I found that parse.go doesn't handle several standard connection string formats, which causes spans to silently lose server.address and db.name attributes.
Three specific issues:
-
parsePostgres() function only handles the postgres:// url scheme and the libpq key value format (host=localhost port=5432 dbname=mydb) fails entirely, both pq and pgx use this as a primary connection method.
-
parseMySQL() function requires parentheses around the host address. DSNs like user:pass@/dbname return "invalid MySQL DSN" even though go sql driver's mysql supports this and defaults to localhost:3306.
-
ParseDbName() function only searches backward for /. key value patterns like dbname=mydb (libpq) and Database=myDB (SQL Server) return empty string.
when any of these fail, client.go falls back to "unknown" for the address and empty string for the db name.
I'm happy to open a PR with fixes and tests. No tests exist for this package currently so yeah wanted to confirm first if this is a known limitation or intentional before putting in the work
While looking at the databasesql instrumentation, I found that parse.go doesn't handle several standard connection string formats, which causes spans to silently lose server.address and db.name attributes.
Three specific issues:
parsePostgres() function only handles the
postgres://url scheme and the libpq key value format(host=localhost port=5432 dbname=mydb)fails entirely, both pq and pgx use this as a primary connection method.parseMySQL() function requires parentheses around the host address. DSNs like
user:pass@/dbnamereturn "invalid MySQL DSN" even though go sql driver's mysql supports this and defaults to localhost:3306.ParseDbName() function only searches backward for /. key value patterns like
dbname=mydb (libpq)andDatabase=myDB (SQL Server)return empty string.when any of these fail, client.go falls back to "unknown" for the address and empty string for the db name.
I'm happy to open a PR with fixes and tests. No tests exist for this package currently so yeah wanted to confirm first if this is a known limitation or intentional before putting in the work