Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify server.address and server.port in forwarded situations #1387

Open
dyladan opened this issue Sep 4, 2024 · 3 comments
Open

Clarify server.address and server.port in forwarded situations #1387

dyladan opened this issue Sep 4, 2024 · 3 comments

Comments

@dyladan
Copy link
Member

dyladan commented Sep 4, 2024

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
Copy link
Member Author

dyladan commented Sep 4, 2024

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
Copy link
Member Author

dyladan commented Sep 4, 2024

I would recommend either option 1 or option 5.

@trask
Copy link
Member

trask commented Sep 4, 2024

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

@joaopgrassi joaopgrassi removed their assignment Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants