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

Database span name clarification and improvements #743

Closed
wants to merge 1 commit into from

Conversation

lmolkova
Copy link
Contributor

Fixes #704

Changes

  • Clarifies span name patterns for databases:
    • db.statement (if low-cardinality) -> db.name db.statement (order matters because ShopDb SELECT * FROM orders WHERE order_id = ? is better than SELECT * FROM orders WHERE order_id = ? ShopDb
    • <db.operation> <db.name>.<db.sql.table> -> {db.name}.{table name} {operation} if no statement (or of a high cardinality)
  • Updates span names for some individual systems to follow the same pattern
    • also adds db.sql.table to mssql - at least in java it's set for everything JDBC and makes sense on mssql
  • Fallback to db.system if nothing else is available

To clarify:

  • case normalization (SELECT * FROM foo vs sElEcT fROm FOO)
  • elastic search uses - can/should we change?
  • hbase - Document HBase attributes #742
  • aws dynamodb - not clear what should be used as a span name
  • CouchDB - probably has other attributes, currently doing what java instrumentation does
  • Redis uses db index as database name, but index looks bad in the span names

Merge requirement checklist

Comment on lines +62 to +63
- ref: db.sql.table
tag: connection-level-tech-specific
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better if db.mssql would just have extends: db.sql? Currently that's what happens in practice (since db.mssql has the only attribute, db.sql.table from db.sql and the span name pattern is also the same). Long term all SQL based databases could just extend db.sql - e.g. I can imagine, over time there would be some Oracle or IBM Db3 specific attributes - all those could then just extend from db.sql.

@gregkalapos
Copy link
Member

To the To Clarify -> elastic search uses - can/should we change? part:

Regarding the span name currently described in the spec: using the endpoint identifier as the span name was a result of multiple rounds of discussions internally, so within Elastic we thought about that part a lot. The endpoint identifier is a finite set of values which meaningfully describes the span for users and Elasticsearch clients can get the value easily as well. However, currently the spec ends with:

If the endpoint id is not available, the span name SHOULD be the http.request.method.

Based on the discussion from #704 - that probably could be changed to also fallback to db.system - that'd be better aligned with the overall spec.

Hope this helps - if this part wasn't about the span name, but about something else from the Elasticsearch spec, happy to comment on that as well.

@lmolkova
Copy link
Contributor Author

There've been a lot of changes in table/database name since this draft was created. I'm going to close this one and open a new PR.

Thanks @gregkalapos for your comment on elastic, I'll use it in the new PR.

@lmolkova lmolkova closed this Apr 24, 2024
@lmolkova lmolkova mentioned this pull request Apr 26, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better guidance on semantic conventions for database client call span names in case of missing information
2 participants