Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(66)

Unified Diff: components/rappor/rappor_service.cc

Issue 509203002: Create a mechanism for reporting rappor metrics from non-UMA (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rename, add params to rappor.xml Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/rappor/rappor_service.cc
diff --git a/components/rappor/rappor_service.cc b/components/rappor/rappor_service.cc
index 11419f5baaf583af9995ba352ec41b326d73a041..c329ef4b52bbcac421698042cb94520509dbc222 100644
--- a/components/rappor/rappor_service.cc
+++ b/components/rappor/rappor_service.cc
@@ -37,18 +37,10 @@ const char kRapporRolloutFieldTrialName[] = "RapporRollout";
// Constant for the finch parameter name for the server URL
const char kRapporRolloutServerUrlParam[] = "ServerUrl";
-// Constant for the finch parameter name for the server URL
-const char kRapporRolloutRequireUmaParam[] = "RequireUma";
-
// The rappor server's URL.
const char kDefaultServerUrl[] = "https://clients4.google.com/rappor";
-GURL GetServerUrl(bool metrics_enabled) {
- bool require_uma = variations::GetVariationParamValue(
- kRapporRolloutFieldTrialName,
- kRapporRolloutRequireUmaParam) != "False";
- if (!metrics_enabled && require_uma)
- return GURL(); // Invalid URL disables Rappor.
+GURL GetServerUrl() {
std::string server_url = variations::GetVariationParamValue(
kRapporRolloutFieldTrialName,
kRapporRolloutServerUrlParam);
@@ -66,7 +58,17 @@ const RapporParameters kRapporParametersForType[NUM_RAPPOR_TYPES] = {
rappor::PROBABILITY_50 /* Fake data probability */,
rappor::PROBABILITY_50 /* Fake one probability */,
rappor::PROBABILITY_75 /* One coin probability */,
- rappor::PROBABILITY_25 /* Zero coin probability */},
+ rappor::PROBABILITY_25 /* Zero coin probability */,
+ false /* Require UMA */},
+ // FULL_POPULATION_RAPPOR_TYPE
+ {128 /* Num cohorts */,
+ 1 /* Bloom filter size bytes */,
+ 2 /* Bloom filter hash count */,
+ rappor::PROBABILITY_50 /* Fake data probability */,
+ rappor::PROBABILITY_50 /* Fake one probability */,
+ rappor::PROBABILITY_75 /* One coin probability */,
+ rappor::PROBABILITY_25 /* Zero coin probability */,
+ true /* Don't require UMA */},
};
} // namespace
@@ -75,8 +77,9 @@ RapporService::RapporService(PrefService* pref_service)
: pref_service_(pref_service),
cohort_(-1),
daily_event_(pref_service,
- prefs::kRapporLastDailySample,
- kRapporDailyEventHistogram) {
+ prefs::kRapporLastDailySample,
+ kRapporDailyEventHistogram),
+ all_metrics_enabled_(false) {
}
RapporService::~RapporService() {
@@ -90,12 +93,14 @@ void RapporService::AddDailyObserver(
void RapporService::Start(net::URLRequestContextGetter* request_context,
bool metrics_enabled) {
Alexei Svitkine (slow) 2014/11/03 21:48:21 Can you document the 2nd param better in the heade
Steven Holte 2014/11/04 02:44:19 Done.
- const GURL server_url = GetServerUrl(metrics_enabled);
+ const GURL server_url = GetServerUrl();
if (!server_url.is_valid()) {
DVLOG(1) << server_url.spec() << " is invalid. "
<< "RapporService not started.";
return;
}
+ all_metrics_enabled_ = metrics_enabled;
+ DVLOG(1) << "RapporService all_metrics_enabled_? " << all_metrics_enabled_;
DVLOG(1) << "RapporService started. Reporting to " << server_url.spec();
DCHECK(!uploader_);
LoadSecret();
@@ -203,10 +208,15 @@ void RapporService::RecordSample(const std::string& metric_name,
if (!IsInitialized())
return;
DCHECK_LT(type, NUM_RAPPOR_TYPES);
+ const RapporParameters& parameters = kRapporParametersForType[type];
+ // Skip this metric if all metrics aren't enabled and this isn't a
+ // whitelisted metric.
+ if (!all_metrics_enabled_ && !parameters.always_enabled)
Alexei Svitkine (slow) 2014/11/03 21:48:21 Can you add a test for this?
Steven Holte 2014/11/04 02:44:20 Done.
+ return;
DVLOG(2) << "Recording sample \"" << sample
<< "\" for metric \"" << metric_name
<< "\" of type: " << type;
- RecordSampleInternal(metric_name, kRapporParametersForType[type], sample);
+ RecordSampleInternal(metric_name, parameters, sample);
}
void RapporService::RecordSampleInternal(const std::string& metric_name,

Powered by Google App Engine
This is Rietveld 408576698