metricsd: Don't cache the metrics status in the daemon.
metrics_daemon should never get a stale answer on whether or not the metrics are enabled. This is important as metrics_daemon will be the "source of truth" for other components. BUG: 24386281 TEST: unit tests. Change-Id: I573568abe5d1b840683cede2fdf32cdae028a81a
This commit is contained in:
parent
b955f476d3
commit
a5b40d077f
4 changed files with 32 additions and 2 deletions
|
@ -48,6 +48,12 @@ class MetricsLibrary : public MetricsLibraryInterface {
|
|||
// Initializes the library.
|
||||
void Init() override;
|
||||
|
||||
// Initializes the library and disables the cache of whether or not the
|
||||
// metrics collection is enabled.
|
||||
// By disabling this, we may have to check for the metrics status more often
|
||||
// but the result will never be stale.
|
||||
void InitWithNoCaching();
|
||||
|
||||
// Returns whether or not the machine is running in guest mode.
|
||||
bool IsGuestMode();
|
||||
|
||||
|
@ -125,6 +131,7 @@ class MetricsLibrary : public MetricsLibraryInterface {
|
|||
friend class MetricsLibraryTest;
|
||||
friend class UploadServiceTest;
|
||||
FRIEND_TEST(MetricsLibraryTest, AreMetricsEnabled);
|
||||
FRIEND_TEST(MetricsLibraryTest, AreMetricsEnabledNoCaching);
|
||||
FRIEND_TEST(MetricsLibraryTest, FormatChromeMessage);
|
||||
FRIEND_TEST(MetricsLibraryTest, FormatChromeMessageTooLong);
|
||||
FRIEND_TEST(MetricsLibraryTest, IsDeviceMounted);
|
||||
|
@ -147,6 +154,9 @@ class MetricsLibrary : public MetricsLibraryInterface {
|
|||
// Cached state of whether or not metrics were enabled.
|
||||
bool cached_enabled_;
|
||||
|
||||
// True iff we should cache the enabled/disabled status.
|
||||
bool use_caching_;
|
||||
|
||||
base::FilePath uma_events_file_;
|
||||
base::FilePath consent_file_;
|
||||
|
||||
|
|
|
@ -90,7 +90,7 @@ int main(int argc, char** argv) {
|
|||
}
|
||||
|
||||
MetricsLibrary metrics_lib;
|
||||
metrics_lib.Init();
|
||||
metrics_lib.InitWithNoCaching();
|
||||
MetricsDaemon daemon;
|
||||
daemon.Init(FLAGS_uploader_test,
|
||||
FLAGS_uploader | FLAGS_uploader_test,
|
||||
|
|
|
@ -126,7 +126,7 @@ bool MetricsLibrary::IsGuestMode() {
|
|||
bool MetricsLibrary::AreMetricsEnabled() {
|
||||
static struct stat stat_buffer;
|
||||
time_t this_check_time = time(nullptr);
|
||||
if (this_check_time != cached_enabled_time_) {
|
||||
if (!use_caching_ || this_check_time != cached_enabled_time_) {
|
||||
cached_enabled_time_ = this_check_time;
|
||||
cached_enabled_ = stat(consent_file_.value().data(), &stat_buffer) >= 0;
|
||||
}
|
||||
|
@ -139,6 +139,12 @@ void MetricsLibrary::Init() {
|
|||
consent_file_ = dir.Append(metrics::kConsentFileName);
|
||||
cached_enabled_ = false;
|
||||
cached_enabled_time_ = 0;
|
||||
use_caching_ = true;
|
||||
}
|
||||
|
||||
void MetricsLibrary::InitWithNoCaching() {
|
||||
Init();
|
||||
use_caching_ = false;
|
||||
}
|
||||
|
||||
void MetricsLibrary::InitForTest(const base::FilePath& metrics_directory) {
|
||||
|
@ -146,6 +152,7 @@ void MetricsLibrary::InitForTest(const base::FilePath& metrics_directory) {
|
|||
consent_file_ = metrics_directory.Append(metrics::kConsentFileName);
|
||||
cached_enabled_ = false;
|
||||
cached_enabled_time_ = 0;
|
||||
use_caching_ = true;
|
||||
}
|
||||
|
||||
bool MetricsLibrary::SendToUMA(const std::string& name,
|
||||
|
|
|
@ -93,3 +93,16 @@ TEST_F(MetricsLibraryTest, AreMetricsEnabledCaching) {
|
|||
VerifyEnabledCacheEviction(false);
|
||||
VerifyEnabledCacheEviction(true);
|
||||
}
|
||||
|
||||
TEST_F(MetricsLibraryTest, AreMetricsEnabledNoCaching) {
|
||||
// disable caching.
|
||||
lib_.use_caching_ = false;
|
||||
|
||||
// Checking the consent repeatedly should return the right result.
|
||||
for (int i=0; i<100; ++i) {
|
||||
SetMetricsConsent(true);
|
||||
ASSERT_EQ(true, lib_.AreMetricsEnabled());
|
||||
SetMetricsConsent(false);
|
||||
ASSERT_EQ(false, lib_.AreMetricsEnabled());
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue