Skip to content

Change server.address and server.port from required to recommended on HTTP Server Spans#114

Merged
reyang merged 3 commits into
open-telemetry:mainfrom
trask:change-server-address-port-to-recommended
Jun 21, 2023
Merged

Change server.address and server.port from required to recommended on HTTP Server Spans#114
reyang merged 3 commits into
open-telemetry:mainfrom
trask:change-server-address-port-to-recommended

Conversation

@trask

@trask trask commented Jun 15, 2023

Copy link
Copy Markdown
Member

Fixes #111

@trask trask requested review from a team June 15, 2023 02:46
@Oberon00

Copy link
Copy Markdown
Member

From the issue:

because you can typically get similar identification of the service from service.name

Citation needed. I think getting similar information from a resource attribute is fine, but is service.name really the right one? Maybe it should be host.name?
There should be a recommendation which (resource) attributes to set & form the URL from in case server.name is missing.

server.port will be even harder to get from resource attributes, the current conditional requirement seems fine?

Given the recent discussion, I think it is also fine to add an additional condition like "if available from a trusted source" (since the Host header cannot be trusted and a local configuration is sometimes not available)

@lmolkova

lmolkova commented Jun 15, 2023

Copy link
Copy Markdown
Member

Citation needed. I think getting similar information from a resource attribute is fine, but is service.name really the right one? Maybe it should be host.name?

it doesn't feel like HTTP concern (a set of resource attributes that describe host precisely). It's the same for all RPC calls and resource attribute definitions would be a good source of information. In practice, it can be a chain of fallbacks where host.name has the highest priority, service.name has the lowest one.

There could be more attributes added to resources in the future that can be even more precise, and HTTP spec should not need to be updated when it happens.

@trask

trask commented Jun 19, 2023

Copy link
Copy Markdown
Member Author

@open-telemetry/specs-semconv-approvers this could use additional reviews, as we are trying to freeze HTTP semantic conventions soon, thx!

@reyang reyang merged commit 2055854 into open-telemetry:main Jun 21, 2023
@trask trask deleted the change-server-address-port-to-recommended branch October 14, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should server.address and server.port be changed from required to recommended on SERVER spans?

6 participants