Skip to content

Clarify that network.protocol.version represents negotiated version#817

Merged
joaopgrassi merged 2 commits intoopen-telemetry:mainfrom
lmolkova:http-protocol-version-negotiated
Mar 25, 2024
Merged

Clarify that network.protocol.version represents negotiated version#817
joaopgrassi merged 2 commits intoopen-telemetry:mainfrom
lmolkova:http-protocol-version-negotiated

Conversation

@lmolkova
Copy link
Copy Markdown
Member

@lmolkova lmolkova commented Mar 14, 2024

Fixes #686

Changes

Clarifies that network.protocol.version represents negotiated version

Merge requirement checklist

@lmolkova lmolkova requested review from a team March 14, 2024 20:25
@lmolkova
Copy link
Copy Markdown
Member Author

lmolkova commented Mar 14, 2024

@antonfirsov could you please take a look?

Following up on your comment here #801 (comment) - .NET client metrics override the note already and this change would not affect them anyway.
I kept .NET note intact, but updated description in the tables though.

Copy link
Copy Markdown
Contributor

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine to me from the perspective the text seems clear. I'll defer to @antonfirsov and @JamesNK to ensure that the description matches what the implementation of HttpClient and Kestrel actually do.

@JamesNK
Copy link
Copy Markdown
Contributor

JamesNK commented Mar 15, 2024

Looks ok to me. @antonfirsov is the client expert.

@lmolkova
Copy link
Copy Markdown
Member Author

@antonfirsov friendly ping

Copy link
Copy Markdown

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this in the proposed form!

@lmolkova lmolkova force-pushed the http-protocol-version-negotiated branch from b243dd2 to 0220b1e Compare March 23, 2024 00:28
@joaopgrassi
Copy link
Copy Markdown
Member

joaopgrassi commented Mar 25, 2024

Should we get more approvals from @open-telemetry/semconv-http-approvers ?

Edit: All http approvers are already here :D

@lmolkova lmolkova force-pushed the http-protocol-version-negotiated branch from 0220b1e to b4c60ac Compare March 25, 2024 15:57
@joaopgrassi joaopgrassi merged commit 83119ef into open-telemetry:main Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Protocol negotiation and ambiguity of network.protocol.version

8 participants