[HTTP Metrics] Rename server.socket.address to network.peer.address

[antonfirsov]

open-telemetry/semantic-conventions#342 is introducing changes to the OTel standard that will effect the conformance of our http.client.open_connections and http.client.connection.duration metrics. The PR is expected to be merged very soon giving us an opportunity to adjust our metrics for 8.0 GA and avoid breaking changes in 9.0.

• We should rename server.socket.address to network.peer.address in both metrics
• Given that it's recommended to set network.peer.port when network.peer.address is set, we should introduce that attribute. The implementation should be trivial, since we are flowing down the peer's IPEndPoint to HttpConnectionBase.

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs

Lines 53 to 59 in 3452a99

	_connectionMetrics = new ConnectionMetrics(
	metrics,
	protocol,
	pool.IsSecure ? "https" : "http",
	pool.OriginAuthority.HostValue,
	pool.IsDefaultPort ? null : pool.OriginAuthority.Port,
	remoteEndPoint?.Address?.ToString());

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

open-telemetry/semantic-conventions#342 is introducing changes to the OTel standard that will effect the conformance of attributes in our http.client.open_connections and http.client.connection.duration metrics. The PR is expected to be merged very soon giving us an opportunity to adjust our metrics and avoid breaking changes in 9.0.

• We should rename server.socket.address to network.peer.address in both metrics
• Given that it's recommended to set network.peer.port when network.peer.address is set, we should introduce that attribute. The implementation is trivial, since we are flowing down the peer's IPEndPoint to HttpConnectionBase.

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs

Lines 53 to 59 in 3452a99

	_connectionMetrics = new ConnectionMetrics(
	metrics,
	protocol,
	pool.IsSecure ? "https" : "http",
	pool.OriginAuthority.HostValue,
	pool.IsDefaultPort ? null : pool.OriginAuthority.Port,
	remoteEndPoint?.Address?.ToString());

Author:	antonfirsov
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	8.0.0

[antonfirsov]

@lmolkova please confirm if my understanding of the required changes is correct.

[antonfirsov]

After further consideration, I prefer to not implement network.peer.address:

• Always including it would lead to a boxing allocation every time we record a connection metric, which I assume we want to avoid.
• We could choose to only include it with non-default ports (just like we do it with server.port), but determining that requires non-trivial logic, and such an approach seems to be against the recommendation of the standard in its' current form. This may change with discussions (see Rename/replace (client|server).socket.(address|port) attributes with network.(peer|local).(address|port). open-telemetry/semantic-conventions#342 (comment)), but it is too late to consider adapting to such changes.
• The information is not of a critical value to users: when there is no proxy, network.peer.address should be equal to server.port. When there is a proxy, the port can be determined from sources other than metrics.
• It is very late in the release cycle, we should keep changes as simple and as low risk as possible.

[antonfirsov]

Fixed by #93414 in release/8.0, and by #93255 in main (9.0).