commit 194939b8ca3c2c36ba7615bbd32348e44fafbce8
author Harald Alvestrand <hta@webrtc.org> Wed Jan 24 15:04:13 2018
committer Commit Bot <commit-bot@chromium.org> Wed Jan 24 16:09:59 2018
tree 8f11ac281896eba6e79a957a48579dfd131d20d9
parent d2b5b1f5bae94c24cc279fe198ff28236821f1ac

Added UMA counters for SDES vs DTLS key agreement

This is required to figure out when we can deprecate and remove
SDES.

Bug: chromium:804275
Change-Id: Ie234e6b3c8f5de8e78dda8d755d955caa61b7aa7
Reviewed-on: https://webrtc-review.googlesource.com/43340
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21746}

api/umametrics.h

diff --git a/api/umametrics.h b/api/umametrics.h
index c512598..2347ac2 100644
--- a/api/umametrics.h
+++ b/api/umametrics.h
@@ -37,6 +37,7 @@
   kEnumCounterDtlsHandshakeError,
   kEnumCounterIceRegathering,
   kEnumCounterIceRestart,
+  kEnumCounterKeyProtocol,
   kPeerConnectionEnumCounterMax
 };
 
@@ -111,6 +112,12 @@
   kIceCandidatePairMax
 };
 
+enum KeyExchangeProtocolType {
+  kEnumCounterKeyProtocolDtls,
+  kEnumCounterKeyProtocolSdes,
+  kEnumCounterKeyProtocolMax
+};
+
 class MetricsObserverInterface : public rtc::RefCountInterface {
  public:
   // |type| is the type of the enum counter to be incremented. |counter|

pc/peerconnection.cc

diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 8431707..d24dcbf 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -364,7 +364,9 @@
 // needs a ufrag and pwd. Mismatches, such as replying with a DTLS fingerprint
 // to SDES keys, will be caught in JsepTransport negotiation, and backstopped
 // by Channel's |srtp_required| check.
-RTCError VerifyCrypto(const SessionDescription* desc, bool dtls_enabled) {
+RTCError VerifyCrypto(const SessionDescription* desc,
+                      bool dtls_enabled,
+                      rtc::scoped_refptr<webrtc::UMAObserver> uma_observer) {
   const cricket::ContentGroup* bundle =
       desc->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
   for (const cricket::ContentInfo& content_info : desc->contents()) {
@@ -396,12 +398,22 @@
         return RTCError(RTCErrorType::INVALID_PARAMETER,
                         kSdpWithoutDtlsFingerprint);
       }
+      if (uma_observer) {
+        uma_observer->IncrementEnumCounter(webrtc::kEnumCounterKeyProtocol,
+                                           webrtc::kEnumCounterKeyProtocolDtls,
+                                           webrtc::kEnumCounterKeyProtocolMax);
+      }
     } else {
       if (media->cryptos().empty()) {
         RTC_LOG(LS_WARNING)
             << "Session description must have SDES when DTLS disabled.";
         return RTCError(RTCErrorType::INVALID_PARAMETER, kSdpWithoutSdesCrypto);
       }
+      if (uma_observer) {
+        uma_observer->IncrementEnumCounter(webrtc::kEnumCounterKeyProtocol,
+                                           webrtc::kEnumCounterKeyProtocolSdes,
+                                           webrtc::kEnumCounterKeyProtocolMax);
+      }
     }
   }
   return RTCError::OK();
@@ -5319,7 +5331,8 @@
   std::string crypto_error;
   if (webrtc_session_desc_factory_->SdesPolicy() == cricket::SEC_REQUIRED ||
       dtls_enabled_) {
-    RTCError crypto_error = VerifyCrypto(sdesc->description(), dtls_enabled_);
+    RTCError crypto_error =
+        VerifyCrypto(sdesc->description(), dtls_enabled_, uma_observer_);
     if (!crypto_error.ok()) {
       return crypto_error;
     }

pc/peerconnection_integrationtest.cc

diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc
index 7bf6ad8..aee5ca1 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -1284,6 +1284,10 @@
 TEST_F(PeerConnectionIntegrationTest, EndToEndCallWithDtls) {
   ASSERT_TRUE(CreatePeerConnectionWrappers());
   ConnectFakeSignaling();
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> caller_observer =
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
+  caller()->pc()->RegisterUMAObserver(caller_observer);
+
   // Do normal offer/answer and wait for some frames to be received in each
   // direction.
   caller()->AddAudioVideoTracks();
@@ -1294,6 +1298,12 @@
       kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount,
       kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount,
       kMaxWaitForFramesMs);
+  EXPECT_LE(
+      1, caller_observer->GetEnumCounter(webrtc::kEnumCounterKeyProtocol,
+                                         webrtc::kEnumCounterKeyProtocolDtls));
+  EXPECT_EQ(
+      0, caller_observer->GetEnumCounter(webrtc::kEnumCounterKeyProtocol,
+                                         webrtc::kEnumCounterKeyProtocolSdes));
 }
 
 // Uses SDES instead of DTLS for key agreement.
@@ -1302,6 +1312,9 @@
   sdes_config.enable_dtls_srtp.emplace(false);
   ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(sdes_config, sdes_config));
   ConnectFakeSignaling();
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> caller_observer =
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
+  caller()->pc()->RegisterUMAObserver(caller_observer);
 
   // Do normal offer/answer and wait for some frames to be received in each
   // direction.
@@ -1313,6 +1326,12 @@
       kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount,
       kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount,
       kMaxWaitForFramesMs);
+  EXPECT_LE(
+      1, caller_observer->GetEnumCounter(webrtc::kEnumCounterKeyProtocol,
+                                         webrtc::kEnumCounterKeyProtocolSdes));
+  EXPECT_EQ(
+      0, caller_observer->GetEnumCounter(webrtc::kEnumCounterKeyProtocol,
+                                         webrtc::kEnumCounterKeyProtocolDtls));
 }
 
 // Tests that the GetRemoteAudioSSLCertificate method returns the remote DTLS