metricsd: Use the metrics directory everywhere.
Instead of passing different filenames around, only rely on the metrics
directory and infer the filepath from it. This makes testing easier.
BUG: 23939404
TEST: unit tests.
Change-Id: I79086acc3a546464114fa8ec4656ec04e1c43e35
diff --git a/constants.h b/constants.h
index 15c15d9..021bc73 100644
--- a/constants.h
+++ b/constants.h
@@ -19,10 +19,10 @@
namespace metrics {
static const char kMetricsDirectory[] = "/data/misc/metrics/";
-static const char kMetricsEventsFilePath[] = "/data/misc/metrics/uma-events";
-static const char kMetricsGUIDFilePath[] = "/data/misc/metrics/Sysinfo.GUID";
+static const char kMetricsEventsFileName[] = "uma-events";
+static const char kMetricsGUIDFileName[] = "Sysinfo.GUID";
static const char kMetricsServer[] = "https://clients4.google.com/uma/v2";
-static const char kConsentFilePath[] = "/data/misc/metrics/enabled";
+static const char kConsentFileName[] = "enabled";
static const char kDefaultVersion[] = "0.0.0.0";
// System properties used.
diff --git a/include/metrics/metrics_library.h b/include/metrics/metrics_library.h
index a956b69..eaae6ca 100644
--- a/include/metrics/metrics_library.h
+++ b/include/metrics/metrics_library.h
@@ -22,6 +22,7 @@
#include <unistd.h>
#include <base/compiler_specific.h>
+#include <base/files/file_path.h>
#include <base/macros.h>
#include <base/memory/scoped_ptr.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
@@ -129,8 +130,7 @@
FRIEND_TEST(MetricsLibraryTest, SendMessageToChrome);
FRIEND_TEST(MetricsLibraryTest, SendMessageToChromeUMAEventsBadFileLocation);
- void InitForTest(const std::string& uma_events_file,
- const std::string& consent_file);
+ void InitForTest(const base::FilePath& metrics_directory);
// Sets |*result| to whether or not the |mounts_file| indicates that
// the |device_name| is currently mounted. Uses |buffer| of
@@ -146,8 +146,8 @@
// Cached state of whether or not metrics were enabled.
static bool cached_enabled_;
- std::string uma_events_file_;
- std::string consent_file_;
+ base::FilePath uma_events_file_;
+ base::FilePath consent_file_;
DISALLOW_COPY_AND_ASSIGN(MetricsLibrary);
};
diff --git a/metrics_client.cc b/metrics_client.cc
index f658b22..78174ef 100644
--- a/metrics_client.cc
+++ b/metrics_client.cc
@@ -140,11 +140,13 @@
}
static int DumpLogs() {
- printf("Metrics from %s\n\n", metrics::kMetricsEventsFilePath);
+ base::FilePath events_file = base::FilePath(
+ metrics::kMetricsDirectory).Append(metrics::kMetricsEventsFileName);
+ printf("Metrics from %s\n\n", events_file.value().data());
ScopedVector<metrics::MetricSample> metrics;
- metrics::SerializationUtils::ReadMetricsFromFile(
- metrics::kMetricsEventsFilePath, &metrics);
+ metrics::SerializationUtils::ReadMetricsFromFile(events_file.value(),
+ &metrics);
for (ScopedVector<metrics::MetricSample>::const_iterator i = metrics.begin();
i != metrics.end(); ++i) {
diff --git a/metrics_daemon.cc b/metrics_daemon.cc
index e35bc28..de7f2ea 100644
--- a/metrics_daemon.cc
+++ b/metrics_daemon.cc
@@ -187,10 +187,10 @@
void MetricsDaemon::RunUploaderTest() {
upload_service_.reset(new UploadService(
- new SystemProfileCache(true, base::FilePath(config_root_)),
+ new SystemProfileCache(true, metrics_directory_),
metrics_lib_,
server_));
- upload_service_->Init(upload_interval_, metrics_file_);
+ upload_service_->Init(upload_interval_, metrics_directory_);
upload_service_->UploadEvent();
}
@@ -223,18 +223,16 @@
const string& cpuinfo_max_freq_path,
const base::TimeDelta& upload_interval,
const string& server,
- const string& metrics_file,
- const string& config_root) {
+ const base::FilePath& metrics_directory) {
CHECK(metrics_lib);
testing_ = testing;
uploader_active_ = uploader_active;
dbus_enabled_ = dbus_enabled;
- config_root_ = config_root;
+ metrics_directory_ = metrics_directory;
metrics_lib_ = metrics_lib;
upload_interval_ = upload_interval;
server_ = server;
- metrics_file_ = metrics_file;
// Get ticks per second (HZ) on this system.
// Sysconf cannot fail, so no sanity checks are needed.
@@ -337,7 +335,7 @@
if (uploader_active_) {
upload_service_.reset(
new UploadService(new SystemProfileCache(), metrics_lib_, server_));
- upload_service_->Init(upload_interval_, metrics_file_);
+ upload_service_->Init(upload_interval_, metrics_directory_);
}
return EX_OK;
diff --git a/metrics_daemon.h b/metrics_daemon.h
index 7f7ea63..b363c5e 100644
--- a/metrics_daemon.h
+++ b/metrics_daemon.h
@@ -50,8 +50,7 @@
const std::string& scaling_max_freq_path,
const base::TimeDelta& upload_interval,
const std::string& server,
- const std::string& metrics_file,
- const std::string& config_root);
+ const base::FilePath& metrics_directory);
// Initializes DBus and MessageLoop variables before running the MessageLoop.
int OnInit() override;
@@ -268,7 +267,7 @@
bool dbus_enabled_;
// Root of the configuration files to use.
- std::string config_root_;
+ base::FilePath metrics_directory_;
// The metrics library handle.
MetricsLibraryInterface* metrics_lib_;
@@ -331,7 +330,6 @@
base::TimeDelta upload_interval_;
std::string server_;
- std::string metrics_file_;
scoped_ptr<UploadService> upload_service_;
};
diff --git a/metrics_daemon_main.cc b/metrics_daemon_main.cc
index 43046f8..c2e794e 100644
--- a/metrics_daemon_main.cc
+++ b/metrics_daemon_main.cc
@@ -75,11 +75,9 @@
DEFINE_string(server,
metrics::kMetricsServer,
"Server to upload the metrics to. (needs -uploader)");
- DEFINE_string(metrics_file,
- metrics::kMetricsEventsFilePath,
- "File to use as a proxy for uploading the metrics");
- DEFINE_string(config_root,
- "/", "Root of the configuration files (testing only)");
+ DEFINE_string(metrics_directory,
+ metrics::kMetricsDirectory,
+ "Root of the configuration files (testing only)");
chromeos::FlagHelper::Init(argc, argv, "Chromium OS Metrics Daemon");
@@ -103,8 +101,7 @@
kCpuinfoMaxFreqPath,
base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
FLAGS_server,
- FLAGS_metrics_file,
- FLAGS_config_root);
+ base::FilePath(FLAGS_metrics_directory));
if (FLAGS_uploader_test) {
daemon.RunUploaderTest();
diff --git a/metrics_daemon_test.cc b/metrics_daemon_test.cc
index 4ef097e..476d0f3 100644
--- a/metrics_daemon_test.cc
+++ b/metrics_daemon_test.cc
@@ -87,8 +87,7 @@
cpu_max_freq_path_.value(),
base::TimeDelta::FromMinutes(30),
metrics::kMetricsServer,
- metrics::kMetricsEventsFilePath,
- "/");
+ temp_dir_.path());
}
// Adds a metrics library mock expectation that the specified metric
diff --git a/metrics_library.cc b/metrics_library.cc
index 5687f1b..6449a24 100644
--- a/metrics_library.cc
+++ b/metrics_library.cc
@@ -56,7 +56,7 @@
time_t MetricsLibrary::cached_enabled_time_ = 0;
bool MetricsLibrary::cached_enabled_ = false;
-MetricsLibrary::MetricsLibrary() : consent_file_(metrics::kConsentFilePath) {}
+MetricsLibrary::MetricsLibrary() {}
MetricsLibrary::~MetricsLibrary() {}
// We take buffer and buffer_size as parameters in order to simplify testing
@@ -131,19 +131,20 @@
time_t this_check_time = time(nullptr);
if (this_check_time != cached_enabled_time_) {
cached_enabled_time_ = this_check_time;
- cached_enabled_ = stat(consent_file_.c_str(), &stat_buffer) >= 0;
+ cached_enabled_ = stat(consent_file_.value().data(), &stat_buffer) >= 0;
}
return cached_enabled_;
}
void MetricsLibrary::Init() {
- uma_events_file_ = metrics::kMetricsEventsFilePath;
+ base::FilePath dir = base::FilePath(metrics::kMetricsDirectory);
+ uma_events_file_ = dir.Append(metrics::kMetricsEventsFileName);
+ consent_file_ = dir.Append(metrics::kConsentFileName);
}
-void MetricsLibrary::InitForTest(const std::string& uma_events_file,
- const std::string& consent_file) {
- uma_events_file_ = uma_events_file;
- consent_file_ = consent_file;
+void MetricsLibrary::InitForTest(const base::FilePath& metrics_directory) {
+ uma_events_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
+ consent_file_ = metrics_directory.Append(metrics::kConsentFileName);
}
bool MetricsLibrary::SendToUMA(const std::string& name,
@@ -154,30 +155,32 @@
return metrics::SerializationUtils::WriteMetricToFile(
*metrics::MetricSample::HistogramSample(name, sample, min, max, nbuckets)
.get(),
- metrics::kMetricsEventsFilePath);
+ uma_events_file_.value());
}
bool MetricsLibrary::SendEnumToUMA(const std::string& name, int sample,
int max) {
return metrics::SerializationUtils::WriteMetricToFile(
*metrics::MetricSample::LinearHistogramSample(name, sample, max).get(),
- metrics::kMetricsEventsFilePath);
+ uma_events_file_.value());
}
bool MetricsLibrary::SendSparseToUMA(const std::string& name, int sample) {
return metrics::SerializationUtils::WriteMetricToFile(
*metrics::MetricSample::SparseHistogramSample(name, sample).get(),
- metrics::kMetricsEventsFilePath);
+ uma_events_file_.value());
}
bool MetricsLibrary::SendUserActionToUMA(const std::string& action) {
return metrics::SerializationUtils::WriteMetricToFile(
- *metrics::MetricSample::UserActionSample(action).get(), metrics::kMetricsEventsFilePath);
+ *metrics::MetricSample::UserActionSample(action).get(),
+ uma_events_file_.value());
}
bool MetricsLibrary::SendCrashToUMA(const char *crash_kind) {
return metrics::SerializationUtils::WriteMetricToFile(
- *metrics::MetricSample::CrashSample(crash_kind).get(), metrics::kMetricsEventsFilePath);
+ *metrics::MetricSample::CrashSample(crash_kind).get(),
+ uma_events_file_.value());
}
bool MetricsLibrary::SendCrosEventToUMA(const std::string& event) {
diff --git a/metrics_library_test.cc b/metrics_library_test.cc
index 7ade6ee..f300d17 100644
--- a/metrics_library_test.cc
+++ b/metrics_library_test.cc
@@ -28,19 +28,17 @@
protected:
virtual void SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
- consent_file_ = temp_dir_.path().Append("consent");
- uma_events_file_ = temp_dir_.path().Append("events");
- lib_.InitForTest(uma_events_file_.value(), consent_file_.value());
- EXPECT_EQ(0, WriteFile(uma_events_file_, "", 0));
+ lib_.InitForTest(temp_dir_.path());
+ EXPECT_EQ(0, WriteFile(lib_.uma_events_file_, "", 0));
// Defeat metrics enabled caching between tests.
lib_.cached_enabled_time_ = 0;
}
void SetMetricsConsent(bool enabled) {
if (enabled) {
- ASSERT_EQ(base::WriteFile(consent_file_, "", 0), 0);
+ ASSERT_EQ(base::WriteFile(lib_.consent_file_, "", 0), 0);
} else {
- ASSERT_TRUE(base::DeleteFile(consent_file_, false));
+ ASSERT_TRUE(base::DeleteFile(lib_.consent_file_, false));
}
}
@@ -49,8 +47,6 @@
MetricsLibrary lib_;
base::ScopedTempDir temp_dir_;
- base::FilePath consent_file_;
- base::FilePath uma_events_file_;
};
TEST_F(MetricsLibraryTest, AreMetricsEnabledFalse) {
diff --git a/uploader/system_profile_cache.cc b/uploader/system_profile_cache.cc
index 8635fb0..e3f6339 100644
--- a/uploader/system_profile_cache.cc
+++ b/uploader/system_profile_cache.cc
@@ -55,16 +55,16 @@
SystemProfileCache::SystemProfileCache()
: initialized_(false),
testing_(false),
- config_root_("/"),
+ metrics_directory_(metrics::kMetricsDirectory),
session_id_(new chromeos_metrics::PersistentInteger(
kPersistentSessionIdFilename)) {
}
SystemProfileCache::SystemProfileCache(bool testing,
- const base::FilePath& config_root)
+ const base::FilePath& metrics_directory)
: initialized_(false),
testing_(testing),
- config_root_(config_root),
+ metrics_directory_(metrics_directory),
session_id_(new chromeos_metrics::PersistentInteger(
kPersistentSessionIdFilename)) {
}
@@ -91,9 +91,11 @@
channel = "";
profile_.version = metrics::kDefaultVersion;
}
- profile_.client_id =
- testing_ ? "client_id_test" :
- GetPersistentGUID(metrics::kMetricsGUIDFilePath);
+ std::string guid_path = metrics_directory_.Append(
+ metrics::kMetricsGUIDFileName).value();
+ profile_.client_id = testing_ ?
+ "client_id_test" :
+ GetPersistentGUID(guid_path);
profile_.hardware_class = "unknown";
profile_.channel = ProtoChannelFromString(channel);
@@ -155,7 +157,7 @@
std::string SystemProfileCache::GetProperty(const std::string& name) {
if (testing_) {
std::string content;
- base::ReadFileToString(config_root_.Append(name), &content);
+ base::ReadFileToString(metrics_directory_.Append(name), &content);
return content;
} else {
char value[PROPERTY_VALUE_MAX];
diff --git a/uploader/system_profile_cache.h b/uploader/system_profile_cache.h
index 7157810..1d22fa1 100644
--- a/uploader/system_profile_cache.h
+++ b/uploader/system_profile_cache.h
@@ -50,7 +50,7 @@
public:
SystemProfileCache();
- SystemProfileCache(bool testing, const base::FilePath& config_root);
+ SystemProfileCache(bool testing, const base::FilePath& metrics_directory);
// Populates the ProfileSystem protobuf with system information.
bool Populate(metrics::ChromeUserMetricsExtension* metrics_proto) override;
@@ -77,13 +77,13 @@
bool InitializeOrCheck();
// Gets a system property as a string.
- // When |testing_| is true, reads the value from |config_root_|/|name|
+ // When |testing_| is true, reads the value from |metrics_directory_|/|name|
// instead.
std::string GetProperty(const std::string& name);
bool initialized_;
bool testing_;
- base::FilePath config_root_;
+ base::FilePath metrics_directory_;
scoped_ptr<chromeos_metrics::PersistentInteger> session_id_;
SystemProfile profile_;
};
diff --git a/uploader/upload_service.cc b/uploader/upload_service.cc
index 2335630..6d7fd54 100644
--- a/uploader/upload_service.cc
+++ b/uploader/upload_service.cc
@@ -29,6 +29,7 @@
#include <base/metrics/statistics_recorder.h>
#include <base/sha1.h>
+#include "constants.h"
#include "serialization/metric_sample.h"
#include "serialization/serialization_utils.h"
#include "uploader/metrics_log.h"
@@ -56,9 +57,9 @@
}
void UploadService::Init(const base::TimeDelta& upload_interval,
- const std::string& metrics_file) {
+ const base::FilePath& metrics_directory) {
base::StatisticsRecorder::Initialize();
- metrics_file_ = metrics_file;
+ metrics_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
if (!testing_) {
base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
@@ -143,7 +144,7 @@
ScopedVector<metrics::MetricSample> vector;
metrics::SerializationUtils::ReadAndTruncateMetricsFromFile(
- metrics_file_, &vector);
+ metrics_file_.value(), &vector);
int i = 0;
for (ScopedVector<metrics::MetricSample>::iterator it = vector.begin();
diff --git a/uploader/upload_service.h b/uploader/upload_service.h
index 7f2f413..d9c690d 100644
--- a/uploader/upload_service.h
+++ b/uploader/upload_service.h
@@ -73,7 +73,7 @@
const std::string& server);
void Init(const base::TimeDelta& upload_interval,
- const std::string& metrics_file);
+ const base::FilePath& metrics_directory);
// Starts a new log. The log needs to be regenerated after each successful
// launch as it is destroyed when staging the log.
@@ -157,7 +157,7 @@
scoped_ptr<MetricsLog> current_log_;
scoped_ptr<MetricsLog> staged_log_;
- std::string metrics_file_;
+ base::FilePath metrics_file_;
bool testing_;
};
diff --git a/uploader/upload_service_test.cc b/uploader/upload_service_test.cc
index 40c235d..d3b5289 100644
--- a/uploader/upload_service_test.cc
+++ b/uploader/upload_service_test.cc
@@ -42,8 +42,7 @@
&metrics_lib_, "", true));
upload_service_->sender_.reset(new SenderMock);
- event_file_ = dir_.path().Append("event");
- upload_service_->Init(base::TimeDelta::FromMinutes(30), event_file_.value());
+ upload_service_->Init(base::TimeDelta::FromMinutes(30), dir_.path());
upload_service_->GatherHistograms();
upload_service_->Reset();
@@ -61,8 +60,6 @@
base::WriteFile(dir_.path().Append(name), value.data(), value.size()));
}
- base::FilePath event_file_;
-
base::ScopedTempDir dir_;
scoped_ptr<UploadService> upload_service_;
MetricsLibraryMock metrics_lib_;
