Add instrumentation to track the IceEndpointType.

talk/app/webrtc/umametrics.h

 /*
  * libjingle
  * Copyright 2014 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 14 matching lines @@
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 // This file contains enums related to IPv4/IPv6 metrics.
 
 #ifndef TALK_APP_WEBRTC_UMAMETRICS_H_
 #define TALK_APP_WEBRTC_UMAMETRICS_H_
 
 namespace webrtc {
 
+// Used to specify which enum counter type we're incrementing in
+// MetricsObserverInterface::IncrementEnumCounter.
+enum PeerConnectionEnumCounterType {
+  kPeerConnectionEnumCounter_AddressFamily,
+  // For next 2 counters, we track them separately based on the first hop

[pthatcher1, 2015/08/18 18:06:22]

next 2 => the next 2

[pthatcher1, 2015/08/18 18:06:22]

What do you mean by "first hop".  I think you mean "the local candidate type in
the case of non-TURN candidates, and the protocol used to connect to the TURN
server in the case of TURN candidates".  But commenting it would be nice.

[guoweis_webrtc, 2015/08/18 22:33:57]

Done.

+  // protocol used by the local candidate.
+  kPeerConnectionEnumCounter_IceCandidatePairTypeUdp,
+  kPeerConnectionEnumCounter_IceCandidatePairTypeTcp,
+  kPeerConnectionEnumCounter_Max

[pthatcher1, 2015/08/18 18:06:22]

I'd prefer names without the underscores.  

[guoweis_webrtc, 2015/08/18 22:33:57]

Done.

+};
+
 // Currently this contains information related to WebRTC network/transport
 // information.
 
-// The difference between PeerConnectionMetricsCounter and
-// PeerConnectionMetricsName is that the "Counter" is only counting the
+// The difference between PeerConnectionEnumCounter and
+// PeerConnectionMetricsName is that the "EnumCounter" is only counting the
 // occurrences of events, while "Name" has a value associated with it which is
 // used to form a histogram.
 
 // This enum is backed by Chromium's histograms.xml,
 // chromium/src/tools/metrics/histograms/histograms.xml
 // Existing values cannot be re-ordered and new enums must be added
 // before kBoundary.
-enum PeerConnectionMetricsCounter {
+enum PeerConnectionAddressFamilyCounter {
   kPeerConnection_IPv4,
   kPeerConnection_IPv6,
   kBestConnections_IPv4,
   kBestConnections_IPv6,
-  kPeerConnectionMetricsCounter_Max,
+  kPeerConnectionAddressFamilyCounter_Max,
 };
 
 // TODO(guoweis): Keep previous name here until all references are renamed.
-#define kBoundary kPeerConnectionMetricsCounter_Max
+#define kBoundary kPeerConnectionAddressFamilyCounter_Max
 
 // TODO(guoweis): Keep previous name here until all references are renamed.
-typedef PeerConnectionMetricsCounter PeerConnectionUMAMetricsCounter;
+typedef PeerConnectionAddressFamilyCounter PeerConnectionUMAMetricsCounter;
 
 // This enum defines types for UMA samples, which will have a range.
 enum PeerConnectionMetricsName {
   kNetworkInterfaces_IPv4,  // Number of IPv4 interfaces.
   kNetworkInterfaces_IPv6,  // Number of IPv6 interfaces.
   kTimeToConnect,           // In milliseconds.
   kLocalCandidates_IPv4,    // Number of IPv4 local candidates.
   kLocalCandidates_IPv6,    // Number of IPv6 local candidates.
   kAudioSrtpCipher,         // Name of SRTP cipher used in audio channel.
   kAudioSslCipher,          // Name of SSL cipher used in audio channel.
   kVideoSrtpCipher,         // Name of SRTP cipher used in video channel.
   kVideoSslCipher,          // Name of SSL cipher used in video channel.
   kDataSrtpCipher,          // Name of SRTP cipher used in data channel.
   kDataSslCipher,           // Name of SSL cipher used in data channel.
   kPeerConnectionMetricsName_Max
 };
 
 // TODO(guoweis): Keep previous name here until all references are renamed.
 typedef PeerConnectionMetricsName PeerConnectionUMAMetricsName;
 
+// The IceCandidatePairType has the format of
+// <local_endpoint>_<remote_endpoint>. It

[pthatcher1, 2015/08/18 18:06:22]

I think you mean <local_candidate_type>_<remote_candidate_type>.

[guoweis_webrtc, 2015/08/18 22:33:57]

Done.

+// is recorded on the BestConnection when we have the first OnTransportCompleted
+// signaled.

[pthatcher1, 2015/08/18 18:06:22]

Might be more clear as "is recorded based on the type of candidate pair used
when the PeerConnection first goes to a connected state".  

Also, you should probably comment on what happens in the non-BUNDLE case.  If
you have separate connections for audio and video, which is recorded?  Both?

[guoweis_webrtc, 2015/08/18 22:33:57]

Done.

+enum IceCandidatePairType {
+  kIceCandidatePair_Host_Host,
+  kIceCandidatePair_Host_Srflx,
+  kIceCandidatePair_Host_Relay,
+  kIceCandidatePair_Host_Prflx,
+  kIceCandidatePair_Srflx_Host,
+  kIceCandidatePair_Srflx_Srflx,
+  kIceCandidatePair_Srflx_Relay,
+  kIceCandidatePair_Srflx_Prflx,
+  kIceCandidatePair_Relay_Host,
+  kIceCandidatePair_Relay_Srflx,
+  kIceCandidatePair_Relay_Relay,
+  kIceCandidatePair_Relay_Prflx,
+  kIceCandidatePair_Prflx_Host,
+  kIceCandidatePair_Prflx_Srflx,
+  kIceCandidatePair_Prflx_Relay,

[pthatcher1, 2015/08/18 18:06:22]

I'm not a fan of underscores here either.

[guoweis_webrtc, 2015/08/18 22:33:57]

Done.

+  kIceCandidatePair_Max
+};
+
 }  // namespace webrtc
 
 #endif  // TALK_APP_WEBRTC_UMA6METRICS_H_