open-telemetry / semantic-conventions

Defines standards for generating consistent, accessible telemetry across a variety of domains
Apache License 2.0
256 stars 165 forks source link

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

Closed trask closed 11 months ago

trask commented 12 months ago

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

lmolkova commented 12 months ago

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

Sources:

The trace with AMQP instrumentation:

image

Conclusion:

Oberon00 commented 12 months ago

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 commented 12 months ago

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 commented 12 months ago

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 commented 11 months ago

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