[Structured metrics] Update terminology in KeyData

Now that keys and IDs are associated with projects, not events, the
documentation and method signatures in key_data is out of date.

I've also changed the name of HashForEventId to HmacMetric, since it
was a bit cryptic. Note that HmacMetric still operates on the values of
an event, not a project.

Bug: 1148168
Change-Id: I4bc8e6360917f4b4f141aaad9a11e5485954871f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2554193Reviewed-by: Thanh Nguyen <thanhdng@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830728}

components/metrics/structured/key_data.cc

@@ -41,17 +41,18 @@ std::string HashToHex(const uint64_t hash) {
   return base::HexEncode(&hash, sizeof(uint64_t));
 }
 
-std::string KeyPath(const uint64_t event) {
-  return base::StrCat({"keys.", base::NumberToString(event), ".key"});
+std::string KeyPath(const uint64_t project) {
+  return base::StrCat({"keys.", base::NumberToString(project), ".key"});
 }
 
-std::string LastRotationPath(const uint64_t event) {
-  return base::StrCat({"keys.", base::NumberToString(event), ".last_rotation"});
+std::string LastRotationPath(const uint64_t project) {
+  return base::StrCat(
+      {"keys.", base::NumberToString(project), ".last_rotation"});
 }
 
-std::string RotationPeriodPath(const uint64_t event) {
+std::string RotationPeriodPath(const uint64_t project) {
   return base::StrCat(
-      {"keys.", base::NumberToString(event), ".rotation_period"});
+      {"keys.", base::NumberToString(project), ".rotation_period"});
 }
 
 }  // namespace
@@ -124,15 +125,15 @@ void KeyData::ValidateKeys() {
   }
 }
 
-void KeyData::SetLastRotation(const uint64_t event, const int last_rotation) {
-  return key_store_->SetValue(LastRotationPath(event),
+void KeyData::SetLastRotation(const uint64_t project, const int last_rotation) {
+  return key_store_->SetValue(LastRotationPath(project),
                               std::make_unique<base::Value>(last_rotation),
                               WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
 }
 
-void KeyData::SetRotationPeriod(const uint64_t event,
+void KeyData::SetRotationPeriod(const uint64_t project,
                                 const int rotation_period) {
-  return key_store_->SetValue(RotationPeriodPath(event),
+  return key_store_->SetValue(RotationPeriodPath(project),
                               std::make_unique<base::Value>(rotation_period),
                               WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
 }
@@ -143,9 +144,9 @@ void KeyData::SetKey(const uint64_t project_name_hash, const std::string& key) {
                               WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
 }
 
-int KeyData::GetLastRotation(const uint64_t event) {
+int KeyData::GetLastRotation(const uint64_t project) {
   const base::Value* value;
-  if (!(key_store_->GetValue(LastRotationPath(event), &value) &&
+  if (!(key_store_->GetValue(LastRotationPath(project), &value) &&
         value->is_int())) {
     LogInternalError(StructuredMetricsError::kMissingLastRotation);
     NOTREACHED();
@@ -154,9 +155,9 @@ int KeyData::GetLastRotation(const uint64_t event) {
   return value->GetInt();
 }
 
-int KeyData::GetRotationPeriod(const uint64_t event) {
+int KeyData::GetRotationPeriod(const uint64_t project) {
   const base::Value* value;
-  if (!(key_store_->GetValue(RotationPeriodPath(event), &value) &&
+  if (!(key_store_->GetValue(RotationPeriodPath(project), &value) &&
         value->is_int())) {
     LogInternalError(StructuredMetricsError::kMissingRotationPeriod);
     NOTREACHED();
@@ -165,8 +166,8 @@ int KeyData::GetRotationPeriod(const uint64_t event) {
   return value->GetInt();
 }
 
-uint64_t KeyData::UserEventId(const uint64_t project_name_hash) {
-  // Retrieve the key for |event|.
+uint64_t KeyData::UserProjectId(const uint64_t project_name_hash) {
+  // Retrieve the key for |project_name_hash|.
   const base::Optional<std::string> key = ValidateAndGetKey(project_name_hash);
   if (!key) {
     NOTREACHED();
@@ -179,8 +180,8 @@ uint64_t KeyData::UserEventId(const uint64_t project_name_hash) {
   return hash;
 }
 
-uint64_t KeyData::HashForEventMetric(const uint64_t project_name_hash,
-                                     const uint64_t metric,
+uint64_t KeyData::HmacMetric(const uint64_t project_name_hash,
+                             const uint64_t metric_name_hash,
                              const std::string& value) {
   // Retrieve the key for |project_name_hash|.
   const base::Optional<std::string> key = ValidateAndGetKey(project_name_hash);
@@ -194,7 +195,8 @@ uint64_t KeyData::HashForEventMetric(const uint64_t project_name_hash,
   CHECK(hmac.Init(key.value()));
 
   // Compute and return the digest.
-  const std::string salted_value = base::StrCat({HashToHex(metric), value});
+  const std::string salted_value =
+      base::StrCat({HashToHex(metric_name_hash), value});
   uint64_t digest;
   CHECK(hmac.Sign(salted_value, reinterpret_cast<uint8_t*>(&digest),
                   sizeof(digest)));

components/metrics/structured/key_data.h

@@ -18,31 +18,31 @@ namespace internal {
 // KeyData is the central class for managing keys and generating hashes for
 // structured metrics.
 //
-// The class maintains one key and its rotation data for every event defined
+// The class maintains one key and its rotation data for every project defined
 // in /tools/metrics/structured.xml. This can be used to generate:
-//  - a user ID for the event with KeyData::UserEventId.
-//  - a hash of a given value for the event with KeyData::Hash.
+//  - a user ID for the project with KeyData::UserProjectId.
+//  - a hash of a given value for an event with KeyData::HmacMetric.
 //
-// KeyData performs key rotation. Every event is associated with a rotation
+// KeyData performs key rotation. Every project is associated with a rotation
 // period, which is 90 days unless specified in structured.xml. Keys are rotated
-// with a resolution of one day. They are guaranteed not to be used for Hash or
-// UserEventId for longer than their rotation period, except in cases of local
-// clock changes.
+// with a resolution of one day. They are guaranteed not to be used for
+// HmacMetric or UserProjectId for longer than their rotation period, except in
+// cases of local clock changes.
 //
-// When first created, every event's key rotation date is selected uniformly so
-// that there is an even distribution of rotations across users. This means
+// When first created, every project's key rotation date is selected uniformly
+// so that there is an even distribution of rotations across users. This means
 // that, for most users, the first rotation period will be shorter than the
-// standard full rotation period for that event.
+// standard full rotation period for that project.
 //
 // Key storage is backed by a JsonPrefStore which is passed to the ctor and must
-// outlive the KeyData instance. Within the pref store, each event has three
+// outlive the KeyData instance. Within the pref store, each project has three
 // pieces of associated data:
-//  - the rotation period for this event in days.
+//  - the rotation period for this project in days.
 //  - the day of the last key rotation, as a day since the unix epoch.
 //  - the key itself.
 //
 // This is stored in the structure:
-//   keys.{event_name_hash}.rotation_period
+//   keys.{project_name_hash}.rotation_period
 //                         .last_rotation
 //                         .key
 //
@@ -57,50 +57,51 @@ class KeyData {
 
   // Returns a digest of |value| for |metric| in the context of
   // |project_name_hash|. Terminology: a metric is a (name, value) pair, and an
-  // event is a bundle of metrics.
+  // event is a bundle of metrics. Each event is associated with a project.
   //
-  //  - |project_name_hash| is the uint64 name hash of an event or project.
-  //  - |metric| is the uint64 name hash of a metric within |project_name_hash|.
+  //  - |project_name_hash| is the uint64 name hash of a project.
+  //  - |metric_name_hash| is the uint64 name hash of a metric.
   //  - |value| is the string value to hash.
   //
   // The result is the HMAC digest of the |value| salted with |metric|, using
   // the key for |project_name_hash|. That is:
   //
-  //   HMAC_SHA256(key(project_name_hash), concat(value, hex(metric)))
+  //   HMAC_SHA256(key(project_name_hash), concat(value, hex(event),
+  //   hex(metric)))
   //
   // Returns 0u in case of an error.
-  uint64_t HashForEventMetric(uint64_t project_name_hash,
-                              uint64_t metric,
+  uint64_t HmacMetric(uint64_t project_name_hash,
+                      uint64_t metric_name_hash,
                       const std::string& value);
 
   // Returns an ID for this (user, |project_name_hash|) pair.
-  // |project_name_hash| is the name of an event or project, represented by the
-  // first 8 bytes of the MD5 hash of its name defined  in structured.xml.
+  // |project_name_hash| is the name of a project, represented by the first 8
+  // bytes of the MD5 hash of its name defined in structured.xml.
   //
   // The derived ID is the first 8 bytes of SHA256(key(project_name_hash)).
   // Returns 0u in case of an error.
   //
-  // This ID is intended as the only ID for a particular structured metrics
-  // event. However, events are uploaded from the device alongside the UMA
-  // client ID, which is only removed after the event reaches the server. This
-  // means events are associated with the client ID when uploaded from the
-  // device. See the class comment of StructuredMetricsProvider for more
-  // details.
-  uint64_t UserEventId(uint64_t project_name_hash);
+  // This ID is intended as the only ID for the events of a particular
+  // structured metrics project. However, events are uploaded from the device
+  // alongside the UMA client ID, which is only removed after the event reaches
+  // the server. This means events are associated with the client ID when
+  // uploaded from the device. See the class comment of
+  // StructuredMetricsProvider for more details.
+  uint64_t UserProjectId(uint64_t project_name_hash);
 
  private:
-  int GetRotationPeriod(uint64_t event);
-  void SetRotationPeriod(uint64_t event, int rotation_period);
+  int GetRotationPeriod(uint64_t project);
+  void SetRotationPeriod(uint64_t project, int rotation_period);
 
-  int GetLastRotation(uint64_t event);
-  void SetLastRotation(uint64_t event, int last_rotation);
+  int GetLastRotation(uint64_t project);
+  void SetLastRotation(uint64_t project, int last_rotation);
 
-  // Ensure that a valid key exists for |event|, and return it. Either returns a
-  // string of size |kKeySize| or base::nullopt, which indicates an error.
+  // Ensure that a valid key exists for |project|, and return it. Either returns
+  // a string of size |kKeySize| or base::nullopt, which indicates an error.
   base::Optional<std::string> ValidateAndGetKey(uint64_t project_name_hash);
-  void SetKey(uint64_t event, const std::string& key);
+  void SetKey(uint64_t project, const std::string& key);
 
-  // Ensure that valid keys exist for all events.
+  // Ensure that valid keys exist for all projects.
   void ValidateKeys();
 
   // Storage for keys and rotation data. Must outlive the KeyData instance.

components/metrics/structured/key_data_unittest.cc

@@ -232,9 +232,8 @@ TEST_F(KeyDataTest, ReuseExistingKeys) {
 // value.
 TEST_F(KeyDataTest, DifferentEventsDifferentHashes) {
   StandardSetup();
-  EXPECT_NE(
-      key_data_->HashForEventMetric(kProjectOneHash, kMetricOneHash, "value"),
-      key_data_->HashForEventMetric(kProjectTwoHash, kMetricOneHash, "value"));
+  EXPECT_NE(key_data_->HmacMetric(kProjectOneHash, kMetricOneHash, "value"),
+            key_data_->HmacMetric(kProjectTwoHash, kMetricOneHash, "value"));
   ExpectNoErrors();
 }
 
@@ -242,9 +241,8 @@ TEST_F(KeyDataTest, DifferentEventsDifferentHashes) {
 // value.
 TEST_F(KeyDataTest, DifferentMetricsDifferentHashes) {
   StandardSetup();
-  EXPECT_NE(
-      key_data_->HashForEventMetric(kProjectOneHash, kMetricOneHash, "value"),
-      key_data_->HashForEventMetric(kProjectOneHash, kMetricTwoHash, "value"));
+  EXPECT_NE(key_data_->HmacMetric(kProjectOneHash, kMetricOneHash, "value"),
+            key_data_->HmacMetric(kProjectOneHash, kMetricTwoHash, "value"));
   ExpectNoErrors();
 }
 
@@ -252,9 +250,8 @@ TEST_F(KeyDataTest, DifferentMetricsDifferentHashes) {
 // metric.
 TEST_F(KeyDataTest, DifferentValuesDifferentHashes) {
   StandardSetup();
-  EXPECT_NE(
-      key_data_->HashForEventMetric(kProjectOneHash, kMetricOneHash, "first"),
-      key_data_->HashForEventMetric(kProjectOneHash, kMetricOneHash, "second"));
+  EXPECT_NE(key_data_->HmacMetric(kProjectOneHash, kMetricOneHash, "first"),
+            key_data_->HmacMetric(kProjectOneHash, kMetricOneHash, "second"));
   ExpectNoErrors();
 }
 
@@ -265,8 +262,8 @@ TEST_F(KeyDataTest, CheckUserIDs) {
   CommitKeyStore();
 
   MakeKeyData();
-  EXPECT_EQ(HashToHex(key_data_->UserEventId(kProjectOneHash)), kUserId);
-  EXPECT_NE(HashToHex(key_data_->UserEventId(kProjectTwoHash)), kUserId);
+  EXPECT_EQ(HashToHex(key_data_->UserProjectId(kProjectOneHash)), kUserId);
+  EXPECT_NE(HashToHex(key_data_->UserProjectId(kProjectTwoHash)), kUserId);
   ExpectNoErrors();
 }
 
@@ -278,11 +275,11 @@ TEST_F(KeyDataTest, CheckHashes) {
   CommitKeyStore();
 
   MakeKeyData();
-  EXPECT_EQ(HashToHex(key_data_->HashForEventMetric(kProjectOneHash,
-                                                    kMetricOneHash, kValueOne)),
+  EXPECT_EQ(HashToHex(key_data_->HmacMetric(kProjectOneHash, kMetricOneHash,
+                                            kValueOne)),
             kValueOneHash);
-  EXPECT_EQ(HashToHex(key_data_->HashForEventMetric(kProjectOneHash,
-                                                    kMetricTwoHash, kValueTwo)),
+  EXPECT_EQ(HashToHex(key_data_->HmacMetric(kProjectOneHash, kMetricTwoHash,
+                                            kValueTwo)),
             kValueTwoHash);
   ExpectNoErrors();
 }
@@ -296,7 +293,7 @@ TEST_F(KeyDataTest, KeysRotated) {
   // logic.
 
   StandardSetup();
-  const uint64_t first_id = key_data_->UserEventId(kProjectOneHash);
+  const uint64_t first_id = key_data_->UserProjectId(kProjectOneHash);
   const int start_day = (base::Time::Now() - base::Time::UnixEpoch()).InDays();
 
   // TestEventOne has a default rotation period of 90 days.
@@ -310,7 +307,7 @@ TEST_F(KeyDataTest, KeysRotated) {
     key_data_.reset();
     time_.Advance(base::TimeDelta::FromDays(50));
     StandardSetup();
-    EXPECT_EQ(key_data_->UserEventId(kProjectOneHash), first_id);
+    EXPECT_EQ(key_data_->UserProjectId(kProjectOneHash), first_id);
     EXPECT_EQ(GetInt(LastRotationPath(kProjectOneHash)), start_day);
   }
 
@@ -320,7 +317,7 @@ TEST_F(KeyDataTest, KeysRotated) {
     key_data_.reset();
     time_.Advance(base::TimeDelta::FromDays(50));
     StandardSetup();
-    EXPECT_NE(key_data_->UserEventId(kProjectOneHash), first_id);
+    EXPECT_NE(key_data_->UserProjectId(kProjectOneHash), first_id);
     EXPECT_EQ(GetInt(LastRotationPath(kProjectOneHash)), start_day + 90);
   }
 

components/metrics/structured/structured_metrics_provider.cc

@@ -190,8 +190,8 @@ void StructuredMetricsProvider::OnRecord(const EventBase& event) {
     if (metric.type == EventBase::MetricType::kString) {
       // Store hashed values as strings, because the JSON parser only retains 53
       // bits of precision for ints. This would corrupt the hashes.
-      name_value.SetStringKey(
-          "value", base::NumberToString(key_data_->HashForEventMetric(
+      name_value.SetStringKey("value",
+                              base::NumberToString(key_data_->HmacMetric(
                                   event.project_name_hash(), metric.name_hash,
                                   metric.string_value)));
     } else if (metric.type == EventBase::MetricType::kInt) {
@@ -205,9 +205,8 @@ void StructuredMetricsProvider::OnRecord(const EventBase& event) {
   // ID that will eventually be used as the profile_event_id of this event.
   base::Value event_value(base::Value::Type::DICTIONARY);
   event_value.SetStringKey("name", base::NumberToString(event.name_hash()));
-  event_value.SetStringKey(
-      "id",
-      base::NumberToString(key_data_->UserEventId(event.project_name_hash())));
+  event_value.SetStringKey("id", base::NumberToString(key_data_->UserProjectId(
+                                     event.project_name_hash())));
   event_value.SetKey("metrics", std::move(metrics));
 
   // Add the event to |storage_|.