Reland "[SystemProfile] Don't allow default SystemProfileSelectionPolicy"

This relands commit 6c2c160d8affdffefcd474801ce7e1291ceb8ace.

Add check in validator that SystemProfileSelectionPolicy is manually set
for the following report types (This is step 3 of the plan in
http://fxbug.dev/85440):

- UNIQUE_DEVICE_COUNTS
- UNIQUE_DEVICE_HISTOGRAMS
- HOURLY_VALUE_HISTOGRAMS

Add check in validator that SystemProfileSelectionPolicy is always set
to REPORT_ALL for the following report types (This is step 4 of the plan
in http://fxbug.dev/85440):

- FLEETWIDE_OCCURRENCE_COUNTS
- FLEETWIDE_HISTOGRAMS
- FLEETWIDE_MEANS
- UNIQUE_DEVICE_NUMERIC_STATS
- HOURLY_VALUE_NUMERIC_STATS
- STRING_COUNTS

Bug: 85440
Change-Id: I1b3b51bb827df0e0cef063f4e476a369cd754385
Reviewed-on: https://fuchsia-review.googlesource.com/c/cobalt/+/602371
Reviewed-by: Cameron Dale <camrdale@google.com>
Commit-Queue: Zach Bush <zmbush@google.com>
Fuchsia-Auto-Submit: Zach Bush <zmbush@google.com>
diff --git a/src/bin/config_parser/src/config_validator/report_definitions.go b/src/bin/config_parser/src/config_validator/report_definitions.go
index cb4ca4d..cab9106 100644
--- a/src/bin/config_parser/src/config_validator/report_definitions.go
+++ b/src/bin/config_parser/src/config_validator/report_definitions.go
@@ -88,6 +88,26 @@
 	}
 }
 
+// For reports that must specify a policy, this is the suggestion we will give.
+var SuggestedSystemProfileSelectionPolicy = map[config.ReportDefinition_ReportType]config.SystemProfileSelectionPolicy{
+	config.ReportDefinition_HOURLY_VALUE_HISTOGRAMS:     config.SystemProfileSelectionPolicy_SELECT_LAST,
+	config.ReportDefinition_UNIQUE_DEVICE_COUNTS:        config.SystemProfileSelectionPolicy_SELECT_LAST,
+	config.ReportDefinition_UNIQUE_DEVICE_HISTOGRAMS:    config.SystemProfileSelectionPolicy_SELECT_LAST,
+	config.ReportDefinition_UNIQUE_DEVICE_STRING_COUNTS: config.SystemProfileSelectionPolicy_SELECT_LAST,
+}
+
+// These report types must not set 'system_profile_selection' in the registry.
+// The 'system_profile_selection' field must always be set to REPORT_ALL.
+// See: //src/bin/config_parser/src/config_parser/expand_defaults.go
+var ReportAllOnlyReports = map[config.ReportDefinition_ReportType]bool{
+	config.ReportDefinition_FLEETWIDE_OCCURRENCE_COUNTS: true,
+	config.ReportDefinition_FLEETWIDE_HISTOGRAMS:        true,
+	config.ReportDefinition_FLEETWIDE_MEANS:             true,
+	config.ReportDefinition_UNIQUE_DEVICE_NUMERIC_STATS: true,
+	config.ReportDefinition_HOURLY_VALUE_NUMERIC_STATS:  true,
+	config.ReportDefinition_STRING_COUNTS:               true,
+}
+
 func validateReportDefinitions(m config.MetricDefinition) error {
 	reportErrors := newValidationErrors("report")
 
@@ -110,6 +130,19 @@
 			reportErrors.addError(r.ReportName, err)
 			continue
 		}
+
+		if suggestion, ok := SuggestedSystemProfileSelectionPolicy[r.ReportType]; ok {
+			if r.SystemProfileSelection == config.SystemProfileSelectionPolicy_SELECT_DEFAULT {
+				reportErrors.addError(r.ReportName, fmt.Errorf("you must manually specify the system_profile_selection for this report (%v may be a good choice)", suggestion))
+			}
+		}
+
+		if _, ok := ReportAllOnlyReports[r.ReportType]; ok {
+			// N.B.: config_parser/expand_defaults.go is run before validation, so SELECT_DEFAULT will get expanded to REPORT_ALL for appropriate metrics.
+			if r.SystemProfileSelection != config.SystemProfileSelectionPolicy_REPORT_ALL {
+				reportErrors.addError(r.ReportName, fmt.Errorf("reports of type %v should not manually specify system_profile_selection. The default value is REPORT_ALL", r.ReportType))
+			}
+		}
 	}
 
 	return reportErrors.err()
diff --git a/src/local_aggregation_1_1/testing/BUILD.gn b/src/local_aggregation_1_1/testing/BUILD.gn
index 096e94d..4389b08 100644
--- a/src/local_aggregation_1_1/testing/BUILD.gn
+++ b/src/local_aggregation_1_1/testing/BUILD.gn
@@ -7,6 +7,7 @@
 import("$cobalt_root/metrics_registry.gni")
 
 metrics_registry("test_registry") {
+  skip_validation = true
   global = true
   features = [
     "generate-config-base64",
@@ -20,6 +21,7 @@
 }
 
 metrics_registry("report_all_test_registry_with_select_last_set") {
+  skip_validation = true
   customer_id = 123
   project_id = 100
   features = [
diff --git a/src/local_aggregation_1_1/testing/test_registry/CustomerA/ProjectA1/metrics.yaml b/src/local_aggregation_1_1/testing/test_registry/CustomerA/ProjectA1/metrics.yaml
index 5bb096b..bfb4007 100644
--- a/src/local_aggregation_1_1/testing/test_registry/CustomerA/ProjectA1/metrics.yaml
+++ b/src/local_aggregation_1_1/testing/test_registry/CustomerA/ProjectA1/metrics.yaml
@@ -13,6 +13,10 @@
         privacy_level: NO_ADDED_PRIVACY
         event_vector_buffer_max: 100
         system_profile_field: [OS, SYSTEM_VERSION]
+        # TODO(fxbug.dev/85440): This is an invalid configuration.
+        # system_profile_selection should not be set for
+        # FLEETWIDE_OCCURRENCE_COUNTS metrics, and requires setting
+        # skip_validation = true in BUILD.gn
         system_profile_selection: SELECT_LAST
       - id: 2
         report_name: "unique_device_counts_report_1_day"
@@ -320,6 +324,7 @@
         privacy_level: NO_ADDED_PRIVACY
         event_vector_buffer_max: 100
         system_profile_field: [OS, SYSTEM_VERSION]
+        system_profile_selection: SELECT_LAST
       - id: 2
         report_name: "at_least_once_7_day"
         report_type: UNIQUE_DEVICE_COUNTS
@@ -329,6 +334,7 @@
         privacy_level: NO_ADDED_PRIVACY
         event_vector_buffer_max: 100
         system_profile_field: [OS, SYSTEM_VERSION]
+        system_profile_selection: SELECT_LAST
       - id: 3
         report_name: "select_first_1_day"
         report_type: UNIQUE_DEVICE_COUNTS
@@ -337,6 +343,7 @@
         expedited_sending: true
         privacy_level: NO_ADDED_PRIVACY
         system_profile_field: [OS, SYSTEM_VERSION]
+        system_profile_selection: SELECT_LAST
       - id: 4
         report_name: "select_first_7_day"
         report_type: UNIQUE_DEVICE_COUNTS
@@ -345,6 +352,7 @@
         expedited_sending: true
         privacy_level: NO_ADDED_PRIVACY
         system_profile_field: [OS, SYSTEM_VERSION]
+        system_profile_selection: SELECT_LAST
       - id: 5
         report_name: "report_all_1_day"
         report_type: UNIQUE_DEVICE_COUNTS
diff --git a/src/logger/test_registries/cobalt1.1_migration_registry.yaml b/src/logger/test_registries/cobalt1.1_migration_registry.yaml
index be4574e..d3c3bbd 100644
--- a/src/logger/test_registries/cobalt1.1_migration_registry.yaml
+++ b/src/logger/test_registries/cobalt1.1_migration_registry.yaml
@@ -12,189 +12,189 @@
 ##########################################################################
 
 metric_definitions:
+  - id: 1
+    metric_name: event_occurred_old
+    metric_type: EVENT_OCCURRED
+    replacement_metric_id: 101
+    metric_dimensions: &dimensions
+      - dimension: color
+        event_codes:
+          0: Red
+          1: Green
+          2: Chartreuse
+        max_event_code: 7
+    meta_data: &meta_data
+      max_release_stage: GA
+      expiration_date: "2021/11/01"
 
-- id: 1
-  metric_name: event_occurred_old
-  metric_type: EVENT_OCCURRED
-  replacement_metric_id: 101
-  metric_dimensions: &dimensions
-    - dimension: color
-      event_codes:
-        0: Red
-        1: Green
-        2: Chartreuse
-      max_event_code: 7
-  meta_data: &meta_data
-    max_release_stage: GA
-    expiration_date: "2021/11/01"
+  - id: 101
+    metric_name: event_occured_new
+    metric_type: OCCURRENCE
+    metric_semantics: [USAGE_COUNTING]
+    metric_dimensions: *dimensions
+    reports:
+      - report_name: event_occurred_new_7_day_actives
+        id: 101
+        report_type: UNIQUE_DEVICE_COUNTS
+        privacy_level: NO_ADDED_PRIVACY
+        local_aggregation_procedure: AT_LEAST_ONCE
+        local_aggregation_period: WINDOW_7_DAYS
+        system_profile_selection: SELECT_LAST
+    meta_data: *meta_data
 
-- id: 101
-  metric_name: event_occured_new
-  metric_type: OCCURRENCE
-  metric_semantics: [USAGE_COUNTING]
-  metric_dimensions: *dimensions
-  reports:
-    - report_name: event_occurred_new_7_day_actives
-      id: 101
-      report_type: UNIQUE_DEVICE_COUNTS
-      privacy_level: NO_ADDED_PRIVACY
-      local_aggregation_procedure: AT_LEAST_ONCE
-      local_aggregation_period: WINDOW_7_DAYS
-  meta_data: *meta_data
+  - id: 2
+    metric_name: event_count
+    metric_type: EVENT_COUNT
+    replacement_metric_id: 102
+    metric_dimensions: *dimensions
+    meta_data: *meta_data
 
-- id: 2
-  metric_name: event_count
-  metric_type: EVENT_COUNT
-  replacement_metric_id: 102
-  metric_dimensions: *dimensions
-  meta_data: *meta_data
+  - id: 102
+    metric_name: event_count_new
+    metric_type: OCCURRENCE
+    metric_semantics: [USAGE_COUNTING]
+    metric_dimensions: *dimensions
+    reports:
+      - report_name: event_count_new_numeric_stats
+        id: 102
+        report_type: HOURLY_VALUE_NUMERIC_STATS
+        privacy_level: NO_ADDED_PRIVACY
+    meta_data: *meta_data
 
-- id: 102
-  metric_name: event_count_new
-  metric_type: OCCURRENCE
-  metric_semantics: [USAGE_COUNTING]
-  metric_dimensions: *dimensions
-  reports:
-    - report_name: event_count_new_numeric_stats
-      id: 102
-      report_type: HOURLY_VALUE_NUMERIC_STATS
-      privacy_level: NO_ADDED_PRIVACY
-  meta_data: *meta_data
+  - id: 8
+    metric_name: bytes_uploaded
+    metric_type: EVENT_COUNT
+    replacement_metric_id: 108
+    metric_dimensions: *dimensions
+    meta_data: *meta_data
 
-- id: 8
-  metric_name: bytes_uploaded
-  metric_type: EVENT_COUNT
-  replacement_metric_id: 108
-  metric_dimensions: *dimensions
-  meta_data: *meta_data
+  - id: 108
+    metric_name: bytes_uploaded_new
+    metric_type: INTEGER
+    metric_units: BYTES
+    metric_semantics: [NETWORK_COMMUNICATION, DATA_SIZE]
+    metric_dimensions: *dimensions
+    reports:
+      - report_name: numeric_stats
+        id: 108
+        local_aggregation_procedure: MAX_PROCEDURE
+        report_type: HOURLY_VALUE_NUMERIC_STATS
+        privacy_level: NO_ADDED_PRIVACY
+    meta_data: *meta_data
 
-- id: 108
-  metric_name: bytes_uploaded_new
-  metric_type: INTEGER
-  metric_units: BYTES
-  metric_semantics: [NETWORK_COMMUNICATION, DATA_SIZE]
-  metric_dimensions: *dimensions
-  reports:
-    - report_name: numeric_stats
-      id: 108
-      local_aggregation_procedure: MAX_PROCEDURE
-      report_type: HOURLY_VALUE_NUMERIC_STATS
-      privacy_level: NO_ADDED_PRIVACY
-  meta_data: *meta_data
+  - id: 3
+    metric_name: event_component_count
+    metric_type: EVENT_COUNT
+    replacement_metric_id: 103
+    metric_dimensions: *dimensions
+    meta_data: *meta_data
 
-- id: 3
-  metric_name: event_component_count
-  metric_type: EVENT_COUNT
-  replacement_metric_id: 103
-  metric_dimensions: *dimensions
-  meta_data: *meta_data
+  - id: 103
+    metric_name: event_component_count_new
+    metric_type: STRING
+    string_candidate_file: file
+    string_buffer_max: 5
+    metric_semantics: [USAGE_COUNTING]
+    metric_dimensions: *dimensions
+    reports:
+      - report_name: string_counst
+        id: 103
+        report_type: STRING_COUNTS
+        privacy_level: NO_ADDED_PRIVACY
+    meta_data: *meta_data
 
-- id: 103
-  metric_name: event_component_count_new
-  metric_type: STRING
-  string_candidate_file: file
-  string_buffer_max: 5
-  metric_semantics: [USAGE_COUNTING]
-  metric_dimensions: *dimensions
-  reports:
-    - report_name: string_counst
-      id: 103
-      report_type: STRING_COUNTS
-      privacy_level: NO_ADDED_PRIVACY
-  meta_data: *meta_data
+  - id: 4
+    metric_name: elapsed_time
+    metric_type: ELAPSED_TIME
+    replacement_metric_id: 104
+    metric_dimensions: *dimensions
+    meta_data: *meta_data
 
-- id: 4
-  metric_name: elapsed_time
-  metric_type: ELAPSED_TIME
-  replacement_metric_id: 104
-  metric_dimensions: *dimensions
-  meta_data: *meta_data
+  - id: 104
+    metric_name: new_elapsed_time
+    metric_type: INTEGER
+    metric_semantics: [LATENCY]
+    metric_units: MICROSECONDS
+    metric_dimensions: *dimensions
+    reports:
+      - id: 104
+        report_name: numeric_stats
+        report_type: UNIQUE_DEVICE_NUMERIC_STATS
+        privacy_level: NO_ADDED_PRIVACY
+        local_aggregation_procedure: MAX_PROCEDURE
+        local_aggregation_period: WINDOW_7_DAYS
+    meta_data: *meta_data
 
-- id: 104
-  metric_name: new_elapsed_time
-  metric_type: INTEGER
-  metric_semantics: [LATENCY]
-  metric_units: MICROSECONDS
-  metric_dimensions: *dimensions
-  reports:
-    - id: 104
-      report_name: numeric_stats
-      report_type: UNIQUE_DEVICE_NUMERIC_STATS
-      privacy_level: NO_ADDED_PRIVACY
-      local_aggregation_procedure: MAX_PROCEDURE
-      local_aggregation_period: WINDOW_7_DAYS
-  meta_data: *meta_data
+  - id: 5
+    metric_name: frame_rate
+    metric_type: FRAME_RATE
+    replacement_metric_id: 105
+    metric_dimensions: *dimensions
+    meta_data: *meta_data
 
-- id: 5
-  metric_name: frame_rate
-  metric_type: FRAME_RATE
-  replacement_metric_id: 105
-  metric_dimensions: *dimensions
-  meta_data: *meta_data
+  - id: 105
+    metric_name: new_frame_rate
+    metric_type: INTEGER
+    metric_semantics: [FRAME_RENDERING]
+    metric_units: SECONDS
+    metric_dimensions: *dimensions
+    reports:
+      - id: 105
+        report_name: numeric_stats
+        report_type: UNIQUE_DEVICE_NUMERIC_STATS
+        privacy_level: NO_ADDED_PRIVACY
+        local_aggregation_procedure: MAX_PROCEDURE
+        local_aggregation_period: WINDOW_7_DAYS
+    meta_data: *meta_data
 
-- id: 105
-  metric_name: new_frame_rate
-  metric_type: INTEGER
-  metric_semantics: [FRAME_RENDERING]
-  metric_units: SECONDS
-  metric_dimensions: *dimensions
-  reports:
-    - id: 105
-      report_name: numeric_stats
-      report_type: UNIQUE_DEVICE_NUMERIC_STATS
-      privacy_level: NO_ADDED_PRIVACY
-      local_aggregation_procedure: MAX_PROCEDURE
-      local_aggregation_period: WINDOW_7_DAYS
-  meta_data: *meta_data
+  - id: 6
+    metric_name: memory_usage
+    metric_type: MEMORY_USAGE
+    replacement_metric_id: 106
+    metric_dimensions: *dimensions
+    meta_data: *meta_data
 
-- id: 6
-  metric_name: memory_usage
-  metric_type: MEMORY_USAGE
-  replacement_metric_id: 106
-  metric_dimensions: *dimensions
-  meta_data: *meta_data
+  - id: 106
+    metric_name: new_memory_usage
+    metric_type: INTEGER
+    metric_semantics: [MEMORY_USAGE]
+    metric_units: KIBIBYTES
+    metric_dimensions: *dimensions
+    reports:
+      - id: 106
+        report_name: numeric_stats
+        report_type: UNIQUE_DEVICE_NUMERIC_STATS
+        privacy_level: NO_ADDED_PRIVACY
+        local_aggregation_procedure: MAX_PROCEDURE
+        local_aggregation_period: WINDOW_7_DAYS
+    meta_data: *meta_data
 
-- id: 106
-  metric_name: new_memory_usage
-  metric_type: INTEGER
-  metric_semantics: [MEMORY_USAGE]
-  metric_units: KIBIBYTES
-  metric_dimensions: *dimensions
-  reports:
-    - id: 106
-      report_name: numeric_stats
-      report_type: UNIQUE_DEVICE_NUMERIC_STATS
-      privacy_level: NO_ADDED_PRIVACY
-      local_aggregation_procedure: MAX_PROCEDURE
-      local_aggregation_period: WINDOW_7_DAYS
-  meta_data: *meta_data
+  - id: 7
+    metric_name: int_histogram
+    metric_type: INT_HISTOGRAM
+    int_buckets:
+      linear:
+        floor: 0
+        num_buckets: 80
+        step_size: 1
+    replacement_metric_id: 107
+    metric_dimensions: *dimensions
+    meta_data: *meta_data
 
-- id: 7
-  metric_name: int_histogram
-  metric_type: INT_HISTOGRAM
-  int_buckets:
-    linear:
-      floor: 0
-      num_buckets: 80
-      step_size: 1
-  replacement_metric_id: 107
-  metric_dimensions: *dimensions
-  meta_data: *meta_data
-
-- id: 107
-  metric_name: integer_histogram
-  metric_type: INTEGER_HISTOGRAM
-  metric_semantics: [OUTSIDE_ENVIRONMENT]
-  metric_units_other: "CELSIUS"
-  metric_dimensions: *dimensions
-  int_buckets:
-    linear:
-      floor: 0
-      num_buckets: 80
-      step_size: 1
-  reports:
-    - id: 107
-      report_name: fleetwide
-      report_type: FLEETWIDE_HISTOGRAMS
-      privacy_level: NO_ADDED_PRIVACY
-  meta_data: *meta_data
+  - id: 107
+    metric_name: integer_histogram
+    metric_type: INTEGER_HISTOGRAM
+    metric_semantics: [OUTSIDE_ENVIRONMENT]
+    metric_units_other: "CELSIUS"
+    metric_dimensions: *dimensions
+    int_buckets:
+      linear:
+        floor: 0
+        num_buckets: 80
+        step_size: 1
+    reports:
+      - id: 107
+        report_name: fleetwide
+        report_type: FLEETWIDE_HISTOGRAMS
+        privacy_level: NO_ADDED_PRIVACY
+    meta_data: *meta_data
diff --git a/src/registry/report_definition.proto b/src/registry/report_definition.proto
index 2b933a9..b3e7871 100644
--- a/src/registry/report_definition.proto
+++ b/src/registry/report_definition.proto
@@ -242,6 +242,7 @@
     // Local aggregation: COUNT
     // Local aggregation period: 1 hour
     // Global aggregation: OCCURRENCE_COUNTS
+    // System Profile Selection Policy: REPORT_ALL
     //
     // Output report row type: OccurrenceCountReportRow
     // (See report_row.proto)
@@ -289,6 +290,8 @@
     //   - local_aggregation_procedure
     //   - local_aggregation_period
     //   - expedited_sending
+    //   - system_profile_selection (SELECT_FIRST and SELECT LAST will maintain
+    //     uniqueness, REPORT_ALL may be useful in some cases)
     UNIQUE_DEVICE_COUNTS = 12;
 
     // For each system_profile SP and each event_vector EV, produces an
@@ -328,6 +331,8 @@
     //   - int_buckets (this is used only on the server for reports without
     //     added privacy, but is used on the client for reports with added
     //     privacy)
+    //   - system_profile_selection (SELECT_FIRST and SELECT LAST will maintain
+    //     uniqueness, REPORT_ALL may be useful in some cases)
     UNIQUE_DEVICE_HISTOGRAMS = 13;
 
     // For each system_profile SP and each event_vector EV, produces an
@@ -371,6 +376,8 @@
     //   - int_buckets (this is used only on the server for reports without
     //     added privacy, but is used on the client for reports with added
     //     privacy)
+    //   - system_profile_selection (SELECT_FIRST and SELECT LAST will maintain
+    //     uniqueness, REPORT_ALL may be useful in some cases)
     HOURLY_VALUE_HISTOGRAMS = 14;
 
     // For each system_profile SP and each event_vector EV, produces an
@@ -391,6 +398,7 @@
     // Local aggregation: INTEGER_HISTOGRAM
     // Local aggregation period: one hour
     // Global aggregation: INTEGER_HISTOGRAMS
+    // System Profile Selection Policy: REPORT_ALL
     //
     // Output report row type: IntegerHistogramReportRow
     // (See report_row.proto)
@@ -414,6 +422,7 @@
     // Local aggregation: SUM_AND_COUNT
     // Local aggregation period: one hour
     // Global aggregation: SUM_AND_COUNTS
+    // System Profile Selection Policy: REPORT_ALL
     //
     // Output report row type: SumAndCountReportRow
     // (See report_row.proto)
@@ -447,6 +456,7 @@
     //                    NUMERIC_STAT (used with INTEGER metrics)
     // Local aggregation period: DAYS_1, DAYS_7 or DAYS_30.
     // Global aggregation: NUMERIC_STATS
+    // System Profile Selection Policy: REPORT_ALL
     //
     // Output report row type: NumericStatsReportRow
     // (See report_row.proto)
@@ -485,6 +495,7 @@
     //                    NUMERIC_STAT (used with INTEGER metrics)
     // Local aggregation period: 1 hour
     // Global aggregation: NUMERIC_STATS
+    // System Profile Selection Policy: REPORT_ALL
     //
     // Output report row type: NumericStatsReportRow
     // (See report_row.proto)
@@ -503,6 +514,7 @@
     // Local aggregation: STRING_HISTOGRAM
     // Local aggregation period: 1 hour
     // Global aggregation: STRING_HISTOGRAMS
+    // System Profile Selection Policy: REPORT_ALL
     //
     // Output report row type: StringCountReportRow
     // (See report_row.proto)
@@ -923,15 +935,26 @@
   // default settings to determine the location of the exported report.
   string export_location_override = 102;
 
+  // This field is required for reports of type UNIQUE_DEVICE_COUNTS,
+  // UNIQUE_DEVICE_HISTOGRAMS, UNIQUE_DEVICE_STRING_COUNTS, and
+  // HOURLY_VALUE_HISTOGRAMS. The value for these reports must be SELECT_LAST,
+  // SELECT_FIRST, or occasionally REPORT_ALL.
+  //
   // If the system profile value changed during the aggregation window specified
   // for this report, system_profile_selection specifies which system profile to
   // report for each device.
+  //
+  // Note: This field should not be set for Cobalt 1.0 report types, as it will do nothing.
   SystemProfileSelectionPolicy system_profile_selection = 103;
 }
 
 // A specification for SystemProfile selection policy.
 enum SystemProfileSelectionPolicy {
-  // The default implementation. May change without notice.
+  // Use the default value. For reports of type FLEETWIDE_OCCURRENCE_COUNTS,
+  // FLEETWIDE_HISTOGRAMS, FLEETWIDE_MEANS, UNIQUE_DEVICE_NUMERIC_STATS,
+  // HOURLY_VALUE_NUMERIC_STATS, and STRING_COUNTS this will resolve to
+  // 'REPORT_ALL' and should not be changed. For all other report types,
+  // SELECT_DEFAULT must not be used.
   SELECT_DEFAULT = 0;
 
   // Always report the last SystemProfile seen in the aggregation_window. For
@@ -944,8 +967,16 @@
   // event* in the aggregation window.
   SELECT_FIRST = 2;
 
-  // Report all system profiles seen at the time of an event in the aggregation
-  // window. Only applicable to Cobalt 1.1 reports.
+  // Report all system profiles in the aggregation_window. For most report
+  // types, this is the most sensible value to use. For reports that depend on
+  // some concept of uniqueness (such as UNIQUE_DEVICE_COUNTS,
+  // UNIQUE_DEVICE_HISTOGRAMS, UNIQUE_DEVICE_STRING_COUNTS, and
+  // HOURLY_VALUE_HISTOGRAMS) this may not be the best choice, since it will no
+  // longer be the case that a single device will only upload one observation
+  // per time period (It will upload one observation per time period *per unique
+  // system_profile*).
+  //
+  // Only applicable to Cobalt 1.1 metrics.
   REPORT_ALL = 3;
 }