RTCTransportStats added.

webrtc/api/rtcstatscollector.cc

Index: webrtc/api/rtcstatscollector.cc
diff --git a/webrtc/api/rtcstatscollector.cc b/webrtc/api/rtcstatscollector.cc
index 5f60ea13cecadb7defc5169701e06a2d7308dc26..2bd446e7f97647f7d7f22dbf537ad78cccf56c4f 100644
--- a/webrtc/api/rtcstatscollector.cc
+++ b/webrtc/api/rtcstatscollector.cc
@@ -17,12 +17,24 @@
 #include "webrtc/api/peerconnection.h"
 #include "webrtc/api/webrtcsession.h"
 #include "webrtc/base/checks.h"
-#include "webrtc/base/sslidentity.h"
 #include "webrtc/p2p/base/candidate.h"
 #include "webrtc/p2p/base/port.h"
 
 namespace webrtc {
 
+namespace {
+
+std::string RTCCertificateID(const std::string& fingerprint) {

[Taylor Brandstetter, 2016/10/17 21:19:03]

nit: "RTCCertificateID" sounds more like a class name than a function name, to
me.

[hbos, 2016/10/19 16:30:00]

Done.

+  return "RTCCertificate_" + fingerprint;
+}
+
+std::string RTCIceCandidatePairStatsID(const cricket::ConnectionInfo& info) {
+  return "RTCIceCandidatePair_" + info.local_candidate.id() + "_" +
+      info.remote_candidate.id();
+}
+
+}  // namespace
+
 const char* CandidateTypeToRTCIceCandidateType(const std::string& type) {
   if (type == cricket::LOCAL_PORT_TYPE)
     return RTCIceCandidateType::kHost;
@@ -107,9 +119,15 @@ void RTCStatsCollector::ProducePartialResultsOnSignalingThread(
 
   SessionStats session_stats;
   if (pc_->session()->GetTransportStats(&session_stats)) {
-    ProduceCertificateStats_s(timestamp_us, session_stats, report.get());
-    ProduceIceCandidateAndPairStats_s(timestamp_us, session_stats,
-                                      report.get());
+    std::map<std::string, CertificateStatsPair> certificate_stats_map =
+        PrepareCertificateStatsMap_s(session_stats);
+
+    ProduceCertificateStats_s(
+        timestamp_us, session_stats, certificate_stats_map, report.get());
+    ProduceIceCandidateAndPairStats_s(
+        timestamp_us, session_stats, report.get());
+    ProduceTransportStats_s(
+        timestamp_us, session_stats, certificate_stats_map, report.get());
   }
   ProducePeerConnectionStats_s(timestamp_us, report.get());
 
@@ -178,43 +196,41 @@ void RTCStatsCollector::DeliverCachedReport() {
 
 void RTCStatsCollector::ProduceCertificateStats_s(
     int64_t timestamp_us, const SessionStats& session_stats,
+    const std::map<std::string, CertificateStatsPair>& certificate_stats_map,
     RTCStatsReport* report) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
+  std::map<std::string, CertificateStatsPair> transport_certificates;
   for (const auto& transport_stats : session_stats.transport_stats) {

[Taylor Brandstetter, 2016/10/17 21:19:03]

nit: Instead of iterating the transport stats and finding certificate stats with
a matching name, would it be possible to directly iterate certificate_stats_map?

[hbos, 2016/10/19 16:30:00]

Oh yeah, done.

-    rtc::scoped_refptr<rtc::RTCCertificate> local_certificate;
-    if (pc_->session()->GetLocalCertificate(
-        transport_stats.second.transport_name, &local_certificate)) {
-      ProduceCertificateStatsFromSSLCertificateAndChain_s(
-          timestamp_us, local_certificate->ssl_certificate(), report);
+    const auto& certificate_stats_it = certificate_stats_map.find(
+        transport_stats.second.transport_name);
+    RTC_DCHECK(certificate_stats_it != certificate_stats_map.cend());
+    if (certificate_stats_it->second.local) {
+      ProduceCertificateStatsFromSSLCertificateStats_s(
+          timestamp_us, *certificate_stats_it->second.local.get(), report);
     }
-    std::unique_ptr<rtc::SSLCertificate> remote_certificate =
-        pc_->session()->GetRemoteSSLCertificate(
-            transport_stats.second.transport_name);
-    if (remote_certificate) {
-      ProduceCertificateStatsFromSSLCertificateAndChain_s(
-          timestamp_us, *remote_certificate.get(), report);
+    if (certificate_stats_it->second.remote) {
+      ProduceCertificateStatsFromSSLCertificateStats_s(
+          timestamp_us, *certificate_stats_it->second.remote.get(), report);
     }
   }
 }
 
-void RTCStatsCollector::ProduceCertificateStatsFromSSLCertificateAndChain_s(
-    int64_t timestamp_us, const rtc::SSLCertificate& certificate,
+void RTCStatsCollector::ProduceCertificateStatsFromSSLCertificateStats_s(
+    int64_t timestamp_us, const rtc::SSLCertificateStats& certificate_stats,
     RTCStatsReport* report) const {
   RTC_DCHECK(signaling_thread_->IsCurrent());
-  std::unique_ptr<rtc::SSLCertificateStats> ssl_stats =
-      certificate.GetStats();
-  RTCCertificateStats* prev_stats = nullptr;
-  for (rtc::SSLCertificateStats* s = ssl_stats.get(); s;
+  RTCCertificateStats* prev_certificate_stats = nullptr;
+  for (const rtc::SSLCertificateStats* s = &certificate_stats; s;
        s = s->issuer.get()) {
-    RTCCertificateStats* stats = new RTCCertificateStats(
-        "RTCCertificate_" + s->fingerprint, timestamp_us);
-    stats->fingerprint = s->fingerprint;
-    stats->fingerprint_algorithm = s->fingerprint_algorithm;
-    stats->base64_certificate = s->base64_certificate;
-    if (prev_stats)
-      prev_stats->issuer_certificate_id = stats->id();
-    report->AddStats(std::unique_ptr<RTCCertificateStats>(stats));
-    prev_stats = stats;
+    RTCCertificateStats* certificate_stats = new RTCCertificateStats(
+        RTCCertificateID(s->fingerprint), timestamp_us);
+    certificate_stats->fingerprint = s->fingerprint;
+    certificate_stats->fingerprint_algorithm = s->fingerprint_algorithm;
+    certificate_stats->base64_certificate = s->base64_certificate;
+    if (prev_certificate_stats)
+      prev_certificate_stats->issuer_certificate_id = certificate_stats->id();
+    report->AddStats(std::unique_ptr<RTCCertificateStats>(certificate_stats));
+    prev_certificate_stats = certificate_stats;
   }
 }
 
@@ -226,10 +242,9 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_s(
     for (const auto& channel_stats : transport_stats.second.channel_stats) {
       for (const cricket::ConnectionInfo& info :
            channel_stats.connection_infos) {
-        const std::string& id = "RTCIceCandidatePair_" +
-            info.local_candidate.id() + "_" + info.remote_candidate.id();
         std::unique_ptr<RTCIceCandidatePairStats> candidate_pair_stats(
-            new RTCIceCandidatePairStats(id, timestamp_us));
+            new RTCIceCandidatePairStats(RTCIceCandidatePairStatsID(info),
+                                         timestamp_us));
 
         // TODO(hbos): Set all of the |RTCIceCandidatePairStats|'s members,
         // crbug.com/633550.
@@ -337,4 +352,71 @@ void RTCStatsCollector::ProducePeerConnectionStats_s(
   report->AddStats(std::move(stats));
 }
 
+void RTCStatsCollector::ProduceTransportStats_s(
+    int64_t timestamp_us, const SessionStats& session_stats,
+    const std::map<std::string, CertificateStatsPair>& certificate_stats_map,
+    RTCStatsReport* report) const {
+  RTC_DCHECK(signaling_thread_->IsCurrent());
+  for (const auto& transport : session_stats.transport_stats) {

[Taylor Brandstetter, 2016/10/17 21:19:03]

Actually, if I understand the spec correctly, there should be a "transport
stats" for each "channel" (for our definition of channel). The ID would be a
combination of the transport name and component (RTP or RTCP).

I realize this is confusing based on the wording in the spec, so I filed an
issue: https://github.com/w3c/webrtc-stats/issues/72

[hbos, 2016/10/19 16:30:00]

Thanks. Code updated.

If this is the case then RTCTransportStats for both the RTP and RTCP case have
the same local and remote certificates.
Is it the case that, for each transport, we always have exactly 1
RTCTransportStats for the RTP case, and 0 or 1 RTCTransportStats for the RTCP
case? Or could you have 0, 1 or more for RTP?

[Taylor Brandstetter, 2016/10/19 19:03:06]

You will always have 1 for RTP, and 0 or 1 for RTCP (depending on whether or not
RTCP mux was negotiated, causing RTP and RTCP to use the same transport).

+    std::unique_ptr<RTCTransportStats> transport_stats(
+        new RTCTransportStats("RTCTransport_" + transport.second.transport_name,
+                              timestamp_us));
+    transport_stats->bytes_sent = 0;
+    transport_stats->bytes_received = 0;
+    transport_stats->active_connection = false;
+    for (const auto& channel_stats : transport.second.channel_stats) {
+      for (const cricket::ConnectionInfo& info :
+           channel_stats.connection_infos) {
+        *transport_stats->bytes_sent += info.sent_total_bytes;
+        *transport_stats->bytes_received += info.recv_total_bytes;
+        if (info.best_connection) {
+          transport_stats->active_connection = true;
+          transport_stats->selected_candidate_pair_id =
+              RTCIceCandidatePairStatsID(info);
+        }
+      }
+    }
+    // TODO(hbos): Set transport_stats->rtcp_transport_stats_id.
+    // crbug.com/653873

[Taylor Brandstetter, 2016/10/17 21:19:03]

There's no real reason why this couldn't be done in the current CL. You could do
something like the following:

// Get reference to RTCP channel, if it exists.
auto rtcp_channel = std::find_if(channel_stats.begin(), channel_stats.end(),
  [] (const TransportChannelStats& ch) {
    return ch.component == COMPONENT_RTCP;
  });

// Later, while iterating channels...

// If there are separate RTP and RTCP channels (due to RTCP mux being
// disabled), link the two via rtcp_transport_stats_id.
if (channel.component == COMPONENT_RTP && rtcp_channel != channel_stats.end()) {
  transport_stats->rtcp_transport_stats_id =
MakeRTCTransportStatsID(*rtcp_channel);
}

[hbos, 2016/10/19 16:30:00]

Done.

+    const auto& certificate_stats_it = certificate_stats_map.find(
+        transport.second.transport_name);
+    RTC_DCHECK(certificate_stats_it != certificate_stats_map.cend());
+    if (certificate_stats_it->second.local) {
+      transport_stats->local_certificate_id =
+          RTCCertificateID(certificate_stats_it->second.local->fingerprint);
+    }
+    if (certificate_stats_it->second.remote) {
+      transport_stats->remote_certificate_id =
+          RTCCertificateID(certificate_stats_it->second.remote->fingerprint);
+    }
+    report->AddStats(std::move(transport_stats));
+  }
+}
+
+std::map<std::string, RTCStatsCollector::CertificateStatsPair>
+RTCStatsCollector::PrepareCertificateStatsMap_s(
+    const SessionStats& session_stats) const {
+  RTC_DCHECK(signaling_thread_->IsCurrent());
+  std::map<std::string, CertificateStatsPair> transport_certificates;
+  for (const auto& transport_stats : session_stats.transport_stats) {
+    CertificateStatsPair certificate_stats_pair;
+    rtc::scoped_refptr<rtc::RTCCertificate> local_certificate;
+    if (pc_->session()->GetLocalCertificate(
+        transport_stats.second.transport_name, &local_certificate)) {
+      certificate_stats_pair.local =
+          local_certificate->ssl_certificate().GetStats();
+    }
+    std::unique_ptr<rtc::SSLCertificate> remote_certificate =
+        pc_->session()->GetRemoteSSLCertificate(
+            transport_stats.second.transport_name);
+    if (remote_certificate) {
+      certificate_stats_pair.remote = remote_certificate->GetStats();
+    }
+    transport_certificates.insert(
+        std::make_pair(transport_stats.second.transport_name,
+                       std::move(certificate_stats_pair)));
+  }
+  return transport_certificates;
+}
+
 }  // namespace webrtc