Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sql] Parse db.operation.name and db.collection.name from db.query.text #2222

Open
alanwest opened this issue Oct 17, 2024 · 2 comments
Open
Assignees
Labels
comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient enhancement New feature or request

Comments

@alanwest
Copy link
Member

alanwest commented Oct 17, 2024

Component

OpenTelemetry.Instrumentation.SqlClient

What is the expected behavior?

The attributes db.operation.name and db.collection.name are conditionally required "if readily available".

Alternatively, they MAY be parsed from db.query.text. See relevant footnotes.

[3]: If readily available. The collection name MAY be parsed from the query text, in which case it SHOULD be the first collection name found in the query.
[6]: If readily available. The operation name MAY be parsed from the query text, in which case it SHOULD be the first operation name found in the query.

These attributes are very valuable to end users.

Parsing db.query.text is a costly operation. What this issue does not attempt to address is whether parsing operation/collection from db.query.text should be on by default. For now, it should be implemented as opt-in, though later we should decide whether it should be on by default.

@alanwest alanwest added the enhancement New feature or request label Oct 17, 2024
@alanwest alanwest self-assigned this Oct 17, 2024
@github-actions github-actions bot added the comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient label Oct 17, 2024
@alanwest alanwest changed the title [feature request] Parse db.operation.name and db.collection.name from db.query.text [sql] Parse db.operation.name and db.collection.name from db.query.text Oct 17, 2024
@hannahramadan
Copy link

Ruby has an in-progress PR to add db.collection.name. Here is the regex:

# Capture the first word (including letters, digits, underscores, & '.', ) that follows common table commands
TABLE_NAME = /\b(?:(?:FROM|INTO|UPDATE)|(?:(?:CREATE|DROP|ALTER)\s+TABLE(?:\s+IF\s+(?:NOT\s+)?EXISTS)?))\s+["]?([\w.]+)["]?/i

Adding this note as more of a resource/ point of discussion when this ticket gets worked on. Might be a better way to grab this.

@perlun
Copy link

perlun commented Nov 26, 2024

Parsing db.query.text is a costly operation. What this issue does not attempt to address is whether parsing operation/collection from db.query.text should be on by default. For now, it should be implemented as opt-in, though later we should decide whether it should be on by default.

@alanwest I think it would also be valuable to be able to override this from an application POV, perhaps with an API similar to SqlClientTraceInstrumentationOptions.Enrich(). The application may be able to better provide the db.operation.name (e.g. findAndModify as given in the examples here: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#common-attributes) than what the SqlClient instrumentation code can determine on its own.

There's also the db.query.summary field (#2238). Given that "The span name SHOULD be {db.query.summary} if a summary is available.", perhaps it would be even more important to be able to override this from user code (if the spec permits it). 🤔

(My take on this: trying to move away from the span name on all trace data being just the database name. 😅 It doesn't look very good when looking at the data in an OpenTelemetry web UI)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants