Database span name clarification and improvements#743
Database span name clarification and improvements#743lmolkova wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
| - ref: db.sql.table | ||
| tag: connection-level-tech-specific |
There was a problem hiding this comment.
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.
|
To the 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:
Based on the discussion from #704 - that probably could be changed to also fallback to 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. |
|
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. |
Fixes #704
Changes
db.statement(if low-cardinality) ->db.name db.statement(order matters becauseShopDb SELECT * FROM orders WHERE order_id = ?is better thanSELECT * 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)db.sql.tableto mssql - at least in java it's set for everything JDBC and makes sense on mssqldb.systemif nothing else is availableTo clarify:
SELECT * FROM foovssElEcT fROm FOO)Merge requirement checklist
[chore]