Remove conditional requirement on network.peer.address and network.peer.port#449
Conversation
mateuszrzeszutek
left a comment
There was a problem hiding this comment.
Should we do the same thing for the server.port in HTTP server spans? Currently the spec says it's conditionally required:
If
server.addressis set and the port is not default (80 forhttpscheme,443for https).
Should we remove the "If server.address is set" part?
My understanding is that if we remove that part, then |
|
oh, maybe you're suggesting then that we (similarly) make
that seems to make sense to me |
I didn't mean that actually. |
ya, it's a good point do you think of adding similar condition on
|
This PR removed that part 😄 -- yes, I think reverting it back would resolve the confusion. |
👍 fixed |
|
@AlexanderWert, @lmolkova, @mateuszrzeszutek and @trisch-me - I reset all the approvals because I've made additional changes, ptal, thx |
I missed these in #342.
Based on #342 (comment):
we decided to remove the conditional and always populate these attributes when network-layer instrumentation exists.
Changes
Remove conditional requirement on
network.peer.addressandnetwork.peer.port.Merge requirement checklist
schema-next.yaml updated with changes to existing conventions.