diff options
-rw-r--r-- | components/rappor/bloom_filter.h | 1 | ||||
-rw-r--r-- | components/rappor/byte_vector_utils.h | 1 | ||||
-rw-r--r-- | components/rappor/log_uploader.cc | 6 | ||||
-rw-r--r-- | components/rappor/log_uploader.h | 20 | ||||
-rw-r--r-- | components/rappor/log_uploader_unittest.cc | 1 | ||||
-rw-r--r-- | components/rappor/rappor_metric.cc | 2 | ||||
-rw-r--r-- | components/rappor/rappor_metric.h | 23 | ||||
-rw-r--r-- | components/rappor/rappor_metric_unittest.cc | 25 | ||||
-rw-r--r-- | components/rappor/rappor_service.cc | 38 | ||||
-rw-r--r-- | components/rappor/rappor_service.h | 26 | ||||
-rw-r--r-- | components/rappor/rappor_service_unittest.cc | 6 |
11 files changed, 90 insertions, 59 deletions
diff --git a/components/rappor/bloom_filter.h b/components/rappor/bloom_filter.h index 0bb9a57..d9acfa4 100644 --- a/components/rappor/bloom_filter.h +++ b/components/rappor/bloom_filter.h @@ -8,6 +8,7 @@ #include <string> #include "base/basictypes.h" +#include "base/macros.h" #include "components/rappor/byte_vector_utils.h" namespace rappor { diff --git a/components/rappor/byte_vector_utils.h b/components/rappor/byte_vector_utils.h index 9154cab..0e33041 100644 --- a/components/rappor/byte_vector_utils.h +++ b/components/rappor/byte_vector_utils.h @@ -8,6 +8,7 @@ #include <vector> #include "base/basictypes.h" +#include "base/macros.h" #include "components/rappor/rappor_parameters.h" #include "crypto/hmac.h" diff --git a/components/rappor/log_uploader.cc b/components/rappor/log_uploader.cc index 4837ba1..ead1a97 100644 --- a/components/rappor/log_uploader.cc +++ b/components/rappor/log_uploader.cc @@ -102,12 +102,12 @@ void LogUploader::OnURLFetchComplete(const net::URLFetcher* source) { DCHECK_EQ(current_fetch_.get(), source); scoped_ptr<net::URLFetcher> fetch(current_fetch_.Pass()); - int response_code = source->GetResponseCode(); + const int response_code = source->GetResponseCode(); // Log a histogram to track response success vs. failure rates. UMA_HISTOGRAM_SPARSE_SLOWLY("Rappor.UploadResponseCode", response_code); - bool upload_succeeded = response_code == 200; + const bool upload_succeeded = response_code == 200; // Determine whether this log should be retransmitted. DiscardReason reason = NUM_DISCARD_REASONS; @@ -128,7 +128,7 @@ void LogUploader::OnURLFetchComplete(const net::URLFetcher* source) { // Error 400 indicates a problem with the log, not with the server, so // don't consider that a sign that the server is in trouble. - bool server_is_healthy = upload_succeeded || response_code == 400; + const bool server_is_healthy = upload_succeeded || response_code == 400; OnUploadFinished(server_is_healthy, !queued_logs_.empty()); } diff --git a/components/rappor/log_uploader.h b/components/rappor/log_uploader.h index 30a129a..a446a34 100644 --- a/components/rappor/log_uploader.h +++ b/components/rappor/log_uploader.h @@ -8,6 +8,8 @@ #include <queue> #include <string> +#include "base/compiler_specific.h" +#include "base/macros.h" #include "base/memory/scoped_ptr.h" #include "base/time/time.h" #include "base/timer/timer.h" @@ -21,16 +23,22 @@ class URLFetcher; namespace rappor { -// Handles uploading logs to an external server. +// Uploads logs from RapporService. Logs are passed in via QueueLog(), stored +// internally, and uploaded one at a time. A queued log will be uploaded at a +// fixed interval after the successful upload of the previous logs. If an +// upload fails, the uploader will keep retrying the upload with an exponential +// backoff interval. class LogUploader : public net::URLFetcherDelegate { public: - // Constructor takes the server_url that logs should be uploaded to, the - // mime_type of the uploaded data, and request_context to create uploads + // Constructor takes the |server_url| that logs should be uploaded to, the + // |mime_type| of the uploaded data, and |request_context| to create uploads // with. LogUploader(const GURL& server_url, const std::string& mime_type, net::URLRequestContextGetter* request_context); + // If the object is destroyed (or the program terminates) while logs are + // queued, the logs are lost. virtual ~LogUploader(); // Adds an entry to the queue of logs to be uploaded to the server. The @@ -39,7 +47,7 @@ class LogUploader : public net::URLFetcherDelegate { void QueueLog(const std::string& log); protected: - // Check if an upload has been scheduled. + // Checks if an upload has been scheduled. virtual bool IsUploadScheduled() const; // Schedules a future call to StartScheduledUpload if one isn't already @@ -54,8 +62,8 @@ class LogUploader : public net::URLFetcherDelegate { static base::TimeDelta BackOffUploadInterval(base::TimeDelta); private: - // Implementation of net::URLFetcherDelegate. Called after transmission - // completes (either successfully or with failure). + // Implements net::URLFetcherDelegate. Called after transmission completes + // (whether successful or not). virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE; // Called when the upload is completed. diff --git a/components/rappor/log_uploader_unittest.cc b/components/rappor/log_uploader_unittest.cc index 46db7db..8c175b4 100644 --- a/components/rappor/log_uploader_unittest.cc +++ b/components/rappor/log_uploader_unittest.cc @@ -4,6 +4,7 @@ #include "components/rappor/log_uploader.h" +#include "base/compiler_specific.h" #include "base/message_loop/message_loop_proxy.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_request_test_util.h" diff --git a/components/rappor/rappor_metric.cc b/components/rappor/rappor_metric.cc index 2da3b31..f8ff162 100644 --- a/components/rappor/rappor_metric.cc +++ b/components/rappor/rappor_metric.cc @@ -30,7 +30,7 @@ ByteVector RapporMetric::GetReport(const std::string& secret) const { // client's secret key + real data as a seed. The inclusion of the secret // in the seed avoids correlations between real and fake data. // The seed isn't a human-readable string. - std::string personalization_string = metric_name_ + + const std::string personalization_string = metric_name_ + std::string(bytes().begin(), bytes().end()); HmacByteVectorGenerator hmac_generator(bytes().size(), secret, personalization_string); diff --git a/components/rappor/rappor_metric.h b/components/rappor/rappor_metric.h index 30147c4..07f422d 100644 --- a/components/rappor/rappor_metric.h +++ b/components/rappor/rappor_metric.h @@ -2,11 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef COMPONENTS_RAPPOR_RAPPOR_H_ -#define COMPONENTS_RAPPOR_RAPPOR_H_ +#ifndef COMPONENTS_RAPPOR_RAPPOR_METRIC_H_ +#define COMPONENTS_RAPPOR_RAPPOR_METRIC_H_ #include <string> +#include "base/basictypes.h" +#include "base/macros.h" #include "components/rappor/bloom_filter.h" #include "components/rappor/byte_vector_utils.h" #include "components/rappor/rappor_parameters.h" @@ -16,17 +18,20 @@ namespace rappor { // A RapporMetric is an object that collects string samples into a Bloom filter, // and generates randomized reports about the collected data. // +// This class should not be used directly by metrics clients. Record metrics +// using RapporService::RecordSample instead. +// // For a full description of the rappor metrics, see // http://www.chromium.org/developers/design-documents/rappor class RapporMetric { public: // Takes the |metric_name| that this will be reported to the server with, - // a |parameters| describing size and probability weights to be used in - // recording this metric, and cohort value, which modifies the hash - // functions and used in the bloom filter. - explicit RapporMetric(const std::string& metric_name, - const RapporParameters& parameters, - int32_t cohort); + // a |parameters| describing size and probability weights used in recording + // this metric, and a |cohort| value, which determines the hash functions + // used in the Bloom filter. + RapporMetric(const std::string& metric_name, + const RapporParameters& parameters, + int32_t cohort); ~RapporMetric(); // Records an additional sample in the Bloom filter. @@ -53,4 +58,4 @@ class RapporMetric { } // namespace rappor -#endif // COMPONENTS_RAPPOR_RAPPOR_H_ +#endif // COMPONENTS_RAPPOR_RAPPOR_METRIC_H_ diff --git a/components/rappor/rappor_metric_unittest.cc b/components/rappor/rappor_metric_unittest.cc index bbcb1ab..7c50fd4 100644 --- a/components/rappor/rappor_metric_unittest.cc +++ b/components/rappor/rappor_metric_unittest.cc @@ -18,8 +18,7 @@ const RapporParameters kTestRapporParameters = { PROBABILITY_75 /* Fake data probability */, PROBABILITY_50 /* Fake one probability */, PROBABILITY_75 /* One coin probability */, - PROBABILITY_50 /* Zero coin probability */ -}; + PROBABILITY_50 /* Zero coin probability */}; const RapporParameters kTestStatsRapporParameters = { 50 /* Bloom filter size bytes */, @@ -27,8 +26,7 @@ const RapporParameters kTestStatsRapporParameters = { PROBABILITY_75 /* Fake data probability */, PROBABILITY_50 /* Fake one probability */, PROBABILITY_75 /* One coin probability */, - PROBABILITY_50 /* Zero coin probability */ -}; + PROBABILITY_50 /* Zero coin probability */}; // Check for basic syntax and use. TEST(RapporMetricTest, BasicMetric) { @@ -41,7 +39,7 @@ TEST(RapporMetricTest, BasicMetric) { TEST(RapporMetricTest, GetReport) { RapporMetric metric("MyRappor", kTestRapporParameters, 0); - ByteVector report = metric.GetReport( + const ByteVector report = metric.GetReport( HmacByteVectorGenerator::GenerateEntropyInput()); EXPECT_EQ(16u, report.size()); } @@ -52,25 +50,26 @@ TEST(RapporMetricTest, GetReportStatistics) { for (char i = 0; i < 50; i++) { metric.AddSample(base::StringPrintf("%d", i)); } - ByteVector real_bits = metric.bytes(); - int real_bit_count = CountBits(real_bits); + const ByteVector real_bits = metric.bytes(); + const int real_bit_count = CountBits(real_bits); EXPECT_EQ(real_bit_count, 152); - std::string secret = HmacByteVectorGenerator::GenerateEntropyInput(); - ByteVector report = metric.GetReport(secret); + const std::string secret = HmacByteVectorGenerator::GenerateEntropyInput(); + const ByteVector report = metric.GetReport(secret); - // For the bits we actually set in the bloom filter, get a count of how + // For the bits we actually set in the Bloom filter, get a count of how // many of them reported true. ByteVector from_true_reports = report; // Real bits AND report bits. ByteVectorMerge(real_bits, real_bits, &from_true_reports); - int true_from_true_count = CountBits(from_true_reports); + const int true_from_true_count = CountBits(from_true_reports); - // For the bits we didn't set in the bloom filter, get a count of how + // For the bits we didn't set in the Bloom filter, get a count of how // many of them reported true. ByteVector from_false_reports = report; ByteVectorOr(real_bits, &from_false_reports); - int true_from_false_count = CountBits(from_false_reports) - real_bit_count; + const int true_from_false_count = + CountBits(from_false_reports) - real_bit_count; // The probability of a true bit being true after redaction = // [fake_prob]*[fake_true_prob] + (1-[fake_prob]) = diff --git a/components/rappor/rappor_service.cc b/components/rappor/rappor_service.cc index 493e7b9..01e6ab0 100644 --- a/components/rappor/rappor_service.cc +++ b/components/rappor/rappor_service.cc @@ -10,8 +10,11 @@ #include "base/prefs/pref_service.h" #include "base/rand_util.h" #include "base/stl_util.h" +#include "base/time/time.h" #include "components/metrics/metrics_hashes.h" +#include "components/rappor/log_uploader.h" #include "components/rappor/proto/rappor_metric.pb.h" +#include "components/rappor/rappor_metric.h" #include "components/rappor/rappor_pref_names.h" #include "components/variations/variations_associated_data.h" @@ -42,14 +45,13 @@ GURL GetServerUrl() { } const RapporParameters kRapporParametersForType[NUM_RAPPOR_TYPES] = { - { // ETLD_PLUS_ONE_RAPPOR_TYPE - 16 /* Bloom filter size bytes */, - 2 /* Bloom filter hash count */, - rappor::PROBABILITY_75 /* Fake data probability */, - rappor::PROBABILITY_50 /* Fake one probability */, - rappor::PROBABILITY_75 /* One coin probability */, - rappor::PROBABILITY_50 /* Zero coin probability */ - }, + // ETLD_PLUS_ONE_RAPPOR_TYPE + {16 /* Bloom filter size bytes */, + 2 /* Bloom filter hash count */, + rappor::PROBABILITY_75 /* Fake data probability */, + rappor::PROBABILITY_50 /* Fake one probability */, + rappor::PROBABILITY_75 /* One coin probability */, + rappor::PROBABILITY_50 /* Zero coin probability */}, }; } // namespace @@ -62,7 +64,7 @@ RapporService::~RapporService() { void RapporService::Start(PrefService* pref_service, net::URLRequestContextGetter* request_context) { - GURL server_url = GetServerUrl(); + const GURL server_url = GetServerUrl(); if (!server_url.is_valid()) return; DCHECK(!uploader_); @@ -98,19 +100,21 @@ void RapporService::RegisterPrefs(PrefRegistrySimple* registry) { } void RapporService::LoadCohort(PrefService* pref_service) { - DCHECK_EQ(cohort_, -1); + DCHECK(!IsInitialized()); cohort_ = pref_service->GetInteger(prefs::kRapporCohort); + // If the user is already assigned to a valid cohort, we're done. if (cohort_ >= 0 && cohort_ < kNumCohorts) return; + // This is the first time the client has started the service (or their + // preferences were corrupted). Randomly assign them to a cohort. cohort_ = base::RandGenerator(kNumCohorts); pref_service->SetInteger(prefs::kRapporCohort, cohort_); } void RapporService::LoadSecret(PrefService* pref_service) { DCHECK(secret_.empty()); - std::string secret_base64 = - pref_service->GetString(prefs::kRapporSecret); + std::string secret_base64 = pref_service->GetString(prefs::kRapporSecret); if (!secret_base64.empty()) { bool decoded = base::Base64Decode(secret_base64, &secret_); if (decoded && secret_.size() == HmacByteVectorGenerator::kEntropyInputSize) @@ -132,8 +136,9 @@ bool RapporService::ExportMetrics(RapporReports* reports) { DCHECK_GE(cohort_, 0); reports->set_cohort(cohort_); - for (std::map<std::string, RapporMetric*>::iterator it = metrics_map_.begin(); - metrics_map_.end() != it; + for (std::map<std::string, RapporMetric*>::const_iterator it = + metrics_map_.begin(); + it != metrics_map_.end(); ++it) { const RapporMetric* metric = it->second; RapporReports::Report* report = reports->add_report(); @@ -163,7 +168,6 @@ void RapporService::RecordSampleInternal(const std::string& metric_name, const RapporParameters& parameters, const std::string& sample) { DCHECK(IsInitialized()); - RapporMetric* metric = LookUpMetric(metric_name, parameters); metric->AddSample(sample); } @@ -171,9 +175,9 @@ void RapporService::RecordSampleInternal(const std::string& metric_name, RapporMetric* RapporService::LookUpMetric(const std::string& metric_name, const RapporParameters& parameters) { DCHECK(IsInitialized()); - std::map<std::string, RapporMetric*>::iterator it = + std::map<std::string, RapporMetric*>::const_iterator it = metrics_map_.find(metric_name); - if (metrics_map_.end() != it) { + if (it != metrics_map_.end()) { RapporMetric* metric = it->second; DCHECK_EQ(parameters.ToString(), metric->parameters().ToString()); return metric; diff --git a/components/rappor/rappor_service.h b/components/rappor/rappor_service.h index 9e2cd9b..323e9ba 100644 --- a/components/rappor/rappor_service.h +++ b/components/rappor/rappor_service.h @@ -5,22 +5,31 @@ #ifndef COMPONENTS_RAPPOR_RAPPOR_SERVICE_H_ #define COMPONENTS_RAPPOR_RAPPOR_SERVICE_H_ +#include <map> #include <string> #include "base/basictypes.h" -#include "base/prefs/pref_service.h" -#include "base/time/time.h" +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" #include "base/timer/timer.h" -#include "components/rappor/log_uploader.h" -#include "components/rappor/proto/rappor_metric.pb.h" -#include "components/rappor/rappor_metric.h" class PrefRegistrySimple; +class PrefService; + +namespace net { +class URLRequestContextGetter; +} namespace rappor { +class LogUploader; +class RapporMetric; +class RapporReports; +struct RapporParameters; + // The type of data stored in a metric. enum RapporType { + // For sampling the eTLD+1 of a URL. ETLD_PLUS_ONE_RAPPOR_TYPE = 0, NUM_RAPPOR_TYPES }; @@ -52,8 +61,9 @@ class RapporService { static void RegisterPrefs(PrefRegistrySimple* registry); protected: - // Logs all of the collected metrics to the reports proto message. Exposed - // for tests. Returns true if any metrics were recorded. + // Logs all of the collected metrics to the reports proto message and clears + // the internal map. Exposed for tests. Returns true if any metrics were + // recorded. bool ExportMetrics(RapporReports* reports); // Records a sample of the rappor metric specified by |parameters|. @@ -91,7 +101,7 @@ class RapporService { // The cohort this client is assigned to. -1 is uninitialized. int32_t cohort_; - // Timer which schedules calls to OnLogInterval() + // Timer which schedules calls to OnLogInterval(). base::OneShotTimer<RapporService> log_rotation_timer_; // A private LogUploader instance for sending reports to the server. diff --git a/components/rappor/rappor_service_unittest.cc b/components/rappor/rappor_service_unittest.cc index 790eae7..a36e960 100644 --- a/components/rappor/rappor_service_unittest.cc +++ b/components/rappor/rappor_service_unittest.cc @@ -4,6 +4,9 @@ #include "components/rappor/rappor_service.h" +#include "components/rappor/byte_vector_utils.h" +#include "components/rappor/proto/rappor_metric.pb.h" +#include "components/rappor/rappor_parameters.h" #include "testing/gtest/include/gtest/gtest.h" namespace rappor { @@ -27,8 +30,7 @@ TEST(RapporServiceTest, RecordAndExportMetrics) { PROBABILITY_75 /* Fake data probability */, PROBABILITY_50 /* Fake one probability */, PROBABILITY_75 /* One coin probability */, - PROBABILITY_50 /* Zero coin probability */ - }; + PROBABILITY_50 /* Zero coin probability */}; TestRapporService rappor_service; rappor_service.SetCohortForTesting(0); |