Reland "[SystemProfile] Expand default SystemProfileSelectionPolicy"
This relands commit 4737f70ea0cfa0f064c8cbafd21da9c42a1367ac.
This is step 2 of the plan in http://fxbug.dev/85440
This sets defaults for report types whose default should be REPORT_ALL.
Other metric types must manually specify SystemProfileSelectionPolicy.
Bug: 85440
Change-Id: Iba0c54da389181f0464ba76917cfa42bf45f1888
Reviewed-on: https://fuchsia-review.googlesource.com/c/cobalt/+/602370
Fuchsia-Auto-Submit: Zach Bush <zmbush@google.com>
Reviewed-by: Cameron Dale <camrdale@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
diff --git a/src/bin/config_parser/src/config_parser/expand_defaults.go b/src/bin/config_parser/src/config_parser/expand_defaults.go
index 77cb480..d6001eb 100644
--- a/src/bin/config_parser/src/config_parser/expand_defaults.go
+++ b/src/bin/config_parser/src/config_parser/expand_defaults.go
@@ -25,8 +25,23 @@
}
}
+var DefaultSystemProfileSelectionPolicy = map[config.ReportDefinition_ReportType]config.SystemProfileSelectionPolicy{
+ config.ReportDefinition_FLEETWIDE_OCCURRENCE_COUNTS: config.SystemProfileSelectionPolicy_REPORT_ALL,
+ config.ReportDefinition_FLEETWIDE_HISTOGRAMS: config.SystemProfileSelectionPolicy_REPORT_ALL,
+ config.ReportDefinition_FLEETWIDE_MEANS: config.SystemProfileSelectionPolicy_REPORT_ALL,
+ config.ReportDefinition_UNIQUE_DEVICE_NUMERIC_STATS: config.SystemProfileSelectionPolicy_REPORT_ALL,
+ config.ReportDefinition_HOURLY_VALUE_NUMERIC_STATS: config.SystemProfileSelectionPolicy_REPORT_ALL,
+ config.ReportDefinition_STRING_COUNTS: config.SystemProfileSelectionPolicy_REPORT_ALL,
+}
+
func expandDefaultsForCobalt11Report(report *config.ReportDefinition) {
if report.LocalAggregationProcedure == config.ReportDefinition_SELECT_FIRST && report.EventVectorBufferMax == 0 {
report.EventVectorBufferMax = 1
}
+
+ if report.SystemProfileSelection == config.SystemProfileSelectionPolicy_SELECT_DEFAULT {
+ if def, ok := DefaultSystemProfileSelectionPolicy[report.ReportType]; ok {
+ report.SystemProfileSelection = def
+ }
+ }
}
diff --git a/src/bin/config_parser/src/config_parser/expand_defaults_test.go b/src/bin/config_parser/src/config_parser/expand_defaults_test.go
index 65575e3..10b74e8 100644
--- a/src/bin/config_parser/src/config_parser/expand_defaults_test.go
+++ b/src/bin/config_parser/src/config_parser/expand_defaults_test.go
@@ -2,6 +2,7 @@
import (
"config"
+ "fmt"
"testing"
)
@@ -33,3 +34,57 @@
t.Errorf("Expanded EventVectorBufferMax default for non SELECT_FIRST procedure")
}
}
+
+var expandDefaultsTest = []struct {
+ ty config.ReportDefinition_ReportType
+ policy config.SystemProfileSelectionPolicy
+}{
+ {ty: config.ReportDefinition_FLEETWIDE_OCCURRENCE_COUNTS, policy: config.SystemProfileSelectionPolicy_REPORT_ALL},
+ {ty: config.ReportDefinition_FLEETWIDE_HISTOGRAMS, policy: config.SystemProfileSelectionPolicy_REPORT_ALL},
+ {ty: config.ReportDefinition_FLEETWIDE_MEANS, policy: config.SystemProfileSelectionPolicy_REPORT_ALL},
+ {ty: config.ReportDefinition_UNIQUE_DEVICE_NUMERIC_STATS, policy: config.SystemProfileSelectionPolicy_REPORT_ALL},
+ {ty: config.ReportDefinition_HOURLY_VALUE_NUMERIC_STATS, policy: config.SystemProfileSelectionPolicy_REPORT_ALL},
+ {ty: config.ReportDefinition_STRING_COUNTS, policy: config.SystemProfileSelectionPolicy_REPORT_ALL},
+
+ {ty: config.ReportDefinition_UNIQUE_DEVICE_COUNTS, policy: config.SystemProfileSelectionPolicy_SELECT_DEFAULT},
+ {ty: config.ReportDefinition_UNIQUE_DEVICE_HISTOGRAMS, policy: config.SystemProfileSelectionPolicy_SELECT_DEFAULT},
+ {ty: config.ReportDefinition_HOURLY_VALUE_HISTOGRAMS, policy: config.SystemProfileSelectionPolicy_SELECT_DEFAULT},
+}
+
+func TestExpandSystemProfileSelectionDefaults(t *testing.T) {
+ for _, tt := range expandDefaultsTest {
+ t.Run(fmt.Sprintf("ReportType_%v", tt.ty), func(t *testing.T) {
+ rep := config.ReportDefinition{
+ ReportType: tt.ty,
+ SystemProfileSelection: config.SystemProfileSelectionPolicy_SELECT_DEFAULT,
+ }
+
+ expandDefaultsForCobalt11Report(&rep)
+
+ if rep.SystemProfileSelection != tt.policy {
+ t.Errorf("saw unexpected SystemProfileSelection, got %v, expected %v", rep.SystemProfileSelection, tt.policy)
+ }
+ })
+ }
+}
+
+func TestNoOverrideSystemProfileSelectionDefaults(t *testing.T) {
+ for _, tt := range expandDefaultsTest {
+ t.Run(fmt.Sprintf("ReportType_%v", tt.ty), func(t *testing.T) {
+ rep := config.ReportDefinition{
+ ReportType: tt.ty,
+ SystemProfileSelection: config.SystemProfileSelectionPolicy_SELECT_FIRST,
+ }
+ if tt.policy == rep.SystemProfileSelection {
+ rep.SystemProfileSelection = config.SystemProfileSelectionPolicy_SELECT_LAST
+ }
+
+ expandDefaultsForCobalt11Report(&rep)
+
+ if rep.SystemProfileSelection == tt.policy {
+ t.Errorf("explicitly set SystemProfileSelection got overridden")
+ }
+ })
+ }
+
+}
diff --git a/src/local_aggregation_1_1/BUILD.gn b/src/local_aggregation_1_1/BUILD.gn
index d90ec25..17f2af1 100644
--- a/src/local_aggregation_1_1/BUILD.gn
+++ b/src/local_aggregation_1_1/BUILD.gn
@@ -51,8 +51,8 @@
"aggregation_procedures:tests",
"local_aggregate_storage",
"local_aggregate_storage:tests",
- "testing:report_all_test_registry_default_system_profile_selection",
"testing:report_all_test_registry_with_report_all_set",
+ "testing:report_all_test_registry_with_select_last_set",
"//third_party/googletest:gmock",
"//third_party/googletest:gtest",
]
diff --git a/src/local_aggregation_1_1/local_aggregation_test.cc b/src/local_aggregation_1_1/local_aggregation_test.cc
index f19f95e..6260fb0 100644
--- a/src/local_aggregation_1_1/local_aggregation_test.cc
+++ b/src/local_aggregation_1_1/local_aggregation_test.cc
@@ -13,8 +13,8 @@
#include "absl/strings/str_cat.h"
#include "src/lib/util/clock.h"
#include "src/lib/util/testing/test_with_files.h"
-#include "src/local_aggregation_1_1/testing/report_all_test_registry_default_system_profile_selection.cb.h"
#include "src/local_aggregation_1_1/testing/report_all_test_registry_with_report_all_set.cb.h"
+#include "src/local_aggregation_1_1/testing/report_all_test_registry_with_select_last_set.cb.h"
#include "src/local_aggregation_1_1/testing/test_registry.cb.h"
#include "src/logger/event_record.h"
#include "src/logger/logger_test_utils.h"
@@ -944,18 +944,17 @@
}
TEST_F(LocalAggregationTest, MigrateToReportAll) {
- OverrideRegistry(report_all_test::default_system_profile_selection::kRegistryBase64);
+ OverrideRegistry(report_all_test::with_select_last_set::kRegistryBase64);
std::shared_ptr<const logger::ProjectContext> project_context = GetProjectContext();
std::unique_ptr<LocalAggregation> aggregation = MakeLocalAggregation();
uint32_t day_index;
{
- auto record =
- logger::EventRecord::MakeEventRecord(
- project_context,
- report_all_test::default_system_profile_selection::kOccurrenceMetricReportAllMetricId)
- .ValueOrDie();
+ auto record = logger::EventRecord::MakeEventRecord(
+ project_context,
+ report_all_test::with_select_last_set::kOccurrenceMetricReportAllMetricId)
+ .ValueOrDie();
record->event()->mutable_occurrence_event()->set_count(1);
time_t now = std::chrono::system_clock::to_time_t(mock_clock_->now());
day_index = cobalt::util::TimeToDayIndex(now, record->metric()->time_zone_policy());
@@ -1040,15 +1039,14 @@
ASSERT_EQ(aggregation->AddEvent(*record, mock_clock_->now()).error_code(), StatusCode::OK);
}
- OverrideRegistry(report_all_test::default_system_profile_selection::kRegistryBase64);
+ OverrideRegistry(report_all_test::with_select_last_set::kRegistryBase64);
project_context = GetProjectContext();
aggregation = MakeLocalAggregation();
{
- auto record =
- logger::EventRecord::MakeEventRecord(
- project_context,
- report_all_test::default_system_profile_selection::kOccurrenceMetricReportAllMetricId)
- .ValueOrDie();
+ auto record = logger::EventRecord::MakeEventRecord(
+ project_context,
+ report_all_test::with_select_last_set::kOccurrenceMetricReportAllMetricId)
+ .ValueOrDie();
record->event()->mutable_occurrence_event()->set_count(1);
time_t now = std::chrono::system_clock::to_time_t(mock_clock_->now());
day_index = cobalt::util::TimeToDayIndex(now, record->metric()->time_zone_policy());
@@ -1070,13 +1068,13 @@
EXPECT_EQ(test_writer_->metadata_received[0]->customer_id(), kCustomerId);
lib::MetricIdentifier metric_id = project_context->Identifier().ForMetric(
- report_all_test::default_system_profile_selection::kOccurrenceMetricReportAllMetricId);
+ report_all_test::with_select_last_set::kOccurrenceMetricReportAllMetricId);
// All 4 metrics should be grouped back together under system_version 101 (the most recently
// seen).
VerifyStoredIntegerObservation(
0,
- metric_id.ForReport(report_all_test::default_system_profile_selection::
+ metric_id.ForReport(report_all_test::with_select_last_set::
kOccurrenceMetricReportAllUniqueDeviceCounts1DayReportAllReportId),
day_index, "101", {4});
}
diff --git a/src/local_aggregation_1_1/testing/BUILD.gn b/src/local_aggregation_1_1/testing/BUILD.gn
index ae6e408..096e94d 100644
--- a/src/local_aggregation_1_1/testing/BUILD.gn
+++ b/src/local_aggregation_1_1/testing/BUILD.gn
@@ -19,15 +19,15 @@
generate_cc = true
}
-metrics_registry("report_all_test_registry_default_system_profile_selection") {
+metrics_registry("report_all_test_registry_with_select_last_set") {
customer_id = 123
project_id = 100
features = [
"generate-config-base64",
"testing",
]
- source = "report_all_test_registry/default_system_profile_selection.yaml"
- namespace = "cobalt.report_all_test.default_system_profile_selection"
+ source = "report_all_test_registry/with_select_last_set.yaml"
+ namespace = "cobalt.report_all_test.with_select_last_set"
var_name = "registry_base64"
generate_cc = true
diff --git a/src/local_aggregation_1_1/testing/report_all_test_registry/default_system_profile_selection.yaml b/src/local_aggregation_1_1/testing/report_all_test_registry/with_select_last_set.yaml
similarity index 91%
rename from src/local_aggregation_1_1/testing/report_all_test_registry/default_system_profile_selection.yaml
rename to src/local_aggregation_1_1/testing/report_all_test_registry/with_select_last_set.yaml
index ee56ec0..4f4beea 100644
--- a/src/local_aggregation_1_1/testing/report_all_test_registry/default_system_profile_selection.yaml
+++ b/src/local_aggregation_1_1/testing/report_all_test_registry/with_select_last_set.yaml
@@ -11,6 +11,7 @@
report_name: unique_device_counts_1_day_report_all
report_type: UNIQUE_DEVICE_NUMERIC_STATS
local_aggregation_period: WINDOW_1_DAY
+ system_profile_selection: SELECT_LAST
privacy_level: NO_ADDED_PRIVACY
event_vector_buffer_max: 100
system_profile_field: [SYSTEM_VERSION]
diff --git a/src/logger/test_registries/cobalt1.1_test_registry.yaml b/src/logger/test_registries/cobalt1.1_test_registry.yaml
index b9cadbc..545d425 100644
--- a/src/logger/test_registries/cobalt1.1_test_registry.yaml
+++ b/src/logger/test_registries/cobalt1.1_test_registry.yaml
@@ -43,9 +43,11 @@
- report_name: "StreamingTime_Aggregation"
id: 1
report_type: FLEETWIDE_MEANS
+ system_profile_selection: SELECT_LAST
- report_name: "StreamingTime_Aggregation_Two"
id: 2
report_type: FLEETWIDE_MEANS
+ system_profile_selection: SELECT_LAST
- id: 3
metric_name: "FileSystemWriteTimes"
@@ -63,6 +65,7 @@
- report_name: "FileSystemWriteTimes_Histogram"
id: 1
report_type: FLEETWIDE_HISTOGRAMS
+ system_profile_selection: SELECT_LAST
int_buckets:
linear:
floor: 0
@@ -71,6 +74,7 @@
- report_name: "FileSystemWriteTimes_Histogram_Two"
id: 2
report_type: FLEETWIDE_HISTOGRAMS
+ system_profile_selection: SELECT_LAST
int_buckets:
linear:
floor: 0
@@ -88,6 +92,8 @@
- report_name: "Component_Histogram"
id: 1
report_type: STRING_COUNTS
+ system_profile_selection: SELECT_LAST
- report_name: "Component_Histogram_Two"
id: 2
report_type: STRING_COUNTS
+ system_profile_selection: SELECT_LAST
