From e1cd1585471f394b3021602569e7ebd80b50455f Mon Sep 17 00:00:00 2001 From: Dor Hen Date: Thu, 16 Feb 2023 09:32:41 +0200 Subject: [PATCH] Allow setting the network labels in NetworkQualityMetricsReporter This CL gives the ability to explicitly set the network labels in the network quality metrics reporter, so that reported metrics can be aligned with peer names in case "alice" and "bob" are not used as peer names. Bug: webrtc:14897 Change-Id: Ib66d4683af71ff4302696aff4dcb9932b47e4efd Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/293161 Reviewed-by: Harald Alvestrand Reviewed-by: Artem Titov Reviewed-by: Andrey Logvin Reviewed-by: Mirko Bonadei Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/main@{#39360} --- test/pc/e2e/BUILD.gn | 1 + .../e2e/network_quality_metrics_reporter.cc | 17 +- .../pc/e2e/network_quality_metrics_reporter.h | 7 + ...nnection_quality_test_metric_names_test.cc | 179 ++++++++++++++++++ 4 files changed, 202 insertions(+), 2 deletions(-) diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 11616d0bf9..ae9dd6414c 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -317,6 +317,7 @@ if (!build_with_chromium) { sources = [ "peer_connection_quality_test_metric_names_test.cc" ] deps = [ ":metric_metadata_keys", + ":network_quality_metrics_reporter", ":peerconnection_quality_test", ":stats_based_network_quality_metrics_reporter", "../..:test_support", diff --git a/test/pc/e2e/network_quality_metrics_reporter.cc b/test/pc/e2e/network_quality_metrics_reporter.cc index 0bb28f0847..60990a87c9 100644 --- a/test/pc/e2e/network_quality_metrics_reporter.cc +++ b/test/pc/e2e/network_quality_metrics_reporter.cc @@ -44,6 +44,19 @@ NetworkQualityMetricsReporter::NetworkQualityMetricsReporter( RTC_CHECK(metrics_logger_); } +NetworkQualityMetricsReporter::NetworkQualityMetricsReporter( + absl::string_view alice_network_label, + EmulatedNetworkManagerInterface* alice_network, + absl::string_view bob_network_label, + EmulatedNetworkManagerInterface* bob_network, + test::MetricsLogger* metrics_logger) + : NetworkQualityMetricsReporter(alice_network, + bob_network, + metrics_logger) { + alice_network_label_ = std::string(alice_network_label); + bob_network_label_ = std::string(bob_network_label); +} + void NetworkQualityMetricsReporter::Start( absl::string_view test_case_name, const TrackIdStreamInfoMap* /*reporter_helper*/) { @@ -92,8 +105,8 @@ void NetworkQualityMetricsReporter::StopAndReportResults() { int64_t bob_packets_loss = bob_stats.overall_outgoing_stats.packets_sent - alice_stats.overall_incoming_stats.packets_received; - ReportStats("alice", alice_stats, alice_packets_loss); - ReportStats("bob", bob_stats, bob_packets_loss); + ReportStats(alice_network_label_, alice_stats, alice_packets_loss); + ReportStats(bob_network_label_, bob_stats, bob_packets_loss); if (!webrtc::field_trial::IsEnabled(kUseStandardBytesStats)) { RTC_LOG(LS_ERROR) diff --git a/test/pc/e2e/network_quality_metrics_reporter.h b/test/pc/e2e/network_quality_metrics_reporter.h index ed894bcf54..1348a58943 100644 --- a/test/pc/e2e/network_quality_metrics_reporter.h +++ b/test/pc/e2e/network_quality_metrics_reporter.h @@ -31,6 +31,11 @@ class NetworkQualityMetricsReporter NetworkQualityMetricsReporter(EmulatedNetworkManagerInterface* alice_network, EmulatedNetworkManagerInterface* bob_network, test::MetricsLogger* metrics_logger); + NetworkQualityMetricsReporter(absl::string_view alice_network_label, + EmulatedNetworkManagerInterface* alice_network, + absl::string_view bob_network_label, + EmulatedNetworkManagerInterface* bob_network, + test::MetricsLogger* metrics_logger); ~NetworkQualityMetricsReporter() override = default; // Network stats must be empty when this method will be invoked. @@ -64,6 +69,8 @@ class NetworkQualityMetricsReporter test::MetricsLogger* const metrics_logger_; Mutex lock_; std::map pc_stats_ RTC_GUARDED_BY(lock_); + std::string alice_network_label_ = "alice"; + std::string bob_network_label_ = "bob"; }; } // namespace webrtc_pc_e2e diff --git a/test/pc/e2e/peer_connection_quality_test_metric_names_test.cc b/test/pc/e2e/peer_connection_quality_test_metric_names_test.cc index 8a47e108e0..a2830af72d 100644 --- a/test/pc/e2e/peer_connection_quality_test_metric_names_test.cc +++ b/test/pc/e2e/peer_connection_quality_test_metric_names_test.cc @@ -25,6 +25,7 @@ #include "test/gmock.h" #include "test/gtest.h" #include "test/pc/e2e/metric_metadata_keys.h" +#include "test/pc/e2e/network_quality_metrics_reporter.h" #include "test/pc/e2e/peer_connection_quality_test.h" #include "test/pc/e2e/stats_based_network_quality_metrics_reporter.h" @@ -32,6 +33,7 @@ namespace webrtc { namespace webrtc_pc_e2e { namespace { +using ::testing::IsSupersetOf; using ::testing::UnorderedElementsAre; using ::webrtc::test::DefaultMetricsLogger; @@ -1097,6 +1099,183 @@ TEST(PeerConnectionE2EQualityTestMetricNamesTest, "test_case"}}})); } +TEST(PeerConnectionE2EQualityTestMetricNamesTest, + ExportedNetworkMetricsHaveCustomNetworkLabelIfSet) { + std::unique_ptr network_emulation = + CreateNetworkEmulationManager(TimeMode::kSimulated); + DefaultMetricsLogger metrics_logger( + network_emulation->time_controller()->GetClock()); + PeerConnectionE2EQualityTest fixture( + "test_case", *network_emulation->time_controller(), + /*audio_quality_analyzer=*/nullptr, /*video_quality_analyzer=*/nullptr, + &metrics_logger); + + EmulatedEndpoint* alice_endpoint = + network_emulation->CreateEndpoint(EmulatedEndpointConfig()); + EmulatedEndpoint* bob_endpoint = + network_emulation->CreateEndpoint(EmulatedEndpointConfig()); + + network_emulation->CreateRoute( + alice_endpoint, {network_emulation->CreateUnconstrainedEmulatedNode()}, + bob_endpoint); + network_emulation->CreateRoute( + bob_endpoint, {network_emulation->CreateUnconstrainedEmulatedNode()}, + alice_endpoint); + + EmulatedNetworkManagerInterface* alice_network = + network_emulation->CreateEmulatedNetworkManagerInterface( + {alice_endpoint}); + EmulatedNetworkManagerInterface* bob_network = + network_emulation->CreateEmulatedNetworkManagerInterface({bob_endpoint}); + + AddDefaultAudioVideoPeer("alice", "alice_audio", "alice_video", + alice_network->network_dependencies(), fixture); + AddDefaultAudioVideoPeer("bob", "bob_audio", "bob_video", + bob_network->network_dependencies(), fixture); + std::string kAliceNetworkLabel = "alice_label"; + std::string kBobNetworkLabel = "bob_label"; + fixture.AddQualityMetricsReporter( + std::make_unique( + kAliceNetworkLabel, alice_network, kBobNetworkLabel, bob_network, + &metrics_logger)); + + fixture.Run(RunParams(TimeDelta::Seconds(1))); + + std::vector metrics = + ToValidationInfo(metrics_logger.GetCollectedMetrics()); + + EXPECT_THAT(metrics, + IsSupersetOf( + // Metrics from PeerConnectionE2EQualityTest + std::vector{ + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "bytes_discarded_no_receiver", + .unit = Unit::kBytes, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "packets_discarded_no_receiver", + .unit = Unit::kUnitless, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "bytes_sent", + .unit = Unit::kBytes, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "packets_sent", + .unit = Unit::kUnitless, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "average_send_rate", + .unit = Unit::kKilobitsPerSecond, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "bytes_received", + .unit = Unit::kBytes, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "packets_received", + .unit = Unit::kUnitless, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "average_receive_rate", + .unit = Unit::kKilobitsPerSecond, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kAliceNetworkLabel, + .name = "sent_packets_loss", + .unit = Unit::kUnitless, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "bytes_discarded_no_receiver", + .unit = Unit::kBytes, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "packets_discarded_no_receiver", + .unit = Unit::kUnitless, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "bytes_sent", + .unit = Unit::kBytes, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "packets_sent", + .unit = Unit::kUnitless, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "average_send_rate", + .unit = Unit::kKilobitsPerSecond, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "bytes_received", + .unit = Unit::kBytes, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "packets_received", + .unit = Unit::kUnitless, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "average_receive_rate", + .unit = Unit::kKilobitsPerSecond, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }, + MetricValidationInfo{ + .test_case = "test_case/" + kBobNetworkLabel, + .name = "sent_packets_loss", + .unit = Unit::kUnitless, + .improvement_direction = + ImprovementDirection::kNeitherIsBetter, + }})); +} + } // namespace } // namespace webrtc_pc_e2e } // namespace webrtc