Revise and extend semantic conventions for databases #575
Conversation
arminru
requested review from
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
jmacd,
reyang,
SergeyKanzhelev,
tedsuo,
tigrannajaryan and
yurishkuro
as code owners
|
@carlosalberto Thanks for the review! @open-telemetry/specs-approvers Anyone else willing to have a look? 🙂 |
| | `net.peer.port` | Defined in the general [network attributes][]. | See below | | ||
| | `net.transport` | Defined in the general [network attributes][]. | See below | | ||
|
|
||
| At least one of `net.peer.name` or `net.peer.ip` from the [network attributes][] is required and `net.peer.port` is recommended. |
There was a problem hiding this comment.
I don't think any of these can be required. Instrumentation may not know any of that.
There was a problem hiding this comment.
- Additionally at least one of `net.peer.name` or `net.peer.ip` from the [network attributes][] is required and `net.peer.port` is recommended.
+ At least one of `net.peer.name` or `net.peer.ip` from the [network attributes][] is required and `net.peer.port` is recommended.This requirement was already there before, I just added the attributes to the table so they're not overlooked that easily.
An instrumentation should be able to capture it, otherwise there will only be little value if the database is unknown. In which cases would it not know about that, given that we expect instrumentation to be built-in into the database client library?
| | `net.transport` | Defined in the general [network attributes][]. | See below | | ||
|
|
||
| At least one of `net.peer.name` or `net.peer.ip` from the [network attributes][] is required and `net.peer.port` is recommended. | ||
| If using a port other than the default port for this DBMS, `net.peer.port` is required. |
There was a problem hiding this comment.
Instrumentation may not know anything about ports.
There was a problem hiding this comment.
@yurishkuro see the response to your comment above #575 (comment)
| At least one of `net.peer.name` or `net.peer.ip` from the [network attributes][] is required and `net.peer.port` is recommended. | ||
| If using a port other than the default port for this DBMS, `net.peer.port` is required. | ||
| Furthermore, it is strongly recommended to add the [`net.transport`][] attribute and follow its guidelines. | ||
| For in-process databases, `net.transport` MUST be set to `"inproc"`. |
There was a problem hiding this comment.
knowledge of in-process vs. external DB my be abstracted away from instrumentation.
There was a problem hiding this comment.
@yurishkuro see the response to your comment above #575 (comment)
|
|
Co-Authored-By: Christian Neumüller <[email protected]>
… of well-known identifiers
| | `net.peer.port` | Defined in the general [network attributes][]. | See below | | ||
| | `net.transport` | Defined in the general [network attributes][]. | See below | | ||
|
|
||
| At least one of `net.peer.name` or `net.peer.ip` from the [network attributes][] is required and `net.peer.port` is recommended. |
Co-authored-by: Christian Neumüller <[email protected]>
|
@open-telemetry/specs-approvers @open-telemetry/specs-trace-approvers Any more reviews or are we good to merge this? People have had the chance to take a look for more than two months now. |
|
I think we are ready to merge this PR (we can always iterate on it later on, if we find we indeed need any refinement). I suggest waiting one day for @bogdandrutu and @yurishkuro to comment on this (as they had issues with this PR). And by EOD Tuesday merge otherwise. |
Release v0.6.0 of the opentelemetry spec included some changes to the database semantic conventions, among them being changes to some of the fields used to convert to Azure Dependency Targets etc. open-telemetry/opentelemetry-specification#575 This commit updates the exporter to handle the new names for these fields, however as the new equivalent to `db.instance` is `db.name`, which can change when the database is changed inside a connection.
Release v0.6.0 of the opentelemetry spec included some changes to the database semantic conventions, among them being changes to some of the fields used to convert to Azure Dependency Targets etc. open-telemetry/opentelemetry-specification#575 This commit updates the exporter to handle the new names for these fields, however as the new equivalent to `db.instance` is `db.name`, which can change when the database is changed inside a connection.
We revised and extended the semantic conventions for database client calls that were initially copied over from OpenTracing by making definitions more precise and adding technology-specific attributes.
This should lead to improved data quality and allow better identification of databases and analysis of calls made to them.
Resolves #245.