Add instrumentation to track the IceCandidatePairType and IceConnectionState

content/renderer/media/rtc_peer_connection_handler.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/rtc_peer_connection_handler.h"
 
 #include <string>
 #include <utility>
 #include <vector>
 
@@ skipping 487 matching lines @@
   if (!pc->GetStats(observer.get(), track.get(), level)) {
     DVLOG(1) << "GetStats failed.";
     observer->OnComplete(StatsReports());
   }
 }
 
 class PeerConnectionUMAObserver : public webrtc::UMAObserver {
  public:
   PeerConnectionUMAObserver() {}
   ~PeerConnectionUMAObserver() override {}
-
-  void IncrementCounter(
-      webrtc::PeerConnectionUMAMetricsCounter counter) override {
-    // Runs on libjingle's signaling thread.
-    UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.IPMetrics",
-                              counter,
-                              webrtc::kBoundary);
+  void IncrementEnumCounter(webrtc::PeerConnectionEnumCounterType counter_type,
+                            int counter,
+                            int counter_max) override {
+    switch (counter_type) {
+      case webrtc::kEnumCounterAddressFamily:
+        UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.IPMetrics", counter,
+                                  counter_max);

[tommi (sloooow) - chröme, 2015/08/19 19:29:25]

missing break?

[guoweis_left_chromium, 2015/08/19 19:37:29]

Thanks for catching this. Since this is based on an ongoing CL from webrtc and I
need to get them both into M46, I took shortcut to start this one and didn't
always try to compile this whenever the other one is updated.

Won't check this in until the WebRTC CL is rolled.

+      case webrtc::kEnumCounterIceEndpointTypeUdp:
+        UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.CandidatePairType_UDP",
+                                  counter, counter_max);
+        break;
+      case webrtc::kEnumCounterIceEndpointTypeTcp:
+        UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.CandidatePairType_TCP",
+                                  counter, counter_max);
+        break;
+      case webrtc::kPeerConnectionEnumCounterMax:

[tommi (sloooow) - chröme, 2015/08/19 19:29:26]

default: too?

[guoweis_left_chromium, 2015/08/19 19:37:29]

I purposedly removed the default so all enums will be listed here and if anyone
is missing in the future, it's a compiler error.

[tommi (sloooow) - chröme, 2015/08/19 19:46:18]

sgtm

+        NOTREACHED();
+        break;
+    }
   }
 
   void AddHistogramSample(webrtc::PeerConnectionUMAMetricsName type,
                           int value) override {
     // Runs on libjingle's signaling thread.
     switch (type) {
       case webrtc::kTimeToConnect:
         UMA_HISTOGRAM_MEDIUM_TIMES(
             "WebRTC.PeerConnection.TimeToConnect",
             base::TimeDelta::FromMilliseconds(value));
@@ skipping 196 matching lines @@
     blink::WebRTCPeerConnectionHandlerClient* client,
     PeerConnectionDependencyFactory* dependency_factory)
     : client_(client),
       dependency_factory_(dependency_factory),
       frame_(NULL),
       num_data_channels_created_(0),
       num_local_candidates_ipv4_(0),
       num_local_candidates_ipv6_(0),
       weak_factory_(this) {
   g_peer_connection_handlers.Get().insert(this);
+  ResetUMAStats();

[tommi (sloooow) - chröme, 2015/08/19 19:29:25]

might not need this if the array is initialized.  I'd prefer that actually.

[guoweis_left_chromium, 2015/08/19 19:37:29]

Done.

 }
 
 RTCPeerConnectionHandler::~RTCPeerConnectionHandler() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   stop();
 
   g_peer_connection_handlers.Get().erase(this);
   if (peer_connection_tracker_)
     peer_connection_tracker_->UnregisterPeerConnection(this);
@@ skipping 568 matching lines @@
   if (peer_connection_tracker_)
     peer_connection_tracker_->TrackSignalingStateChange(this, state);
   if (client_)
     client_->didChangeSignalingState(state);
 }
 
 // Called any time the IceConnectionState changes
 void RTCPeerConnectionHandler::OnIceConnectionChange(
     webrtc::PeerConnectionInterface::IceConnectionState new_state) {
   TRACE_EVENT0("webrtc", "RTCPeerConnectionHandler::OnIceConnectionChange");
+  ReportICEState(new_state);

[tommi (sloooow) - chröme, 2015/08/19 19:29:26]

move below the thread check

[guoweis_left_chromium, 2015/08/19 19:37:29]

Done.

   DCHECK(thread_checker_.CalledOnValidThread());
   if (new_state == webrtc::PeerConnectionInterface::kIceConnectionChecking) {
     ice_connection_checking_start_ = base::TimeTicks::Now();
   } else if (new_state ==
       webrtc::PeerConnectionInterface::kIceConnectionConnected) {
     // If the state becomes connected, send the time needed for PC to become
     // connected from checking to UMA. UMA data will help to know how much
     // time needed for PC to connect with remote peer.
     if (ice_connection_checking_start_.is_null()) {
       // From UMA, we have observed a large number of calls falling into the
@@ skipping 34 matching lines @@
 
     UMA_HISTOGRAM_COUNTS_100("WebRTC.PeerConnection.IPv4LocalCandidates",
                              num_local_candidates_ipv4_);
 
     UMA_HISTOGRAM_COUNTS_100("WebRTC.PeerConnection.IPv6LocalCandidates",
                              num_local_candidates_ipv6_);
   } else if (new_state ==
              webrtc::PeerConnectionInterface::kIceGatheringGathering) {
     // ICE restarts will change gathering state back to "gathering",
     // reset the counter.
-    num_local_candidates_ipv6_ = 0;
-    num_local_candidates_ipv4_ = 0;
+    ResetUMAStats();
   }
 
   blink::WebRTCPeerConnectionHandlerClient::ICEGatheringState state =
       GetWebKitIceGatheringState(new_state);
   if (peer_connection_tracker_)
     peer_connection_tracker_->TrackIceGatheringStateChange(this, state);
   if (client_)
     client_->didChangeICEGatheringState(state);
 }
 
@@ skipping 134 matching lines @@
   } else {
     base::WaitableEvent event(false, false);
     thread->PostTask(FROM_HERE,
         base::Bind(&RunSynchronousClosure, closure,
                    base::Unretained(trace_event_name),
                    base::Unretained(&event)));
     event.Wait();
   }
 }
 
+void RTCPeerConnectionHandler::ReportICEState(

[tommi (sloooow) - chröme, 2015/08/19 19:29:26]

add DCHECK(thread_checker_.CalledOnValidThread());

[guoweis_left_chromium, 2015/08/19 19:37:29]

Done.

+    webrtc::PeerConnectionInterface::IceConnectionState new_state) {
+  if (ice_state_seen_[new_state])
+    return;
+  ice_state_seen_[new_state] = true;
+  UMA_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.ConnectionState", new_state,
+                            webrtc::PeerConnectionInterface::kIceConnectionMax);
+}
+
+void RTCPeerConnectionHandler::ResetUMAStats() {

[tommi (sloooow) - chröme, 2015/08/19 19:29:26]

add DCHECK(thread_checker_.CalledOnValidThread());

[guoweis_left_chromium, 2015/08/19 19:37:29]

Done.

+  num_local_candidates_ipv6_ = 0;
+  num_local_candidates_ipv4_ = 0;
+  ice_connection_checking_start_ = base::TimeTicks();
+  memset(ice_state_seen_, 0, sizeof(ice_state_seen_));
+}
 }  // namespace content