diff --git a/metricsd/constants.h b/metricsd/constants.h index 3a7569bdc..ee0c9cb5a 100644 --- a/metricsd/constants.h +++ b/metricsd/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/metricsd/metrics_client.cc b/metricsd/metrics_client.cc index 78174efba..946b36a86 100644 --- a/metricsd/metrics_client.cc +++ b/metricsd/metrics_client.cc @@ -140,8 +140,8 @@ static int IsGuestMode() { } 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; diff --git a/metricsd/metrics_collector.cc b/metricsd/metrics_collector.cc index ddf9bc150..28f9ad399 100644 --- a/metricsd/metrics_collector.cc +++ b/metricsd/metrics_collector.cc @@ -151,50 +151,52 @@ uint32_t MetricsCollector::GetOsVersionHash() { 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 @@ void MetricsCollector::OnEnableMetrics( 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 @@ void MetricsCollector::OnDisableMetrics( 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/metricsd/metrics_collector.h b/metricsd/metrics_collector.h index e080ac0d9..69747d0be 100644 --- a/metricsd/metrics_collector.h +++ b/metricsd/metrics_collector.h @@ -48,7 +48,8 @@ class MetricsCollector : public brillo::DBusDaemon { 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 @@ class MetricsCollector : public brillo::DBusDaemon { // 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/metricsd/metrics_collector_main.cc b/metricsd/metrics_collector_main.cc index 117426e06..d7aaaf57e 100644 --- a/metricsd/metrics_collector_main.cc +++ b/metricsd/metrics_collector_main.cc @@ -51,9 +51,13 @@ const std::string MetricsMainDiskStatsPath() { 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 @@ int main(int argc, char** argv) { 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/metricsd/metrics_collector_test.cc b/metricsd/metrics_collector_test.cc index 004636045..956e56bdf 100644 --- a/metricsd/metrics_collector_test.cc +++ b/metricsd/metrics_collector_test.cc @@ -45,7 +45,14 @@ class MetricsCollectorTest : public testing::Test { 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/metricsd/metrics_library.cc b/metricsd/metrics_library.cc index 735d39ff8..686c92661 100644 --- a/metricsd/metrics_library.cc +++ b/metricsd/metrics_library.cc @@ -134,7 +134,7 @@ bool MetricsLibrary::AreMetricsEnabled() { } 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/metricsd.rc b/metricsd/metricsd.rc index b5e7b8273..359d0d129 100644 --- a/metricsd/metricsd.rc +++ b/metricsd/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/metricsd_main.cc b/metricsd/metricsd_main.cc index ab71e6b66..eee8a94aa 100644 --- a/metricsd/metricsd_main.cc +++ b/metricsd/metricsd_main.cc @@ -43,9 +43,13 @@ int main(int argc, char** argv) { 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 @@ int main(int argc, char** argv) { 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/metricsd/uploader/system_profile_cache.cc b/metricsd/uploader/system_profile_cache.cc index 6afc86c8e..70f6afd50 100644 --- a/metricsd/uploader/system_profile_cache.cc +++ b/metricsd/uploader/system_profile_cache.cc @@ -56,7 +56,7 @@ std::string ChannelToString( 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/metricsd/uploader/upload_service.cc b/metricsd/uploader/upload_service.cc index b4731e900..3e0c503fd 100644 --- a/metricsd/uploader/upload_service.cc +++ b/metricsd/uploader/upload_service.cc @@ -43,14 +43,17 @@ const int UploadService::kMaxFailedUpload = 10; 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 @@ void UploadService::RemoveFailedLog() { bool UploadService::AreMetricsEnabled() { return base::PathExists(consent_file_); } - diff --git a/metricsd/uploader/upload_service.h b/metricsd/uploader/upload_service.h index 7faf3577b..eed0d9de8 100644 --- a/metricsd/uploader/upload_service.h +++ b/metricsd/uploader/upload_service.h @@ -71,7 +71,8 @@ class UploadService : public base::HistogramFlattener, public brillo::Daemon { 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 @@ class UploadService : public base::HistogramFlattener, public brillo::Daemon { scoped_ptr sender_; chromeos_metrics::PersistentInteger failed_upload_count_; scoped_ptr current_log_; - + base::TimeDelta upload_interval_; base::FilePath consent_file_; diff --git a/metricsd/uploader/upload_service_test.cc b/metricsd/uploader/upload_service_test.cc index c77b3424c..9fc5e71d1 100644 --- a/metricsd/uploader/upload_service_test.cc +++ b/metricsd/uploader/upload_service_test.cc @@ -39,11 +39,18 @@ class UploadServiceTest : public testing::Test { 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, EmptyLogsAreNotSent) { } 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 @@ TEST_F(UploadServiceTest, LogContainsAggregatedValues) { metrics::MetricSample::HistogramSample("foo", 10, 0, 42, 10); upload_service_->AddSample(*histogram.get()); - scoped_ptr histogram2 = metrics::MetricSample::HistogramSample("foo", 11, 0, 42, 10); upload_service_->AddSample(*histogram2.get());