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

Prototype network.peer/local.* semconv in a more esoteric networking instrumentation #373

Closed
trask opened this issue Oct 10, 2023 · 5 comments
Assignees

Comments

@trask
Copy link
Member

trask commented Oct 10, 2023

Requested by @jsuereth in SemConv SIG meeting. @lmolkova proposed possibly using AMQP in Azure SDK.

@lmolkova
Copy link
Contributor

lmolkova commented Oct 11, 2023

Prototyped AMQP instrumentation in Java inside Azure SDK built on top of Apache Qpid Proton library.
Findings:

  • Java's SocketAddress and its implementations map well to network.* attributes.
  • Figuring out network.transport could be tricky as it needs to be deduced from multiple things:
    • the type of SocketAddress ( InetSocketAddress or UnixDomainSocketAddress). Specific libraries may have their own socket implementations (e.g. netty has DomainSocketAddress). Instrumentations need to know which implementations are available in their ecosystem. .NET has the same behavior where EndPoint implementation type is a marker of transport.
    • Java's UnixDomainSocketAddress was added in Java 16 which could create problems for instrumentations/libs that use earlier versions as a baseline
    • UDP runs on top of IP, to distinguish it from TCP, we need to look at SocketChannel implementation type (DatagramChannel presumably points to UDP). A similar problem exists on .NET.
  • Qpid library we use for AMQP stack does not really expose a socket address through public APIs (or it's not a trivial task). A bit of semi-public reflection is needed to access it. this is not a problem in Netty (which also supports different protocols and transports).

Sources:

The trace with AMQP instrumentation:
image

Conclusion:

  • Attribute mapping to Sockets APIs in Java (EndPoint API in .NET) is reasonable
  • Determining network.transport could be tricky
  • Individual libraries may not expose socket-level data on the public surface, so populating attributes cannot be required.

@Oberon00
Copy link
Member

Figuring out network.transport could be tricky as it needs to be deduced from multiple things:

This sounds like it could be done once generically and would then support all kinds of libraries? I wonder if we won't have the same problem for HTTP, since based on the URL you don't know if it uses some UDP-based HTTP/3. And I wonder what would even be semantically correct, isn't there some kind of switching going on?

@lmolkova
Copy link
Contributor

lmolkova commented Oct 11, 2023

This sounds like it could be done once generically and would then support all kinds of libraries?

It should be possible to create a reusable helper that'd work for the common case. It should work great in .NET where the ecosystem mostly uses types provided by the platform. It can be tricky in Java, Python, or JS - instrumentation for libraries like Netty would still need to do something custom because of custom implementations for sockets/channels in the library.

I wonder if we won't have the same problem for HTTP, since based on the URL you don't know if it uses some UDP-based HTTP/3. And I wonder what would even be semantically correct, isn't there some kind of switching going on?

Yep, this applies to HTTP as well - channels/sockets/other platform network low-level types are the same between different protocols.

It's still possible to determine if it's UDP, it just needs more work and knowing the implementation details. E.g. there are different DatagramChannel classes - one in java.nio.channels.DatagramChannel and another one in netty. It's easy to check against one and miss the other.

In any case, it seems it's not the problem of attribute definition, but the availability of this information.

One thing we can consider is to require network.transport presence for HTTP/3 if it's known for sure

I.e. change
"If not default (tcp for HTTP/1.1 and HTTP/2, udp for HTTP/3)."

to

"If not default (tcp for HTTP/1.1 and HTTP/2). MUST be set for HTTP/3 if it is known"

This way, a lack of protocol would mean a lack of information (and not default value). We should still be able to change the condition in the future if we'd have reliable ways to determine the protocol.

@Oberon00
Copy link
Member

It can be tricky in Java, Python, or JS

For Python specifically, I contributed a base http instrumentation component that can be used to enrich the HTTP spans of most (all?) libraries based on it with attributes captured from the raw socket. IIRC it works by cooperating with the actual HTTP instrumentations which set a context attribute telling that they request socket attributes. When the socket is then created, the library kicks in, sees the context attribute and enriches the currently active span. This approach is tricky, but could also work in other technologies whenever the library doesn't let you access the socket (attributes) you need.

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py

@trask
Copy link
Member Author

trask commented Oct 16, 2023

Closing, I believe the prototype achieved what we wanted and resulted in a few new issues/PRs for consideration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

4 participants