From e8ad58272e10377d24860dc7f2a186275619b1a4 Mon Sep 17 00:00:00 2001 From: Darnell Andries Date: Tue, 1 Oct 2024 16:04:55 -0700 Subject: [PATCH] Add metric for sync join --- components/brave_sync/brave_sync_p3a.cc | 14 ++++++++++ components/brave_sync/brave_sync_p3a.h | 24 ++++++++++++++++ .../brave_sync/brave_sync_p3a_unittest.cc | 28 +++++++++++++++++++ components/p3a/metric_names.h | 2 ++ .../sync/service/brave_sync_service_impl.cc | 3 ++ .../sync/service/brave_sync_service_impl.h | 3 ++ 6 files changed, 74 insertions(+) diff --git a/components/brave_sync/brave_sync_p3a.cc b/components/brave_sync/brave_sync_p3a.cc index ebec9ed467a6..b80cc0a20bb4 100644 --- a/components/brave_sync/brave_sync_p3a.cc +++ b/components/brave_sync/brave_sync_p3a.cc @@ -54,4 +54,18 @@ void RecordSyncedObjectsCount(int total_entities) { {1000, 10000, 49000}, total_entities); } +void SyncCodeMonitor::RecordCodeGenerated() { + code_generated_ = true; + base::UmaHistogramEnumeration(kSyncJoinTypeHistogramName, + SyncJoinType::kChainCreated); +} + +void SyncCodeMonitor::RecordCodeSet() { + if (!code_generated_) { + base::UmaHistogramEnumeration(kSyncJoinTypeHistogramName, + SyncJoinType::kChainJoined); + } + code_generated_ = false; +} + } // namespace brave_sync::p3a diff --git a/components/brave_sync/brave_sync_p3a.h b/components/brave_sync/brave_sync_p3a.h index 7c216bcbc4dc..b0cfdc00e74c 100644 --- a/components/brave_sync/brave_sync_p3a.h +++ b/components/brave_sync/brave_sync_p3a.h @@ -17,6 +17,7 @@ inline constexpr char kEnabledTypesHistogramName[] = "Brave.Sync.EnabledTypes"; // Improved version of metric which includes count of synced History objects inline constexpr char kSyncedObjectsCountHistogramNameV2[] = "Brave.Sync.SyncedObjectsCount.2"; +inline constexpr char kSyncJoinTypeHistogramName[] = "Brave.Sync.JoinType"; enum class EnabledTypesAnswer { kEmptyOrBookmarksOnly = 0, @@ -26,10 +27,33 @@ enum class EnabledTypesAnswer { kMaxValue = kAllTypes }; +enum class SyncJoinType { + kChainCreated = 1, + kChainJoined = 2, + kMaxValue = kChainJoined +}; + void RecordEnabledTypes(bool sync_everything_enabled, const syncer::UserSelectableTypeSet& selected_types); void RecordSyncedObjectsCount(int total_entities); +// Monitors sync code generation and setting events in order +// to report the `Brave.Sync.JoinType` metric. +class SyncCodeMonitor { + public: + SyncCodeMonitor() = default; + ~SyncCodeMonitor() = default; + + SyncCodeMonitor(const SyncCodeMonitor&) = delete; + SyncCodeMonitor& operator=(const SyncCodeMonitor&) = delete; + + void RecordCodeGenerated(); + void RecordCodeSet(); + + private: + bool code_generated_ = false; +}; + } // namespace p3a } // namespace brave_sync diff --git a/components/brave_sync/brave_sync_p3a_unittest.cc b/components/brave_sync/brave_sync_p3a_unittest.cc index 5ee468fe505d..7eba098567dd 100644 --- a/components/brave_sync/brave_sync_p3a_unittest.cc +++ b/components/brave_sync/brave_sync_p3a_unittest.cc @@ -75,4 +75,32 @@ TEST(BraveSyncP3ATest, TestSyncedObjectsCount) { histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramNameV2, 3, 1); } +TEST(BraveSyncP3ATest, TestSyncCodeMonitor) { + base::HistogramTester histogram_tester; + SyncCodeMonitor monitor; + + monitor.RecordCodeGenerated(); + histogram_tester.ExpectUniqueSample( + kSyncJoinTypeHistogramName, static_cast(SyncJoinType::kChainCreated), + 1); + + monitor.RecordCodeSet(); + histogram_tester.ExpectUniqueSample( + kSyncJoinTypeHistogramName, static_cast(SyncJoinType::kChainCreated), + 1); + + monitor.RecordCodeSet(); + histogram_tester.ExpectBucketCount( + kSyncJoinTypeHistogramName, static_cast(SyncJoinType::kChainJoined), + 1); + + SyncCodeMonitor monitor2; + monitor2.RecordCodeSet(); + histogram_tester.ExpectBucketCount( + kSyncJoinTypeHistogramName, static_cast(SyncJoinType::kChainJoined), + 2); + + histogram_tester.ExpectTotalCount(kSyncJoinTypeHistogramName, 3); +} + } // namespace brave_sync::p3a diff --git a/components/p3a/metric_names.h b/components/p3a/metric_names.h index 3505038633c5..08a0099aad4a 100644 --- a/components/p3a/metric_names.h +++ b/components/p3a/metric_names.h @@ -192,6 +192,7 @@ inline constexpr auto kCollectedTypicalHistograms = "Brave.Sidebar.SettingChange.SidebarEnabledB", "Brave.SpeedReader.Enabled", "Brave.SpeedReader.ToggleCount", + "Brave.Sync.JoinType", "Brave.Sync.ProgressTokenEverReset", "Brave.Sync.Status.2", "Brave.Today.ClickCardDepth", @@ -346,6 +347,7 @@ inline constexpr auto kEphemeralHistograms = "Brave.Search.QueriesBeforeChurn", "Brave.Search.WidgetUsage", "Brave.Shields.AllowScriptOnce", + "Brave.Sync.JoinType", "Brave.Today.ChannelCount.2", "Brave.Today.ClickCardDepth", "Brave.Today.DirectFeedsTotal.3", diff --git a/components/sync/service/brave_sync_service_impl.cc b/components/sync/service/brave_sync_service_impl.cc index 8c99a58a6d81..ae526c78a609 100644 --- a/components/sync/service/brave_sync_service_impl.cc +++ b/components/sync/service/brave_sync_service_impl.cc @@ -98,6 +98,7 @@ std::string BraveSyncServiceImpl::GetOrCreateSyncCode() { if (sync_code.empty()) { std::vector seed = brave_sync::crypto::GetSeed(); sync_code = brave_sync::crypto::PassphraseFromBytes32(seed); + sync_code_monitor_.RecordCodeGenerated(); } CHECK(!sync_code.empty()) << "Attempt to return empty sync code"; @@ -121,6 +122,8 @@ bool BraveSyncServiceImpl::SetSyncCode(const std::string& sync_code) { initiated_self_device_info_deleted_ = false; initiated_join_chain_ = true; + sync_code_monitor_.RecordCodeSet(); + return true; } diff --git a/components/sync/service/brave_sync_service_impl.h b/components/sync/service/brave_sync_service_impl.h index 197c646c5784..e934c1562792 100644 --- a/components/sync/service/brave_sync_service_impl.h +++ b/components/sync/service/brave_sync_service_impl.h @@ -12,6 +12,7 @@ #include "base/gtest_prod_util.h" #include "base/memory/weak_ptr.h" +#include "brave/components/brave_sync/brave_sync_p3a.h" #include "brave/components/brave_sync/brave_sync_prefs.h" #include "components/prefs/pref_change_registrar.h" #include "components/sync/engine/sync_protocol_error.h" @@ -114,6 +115,8 @@ class BraveSyncServiceImpl : public SyncServiceImpl { PrefChangeRegistrar brave_sync_prefs_change_registrar_; + brave_sync::p3a::SyncCodeMonitor sync_code_monitor_; + // This is set to true between |PermanentlyDeleteAccount| succeeded call and // new sync chain setup or browser exit. This is used to avoid show the // infobar to ourselves, because we know what we have done