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

[Http] network.protocol.version value in case of connection error/no response #520

Closed
vishweshbankwar opened this issue Nov 14, 2023 · 7 comments
Assignees

Comments

@vishweshbankwar
Copy link
Member

From Spec: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes

[5]: network.protocol.version refers to the version of the protocol used and might be different from the protocol client's version. If the HTTP client has a version of 0.27.2, but sends HTTP version 1.1, this attribute should be set to 1.1.

What should be behavior in case of requests that does not end up in a connection or response from server. Should instrumentations use the version specified by the client or skip setting the tag on Span/Metric?

@antonfirsov
Copy link

I would also note that the wording sounds confusing in its current form. What does a "protocol client's version" mean?

HTTP client has a version of 0.27.2

How can an HTTP client "have a version"?

@antonfirsov
Copy link

antonfirsov commented Nov 14, 2023

Another note:

requests that does not end up in a connection or response from server

Strictly speaking, these are two different cases. When a connection is available but the client fails to fetch a valid response (eg. because it's malformed), the protocol version is actually available. Nevertheless, implementations may not be able to report it -- which is the case with .NET because of the way things are layered internally.

@AlexanderWert
Copy link
Member

network.protocol.version has the requirement level Recommended. So, when the information is not available we should keep the attribute unset.

I would also note that the wording sounds confusing in its current form. What does a "protocol client's version" mean?

Agree! I think the intention here is to use the HTTP protocol version (when it is available) and NOT the HTTP client version (such as OkHttp client version in Java or so).

@joaopgrassi
Copy link
Member

Hi @antonfirsov , @AlexanderWert , @vishweshbankwar do we want to go back to this issue and address the wording there or are we good? It's not clear what should be done. Please let me know.

@antonfirsov
Copy link

antonfirsov commented Feb 29, 2024

There are two points:

  • As OP mentioned, it is not clear what version should be reported when there is no response, especially when no connection has been established. I think it makes sense to clarify this and in the latter case I would explicitly recommend to leave network.protocol.version empty, but others may disagree. Edit: the best would be to clarify that if there is a protocol negotiation (eg. ALPN) involved, and the negotiated protocol version can differ from the one preferred by the client, network.protocol.version should contain the negotiated protocol that is being actually used for communication. (When the connection has failed, it might be left empty given it's a Recommended property.) For context: .NET has a Version and a VersionPolicy proerty to request protcols, so a user may expect the requested Version to show up here when the connection failed and no protocol has been negotiated. Other clients like okhttp may use an array of protocols to declare to the server what versions can they use, making it practically impossible to log the requested version here. Related discussion in Discrepancy between OTel semantic conventions and built-in System.Net metrics dotnet/runtime#97395.
  • [7] uses a term "protocol client's version" which feels very confusing in this context, since it's not clear it refers to the version of the software component. (For context: in .NET there is a default protocol version associated with every HttpClient instance!) I don't see why is this even emphasized, the name network.protocol.version feels self-explanatory, I don't see why would anyone confuse it with sg like a software package version.

@joaopgrassi
Copy link
Member

Related to: #686

@lmolkova
Copy link
Contributor

I believe it's fixed in #817 which clarifies that the attribute captures actual (negotiated version) and should not be set otherwise.

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

5 participants