Add UMA metrics for ICE regathering reasons.

webrtc/p2p/base/p2ptransportchannel_unittest.cc

 /*
  *  Copyright 2009 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <algorithm>
 #include <memory>
 
+#include "webrtc/api/fakemetricsobserver.h"
 #include "webrtc/p2p/base/fakeportallocator.h"
 #include "webrtc/p2p/base/p2ptransportchannel.h"
 #include "webrtc/p2p/base/testrelayserver.h"
 #include "webrtc/p2p/base/teststunserver.h"
 #include "webrtc/p2p/base/testturnserver.h"
 #include "webrtc/p2p/client/basicportallocator.h"
 #include "webrtc/base/dscp.h"
 #include "webrtc/base/fakeclock.h"
 #include "webrtc/base/fakenetwork.h"
 #include "webrtc/base/firewallsocketserver.h"
@@ skipping 923 matching lines @@
                                                      "tcp",
                                                      "local",
                                                      "tcp",
                                                      3000);
 
 // Test the matrix of all the connectivity types we expect to see in the wild.
 // Just test every combination of the configs in the Config enum.
 class P2PTransportChannelTest : public P2PTransportChannelTestBase {
  protected:
   static const Result* kMatrix[NUM_CONFIGS][NUM_CONFIGS];
-  void ConfigureEndpoints(Config config1,
-                          Config config2,
-                          int allocator_flags1,
-                          int allocator_flags2) {
+  void ConfigureEndpoints(
+      Config config1,
+      Config config2,
+      int allocator_flags1,
+      int allocator_flags2,
+      webrtc::MetricsObserverInterface* observer1 = nullptr,
+      webrtc::MetricsObserverInterface* observer2 = nullptr) {

[Taylor Brandstetter, 2016/10/03 20:33:35]

I'd prefer if the Endpoint owned its own metric observer and hooked it up by
default. Then the individual test cases don't need to do anything but access it.

This is what other tests such as WebRtcSessionTest do.

[honghaiz3, 2016/10/04 01:03:55]

Done.

     int delay = kMinimumStepDelay;
     ConfigureEndpoint(0, config1);
     SetAllocatorFlags(0, allocator_flags1);
     SetAllocationStepDelay(0, delay);
     ConfigureEndpoint(1, config2);
     SetAllocatorFlags(1, allocator_flags2);
     SetAllocationStepDelay(1, delay);
 
     set_remote_ice_parameter_source(FROM_SETICEPARAMETERS);
+    if (observer1) {
+      GetAllocator(0)->set_metrics_observer(observer1);
+    }
+    if (observer2) {
+      GetAllocator(1)->set_metrics_observer(observer2);
+    }
   }
   void ConfigureEndpoint(int endpoint, Config config) {
     switch (config) {
       case OPEN:
         AddAddress(endpoint, kPublicAddrs[endpoint]);
         break;
       case NAT_FULL_CONE:
       case NAT_ADDR_RESTRICTED:
       case NAT_PORT_RESTRICTED:
       case NAT_SYMMETRIC:
@@ skipping 185 matching lines @@
   EXPECT_TRUE(best_conn_info->writable);
   EXPECT_FALSE(best_conn_info->timeout);
   EXPECT_EQ(10U, best_conn_info->sent_total_packets);
   EXPECT_EQ(0U, best_conn_info->sent_discarded_packets);
   EXPECT_EQ(10 * 36U, best_conn_info->sent_total_bytes);
   EXPECT_EQ(10 * 36U, best_conn_info->recv_total_bytes);
   EXPECT_GT(best_conn_info->rtt, 0U);
   DestroyChannels();
 }
 
+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonIceRestartWhenDisconnected) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer,
+                     observer);
+
+  CreateChannels();
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);

[Taylor Brandstetter, 2016/10/03 20:33:35]

Could we use constants here instead of 3000 and 6000?

[honghaiz3, 2016/10/04 01:03:55]

Done.

+
+  // Drop all packets so that both channels become not writable.
+  fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]);
+  EXPECT_TRUE_SIMULATED_WAIT(!ep1_ch1()->writable() && !ep2_ch1()->writable(),
+                             6000, clock);
+
+  ep1_ch1()->SetIceParameters(kIceParams[2]);
+  ep1_ch1()->SetRemoteIceParameters(kIceParams[3]);
+  ep1_ch1()->MaybeStartGathering();
+  EXPECT_EQ(1, observer->GetEnumCounter(
+                   webrtc::kEnumCounterIceRegatheringReason,
+                   static_cast<int>(
+                       IceRegatheringReason::ICE_RESTART_WHEN_DISCONNECTED)));
+
+  ep2_ch1()->SetIceParameters(kIceParams[3]);
+  ep2_ch1()->SetRemoteIceParameters(kIceParams[2]);
+  ep2_ch1()->MaybeStartGathering();
+  EXPECT_EQ(2, observer->GetEnumCounter(
+                   webrtc::kEnumCounterIceRegatheringReason,
+                   static_cast<int>(
+                       IceRegatheringReason::ICE_RESTART_WHEN_DISCONNECTED)));
+
+  DestroyChannels();
+}
+
+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonIceRestartWhenConnected) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer,
+                     observer);
+
+  CreateChannels();
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);
+
+  ep1_ch1()->SetIceParameters(kIceParams[2]);
+  ep1_ch1()->SetRemoteIceParameters(kIceParams[3]);
+  ep1_ch1()->MaybeStartGathering();
+  EXPECT_EQ(
+      1,
+      observer->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(IceRegatheringReason::ICE_RESTART_WHEN_CONNECTED)));
+
+  ep2_ch1()->SetIceParameters(kIceParams[3]);
+  ep2_ch1()->SetRemoteIceParameters(kIceParams[2]);
+  ep2_ch1()->MaybeStartGathering();
+  EXPECT_EQ(
+      2,
+      observer->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(IceRegatheringReason::ICE_RESTART_WHEN_CONNECTED)));
+
+  DestroyChannels();
+}
+
+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonIceRestartWhenConnecting) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer,
+                     observer);
+
+  // Create the channels without waiting for them to become connected.
+  CreateChannels();
+
+  ep1_ch1()->SetIceParameters(kIceParams[2]);
+  ep1_ch1()->SetRemoteIceParameters(kIceParams[3]);
+  ep1_ch1()->MaybeStartGathering();
+  EXPECT_EQ(
+      1,
+      observer->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(IceRegatheringReason::ICE_RESTART_WHEN_CONNECTING)));
+
+  ep2_ch1()->SetIceParameters(kIceParams[3]);
+  ep2_ch1()->SetRemoteIceParameters(kIceParams[2]);
+  ep2_ch1()->MaybeStartGathering();
+  EXPECT_EQ(
+      2,
+      observer->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(IceRegatheringReason::ICE_RESTART_WHEN_CONNECTING)));
+
+  DestroyChannels();
+}
+

[Taylor Brandstetter, 2016/10/03 20:33:35]

I think it would be helpful to add a comment explaining that this tests one
channel with continual gathering enabled and one with it disabled. Same with the
test below.

[honghaiz3, 2016/10/04 01:03:54]

Done.

+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonContinualGatheringByNetworkChange) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer1(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer2(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer1,
+                     observer2);
+
+  // ep1 gathers continually but ep2 does not.
+  IceConfig continual_gathering_config =
+      CreateIceConfig(1000, GATHER_CONTINUALLY);
+  IceConfig default_config;
+  CreateChannels(continual_gathering_config, default_config);
+
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);
+
+  // Add address in ep1 will trigger continual gathering.
+  AddAddress(0, kAlternateAddrs[0]);
+  EXPECT_EQ_SIMULATED_WAIT(
+      1, observer1->GetEnumCounter(
+             webrtc::kEnumCounterIceRegatheringReason,
+             static_cast<int>(
+                 IceRegatheringReason::CONTINUAL_GATHERING_BY_NETWORK_CHANGE)),
+      kDefaultTimeout, clock);
+
+  ep2_ch1()->SetIceParameters(kIceParams[3]);
+  ep2_ch1()->SetRemoteIceParameters(kIceParams[2]);
+  ep2_ch1()->MaybeStartGathering();
+
+  AddAddress(1, kAlternateAddrs[1]);
+  SIMULATED_WAIT(false, kDefaultTimeout, clock);
+  // ep2 has not enabled continual gathering.
+  EXPECT_EQ(
+      0, observer2->GetEnumCounter(
+             webrtc::kEnumCounterIceRegatheringReason,
+             static_cast<int>(
+                 IceRegatheringReason::CONTINUAL_GATHERING_BY_NETWORK_CHANGE)));
+
+  DestroyChannels();
+}
+
+TEST_F(P2PTransportChannelTest,
+       TestIceRegatheringReasonContinualGatheringByNetworkFailure) {
+  rtc::ScopedFakeClock clock;
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer1(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  rtc::scoped_refptr<webrtc::FakeMetricsObserver> observer2(
+      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>());
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts, observer1,
+                     observer2);
+
+  // ep1 gathers continually but ep2 does not.
+  IceConfig config1 = CreateIceConfig(1000, GATHER_CONTINUALLY);
+  config1.regather_on_failed_networks_interval = rtc::Optional<int>(2000);
+  IceConfig config2;
+  config2.regather_on_failed_networks_interval = rtc::Optional<int>(2000);
+  CreateChannels(config1, config2);
+
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);
+
+  fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]);
+
+  SIMULATED_WAIT(false, 35000, clock);

[Taylor Brandstetter, 2016/10/03 20:33:35]

Could you replace 35000 with a constant?

[honghaiz3, 2016/10/04 01:03:54]

Done.

+  EXPECT_LE(
+      1,
+      observer1->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(
+              IceRegatheringReason::CONTINUAL_GATHERING_BY_NETWORK_FAILURE)));
+  LOG(LS_INFO) << "x1";

[Taylor Brandstetter, 2016/10/03 20:33:35]

Remember to remove this log statement

[honghaiz3, 2016/10/04 01:03:54]

Done.

+  EXPECT_EQ(
+      0,
+      observer2->GetEnumCounter(
+          webrtc::kEnumCounterIceRegatheringReason,
+          static_cast<int>(
+              IceRegatheringReason::CONTINUAL_GATHERING_BY_NETWORK_FAILURE)));
+
+  DestroyChannels();
+}
+
 // Test that we properly create a connection on a STUN ping from unknown address
 // when the signaling is slow.
 TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {
   ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
                      kDefaultPortAllocatorFlags);
   // Emulate no remote parameters coming in.
   set_remote_ice_parameter_source(FROM_CANDIDATE);
   CreateChannels();
   // Only have remote parameters come in for ep2, not ep1.
   ep2_ch1()->SetRemoteIceParameters(kIceParams[0]);
@@ skipping 2815 matching lines @@
 
   // TCP Relay/Relay is the next.
   VerifyNextPingableConnection(RELAY_PORT_TYPE, RELAY_PORT_TYPE,
                                TCP_PROTOCOL_NAME);
 
   // Finally, Local/Relay will be pinged.
   VerifyNextPingableConnection(LOCAL_PORT_TYPE, RELAY_PORT_TYPE);
 }
 
 }  // namespace cricket {