From 0642a68194e252140ac2b13760825e0b26bf6e2d Mon Sep 17 00:00:00 2001 From: alaotach Date: Thu, 7 May 2026 01:10:39 +0530 Subject: [PATCH 1/2] fix(pkg/instrumentation/databasesql): traces drop server.address for standard DSN formats --- pkg/instrumentation/databasesql/parse.go | 82 +++++++++++++++++++++++- test/apps/dbclient/main.go | 5 ++ test/integration/db_client_test.go | 57 ++++++++++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) diff --git a/pkg/instrumentation/databasesql/parse.go b/pkg/instrumentation/databasesql/parse.go index a7cf15841..e285d9d80 100644 --- a/pkg/instrumentation/databasesql/parse.go +++ b/pkg/instrumentation/databasesql/parse.go @@ -7,10 +7,17 @@ import ( "errors" "fmt" nurl "net/url" + "regexp" "strings" ) +var dbNamePattern = regexp.MustCompile(`(?i)(^|[\s;?&])(?:dbname|database)\s*=\s*('([^']*)'|"([^"]*)"|[^;\s&]+)`) + func ParseDbName(dsn string) string { + if name := parseDBNameFromKeyValue(dsn); name != "" { + return name + } + var name string var err error for i := len(dsn) - 1; i >= 0; i-- { @@ -30,6 +37,30 @@ func ParseDbName(dsn string) string { return name } +func parseDBNameFromKeyValue(dsn string) string { + match := dbNamePattern.FindStringSubmatch(dsn) + if len(match) == 0 { + return "" + } + + value := strings.TrimSpace(match[2]) + if value == "" { + return "" + } + + if len(value) >= 2 { + if (value[0] == '\'' && value[len(value)-1] == '\'') || (value[0] == '"' && value[len(value)-1] == '"') { + value = value[1 : len(value)-1] + } + } + + name, err := nurl.PathUnescape(value) + if err != nil { + return "" + } + return name +} + func parseDSN(driverName, dsn string) (addr string, err error) { // TODO: need a more delegate DFA switch driverName { @@ -53,6 +84,17 @@ func parseDSN(driverName, dsn string) (addr string, err error) { } func parsePostgres(url string) (addr string, err error) { + if !strings.Contains(url, "://") { + host, port, ok := parseConnectionStringPair(url, []string{"host"}) + if !ok || host == "" { + return "", errors.New("invalid Postgres DSN") + } + if port == "" { + port = "5432" + } + return host + ":" + port, nil + } + u, err := nurl.Parse(url) if err != nil { return "", err @@ -97,11 +139,49 @@ func parseMySQL(dsn string) (addr string, err error) { } } if i >= 0 && j > i { - return dsn[i+1 : j], nil + addr := dsn[i+1 : j] + if strings.TrimSpace(addr) == "" { + return "localhost:3306", nil + } + return addr, nil + } + + if strings.Contains(dsn, "@/") || strings.HasPrefix(dsn, "/") { + return "localhost:3306", nil } return "", errors.New("invalid MySQL DSN") } +func parseConnectionStringPair(dsn string, keys []string) (host string, port string, ok bool) { + pairs := strings.Fields(dsn) + var hostFound bool + + for _, pair := range pairs { + parts := strings.SplitN(pair, "=", 2) + if len(parts) != 2 { + continue + } + key := strings.ToLower(strings.TrimSpace(parts[0])) + value := strings.Trim(strings.TrimSpace(parts[1]), `"'`) + + for _, k := range keys { + if key == k { + host = value + hostFound = true + break + } + } + if key == "port" { + port = value + } + } + + if !hostFound { + return "", "", false + } + return host, port, true +} + func parseClickHouse(dsn string) (addr string, err error) { // ClickHouse DSN formats: // tcp://host:port?database=dbname&username=user&password=pass diff --git a/test/apps/dbclient/main.go b/test/apps/dbclient/main.go index ac7af92f8..ee4e0d1d3 100644 --- a/test/apps/dbclient/main.go +++ b/test/apps/dbclient/main.go @@ -25,6 +25,11 @@ var ( func init() { sql.Register("testdb", &testDriver{}) + sql.Register("mysql", &testDriver{}) + sql.Register("postgres", &testDriver{}) + sql.Register("postgresql", &testDriver{}) + sql.Register("mssql", &testDriver{}) + sql.Register("sqlserver", &testDriver{}) } func main() { diff --git a/test/integration/db_client_test.go b/test/integration/db_client_test.go index 5d5a931eb..7482caacf 100644 --- a/test/integration/db_client_test.go +++ b/test/integration/db_client_test.go @@ -181,3 +181,60 @@ func TestDBClientAll(t *testing.T) { "testdb", ) } + +func TestDBClientPostgresLibpqDSN(t *testing.T) { + f := testutil.NewTestFixture(t) + + f.BuildAndRun("dbclient", + "-driver=postgres", + "-dsn=host=localhost port=5432 dbname=mydb user=postgres", + "-op=ping", + ) + + span := f.RequireSingleSpan() + require.Equal(t, "PING", span.Name()) + testutil.RequireDBClientSemconv(t, span, + "PING", + "ping", + "localhost", 5432, + "mydb", + ) +} + +func TestDBClientMySQLDefaultHostDSN(t *testing.T) { + f := testutil.NewTestFixture(t) + + f.BuildAndRun("dbclient", + "-driver=mysql", + "-dsn=user:pass@/mydb", + "-op=ping", + ) + + span := f.RequireSingleSpan() + require.Equal(t, "PING", span.Name()) + testutil.RequireDBClientSemconv(t, span, + "PING", + "ping", + "localhost", 3306, + "mydb", + ) +} + +func TestDBClientSQLServerDatabaseKeyDSN(t *testing.T) { + f := testutil.NewTestFixture(t) + + f.BuildAndRun("dbclient", + "-driver=sqlserver", + "-dsn=Server=localhost,1433;Database=myDB;User Id=sa;Password=Pass123", + "-op=ping", + ) + + span := f.RequireSingleSpan() + require.Equal(t, "PING", span.Name()) + testutil.RequireDBClientSemconv(t, span, + "PING", + "ping", + "localhost", 1433, + "myDB", + ) +} From 4c64234560301523f4f79fa08a5fd76123ec0d3a Mon Sep 17 00:00:00 2001 From: alaotach Date: Thu, 7 May 2026 12:09:32 +0530 Subject: [PATCH 2/2] databasesql: fix libpq IPv6 parsing and reuse dbclient build Use JoinHostPort for libpq key/value hosts and add IPv6 DSN coverage. Reuse the dbclient build across DB integration tests to cut runtime. --- pkg/instrumentation/databasesql/parse.go | 3 +- test/integration/db_client_test.go | 37 ++++++++++---- test/testutil/fixture.go | 11 +++++ test/testutil/infra.go | 63 +++++++++++++++++++----- 4 files changed, 91 insertions(+), 23 deletions(-) diff --git a/pkg/instrumentation/databasesql/parse.go b/pkg/instrumentation/databasesql/parse.go index e285d9d80..2418b12fd 100644 --- a/pkg/instrumentation/databasesql/parse.go +++ b/pkg/instrumentation/databasesql/parse.go @@ -6,6 +6,7 @@ package db import ( "errors" "fmt" + "net" nurl "net/url" "regexp" "strings" @@ -92,7 +93,7 @@ func parsePostgres(url string) (addr string, err error) { if port == "" { port = "5432" } - return host + ":" + port, nil + return net.JoinHostPort(strings.Trim(host, "[]"), port), nil } u, err := nurl.Parse(url) diff --git a/test/integration/db_client_test.go b/test/integration/db_client_test.go index 7482caacf..a4cd7c343 100644 --- a/test/integration/db_client_test.go +++ b/test/integration/db_client_test.go @@ -17,7 +17,7 @@ import ( func TestDBClientPing(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", "-op=ping") + f.BuildSharedAndRun("dbclient", "-op=ping") span := f.RequireSingleSpan() require.Equal(t, "PING", span.Name()) @@ -32,7 +32,7 @@ func TestDBClientPing(t *testing.T) { func TestDBClientExec(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", "-op=exec") + f.BuildSharedAndRun("dbclient", "-op=exec") span := f.RequireSingleSpan() require.Equal(t, "INSERT", span.Name()) @@ -47,7 +47,7 @@ func TestDBClientExec(t *testing.T) { func TestDBClientQuery(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", "-op=query") + f.BuildSharedAndRun("dbclient", "-op=query") span := f.RequireSingleSpan() require.Equal(t, "SELECT", span.Name()) @@ -62,7 +62,7 @@ func TestDBClientQuery(t *testing.T) { func TestDBClientPrepareAndQuery(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", "-op=prepare") + f.BuildSharedAndRun("dbclient", "-op=prepare") // PrepareContext doesn't create a span directly, but stmt.QueryContext does spans := testutil.AllSpans(f.Traces()) @@ -76,7 +76,7 @@ func TestDBClientPrepareAndQuery(t *testing.T) { func TestDBClientTransaction(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", "-op=tx") + f.BuildSharedAndRun("dbclient", "-op=tx") spans := testutil.AllSpans(f.Traces()) // BeginTx -> ExecContext -> Commit = 3 spans @@ -105,7 +105,7 @@ func TestDBClientTransaction(t *testing.T) { func TestDBClientAll(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", + f.BuildSharedAndRun("dbclient", "-driver=testdb", "-dsn=user:pass@tcp(127.0.0.1:3306)/testdb?charset=utf8", "-op=all", @@ -185,7 +185,7 @@ func TestDBClientAll(t *testing.T) { func TestDBClientPostgresLibpqDSN(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", + f.BuildSharedAndRun("dbclient", "-driver=postgres", "-dsn=host=localhost port=5432 dbname=mydb user=postgres", "-op=ping", @@ -201,10 +201,29 @@ func TestDBClientPostgresLibpqDSN(t *testing.T) { ) } +func TestDBClientPostgresLibpqIPv6DSN(t *testing.T) { + f := testutil.NewTestFixture(t) + + f.BuildSharedAndRun("dbclient", + "-driver=postgres", + "-dsn=host=::1 port=5432 dbname=mydb user=postgres", + "-op=ping", + ) + + span := f.RequireSingleSpan() + require.Equal(t, "PING", span.Name()) + testutil.RequireDBClientSemconv(t, span, + "PING", + "ping", + "::1", 5432, + "mydb", + ) +} + func TestDBClientMySQLDefaultHostDSN(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", + f.BuildSharedAndRun("dbclient", "-driver=mysql", "-dsn=user:pass@/mydb", "-op=ping", @@ -223,7 +242,7 @@ func TestDBClientMySQLDefaultHostDSN(t *testing.T) { func TestDBClientSQLServerDatabaseKeyDSN(t *testing.T) { f := testutil.NewTestFixture(t) - f.BuildAndRun("dbclient", + f.BuildSharedAndRun("dbclient", "-driver=sqlserver", "-dsn=Server=localhost,1433;Database=myDB;User Id=sa;Password=Pass123", "-op=ping", diff --git a/test/testutil/fixture.go b/test/testutil/fixture.go index 8170d6b18..bf7925d54 100644 --- a/test/testutil/fixture.go +++ b/test/testutil/fixture.go @@ -92,6 +92,11 @@ func (f *TestFixture) Build(appName string) { Build(f.t, f.resolveAppPath(appName), "go", "build", "-a") } +// BuildShared builds a test application once and reuses the binary across tests. +func (f *TestFixture) BuildShared(appName string) { + BuildShared(f.t, f.resolveAppPath(appName), "go", "build", "-a") +} + // Server represents a running server process. type Server struct { t *testing.T @@ -126,6 +131,12 @@ func (f *TestFixture) BuildAndRun(appName string, args ...string) string { return f.Run(appName, args...) } +// BuildSharedAndRun builds a test application once and runs it. +func (f *TestFixture) BuildSharedAndRun(appName string, args ...string) string { + f.BuildShared(appName) + return f.Run(appName, args...) +} + // RequireTraceCount asserts the expected number of traces were collected. func (f *TestFixture) RequireTraceCount(expected int) { stats := AnalyzeTraces(f.t, f.collector.GetTraces()) diff --git a/test/testutil/infra.go b/test/testutil/infra.go index 30398e846..2c71eee97 100644 --- a/test/testutil/infra.go +++ b/test/testutil/infra.go @@ -5,9 +5,11 @@ package testutil import ( "context" + "fmt" "os" "os/exec" "path/filepath" + "sync" "testing" "github.com/stretchr/testify/require" @@ -34,31 +36,66 @@ func newCmd(ctx context.Context, dir string, args ...string) *exec.Cmd { return cmd } -// Build builds the application with the instrumentation tool. -func Build(t *testing.T, appDir string, args ...string) { +type sharedBuild struct { + once sync.Once + err error +} + +var sharedBuilds sync.Map + +func appBinaryName() string { + name := appBinName + if util.IsWindows() { + name += ".exe" + } + return name +} + +func appBinaryPath(appDir string) string { + return filepath.Join(appDir, appBinaryName()) +} + +func buildApp(ctx context.Context, appDir string, args ...string) error { binName := otelcBinName if util.IsWindows() { binName += ".exe" } pwd, err := os.Getwd() - require.NoError(t, err) - otelcPath := filepath.Join(pwd, "..", "..", binName) - - // Use a consistent binary name for all test apps - outputName := appBinName - if util.IsWindows() { - outputName += ".exe" + if err != nil { + return err } + otelcPath := filepath.Join(pwd, "..", "..", binName) - args = append(args, "-o", outputName) + args = append(args, "-o", appBinaryName()) args = append([]string{otelcPath}, args...) - cmd := newCmd(t.Context(), appDir, args...) + cmd := newCmd(ctx, appDir, args...) out, err := cmd.CombinedOutput() - require.NoError(t, err, string(out)) + if err != nil { + return fmt.Errorf("build failed: %w: %s", err, string(out)) + } + return nil +} + +// Build builds the application with the instrumentation tool. +func Build(t *testing.T, appDir string, args ...string) { + if err := buildApp(t.Context(), appDir, args...); err != nil { + require.NoError(t, err) + } t.Cleanup(func() { - os.Remove(filepath.Join(appDir, outputName)) + _ = os.Remove(appBinaryPath(appDir)) + }) +} + +// BuildShared builds the app once per process and keeps the binary for reuse. +func BuildShared(t *testing.T, appDir string, args ...string) { + t.Helper() + entry, _ := sharedBuilds.LoadOrStore(appDir, &sharedBuild{}) + build := entry.(*sharedBuild) + build.once.Do(func() { + build.err = buildApp(t.Context(), appDir, args...) }) + require.NoError(t, build.err) } // Run runs the application and returns the output.