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

DB/messaging/rpc(?): should logical operations include network.* attributes #690

Closed
lmolkova opened this issue Feb 5, 2024 · 2 comments
Closed

Comments

@lmolkova
Copy link
Contributor

lmolkova commented Feb 5, 2024

DB, messaging (maybe rpc and other conventions) arguably define logical operations.

E.g. database call could be a complex operation involving multiple underlying network calls (auth, retries, check-if-exists, etc).

  • Assuming the underlying protocol is instrumented, it introduces duplication and confusion

  • Some systems can operate on top of different interchangeable protocols and network information can be missing/too verbose/irrelevant:

    • GCP frequently operates on HTTP and gRPC
    • Azure messaging clients use AMQP, but to some extent can operate on top of HTTP
    • ORM systems don't even have to know the network details of underlying DB driver.
  • Retries are ambiguous

    • retries can have different network.peer|local.address|port if they reach different nodes of the service
  • Network calls within one logical call can use different protocols, e.g. in case of Azure CosmosDB:

    • Auth can be HTTP
    • Some other rare operations are HTTP
    • Things on the hot path are done over proprietary TCP-based protocol

Proposal:

Remove network.* attributes from generic db/messaging logical operations:

  • this would make network.* attributes opt-in
  • logical operations should still use 'logical' attributes such as server.address|port
  • individual DB/messaging extensions can still reference/require/recommend network.* when it makes sense for them

Related: #674 (extracting db/messaging blocking part from that issue)

@pyohannes
Copy link
Contributor

For messaging, this would affect the following attributes:

Attribute Example Requirement Level
network.peer.address 10.1.2.80; /tmp/my.sock Recommended
network.peer.port 65123 Recommended: If network.peer.address is set.
network.protocol.name amqp; mqtt Conditionally Required: [15]
network.protocol.version 3.1.1 Recommended
network.transport tcp; udp Recommended
network.type ipv4; ipv6 Recommended

It makes sense to make all the recommended ones Opt-In. For network.protocol.name, we currently have it as conditionally required for messaging systems that support more than one protocol.

However, we could remove this requirement from generic semantic conventions and add it to system-specific conventions where it makes sense. This would save us one more footnote ... 😄

@lmolkova
Copy link
Contributor Author

lmolkova commented Apr 4, 2024

Fixed in #768 and #780

@lmolkova lmolkova closed this as completed Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: V1 - Stable Semantics
Development

No branches or pull requests

3 participants