Skip to content

Commit

Permalink
Add a mechanism for collecting Rappor samples on a daily interval.
Browse files Browse the repository at this point in the history
BUG=400927

Review URL: https://codereview.chromium.org/511623002

Cr-Commit-Position: refs/heads/master@{#296025}
  • Loading branch information
holte authored and Commit bot committed Sep 22, 2014
1 parent 9afeee5 commit 07637b0
Show file tree
Hide file tree
Showing 15 changed files with 396 additions and 33 deletions.
1 change: 0 additions & 1 deletion chrome/browser/chrome_browser_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,6 @@ void ChromeBrowserMainParts::StartMetricsRecording() {
// TODO(asvitkine): Since this function is not run on Android, RAPPOR is
// currently disabled there. http://crbug.com/370041
browser_process_->rappor_service()->Start(
browser_process_->local_state(),
browser_process_->system_request_context(),
metrics_enabled);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/prefs/testing_pref_service.h"
#include "base/time/time.h"
#include "chrome/browser/extensions/activity_log/activity_actions.h"
#include "components/rappor/byte_vector_utils.h"
Expand Down Expand Up @@ -42,9 +43,13 @@ class TestRapporService : public rappor::RapporService {
// be from the last time GetReports() was called (not from the beginning of
// the test).
rappor::RapporReports GetReports();

protected:
TestingPrefServiceSimple prefs_;
};

TestRapporService::TestRapporService() {
TestRapporService::TestRapporService()
: rappor::RapporService(&prefs_) {
// Initialize the RapporService for testing.
SetCohortForTesting(0);
SetSecretForTesting(rappor::HmacByteVectorGenerator::GenerateEntropyInput());
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/metrics/metrics_services_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ metrics::MetricsService* MetricsServicesManager::GetMetricsService() {
rappor::RapporService* MetricsServicesManager::GetRapporService() {
DCHECK(thread_checker_.CalledOnValidThread());
if (!rappor_service_)
rappor_service_.reset(new rappor::RapporService);
rappor_service_.reset(new rappor::RapporService(local_state_));
return rappor_service_.get();
}

Expand Down
1 change: 1 addition & 0 deletions components/components_tests.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@
'language_usage_metrics/language_usage_metrics_unittest.cc',
'leveldb_proto/proto_database_impl_unittest.cc',
'metrics/compression_utils_unittest.cc',
'metrics/daily_event_unittest.cc',
'metrics/machine_id_provider_win_unittest.cc',
'metrics/metrics_hashes_unittest.cc',
'metrics/metrics_log_manager_unittest.cc',
Expand Down
2 changes: 2 additions & 0 deletions components/metrics.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
'metrics/cloned_install_detector.h',
'metrics/compression_utils.cc',
'metrics/compression_utils.h',
'metrics/daily_event.cc',
'metrics/daily_event.h',
'metrics/machine_id_provider.h',
'metrics/machine_id_provider_stub.cc',
'metrics/machine_id_provider_win.cc',
Expand Down
2 changes: 2 additions & 0 deletions components/metrics/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ source_set("metrics") {
"cloned_install_detector.h",
"compression_utils.cc",
"compression_utils.h",
"daily_event.cc",
"daily_event.h",
"machine_id_provider.h",
"machine_id_provider_stub.cc",
"machine_id_provider_win.cc",
Expand Down
106 changes: 106 additions & 0 deletions components/metrics/daily_event.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/metrics/daily_event.h"

#include "base/i18n/time_formatting.h"
#include "base/metrics/histogram.h"
#include "base/prefs/pref_registry_simple.h"
#include "base/prefs/pref_service.h"

namespace metrics {

namespace {

enum IntervalType {
FIRST_RUN,
DAY_ELAPSED,
CLOCK_CHANGED,
NUM_INTERVAL_TYPES
};

void RecordIntervalTypeHistogram(const std::string& histogram_name,
IntervalType type) {
base::Histogram::FactoryGet(
histogram_name,
1,
NUM_INTERVAL_TYPES,
NUM_INTERVAL_TYPES + 1,
base::HistogramBase::kUmaTargetedHistogramFlag)->Add(type);
}

} // namespace

DailyEvent::Observer::Observer() {
}

DailyEvent::Observer::~Observer() {
}

DailyEvent::DailyEvent(PrefService* pref_service,
const char* pref_name,
const std::string& histogram_name)
: pref_service_(pref_service),
pref_name_(pref_name),
histogram_name_(histogram_name) {
}

DailyEvent::~DailyEvent() {
}

// static
void DailyEvent::RegisterPref(PrefRegistrySimple* registry,
const char* pref_name) {
registry->RegisterInt64Pref(pref_name, base::Time().ToInternalValue());
}

void DailyEvent::AddObserver(scoped_ptr<DailyEvent::Observer> observer) {
DVLOG(2) << "DailyEvent observer added.";
DCHECK(last_fired_.is_null());
observers_.push_back(observer.release());
}

void DailyEvent::CheckInterval() {
base::Time now = base::Time::Now();
if (last_fired_.is_null()) {
// The first time we call CheckInterval, we read the time stored in prefs.
last_fired_ = base::Time::FromInternalValue(
pref_service_->GetInt64(pref_name_));
DVLOG(1) << "DailyEvent time loaded: "
<< base::TimeFormatShortDateAndTime(last_fired_);
if (last_fired_.is_null()) {
DVLOG(1) << "DailyEvent first run.";
RecordIntervalTypeHistogram(pref_name_, FIRST_RUN);
OnInterval(now);
return;
}
}
int days_elapsed = (now - last_fired_).InDays();
if (days_elapsed >= 1) {
DVLOG(1) << "DailyEvent day elapsed.";
RecordIntervalTypeHistogram(pref_name_, DAY_ELAPSED);
OnInterval(now);
} else if (days_elapsed <= -1) {
// The "last fired" time is more than a day in the future, so the clock
// must have been changed.
DVLOG(1) << "DailyEvent clock change detected.";
RecordIntervalTypeHistogram(pref_name_, CLOCK_CHANGED);
OnInterval(now);
}
}

void DailyEvent::OnInterval(base::Time now) {
DCHECK(!now.is_null());
last_fired_ = now;
pref_service_->SetInt64(pref_name_, last_fired_.ToInternalValue());

// Notify all observers
for (ScopedVector<DailyEvent::Observer>::iterator it = observers_.begin();
it != observers_.end();
++it) {
(*it)->OnDailyEvent();
}
}

} // namespace metrics
92 changes: 92 additions & 0 deletions components/metrics/daily_event.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_METRICS_DAILY_EVENT_H_
#define COMPONENTS_METRICS_DAILY_EVENT_H_

#include "base/macros.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
#include "base/time/time.h"

class PrefRegistrySimple;
class PrefService;

namespace metrics {

// DailyEvent is used for throttling an event to about once per day, even if
// the program is restarted more frequently. It is based on local machine
// time, so it could be fired more often if the clock is changed.
//
// The service using the DailyEvent should first provide all of the Observers
// for the interval, and then arrange for CheckInterval() to be called
// periodically to test if the event should be fired.
class DailyEvent {
public:
// Observer receives notifications from a DailyEvent.
// Observers must be added before the DailyEvent begins checking time,
// and will be owned by the DailyEvent.
class Observer {
public:
Observer();
virtual ~Observer();

// Called when the daily event is fired.
virtual void OnDailyEvent() = 0;

private:
DISALLOW_COPY_AND_ASSIGN(Observer);
};

// Constructs DailyEvent monitor which stores the time it last fired in the
// preference |pref_name|. |pref_name| should be registered by calling
// RegisterPref before using this object.
// Caller is responsible for ensuring that |pref_service| and |pref_name|
// outlive the DailyEvent.
// |histogram_name| is the name of the UMA metric which record when this
// interval fires, and should be registered in histograms.xml
DailyEvent(PrefService* pref_service,
const char* pref_name,
const std::string& histogram_name);
~DailyEvent();

// Adds a observer to be notified when a day elapses. All observers should
// be registered before the the DailyEvent starts checking time.
void AddObserver(scoped_ptr<Observer> observer);

// Checks if a day has elapsed. If it has, OnDailyEvent will be called on
// all observers.
void CheckInterval();

// Registers the preference used by this interval.
static void RegisterPref(PrefRegistrySimple* registry, const char* pref_name);

private:
// Handles an interval elapsing.
void OnInterval(base::Time now);

// A weak pointer to the PrefService object to read and write preferences
// from. Calling code should ensure this object continues to exist for the
// lifetime of the DailyEvent object.
PrefService* pref_service_;

// The name of the preference to store the last fired time in.
// Calling code should ensure this outlives the DailyEvent.
const char* pref_name_;

// The name of the histogram to record intervals.
std::string histogram_name_;

// A list of observers.
ScopedVector<Observer> observers_;

// The time that the daily event was last fired.
base::Time last_fired_;

DISALLOW_COPY_AND_ASSIGN(DailyEvent);
};

} // namespace metrics

#endif // COMPONENTS_METRICS_DAILY_EVENT_H_
96 changes: 96 additions & 0 deletions components/metrics/daily_event_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/metrics/daily_event.h"

#include "base/prefs/testing_pref_service.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace metrics {

namespace {

const char kTestPrefName[] = "TestPref";
const char kTestMetricName[] = "TestMetric";

class TestDailyObserver : public DailyEvent::Observer {
public:
TestDailyObserver() : fired_(false) {}

bool fired() const { return fired_; }

virtual void OnDailyEvent() OVERRIDE {
fired_ = true;
}

void Reset() {
fired_ = false;
}

private:
// True if this event has been observed.
bool fired_;

DISALLOW_COPY_AND_ASSIGN(TestDailyObserver);
};

class DailyEventTest : public testing::Test {
public:
DailyEventTest() : event_(&prefs_, kTestPrefName, kTestMetricName) {
DailyEvent::RegisterPref(prefs_.registry(), kTestPrefName);
observer_ = new TestDailyObserver();
scoped_ptr<metrics::DailyEvent::Observer> p(observer_);
event_.AddObserver(p.Pass());
}

protected:
TestingPrefServiceSimple prefs_;
TestDailyObserver* observer_;
DailyEvent event_;

private:
DISALLOW_COPY_AND_ASSIGN(DailyEventTest);
};

} // namespace

// The event should fire if the preference is not available.
TEST_F(DailyEventTest, TestNewFires) {
event_.CheckInterval();
EXPECT_TRUE(observer_->fired());
}

// The event should fire if the preference is more than a day old.
TEST_F(DailyEventTest, TestOldFires) {
base::Time last_time = base::Time::Now() - base::TimeDelta::FromHours(25);
prefs_.SetInt64(kTestPrefName, last_time.ToInternalValue());
event_.CheckInterval();
EXPECT_TRUE(observer_->fired());
}

// The event should fire if the preference is more than a day in the future.
TEST_F(DailyEventTest, TestFutureFires) {
base::Time last_time = base::Time::Now() + base::TimeDelta::FromHours(25);
prefs_.SetInt64(kTestPrefName, last_time.ToInternalValue());
event_.CheckInterval();
EXPECT_TRUE(observer_->fired());
}

// The event should not fire if the preference is more recent than a day.
TEST_F(DailyEventTest, TestRecentNotFired) {
base::Time last_time = base::Time::Now() - base::TimeDelta::FromMinutes(2);
prefs_.SetInt64(kTestPrefName, last_time.ToInternalValue());
event_.CheckInterval();
EXPECT_FALSE(observer_->fired());
}

// The event should not fire if the preference is less than a day in the future.
TEST_F(DailyEventTest, TestSoonNotFired) {
base::Time last_time = base::Time::Now() + base::TimeDelta::FromMinutes(2);
prefs_.SetInt64(kTestPrefName, last_time.ToInternalValue());
event_.CheckInterval();
EXPECT_FALSE(observer_->fired());
}

} // namespace metrics
3 changes: 3 additions & 0 deletions components/rappor/rappor_pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ const char kRapporCohortDeprecated[] = "rappor.cohort";
// A randomly generated number, which determines cohorts data is reported for.
const char kRapporCohortSeed[] = "rappor.cohort_seed";

// Timestamp of the last time we sampled daily metrics.
const char kRapporLastDailySample[] = "rappor.last_daily_sample";

// A base-64 encoded, randomly generated byte string, which is used as a seed
// for redacting collected data.
// Important: This value should remain secret at the client, and never be
Expand Down
1 change: 1 addition & 0 deletions components/rappor/rappor_pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace prefs {
// component. Keep alphabetized, and document each in the .cc file.
extern const char kRapporCohortDeprecated[];
extern const char kRapporCohortSeed[];
extern const char kRapporLastDailySample[];
extern const char kRapporSecret[];

} // namespace prefs
Expand Down
Loading

0 comments on commit 07637b0

Please sign in to comment.