From bae5dcce9b0ec357d3fe0340583537354439bbe0 Mon Sep 17 00:00:00 2001 From: Bertrand SIMONNET Date: Tue, 4 Aug 2015 14:12:10 -0700 Subject: [PATCH] metrics: Enable for non-official builds. The metrics uploader used to be disabled for non-official images to avoid polluting the production data with possibly wrong measurements. This is inconvenient for developers as they can only be sure that a new metric was added properly until the changes reached a product device. Instead, this CL change the metrics daemon to upload metrics iff the consent file exists. To ensure that testing data does not pollute the production data, we set the channel to UNKNOWN and the version to 0.0.0.0 when the image is not official (implied by channel and version missing). BUG: 22879597 Change-Id: If03847090b732cc06270cbcc8b386b5f9e544a3d --- metrics/constants.h | 1 + metrics/metrics_daemon.cc | 36 ++++++++---------------- metrics/metrics_daemon.h | 3 -- metrics/uploader/system_profile_cache.cc | 15 +++++++--- 4 files changed, 23 insertions(+), 32 deletions(-) diff --git a/metrics/constants.h b/metrics/constants.h index d44a74f84..d65e0e0cc 100644 --- a/metrics/constants.h +++ b/metrics/constants.h @@ -23,6 +23,7 @@ static const char kMetricsEventsFilePath[] = "/data/misc/metrics/uma-events"; static const char kMetricsGUIDFilePath[] = "/data/misc/metrics/Sysinfo.GUID"; static const char kMetricsServer[] = "http://clients4.google.com/uma/v2"; static const char kConsentFilePath[] = "/data/misc/metrics/enabled"; +static const char kDefaultVersion[] = "0.0.0.0"; } // namespace metrics #endif // METRICS_CONSTANTS_H_ diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc index 2881f691b..634027842 100644 --- a/metrics/metrics_daemon.cc +++ b/metrics/metrics_daemon.cc @@ -23,6 +23,8 @@ #include #include #include + +#include "constants.h" #include "uploader/upload_service.h" using base::FilePath; @@ -44,10 +46,6 @@ const char kCrashReporterUserCrashSignal[] = "UserCrash"; const char kCrashReporterMatchRule[] = "type='signal',interface='%s',path='/',member='%s'"; -// Build type of an official build. -// See src/third_party/chromiumos-overlay/chromeos/scripts/cros_set_lsb_release. -const char kOfficialBuild[] = "Official Build"; - const int kSecondsPerMinute = 60; const int kMinutesPerHour = 60; const int kHoursPerDay = 24; @@ -199,24 +197,17 @@ uint32_t MetricsDaemon::GetOsVersionHash() { if (version_hash_is_cached) return cached_version_hash; version_hash_is_cached = true; - std::string version; - if (base::SysInfo::GetLsbReleaseValue("CHROMEOS_RELEASE_VERSION", &version)) { - cached_version_hash = base::Hash(version); - } else if (testing_) { + std::string version = metrics::kDefaultVersion; + // The version might not be set for development devices. In this case, use the + // zero version. + base::SysInfo::GetLsbReleaseValue("BRILLO_VERSION", &version); + cached_version_hash = base::Hash(version); + if (testing_) { cached_version_hash = 42; // return any plausible value for the hash - } else { - LOG(FATAL) << "could not find CHROMEOS_RELEASE_VERSION"; } return cached_version_hash; } -bool MetricsDaemon::IsOnOfficialBuild() const { - std::string build_type; - return (base::SysInfo::GetLsbReleaseValue("CHROMEOS_RELEASE_BUILD_TYPE", - &build_type) && - build_type == kOfficialBuild); -} - void MetricsDaemon::Init(bool testing, bool uploader_active, MetricsLibraryInterface* metrics_lib, @@ -317,14 +308,9 @@ int MetricsDaemon::OnInit() { } if (uploader_active_) { - if (IsOnOfficialBuild()) { - LOG(INFO) << "uploader enabled"; - upload_service_.reset( - new UploadService(new SystemProfileCache(), metrics_lib_, server_)); - upload_service_->Init(upload_interval_, metrics_file_); - } else { - LOG(INFO) << "uploader disabled on non-official build"; - } + upload_service_.reset( + new UploadService(new SystemProfileCache(), metrics_lib_, server_)); + upload_service_->Init(upload_interval_, metrics_file_); } return EX_OK; diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h index bcfec14d5..49e6d7058 100644 --- a/metrics/metrics_daemon.h +++ b/metrics/metrics_daemon.h @@ -268,9 +268,6 @@ class MetricsDaemon : public chromeos::DBusDaemon { // to a unsigned 32-bit int. uint32_t GetOsVersionHash(); - // Returns true if the system is using an official build. - bool IsOnOfficialBuild() const; - // Updates stats, additionally sending them to UMA if enough time has elapsed // since the last report. void UpdateStats(base::TimeTicks now_ticks, base::Time now_wall_time); diff --git a/metrics/uploader/system_profile_cache.cc b/metrics/uploader/system_profile_cache.cc index 82401c648..adbe0ae0a 100644 --- a/metrics/uploader/system_profile_cache.cc +++ b/metrics/uploader/system_profile_cache.cc @@ -61,15 +61,22 @@ bool SystemProfileCache::Initialize() { CHECK(!initialized_) << "this should be called only once in the metrics_daemon lifetime."; - std::string channel; - if (!base::SysInfo::GetLsbReleaseValue("BRILLO_CHANNEL", &channel) || - !base::SysInfo::GetLsbReleaseValue("BRILLO_VERSION", &profile_.version) || - !base::SysInfo::GetLsbReleaseValue("BRILLO_BUILD_TARGET_ID", + if (!base::SysInfo::GetLsbReleaseValue("BRILLO_BUILD_TARGET_ID", &profile_.build_target_id)) { LOG(ERROR) << "Could not initialize system profile."; return false; } + std::string channel; + if (!base::SysInfo::GetLsbReleaseValue("BRILLO_CHANNEL", &channel) || + !base::SysInfo::GetLsbReleaseValue("BRILLO_VERSION", &profile_.version)) { + // If the channel or version is missing, the image is not official. + // In this case, set the channel to unknown and the version to 0.0.0.0 to + // avoid polluting the production data. + channel = ""; + profile_.version = metrics::kDefaultVersion; + + } profile_.client_id = testing_ ? "client_id_test" : GetPersistentGUID(metrics::kMetricsGUIDFilePath);