Change WebRTC SslCipher to be exposed as number only.

talk/app/webrtc/webrtcsession.cc

Index: talk/app/webrtc/webrtcsession.cc
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index 0c0e44d0e07b5dd6a6325cc4aa4be812a765cd68..d999811741a6c5ca631a0254d72c96a10d9221f5 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -88,6 +88,99 @@ const char kDtlsSetupFailureRtcp[] =
     "Couldn't set up DTLS-SRTP on RTCP channel.";
 const int kMaxUnsignalledRecvStreams = 20;
 
+SrtpCipherType GetSrtpCipherType(const std::string& cipher) {
+  if (cipher == "AES_CM_128_HMAC_SHA1_32")
+    return SrtpCipherType_AES_CM_128_HMAC_SHA1_32;
+  if (cipher == "AES_CM_128_HMAC_SHA1_80")
+    return SrtpCipherType_AES_CM_128_HMAC_SHA1_80;

[Ryan Sleevi, 2015/09/22 21:42:15]

For what it's worth, this registration is also covered by IANA (
https://www.iana.org/assignments/sdp-security-descriptions/sdp-security-descr...
), but since SDP uses string identifiers, I suppose you can't escape needing to
map from string -> histogram value.

[guoweis_webrtc, 2015/09/23 06:46:16]

Even though these are also defined in IANA, do you know why SRTP Crypto doesn't
have value defined as TLS cipher? 

+  return SrtpCipherType_Unknown;
+}
+
+SslCipherType GetSslCipherType(const std::string& cipher) {
+  // TLS v1.0 ciphersuites from RFC2246.
+  if (cipher == "TLS_RSA_RC4_128_SHA")

[Ryan Sleevi, 2015/09/22 21:42:15]

As mentioned by davidben@, you should not design your interface to convert from
uint->string->enum->uint

Each of these ciphersuites has an IANA-assigned value (
http://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-param...
), so you can design your interface such that you expose this uint16_t value
from the socket, expose it as a *number* to the stats channel, and then
histogram that numeric value via the metrics observer.

+    return SslCipherType_TLS_RSA_RC4_128_SHA;
+  if (cipher == "TLS_RSA_WITH_3DES_EDE_CBC_SHA")
+    return SslCipherType_TLS_RSA_WITH_3DES_EDE_CBC_SHA;
+
+  // AES ciphersuites from RFC3268.
+  if (cipher == "TLS_RSA_WITH_AES_128_CBC_SHA")
+    return SslCipherType_TLS_RSA_WITH_AES_128_CBC_SHA;
+  if (cipher == "TLS_DHE_RSA_WITH_AES_128_CBC_SHA")
+    return SslCipherType_TLS_DHE_RSA_WITH_AES_128_CBC_SHA;
+  if (cipher == "TLS_RSA_WITH_AES_256_CBC_SHA")
+    return SslCipherType_TLS_RSA_WITH_AES_256_CBC_SHA;
+  if (cipher == "TLS_DHE_RSA_WITH_AES_256_CBC_SHA")
+    return SslCipherType_TLS_DHE_RSA_WITH_AES_256_CBC_SHA;
+
+  // ECC ciphersuites from RFC4492.
+  if (cipher == "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA")
+    return SslCipherType_TLS_ECDHE_ECDSA_WITH_RC4_128_SHA;
+  if (cipher == "TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA")
+    return SslCipherType_TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA;
+  if (cipher == "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA")
+    return SslCipherType_TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA;
+  if (cipher == "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA")
+    return SslCipherType_TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA;
+
+  if (cipher == "TLS_ECDHE_RSA_WITH_RC4_128_SHA")
+    return SslCipherType_TLS_ECDHE_RSA_WITH_RC4_128_SHA;
+  if (cipher == "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA")
+    return SslCipherType_TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA;
+  if (cipher == "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA")
+    return SslCipherType_TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA;
+  if (cipher == "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA")
+    return SslCipherType_TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA;
+
+  // TLS v1.2 ciphersuites.
+  if (cipher == "TLS_RSA_WITH_AES_128_CBC_SHA256")
+    return SslCipherType_TLS_RSA_WITH_AES_128_CBC_SHA256;
+  if (cipher == "TLS_RSA_WITH_AES_256_CBC_SHA256")
+    return SslCipherType_TLS_RSA_WITH_AES_256_CBC_SHA256;
+  if (cipher == "TLS_DHE_RSA_WITH_AES_128_CBC_SHA256")
+    return SslCipherType_TLS_DHE_RSA_WITH_AES_128_CBC_SHA256;
+  if (cipher == "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256")
+    return SslCipherType_TLS_DHE_RSA_WITH_AES_256_CBC_SHA256;
+
+  // TLS v1.2 GCM ciphersuites from RFC5288.
+  if (cipher == "TLS_RSA_WITH_AES_128_GCM_SHA256")
+    return SslCipherType_TLS_RSA_WITH_AES_128_GCM_SHA256;
+  if (cipher == "TLS_RSA_WITH_AES_256_GCM_SHA384")
+    return SslCipherType_TLS_RSA_WITH_AES_256_GCM_SHA384;
+  if (cipher == "TLS_DHE_RSA_WITH_AES_128_GCM_SHA256")
+    return SslCipherType_TLS_DHE_RSA_WITH_AES_128_GCM_SHA256;
+  if (cipher == "TLS_DHE_RSA_WITH_AES_256_GCM_SHA384")
+    return SslCipherType_TLS_DHE_RSA_WITH_AES_256_GCM_SHA384;
+  if (cipher == "TLS_DH_RSA_WITH_AES_128_GCM_SHA256")
+    return SslCipherType_TLS_DH_RSA_WITH_AES_128_GCM_SHA256;
+  if (cipher == "TLS_DH_RSA_WITH_AES_256_GCM_SHA384")
+    return SslCipherType_TLS_DH_RSA_WITH_AES_256_GCM_SHA384;
+
+  // ECDH HMAC based ciphersuites from RFC5289.
+  if (cipher == "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256")
+    return SslCipherType_TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256;
+  if (cipher == "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384")
+    return SslCipherType_TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384;
+  if (cipher == "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256")
+    return SslCipherType_TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256;
+  if (cipher == "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384")
+    return SslCipherType_TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384;
+
+  // ECDH GCM based ciphersuites from RFC5289.
+  if (cipher == "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256")
+    return SslCipherType_TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256;
+  if (cipher == "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384")
+    return SslCipherType_TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384;
+  if (cipher == "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256")
+    return SslCipherType_TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256;
+  if (cipher == "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384")
+    return SslCipherType_TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384;
+  if (cipher == "TLS_RSA_RC4_128_SHA")
+    return SslCipherType_TLS_RSA_RC4_128_SHA;
+
+  return SslCipherType_Unknown;
+}
+
 IceCandidatePairType GetIceCandidatePairCounter(
     const cricket::Candidate& local,
     const cricket::Candidate& remote) {
@@ -2054,6 +2147,9 @@ void WebRtcSession::ReportBestConnectionState(
   }
 }
 
+// Since this is reported in the first OnTransportCompleted, in the non-bundled
+// case, we might not be reporting anything other than the first completed
+// channel.
 void WebRtcSession::ReportNegotiatedCiphers(
     const cricket::TransportStats& stats) {
   RTC_DCHECK(metrics_observer_ != NULL);
@@ -2067,27 +2163,29 @@ void WebRtcSession::ReportNegotiatedCiphers(
     return;
   }
 
-  PeerConnectionMetricsName srtp_name;
-  PeerConnectionMetricsName ssl_name;
+  PeerConnectionEnumCounterType srtp_counter_type;
+  PeerConnectionEnumCounterType ssl_counter_type;
   if (stats.content_name == cricket::CN_AUDIO) {
-    srtp_name = kAudioSrtpCipher;
-    ssl_name = kAudioSslCipher;
+    srtp_counter_type = kEnumCounterAudioSrtpCipher;
+    ssl_counter_type = kEnumCounterAudioSslCipher;
   } else if (stats.content_name == cricket::CN_VIDEO) {
-    srtp_name = kVideoSrtpCipher;
-    ssl_name = kVideoSslCipher;
+    srtp_counter_type = kEnumCounterVideoSrtpCipher;
+    ssl_counter_type = kEnumCounterVideoSslCipher;
   } else if (stats.content_name == cricket::CN_DATA) {
-    srtp_name = kDataSrtpCipher;
-    ssl_name = kDataSslCipher;
+    srtp_counter_type = kEnumCounterDataSrtpCipher;
+    ssl_counter_type = kEnumCounterDataSslCipher;
   } else {
     RTC_NOTREACHED();
     return;
   }
 
   if (!srtp_cipher.empty()) {
-    metrics_observer_->AddHistogramSample(srtp_name, srtp_cipher);
+    metrics_observer_->IncrementEnumCounter(
+        srtp_counter_type, GetSrtpCipherType(srtp_cipher), SrtpCipherType_Max);
   }
   if (!ssl_cipher.empty()) {
-    metrics_observer_->AddHistogramSample(ssl_name, ssl_cipher);
+    metrics_observer_->IncrementEnumCounter(
+        ssl_counter_type, GetSslCipherType(ssl_cipher), SslCipherType_Max);
   }
 }