metricsd: Specify directory for persistent integers.
Instead of using a global directory for persistent integers, specify the
directory to use in the constructor.
This will make changing the backing directory easier.
Bug: 25886951
Change-Id: I590816b195fa81b179a5ec78b9cdf41bc86353dc
diff --git a/metrics_collector.cc b/metrics_collector.cc
index 28194a1..ddf9bc1 100644
--- a/metrics_collector.cc
+++ b/metrics_collector.cc
@@ -149,51 +149,52 @@
return version_hash;
}
-void MetricsCollector::Init(bool testing,
- MetricsLibraryInterface* metrics_lib,
- const string& diskstats_path,
- const base::FilePath& metrics_directory) {
+void MetricsCollector::Init(bool testing, MetricsLibraryInterface* metrics_lib,
+ const string& diskstats_path,
+ const base::FilePath& metrics_directory) {
CHECK(metrics_lib);
testing_ = testing;
metrics_directory_ = metrics_directory;
metrics_lib_ = metrics_lib;
daily_active_use_.reset(
- new PersistentInteger("Platform.UseTime.PerDay"));
+ new PersistentInteger("Platform.UseTime.PerDay", metrics_directory_));
version_cumulative_active_use_.reset(
- new PersistentInteger("Platform.CumulativeUseTime"));
+ new PersistentInteger("Platform.CumulativeUseTime", metrics_directory_));
version_cumulative_cpu_use_.reset(
- new PersistentInteger("Platform.CumulativeCpuTime"));
+ new PersistentInteger("Platform.CumulativeCpuTime", metrics_directory_));
- kernel_crash_interval_.reset(
- new PersistentInteger("Platform.KernelCrashInterval"));
- unclean_shutdown_interval_.reset(
- new PersistentInteger("Platform.UncleanShutdownInterval"));
+ kernel_crash_interval_.reset(new PersistentInteger(
+ "Platform.KernelCrashInterval", metrics_directory_));
+ unclean_shutdown_interval_.reset(new PersistentInteger(
+ "Platform.UncleanShutdownInterval", metrics_directory_));
user_crash_interval_.reset(
- new PersistentInteger("Platform.UserCrashInterval"));
+ new PersistentInteger("Platform.UserCrashInterval", metrics_directory_));
any_crashes_daily_count_.reset(
- new PersistentInteger("Platform.AnyCrashes.PerDay"));
+ new PersistentInteger("Platform.AnyCrashes.PerDay", metrics_directory_));
any_crashes_weekly_count_.reset(
- new PersistentInteger("Platform.AnyCrashes.PerWeek"));
+ new PersistentInteger("Platform.AnyCrashes.PerWeek", metrics_directory_));
user_crashes_daily_count_.reset(
- new PersistentInteger("Platform.UserCrashes.PerDay"));
- user_crashes_weekly_count_.reset(
- new PersistentInteger("Platform.UserCrashes.PerWeek"));
- kernel_crashes_daily_count_.reset(
- new PersistentInteger("Platform.KernelCrashes.PerDay"));
- kernel_crashes_weekly_count_.reset(
- new PersistentInteger("Platform.KernelCrashes.PerWeek"));
- kernel_crashes_version_count_.reset(
- new PersistentInteger("Platform.KernelCrashesSinceUpdate"));
- unclean_shutdowns_daily_count_.reset(
- new PersistentInteger("Platform.UncleanShutdown.PerDay"));
- unclean_shutdowns_weekly_count_.reset(
- new PersistentInteger("Platform.UncleanShutdowns.PerWeek"));
+ new PersistentInteger("Platform.UserCrashes.PerDay", metrics_directory_));
+ user_crashes_weekly_count_.reset(new PersistentInteger(
+ "Platform.UserCrashes.PerWeek", metrics_directory_));
+ kernel_crashes_daily_count_.reset(new PersistentInteger(
+ "Platform.KernelCrashes.PerDay", metrics_directory_));
+ kernel_crashes_weekly_count_.reset(new PersistentInteger(
+ "Platform.KernelCrashes.PerWeek", metrics_directory_));
+ kernel_crashes_version_count_.reset(new PersistentInteger(
+ "Platform.KernelCrashesSinceUpdate", metrics_directory_));
+ unclean_shutdowns_daily_count_.reset(new PersistentInteger(
+ "Platform.UncleanShutdown.PerDay", metrics_directory_));
+ unclean_shutdowns_weekly_count_.reset(new PersistentInteger(
+ "Platform.UncleanShutdowns.PerWeek", metrics_directory_));
- daily_cycle_.reset(new PersistentInteger("daily.cycle"));
- weekly_cycle_.reset(new PersistentInteger("weekly.cycle"));
- version_cycle_.reset(new PersistentInteger("version.cycle"));
+ daily_cycle_.reset(new PersistentInteger("daily.cycle", metrics_directory_));
+ weekly_cycle_.reset(
+ new PersistentInteger("weekly.cycle", metrics_directory_));
+ version_cycle_.reset(
+ new PersistentInteger("version.cycle", metrics_directory_));
disk_usage_collector_.reset(new DiskUsageCollector(metrics_lib_));
averaged_stats_collector_.reset(
diff --git a/metrics_collector_test.cc b/metrics_collector_test.cc
index a0e7087..0046360 100644
--- a/metrics_collector_test.cc
+++ b/metrics_collector_test.cc
@@ -45,9 +45,6 @@
virtual void SetUp() {
brillo::FlagHelper::Init(0, nullptr, "");
EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
-
- chromeos_metrics::PersistentInteger::SetMetricsDirectory(
- temp_dir_.path().value());
daemon_.Init(true, &metrics_lib_, "", temp_dir_.path());
}
diff --git a/persistent_integer.cc b/persistent_integer.cc
index ddc4b50..7fe355e 100644
--- a/persistent_integer.cc
+++ b/persistent_integer.cc
@@ -23,19 +23,15 @@
#include "constants.h"
-
namespace chromeos_metrics {
-// Static class member instantiation.
-std::string PersistentInteger::metrics_directory_ = metrics::kMetricsDirectory;
-
-PersistentInteger::PersistentInteger(const std::string& name) :
- value_(0),
+PersistentInteger::PersistentInteger(const std::string& name,
+ const base::FilePath& directory)
+ : value_(0),
version_(kVersion),
name_(name),
- synced_(false) {
- backing_file_name_ = metrics_directory_ + name_;
-}
+ backing_file_path_(directory.Append(name_)),
+ synced_(false) {}
PersistentInteger::~PersistentInteger() {}
@@ -62,23 +58,25 @@
}
void PersistentInteger::Write() {
- int fd = HANDLE_EINTR(open(backing_file_name_.c_str(),
+ int fd = HANDLE_EINTR(open(backing_file_path_.value().c_str(),
O_WRONLY | O_CREAT | O_TRUNC,
S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH));
- PCHECK(fd >= 0) << "cannot open " << backing_file_name_ << " for writing";
+ PCHECK(fd >= 0) << "cannot open " << backing_file_path_.value()
+ << " for writing";
PCHECK((HANDLE_EINTR(write(fd, &version_, sizeof(version_))) ==
sizeof(version_)) &&
(HANDLE_EINTR(write(fd, &value_, sizeof(value_))) ==
sizeof(value_)))
- << "cannot write to " << backing_file_name_;
+ << "cannot write to " << backing_file_path_.value();
close(fd);
synced_ = true;
}
bool PersistentInteger::Read() {
- int fd = HANDLE_EINTR(open(backing_file_name_.c_str(), O_RDONLY));
+ int fd = HANDLE_EINTR(open(backing_file_path_.value().c_str(), O_RDONLY));
if (fd < 0) {
- PLOG(WARNING) << "cannot open " << backing_file_name_ << " for reading";
+ PLOG(WARNING) << "cannot open " << backing_file_path_.value()
+ << " for reading";
return false;
}
int32_t version;
@@ -95,9 +93,4 @@
return read_succeeded;
}
-void PersistentInteger::SetMetricsDirectory(const std::string& directory) {
- metrics_directory_ = directory;
-}
-
-
} // namespace chromeos_metrics
diff --git a/persistent_integer.h b/persistent_integer.h
index ecef3d1..96d9fc0 100644
--- a/persistent_integer.h
+++ b/persistent_integer.h
@@ -21,6 +21,8 @@
#include <string>
+#include <base/files/file_path.h>
+
namespace chromeos_metrics {
// PersistentIntegers is a named 64-bit integer value backed by a file.
@@ -29,7 +31,7 @@
class PersistentInteger {
public:
- explicit PersistentInteger(const std::string& name);
+ PersistentInteger(const std::string& name, const base::FilePath& directory);
// Virtual only because of mock.
virtual ~PersistentInteger();
@@ -50,10 +52,6 @@
// Virtual only because of mock.
virtual void Add(int64_t x);
- // Sets the directory path for all persistent integers.
- // This is used in unittests to change where the counters are stored.
- static void SetMetricsDirectory(const std::string& directory);
-
private:
static const int kVersion = 1001;
@@ -68,8 +66,7 @@
int64_t value_;
int32_t version_;
std::string name_;
- std::string backing_file_name_;
- static std::string metrics_directory_;
+ base::FilePath backing_file_path_;
bool synced_;
};
diff --git a/persistent_integer_mock.h b/persistent_integer_mock.h
index acc5389..0be54d4 100644
--- a/persistent_integer_mock.h
+++ b/persistent_integer_mock.h
@@ -27,9 +27,10 @@
class PersistentIntegerMock : public PersistentInteger {
public:
- explicit PersistentIntegerMock(const std::string& name)
- : PersistentInteger(name) {}
- MOCK_METHOD1(Add, void(int64_t count));
+ explicit PersistentIntegerMock(const std::string& name,
+ const base::FilePath& directory)
+ : PersistentInteger(name, directory) {}
+ MOCK_METHOD1(Add, void(int64_t count));
};
} // namespace chromeos_metrics
diff --git a/persistent_integer_test.cc b/persistent_integer_test.cc
index 5e2067f..55d6cbc 100644
--- a/persistent_integer_test.cc
+++ b/persistent_integer_test.cc
@@ -24,7 +24,6 @@
#include "persistent_integer.h"
const char kBackingFileName[] = "1.pibakf";
-const char kBackingFilePattern[] = "*.pibakf";
using chromeos_metrics::PersistentInteger;
@@ -32,28 +31,15 @@
void SetUp() override {
// Set testing mode.
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
- chromeos_metrics::PersistentInteger::SetMetricsDirectory(
- temp_dir_.path().value());
}
- void TearDown() override {
- // Remove backing files. The convention is that they all end in ".pibakf".
- base::FileEnumerator f_enum(base::FilePath("."),
- false,
- base::FileEnumerator::FILES,
- FILE_PATH_LITERAL(kBackingFilePattern));
- for (base::FilePath name = f_enum.Next();
- !name.empty();
- name = f_enum.Next()) {
- base::DeleteFile(name, false);
- }
- }
-
+ protected:
base::ScopedTempDir temp_dir_;
};
TEST_F(PersistentIntegerTest, BasicChecks) {
- scoped_ptr<PersistentInteger> pi(new PersistentInteger(kBackingFileName));
+ scoped_ptr<PersistentInteger> pi(
+ new PersistentInteger(kBackingFileName, temp_dir_.path()));
// Test initialization.
EXPECT_EQ(0, pi->Get());
@@ -65,7 +51,7 @@
EXPECT_EQ(5, pi->Get());
// Test persistence.
- pi.reset(new PersistentInteger(kBackingFileName));
+ pi.reset(new PersistentInteger(kBackingFileName, temp_dir_.path()));
EXPECT_EQ(5, pi->Get());
// Test GetAndClear.
@@ -73,6 +59,6 @@
EXPECT_EQ(pi->Get(), 0);
// Another persistence test.
- pi.reset(new PersistentInteger(kBackingFileName));
+ pi.reset(new PersistentInteger(kBackingFileName, temp_dir_.path()));
EXPECT_EQ(0, pi->Get());
}
diff --git a/uploader/system_profile_cache.cc b/uploader/system_profile_cache.cc
index 8928a0d..6afc86c 100644
--- a/uploader/system_profile_cache.cc
+++ b/uploader/system_profile_cache.cc
@@ -55,11 +55,10 @@
SystemProfileCache::SystemProfileCache()
: initialized_(false),
- testing_(false),
- metrics_directory_(metrics::kMetricsDirectory),
- session_id_(new chromeos_metrics::PersistentInteger(
- kPersistentSessionIdFilename)) {
-}
+ testing_(false),
+ metrics_directory_(metrics::kMetricsDirectory),
+ session_id_(new chromeos_metrics::PersistentInteger(
+ kPersistentSessionIdFilename, metrics_directory_)) {}
SystemProfileCache::SystemProfileCache(bool testing,
const base::FilePath& metrics_directory)
@@ -67,8 +66,7 @@
testing_(testing),
metrics_directory_(metrics_directory),
session_id_(new chromeos_metrics::PersistentInteger(
- kPersistentSessionIdFilename)) {
-}
+ kPersistentSessionIdFilename, metrics_directory)) {}
bool SystemProfileCache::Initialize() {
CHECK(!initialized_)
diff --git a/uploader/upload_service.cc b/uploader/upload_service.cc
index ca5024e..b4731e9 100644
--- a/uploader/upload_service.cc
+++ b/uploader/upload_service.cc
@@ -46,7 +46,7 @@
const base::FilePath& metrics_directory)
: histogram_snapshot_manager_(this),
sender_(new HttpSender(server)),
- failed_upload_count_(metrics::kFailedUploadCountName),
+ failed_upload_count_(metrics::kFailedUploadCountName, metrics_directory),
upload_interval_(upload_interval) {
metrics_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
staged_log_path_ = metrics_directory.Append(metrics::kStagedLogName);
diff --git a/uploader/upload_service_test.cc b/uploader/upload_service_test.cc
index 24e3127..c77b342 100644
--- a/uploader/upload_service_test.cc
+++ b/uploader/upload_service_test.cc
@@ -39,8 +39,6 @@
protected:
virtual void SetUp() {
CHECK(dir_.CreateUniqueTempDir());
- chromeos_metrics::PersistentInteger::SetMetricsDirectory(
- dir_.path().value());
metrics_lib_.InitForTest(dir_.path());
ASSERT_EQ(0, base::WriteFile(
dir_.path().Append(metrics::kConsentFileName), "", 0));
