Clarify `server.address` and `server.port` in forwarded situations

[dyladan]

Area(s)

area:server

Is your change request related to a problem? Please describe.

In https://github.com/open-telemetry/semantic-conventions/blob/bf0a2c1134f206f034408b201dbec37960ed60ec/docs/http/http-spans.md#clientserver-example-with-reverse-proxy the forwarded header may not contain enough information to determine the port if it contains host but not proto. The http semconv states that port is required if address exists.

Describe the solution you'd like

Please clarify what to do in this situation:

1. Leave port undefined
2. Do not set port or address
3. Set port to a default value like 80 or 443
4. look for port in further fallback options like x-forwarded-host or host headers, or use port of proxied request
5. Capture the full Forwarded header
6. Something else

Describe alternatives you've considered

No response

Additional context

While the Forwarded should contain enough information to determine the port, it may not in every situation. In these types of unexpected situations, telemetry is more important than ever.

[dyladan]

The same clarification would be needed if x-forwarded-host exists but x-forwarded-proto does not, and x-forwarded-host does not contain port information.

[dyladan]

I would recommend either option 1 or option 5.

[trask]

my initial thought is option 1

IIRC the purpose of making the port required when address is present was just to avoid the "default value" problem (does missing port imply default or imply it just wasn't captured)

in this case, a missing port value really would mean that it just couldn't be collected for some reason

[dyladan]

@trask I guess I can just close this? Or do you think there should be some clarification in the semconv?

[trask]

I think it probably does require clarification, especially since that guidance would go against current spec "Conditionally Required if server.address is set."

I've added this to Monday's semconv SIG agenda to make sure it's ok for us to relax this.

[trask]

Discussed in SIG and consensus was that this is not a breaking change to relax this, since it does not affect metrics (server.port is opt-in on metrics) and previously instrumentation was unable to send this telemetry in the first place, and so it is net new span telemetry.