[Structured metrics] Add KeyData class.

Adds the central class for managing project secrets and hashing values.

There two TODOs left for KeyData:
 1. UMA metrics for failure states.
 2. allow overriding of key rotation period per-event.

I'll add these in follow-up CLs, as they both require edits to other
files.

Bug: 1016655
Change-Id: I7a294fd9be606aa6fc510f4f25058e54ab3d327d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928451
Commit-Queue: Tony Yeoman <tby@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Thanh Nguyen <thanhdng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733793}

components/metrics/structured/BUILD.gn

@@ -10,6 +10,8 @@ static_library("structured") {
   sources = [
     "event_base.cc",
     "event_base.h",
+    "key_data.cc",
+    "key_data.h",
     "recorder.cc",
     "recorder.h",
     "structured_metrics_provider.cc",
@@ -22,13 +24,17 @@ static_library("structured") {
     "//base",
     "//components/metrics",
     "//components/prefs",
+    "//crypto",
     "//tools/metrics/structured:structured_events",
   ]
 }
 
 source_set("unit_tests") {
   testonly = true
-  sources = [ "structured_metrics_provider_unittest.cc" ]
+  sources = [
+    "key_data_unittest.cc",
+    "structured_metrics_provider_unittest.cc",
+  ]
 
   deps = [
     ":structured",

components/metrics/structured/DEPS

 include_rules = [
   "+components/metrics",
   "+components/prefs",
+  "+tools/metrics/structured",
 ]

components/metrics/structured/key_data.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/metrics/structured/key_data.h"
+
+#include <memory>
+
+#include "base/rand_util.h"
+#include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/time/time.h"
+#include "base/unguessable_token.h"
+#include "base/values.h"
+#include "components/prefs/json_pref_store.h"
+#include "crypto/hmac.h"
+#include "crypto/sha2.h"
+#include "tools/metrics/structured/structured_events.h"
+
+namespace metrics {
+namespace structured {
+namespace internal {
+namespace {
+
+// The expected size of a key, in bytes.
+constexpr size_t kKeySize = 32;
+
+// The default maximum number of days before rotating keys.
+constexpr size_t kDefaultRotationPeriod = 90;
+
+// Generates a key, which is the string representation of
+// base::UnguessableToken, and is of size |kKeySize| bytes.
+std::string GenerateKey() {
+  const std::string key = base::UnguessableToken::Create().ToString();
+  DCHECK_EQ(key.size(), kKeySize);
+  return key;
+}
+
+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 LastRotationPath(const uint64_t event) {
+  return base::StrCat({"keys.", base::NumberToString(event), ".last_rotation"});
+}
+
+std::string RotationPeriodPath(const uint64_t event) {
+  return base::StrCat(
+      {"keys.", base::NumberToString(event), ".rotation_period"});
+}
+
+}  // namespace
+
+KeyData::KeyData(JsonPrefStore* key_store) : key_store_(key_store) {
+  DCHECK(key_store_);
+  ValidateKeys();
+}
+
+KeyData::~KeyData() = default;
+
+base::Optional<std::string> KeyData::ValidateAndGetKey(const uint64_t event) {
+  DCHECK(key_store_);
+  const int now = (base::Time::Now() - base::Time::UnixEpoch()).InDays();
+
+  // If the key for |key_path| doesn't exist, initialize new key data. Set the
+  // last rotation to a uniformly selected day between today and
+  // |kDefaultRotationPeriod| days ago, to uniformly distribute users amongst
+  // rotation cohorts.
+  if (!key_store_->GetValue(KeyPath(event), nullptr)) {
+    const int rotation_seed = base::RandInt(0, kDefaultRotationPeriod - 1);
+    SetRotationPeriod(event, kDefaultRotationPeriod);
+    SetLastRotation(event, now - rotation_seed);
+    SetKey(event, GenerateKey());
+  }
+
+  // If the key for |event| is outdated, generate a new key and write it to
+  // the |keys| pref store along with updated rotation data. Update the last
+  // rotation such that the user stays in the same cohort.
+  const int rotation_period = GetRotationPeriod(event);
+  const int last_rotation = GetLastRotation(event);
+  if (now - last_rotation > rotation_period) {
+    const int new_last_rotation = now - (now - last_rotation) % rotation_period;
+    SetLastRotation(event, new_last_rotation);
+    SetKey(event, GenerateKey());
+  }
+
+  const base::Value* key_json;
+  if (!(key_store_->GetValue(KeyPath(event), &key_json) &&
+        key_json->is_string())) {
+    // TODO(crbug.com/1016655): log an error to UMA.
+    return base::nullopt;
+  }
+
+  const std::string key = key_json->GetString();
+  if (key.size() != kKeySize) {
+    // TODO(crbug.com/1016655): log an error to UMA.
+    return base::nullopt;
+  }
+
+  return key;
+}
+
+void KeyData::ValidateKeys() {
+  for (const uint64_t event : metrics::structured::events::kEventNameHashes) {
+    ValidateAndGetKey(event);
+  }
+}
+
+void KeyData::SetLastRotation(const uint64_t event, const int last_rotation) {
+  return key_store_->SetValue(LastRotationPath(event),
+                              std::make_unique<base::Value>(last_rotation),
+                              WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
+}
+
+void KeyData::SetRotationPeriod(const uint64_t event,
+                                const int rotation_period) {
+  return key_store_->SetValue(RotationPeriodPath(event),
+                              std::make_unique<base::Value>(rotation_period),
+                              WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
+}
+
+void KeyData::SetKey(const uint64_t event, const std::string& key) {
+  return key_store_->SetValue(KeyPath(event),
+                              std::make_unique<base::Value>(key),
+                              WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
+}
+
+int KeyData::GetLastRotation(const uint64_t event) {
+  const base::Value* value;
+  if (!(key_store_->GetValue(LastRotationPath(event), &value) &&
+        value->is_int())) {
+    // TODO(crbug.com/1016655): log an error to UMA.
+    DCHECK(false);
+    return 0u;
+  }
+  return value->GetInt();
+}
+
+int KeyData::GetRotationPeriod(const uint64_t event) {
+  const base::Value* value;
+  if (!(key_store_->GetValue(RotationPeriodPath(event), &value) &&
+        value->is_int())) {
+    // TODO(crbug.com/1016655): log an error to UMA.
+    DCHECK(false);
+    return 0u;
+  }
+  return value->GetInt();
+}
+
+uint64_t KeyData::UserEventId(const uint64_t event) {
+  // Retrieve the key for |event|.
+  const base::Optional<std::string> key = ValidateAndGetKey(event);
+  if (!key) {
+    // TODO(crbug.com/1016655): log an error to UMA.
+    return 0u;
+  }
+
+  // Compute and return the hash.
+  uint64_t hash;
+  crypto::SHA256HashString(key.value(), &hash, sizeof(uint64_t));
+  return hash;
+}
+
+uint64_t KeyData::HashForEventMetric(const uint64_t event,
+                                     const uint64_t metric,
+                                     const std::string& value) {
+  // Retrieve the key for |event|.
+  const base::Optional<std::string> key = ValidateAndGetKey(event);
+  if (!key) {
+    // TODO(crbug.com/1016655): log an error to UMA.
+    DCHECK(false);
+    return 0u;
+  }
+
+  // Initialize the HMAC.
+  crypto::HMAC hmac(crypto::HMAC::HashAlgorithm::SHA256);
+  CHECK(hmac.Init(key.value()));
+
+  // Compute and return the digest.
+  const std::string salted_value = base::StrCat({HashToHex(metric), value});
+  uint64_t digest;
+  CHECK(hmac.Sign(salted_value, reinterpret_cast<uint8_t*>(&digest),
+                  sizeof(digest)));
+  return digest;
+}
+
+}  // namespace internal
+}  // namespace structured
+}  // namespace metrics

components/metrics/structured/key_data.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_METRICS_STRUCTURED_KEY_DATA_H_
+#define COMPONENTS_METRICS_STRUCTURED_KEY_DATA_H_
+
+#include <string>
+
+#include "base/optional.h"
+
+class JsonPrefStore;
+
+namespace metrics {
+namespace structured {
+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
+// 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.
+//
+// KeyData performs key rotation. Every event 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.
+//
+// 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
+// that, for most users, the first rotation period will be shorter than the
+// standard full rotation period for that event.
+//
+// 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
+// pieces of associated data:
+//  - the rotation period for this event 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
+//                         .last_rotation
+//                         .key
+//
+// TODO(crbug.com/1016655): log UMA error metrics
+// TODO(crbug.com/1016655): add ability to override default rotation period
+class KeyData {
+ public:
+  explicit KeyData(JsonPrefStore* key_store);
+  ~KeyData();
+
+  KeyData(const KeyData&) = delete;
+  KeyData& operator=(const KeyData&) = delete;
+
+  // Returns a digest of |value| for |metric| in the context of |event|.
+  // Terminology: a metric is a (name, value) pair, and an event is a bundle of
+  // metrics.
+  //
+  //  - |event| is the uint64 name hash of an event.
+  //  - |metric| is the uint64 name hash of a metric within |event|.
+  //  - |value| is the string value to hash.
+  //
+  // The result is the HMAC digest of the |value| salted with |metric|, using
+  // the key for |event|. That is:
+  //
+  //   HMAC_SHA256(key(event), concat(value, hex(metric)))
+  //
+  // Returns 0u in case of an error.
+  uint64_t HashForEventMetric(uint64_t event,
+                              uint64_t metric,
+                              const std::string& value);
+
+  // Returns an ID for this (user, |event|) pair. |event| is the name of an
+  // event, 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(event)). 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 event);
+
+ private:
+  int GetRotationPeriod(uint64_t event);
+  void SetRotationPeriod(uint64_t event, int rotation_period);
+
+  int GetLastRotation(uint64_t event);
+  void SetLastRotation(uint64_t event, 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.
+  base::Optional<std::string> ValidateAndGetKey(uint64_t event);
+  void SetKey(uint64_t event, const std::string& key);
+
+  // Ensure that valid keys exist for all events.
+  void ValidateKeys();
+
+  // Storage for keys and rotation data. Must outlive the KeyData instance.
+  JsonPrefStore* key_store_;
+};
+
+}  // namespace internal
+}  // namespace structured
+}  // namespace metrics
+
+#endif  // COMPONENTS_METRICS_STRUCTURED_KEY_DATA_H_

components/metrics/structured/key_data_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/metrics/structured/key_data.h"
+
+#include <memory>
+#include <string>
+
+#include "base/containers/flat_set.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/test/scoped_mock_clock_override.h"
+#include "base/test/task_environment.h"
+#include "base/values.h"
+#include "components/metrics/structured/event_base.h"
+#include "components/metrics/structured/recorder.h"
+#include "components/prefs/json_pref_store.h"
+#include "components/prefs/persistent_pref_store.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace metrics {
+namespace structured {
+namespace internal {
+
+namespace {
+
+// 32 byte long test key, matching the size of a real key.
+constexpr char kKey[] = "abcdefghijklmnopqrstuvwxyzabcdef";
+
+// These event and metric names are used for testing.
+// - event: TestEventOne
+//   - metric: TestValueOne
+//   - metric: TestValueTwo
+// - event: TestEventTwo
+//   - metric: TestValueOne
+
+// The name hash of "TestEventOne".
+constexpr uint64_t kEventOneHash = UINT64_C(15619026293081468407);
+// The name hash of "TestEventTwo".
+constexpr uint64_t kEventTwoHash = UINT64_C(15791833939776536363);
+
+// The name hash of "TestMetricOne".
+constexpr uint64_t kMetricOneHash = UINT64_C(637929385654885975);
+// The name hash of "TestMetricTwo".
+constexpr uint64_t kMetricTwoHash = UINT64_C(14083999144141567134);
+
+// The hex-encoded frst 8 bytes of SHA256(kKey), ie. the user ID for key kKey.
+constexpr char kUserId[] = "2070DF23E0D95759";
+
+// Test values and their hashes. Hashes are the first 8 bytes of:
+//
+// HMAC_SHA256(concat(hex(kMetricNHash), kValueN),
+// "abcdefghijklmnopqrstuvwxyzabcdef")
+constexpr char kValueOne[] = "value one";
+constexpr char kValueTwo[] = "value two";
+constexpr char kValueOneHash[] = "805B8790DC69B773";
+constexpr char kValueTwoHash[] = "87CEF12FB15E0B3A";
+
+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 LastRotationPath(const uint64_t event) {
+  return base::StrCat({"keys.", base::NumberToString(event), ".last_rotation"});
+}
+
+std::string RotationPeriodPath(const uint64_t event) {
+  return base::StrCat(
+      {"keys.", base::NumberToString(event), ".rotation_period"});
+}
+
+}  // namespace
+
+class KeyDataTest : public testing::Test {
+ protected:
+  void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
+
+  void StandardSetup() {
+    MakeKeyStore();
+    MakeKeyData();
+    CommitKeyStore();
+  }
+
+  void ResetState() {
+    key_data_.reset();
+    key_store_.reset();
+    base::DeleteFile(GetKeyStorePath(), false);
+    ASSERT_FALSE(base::PathExists(GetKeyStorePath()));
+  }
+
+  void MakeKeyStore() {
+    key_store_ = new JsonPrefStore(GetKeyStorePath());
+    key_store_->ReadPrefs();
+  }
+
+  void MakeKeyData() { key_data_ = std::make_unique<KeyData>(GetKeyStore()); }
+
+  void CommitKeyStore() {
+    key_store_->CommitPendingWrite();
+    Wait();
+    ASSERT_TRUE(base::PathExists(GetKeyStorePath()));
+  }
+
+  JsonPrefStore* GetKeyStore() { return key_store_.get(); }
+
+  base::FilePath GetKeyStorePath() {
+    return temp_dir_.GetPath().Append("keys.json");
+  }
+
+  std::string GetString(const std::string& path) {
+    const base::Value* value;
+    GetKeyStore()->GetValue(path, &value);
+    return value->GetString();
+  }
+
+  int GetInt(const std::string& path) {
+    const base::Value* value;
+    GetKeyStore()->GetValue(path, &value);
+    return value->GetInt();
+  }
+
+  void SetString(const std::string& path, const std::string& value) {
+    key_store_->SetValue(path, std::make_unique<base::Value>(value),
+                         WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
+    CommitKeyStore();
+  }
+
+  void SetInt(const std::string& path, const int value) {
+    key_store_->SetValue(path, std::make_unique<base::Value>(value),
+                         WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
+    CommitKeyStore();
+  }
+
+  void SetKeyData(const uint64_t event,
+                  const std::string& key,
+                  const int last_rotation,
+                  const int rotation_period) {
+    SetString(KeyPath(event), key);
+    SetInt(LastRotationPath(event), last_rotation);
+    SetInt(RotationPeriodPath(event), rotation_period);
+  }
+
+  void Wait() { task_environment_.RunUntilIdle(); }
+
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::MainThreadType::UI,
+      base::test::TaskEnvironment::ThreadPoolExecutionMode::QUEUED};
+  base::ScopedTempDir temp_dir_;
+  base::ScopedMockClockOverride time_;
+  scoped_refptr<JsonPrefStore> key_store_;
+
+  std::unique_ptr<KeyData> key_data_;
+};
+
+// If there is no key store file present, check that new keys are generated for
+// each event, and those keys are of the right length and different from each
+// other.
+TEST_F(KeyDataTest, GeneratesKeysForEvents) {
+  StandardSetup();
+  const std::string key_one = GetString(KeyPath(kEventOneHash));
+  const std::string key_two = GetString(KeyPath(kEventTwoHash));
+
+  EXPECT_EQ(key_one.size(), 32ul);
+  EXPECT_EQ(key_two.size(), 32ul);
+  EXPECT_NE(key_one, key_two);
+}
+
+// When repeatedly initialized with no key store file present, ensure the keys
+// generated each time are distinct.
+TEST_F(KeyDataTest, GeneratesDistinctKeys) {
+  base::flat_set<std::string> keys;
+
+  for (int i = 0; i < 10; ++i) {
+    ResetState();
+    StandardSetup();
+    keys.insert(GetString(KeyPath(kEventOneHash)));
+  }
+
+  EXPECT_EQ(keys.size(), 10ul);
+}
+
+// If there is an existing key store file, check that its keys are not replaced.
+TEST_F(KeyDataTest, ReuseExistingKeys) {
+  StandardSetup();
+  const std::string key_one = GetString(KeyPath(kEventOneHash));
+  CommitKeyStore();
+
+  key_data_.reset();
+  key_store_.reset();
+  StandardSetup();
+  const std::string key_two = GetString(KeyPath(kEventOneHash));
+
+  EXPECT_EQ(key_one, key_two);
+}
+
+// Check that different events have different hashes for the same metric and
+// value.
+TEST_F(KeyDataTest, DifferentEventsDifferentHashes) {
+  StandardSetup();
+  // Even though
+  EXPECT_NE(
+      key_data_->HashForEventMetric(kEventOneHash, kMetricOneHash, "value"),
+      key_data_->HashForEventMetric(kEventTwoHash, kMetricOneHash, "value"));
+}
+
+// Check that an event has different hashes for different values of the same
+// metric.
+TEST_F(KeyDataTest, DifferentMetricsDifferentHashes) {
+  StandardSetup();
+  EXPECT_NE(
+      key_data_->HashForEventMetric(kEventOneHash, kMetricOneHash, "first"),
+      key_data_->HashForEventMetric(kEventOneHash, kMetricOneHash, "second"));
+}
+
+// Check that an event has different hashes for different metrics with the same
+// value.
+TEST_F(KeyDataTest, DifferentValuesDifferentHashes) {
+  StandardSetup();
+  EXPECT_NE(
+      key_data_->HashForEventMetric(kEventOneHash, kMetricOneHash, "value"),
+      key_data_->HashForEventMetric(kEventOneHash, kMetricTwoHash, "value"));
+}
+
+// Ensure that KeyData::UserId is the expected value of SHA256(key).
+TEST_F(KeyDataTest, CheckUserIDs) {
+  MakeKeyStore();
+  SetKeyData(kEventOneHash, kKey, 0, 90);
+  CommitKeyStore();
+
+  MakeKeyData();
+  EXPECT_EQ(HashToHex(key_data_->UserEventId(kEventOneHash)), kUserId);
+  EXPECT_NE(HashToHex(key_data_->UserEventId(kEventTwoHash)), kUserId);
+}
+
+// Ensure that KeyData::Hash returns expected values for a known key and value.
+TEST_F(KeyDataTest, CheckHashes) {
+  MakeKeyStore();
+  SetString(KeyPath(kEventOneHash), kKey);
+  SetKeyData(kEventOneHash, kKey, 0, 90);
+  CommitKeyStore();
+
+  MakeKeyData();
+  EXPECT_EQ(HashToHex(key_data_->HashForEventMetric(kEventOneHash,
+                                                    kMetricOneHash, kValueOne)),
+            kValueOneHash);
+  EXPECT_EQ(HashToHex(key_data_->HashForEventMetric(kEventOneHash,
+                                                    kMetricTwoHash, kValueTwo)),
+            kValueTwoHash);
+}
+
+// Check that keys for a event are correctly rotated after the default 90 day
+// rotation period.
+TEST_F(KeyDataTest, KeysRotated) {
+  StandardSetup();
+  const uint64_t first_id = key_data_->UserEventId(kEventOneHash);
+  const int start_day = (base::Time::Now() - base::Time::UnixEpoch()).InDays();
+
+  // TestEventOne has a default rotation period of 90 days.
+  EXPECT_EQ(GetInt(RotationPeriodPath(kEventOneHash)), 90);
+
+  // Set the last rotation to today for testing.
+  SetInt(LastRotationPath(kEventOneHash), start_day);
+
+  {
+    // Advancing by 50 days, the key should not be rotated.
+    key_data_.reset();
+    time_.Advance(base::TimeDelta::FromDays(50));
+    StandardSetup();
+    EXPECT_EQ(key_data_->UserEventId(kEventOneHash), first_id);
+    EXPECT_EQ(GetInt(LastRotationPath(kEventOneHash)), start_day);
+  }
+
+  {
+    // Advancing by another 50 days, the key should be rotated and the last
+    // rotation day should be incremented by 90.
+    key_data_.reset();
+    time_.Advance(base::TimeDelta::FromDays(50));
+    StandardSetup();
+    EXPECT_NE(key_data_->UserEventId(kEventOneHash), first_id);
+    EXPECT_EQ(GetInt(LastRotationPath(kEventOneHash)), start_day + 90);
+  }
+
+  {
+    // Advancing by 453 days, the last rotation day should now 6 periods of 90
+    // days ahead.
+    key_data_.reset();
+    time_.Advance(base::TimeDelta::FromDays(453));
+    StandardSetup();
+    EXPECT_EQ(GetInt(LastRotationPath(kEventOneHash)), start_day + 6 * 90);
+  }
+}
+
+}  // namespace internal
+}  // namespace structured
+}  // namespace metrics

components/metrics/structured/structured_metrics_provider.h

@@ -28,6 +28,13 @@ namespace structured {
 // thread safe and should only be called on the browser UI sequence, because
 // calls from the metrics service come on the UI sequence.
 //
+// Each structured metrics event is sent with other UMA data, and so is
+// associated with the UMA client ID when received by the UMA server. The client
+// ID is stripped from the events after they reach the server, and so data at
+// rest is not attached to the client ID. However, please note that structured
+// events are *not* separated from the client ID at the point of upload from
+// the device.
+//
 // Currently, the structured metrics system is cros-only and relies on the cros
 // cryptohome to store keys and unsent logs, collectively called 'state'. This
 // means structured metrics collection cannot begin until a profile eligible

tools/metrics/structured/structured.xml

@@ -2,22 +2,29 @@
 
 <!-- Structured metrics is under development and isn't available for use yet. -->
 
-<event name="TestEvent">
+<event name="TestEventOne">
   <owner>tby@chromium.org</owner>
   <summary>
     Event for unit testing, do not use.
   </summary>
-  <metric name="ValueOne" kind="hashed-string">
+  <metric name="TestMetricOne" kind="hashed-string">
     <summary>
       A per-user keyed hashed value.
     </summary>
   </metric>
-  <metric name="ValueThree" kind="int">
+  <metric name="TestMetricTwo" kind="int">
     <summary>
       An unhashed value, recorded as-is.
     </summary>
   </metric>
-  <metric name="ValueTwo" kind="hashed-string">
+</event>
+
+<event name="TestEventTwo">
+  <owner>tby@chromium.org</owner>
+  <summary>
+    Event for unit testing, do not use.
+  </summary>
+  <metric name="TestMetricOne" kind="hashed-string">
     <summary>
       A per-user keyed hashed value.
     </summary>