Remove db.instance.id

[trask]

Changes

Removes db.instance.id and reverts the Elasticsearch usage of it to db.elasticsearch.node.name.

Reason

In many database systems, the idea of db.instance.id corresponds to server.address or network.peer.address.

After adding db.namespace, we're currently only using db.instance.id in one place, and so I think it may be too early to try to add this generalized attribute.

Also, "instance id" is a potentially confusing name given the different meanings it has in SQL Server and Oracle.

[lmolkova]

I didn't find any usage of db.instance.id in existing instrumentation libraries in otel repos.

[trisch-me]

should we update schema as well?

[trask]

should we update schema as well?

Can you clarify what you had in mind? Thanks

[arielvalentin]

@trask @lmolkova I proposed this attribute and use it here.

https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/743796d425de3aa6c13fd89dffb3ad869106d261/instrumentation/trilogy/lib/opentelemetry/instrumentation/trilogy/patches/client.rb#L76

The reason why server.address or net.peer.name at the time did not work for us was discussed here: #345

I suppose we should follow the pattern that elasticsearch uses and have a separate name under db.mysql then?

would this be the node executing the query?

[17] network.peer.address: Semantic conventions for individual database systems SHOULD document whether network.peer.* attributes are applicable. Network peer address and port are useful when the application interacts with individual database nodes directly. If a database operation involved multiple network calls (for example retries), the address of the last contacted node SHOULD be used.

and this the logical hostname?

[18] server.address: When observed from the client side, and when communicating through an intermediary, server.address SHOULD represent the server address behind any intermediaries, for example proxies, if it’s available.

[lmolkova]

thanks for the context @arielvalentin !

I'd suggest using server.address for cluster address and network.peer.address for the individual node. You can provide either one or both of them together on one span. And that's the pattern we use for cassandra, rabbitmq, and some other clustered/self-hosted things). We should probably update mysql to mention network attributes too.

In case of mySQL I assume what you have there as @connected_host is the network address of the mysql node?

Looking at this comment #345 (comment) it seems it is

The configured hostname would appear as server.address: mysql-db.example.com vs node instance db.mysql.instance.address: mysql-e26b99z.example.com

And then network.peer.address is a good fit:

• server.address: mysql-db.example.com
• network.peer.address: mysql-e26b99z.example.com

You may populate either or both of them for single-db deployments, just in a consistent way.

I believe elastic uses db.elasticsearch.node.name because it's not an address.

PS: we don't always pay attention to comments on closed issues/PRs, so please don't hesitate to create new issues.

[arielvalentin]

Understood. We're very behind on semconv and haven't started work on introducing stability opt-in flags so I've been reluctant to start to migrate individual attributes over so we can avoid being in a more unspecified state than we already are.

I'll see what we can do next quarter to address stability migrations.