Adds getStats() method to QuicTransportBase.

[shampson]

This basically is just reusing what was implemented in the RTCQuicTransport origin trial. Feel free to suggest removing/editing certain stats. The main motivation for this PR is:

1. Get basic stats in the spec
2. Have a place to point for at least some basic congestion control information exposed in the API.

See issues:

• Stats, there got to be stats... p2p-webtransport#66 (Initial suggestion for P2P QUIC transport stats)
• Stats for congestion control and bandwidth estimation #21 (bandwidth prediction)

[shampson]

It seems I don't have contributor's access. Could someone help me out? The error says:
"shampson did not make IPR commitments for this group."

[shampson]

@pthatcherg @aboba PTAL. I don't have access to add you as reviewers unfortunately. Thanks!

[aboba]

The approach used in ORTC for handling per-object stats is a bit different. Using that approach, we would have something like this (could also be used if stats were desired for the IceTransport or QuicStream).

interface QuicTransport : RTCStatsProvider {
}

interface RTCStatsProvider {
Promise RTCStatsReport getStats();
};

dictionary RTCStats {
DOMHighResTimeStamp timestamp;
RTCStatsType type;
DOMString id;
};

New RTCStatsType Enums would be needed, such as "quic-transport" and "quic-stream".

dictionary QuicTransportStats : RTCStats {
unsigned long long bytesSent;
unsigned long long packetsSent;
unsigned long long streamBytesSent;
unsigned long long streamBytesReceived;
unsigned long numOutgoingStreamsCreated;
unsigned long numIncomingStreamsCreated;
unsigned long long bytesReceived;
unsigned long long packetsReceived;
unsigned long long packetsProcessed;
unsigned long long bytesRetransmitted;
unsigned long long packetsRetransmitted;
unsigned long long packetsLost;
unsigned long long packetsDropped;
unsigned long long cryptoRetransmitCount;
unsigned long long minRttUs;
unsigned long long smoothedRttUs;
unsigned short maxPacketSize;
unsigned short maxReceivedPacketSize;
unsigned long estimatedBandwidthBps;
unsigned long long packetsReordered;
unsigned long long blockedFramesReceived;
unsigned long long blockedFramesSent;
unsigned long long connectivityProbingPacketsReceived;
}

dictionary QuicStreamStats : RTCStats {
// Stream statistics go here
}

[shampson]

@aboba I would prefer not to use this approach for a few reasons:

• It's a much more complex model targeted at monolithic API surfaces (like PeerConnection) that include a lot of other sub-objects - streams, tracks, transports, senders, receivers, etc.
• It's a ORTC/WebRTC specific approach with lots of "RTC"
• I don't see a reason not to use the simple approach. What's wrong with the following code for getting stats on different objects?

const transport = new QuicTransport(host, port);
const stream = await transport.createSendStream();
const streamStats = await stream.getStats();
const transportStats = await transport.getStats();

I'm open to doing it the ORTC way but I'd like to better understand the benefits and why we would want to do this.

[aboba]

The major difference from what you are proposing is that in ORTC getStats() returns an RTCStatsReport and individual dictionaries (such as QuicTransportStats) inherits the timestamp, id and type from RTCStats. This allows the same type to be returned by getStats() called on any object, since statistics reports are self-identifying. This doesn't assume that there is an RTCPeerConnection - each object has its own getStats() method.

[shampson]

I was more calling out RTCPeerConnection as an example of an object where this stats model you’re suggesting makes more sense, because it has a bunch of underlying objects associated with it that you would also like to pull the stats from. We could use this model and remove the “RTC” stuff.

So the main advantage of the ORTC/WebRTC model is the self identification of the stats, that allows pulling the same stats from multiple objects. In our case that would mean you could get QUIC stream stats, ICE stats, as well as QUIC connection stats all from a QuicTranpsort.getStats() call.

My opinion is that this model is over complicated for the WebTransport/QuicTransport case, because what we’re trying to do is expose a lower level object. I think pulling the stats from the objects directly is sufficient.

Maybe @vr000m would like to weigh in.

The main goal of this PR is to get some initial stats we can reference for discussions in open issues like the one around congestion control. WDYT of landing it then continuing the discussion around the stats model in a separate issue?

[aboba]

In the ORTC model, each object has a getStats() method and calling this returns the statistics relevant to that object. However, the type may still be useful, in case multiple reports are to be returned for a given object, or the statistics are stored for later retrieval. As an example, in WebRTC-ICE the RTCIceTransport does not have a getStats method, but the ORTC RTCIceTransport does have such a method because it inherits from RTCStatsProvider.

[aboba]

@shampson Can you get nominated as a WICG member so your PRs can pass the IPR check?

[shampson]

@aboba Victor helped me resolve the issue, thank you. Thanks for looking over the PR @vasilvv. Let's decide on the minimum stats we want to add now. I made a suggestion in a comment. I'd also like to discuss as a group the need for the ORTC model for stats.

p.s. I snuck in the numReceivedDatagramsDropped stat for datagrams that are dropped, as discussed here: Issue 124- Alternatively using stats for dropped datagrams

[vr000m]

At the high level,

1. I do not want to traverse getStats to figure out the relationship, if that was explicit somewhere then the getStats could be called on the individual objects. That would avoid get back a mega object.
2. want to getStats through the lifecycle of the object.

cc: @lennart-csio

[shampson]

I created an issue for the stats API design. I think this PR is valuable to land without this discussion being resolved, so that we have a place to point to for other discussions like congestion control & datagram feedback.

I'd be in favor of landing the simple stats API now (with a subset of the current metrics), then following up in the issue. How does that sound?

[vr000m]

I am okay with the way forward as long as we are open to make breaking changes when we have that discussion

[shampson]

Breaking changes are fine to make right now and will be until the spec becomes more mature and is begins the process to ship. Development hasn't even begun for the WebTransport API in an origin trial. Since the RTCQuicTransport API shares the same base object, it is dependent on this as well before shipping.

I'd like to resolve this before we have a WebTransport origin trial, which is intended as a way to experiment with APIs that don't become baked into the browser.

[shampson]

@aboba are you okay landing this change, given the conversation about statistics API requirements has been moved out in to a separate issue?

If so, could you and @vasilvv please give this a look and see if the given statistics seems reasonable? Thanks!

[aboba]

@shampson Yes, am ok with landing this change and creating a new issue for API design.

[shampson]

@vasilvv would you mind looking over this again?

[vasilvv]

Looks good with exception of couple of things I left comments for inline.

[shampson]

Okay, I've resolved all outstanding comments and merged with the bikshed PR, so the change is in index.bs now. @vasilvv thanks a lot for updating the spec to bikeshed, the merge was clumsy, but it's so much easier to edit!

I don't have the ability to merge this, could an owner go ahead and land the PR? @pthatcherg @aboba
Thanks!

[pthatcherg]

Over what time window? The entire connection?

[shampson]

The entire connection. It looks like QUICHE does this for the entire time the connection has been open:
https://cs.chromium.org/chromium/src/net/third_party/quiche/src/quic/core/congestion_control/rtt_stats.cc?dr=CSs&g=0&l=43

[shampson]

I updated to "The minimum RTT observed on the entire connection." Leaving this comment open and leaving it to you to resolve or suggest something different. I think this is the most straight forward thing to do, but maybe in the future a given time window would be better.

[shampson]

Thanks for also giving this a look @pthatcherg