webrtc-pc icon indicating copy to clipboard operation
webrtc-pc copied to clipboard

Remove "MAY generate statistics that are not documented."

Open jan-ivar opened this issue 1 year ago • 2 comments

§ 8.6 Mandatory To Implement Stats normatively references stats that MUST be implemented for conformance with this spec.

However, the section ends by saying:

  1. "An implementation MAY support generating any other statistic defined in [WEBRTC-STATS],"
  2. "and MAY generate statistics that are not documented."

2 reads as the WG sanctioning undocumented APIs, which I think we should refrain from. On 1, I don't see a reason to limit documented extension to [WEBRTC-STATS]. AFAIK any spec may extend existing APIs unless the base spec explicitly forbids it.

I think the purpose here was to help clarify for readers where the conformance criteria end, which seems doable with a note. E.g. "Note: implementations might generate additional statistics beyond the ones above, as defined in other specifications such as [WEBRTC-STATS]."

jan-ivar avatar Mar 21 '23 21:03 jan-ivar

I'm very surprised the spec says "MAY generate statistics that are not documented", this is something I have fought valiantly against for years and seems to describe the legacy callback-based getStats() which I'm deprecating (grrrr!). Yes, please remove this sentence.

TL;DR: Generally speaking, undocumented metrics is not a thing anymore. But there are a couple of exceptions if you keep reading:

Thanks to recent WebIDL-ification of the RTCStatsReport we have an IDL overview of the stats and it appears that there are only find a handful of references to metrics that are not documented in webrtc-stats and these appear to exist in webrtc-provisional-stats, so they're still documented (with one exception):

  • contentType, because it references a header extension that I don't know is WG sanctioned.
  • rtcpTransportStatsId, I think this was moved from webrtc-stats to webrtc-provisional-stats due to not showing up in any WPTs. We did implement something, question is how you trigger an RTCP transport case, is it ever not missing? Implementation status unknown.
  • googTimingFrameInfo: This is the exception. You'll have to forgive me on this one, it's not in the spec, but it was in the legacy callback-based getStats() and was the only thing blocking migration, so I exposed it in the modern API as an unblocker (grrrr!). I don't like this metric at all. It also relies on some header extension that is probably not WG sanctioned.

There are also a couple of metrics that have been renamed or removed but the implementation still exposes them for backwards-compat (implemented prior to spec changes). These existed in the webrtc-stats spec at some point years ago, but not anymore. I think they fall into the category of "should be deprecated and removed", but for the time being they're technically undocumented. This includes:

  • mediaType, which was renamed kind (Chromium exposes both names of this metric).
  • ip, which was renamed address (both still exposed).
  • isRemote, which was removed from the spec because the type (local-candidate or remote-candidate) already exposes the same information.
  • writable, priority... I don't remember what these are.

henbos avatar Mar 22 '23 07:03 henbos

Other than the exceptions listed above, I consider webrtc-provisional-stats to mostly be a graveyard of unimplemented metrics. Most of which originate from webrtc-stats but moved due to lack of implementer interest.

henbos avatar Mar 22 '23 08:03 henbos