From 149dc72dfa9a49fa348b529f4e21776a95949d2f Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Wed, 28 Aug 2019 08:10:27 +0200 Subject: [PATCH] Add support for RTCTransportStats.selectedCandidatePairChanges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds accounting and reporting needed for newly added RTCTransportStats.selectedCandidatePairChanges, https://w3c.github.io/webrtc-stats/#dom-rtctransportstats-selectedcandidatepairchanges a) P2PTransportChannel counts everytime selected_connection_ is modified and reports this counter in the GetStats()-call. b) RTCStatsCollector puts the counter into the standardized stats object. Bug: webrtc:10900 Change-Id: Ibaeca18706b8edcbcb44b0c6f2754854bcb545ba Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149830 Reviewed-by: Qingsi Wang Reviewed-by: Henrik Boström Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/master@{#28987} --- api/stats/rtcstats_objects.h | 1 + p2p/base/fake_ice_transport.h | 11 +- p2p/base/ice_transport_internal.h | 12 +- p2p/base/mock_ice_transport.h | 4 +- p2p/base/p2p_transport_channel.cc | 16 ++- p2p/base/p2p_transport_channel.h | 6 +- p2p/base/p2p_transport_channel_unittest.cc | 149 +++++++++++++++++---- pc/jsep_transport.cc | 2 +- pc/peer_connection.cc | 2 +- pc/rtc_stats_collector.cc | 6 +- pc/rtc_stats_collector_unittest.cc | 69 ++++++---- pc/rtc_stats_integrationtest.cc | 2 + pc/stats_collector.cc | 4 +- pc/stats_collector_unittest.cc | 2 +- pc/transport_stats.h | 4 +- stats/rtcstats_objects.cc | 9 +- 16 files changed, 211 insertions(+), 88 deletions(-) diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h index b492203635..5e8df330cb 100644 --- a/api/stats/rtcstats_objects.h +++ b/api/stats/rtcstats_objects.h @@ -594,6 +594,7 @@ class RTC_EXPORT RTCTransportStats final : public RTCStats { RTCStatsMember selected_candidate_pair_id; RTCStatsMember local_certificate_id; RTCStatsMember remote_certificate_id; + RTCStatsMember selected_candidate_pair_changes; }; } // namespace webrtc diff --git a/p2p/base/fake_ice_transport.h b/p2p/base/fake_ice_transport.h index 467ee2ec1c..b1a83b8ced 100644 --- a/p2p/base/fake_ice_transport.h +++ b/p2p/base/fake_ice_transport.h @@ -193,14 +193,13 @@ class FakeIceTransport : public IceTransportInternal { void RemoveAllRemoteCandidates() override { remote_candidates_.clear(); } - bool GetStats(ConnectionInfos* candidate_pair_stats_list, - CandidateStatsList* candidate_stats_list) override { + bool GetStats(IceTransportStats* ice_transport_stats) override { CandidateStats candidate_stats; ConnectionInfo candidate_pair_stats; - candidate_stats_list->clear(); - candidate_stats_list->push_back(candidate_stats); - candidate_pair_stats_list->clear(); - candidate_pair_stats_list->push_back(candidate_pair_stats); + ice_transport_stats->candidate_stats_list.clear(); + ice_transport_stats->candidate_stats_list.push_back(candidate_stats); + ice_transport_stats->connection_infos.clear(); + ice_transport_stats->connection_infos.push_back(candidate_pair_stats); return true; } diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h index 7f1d70bb94..94b5b194ff 100644 --- a/p2p/base/ice_transport_internal.h +++ b/p2p/base/ice_transport_internal.h @@ -30,6 +30,15 @@ namespace cricket { +struct IceTransportStats { + CandidateStatsList candidate_stats_list; + ConnectionInfos connection_infos; + // Number of times the selected candidate pair has changed + // Initially 0 and 1 once the first candidate pair has been selected. + // The counter is increase also when "unselecting" a connection. + uint32_t selected_candidate_pair_changes = 0; +}; + typedef std::vector Candidates; enum IceConnectionState { @@ -256,8 +265,7 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { virtual IceGatheringState gathering_state() const = 0; // Returns the current stats for this connection. - virtual bool GetStats(ConnectionInfos* candidate_pair_stats_list, - CandidateStatsList* candidate_stats_list) = 0; + virtual bool GetStats(IceTransportStats* ice_transport_stats) = 0; // Returns RTT estimate over the currently active connection, or an empty // absl::optional if there is none. diff --git a/p2p/base/mock_ice_transport.h b/p2p/base/mock_ice_transport.h index a28c796970..1436cacb50 100644 --- a/p2p/base/mock_ice_transport.h +++ b/p2p/base/mock_ice_transport.h @@ -40,9 +40,7 @@ class MockIceTransport : public IceTransportInternal { MOCK_METHOD2(SetOption, int(rtc::Socket::Option opt, int value)); MOCK_METHOD0(GetError, int()); MOCK_CONST_METHOD0(GetIceRole, cricket::IceRole()); - MOCK_METHOD2(GetStats, - bool(cricket::ConnectionInfos* candidate_pair_stats_list, - cricket::CandidateStatsList* candidate_stats_list)); + MOCK_METHOD1(GetStats, bool(cricket::IceTransportStats* ice_transport_stats)); IceTransportState GetState() const override { return IceTransportState::STATE_INIT; diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index f6a8bbc8d7..9e1c7209ff 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -1493,15 +1493,15 @@ int P2PTransportChannel::SendPacket(const char* data, return sent; } -bool P2PTransportChannel::GetStats(ConnectionInfos* candidate_pair_stats_list, - CandidateStatsList* candidate_stats_list) { +bool P2PTransportChannel::GetStats(IceTransportStats* ice_transport_stats) { RTC_DCHECK_RUN_ON(network_thread_); // Gather candidate and candidate pair stats. - candidate_stats_list->clear(); - candidate_pair_stats_list->clear(); + ice_transport_stats->candidate_stats_list.clear(); + ice_transport_stats->connection_infos.clear(); if (!allocator_sessions_.empty()) { - allocator_session()->GetCandidateStatsFromReadyPorts(candidate_stats_list); + allocator_session()->GetCandidateStatsFromReadyPorts( + &ice_transport_stats->candidate_stats_list); } // TODO(qingsi): Remove naming inconsistency for candidate pair/connection. @@ -1510,10 +1510,12 @@ bool P2PTransportChannel::GetStats(ConnectionInfos* candidate_pair_stats_list, stats.local_candidate = SanitizeLocalCandidate(stats.local_candidate); stats.remote_candidate = SanitizeRemoteCandidate(stats.remote_candidate); stats.best_connection = (selected_connection_ == connection); - candidate_pair_stats_list->push_back(std::move(stats)); + ice_transport_stats->connection_infos.push_back(std::move(stats)); connection->set_reported(true); } + ice_transport_stats->selected_candidate_pair_changes = + selected_candidate_pair_changes_; return true; } @@ -1991,6 +1993,8 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn, SignalCandidatePairChanged(pair_change); } SignalNetworkRouteChanged(network_route_); + + ++selected_candidate_pair_changes_; } // Warning: UpdateState should eventually be called whenever a connection diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 6fa64e0055..9f70e6564e 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -131,8 +131,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { int SetOption(rtc::Socket::Option opt, int value) override; bool GetOption(rtc::Socket::Option opt, int* value) override; int GetError() override; - bool GetStats(std::vector* candidate_pair_stats_list, - std::vector* candidate_stats_list) override; + bool GetStats(IceTransportStats* ice_transport_stats) override; absl::optional GetRttEstimate() override; const Connection* selected_connection() const override; absl::optional GetSelectedCandidatePair() const override; @@ -499,6 +498,9 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { void AddRemoteCandidateWithResolver(Candidate candidate, rtc::AsyncResolverInterface* resolver); + // Number of times the selected_connection_ has been modified. + uint32_t selected_candidate_pair_changes_ = 0; + RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel); }; diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 65f7d20eba..72ab65ceac 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -1231,13 +1231,13 @@ TEST_F(P2PTransportChannelTest, GetStats) { ep2_ch1()->writable(), kMediumTimeout, clock); TestSendRecv(&clock); - ConnectionInfos infos; - CandidateStatsList candidate_stats_list; - ASSERT_TRUE(ep1_ch1()->GetStats(&infos, &candidate_stats_list)); - ASSERT_GE(infos.size(), 1u); - ASSERT_GE(candidate_stats_list.size(), 1u); + IceTransportStats ice_transport_stats; + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + ASSERT_GE(ice_transport_stats.connection_infos.size(), 1u); + ASSERT_GE(ice_transport_stats.candidate_stats_list.size(), 1u); + EXPECT_EQ(ice_transport_stats.selected_candidate_pair_changes, 1u); ConnectionInfo* best_conn_info = nullptr; - for (ConnectionInfo& info : infos) { + for (ConnectionInfo& info : ice_transport_stats.connection_infos) { if (info.best_connection) { best_conn_info = &info; break; @@ -1582,13 +1582,16 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveRemoteCandidateIsSanitized) { EXPECT_EQ(PRFLX_PORT_TYPE, pair_ep1->remote_candidate().type()); EXPECT_TRUE(pair_ep1->remote_candidate().address().ipaddr().IsNil()); - ConnectionInfos pair_stats; - CandidateStatsList candidate_stats; - ep1_ch1()->GetStats(&pair_stats, &candidate_stats); + IceTransportStats ice_transport_stats; + ep1_ch1()->GetStats(&ice_transport_stats); // Check the candidate pair stats. - ASSERT_EQ(1u, pair_stats.size()); - EXPECT_EQ(PRFLX_PORT_TYPE, pair_stats[0].remote_candidate.type()); - EXPECT_TRUE(pair_stats[0].remote_candidate.address().ipaddr().IsNil()); + ASSERT_EQ(1u, ice_transport_stats.connection_infos.size()); + EXPECT_EQ(PRFLX_PORT_TYPE, + ice_transport_stats.connection_infos[0].remote_candidate.type()); + EXPECT_TRUE(ice_transport_stats.connection_infos[0] + .remote_candidate.address() + .ipaddr() + .IsNil()); // Let ep1 receive the remote candidate to update its type from prflx to host. ResumeCandidates(1); @@ -1608,12 +1611,14 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveRemoteCandidateIsSanitized) { EXPECT_TRUE( updated_pair_ep1->remote_candidate().address().EqualIPs(kPublicAddrs[1])); - ep1_ch1()->GetStats(&pair_stats, &candidate_stats); + ep1_ch1()->GetStats(&ice_transport_stats); // Check the candidate pair stats. - ASSERT_EQ(1u, pair_stats.size()); - EXPECT_EQ(LOCAL_PORT_TYPE, pair_stats[0].remote_candidate.type()); - EXPECT_TRUE( - pair_stats[0].remote_candidate.address().EqualIPs(kPublicAddrs[1])); + ASSERT_EQ(1u, ice_transport_stats.connection_infos.size()); + EXPECT_EQ(LOCAL_PORT_TYPE, + ice_transport_stats.connection_infos[0].remote_candidate.type()); + EXPECT_TRUE(ice_transport_stats.connection_infos[0] + .remote_candidate.address() + .EqualIPs(kPublicAddrs[1])); DestroyChannels(); } @@ -5010,17 +5015,15 @@ TEST_F(P2PTransportChannelTest, ASSERT_EQ_WAIT(3u, ep1_ch1()->connections().size(), kMediumTimeout); ASSERT_EQ_WAIT(3u, ep2_ch1()->connections().size(), kMediumTimeout); - ConnectionInfos connection_infos_ep1; - CandidateStatsList candidate_stats_list_ep1; - ConnectionInfos connection_infos_ep2; - CandidateStatsList candidate_stats_list_ep2; - ep1_ch1()->GetStats(&connection_infos_ep1, &candidate_stats_list_ep1); - ep2_ch1()->GetStats(&connection_infos_ep2, &candidate_stats_list_ep2); - EXPECT_EQ(3u, connection_infos_ep1.size()); - EXPECT_EQ(3u, candidate_stats_list_ep1.size()); - EXPECT_EQ(3u, connection_infos_ep2.size()); + IceTransportStats ice_transport_stats1; + IceTransportStats ice_transport_stats2; + ep1_ch1()->GetStats(&ice_transport_stats1); + ep2_ch1()->GetStats(&ice_transport_stats2); + EXPECT_EQ(3u, ice_transport_stats1.connection_infos.size()); + EXPECT_EQ(3u, ice_transport_stats1.candidate_stats_list.size()); + EXPECT_EQ(3u, ice_transport_stats2.connection_infos.size()); // Check the stats of ep1 seen by ep1. - for (const auto& connection_info : connection_infos_ep1) { + for (const auto& connection_info : ice_transport_stats1.connection_infos) { const auto& local_candidate = connection_info.local_candidate; if (local_candidate.type() == LOCAL_PORT_TYPE) { EXPECT_TRUE(local_candidate.address().IsUnresolvedIP()); @@ -5037,7 +5040,7 @@ TEST_F(P2PTransportChannelTest, } } // Check the stats of ep1 seen by ep2. - for (const auto& connection_info : connection_infos_ep2) { + for (const auto& connection_info : ice_transport_stats2.connection_infos) { const auto& remote_candidate = connection_info.remote_candidate; if (remote_candidate.type() == LOCAL_PORT_TYPE) { EXPECT_TRUE(remote_candidate.address().IsUnresolvedIP()); @@ -5053,6 +5056,96 @@ TEST_F(P2PTransportChannelTest, DestroyChannels(); } +TEST_F(P2PTransportChannelTest, + ConnectingIncreasesSelectedCandidatePairChanges) { + rtc::ScopedFakeClock clock; + ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); + CreateChannels(); + + IceTransportStats ice_transport_stats; + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + EXPECT_EQ(0u, ice_transport_stats.selected_candidate_pair_changes); + + // Let the channels connect. + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr, + kMediumTimeout, clock); + + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + EXPECT_EQ(1u, ice_transport_stats.selected_candidate_pair_changes); + + DestroyChannels(); +} + +TEST_F(P2PTransportChannelTest, + DisconnectedIncreasesSelectedCandidatePairChanges) { + rtc::ScopedFakeClock clock; + ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); + CreateChannels(); + + IceTransportStats ice_transport_stats; + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + EXPECT_EQ(0u, ice_transport_stats.selected_candidate_pair_changes); + + // Let the channels connect. + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr, + kMediumTimeout, clock); + + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + EXPECT_EQ(1u, ice_transport_stats.selected_candidate_pair_changes); + + // Prune connections and wait for disconnect. + for (Connection* con : ep1_ch1()->connections()) { + con->Prune(); + } + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() == nullptr, + kMediumTimeout, clock); + + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + EXPECT_EQ(2u, ice_transport_stats.selected_candidate_pair_changes); + + DestroyChannels(); +} + +TEST_F(P2PTransportChannelTest, + NewSelectionIncreasesSelectedCandidatePairChanges) { + rtc::ScopedFakeClock clock; + ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); + CreateChannels(); + + IceTransportStats ice_transport_stats; + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + EXPECT_EQ(0u, ice_transport_stats.selected_candidate_pair_changes); + + // Let the channels connect. + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr, + kMediumTimeout, clock); + + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + EXPECT_EQ(1u, ice_transport_stats.selected_candidate_pair_changes); + + // Prune the currently selected connection and wait for selection + // of a new one. + const Connection* selected_connection = ep1_ch1()->selected_connection(); + for (Connection* con : ep1_ch1()->connections()) { + if (con == selected_connection) { + con->Prune(); + } + } + EXPECT_TRUE_SIMULATED_WAIT( + ep1_ch1()->selected_connection() != nullptr && + (ep1_ch1()->GetStats(&ice_transport_stats), + ice_transport_stats.selected_candidate_pair_changes >= 2u), + kMediumTimeout, clock); + + ASSERT_TRUE(ep1_ch1()->GetStats(&ice_transport_stats)); + EXPECT_GE(ice_transport_stats.selected_candidate_pair_changes, 2u); + + DestroyChannels(); +} + // A similar test as above to check the selected candidate pair is sanitized // when it is queried via GetSelectedCandidatePair. TEST_F(P2PTransportChannelTest, diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index 519c6fcfd1..c800232879 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -747,7 +747,7 @@ bool JsepTransport::GetTransportStats(DtlsTransportInternal* dtls_transport, dtls_transport->GetSslCipherSuite(&substats.ssl_cipher_suite); substats.dtls_state = dtls_transport->dtls_state(); if (!dtls_transport->ice_transport()->GetStats( - &substats.connection_infos, &substats.candidate_stats_list)) { + &substats.ice_transport_stats)) { return false; } stats->channel_stats.push_back(substats); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 927155928a..96fdd6c788 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -7251,7 +7251,7 @@ void PeerConnection::ReportBestConnectionState( for (const cricket::TransportChannelStats& channel_stats : stats.channel_stats) { for (const cricket::ConnectionInfo& connection_info : - channel_stats.connection_infos) { + channel_stats.ice_transport_stats.connection_infos) { if (!connection_info.best_connection) { continue; } diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 34911fbef3..9fd0df03c0 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -1258,7 +1258,7 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_n( std::string transport_id = RTCTransportStatsIDFromTransportChannel( transport_name, channel_stats.component); for (const cricket::ConnectionInfo& info : - channel_stats.connection_infos) { + channel_stats.ice_transport_stats.connection_infos) { std::unique_ptr candidate_pair_stats( new RTCIceCandidatePairStats( RTCIceCandidatePairStatsIDFromConnectionInfo(info), @@ -1689,8 +1689,10 @@ void RTCStatsCollector::ProduceTransportStats_n( transport_stats->bytes_received = 0; transport_stats->dtls_state = DtlsTransportStateToRTCDtlsTransportState(channel_stats.dtls_state); + transport_stats->selected_candidate_pair_changes = + channel_stats.ice_transport_stats.selected_candidate_pair_changes; for (const cricket::ConnectionInfo& info : - channel_stats.connection_infos) { + channel_stats.ice_transport_stats.connection_infos) { *transport_stats->bytes_sent += info.sent_total_bytes; *transport_stats->bytes_received += info.recv_total_bytes; if (info.best_connection) { diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index de66b951fa..2efb7e247a 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -1135,35 +1135,35 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidateStats) { // Add candidate pairs to connection. cricket::TransportChannelStats a_transport_channel_stats; - a_transport_channel_stats.connection_infos.push_back( + a_transport_channel_stats.ice_transport_stats.connection_infos.push_back( cricket::ConnectionInfo()); - a_transport_channel_stats.connection_infos[0].local_candidate = - *a_local_host.get(); - a_transport_channel_stats.connection_infos[0].remote_candidate = - *a_remote_srflx.get(); - a_transport_channel_stats.connection_infos.push_back( + a_transport_channel_stats.ice_transport_stats.connection_infos[0] + .local_candidate = *a_local_host.get(); + a_transport_channel_stats.ice_transport_stats.connection_infos[0] + .remote_candidate = *a_remote_srflx.get(); + a_transport_channel_stats.ice_transport_stats.connection_infos.push_back( cricket::ConnectionInfo()); - a_transport_channel_stats.connection_infos[1].local_candidate = - *a_local_prflx.get(); - a_transport_channel_stats.connection_infos[1].remote_candidate = - *a_remote_relay.get(); - a_transport_channel_stats.connection_infos.push_back( + a_transport_channel_stats.ice_transport_stats.connection_infos[1] + .local_candidate = *a_local_prflx.get(); + a_transport_channel_stats.ice_transport_stats.connection_infos[1] + .remote_candidate = *a_remote_relay.get(); + a_transport_channel_stats.ice_transport_stats.connection_infos.push_back( cricket::ConnectionInfo()); - a_transport_channel_stats.connection_infos[2].local_candidate = - *a_local_relay.get(); - a_transport_channel_stats.connection_infos[2].remote_candidate = - *a_remote_relay.get(); + a_transport_channel_stats.ice_transport_stats.connection_infos[2] + .local_candidate = *a_local_relay.get(); + a_transport_channel_stats.ice_transport_stats.connection_infos[2] + .remote_candidate = *a_remote_relay.get(); pc_->AddVoiceChannel("audio", "a"); pc_->SetTransportStats("a", a_transport_channel_stats); cricket::TransportChannelStats b_transport_channel_stats; - b_transport_channel_stats.connection_infos.push_back( + b_transport_channel_stats.ice_transport_stats.connection_infos.push_back( cricket::ConnectionInfo()); - b_transport_channel_stats.connection_infos[0].local_candidate = - *b_local.get(); - b_transport_channel_stats.connection_infos[0].remote_candidate = - *b_remote.get(); + b_transport_channel_stats.ice_transport_stats.connection_infos[0] + .local_candidate = *b_local.get(); + b_transport_channel_stats.ice_transport_stats.connection_infos[0] + .remote_candidate = *b_remote.get(); pc_->AddVideoChannel("video", "b"); pc_->SetTransportStats("b", b_transport_channel_stats); @@ -1225,7 +1225,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { cricket::TransportChannelStats transport_channel_stats; transport_channel_stats.component = cricket::ICE_CANDIDATE_COMPONENT_RTP; - transport_channel_stats.connection_infos.push_back(connection_info); + transport_channel_stats.ice_transport_stats.connection_infos.push_back( + connection_info); pc_->AddVideoChannel("video", kTransportName); pc_->SetTransportStats(kTransportName, transport_channel_stats); @@ -1266,7 +1267,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { EXPECT_TRUE(report->Get(*expected_pair.transport_id)); // Set nominated and "GetStats" again. - transport_channel_stats.connection_infos[0].nominated = true; + transport_channel_stats.ice_transport_stats.connection_infos[0].nominated = + true; pc_->SetTransportStats(kTransportName, transport_channel_stats); report = stats_->GetFreshStatsReport(); expected_pair.nominated = true; @@ -1277,8 +1279,10 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { EXPECT_TRUE(report->Get(*expected_pair.transport_id)); // Set round trip times and "GetStats" again. - transport_channel_stats.connection_infos[0].total_round_trip_time_ms = 7331; - transport_channel_stats.connection_infos[0].current_round_trip_time_ms = 1337; + transport_channel_stats.ice_transport_stats.connection_infos[0] + .total_round_trip_time_ms = 7331; + transport_channel_stats.ice_transport_stats.connection_infos[0] + .current_round_trip_time_ms = 1337; pc_->SetTransportStats(kTransportName, transport_channel_stats); report = stats_->GetFreshStatsReport(); expected_pair.total_round_trip_time = 7.331; @@ -1290,7 +1294,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { EXPECT_TRUE(report->Get(*expected_pair.transport_id)); // Make pair the current pair, clear bandwidth and "GetStats" again. - transport_channel_stats.connection_infos[0].best_connection = true; + transport_channel_stats.ice_transport_stats.connection_infos[0] + .best_connection = true; pc_->SetTransportStats(kTransportName, transport_channel_stats); report = stats_->GetFreshStatsReport(); // |expected_pair.available_[outgoing/incoming]_bitrate| should still be @@ -2066,8 +2071,11 @@ TEST_F(RTCStatsCollectorTest, CollectRTCTransportStats) { rtp_connection_info.recv_total_bytes = 1337; cricket::TransportChannelStats rtp_transport_channel_stats; rtp_transport_channel_stats.component = cricket::ICE_CANDIDATE_COMPONENT_RTP; - rtp_transport_channel_stats.connection_infos.push_back(rtp_connection_info); + rtp_transport_channel_stats.ice_transport_stats.connection_infos.push_back( + rtp_connection_info); rtp_transport_channel_stats.dtls_state = cricket::DTLS_TRANSPORT_NEW; + rtp_transport_channel_stats.ice_transport_stats + .selected_candidate_pair_changes = 1; pc_->SetTransportStats(kTransportName, {rtp_transport_channel_stats}); // Get stats without RTCP, an active connection or certificates. @@ -2080,6 +2088,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCTransportStats) { expected_rtp_transport.bytes_sent = 42; expected_rtp_transport.bytes_received = 1337; expected_rtp_transport.dtls_state = RTCDtlsTransportState::kNew; + expected_rtp_transport.selected_candidate_pair_changes = 1; ASSERT_TRUE(report->Get(expected_rtp_transport.id())); EXPECT_EQ( @@ -2095,7 +2104,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCTransportStats) { cricket::TransportChannelStats rtcp_transport_channel_stats; rtcp_transport_channel_stats.component = cricket::ICE_CANDIDATE_COMPONENT_RTCP; - rtcp_transport_channel_stats.connection_infos.push_back(rtcp_connection_info); + rtcp_transport_channel_stats.ice_transport_stats.connection_infos.push_back( + rtcp_connection_info); rtcp_transport_channel_stats.dtls_state = cricket::DTLS_TRANSPORT_CONNECTING; pc_->SetTransportStats(kTransportName, {rtp_transport_channel_stats, rtcp_transport_channel_stats}); @@ -2110,9 +2120,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCTransportStats) { expected_rtcp_transport.bytes_sent = 1337; expected_rtcp_transport.bytes_received = 42; expected_rtcp_transport.dtls_state = RTCDtlsTransportState::kConnecting; + expected_rtcp_transport.selected_candidate_pair_changes = 0; expected_rtp_transport.rtcp_transport_stats_id = expected_rtcp_transport.id(); - ASSERT_TRUE(report->Get(expected_rtp_transport.id())); EXPECT_EQ( expected_rtp_transport, @@ -2123,7 +2133,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCTransportStats) { report->Get(expected_rtcp_transport.id())->cast_to()); // Get stats with an active connection (selected candidate pair). - rtcp_transport_channel_stats.connection_infos[0].best_connection = true; + rtcp_transport_channel_stats.ice_transport_stats.connection_infos[0] + .best_connection = true; pc_->SetTransportStats(kTransportName, {rtp_transport_channel_stats, rtcp_transport_channel_stats}); diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc index 16ca58c6df..c36566a84c 100644 --- a/pc/rtc_stats_integrationtest.cc +++ b/pc/rtc_stats_integrationtest.cc @@ -959,6 +959,8 @@ class RTCStatsReportVerifier { RTCCertificateStats::kType); verifier.TestMemberIsIDReference(transport.remote_certificate_id, RTCCertificateStats::kType); + verifier.TestMemberIsPositive( + transport.selected_candidate_pair_changes); return verifier.ExpectAllMembersSuccessfullyTested(); } diff --git a/pc/stats_collector.cc b/pc/stats_collector.cc index 260f601a23..a65a5e75f0 100644 --- a/pc/stats_collector.cc +++ b/pc/stats_collector.cc @@ -865,13 +865,13 @@ void StatsCollector::ExtractSessionInfo() { // not paired. Also, the candidate report generated in // AddConnectionInfoReport do not report port stats like StunStats. for (const cricket::CandidateStats& stats : - channel_iter.candidate_stats_list) { + channel_iter.ice_transport_stats.candidate_stats_list) { AddCandidateReport(stats, true); } int connection_id = 0; for (const cricket::ConnectionInfo& info : - channel_iter.connection_infos) { + channel_iter.ice_transport_stats.connection_infos) { StatsReport* connection_report = AddConnectionInfoReport( transport_name, channel_iter.component, connection_id++, channel_report->id(), info); diff --git a/pc/stats_collector_unittest.cc b/pc/stats_collector_unittest.cc index f2abdd0ff0..858e7b6e02 100644 --- a/pc/stats_collector_unittest.cc +++ b/pc/stats_collector_unittest.cc @@ -1296,7 +1296,7 @@ TEST_F(StatsCollectorTest, IceCandidateReport) { connection_info.local_candidate = local; connection_info.remote_candidate = remote; TransportChannelStats channel_stats; - channel_stats.connection_infos.push_back(connection_info); + channel_stats.ice_transport_stats.connection_infos.push_back(connection_info); pc->AddVoiceChannel("audio", kTransportName); pc->SetTransportStats(kTransportName, channel_stats); diff --git a/pc/transport_stats.h b/pc/transport_stats.h index bec1c2b065..4f6ce2a22a 100644 --- a/pc/transport_stats.h +++ b/pc/transport_stats.h @@ -15,6 +15,7 @@ #include #include "p2p/base/dtls_transport_internal.h" +#include "p2p/base/ice_transport_internal.h" #include "p2p/base/port.h" #include "rtc_base/ssl_stream_adapter.h" @@ -26,11 +27,10 @@ struct TransportChannelStats { ~TransportChannelStats(); int component = 0; - CandidateStatsList candidate_stats_list; - ConnectionInfos connection_infos; int srtp_crypto_suite = rtc::SRTP_INVALID_CRYPTO_SUITE; int ssl_cipher_suite = rtc::TLS_NULL_WITH_NULL_NULL; DtlsTransportState dtls_state = DTLS_TRANSPORT_NEW; + IceTransportStats ice_transport_stats; }; // Information about all the channels of a transport. diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc index 4815640de0..9ecb2a5622 100644 --- a/stats/rtcstats_objects.cc +++ b/stats/rtcstats_objects.cc @@ -868,7 +868,8 @@ WEBRTC_RTCSTATS_IMPL(RTCTransportStats, RTCStats, "transport", &dtls_state, &selected_candidate_pair_id, &local_certificate_id, - &remote_certificate_id) + &remote_certificate_id, + &selected_candidate_pair_changes) // clang-format on RTCTransportStats::RTCTransportStats(const std::string& id, @@ -883,7 +884,8 @@ RTCTransportStats::RTCTransportStats(std::string&& id, int64_t timestamp_us) dtls_state("dtlsState"), selected_candidate_pair_id("selectedCandidatePairId"), local_certificate_id("localCertificateId"), - remote_certificate_id("remoteCertificateId") {} + remote_certificate_id("remoteCertificateId"), + selected_candidate_pair_changes("selectedCandidatePairChanges") {} RTCTransportStats::RTCTransportStats(const RTCTransportStats& other) : RTCStats(other.id(), other.timestamp_us()), @@ -893,7 +895,8 @@ RTCTransportStats::RTCTransportStats(const RTCTransportStats& other) dtls_state(other.dtls_state), selected_candidate_pair_id(other.selected_candidate_pair_id), local_certificate_id(other.local_certificate_id), - remote_certificate_id(other.remote_certificate_id) {} + remote_certificate_id(other.remote_certificate_id), + selected_candidate_pair_changes(other.selected_candidate_pair_changes) {} RTCTransportStats::~RTCTransportStats() {}