Commit f4c09c85 authored by Yue Ru Sun's avatar Yue Ru Sun Committed by Commit Bot

Log the relative size of non-UKM data in uploaded reports

This patch adds the logging of the ratio X/Y where

X is the size of serialized report with all UKM data omitted
Y is the size of the serialized full report

as a percentage to the UMA histogram UKM.ReportSize.NonUkmPercentage.

This is to provide us an estimate on how much of the bandwidth usage
is taken up by UKM data and non-UKM data. (Hopefully the non-UKM part is
small enough and has negligible influence on overall bandwidth usage.)

Change-Id: Ic73349e19736d326b3a585608afec07a855a2687
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2491423Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Commit-Queue: Yue Ru Sun <yrsun@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821251}
parent 59d71d64
...@@ -87,10 +87,13 @@ void MetricsReportingService::LogResponseOrErrorCode(int response_code, ...@@ -87,10 +87,13 @@ void MetricsReportingService::LogResponseOrErrorCode(int response_code,
} }
} }
void MetricsReportingService::LogSuccess(size_t log_size) { void MetricsReportingService::LogSuccessLogSize(size_t log_size) {
UMA_HISTOGRAM_COUNTS_10000("UMA.LogSize.OnSuccess", log_size / 1024); UMA_HISTOGRAM_COUNTS_10000("UMA.LogSize.OnSuccess", log_size / 1024);
} }
void MetricsReportingService::LogSuccessMetadata(
const std::string& staged_log) {}
void MetricsReportingService::LogLargeRejection(size_t log_size) { void MetricsReportingService::LogLargeRejection(size_t log_size) {
} }
......
...@@ -55,7 +55,8 @@ class MetricsReportingService : public ReportingService { ...@@ -55,7 +55,8 @@ class MetricsReportingService : public ReportingService {
void LogResponseOrErrorCode(int response_code, void LogResponseOrErrorCode(int response_code,
int error_code, int error_code,
bool was_https) override; bool was_https) override;
void LogSuccess(size_t log_size) override; void LogSuccessLogSize(size_t log_size) override;
void LogSuccessMetadata(const std::string& staged_log) override;
void LogLargeRejection(size_t log_size) override; void LogLargeRejection(size_t log_size) override;
MetricsLogStore metrics_log_store_; MetricsLogStore metrics_log_store_;
......
...@@ -181,9 +181,11 @@ void ReportingService::OnLogUploadComplete(int response_code, ...@@ -181,9 +181,11 @@ void ReportingService::OnLogUploadComplete(int response_code,
if (log_store()->has_staged_log()) { if (log_store()->has_staged_log()) {
// Provide boolean for error recovery (allow us to ignore response_code). // Provide boolean for error recovery (allow us to ignore response_code).
bool discard_log = false; bool discard_log = false;
const size_t log_size = log_store()->staged_log().length(); const std::string& staged_log = log_store()->staged_log();
const size_t log_size = staged_log.length();
if (upload_succeeded) { if (upload_succeeded) {
LogSuccess(log_size); LogSuccessLogSize(log_size);
LogSuccessMetadata(staged_log);
} else if (log_size > max_retransmit_size_) { } else if (log_size > max_retransmit_size_) {
LogLargeRejection(log_size); LogLargeRejection(log_size);
discard_log = true; discard_log = true;
......
...@@ -90,7 +90,8 @@ class ReportingService { ...@@ -90,7 +90,8 @@ class ReportingService {
virtual void LogResponseOrErrorCode(int response_code, virtual void LogResponseOrErrorCode(int response_code,
int error_code, int error_code,
bool was_https) {} bool was_https) {}
virtual void LogSuccess(size_t log_size) {} virtual void LogSuccessLogSize(size_t log_size) {}
virtual void LogSuccessMetadata(const std::string& staged_log) {}
virtual void LogLargeRejection(size_t log_size) {} virtual void LogLargeRejection(size_t log_size) {}
// If recording is enabled, begins uploading the next completed log from // If recording is enabled, begins uploading the next completed log from
......
...@@ -13,9 +13,10 @@ ...@@ -13,9 +13,10 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "components/metrics/metrics_service_client.h" #include "components/metrics/metrics_service_client.h"
#include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_registry_simple.h"
#include "components/ukm/unsent_log_store_metrics_impl.h"
#include "components/ukm/ukm_pref_names.h" #include "components/ukm/ukm_pref_names.h"
#include "components/ukm/ukm_service.h" #include "components/ukm/ukm_service.h"
#include "components/ukm/unsent_log_store_metrics_impl.h"
#include "third_party/zlib/google/compression_utils.h"
namespace ukm { namespace ukm {
...@@ -106,10 +107,40 @@ void UkmReportingService::LogResponseOrErrorCode(int response_code, ...@@ -106,10 +107,40 @@ void UkmReportingService::LogResponseOrErrorCode(int response_code,
response_code >= 0 ? response_code : error_code); response_code >= 0 ? response_code : error_code);
} }
void UkmReportingService::LogSuccess(size_t log_size) { void UkmReportingService::LogSuccessLogSize(size_t log_size) {
UMA_HISTOGRAM_COUNTS_10000("UKM.LogSize.OnSuccess", log_size / 1024); UMA_HISTOGRAM_COUNTS_10000("UKM.LogSize.OnSuccess", log_size / 1024);
} }
void UkmReportingService::LogSuccessMetadata(const std::string& staged_log) {
// Recover the report from the compressed staged log.
std::string uncompressed_log_data;
bool uncompress_successful =
compression::GzipUncompress(staged_log, &uncompressed_log_data);
DCHECK(uncompress_successful);
Report report;
report.ParseFromString(uncompressed_log_data);
// Log the relative size of the report with relevant UKM data omitted. This
// helps us to estimate the bandwidth usage of logs upload that is not
// directly attributed to UKM data, for example the system profile info.
// Note that serialized logs are further compressed before upload, thus the
// percentages here are not the exact percentage of bandwidth they ended up
// taking.
std::string log_without_ukm_data;
report.clear_sources();
report.clear_source_counts();
report.clear_entries();
report.clear_aggregates();
report.SerializeToString(&log_without_ukm_data);
int non_ukm_percentage =
log_without_ukm_data.length() * 100 / uncompressed_log_data.length();
DCHECK_GE(non_ukm_percentage, 0);
DCHECK_LE(non_ukm_percentage, 100);
base::UmaHistogramPercentage("UKM.ReportSize.NonUkmPercentage",
non_ukm_percentage);
}
void UkmReportingService::LogLargeRejection(size_t log_size) {} void UkmReportingService::LogLargeRejection(size_t log_size) {}
} // namespace ukm } // namespace ukm
...@@ -12,8 +12,9 @@ ...@@ -12,8 +12,9 @@
#include <string> #include <string>
#include "base/macros.h" #include "base/macros.h"
#include "components/metrics/unsent_log_store.h"
#include "components/metrics/reporting_service.h" #include "components/metrics/reporting_service.h"
#include "components/metrics/unsent_log_store.h"
#include "third_party/metrics_proto/ukm/report.pb.h"
class PrefService; class PrefService;
class PrefRegistrySimple; class PrefRegistrySimple;
...@@ -56,7 +57,8 @@ class UkmReportingService : public metrics::ReportingService { ...@@ -56,7 +57,8 @@ class UkmReportingService : public metrics::ReportingService {
void LogResponseOrErrorCode(int response_code, void LogResponseOrErrorCode(int response_code,
int error_code, int error_code,
bool was_https) override; bool was_https) override;
void LogSuccess(size_t log_size) override; void LogSuccessLogSize(size_t log_size) override;
void LogSuccessMetadata(const std::string& staged_log) override;
void LogLargeRejection(size_t log_size) override; void LogLargeRejection(size_t log_size) override;
metrics::UnsentLogStore unsent_log_store_; metrics::UnsentLogStore unsent_log_store_;
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "base/metrics/field_trial_params.h" #include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/rand_util.h" #include "base/rand_util.h"
#include "base/threading/sequenced_task_runner_handle.h" #include "base/threading/sequenced_task_runner_handle.h"
...@@ -153,11 +154,8 @@ void PurgeExtensionDataFromUnsentLogStore( ...@@ -153,11 +154,8 @@ void PurgeExtensionDataFromUnsentLogStore(
}, },
report.entries(), report.mutable_entries()); report.entries(), report.mutable_entries());
std::string reserialized_log_data; std::string reserialized_log_data =
report.SerializeToString(&reserialized_log_data); UkmService::SerializeReportProtoToString(&report);
// This allows catching errors with bad UKM serialization we've seen before
// that would otherwise only be noticed on the server.
DCHECK(UkmService::LogCanBeParsed(reserialized_log_data));
// Replace the compressed log in the store by its filtered version. // Replace the compressed log in the store by its filtered version.
const std::string old_compressed_log_data = const std::string old_compressed_log_data =
...@@ -181,13 +179,23 @@ bool UkmService::LogCanBeParsed(const std::string& serialized_data) { ...@@ -181,13 +179,23 @@ bool UkmService::LogCanBeParsed(const std::string& serialized_data) {
bool report_parse_successful = report.ParseFromString(serialized_data); bool report_parse_successful = report.ParseFromString(serialized_data);
if (!report_parse_successful) if (!report_parse_successful)
return false; return false;
// Make sure the reserialzed log from this |report| matches the input // Make sure the reserialized log from this |report| matches the input
// |serialized_data|. // |serialized_data|.
std::string reserialized_from_report; std::string reserialized_from_report;
report.SerializeToString(&reserialized_from_report); report.SerializeToString(&reserialized_from_report);
return reserialized_from_report == serialized_data; return reserialized_from_report == serialized_data;
} }
std::string UkmService::SerializeReportProtoToString(Report* report) {
std::string serialized_full_log;
report->SerializeToString(&serialized_full_log);
// This allows catching errors with bad UKM serialization we've seen before
// that would otherwise only be noticed on the server.
DCHECK(UkmService::LogCanBeParsed(serialized_full_log));
return serialized_full_log;
}
UkmService::UkmService(PrefService* pref_service, UkmService::UkmService(PrefService* pref_service,
metrics::MetricsServiceClient* client, metrics::MetricsServiceClient* client,
std::unique_ptr<metrics::UkmDemographicMetricsProvider> std::unique_ptr<metrics::UkmDemographicMetricsProvider>
...@@ -418,11 +426,8 @@ void UkmService::BuildAndStoreLog() { ...@@ -418,11 +426,8 @@ void UkmService::BuildAndStoreLog() {
AddSyncedUserNoiseBirthYearAndGenderToReport(&report); AddSyncedUserNoiseBirthYearAndGenderToReport(&report);
std::string serialized_log; std::string serialized_log =
report.SerializeToString(&serialized_log); UkmService::SerializeReportProtoToString(&report);
// This allows catching errors with bad UKM serialization we've seen before
// that would otherwise only be noticed on the server.
DCHECK(LogCanBeParsed(serialized_log));
reporting_service_.ukm_log_store()->StoreLog(serialized_log, base::nullopt); reporting_service_.ukm_log_store()->StoreLog(serialized_log, base::nullopt);
} }
......
...@@ -117,9 +117,12 @@ class UkmService : public UkmRecorderImpl { ...@@ -117,9 +117,12 @@ class UkmService : public UkmRecorderImpl {
// components/metrics/demographics/demographic_metrics_provider.h. // components/metrics/demographics/demographic_metrics_provider.h.
static const base::Feature kReportUserNoisedUserBirthYearAndGender; static const base::Feature kReportUserNoisedUserBirthYearAndGender;
// Makes sure that the serialized ukm report can be parsed. // Makes sure that the serialized UKM report can be parsed.
static bool LogCanBeParsed(const std::string& serialized_data); static bool LogCanBeParsed(const std::string& serialized_data);
// Serializes the input UKM report into a string and validates it.
static std::string SerializeReportProtoToString(Report* report);
private: private:
friend ::metrics::UkmBrowserTestBase; friend ::metrics::UkmBrowserTestBase;
friend ::ukm::UkmTestHelper; friend ::ukm::UkmTestHelper;
......
...@@ -196,6 +196,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -196,6 +196,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary> </summary>
</histogram> </histogram>
<histogram name="UKM.ReportSize.NonUkmPercentage" units="%"
expires_after="2021-07-01">
<owner>yrsun@chromium.org</owner>
<owner>ukm-team@google.com</owner>
<summary>
Percentage that data non-directly attributed (e.g. system profile info, etc)
to UKM take up in a serialized ukm::Report protocol buffer, before
compression.
</summary>
</histogram>
<histogram name="UKM.ResetReason" enum="UkmResetReason" <histogram name="UKM.ResetReason" enum="UkmResetReason"
expires_after="2021-07-01"> expires_after="2021-07-01">
<owner>rkaplow@chromium.org</owner> <owner>rkaplow@chromium.org</owner>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment