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
diff --git a/metricsd/include/metrics/metrics_library.h b/metricsd/include/metrics/metrics_library.h
index 5556e57..b766194 100644
--- a/metricsd/include/metrics/metrics_library.h
+++ b/metricsd/include/metrics/metrics_library.h
@@ -48,6 +48,12 @@
// 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 @@
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 @@
// 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_;
diff --git a/metricsd/metrics_daemon_main.cc b/metricsd/metrics_daemon_main.cc
index c2e794e..df13944 100644
--- a/metricsd/metrics_daemon_main.cc
+++ b/metricsd/metrics_daemon_main.cc
@@ -90,7 +90,7 @@
}
MetricsLibrary metrics_lib;
- metrics_lib.Init();
+ metrics_lib.InitWithNoCaching();
MetricsDaemon daemon;
daemon.Init(FLAGS_uploader_test,
FLAGS_uploader | FLAGS_uploader_test,
diff --git a/metricsd/metrics_library.cc b/metricsd/metrics_library.cc
index 3109704..a651b76 100644
--- a/metricsd/metrics_library.cc
+++ b/metricsd/metrics_library.cc
@@ -126,7 +126,7 @@
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 @@
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 @@
consent_file_ = metrics_directory.Append(metrics::kConsentFileName);
cached_enabled_ = false;
cached_enabled_time_ = 0;
+ use_caching_ = true;
}
bool MetricsLibrary::SendToUMA(const std::string& name,
diff --git a/metricsd/metrics_library_test.cc b/metricsd/metrics_library_test.cc
index f300d17..be8a4bb 100644
--- a/metricsd/metrics_library_test.cc
+++ b/metricsd/metrics_library_test.cc
@@ -93,3 +93,16 @@
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());
+ }
+}