metrics: add support for other product id
metrics_uploader should use the GOOGLE_METRICS_PRODUCT_ID field from os-release
whenever possible instead of the default Chrome product id.
BUG=chromium:415744
TEST=FEATURES=test emerge-gizmo metrics succeeds.
TEST=test_that platform_MetricsUploader succeeds.
CQ-DEPEND=CL:221963
Change-Id: I69b1a6ca766048ad80d93008a2fe3b18879bf1da
Reviewed-on: https://chromium-review.googlesource.com/221953
Tested-by: Bertrand Simonnet <bsimonnet@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Bertrand Simonnet <bsimonnet@chromium.org>
diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc
index bdd181c..b0247c6 100644
--- a/metrics/metrics_daemon.cc
+++ b/metrics/metrics_daemon.cc
@@ -187,7 +187,9 @@
}
void MetricsDaemon::RunUploaderTest() {
- upload_service_.reset(new UploadService(testing_, server_));
+ upload_service_.reset(new UploadService(new SystemProfileCache(true,
+ config_root_),
+ server_));
upload_service_->Init(upload_interval_secs_, metrics_file_);
upload_service_->UploadEvent();
}
@@ -218,8 +220,10 @@
const string& cpuinfo_max_freq_path,
int upload_interval_secs,
const string& server,
- const string& metrics_file) {
+ const string& metrics_file,
+ const string& config_root) {
testing_ = testing;
+ config_root_ = config_root;
DCHECK(metrics_lib != nullptr);
metrics_lib_ = metrics_lib;
@@ -321,7 +325,7 @@
if (uploader_active) {
LOG(INFO) << "uploader enabled";
- upload_service_.reset(new UploadService(testing_, server_));
+ upload_service_.reset(new UploadService(new SystemProfileCache(), server_));
upload_service_->Init(upload_interval_secs_, metrics_file_);
}
}
diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h
index 07072f9..7c7f8d2 100644
--- a/metrics/metrics_daemon.h
+++ b/metrics/metrics_daemon.h
@@ -39,7 +39,8 @@
const std::string& scaling_max_freq_path,
int upload_interval_secs,
const std::string& server,
- const std::string& metrics_file);
+ const std::string& metrics_file,
+ const std::string& config_root);
// Does all the work. If |run_as_daemon| is true, daemonizes by
// forking.
@@ -300,6 +301,9 @@
// Test mode.
bool testing_;
+ // Root of the configuration files to use.
+ std::string config_root_;
+
// The metrics library handle.
MetricsLibraryInterface* metrics_lib_;
diff --git a/metrics/metrics_daemon_main.cc b/metrics/metrics_daemon_main.cc
index e92ce31..f4a660b 100644
--- a/metrics/metrics_daemon_main.cc
+++ b/metrics/metrics_daemon_main.cc
@@ -65,6 +65,8 @@
DEFINE_string(metrics_file,
"/var/run/metrics/uma-events",
"File to use as a proxy for uploading the metrics");
+ DEFINE_string(config_root,
+ "/", "Root of the configuration files (testing only)");
chromeos::FlagHelper::Init(argc, argv, "Chromium OS Metrics Daemon");
@@ -83,7 +85,8 @@
kCpuinfoMaxFreqPath,
FLAGS_upload_interval_secs,
FLAGS_server,
- FLAGS_metrics_file);
+ FLAGS_metrics_file,
+ FLAGS_config_root);
base::AtExitManager at_exit_manager;
diff --git a/metrics/metrics_daemon_test.cc b/metrics/metrics_daemon_test.cc
index 78559df..947be87 100644
--- a/metrics/metrics_daemon_test.cc
+++ b/metrics/metrics_daemon_test.cc
@@ -74,7 +74,8 @@
kFakeCpuinfoMaxFreqPath,
1800,
kMetricsServer,
- kMetricsFilePath);
+ kMetricsFilePath,
+ "/");
// Replace original persistent values with mock ones.
daily_active_use_mock_ =
diff --git a/metrics/uploader/system_profile_cache.cc b/metrics/uploader/system_profile_cache.cc
index 604c060..b08fbd9 100644
--- a/metrics/uploader/system_profile_cache.cc
+++ b/metrics/uploader/system_profile_cache.cc
@@ -11,20 +11,35 @@
#include "base/files/file_util.h"
#include "base/guid.h"
#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
#include "base/sys_info.h"
#include "components/metrics/metrics_log_base.h"
#include "components/metrics/proto/chrome_user_metrics_extension.pb.h"
#include "metrics/persistent_integer.h"
#include "vboot/crossystem.h"
-const char* SystemProfileCache::kPersistentGUIDFile =
- "/var/lib/metrics/Sysinfo.GUID";
-const char* SystemProfileCache::kPersistentSessionIdFilename =
- "Sysinfo.SessionId";
+namespace {
-SystemProfileCache::SystemProfileCache(bool testing)
+const char kPersistentGUIDFile[] = "/var/lib/metrics/Sysinfo.GUID";
+const char kPersistentSessionIdFilename[] = "Sysinfo.SessionId";
+const char kProductIdFieldName[] = "GOOGLE_METRICS_PRODUCT_ID";
+
+} // namespace
+
+
+SystemProfileCache::SystemProfileCache()
+ : initialized_(false),
+ testing_(false),
+ config_root_("/"),
+ session_id_(new chromeos_metrics::PersistentInteger(
+ kPersistentSessionIdFilename)) {
+}
+
+SystemProfileCache::SystemProfileCache(bool testing,
+ const std::string& config_root)
: initialized_(false),
testing_(testing),
+ config_root_(config_root),
session_id_(new chromeos_metrics::PersistentInteger(
kPersistentSessionIdFilename)) {
}
@@ -48,6 +63,12 @@
base::SysInfo::GetLsbReleaseValue("CHROMEOS_RELEASE_TRACK", &channel_string);
profile_.channel = ProtoChannelFromString(channel_string);
+ // If the product id is not defined, use the default one from the protobuf.
+ profile_.product_id = metrics::ChromeUserMetricsExtension::CHROME;
+ if (GetProductId(&profile_.product_id)) {
+ DLOG(INFO) << "Set the product id to " << profile_.product_id;
+ }
+
profile_.client_id =
testing_ ? "client_id_test" : GetPersistentGUID(kPersistentGUIDFile);
@@ -78,6 +99,9 @@
metrics::MetricsLogBase::Hash(profile_.client_id));
metrics_proto->set_session_id(profile_.session_id);
+ // Sets the product id.
+ metrics_proto->set_product(profile_.product_id);
+
metrics::SystemProfileProto* profile_proto =
metrics_proto->mutable_system_profile();
profile_proto->mutable_hardware()->set_hardware_class(
@@ -122,6 +146,25 @@
return true;
}
+bool SystemProfileCache::GetProductId(int* product_id) const {
+ chromeos::OsReleaseReader reader;
+ if (testing_) {
+ base::FilePath root(config_root_);
+ CHECK(reader.LoadTestingOnly(root)) << "Failed to load os-release fields "
+ "from" << root.value();
+ } else {
+ CHECK(reader.Load()) << "Failed to load os-release fields.";
+ }
+
+ std::string id;
+ if (reader.GetString(kProductIdFieldName, &id)) {
+ CHECK(base::StringToInt(id, product_id)) << "Failed to convert product_id "
+ << id << " to int.";
+ return true;
+ }
+ return false;
+}
+
metrics::SystemProfileProto_Channel SystemProfileCache::ProtoChannelFromString(
const std::string& channel) {
diff --git a/metrics/uploader/system_profile_cache.h b/metrics/uploader/system_profile_cache.h
index f85c1b1..95f554f 100644
--- a/metrics/uploader/system_profile_cache.h
+++ b/metrics/uploader/system_profile_cache.h
@@ -12,6 +12,7 @@
#include "base/compiler_specific.h"
#include "base/gtest_prod_util.h"
#include "base/memory/scoped_ptr.h"
+#include "chromeos/osrelease_reader.h"
#include "components/metrics/proto/system_profile.pb.h"
#include "metrics/persistent_integer.h"
#include "metrics/uploader/system_profile_setter.h"
@@ -28,6 +29,7 @@
std::string hardware_class;
std::string client_id;
int32_t session_id;
+ int32_t product_id;
};
// Retrieves general system informations needed by the protobuf for context and
@@ -36,7 +38,9 @@
// The cache is populated lazily. The only method needed is Populate.
class SystemProfileCache : public SystemProfileSetter {
public:
- explicit SystemProfileCache(bool testing);
+ SystemProfileCache();
+
+ SystemProfileCache(bool testing, const std::string& config_root);
// Populates the ProfileSystem protobuf with system information.
void Populate(metrics::ChromeUserMetricsExtension* profile_proto) override;
@@ -56,9 +60,6 @@
FRIEND_TEST(UploadServiceTest, SessionIdIncrementedAtInitialization);
FRIEND_TEST(UploadServiceTest, ValuesInConfigFileAreSent);
- static const char* kPersistentGUIDFile;
- static const char* kPersistentSessionIdFilename;
-
// Fetches all informations and populates |profile_|
bool Initialize();
@@ -68,8 +69,12 @@
// Gets the hardware ID using crossystem
bool GetHardwareId(std::string* hwid);
+ // Gets the product ID from the GOOGLE_METRICS_PRODUCT_ID field.
+ bool GetProductId(int* product_id) const;
+
bool initialized_;
bool testing_;
+ std::string config_root_;
scoped_ptr<chromeos_metrics::PersistentInteger> session_id_;
SystemProfile profile_;
};
diff --git a/metrics/uploader/upload_service.cc b/metrics/uploader/upload_service.cc
index 0ebfc78..55d76a4 100644
--- a/metrics/uploader/upload_service.cc
+++ b/metrics/uploader/upload_service.cc
@@ -24,8 +24,9 @@
const int UploadService::kMaxFailedUpload = 10;
-UploadService::UploadService(bool testing, const std::string& server)
- : system_profile_setter_(new SystemProfileCache(testing)),
+UploadService::UploadService(SystemProfileSetter* setter,
+ const std::string& server)
+ : system_profile_setter_(setter),
histogram_snapshot_manager_(this),
sender_(new HttpSender(server)) {
}
diff --git a/metrics/uploader/upload_service.h b/metrics/uploader/upload_service.h
index c0a1430..a72349e 100644
--- a/metrics/uploader/upload_service.h
+++ b/metrics/uploader/upload_service.h
@@ -54,7 +54,8 @@
//
class UploadService : public base::HistogramFlattener {
public:
- explicit UploadService(bool testing, const std::string& server);
+ explicit UploadService(SystemProfileSetter* setter,
+ const std::string& server);
void Init(int upload_interval_secs,
const std::string& metrics_file);
diff --git a/metrics/uploader/upload_service_test.cc b/metrics/uploader/upload_service_test.cc
index 9badf64..2cdc4f5 100644
--- a/metrics/uploader/upload_service_test.cc
+++ b/metrics/uploader/upload_service_test.cc
@@ -25,12 +25,11 @@
class UploadServiceTest : public testing::Test {
protected:
UploadServiceTest()
- : cache_(true),
- upload_service_(true, kMetricsServer),
+ : cache_(true, "/"),
+ upload_service_(new MockSystemProfileSetter(), kMetricsServer),
exit_manager_(new base::AtExitManager()) {
sender_ = new SenderMock;
upload_service_.sender_.reset(sender_);
- upload_service_.system_profile_setter_.reset(new MockSystemProfileSetter());
upload_service_.Init(1800, kMetricsFilePath);
}
@@ -127,7 +126,7 @@
}
TEST_F(UploadServiceTest, LogEmptyByDefault) {
- UploadService upload_service(true, kMetricsServer);
+ UploadService upload_service(new MockSystemProfileSetter(), kMetricsServer);
// current_log_ should be initialized later as it needs AtExitManager to exit
// in order to gather system information from SysInfo.
@@ -195,7 +194,7 @@
base::SysInfo::SetChromeOSVersionInfoForTest(content, base::Time());
scoped_ptr<metrics::MetricSample> histogram =
metrics::MetricSample::SparseHistogramSample("myhistogram", 1);
- SystemProfileCache* local_cache_ = new SystemProfileCache(true);
+ SystemProfileCache* local_cache_ = new SystemProfileCache(true, "/");
local_cache_->session_id_.reset(new chromeos_metrics::PersistentInteger(
dir_.path().Append("session_id").value()));