metricsd: Use different directories for each daemon.
Instead of using a single directory for both the internal data of
metricsd and metrics_collector and the shared files (metrics samples log
file and the metrics enabled file), we should use separate directory to
allow for a finer access control.
The new structure will be:
* /data/misc/metrics for the files accessible to all daemons reporting
metrics, metricsd and metrics_collector.
* /data/misc/metricsd for the private files of metricsd.
* /data/misc/metrics_collector for the private files of
metrics_collector.
Bug: 25886951
Test: Unit tests.
Test: Manual: metricsd and metrics_collector run without errors.
Change-Id: I006d19f45f5f419d2b08744126c2e2a0b899c9fa
diff --git a/constants.h b/constants.h
index 3a7569b..ee0c9cb 100644
--- a/constants.h
+++ b/constants.h
@@ -18,7 +18,10 @@
#define METRICS_CONSTANTS_H_
namespace metrics {
-static const char kMetricsDirectory[] = "/data/misc/metrics/";
+static const char kSharedMetricsDirectory[] = "/data/misc/metrics/";
+static const char kMetricsdDirectory[] = "/data/misc/metricsd/";
+static const char kMetricsCollectorDirectory[] =
+ "/data/misc/metrics_collector/";
static const char kMetricsEventsFileName[] = "uma-events";
static const char kMetricsGUIDFileName[] = "Sysinfo.GUID";
static const char kMetricsServer[] = "https://clients4.google.com/uma/v2";
diff --git a/metrics_client.cc b/metrics_client.cc
index 78174ef..946b36a 100644
--- a/metrics_client.cc
+++ b/metrics_client.cc
@@ -140,8 +140,8 @@
}
static int DumpLogs() {
- base::FilePath events_file = base::FilePath(
- metrics::kMetricsDirectory).Append(metrics::kMetricsEventsFileName);
+ base::FilePath events_file = base::FilePath(metrics::kSharedMetricsDirectory)
+ .Append(metrics::kMetricsEventsFileName);
printf("Metrics from %s\n\n", events_file.value().data());
ScopedVector<metrics::MetricSample> metrics;
diff --git a/metrics_collector.cc b/metrics_collector.cc
index ddf9bc1..28f9ad3 100644
--- a/metrics_collector.cc
+++ b/metrics_collector.cc
@@ -151,50 +151,52 @@
void MetricsCollector::Init(bool testing, MetricsLibraryInterface* metrics_lib,
const string& diskstats_path,
- const base::FilePath& metrics_directory) {
+ const base::FilePath& private_metrics_directory,
+ const base::FilePath& shared_metrics_directory) {
CHECK(metrics_lib);
testing_ = testing;
- metrics_directory_ = metrics_directory;
+ shared_metrics_directory_ = shared_metrics_directory;
metrics_lib_ = metrics_lib;
- daily_active_use_.reset(
- new PersistentInteger("Platform.UseTime.PerDay", metrics_directory_));
- version_cumulative_active_use_.reset(
- new PersistentInteger("Platform.CumulativeUseTime", metrics_directory_));
- version_cumulative_cpu_use_.reset(
- new PersistentInteger("Platform.CumulativeCpuTime", metrics_directory_));
+ daily_active_use_.reset(new PersistentInteger("Platform.UseTime.PerDay",
+ private_metrics_directory));
+ version_cumulative_active_use_.reset(new PersistentInteger(
+ "Platform.CumulativeUseTime", private_metrics_directory));
+ version_cumulative_cpu_use_.reset(new PersistentInteger(
+ "Platform.CumulativeCpuTime", private_metrics_directory));
kernel_crash_interval_.reset(new PersistentInteger(
- "Platform.KernelCrashInterval", metrics_directory_));
+ "Platform.KernelCrashInterval", private_metrics_directory));
unclean_shutdown_interval_.reset(new PersistentInteger(
- "Platform.UncleanShutdownInterval", metrics_directory_));
- user_crash_interval_.reset(
- new PersistentInteger("Platform.UserCrashInterval", metrics_directory_));
+ "Platform.UncleanShutdownInterval", private_metrics_directory));
+ user_crash_interval_.reset(new PersistentInteger("Platform.UserCrashInterval",
+ private_metrics_directory));
- any_crashes_daily_count_.reset(
- new PersistentInteger("Platform.AnyCrashes.PerDay", metrics_directory_));
- any_crashes_weekly_count_.reset(
- new PersistentInteger("Platform.AnyCrashes.PerWeek", metrics_directory_));
- user_crashes_daily_count_.reset(
- new PersistentInteger("Platform.UserCrashes.PerDay", metrics_directory_));
+ any_crashes_daily_count_.reset(new PersistentInteger(
+ "Platform.AnyCrashes.PerDay", private_metrics_directory));
+ any_crashes_weekly_count_.reset(new PersistentInteger(
+ "Platform.AnyCrashes.PerWeek", private_metrics_directory));
+ user_crashes_daily_count_.reset(new PersistentInteger(
+ "Platform.UserCrashes.PerDay", private_metrics_directory));
user_crashes_weekly_count_.reset(new PersistentInteger(
- "Platform.UserCrashes.PerWeek", metrics_directory_));
+ "Platform.UserCrashes.PerWeek", private_metrics_directory));
kernel_crashes_daily_count_.reset(new PersistentInteger(
- "Platform.KernelCrashes.PerDay", metrics_directory_));
+ "Platform.KernelCrashes.PerDay", private_metrics_directory));
kernel_crashes_weekly_count_.reset(new PersistentInteger(
- "Platform.KernelCrashes.PerWeek", metrics_directory_));
+ "Platform.KernelCrashes.PerWeek", private_metrics_directory));
kernel_crashes_version_count_.reset(new PersistentInteger(
- "Platform.KernelCrashesSinceUpdate", metrics_directory_));
+ "Platform.KernelCrashesSinceUpdate", private_metrics_directory));
unclean_shutdowns_daily_count_.reset(new PersistentInteger(
- "Platform.UncleanShutdown.PerDay", metrics_directory_));
+ "Platform.UncleanShutdown.PerDay", private_metrics_directory));
unclean_shutdowns_weekly_count_.reset(new PersistentInteger(
- "Platform.UncleanShutdowns.PerWeek", metrics_directory_));
+ "Platform.UncleanShutdowns.PerWeek", private_metrics_directory));
- daily_cycle_.reset(new PersistentInteger("daily.cycle", metrics_directory_));
+ daily_cycle_.reset(
+ new PersistentInteger("daily.cycle", private_metrics_directory));
weekly_cycle_.reset(
- new PersistentInteger("weekly.cycle", metrics_directory_));
+ new PersistentInteger("weekly.cycle", private_metrics_directory));
version_cycle_.reset(
- new PersistentInteger("version.cycle", metrics_directory_));
+ new PersistentInteger("version.cycle", private_metrics_directory));
disk_usage_collector_.reset(new DiskUsageCollector(metrics_lib_));
averaged_stats_collector_.reset(
@@ -289,8 +291,9 @@
if (!command)
return;
- if (base::WriteFile(metrics_directory_.Append(metrics::kConsentFileName),
- "", 0) != 0) {
+ if (base::WriteFile(
+ shared_metrics_directory_.Append(metrics::kConsentFileName), "", 0) !=
+ 0) {
PLOG(ERROR) << "Could not create the consent file.";
command->Abort("metrics_error", "Could not create the consent file",
nullptr);
@@ -307,8 +310,8 @@
if (!command)
return;
- if (!base::DeleteFile(metrics_directory_.Append(metrics::kConsentFileName),
- false)) {
+ if (!base::DeleteFile(
+ shared_metrics_directory_.Append(metrics::kConsentFileName), false)) {
PLOG(ERROR) << "Could not delete the consent file.";
command->Abort("metrics_error", "Could not delete the consent file",
nullptr);
diff --git a/metrics_collector.h b/metrics_collector.h
index e080ac0..69747d0 100644
--- a/metrics_collector.h
+++ b/metrics_collector.h
@@ -48,7 +48,8 @@
void Init(bool testing,
MetricsLibraryInterface* metrics_lib,
const std::string& diskstats_path,
- const base::FilePath& metrics_directory);
+ const base::FilePath& private_metrics_directory,
+ const base::FilePath& shared_metrics_directory);
// Initializes DBus and MessageLoop variables before running the MessageLoop.
int OnInit() override;
@@ -225,8 +226,8 @@
// Test mode.
bool testing_;
- // Root of the configuration files to use.
- base::FilePath metrics_directory_;
+ // Publicly readable metrics directory.
+ base::FilePath shared_metrics_directory_;
// The metrics library handle.
MetricsLibraryInterface* metrics_lib_;
diff --git a/metrics_collector_main.cc b/metrics_collector_main.cc
index 117426e..d7aaaf5 100644
--- a/metrics_collector_main.cc
+++ b/metrics_collector_main.cc
@@ -51,9 +51,13 @@
int main(int argc, char** argv) {
DEFINE_bool(foreground, false, "Don't daemonize");
- DEFINE_string(metrics_directory,
- metrics::kMetricsDirectory,
- "Root of the configuration files (testing only)");
+ DEFINE_string(private_directory, metrics::kMetricsCollectorDirectory,
+ "Path to the private directory used by metrics_collector "
+ "(testing only)");
+ DEFINE_string(shared_directory, metrics::kSharedMetricsDirectory,
+ "Path to the shared metrics directory, used by "
+ "metrics_collector, metricsd and all metrics clients "
+ "(testing only)");
DEFINE_bool(logtostderr, false, "Log to standard error");
DEFINE_bool(logtosyslog, false, "Log to syslog");
@@ -86,7 +90,8 @@
daemon.Init(false,
&metrics_lib,
MetricsMainDiskStatsPath(),
- base::FilePath(FLAGS_metrics_directory));
+ base::FilePath(FLAGS_private_directory),
+ base::FilePath(FLAGS_shared_directory));
daemon.Run();
}
diff --git a/metrics_collector_test.cc b/metrics_collector_test.cc
index 0046360..956e56b 100644
--- a/metrics_collector_test.cc
+++ b/metrics_collector_test.cc
@@ -45,7 +45,14 @@
virtual void SetUp() {
brillo::FlagHelper::Init(0, nullptr, "");
EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
- daemon_.Init(true, &metrics_lib_, "", temp_dir_.path());
+
+ base::FilePath private_dir = temp_dir_.path().Append("private");
+ base::FilePath shared_dir = temp_dir_.path().Append("shared");
+
+ EXPECT_TRUE(base::CreateDirectory(private_dir));
+ EXPECT_TRUE(base::CreateDirectory(shared_dir));
+
+ daemon_.Init(true, &metrics_lib_, "", private_dir, shared_dir);
}
// Adds a metrics library mock expectation that the specified metric
diff --git a/metrics_library.cc b/metrics_library.cc
index 735d39f..686c926 100644
--- a/metrics_library.cc
+++ b/metrics_library.cc
@@ -134,7 +134,7 @@
}
void MetricsLibrary::Init() {
- base::FilePath dir = base::FilePath(metrics::kMetricsDirectory);
+ base::FilePath dir = base::FilePath(metrics::kSharedMetricsDirectory);
uma_events_file_ = dir.Append(metrics::kMetricsEventsFileName);
consent_file_ = dir.Append(metrics::kConsentFileName);
cached_enabled_ = false;
diff --git a/metricsd.rc b/metricsd.rc
index b5e7b82..359d0d1 100644
--- a/metricsd.rc
+++ b/metricsd.rc
@@ -1,5 +1,7 @@
on post-fs-data
mkdir /data/misc/metrics 0770 system system
+ mkdir /data/misc/metricsd 0700 system system
+ mkdir /data/misc/metrics_collector 0700 system system
service metricsd /system/bin/metricsd --foreground --logtosyslog
class late_start
diff --git a/metricsd_main.cc b/metricsd_main.cc
index ab71e6b..eee8a94 100644
--- a/metricsd_main.cc
+++ b/metricsd_main.cc
@@ -43,9 +43,13 @@
DEFINE_string(server,
metrics::kMetricsServer,
"Server to upload the metrics to. (needs -uploader)");
- DEFINE_string(metrics_directory,
- metrics::kMetricsDirectory,
- "Root of the configuration files (testing only)");
+ DEFINE_string(private_directory, metrics::kMetricsdDirectory,
+ "Path to the private directory used by metricsd "
+ "(testing only)");
+ DEFINE_string(shared_directory, metrics::kSharedMetricsDirectory,
+ "Path to the shared metrics directory, used by "
+ "metrics_collector, metricsd and all metrics clients "
+ "(testing only)");
DEFINE_bool(logtostderr, false, "Log to standard error");
DEFINE_bool(logtosyslog, false, "Log to syslog");
@@ -72,9 +76,10 @@
return errno;
}
- UploadService service(FLAGS_server,
- base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
- base::FilePath(FLAGS_metrics_directory));
+ UploadService service(
+ FLAGS_server, base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
+ base::FilePath(FLAGS_private_directory),
+ base::FilePath(FLAGS_shared_directory));
service.Run();
}
diff --git a/uploader/system_profile_cache.cc b/uploader/system_profile_cache.cc
index 6afc86c..70f6afd 100644
--- a/uploader/system_profile_cache.cc
+++ b/uploader/system_profile_cache.cc
@@ -56,7 +56,7 @@
SystemProfileCache::SystemProfileCache()
: initialized_(false),
testing_(false),
- metrics_directory_(metrics::kMetricsDirectory),
+ metrics_directory_(metrics::kMetricsdDirectory),
session_id_(new chromeos_metrics::PersistentInteger(
kPersistentSessionIdFilename, metrics_directory_)) {}
diff --git a/uploader/upload_service.cc b/uploader/upload_service.cc
index b4731e9..3e0c503 100644
--- a/uploader/upload_service.cc
+++ b/uploader/upload_service.cc
@@ -43,14 +43,17 @@
UploadService::UploadService(const std::string& server,
const base::TimeDelta& upload_interval,
- const base::FilePath& metrics_directory)
+ const base::FilePath& private_metrics_directory,
+ const base::FilePath& shared_metrics_directory)
: histogram_snapshot_manager_(this),
sender_(new HttpSender(server)),
- failed_upload_count_(metrics::kFailedUploadCountName, metrics_directory),
+ failed_upload_count_(metrics::kFailedUploadCountName,
+ private_metrics_directory),
upload_interval_(upload_interval) {
- metrics_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
- staged_log_path_ = metrics_directory.Append(metrics::kStagedLogName);
- consent_file_ = metrics_directory.Append(metrics::kConsentFileName);
+ metrics_file_ =
+ shared_metrics_directory.Append(metrics::kMetricsEventsFileName);
+ staged_log_path_ = private_metrics_directory.Append(metrics::kStagedLogName);
+ consent_file_ = shared_metrics_directory.Append(metrics::kConsentFileName);
}
int UploadService::OnInit() {
@@ -265,4 +268,3 @@
bool UploadService::AreMetricsEnabled() {
return base::PathExists(consent_file_);
}
-
diff --git a/uploader/upload_service.h b/uploader/upload_service.h
index 7faf357..eed0d9d 100644
--- a/uploader/upload_service.h
+++ b/uploader/upload_service.h
@@ -71,7 +71,8 @@
public:
UploadService(const std::string& server,
const base::TimeDelta& upload_interval,
- const base::FilePath& metrics_directory);
+ const base::FilePath& private_metrics_directory,
+ const base::FilePath& shared_metrics_directory);
// Initializes the upload service.
int OnInit();
@@ -162,7 +163,7 @@
scoped_ptr<Sender> sender_;
chromeos_metrics::PersistentInteger failed_upload_count_;
scoped_ptr<MetricsLog> current_log_;
-
+
base::TimeDelta upload_interval_;
base::FilePath consent_file_;
diff --git a/uploader/upload_service_test.cc b/uploader/upload_service_test.cc
index c77b342..9fc5e71 100644
--- a/uploader/upload_service_test.cc
+++ b/uploader/upload_service_test.cc
@@ -39,11 +39,18 @@
protected:
virtual void SetUp() {
CHECK(dir_.CreateUniqueTempDir());
- metrics_lib_.InitForTest(dir_.path());
- ASSERT_EQ(0, base::WriteFile(
- dir_.path().Append(metrics::kConsentFileName), "", 0));
- upload_service_.reset(new UploadService("", base::TimeDelta(),
- dir_.path()));
+
+ base::FilePath private_dir = dir_.path().Append("private");
+ base::FilePath shared_dir = dir_.path().Append("shared");
+
+ EXPECT_TRUE(base::CreateDirectory(private_dir));
+ EXPECT_TRUE(base::CreateDirectory(shared_dir));
+
+ metrics_lib_.InitForTest(shared_dir);
+ ASSERT_EQ(0, base::WriteFile(shared_dir.Append(metrics::kConsentFileName),
+ "", 0));
+ upload_service_.reset(
+ new UploadService("", base::TimeDelta(), private_dir, shared_dir));
upload_service_->sender_.reset(new SenderMock);
upload_service_->InitForTest(new MockSystemProfileSetter);
@@ -158,7 +165,7 @@
}
TEST_F(UploadServiceTest, LogEmptyByDefault) {
- UploadService upload_service("", base::TimeDelta(), dir_.path());
+ UploadService upload_service("", base::TimeDelta(), dir_.path(), dir_.path());
// current_log_ should be initialized later as it needs AtExitManager to exit
// in order to gather system information from SysInfo.
@@ -194,7 +201,6 @@
metrics::MetricSample::HistogramSample("foo", 10, 0, 42, 10);
upload_service_->AddSample(*histogram.get());
-
scoped_ptr<metrics::MetricSample> histogram2 =
metrics::MetricSample::HistogramSample("foo", 11, 0, 42, 10);
upload_service_->AddSample(*histogram2.get());
