Introduce ability to set peer name for PC level tests

Add peer's name to params and use it for logging and metrics naming
for whole peer related metrics.

Bug: webrtc:11479
Change-Id: Ia7e3fc4839c90a958d66910614515ac02a96e389
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174752
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31215}

api/test/peerconnection_quality_test_fixture.h

@@ -253,6 +253,11 @@ class PeerConnectionE2EQualityTestFixture {
    public:
     virtual ~PeerConfigurer() = default;
 
+    // Sets peer name that will be used to report metrics related to this peer.
+    // If not set, some default name will be assigned. All names have to be
+    // unique.
+    virtual PeerConfigurer* SetName(absl::string_view name) = 0;
+
     // The parameters of the following 9 methods will be passed to the
     // PeerConnectionFactoryInterface implementation that will be created for
     // this peer.

test/pc/e2e/BUILD.gn

@@ -257,6 +257,7 @@ if (rtc_include_tests) {
       "../../../p2p:rtc_p2p",
       "../../../rtc_base:rtc_task_queue",
       "//third_party/abseil-cpp/absl/memory",
+      "//third_party/abseil-cpp/absl/strings",
     ]
   }
 
@@ -304,6 +305,7 @@ if (rtc_include_tests) {
       "../../../api/transport/media:media_transport_interface",
       "../../../api/video_codecs:video_codecs_api",
       "../../../rtc_base",
+      "//third_party/abseil-cpp/absl/strings",
     ]
   }
 

test/pc/e2e/peer_configurer.cc

@@ -12,10 +12,13 @@
 
 #include <set>
 
+#include "absl/strings/string_view.h"
+#include "rtc_base/arraysize.h"
 #include "test/testsupport/file_utils.h"
 
 namespace webrtc {
 namespace webrtc_pc_e2e {
+namespace {
 
 using AudioConfig = PeerConnectionE2EQualityTestFixture::AudioConfig;
 using VideoConfig = PeerConnectionE2EQualityTestFixture::VideoConfig;
@@ -24,35 +27,70 @@ using VideoGeneratorType =
     PeerConnectionE2EQualityTestFixture::VideoGeneratorType;
 using VideoCodecConfig = PeerConnectionE2EQualityTestFixture::VideoCodecConfig;
 
+// List of default names of generic participants according to
+// https://en.wikipedia.org/wiki/Alice_and_Bob
+constexpr absl::string_view kDefaultNames[] = {"alice", "bob",  "charlie",
+                                               "david", "erin", "frank"};
+
+class DefaultNamesProvider {
+ public:
+  // Caller have to ensure that default names array will outlive names provider
+  // instance.
+  explicit DefaultNamesProvider(
+      absl::string_view prefix,
+      rtc::ArrayView<const absl::string_view> default_names = {})
+      : prefix_(prefix), default_names_(default_names) {}
+
+  void MaybeSetName(absl::optional<std::string>* name) {
+    if (name->has_value()) {
+      known_names_.insert(name->value());
+    } else {
+      *name = GenerateName();
+    }
+  }
+
+ private:
+  std::string GenerateName() {
+    std::string name;
+    do {
+      name = GenerateNameInternal();
+    } while (!known_names_.insert(name).second);
+    return name;
+  }
+
+  std::string GenerateNameInternal() {
+    if (counter_ < default_names_.size()) {
+      return std::string(default_names_[counter_++]);
+    }
+    return prefix_ + std::to_string(counter_++);
+  }
+
+  const std::string prefix_;
+  const rtc::ArrayView<const absl::string_view> default_names_;
+
+  std::set<std::string> known_names_;
+  size_t counter_ = 0;
+};
+
+}  // namespace
+
 void SetDefaultValuesForMissingParams(
     RunParams* run_params,
     std::vector<std::unique_ptr<PeerConfigurerImpl>>* peers) {
-  int video_counter = 0;
+  DefaultNamesProvider peer_names_provider("peer_", kDefaultNames);
-  int audio_counter = 0;
-  std::set<std::string> video_labels;
-  std::set<std::string> audio_labels;
   for (size_t i = 0; i < peers->size(); ++i) {
     auto* peer = peers->at(i).get();
     auto* p = peer->params();
+    peer_names_provider.MaybeSetName(&p->name);
+    DefaultNamesProvider video_stream_names_provider(
+        *p->name + "_auto_video_stream_label_");
     for (VideoConfig& video_config : p->video_configs) {
-      if (!video_config.stream_label) {
+      video_stream_names_provider.MaybeSetName(&video_config.stream_label);
-        std::string label;
-        do {
-          label = "_auto_video_stream_label_" + std::to_string(video_counter);
-          ++video_counter;
-        } while (!video_labels.insert(label).second);
-        video_config.stream_label = label;
-      }
     }
     if (p->audio_config) {
-      if (!p->audio_config->stream_label) {
+      DefaultNamesProvider audio_stream_names_provider(
-        std::string label;
+          *p->name + "_auto_audio_stream_label_");
-        do {
+      audio_stream_names_provider.MaybeSetName(&p->audio_config->stream_label);
-          label = "_auto_audio_stream_label_" + std::to_string(audio_counter);
-          ++audio_counter;
-        } while (!audio_labels.insert(label).second);
-        p->audio_config->stream_label = label;
-      }
     }
   }
 
@@ -67,6 +105,7 @@ void ValidateParams(
     const std::vector<std::unique_ptr<PeerConfigurerImpl>>& peers) {
   RTC_CHECK_GT(run_params.video_encoder_bitrate_multiplier, 0.0);
 
+  std::set<std::string> peer_names;
   std::set<std::string> video_labels;
   std::set<std::string> audio_labels;
   int media_streams_count = 0;
@@ -74,6 +113,13 @@ void ValidateParams(
   bool has_simulcast = false;
   for (size_t i = 0; i < peers.size(); ++i) {
     Params* p = peers[i]->params();
+
+    {
+      RTC_CHECK(p->name);
+      bool inserted = peer_names.insert(p->name.value()).second;
+      RTC_CHECK(inserted) << "Duplicate name=" << p->name.value();
+    }
+
     if (p->audio_config) {
       media_streams_count++;
     }

test/pc/e2e/peer_configurer.h

@@ -15,6 +15,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/strings/string_view.h"
 #include "api/async_resolver_factory.h"
 #include "api/call/call_factory_interface.h"
 #include "api/fec_controller.h"
@@ -44,6 +45,11 @@ class PeerConfigurerImpl final
                                                            network_manager)),
         params_(std::make_unique<Params>()) {}
 
+  PeerConfigurer* SetName(absl::string_view name) override {
+    params_->name = std::string(name);
+    return this;
+  }
+
   // Implementation of PeerConnectionE2EQualityTestFixture::PeerConfigurer.
   PeerConfigurer* SetTaskQueueFactory(
       std::unique_ptr<TaskQueueFactory> task_queue_factory) override {

test/pc/e2e/peer_connection_e2e_smoke_test.cc

@@ -162,14 +162,15 @@ TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Smoke) {
         audio.sync_group = "alice-media";
         alice->SetAudioConfig(std::move(audio));
       },
-      [](PeerConfigurer* bob) {
+      [](PeerConfigurer* charlie) {
+        charlie->SetName("charlie");
         VideoConfig video(640, 360, 30);
-        video.stream_label = "bob-video";
+        video.stream_label = "charlie-video";
         video.temporal_layers_count = 2;
-        bob->AddVideoConfig(std::move(video));
+        charlie->AddVideoConfig(std::move(video));
 
         VideoConfig screenshare(640, 360, 30);
-        screenshare.stream_label = "bob-screenshare";
+        screenshare.stream_label = "charlie-screenshare";
         screenshare.content_hint = VideoTrackInterface::ContentHint::kText;
         ScreenShareConfig screen_share_config =
             ScreenShareConfig(TimeDelta::Seconds(2));
@@ -177,15 +178,15 @@ TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Smoke) {
             TimeDelta::Millis(1800), kDefaultSlidesWidth, kDefaultSlidesHeight);
         auto screen_share_frame_generator =
             CreateScreenShareFrameGenerator(screenshare, screen_share_config);
-        bob->AddVideoConfig(std::move(screenshare),
+        charlie->AddVideoConfig(std::move(screenshare),
                                 std::move(screen_share_frame_generator));
 
         AudioConfig audio;
-        audio.stream_label = "bob-audio";
+        audio.stream_label = "charlie-audio";
         audio.mode = AudioConfig::Mode::kFile;
         audio.input_file_name =
             test::ResourcePath("pc_quality_smoke_test_bob_source", "wav");
-        bob->SetAudioConfig(std::move(audio));
+        charlie->SetAudioConfig(std::move(audio));
       });
 }
 

test/pc/e2e/peer_connection_quality_test.cc

@@ -175,13 +175,17 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
   SetupRequiredFieldTrials(run_params);
 
   // Print test summary
-  RTC_LOG(INFO)
+  RTC_LOG(INFO) << "Media quality test: " << *alice_configurer->params()->name
-      << "Media quality test: Alice will make a call to Bob with media video="
+                << " will make a call to " << *bob_configurer->params()->name
+                << " with media video="
                 << !alice_configurer->params()->video_configs.empty()
-      << "; audio=" << alice_configurer->params()->audio_config.has_value()
+                << "; audio="
-      << ". Bob will respond with media video="
+                << alice_configurer->params()->audio_config.has_value() << ". "
+                << *bob_configurer->params()->name
+                << " will respond with media video="
                 << !bob_configurer->params()->video_configs.empty()
-      << "; audio=" << bob_configurer->params()->audio_config.has_value();
+                << "; audio="
+                << bob_configurer->params()->audio_config.has_value();
 
   const std::unique_ptr<rtc::Thread> signaling_thread = rtc::Thread::Create();
   signaling_thread->SetName(kSignalThreadName, nullptr);
@@ -273,7 +277,8 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
                                       return kAliveMessageLogInterval;
                                     });
 
-  RTC_LOG(INFO) << "Configuration is done. Now Alice is calling to Bob...";
+  RTC_LOG(INFO) << "Configuration is done. Now " << *alice_->params()->name
+                << " is calling to " << *bob_->params()->name << "...";
 
   // Setup stats poller.
   std::vector<StatsObserverInterface*> observers = {
@@ -282,8 +287,8 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
   for (auto& reporter : quality_metrics_reporters_) {
     observers.push_back(reporter.get());
   }
-  StatsPoller stats_poller(observers,
+  StatsPoller stats_poller(observers, {{*alice_->params()->name, alice_.get()},
-                           {{"alice", alice_.get()}, {"bob", bob_.get()}});
+                                       {*bob_->params()->name, bob_.get()}});
   executor_->ScheduleActivity(TimeDelta::Zero(), kStatsUpdateInterval,
                               [&stats_poller](TimeDelta) {
                                 stats_poller.PollStatsAndNotifyObservers();
@@ -345,10 +350,8 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
   // Reset |task_queue_| after test to cleanup.
   task_queue_.reset();
 
-  // Ensuring that TestPeers have been destroyed in order to correctly close
+  alice_ = nullptr;
-  // Audio dumps.
+  bob_ = nullptr;
-  RTC_CHECK(!alice_);
-  RTC_CHECK(!bob_);
   // Ensuring that TestVideoCapturerVideoTrackSource are destroyed on the right
   // thread.
   RTC_CHECK(alice_video_sources_.empty());
@@ -585,7 +588,8 @@ void PeerConnectionE2EQualityTest::ExchangeIceCandidates(
   for (auto& candidate : alice_candidates) {
     std::string candidate_str;
     RTC_CHECK(candidate->ToString(&candidate_str));
-    RTC_LOG(INFO) << "Alice ICE candidate(mid= " << candidate->sdp_mid()
+    RTC_LOG(INFO) << *alice_->params()->name
+                  << " ICE candidate(mid= " << candidate->sdp_mid()
                   << "): " << candidate_str;
   }
   ASSERT_TRUE(bob_->AddIceCandidates(std::move(alice_candidates)));
@@ -595,7 +599,8 @@ void PeerConnectionE2EQualityTest::ExchangeIceCandidates(
   for (auto& candidate : bob_candidates) {
     std::string candidate_str;
     RTC_CHECK(candidate->ToString(&candidate_str));
-    RTC_LOG(INFO) << "Bob ICE candidate(mid= " << candidate->sdp_mid()
+    RTC_LOG(INFO) << *bob_->params()->name
+                  << " ICE candidate(mid= " << candidate->sdp_mid()
                   << "): " << candidate_str;
   }
   ASSERT_TRUE(alice_->AddIceCandidates(std::move(bob_candidates)));
@@ -624,19 +629,19 @@ void PeerConnectionE2EQualityTest::TearDownCall() {
 
   alice_video_sources_.clear();
   bob_video_sources_.clear();
-  alice_.reset();
-  bob_.reset();
 
-  media_helper_.reset();
+  media_helper_ = nullptr;
 }
 
 void PeerConnectionE2EQualityTest::ReportGeneralTestResults() {
-  test::PrintResult(
+  test::PrintResult(*alice_->params()->name + "_connected", "", test_case_name_,
-      "alice_connected", "", test_case_name_, alice_connected_, "unitless",
+                    alice_connected_, "unitless",
-      /*important=*/false, test::ImproveDirection::kBiggerIsBetter);
+                    /*important=*/false,
-  test::PrintResult(
+                    test::ImproveDirection::kBiggerIsBetter);
-      "bob_connected", "", test_case_name_, bob_connected_, "unitless",
+  test::PrintResult(*bob_->params()->name + "_connected", "", test_case_name_,
-      /*important=*/false, test::ImproveDirection::kBiggerIsBetter);
+                    bob_connected_, "unitless",
+                    /*important=*/false,
+                    test::ImproveDirection::kBiggerIsBetter);
 }
 
 Timestamp PeerConnectionE2EQualityTest::Now() const {

test/pc/e2e/peer_connection_quality_test_params.h

@@ -100,6 +100,8 @@ struct InjectableComponents {
 // unlimited amount of video streams) and rtc configuration, that will be used
 // to set up peer connection.
 struct Params {
+  // Peer name. If empty - default one will be set by the fixture.
+  absl::optional<std::string> name;
   // If |video_configs| is empty - no video should be added to the test call.
   std::vector<PeerConnectionE2EQualityTestFixture::VideoConfig> video_configs;
   // If |audio_config| is set audio stream will be configured

test/pc/e2e/test_peer_factory.h

@@ -16,6 +16,7 @@
 #include <string>
 #include <vector>
 
+#include "absl/strings/string_view.h"
 #include "api/rtc_event_log/rtc_event_log_factory.h"
 #include "api/test/peerconnection_quality_test_fixture.h"
 #include "modules/audio_device/include/test_audio_device.h"