Extract `db.namespace` from DSN in `AttributesFromDSN` by cyrille-leclerc · Pull Request #608 · XSAM/otelsql

cyrille-leclerc · 2026-03-26T08:02:49Z

Summary

I explore 2 solutions to add db.namespace.

In all cases, we extract db.namespace only for well-known DSN patterns to avoid mapping it to incorrect values. Well-known DSN patterns: “postgresql”, “postgres”, “mysql”, “clickhouse”, “sqlserver”, or “mssql”.

Solutions:

In this PR, db.namespace is parsed in the AttributesFromDSN function, no code change for users to benefit of it. The downside is a change of the existing behaviour. Code style remains:
```
 attrs := append(otelsql.AttributesFromDSN(mysqlDSN), semconv.DBSystemNameMySQL)

 db, err := otelsql.Open("mysql", mysqlDSN, otelsql.WithAttributes(attrs...,))
```

In PR Add DBNamespaceFromDSN helper and fix AttributesFromDSN bugs #623, we introduce an additional helper function DBNamespaceFromDSN, and users have to change their code to benefit of this improvement. New code style looks like:

 attrs := append(otelsql.AttributesFromDSN(mysqlDSN), semconv.DBSystemNameMySQL)

 if dbNamespace, err := otelsql.DBNamespaceFromDSN(mysqlDSN); err == nil {
 	attrs = append(attrs, dbNamespace)
 }

 db, err := otelsql.Open("mysql", mysqlDSN, otelsql.WithAttributes(attrs...,))

PR Summary

AttributesFromDSN now extracts the database name for the schemes mysql, postgresql, postgres, clickhouse, sqlserver, and mssql, and sets it as the db.namespace attribute (semconv.DBNamespaceKey)
Fix a panic in AttributesFromDSN when the closing protocol
parenthesis is missing and the DSN has no path nor query string,
fix server.port parsing when the DSN has a query string but no path.

Fixes:

Add db.namespace semconv attribute to DB client spans and metrics #605

codecov · 2026-03-26T09:04:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.8%. Comparing base (31f7989) to head (c17a842).

Additional details and impacted files

@@           Coverage Diff           @@
##            main    #608     +/-   ##
=======================================
+ Coverage   86.4%   86.8%   +0.4%     
=======================================
  Files         16      16             
  Lines        752     778     +26     
=======================================
+ Hits         650     676     +26     
  Misses        75      75             
  Partials      27      27

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

XSAM

@cyrille-leclerc, thanks for raising the PR! I like the idea of adding db.namespace, but several comments need to be addressed.

# Conflicts: # CHANGELOG.md

AttributesFromDSN no longer captures the db.system.name attribute, as callers are better positioned to set it based on the driver in use. The dbSystemFromScheme helper and its scheme map are removed accordingly.

cyrille-leclerc · 2026-03-31T17:37:57Z

@XSAM I removed the extraction of the db.system.name attribute and restored the mistakenly removed code comments.

cyrille-leclerc · 2026-04-05T08:25:29Z

@XSAM I have integrated all your feedback, please review

XSAM

I just realized SQL Server has a different URL schema than other databases. It uses sqlserver://username:password@host/instance?...&database=master.... So the db.namespace from AttributesFromDSN could be an instance name, instead of a db name.

Given how unreliable it could become after adding this new logic to AttributesFromDSN, it might not be worth a breaking change.

Could you move the extraction of db.namespace to a new method DBNamespaceFromDSN returning string, with "" meaning “unknown"? Thank you!

# Conflicts: # CHANGELOG.md

parseDSN now extracts the query string before parsing the path, and handles sqlserver DSNs specifically: the path segment is treated as the instance name (not the database name), and db.namespace is read from the "database" query param instead. Also fixes a bug where a bare "host:port?param=val" DSN (no path) would leave the query string attached to the host, causing net.SplitHostPort to drop the port.

Replace the manual &-split loop with url.ParseQuery, which always returns a non-nil map even on error, making the err check unnecessary.

cyrille-leclerc · 2026-04-07T21:28:40Z

@XSAM the extraction of db.namespace, server.address, and server.port is tightly coupled, as shown in this PR.

Would you prefer introducing a dedicated parseDbNamespaceFromDSN function (even if it means parsing the DSN twice), or evolving the existing parsing logic with something like a parseDSNV2 that also extracts db.namespace?

Note that I think the logic to extract the database name is robust. I did extensive research on DSN patterns for Golang database/sql and sqlserver is the only one I found that doesn't use the path of the url to specify the database.
I researched looking at database/sql drivers and at existing OSS instrumentation code.

XSAM · 2026-04-08T23:10:04Z

Would you prefer introducing a dedicated parseDbNamespaceFromDSN function (even if it means parsing the DSN twice), or evolving the existing parsing logic with something like a parseDSNV2 that also extracts db.namespace?

I think the logic for DBNamespaceFromDSN can be separated from DBNamespaceFromDSN, as the logic in DBNamespaceFromDSN is quite simple:

Find ://, and remove the content on the left if any
Find /, and remove the content on the left if any
Find ?, and remove the content on the right if any

For that, we don't need to touch AttributesFromDSN at all.

Note that I think the logic to extract the database name is robust. I did extensive research on DSN patterns for Golang database/sql and sqlserver is the only one I found that doesn't use the path of the url to specify the database.
I researched looking at database/sql drivers and at existing OSS instrumentation code.

Oracledb has a similar concept like sqlserver. For instance, an OracleDB string: host:1521/orclpdb1. The orclpdb1 means service name, not database name.

cyrille-leclerc · 2026-04-09T06:37:13Z

Thanks @XSAM , I'll follow your guidance.

cyrille-leclerc · 2026-04-12T21:46:59Z

I am exploring 2 solutions: this PR and:

Add DBNamespaceFromDSN helper and fix AttributesFromDSN bugs #623

…lexity - Extract parseHostPort() helper so parseDSN cyclomatic complexity drops from 13 to 8 (limit is 10) - Replace the catch-all else branch with an explicit switch: db.namespace is extracted from the URL path only for "postgresql", "postgres", "mysql", and "clickhouse" schemes; "sqlserver"/"mssql" continue to use the ?database= query param; all other schemes and scheme-less DSNs produce no db.namespace - Update test expectations accordingly

- Document supported schemes and bug fixes in [Unreleased] - Add test case for missing closing parenthesis with no path/query string

cyrille-leclerc · 2026-04-13T07:50:00Z

 	}

-	// [user[:password]@][protocol([addr])]/dbname[?param1=value1&paramN=valueN]
+	// [user[:password]@][protocol([addr])][/path][?param1=value1&paramN=valueN]


We see with sqlserver and oracle that the /path is not always the dbname

cyrille-leclerc · 2026-04-13T08:41:10Z


 func getDummyAttributesGetter() AttributesGetter {
 	return func(_ context.Context, method Method, query string, args []driver.NamedValue) []attribute.KeyValue {
-		//nolint:prealloc


Running locally make command removes //nolint:prealloc and breaks CI so I preferred to fix the root cause.

cyrille-leclerc · 2026-04-14T13:04:20Z

@XSAM I have now 2 PRs showing 2 slightly different user experiences. Would you mind reviewing them?

In all cases, we extract db.namespace only for well-known DSN patterns to avoid mapping it to incorrect values. Well-known DSN patterns: “postgresql”, “postgres”, “mysql”, “clickhouse”, “sqlserver”, or “mssql”.

Solutions:

In this PR, db.namespace is parsed in the AttributesFromDSN function, no code change for users to benefit of it. Code style remains:

 attrs := append(otelsql.AttributesFromDSN(mysqlDSN), semconv.DBSystemNameMySQL)

 db, err := otelsql.Open("mysql", mysqlDSN, otelsql.WithAttributes(attrs...,))

In PR Add DBNamespaceFromDSN helper and fix AttributesFromDSN bugs #623, we introduce an additional helper function DBNamespaceFromDSN, and users have to change their code to benefit of this improvement. New code style looks like:

 attrs := append(otelsql.AttributesFromDSN(mysqlDSN), semconv.DBSystemNameMySQL)

 if dbNamespace, err := otelsql.DBNamespaceFromDSN(mysqlDSN); err == nil {
 	attrs = append(attrs, dbNamespace)
 }

 db, err := otelsql.Open("mysql", mysqlDSN, otelsql.WithAttributes(attrs...,))

Extract db.namespace from DSN in AttributesFromDSN

093ef23

cyrille-leclerc requested a review from XSAM as a code owner March 26, 2026 08:02

cyrille-leclerc force-pushed the extract-db-namespace-from-dsn-2 branch from 093ef23 to c77428e Compare March 26, 2026 09:04

cyrille-leclerc added 2 commits March 26, 2026 10:42

Extract db.namespace from DSN in AttributesFromDSN

6fabdca

Better code coverage through unit tests

e4d5e0e

cyrille-leclerc force-pushed the extract-db-namespace-from-dsn-2 branch from c77428e to e4d5e0e Compare March 26, 2026 09:45

Fix test-coverage goal

ae48eda

cyrille-leclerc changed the title ~~Extract db.namespace from DSN in AttributesFromDSN~~ Extract db.namespace and db.system.name from DSN in AttributesFromDSN Mar 26, 2026

update changelog.md

0115bb2

XSAM requested changes Mar 31, 2026

View reviewed changes

Comment thread helpers.go Outdated

Comment thread helpers.go

cyrille-leclerc added 3 commits March 31, 2026 12:49

Merge branch 'main' into extract-db-namespace-from-dsn-2

04afb00

# Conflicts: # CHANGELOG.md

Better comments

3582ac6

Remove db.system.name from AttributesFromDSN

dce7fbd

AttributesFromDSN no longer captures the db.system.name attribute, as callers are better positioned to set it based on the driver in use. The dbSystemFromScheme helper and its scheme map are removed accordingly.

cyrille-leclerc changed the title ~~Extract db.namespace and db.system.name from DSN in AttributesFromDSN~~ Extract db.namespace from DSN in AttributesFromDSN Mar 31, 2026

Better commeRemove db.system.name from AttributesFromDSNnts

3b8b728

cyrille-leclerc requested a review from XSAM March 31, 2026 17:38

XSAM reviewed Apr 2, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Comment thread CHANGELOG.md Outdated

Comment thread helpers.go Outdated

Comment thread helpers.go Outdated

Comment thread helpers.go Outdated

Comment thread CHANGELOG.md Outdated

Comment thread Makefile Outdated

cyrille-leclerc added 9 commits April 2, 2026 20:39

remove the scheme returned value + fix edge case

6d55c88

Fix CHANGELOG.md

1d35f5c

Better comments

0a00b8e

Revert Makefile change

6cdd26f

Fix CHANGELOG.md

8a42793

Better align with previous code

70690a7

Better align with previous code

7add5cc

Better align with previous code

1ee7247

Better align with previous code

e74e5cb

XSAM reviewed Apr 6, 2026

View reviewed changes

cyrille-leclerc added 4 commits April 7, 2026 21:54

Merge branch 'main' into extract-db-namespace-from-dsn-2

55c61f5

# Conflicts: # CHANGELOG.md

Simplify parseDSN: use url.ParseQuery for sqlserver database param

77e2718

Replace the manual &-split loop with url.ParseQuery, which always returns a non-nil map even on error, making the err check unnecessary.

restore "nolint"

a9ff19a

cyrille-leclerc closed this Apr 12, 2026

cyrille-leclerc reopened this Apr 13, 2026

cyrille-leclerc added 5 commits April 13, 2026 09:32

Restore nolint:gosec on TestAttributesFromDSN

05e3bde

Update CHANGELOG and add test for malformed DSN

1e56f4f

- Document supported schemes and bug fixes in [Unreleased] - Add test case for missing closing parenthesis with no path/query string

Merge branch 'main' into extract-db-namespace-from-dsn-2

a56b1cc

Preallocate attrs slice in getDummyAttributesGetter

0232957

Consolidate Fixed section in CHANGELOG [Unreleased]

c17a842

cyrille-leclerc commented Apr 13, 2026

View reviewed changes

cyrille-leclerc mentioned this pull request Apr 13, 2026

Add DBNamespaceFromDSN helper and fix AttributesFromDSN bugs #623

Open

Conversation

cyrille-leclerc commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

PR Summary

Uh oh!

codecov bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

XSAM left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cyrille-leclerc commented Mar 31, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cyrille-leclerc commented Apr 5, 2026

Uh oh!

XSAM left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyrille-leclerc commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XSAM commented Apr 8, 2026

Uh oh!

cyrille-leclerc commented Apr 9, 2026

Uh oh!

cyrille-leclerc commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cyrille-leclerc Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

cyrille-leclerc Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

cyrille-leclerc commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cyrille-leclerc commented Mar 26, 2026 •

edited

Loading

codecov bot commented Mar 26, 2026 •

edited

Loading

XSAM left a comment •

edited

Loading

cyrille-leclerc commented Apr 7, 2026 •

edited

Loading

cyrille-leclerc commented Apr 12, 2026 •

edited

Loading