Replace confusing URI origin with more specific guidance#2463
Merged
lmolkova merged 5 commits intoopen-telemetry:mainfrom Jul 8, 2025
Merged
Replace confusing URI origin with more specific guidance#2463lmolkova merged 5 commits intoopen-telemetry:mainfrom
lmolkova merged 5 commits intoopen-telemetry:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes vague references to the RFC 9110 “URI origin” for server.address and server.port and replaces them with precise guidance on when to use the request-target host/port versus the Host/:authority header or IP address.
- Remove all “URI origin” brief descriptions for
server.addressandserver.portin YAML models. - Add detailed
notesections describing HTTP/1.1 absolute-form request-target and header-based fallbacks. - Update documentation tables and footnotes in
http-spans.mdandhttp-metrics.mdto match the new guidance.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| model/http/metrics.yaml | Replaced “URI origin” briefs with detailed notes for server.address and server.port. |
| model/http/common.yaml | Updated common attributes: removed brief, added notes for matching host/port semantics. |
| docs/http/http-spans.md | Updated table entries and footnotes [2]/[3] to remove “URI origin” and add specific host/port guidance. |
| docs/http/http-metrics.md | Replaced span and metric tables and footnotes with new wording for server.address and server.port. |
Comments suppressed due to low confidence (1)
model/http/metrics.yaml:197
- Typo: 'absolte-form' should be 'absolute-form'.
is passed in its [absolte-form](https://www.rfc-editor.org/rfc/rfc9112.html#section-3.2.2),
antonfirsov
reviewed
Jul 2, 2025
joaopgrassi
approved these changes
Jul 4, 2025
lmolkova
moved this from Untriaged
to Needs More Approval
in Semantic Conventions Triage
antonfirsov
reviewed
Jul 7, 2025
trask
approved these changes
Jul 8, 2025
Member
Author
|
@antonfirsov could you please take another look? |
antonfirsov
approved these changes
Jul 8, 2025
antonfirsov
left a comment
antonfirsov
left a comment
There was a problem hiding this comment.
LGTM, thanks a lot for addressing this!
spurplewang
pushed a commit
to spurplewang/semantic-conventions
that referenced
this pull request
Jul 10, 2025
antonfirsov
pushed a commit
to dotnet/runtime
that referenced
this pull request
Jul 18, 2025
#117540) With the merge of open-telemetry/semantic-conventions#2463, we are good to prioritize the contents of the `Host` header in cases no proxy is being used. The PR implements the change for both request and connection traces+metrics. There is a non-negligible risk: actually, we do not and (with the current code structure) can not emit `network.peer.address` for request telemetry, meaning that with `http(s)://x.x.x.x/..` target Uri-s, IP information will be no longer present when a there is a Host header. I believe that most users would still prefer to see the contents of the Host header if there is a mismatch. Others can opt into connection metrics/traces where `network.peer.address` is available.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #2443