From 8f4338495d7994faae2eb2db1edee9969e3c7d7f Mon Sep 17 00:00:00 2001 From: Jonas Olsson Date: Mon, 25 Mar 2019 10:10:31 +0100 Subject: [PATCH] Add probing histograms. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL adds a handful of histograms that track how many probes we send, and some details about them. We'll track the total number of requested probe clusters and how many of them timed out, recorded when we destroy the prober. For the successful clusters we'll also track how many bytes it took, how many packets we sent and how long it took to send. Bug: webrtc:10413 Change-Id: I4a223caa6d3c2829a36788d4fa615a41286b183f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/127884 Reviewed-by: Åsa Persson Commit-Queue: Jonas Olsson Cr-Commit-Position: refs/heads/master@{#27270} --- modules/pacing/BUILD.gn | 1 + modules/pacing/bitrate_prober.cc | 23 +++++++++++++++++++++-- modules/pacing/bitrate_prober.h | 3 +++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/modules/pacing/BUILD.gn b/modules/pacing/BUILD.gn index 00fde9ec2a..26e60c97fc 100644 --- a/modules/pacing/BUILD.gn +++ b/modules/pacing/BUILD.gn @@ -42,6 +42,7 @@ rtc_static_library("pacing") { "../../rtc_base/experiments:field_trial_parser", "../../system_wrappers", "../../system_wrappers:field_trial", + "../../system_wrappers:metrics", "../congestion_controller/goog_cc:alr_detector", "../remote_bitrate_estimator", "../rtp_rtcp", diff --git a/modules/pacing/bitrate_prober.cc b/modules/pacing/bitrate_prober.cc index 890eb7be33..c81a18c3b4 100644 --- a/modules/pacing/bitrate_prober.cc +++ b/modules/pacing/bitrate_prober.cc @@ -18,6 +18,7 @@ #include "logging/rtc_event_log/rtc_event_log.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "system_wrappers/include/metrics.h" namespace webrtc { @@ -47,11 +48,19 @@ constexpr int64_t kProbeClusterTimeoutMs = 5000; BitrateProber::BitrateProber() : BitrateProber(nullptr) {} -BitrateProber::~BitrateProber() = default; +BitrateProber::~BitrateProber() { + RTC_HISTOGRAM_COUNTS_1000("WebRTC.BWE.Probing.TotalProbeClustersRequested", + total_probe_count_); + RTC_HISTOGRAM_COUNTS_1000("WebRTC.BWE.Probing.TotalFailedProbeClusters", + total_failed_probe_count_); +} // TODO(psla): Remove this constructor in a follow up change. BitrateProber::BitrateProber(RtcEventLog* event_log) - : probing_state_(ProbingState::kDisabled), next_probe_time_ms_(-1) { + : probing_state_(ProbingState::kDisabled), + next_probe_time_ms_(-1), + total_probe_count_(0), + total_failed_probe_count_(0) { SetEnabled(true); } @@ -88,9 +97,12 @@ void BitrateProber::CreateProbeCluster(int bitrate_bps, int cluster_id) { RTC_DCHECK(probing_state_ != ProbingState::kDisabled); RTC_DCHECK_GT(bitrate_bps, 0); + + total_probe_count_++; while (!clusters_.empty() && now_ms - clusters_.front().time_created_ms > kProbeClusterTimeoutMs) { clusters_.pop(); + total_failed_probe_count_++; } ProbeCluster cluster; @@ -162,6 +174,13 @@ void BitrateProber::ProbeSent(int64_t now_ms, size_t bytes) { next_probe_time_ms_ = GetNextProbeTime(*cluster); if (cluster->sent_bytes >= cluster->pace_info.probe_cluster_min_bytes && cluster->sent_probes >= cluster->pace_info.probe_cluster_min_probes) { + RTC_HISTOGRAM_COUNTS_100000("WebRTC.BWE.Probing.ProbeClusterSizeInBytes", + cluster->sent_bytes); + RTC_HISTOGRAM_COUNTS_100("WebRTC.BWE.Probing.ProbesPerCluster", + cluster->sent_probes); + RTC_HISTOGRAM_COUNTS_10000("WebRTC.BWE.Probing.TimePerProbeCluster", + now_ms - cluster->time_started_ms); + clusters_.pop(); } if (clusters_.empty()) diff --git a/modules/pacing/bitrate_prober.h b/modules/pacing/bitrate_prober.h index a8e964c08c..e970eee1d1 100644 --- a/modules/pacing/bitrate_prober.h +++ b/modules/pacing/bitrate_prober.h @@ -98,6 +98,9 @@ class BitrateProber { // Time the next probe should be sent when in kActive state. int64_t next_probe_time_ms_; + + int total_probe_count_; + int total_failed_probe_count_; }; } // namespace webrtc