Skip to content

DbInfo simplification#16248

Merged
laurit merged 2 commits into
open-telemetry:mainfrom
trask:small-db-info-prefactor
Feb 25, 2026
Merged

DbInfo simplification#16248
laurit merged 2 commits into
open-telemetry:mainfrom
trask:small-db-info-prefactor

Conversation

@trask

@trask trask commented Feb 21, 2026

Copy link
Copy Markdown
Member

This is a preliminary simplification before adding support for db.namespace to DbInfo parsing.

@github-actions github-actions Bot added the test native This label can be applied to PRs to trigger them to run native tests label Feb 21, 2026
@trask trask marked this pull request as ready for review February 21, 2026 02:44
@trask trask requested a review from a team as a code owner February 21, 2026 02:44
.setHost("ss.host")
.setPort(1444)
.setName("ssinstance")
.setDb("ssdb")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Elsewhere name seems to be database name but here it is an instance name, is this correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's correct, but it's existing behavior. I've fixed it under stable semconv flag in #16277.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/database/database-spans.md says

In some SQL databases, the database name to be used is called "schema name". In case there are multiple layers that could be considered for database name (e.g. Oracle instance name and schema name), the database name to be used is the more specific layer (e.g. Oracle schema name).

Based on that I think we shouldn't bother with the instance name at all.

@laurit laurit merged commit dae3d57 into open-telemetry:main Feb 25, 2026
87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test native This label can be applied to PRs to trigger them to run native tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants