Skip to content

Commit 3b01859

Browse files
author
Henrik Grunell
committed
UMA stats for WebRTC text logging.
Adds stats for start, discard, upload started, upload succeeded, and upload failed. Bug: 809903 Change-Id: I330cff6f975ddfa93a189d3de131627cb7249155 Reviewed-on: https://chromium-review.googlesource.com/c/1354446 Reviewed-by: Tommi <[email protected]> Reviewed-by: Jesse Doherty <[email protected]> Reviewed-by: Oskar Sundbom <[email protected]> Commit-Queue: Henrik Grunell <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#613912}(cherry picked from commit e94a2eb) Reviewed-on: https://chromium-review.googlesource.com/c/1365243 Reviewed-by: Henrik Grunell <[email protected]> Cr-Commit-Position: refs/branch-heads/3626@{#107} Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
1 parent b0656cb commit 3b01859

File tree

8 files changed

+88
-2
lines changed

8 files changed

+88
-2
lines changed

chrome/browser/media/webrtc/webrtc_log_uploader.cc

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "base/files/file_path.h"
1111
#include "base/files/file_util.h"
1212
#include "base/logging.h"
13+
#include "base/metrics/histogram_functions.h"
1314
#include "base/pickle.h"
1415
#include "base/strings/string_number_conversions.h"
1516
#include "base/strings/string_split.h"
@@ -189,6 +190,8 @@ void WebRtcLogUploader::UploadStoredLog(
189190
std::string compressed_log;
190191
if (!base::ReadFileToString(native_log_path, &compressed_log)) {
191192
DPLOG(WARNING) << "Could not read WebRTC log file.";
193+
base::UmaHistogramSparse("WebRtcTextLogging.UploadFailed",
194+
upload_data.web_app_id);
192195
base::PostTaskWithTraits(
193196
FROM_HERE, {BrowserThread::UI},
194197
base::BindOnce(upload_data.callback, false, "", "Log doesn't exist."));
@@ -473,6 +476,9 @@ void WebRtcLogUploader::UploadCompressedLog(
473476

474477
DecreaseLogCount();
475478

479+
// We don't log upload failure to UMA in case of shutting down for
480+
// consistency, since there are other cases during shutdown were we don't get
481+
// a chance to log.
476482
if (shutting_down_)
477483
return;
478484

@@ -641,7 +647,12 @@ void WebRtcLogUploader::NotifyUploadDone(
641647
if (!upload_done_data.callback.is_null()) {
642648
bool success = response_code == net::HTTP_OK;
643649
std::string error_message;
644-
if (!success) {
650+
if (success) {
651+
base::UmaHistogramSparse("WebRtcTextLogging.UploadSuccessful",
652+
upload_done_data.web_app_id);
653+
} else {
654+
base::UmaHistogramSparse("WebRtcTextLogging.UploadFailed",
655+
upload_done_data.web_app_id);
645656
error_message = "Uploading failed, response code: " +
646657
base::IntToString(response_code);
647658
}

chrome/browser/media/webrtc/webrtc_log_uploader.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ struct WebRtcLogUploadDoneData : public WebRtcLogPaths {
3434
WebRtcLoggingHandlerHost::UploadDoneCallback callback;
3535
scoped_refptr<WebRtcLoggingHandlerHost> host;
3636
std::string local_log_id;
37+
// Used for statistics. See |WebRtcLoggingHandlerHost::web_app_id_|.
38+
int web_app_id;
3739
};
3840

3941
// WebRtcLogUploader uploads WebRTC logs, keeps count of how many logs have

chrome/browser/media/webrtc/webrtc_logging_handler_host.cc

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "base/command_line.h"
1212
#include "base/files/file_util.h"
1313
#include "base/logging.h"
14+
#include "base/metrics/histogram_functions.h"
1415
#include "base/task/post_task.h"
1516
#include "build/build_config.h"
1617
#include "chrome/browser/bad_message.h"
@@ -65,6 +66,15 @@ void WebRtcLoggingHandlerHost::SetMetaData(
6566
DCHECK_CURRENTLY_ON(BrowserThread::IO);
6667
DCHECK(!callback.is_null());
6768

69+
// Set the web app ID if there's a "client" key, otherwise leave it unchanged.
70+
for (const auto& it : *meta_data) {
71+
if (it.first == "client") {
72+
web_app_id_ = static_cast<int>(base::PersistentHash(it.second));
73+
text_log_handler_->SetWebAppId(web_app_id_);
74+
break;
75+
}
76+
}
77+
6878
text_log_handler_->SetMetaData(std::move(meta_data), callback);
6979
}
7080

@@ -106,6 +116,8 @@ void WebRtcLoggingHandlerHost::UploadLog(const UploadDoneCallback& callback) {
106116
// Would it be better to upload whatever logs we have, or would the lack of
107117
// an error callback make it harder to debug potential errors?
108118

119+
base::UmaHistogramSparse("WebRtcTextLogging.UploadStarted", web_app_id_);
120+
109121
base::PostTaskAndReplyWithResult(
110122
log_uploader_->background_task_runner().get(), FROM_HERE,
111123
base::Bind(&WebRtcLoggingHandlerHost::GetLogDirectoryAndEnsureExists,
@@ -119,14 +131,18 @@ void WebRtcLoggingHandlerHost::UploadStoredLog(
119131
DCHECK_CURRENTLY_ON(BrowserThread::IO);
120132
DCHECK(!callback.is_null());
121133

134+
base::UmaHistogramSparse("WebRtcTextLogging.UploadStoredStarted",
135+
web_app_id_);
136+
122137
log_uploader_->background_task_runner()->PostTask(
123138
FROM_HERE,
124139
base::BindOnce(&WebRtcLoggingHandlerHost::UploadStoredLogOnFileThread,
125-
this, log_id, callback));
140+
this, log_id, web_app_id_, callback));
126141
}
127142

128143
void WebRtcLoggingHandlerHost::UploadStoredLogOnFileThread(
129144
const std::string& log_id,
145+
int web_app_id,
130146
const UploadDoneCallback& callback) {
131147
DCHECK(log_uploader_->background_task_runner()->RunsTasksInCurrentSequence());
132148

@@ -135,6 +151,7 @@ void WebRtcLoggingHandlerHost::UploadStoredLogOnFileThread(
135151
upload_data.callback = callback;
136152
upload_data.host = this;
137153
upload_data.local_log_id = log_id;
154+
upload_data.web_app_id = web_app_id;
138155

139156
log_uploader_->UploadStoredLog(upload_data);
140157
}
@@ -507,6 +524,13 @@ void WebRtcLoggingHandlerHost::DoUploadLogAndRtpDumps(
507524
text_logging_state != WebRtcTextLogHandler::STOPPED) ||
508525
(channel_is_closing &&
509526
text_log_handler_->GetState() == WebRtcTextLogHandler::CLOSED)) {
527+
// If the channel is not closing the log is expected to be uploaded, so
528+
// it's considered a failure if it isn't.
529+
// If the channel is closing we don't log failure to UMA for consistency,
530+
// since there are other cases during shutdown were we don't get a chance
531+
// to log.
532+
if (!channel_is_closing)
533+
base::UmaHistogramSparse("WebRtcTextLogging.UploadFailed", web_app_id_);
510534
base::PostTaskWithTraits(
511535
FROM_HERE, {content::BrowserThread::UI},
512536
base::BindOnce(callback, false, "",
@@ -518,6 +542,7 @@ void WebRtcLoggingHandlerHost::DoUploadLogAndRtpDumps(
518542
upload_done_data.log_path = log_directory;
519543
upload_done_data.callback = callback;
520544
upload_done_data.host = this;
545+
upload_done_data.web_app_id = web_app_id_;
521546
ReleaseRtpDumps(&upload_done_data);
522547

523548
std::unique_ptr<WebRtcLogBuffer> log_buffer;

chrome/browser/media/webrtc/webrtc_logging_handler_host.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ class WebRtcLoggingHandlerHost : public content::BrowserMessageFilter {
187187
const base::FilePath& directory);
188188

189189
void UploadStoredLogOnFileThread(const std::string& log_id,
190+
int web_app_id,
190191
const UploadDoneCallback& callback);
191192

192193
// A helper for TriggerUpload to do the real work.
@@ -250,6 +251,11 @@ class WebRtcLoggingHandlerHost : public content::BrowserMessageFilter {
250251
// Ownership lies with the browser process.
251252
WebRtcLogUploader* const log_uploader_;
252253

254+
// Web app id used for statistics. Created as the hash of the value of a
255+
// "client" meta data key, if exists. 0 means undefined, and is the hash of
256+
// the empty string. Must only be accessed on the IO thread.
257+
int web_app_id_ = 0;
258+
253259
DISALLOW_COPY_AND_ASSIGN(WebRtcLoggingHandlerHost);
254260
};
255261

chrome/browser/media/webrtc/webrtc_text_log_handler.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "base/cpu.h"
1414
#include "base/feature_list.h"
1515
#include "base/logging.h"
16+
#include "base/metrics/histogram_functions.h"
1617
#include "base/strings/strcat.h"
1718
#include "base/strings/string_number_conversions.h"
1819
#include "base/strings/stringprintf.h"
@@ -243,6 +244,8 @@ void WebRtcTextLogHandler::StartDone(const GenericDoneCallback& callback) {
243244

244245
DCHECK_EQ(STARTING, logging_state_);
245246

247+
base::UmaHistogramSparse("WebRtcTextLogging.Start", web_app_id_);
248+
246249
logging_started_time_ = base::Time::Now();
247250
logging_state_ = STARTED;
248251
FireGenericDoneCallback(callback, true, "");
@@ -306,6 +309,8 @@ void WebRtcTextLogHandler::DiscardLog() {
306309
DCHECK(logging_state_ == STOPPED ||
307310
(channel_is_closing_ && logging_state_ != CLOSED));
308311

312+
base::UmaHistogramSparse("WebRtcTextLogging.Discard", web_app_id_);
313+
309314
log_buffer_.reset();
310315
meta_data_.reset();
311316
logging_state_ = LoggingState::CLOSED;
@@ -410,6 +415,11 @@ void WebRtcTextLogHandler::FireGenericDoneCallback(
410415
base::BindOnce(callback, success, error_message_with_state));
411416
}
412417

418+
void WebRtcTextLogHandler::SetWebAppId(int web_app_id) {
419+
DCHECK_CURRENTLY_ON(BrowserThread::IO);
420+
web_app_id_ = web_app_id;
421+
}
422+
413423
void WebRtcTextLogHandler::LogInitialInfoOnIOThread(
414424
const GenericDoneCallback& callback,
415425
const net::NetworkInterfaceList& network_list) {

chrome/browser/media/webrtc/webrtc_text_log_handler.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ class WebRtcTextLogHandler
133133
bool success,
134134
const std::string& error_message);
135135

136+
// Must be called on the IO thread.
137+
void SetWebAppId(int web_app_id);
138+
136139
private:
137140
friend class base::RefCountedThreadSafe<WebRtcTextLogHandler>;
138141
~WebRtcTextLogHandler();
@@ -172,6 +175,11 @@ class WebRtcTextLogHandler
172175
// changes to STOPPED.
173176
base::Time logging_started_time_;
174177

178+
// Web app id used for statistics. See
179+
// |WebRtcLoggingHandlerHost::web_app_id_|. Must only be accessed on the IO
180+
// thread.
181+
int web_app_id_ = 0;
182+
175183
DISALLOW_COPY_AND_ASSIGN(WebRtcTextLogHandler);
176184
};
177185

tools/metrics/histograms/enums.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53420,6 +53420,11 @@ Full version information for the fingerprint enum values:
5342053420
<int value="1" label="Error"/>
5342153421
</enum>
5342253422

53423+
<enum name="WebRtcLoggingWebAppIdHash">
53424+
<int value="-100222544" label="Hangouts"/>
53425+
<int value="0" label="Undefined"/>
53426+
</enum>
53427+
5342353428
<enum name="WebRtcNativeRate">
5342453429
<int value="0" label="8kHz"/>
5342553430
<int value="1" label="16kHz"/>

tools/metrics/histograms/histograms.xml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126115,6 +126115,15 @@ uploading your change for review.
126115126115
<summary>Time for capturing one frame in window capturing.</summary>
126116126116
</histogram>
126117126117

126118+
<histogram base="true" name="WebRtcTextLogging"
126119+
enum="WebRtcLoggingWebAppIdHash">
126120+
<owner>[email protected]</owner>
126121+
<summary>
126122+
Counts the number of WebRTC text log events per web application. Suffixed by
126123+
event.
126124+
</summary>
126125+
</histogram>
126126+
126118126127
<histogram name="WebShare.ApiCount" enum="WebShareMethod">
126119126128
<owner>[email protected]</owner>
126120126129
<summary>
@@ -140032,6 +140041,16 @@ uploading your change for review.
140032140041
<affected-histogram name="WebRTC.Audio.EchoCanceller.SuppressorGainBand1"/>
140033140042
</histogram_suffixes>
140034140043

140044+
<histogram_suffixes name="WebRtcLoggingEvent" separator=".">
140045+
<suffix name="Discard" label="Discard"/>
140046+
<suffix name="Start" label="Start"/>
140047+
<suffix name="UploadFailed" label="Upload failed"/>
140048+
<suffix name="UploadStarted" label="Upload started"/>
140049+
<suffix name="UploadStoredStarted" label="Upload of a stored log started"/>
140050+
<suffix name="UploadSuccessful" label="Upload successful"/>
140051+
<affected-histogram name="WebRtcTextLogging"/>
140052+
</histogram_suffixes>
140053+
140035140054
<histogram_suffixes name="WebRTCMediaType" separator=".">
140036140055
<suffix name="Audio" label="Audio"/>
140037140056
<suffix name="Data" label="Data"/>

0 commit comments

Comments
 (0)