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

network: address (+domain) attributes have confusingly inconsistent meanings #206

Closed
Oberon00 opened this issue Jul 25, 2023 · 10 comments
Closed
Assignees

Comments

@Oberon00
Copy link
Member

Oberon00 commented Jul 25, 2023

It seems that server/client.socket.address are equivalent to source/destination.address while there is no attribute with equivalent meaning of server/client.address for source/destination (most closely seems to be source/destination.domain, which also exists under server/client.socket, but which seems to be defined solely for DNS names (why?), while server/client.address is more generic)

The *.address attributes:

  • client.socket.address: "Immediate client peer address - unix domain socket name, IPv4 or IPv6 address."
  • server.socket.address: "Physical server IP address or Unix socket address. If set from the client, should simply use the socket's peer address, and not attempt to find any actual server IP (i.e., if set from client, this may represent some proxy server instead of the logical server)." (much more elaborate than client -- why?)
  • client.address: "Client address - unix domain socket name, IPv4 or IPv6 address. [1]" "[1]: When observed from the server side, and when communicating through an intermediary, client.address SHOULD represent client address behind any intermediaries (e.g. proxies) if it's available."
  • server.address: "Logical server hostname, matches server FQDN if available, and IP or socket address if FQDN is not known." (why no note about proxies here?)
  • source.address: "Source address, for example IP address or Unix socket name."
  • destination.address: "Peer address, for example IP address or UNIX socket name." (why does it use the removed term "peer" here? That would mean something rather different, and would match our old remote/local concept)

The closely related *.domain attributes:

  • client.socket.domain seems to be missing?
  • server.socket.domain: "The domain name of an immediate peer. [1]" "[1]: Typically observed from the client side, and represents a proxy or other intermediary domain name."
  • source.domain: "The domain name of the source system. [1] [1]: This value may be a host name, a fully qualified domain name, or another host naming format."
  • destination.domain: "The domain name of the destination system. [1] [1]: This value may be a host name, a fully qualified domain name, or another host naming format."

This seems all rather messy to me, there is barely any any attribute pair with the symmetry I would have expected.

CC @lmolkova @jsuereth

I feel like this should be addressed before stabilization (though the HTTP-specific meaning is hopefully clear enough given the additional context there so I wouldn't say it's a blocker for stabilizing HTTP -- might want to add a note that details regarding proxy address semantics might change though)

@Oberon00 Oberon00 changed the title network: address attribute has confusingly inconsistent meaning in server & client vs. source & destination network: address (+domain) attributes have confusingly inconsistent meanings Jul 25, 2023
@lmolkova
Copy link
Contributor

@Oberon00 could you please propose specific changes you'd like to see?

@Oberon00
Copy link
Member Author

Oberon00 commented Jul 26, 2023

I mentioned some specific points I think are odd, and overall there is a lack of symmetry.

One concrete suggestion I can add: Move the current source/destination.address to source/destination.socket.address. Add a new source/destination.address that is equivalent to client/server.address.

Of course before that, client.address should be made symmetrical to server.address. (For example add to server.address: "When observed from the client side, and when communicating through an intermediary, server.address SHOULD represent server address behind any intermediaries (e.g. proxies) if it's available.", or remove the equivalent from client.addrress)

@trask
Copy link
Member

trask commented Aug 14, 2023

Of course before that, client.address should be made symmetrical to server.address. (For example add to server.address: "When observed from the client side, and when communicating through an intermediary, server.address SHOULD represent server address behind any intermediaries (e.g. proxies) if it's available.", or remove the equivalent from client.addrress)

this is done now (#244)

@trask
Copy link
Member

trask commented Aug 14, 2023

One concrete suggestion I can add: Move the current source/destination.address to source/destination.socket.address. Add a new source/destination.address that is equivalent to client/server.address.

I created #254 for this

@trask
Copy link
Member

trask commented Aug 14, 2023

The *.address attributes:

  • client.socket.address: "Immediate client peer address - unix domain socket name, IPv4 or IPv6 address."
  • server.socket.address: "Physical server IP address or Unix socket address. If set from the client, should simply use the socket's peer address, and not attempt to find any actual server IP (i.e., if set from client, this may represent some proxy server instead of the logical server)." (much more elaborate than client -- why?)
  • client.address: "Client address - unix domain socket name, IPv4 or IPv6 address. [1]" "[1]: When observed from the server side, and when communicating through an intermediary, client.address SHOULD represent client address behind any intermediaries (e.g. proxies) if it's available."
  • server.address: "Logical server hostname, matches server FQDN if available, and IP or socket address if FQDN is not known." (why no note about proxies here?)
  • source.address: "Source address, for example IP address or Unix socket name."
  • destination.address: "Peer address, for example IP address or UNIX socket name." (why does it use the removed term "peer" here? That would mean something rather different, and would match our old remote/local concept)

I think all of these are addressed now (by #243 and #244)

@trask
Copy link
Member

trask commented Aug 14, 2023

client.socket.domain seems to be missing?

I've opened #255 for this

@trask
Copy link
Member

trask commented Aug 14, 2023

@Oberon00 can you check that I've accurately captured your remaining concerns about client/server/source/destination in these issues (and we can close this issue)?

@trask
Copy link
Member

trask commented Aug 28, 2023

@Oberon00 it would be really helpful if you could take another pass and confirm that your concerns are covered by the 4 issues above, thx!

@trask trask moved this to Blocker for stability in Spec: HTTP Semantic Conventions Aug 28, 2023
@Oberon00
Copy link
Member Author

Hi, I will take a look by tomorrow at the same time

@Oberon00
Copy link
Member Author

Oberon00 commented Aug 29, 2023

Going over the conventions again I think indeed all the old points are addressed or captured in issues! Thank you @trask! I added two PRs for two of the open issues. I also found these points, but they probably don't justify keeping the issue open:

Feel free to close this issue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

4 participants