[transports/webrtc] Record total number of bytes sent / received in UDPMuxNewAddr

[melekes]

Description

so the last thing which stops me from releasing webrtc in Substrate (blockchain framework) is bandwidth calculation. The way it's commonly done for other libp2p transports is wrapping Transport with https://docs.rs/libp2p/latest/libp2p/bandwidth/struct.BandwidthLogging.html, which basically records number of bytes written / read across all streams.

BUT the problem with webrtc Transport is that a call to DataChannel::write reports "invalid" number of bytes.
by "invalid" I mean a call write([1,2,3]) will return n=3, but the actual number of bytes will be higher since [1,2,3] is wrapped into ChunkPayloadData

the correct amount is recorded by Association (https://github.com/webrtc-rs/webrtc/blob/cbfd3033c7605eff97988eef7257694fc3dcd717/sctp/src/association/mod.rs#L471)
but it's neither exposed nor present in stats.

One possible solution is to record total bytes sent / received in UDPMuxNewAddr, where the actual writing / reading from UDP socket happens. Then expose this info in ListenStream.

Motivation

Being able to accurately measure WebRTC connection's bandwidth.

Open questions

• check that it will be possible to return a trait instead of BandwidthSinks struct in Substrate

Are you planning to do it yourself in a pull request?

Yes

[melekes]

Also I don't think it's possible to wrap Transport which does multiplexing (webrtc / quic) with https://docs.rs/libp2p/latest/libp2p/bandwidth/struct.BandwidthLogging.html. Solution seems to be return it along with new (or add a new method new_with_bandwidth). BUT if webrtc returns Arc<BandwidthSinks> then how do I combine it with previous instance of BandwidthSinks coming from wrapped TCP transport.

Seems like we'll need a new type AndBandwidthSinks, which sums total_inbound/total_outbound of all underlying BandwidthSinks.

Refs paritytech/substrate#12529

[thomaseizinger]

Also I don't think it's possible to wrap Transport which does multiplexing (webrtc / quic) with docs.rs/libp2p/latest/libp2p/bandwidth/struct.BandwidthLogging.html.

Yes. Additionally, both of these are UDP-based which means we don't have a stream that we can wrap. All data that is sent is sent as datagrams.

I wish we would be able to do this but it requires specialization:

#[cfg(feature = "webrtc")]
impl Transport for BandwidthLogging<libp2p_webrtc::tokio::Transport> { }

[thomaseizinger]

I think I found a solution. See #3161.

[mxinden]

When tracking ingress and egress, one has to count on a specific level, e.g. on the IP layer, the transport layer, the application layer, ... I would boldly state that none of them are correct (as you can always drill deeper), but some are useful / intuitive.

#3161 and #3162 count at the Transport layer, i.e. bytes sent and received on a TCP or UDP socket. Though #3162 makes some exceptions:

    /// ICE binding requests or any other WebRTC control data transmitted via UDP socket is not
    /// logged. Only data flowing from / to connections are being logged.

https://github.com/libp2p/rust-libp2p/pull/3162/files#diff-6947105990e50585971da979428b8714411b27bac4f5e8f8ad93bd598f4c31c5R101-R108

With all of the above, I want to propose an alternative approach. How about counting at the application level? How about counting the goodput? In other words, how about counting at the stream layer?

Unless I am missing something, this would only require a StreamMuxer implementation, wrapping around existing transport stacks (e.g. quic, webrtc, tcp/noise/yamux). In my eyes, this would still be useful / intuitive, and it would be above all less intrusive, i.e. transparent to the various transports and muxers.

@melekes @thomaseizinger what do you think?

(@melekes I know this would be a mildly breaking change on the Substrate side. Though in my eyes, a change worth considering.)

[melekes]

With all of the above, I want to propose an alternative approach. How about counting at the application level? How about counting the goodput? In other words, how about counting at the stream layer?

From the perspective of libp2p user, I don't have a strong opinion. If counting only the goodput results in a good abstraction, I think we should go for it!

From the perspective of webrtc-rs dev, it would be good to measure / know the overhead of webrtc proto, but that's a different topic (and doesn't belong to this repo anyway).

[thomaseizinger]

I don't mind much where we attach the logging. If it works for users that we only log application traffic, then this will obviously make our life easier because we get away with a single abstraction.

[melekes]

@tomaka any thoughts on #3157 (comment)?

[tomaka]

It's what I've mentioned in another issue or PR: I think that building the bandwidth measurement on top of the StreamMuxer is better, as it's more "orthogonal" to the transport and also reusable for example for QUIC.

And from a point of view of libp2p user I don't care that much about missing some bytes. I would have a different opinion if the bandwidth measurement of TCP was exact, because that would make us switch from "exact measurement" to "approximate measurement", but as mentioned it's not.