From c7b964cd71fb3fc937d98f695b5960eb23580059 Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Thu, 1 Feb 2018 14:39:45 -0800 Subject: [PATCH] Report cipher usage to UMA for all media types on a transport Previously, the code which reported cipher stats to UMA for all transports would classify the media type based on the transport name, which is brittle and misleading with BUNDLE. This corrects the code to track all media types (audio, video, data) which use the transport and report once for each. Bug: None Change-Id: I8506f64f0011788b744b8386ac58518a21914b52 Reviewed-on: https://webrtc-review.googlesource.com/47247 Reviewed-by: Taylor Brandstetter Commit-Queue: Steve Anton Cr-Commit-Position: refs/heads/master@{#21863} --- pc/peerconnection.cc | 101 +++++++++++++++++++++++-------------------- pc/peerconnection.h | 3 +- 2 files changed, 57 insertions(+), 47 deletions(-) diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index b4908f5359..4758f4f62c 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -5588,26 +5588,31 @@ void PeerConnection::OnTransportControllerGatheringState( } void PeerConnection::ReportTransportStats() { - // Use a set so we don't report the same stats twice if two channels share - // a transport. - std::set transport_names; - if (voice_channel()) { - transport_names.insert(voice_channel()->transport_name()); - } - if (video_channel()) { - transport_names.insert(video_channel()->transport_name()); + std::map> + media_types_by_transport_name; + for (auto transceiver : transceivers_) { + if (transceiver->internal()->channel()) { + const std::string& transport_name = + transceiver->internal()->channel()->transport_name(); + media_types_by_transport_name[transport_name].insert( + transceiver->internal()->media_type()); + } } if (rtp_data_channel()) { - transport_names.insert(rtp_data_channel()->transport_name()); + media_types_by_transport_name[rtp_data_channel()->transport_name()].insert( + cricket::MEDIA_TYPE_DATA); } if (sctp_transport_name_) { - transport_names.insert(*sctp_transport_name_); + media_types_by_transport_name[*sctp_transport_name_].insert( + cricket::MEDIA_TYPE_DATA); } - for (const auto& name : transport_names) { + for (const auto& entry : media_types_by_transport_name) { + const std::string& transport_name = entry.first; + const std::set media_types = entry.second; cricket::TransportStats stats; - if (transport_controller_->GetStats(name, &stats)) { + if (transport_controller_->GetStats(transport_name, &stats)) { ReportBestConnectionState(stats); - ReportNegotiatedCiphers(stats); + ReportNegotiatedCiphers(stats, media_types); } } } @@ -5616,19 +5621,17 @@ void PeerConnection::ReportTransportStats() { void PeerConnection::ReportBestConnectionState( const cricket::TransportStats& stats) { RTC_DCHECK(metrics_observer()); - for (cricket::TransportChannelStatsList::const_iterator it = - stats.channel_stats.begin(); - it != stats.channel_stats.end(); ++it) { - for (cricket::ConnectionInfos::const_iterator it_info = - it->connection_infos.begin(); - it_info != it->connection_infos.end(); ++it_info) { - if (!it_info->best_connection) { + for (const cricket::TransportChannelStats& channel_stats : + stats.channel_stats) { + for (const cricket::ConnectionInfo& connection_info : + channel_stats.connection_infos) { + if (!connection_info.best_connection) { continue; } PeerConnectionEnumCounterType type = kPeerConnectionEnumCounterMax; - const cricket::Candidate& local = it_info->local_candidate; - const cricket::Candidate& remote = it_info->remote_candidate; + const cricket::Candidate& local = connection_info.local_candidate; + const cricket::Candidate& remote = connection_info.remote_candidate; // Increment the counter for IceCandidatePairType. if (local.protocol() == cricket::TCP_PROTOCOL_NAME || @@ -5664,7 +5667,8 @@ void PeerConnection::ReportBestConnectionState( } void PeerConnection::ReportNegotiatedCiphers( - const cricket::TransportStats& stats) { + const cricket::TransportStats& stats, + const std::set& media_types) { RTC_DCHECK(metrics_observer()); if (!dtls_enabled_ || stats.channel_stats.empty()) { return; @@ -5677,29 +5681,34 @@ void PeerConnection::ReportNegotiatedCiphers( return; } - PeerConnectionEnumCounterType srtp_counter_type; - PeerConnectionEnumCounterType ssl_counter_type; - if (stats.transport_name == cricket::CN_AUDIO) { - srtp_counter_type = kEnumCounterAudioSrtpCipher; - ssl_counter_type = kEnumCounterAudioSslCipher; - } else if (stats.transport_name == cricket::CN_VIDEO) { - srtp_counter_type = kEnumCounterVideoSrtpCipher; - ssl_counter_type = kEnumCounterVideoSslCipher; - } else if (stats.transport_name == cricket::CN_DATA) { - srtp_counter_type = kEnumCounterDataSrtpCipher; - ssl_counter_type = kEnumCounterDataSslCipher; - } else { - RTC_NOTREACHED(); - return; - } - - if (srtp_crypto_suite != rtc::SRTP_INVALID_CRYPTO_SUITE) { - metrics_observer()->IncrementSparseEnumCounter(srtp_counter_type, - srtp_crypto_suite); - } - if (ssl_cipher_suite != rtc::TLS_NULL_WITH_NULL_NULL) { - metrics_observer()->IncrementSparseEnumCounter(ssl_counter_type, - ssl_cipher_suite); + for (cricket::MediaType media_type : media_types) { + PeerConnectionEnumCounterType srtp_counter_type; + PeerConnectionEnumCounterType ssl_counter_type; + switch (media_type) { + case cricket::MEDIA_TYPE_AUDIO: + srtp_counter_type = kEnumCounterAudioSrtpCipher; + ssl_counter_type = kEnumCounterAudioSslCipher; + break; + case cricket::MEDIA_TYPE_VIDEO: + srtp_counter_type = kEnumCounterVideoSrtpCipher; + ssl_counter_type = kEnumCounterVideoSslCipher; + break; + case cricket::MEDIA_TYPE_DATA: + srtp_counter_type = kEnumCounterDataSrtpCipher; + ssl_counter_type = kEnumCounterDataSslCipher; + break; + default: + RTC_NOTREACHED(); + continue; + } + if (srtp_crypto_suite != rtc::SRTP_INVALID_CRYPTO_SUITE) { + metrics_observer()->IncrementSparseEnumCounter(srtp_counter_type, + srtp_crypto_suite); + } + if (ssl_cipher_suite != rtc::TLS_NULL_WITH_NULL_NULL) { + metrics_observer()->IncrementSparseEnumCounter(ssl_counter_type, + ssl_cipher_suite); + } } } diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 8dd40f4597..9523ada0f6 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -882,7 +882,8 @@ class PeerConnection : public PeerConnectionInternal, // Gather the usage of IPv4/IPv6 as best connection. void ReportBestConnectionState(const cricket::TransportStats& stats); - void ReportNegotiatedCiphers(const cricket::TransportStats& stats); + void ReportNegotiatedCiphers(const cricket::TransportStats& stats, + const std::set& media_types); void OnSentPacket_w(const rtc::SentPacket& sent_packet);