[Event Sequence] Add reset counter to every sequenced event.
Client design doc: go/cros-events-client-dd
Bug: 1350322
Change-Id: I1c332f45fb0427603cf832b2505e69d007aee341
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4148192
Reviewed-by: Tony Yeoman <[email protected]>
Commit-Queue: Jong Ahn <[email protected]>
Reviewed-by: Dominic Battré <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1094257}
diff --git a/chrome/browser/metrics/structured/BUILD.gn b/chrome/browser/metrics/structured/BUILD.gn
index df9cd4f..f4a8a0f 100644
--- a/chrome/browser/metrics/structured/BUILD.gn
+++ b/chrome/browser/metrics/structured/BUILD.gn
@@ -13,6 +13,8 @@
"chrome_structured_metrics_recorder.h",
"cros_events_processor.cc",
"cros_events_processor.h",
+ "structured_metric_prefs.cc",
+ "structured_metric_prefs.h",
]
deps = [
@@ -33,6 +35,7 @@
]
deps += [
+ "//chrome/browser:browser_process",
"//chrome/browser/ash/crosapi",
"//components/user_manager:user_manager",
]
diff --git a/chrome/browser/metrics/structured/chrome_structured_metrics_recorder.cc b/chrome/browser/metrics/structured/chrome_structured_metrics_recorder.cc
index 3ebb0d07..25468d90 100644
--- a/chrome/browser/metrics/structured/chrome_structured_metrics_recorder.cc
+++ b/chrome/browser/metrics/structured/chrome_structured_metrics_recorder.cc
@@ -8,12 +8,12 @@
#include "base/no_destructor.h"
#include "base/task/sequenced_task_runner.h"
-#include "build/chromeos_buildflags.h"
#include "chrome/browser/metrics/structured/cros_events_processor.h"
#include "components/metrics/structured/histogram_util.h"
#include "components/metrics/structured/structured_metrics_features.h"
#if BUILDFLAG(IS_CHROMEOS_ASH)
+#include "chrome/browser/browser_process.h" // nogncheck
#include "chrome/browser/metrics/structured/ash_structured_metrics_recorder.h" // nogncheck
#elif BUILDFLAG(IS_CHROMEOS_LACROS)
#include "base/task/current_thread.h"
@@ -58,26 +58,23 @@
return chrome_recorder.get();
}
+#if BUILDFLAG(IS_CHROMEOS_ASH)
// static
void ChromeStructuredMetricsRecorder::RegisterLocalStatePrefs(
PrefRegistrySimple* registry) {
cros_event::CrOSEventsProcessor::RegisterLocalStatePrefs(registry);
}
-
-// static
-void ChromeStructuredMetricsRecorder::RegisterUserProfilePrefs(
- PrefRegistrySimple* registry) {
- cros_event::CrOSEventsProcessor::RegisterUserProfilePrefs(registry);
-}
+#endif
void ChromeStructuredMetricsRecorder::Initialize() {
+#if BUILDFLAG(IS_CHROMEOS_ASH)
// Adds CrOSEvents processor if feature is enabled.
if (base::FeatureList::IsEnabled(kEventSequenceLogging)) {
StructuredMetricsClient::Get()->AddEventsProcessor(
- std::make_unique<cros_event::CrOSEventsProcessor>());
+ std::make_unique<cros_event::CrOSEventsProcessor>(
+ g_browser_process->local_state()));
}
-#if BUILDFLAG(IS_CHROMEOS_ASH)
auto* ash_recorder =
static_cast<AshStructuredMetricsRecorder*>(delegate_.get());
ash_recorder->Initialize();
diff --git a/chrome/browser/metrics/structured/chrome_structured_metrics_recorder.h b/chrome/browser/metrics/structured/chrome_structured_metrics_recorder.h
index 795686d..9a6c53f 100644
--- a/chrome/browser/metrics/structured/chrome_structured_metrics_recorder.h
+++ b/chrome/browser/metrics/structured/chrome_structured_metrics_recorder.h
@@ -8,13 +8,13 @@
#include <memory>
#include "base/no_destructor.h"
+#include "build/chromeos_buildflags.h"
#include "components/metrics/structured/event.h"
#include "components/metrics/structured/structured_events.h"
#include "components/metrics/structured/structured_metrics_client.h"
#include "components/prefs/pref_registry_simple.h"
-namespace metrics {
-namespace structured {
+namespace metrics::structured {
namespace {
using RecordingDelegate = StructuredMetricsClient::RecordingDelegate;
@@ -33,9 +33,13 @@
// Pointer to singleton.
static ChromeStructuredMetricsRecorder* Get();
+#if BUILDFLAG(IS_CHROMEOS_ASH)
// Registers prefs.
+ //
+ // TODO(crbug/1350322): Once reset counter is available to use from platform2,
+ // remove this and use the more relaible reset counter.
static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
- static void RegisterUserProfilePrefs(PrefRegistrySimple* registry);
+#endif
ChromeStructuredMetricsRecorder(
const ChromeStructuredMetricsRecorder& recorder) = delete;
@@ -67,7 +71,6 @@
bool is_initialized_ = false;
};
-} // namespace structured
-} // namespace metrics
+} // namespace metrics::structured
#endif // CHROME_BROWSER_METRICS_STRUCTURED_CHROME_STRUCTURED_METRICS_RECORDER_H_
diff --git a/chrome/browser/metrics/structured/cros_events_processor.cc b/chrome/browser/metrics/structured/cros_events_processor.cc
index f008a06..68046fb0 100644
--- a/chrome/browser/metrics/structured/cros_events_processor.cc
+++ b/chrome/browser/metrics/structured/cros_events_processor.cc
@@ -4,22 +4,57 @@
#include "chrome/browser/metrics/structured/cros_events_processor.h"
+#include "base/logging.h"
+#include "base/system/sys_info.h"
+#include "chrome/browser/metrics/structured/structured_metric_prefs.h"
+
namespace metrics::structured::cros_event {
+CrOSEventsProcessor::CrOSEventsProcessor(PrefService* pref_service)
+ : pref_service_(pref_service) {}
CrOSEventsProcessor::~CrOSEventsProcessor() = default;
// static
void CrOSEventsProcessor::RegisterLocalStatePrefs(
- PrefRegistrySimple* registry) {}
-
-// static
-void CrOSEventsProcessor::RegisterUserProfilePrefs(
- PrefRegistrySimple* registry) {}
+ PrefRegistrySimple* registry) {
+ // These prefs are modified multiple times per minute and should be registered
+ // as LOSSY_PREF since persisting prefs constantly is expensive.
+ registry->RegisterIntegerPref(kEventSequenceResetCounter, 0,
+ PrefRegistry::LOSSY_PREF);
+ registry->RegisterInt64Pref(kEventSequenceLastSystemUptime, 0,
+ PrefRegistry::LOSSY_PREF);
+}
bool CrOSEventsProcessor::ShouldProcessOnEventRecord(const Event& event) {
return event.IsEventSequenceType();
}
-void CrOSEventsProcessor::OnEventsRecord(Event* event) {}
+void CrOSEventsProcessor::SetCurrentUptimeForTesting(int64_t current_uptime) {
+ current_uptime_for_testing_ = current_uptime;
+}
+
+void CrOSEventsProcessor::OnEventsRecord(Event* event) {
+ auto current_reset_counter =
+ pref_service_->GetInteger(kEventSequenceResetCounter);
+ auto last_system_uptime =
+ pref_service_->GetInt64(kEventSequenceLastSystemUptime);
+
+ auto current_uptime = current_uptime_for_testing_
+ ? current_uptime_for_testing_
+ : base::SysInfo::Uptime().InMilliseconds();
+
+ // If the last uptime is larger than the current uptime, a reset most likely
+ // happened.
+ if (last_system_uptime > current_uptime) {
+ ++current_reset_counter;
+ pref_service_->SetInteger(kEventSequenceResetCounter,
+ current_reset_counter);
+ }
+
+ pref_service_->SetInt64(kEventSequenceLastSystemUptime, current_uptime);
+
+ event->SetEventSequenceMetadata(
+ Event::EventSequenceMetadata(current_reset_counter));
+}
} // namespace metrics::structured::cros_event
diff --git a/chrome/browser/metrics/structured/cros_events_processor.h b/chrome/browser/metrics/structured/cros_events_processor.h
index 1328fa8..a68e642 100644
--- a/chrome/browser/metrics/structured/cros_events_processor.h
+++ b/chrome/browser/metrics/structured/cros_events_processor.h
@@ -7,21 +7,31 @@
#include "components/metrics/structured/events_processor_interface.h"
#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
namespace metrics::structured::cros_event {
+// Post-processor that will process only sequenceable events and attach metadata
+// to the events.
class CrOSEventsProcessor : public EventsProcessorInterface {
public:
+ explicit CrOSEventsProcessor(PrefService* pref_service);
~CrOSEventsProcessor() override;
// Registers device-level prefs.
static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
- // Registers user-level prefs.
- static void RegisterUserProfilePrefs(PrefRegistrySimple* registry);
-
+ // EventsProcessorInterface:
bool ShouldProcessOnEventRecord(const Event& event) override;
void OnEventsRecord(Event* event) override;
+
+ // Used only to set the current uptime to check against for testing.
+ // If this value is not set explicitly, system clock will be used.
+ void SetCurrentUptimeForTesting(int64_t current_uptime);
+
+ private:
+ PrefService* pref_service_;
+ int64_t current_uptime_for_testing_ = 0;
};
} // namespace metrics::structured::cros_event
diff --git a/chrome/browser/metrics/structured/cros_events_processor_unittest.cc b/chrome/browser/metrics/structured/cros_events_processor_unittest.cc
new file mode 100644
index 0000000..9efd05a
--- /dev/null
+++ b/chrome/browser/metrics/structured/cros_events_processor_unittest.cc
@@ -0,0 +1,81 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/metrics/structured/cros_events_processor.h"
+
+#include "base/time/time.h"
+#include "chrome/browser/metrics/structured/structured_metric_prefs.h"
+#include "components/metrics/structured/structured_events.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace metrics::structured::cros_event {
+
+namespace {
+
+class CrOSEventsProcessorTest : public testing::Test {
+ protected:
+ void SetUp() override {
+ processor_ = std::make_unique<CrOSEventsProcessor>(&test_pref_service_);
+ CrOSEventsProcessor::RegisterLocalStatePrefs(test_pref_service_.registry());
+ }
+
+ void Initialize(int64_t last_uptime, int reset_counter) {
+ test_pref_service_.SetInteger(kEventSequenceResetCounter, reset_counter);
+ test_pref_service_.SetInt64(kEventSequenceLastSystemUptime, last_uptime);
+ }
+
+ int GetResetCounter() {
+ return test_pref_service_.GetInteger(kEventSequenceResetCounter);
+ }
+
+ int64_t GetLastSystemUptime() {
+ return test_pref_service_.GetInt64(kEventSequenceLastSystemUptime);
+ }
+
+ TestingPrefServiceSimple test_pref_service_;
+ std::unique_ptr<cros_event::CrOSEventsProcessor> processor_;
+};
+
+} // namespace
+
+TEST_F(CrOSEventsProcessorTest, CheckResetCounterUpdatedOnReset) {
+ int64_t last_uptime = 20;
+ int reset_counter = 10;
+
+ Initialize(last_uptime, reset_counter);
+ // Sets the current uptime to be less than the last uptime to emulate a reset
+ // counter scenario.
+ processor_->SetCurrentUptimeForTesting(last_uptime - 1);
+
+ events::v2::cr_os_events::Test1 test_event;
+ test_event.SetRecordedTimeSinceBoot(base::Milliseconds(last_uptime));
+ processor_->OnEventsRecord(&test_event);
+
+ EXPECT_EQ(test_event.event_sequence_metadata().reset_counter,
+ reset_counter + 1);
+ EXPECT_EQ(GetResetCounter(), reset_counter + 1);
+ EXPECT_EQ(GetLastSystemUptime(), last_uptime - 1);
+}
+
+TEST_F(CrOSEventsProcessorTest, ResetCounterNotUpdated) {
+ int64_t last_uptime = 20;
+ int reset_counter = 10;
+
+ Initialize(last_uptime, reset_counter);
+ // Sets the current uptime to be greater than the last uptime to emulate a
+ // scenario where reset counter should not be incremented.
+ processor_->SetCurrentUptimeForTesting(last_uptime + 1);
+
+ events::v2::cr_os_events::Test1 test_event;
+ test_event.SetRecordedTimeSinceBoot(base::Milliseconds(last_uptime));
+ processor_->OnEventsRecord(&test_event);
+
+ EXPECT_EQ(test_event.event_sequence_metadata().reset_counter, reset_counter);
+ EXPECT_EQ(GetResetCounter(), reset_counter);
+ EXPECT_EQ(GetLastSystemUptime(), last_uptime + 1);
+}
+
+} // namespace metrics::structured::cros_event
diff --git a/chrome/browser/metrics/structured/structured_metric_prefs.cc b/chrome/browser/metrics/structured/structured_metric_prefs.cc
new file mode 100644
index 0000000..6d329dc
--- /dev/null
+++ b/chrome/browser/metrics/structured/structured_metric_prefs.cc
@@ -0,0 +1,21 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/metrics/structured/structured_metric_prefs.h"
+
+namespace metrics::structured {
+
+// Keeps track of the system uptime of the last recorded uptime. This is used to
+// determine whether the reset counter or not.
+//
+// TODO(crbug/1350322): Remove this and kEventSequenceResetCounter once
+// implementation for handling resets for CrOS is implemented in platform2.
+const char kEventSequenceLastSystemUptime[] =
+ "metrics.event_sequence.last_system_uptime";
+
+// Keeps track of the device reset counter.
+const char kEventSequenceResetCounter[] =
+ "metrics.event_sequence.reset_counter";
+
+} // namespace metrics::structured
diff --git a/chrome/browser/metrics/structured/structured_metric_prefs.h b/chrome/browser/metrics/structured/structured_metric_prefs.h
new file mode 100644
index 0000000..fc6861d
--- /dev/null
+++ b/chrome/browser/metrics/structured/structured_metric_prefs.h
@@ -0,0 +1,16 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_METRICS_STRUCTURED_STRUCTURED_METRIC_PREFS_H_
+#define CHROME_BROWSER_METRICS_STRUCTURED_STRUCTURED_METRIC_PREFS_H_
+
+namespace metrics::structured {
+
+// Document each in the .cc file.
+extern const char kEventSequenceResetCounter[];
+extern const char kEventSequenceLastSystemUptime[];
+
+} // namespace metrics::structured
+
+#endif // CHROME_BROWSER_METRICS_STRUCTURED_STRUCTURED_METRIC_PREFS_H_
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 576bf34..bb8cc82d 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -399,6 +399,7 @@
#include "chrome/browser/extensions/extension_assets_manager_chromeos.h"
#include "chrome/browser/media/protected_media_identifier_permission_context.h"
#include "chrome/browser/metrics/chromeos_metrics_provider.h"
+#include "chrome/browser/metrics/structured/chrome_structured_metrics_recorder.h"
#include "chrome/browser/ui/ash/shelf/chrome_shelf_prefs.h"
#include "chrome/browser/ui/webui/ash/login/enable_debugging_screen_handler.h"
#include "chrome/browser/ui/webui/ash/login/signin_screen_handler.h"
@@ -1271,6 +1272,8 @@
registry);
extensions::login_api::RegisterLocalStatePrefs(registry);
::onc::RegisterPrefs(registry);
+ metrics::structured::ChromeStructuredMetricsRecorder::RegisterLocalStatePrefs(
+ registry);
policy::ActiveDirectoryDeviceStateUploader::RegisterLocalStatePrefs(registry);
policy::ActiveDirectoryMigrationManager::RegisterLocalStatePrefs(registry);
policy::AdbSideloadingAllowanceModePolicyHandler::RegisterPrefs(registry);
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index c2c6f634..c427dc0 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -7502,6 +7502,7 @@
"../browser/metrics/perf/process_type_collector_unittest.cc",
"../browser/metrics/perf/profile_provider_chromeos_unittest.cc",
"../browser/metrics/perf/windowed_incognito_observer_unittest.cc",
+ "../browser/metrics/structured/cros_events_processor_unittest.cc",
"../browser/nearby_sharing/bluetooth_advertising_interval_client_unittest.cc",
"../browser/nearby_sharing/fast_initiation/fast_initiation_advertiser_unittest.cc",
"../browser/nearby_sharing/fast_initiation/fast_initiation_scanner_feature_usage_metrics_unittest.cc",
@@ -7655,6 +7656,7 @@
"//chrome/browser/chromeos",
"//chrome/browser/chromeos/launcher_search:search_util",
"//chrome/browser/enterprise/connectors/device_trust/attestation/ash",
+ "//chrome/browser/metrics/structured",
"//chrome/browser/nearby_sharing:share_target",
"//chrome/browser/nearby_sharing/certificates",
"//chrome/browser/nearby_sharing/certificates:test_support",
@@ -7740,6 +7742,7 @@
"//components/app_restore:unit_tests",
"//components/arc:arc_test_support",
"//components/arc/common",
+ "//components/metrics/structured:structured_events",
"//components/omnibox/browser:vector_icons",
"//components/services/app_service/public/cpp:app_update",
"//components/services/app_service/public/cpp:instance_update",
