Skip to content

Commit

Permalink
Merge pull request #25761 from brave/sync-start-p3a
Browse files Browse the repository at this point in the history
Add metric for sync join
  • Loading branch information
DJAndries authored Oct 3, 2024
2 parents 4f54357 + e8ad582 commit 9727b80
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 0 deletions.
14 changes: 14 additions & 0 deletions components/brave_sync/brave_sync_p3a.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
24 changes: 24 additions & 0 deletions components/brave_sync/brave_sync_p3a.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Expand Down
28 changes: 28 additions & 0 deletions components/brave_sync/brave_sync_p3a_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(SyncJoinType::kChainCreated),
1);

monitor.RecordCodeSet();
histogram_tester.ExpectUniqueSample(
kSyncJoinTypeHistogramName, static_cast<int>(SyncJoinType::kChainCreated),
1);

monitor.RecordCodeSet();
histogram_tester.ExpectBucketCount(
kSyncJoinTypeHistogramName, static_cast<int>(SyncJoinType::kChainJoined),
1);

SyncCodeMonitor monitor2;
monitor2.RecordCodeSet();
histogram_tester.ExpectBucketCount(
kSyncJoinTypeHistogramName, static_cast<int>(SyncJoinType::kChainJoined),
2);

histogram_tester.ExpectTotalCount(kSyncJoinTypeHistogramName, 3);
}

} // namespace brave_sync::p3a
2 changes: 2 additions & 0 deletions components/p3a/metric_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions components/sync/service/brave_sync_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ std::string BraveSyncServiceImpl::GetOrCreateSyncCode() {
if (sync_code.empty()) {
std::vector<uint8_t> 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";
Expand All @@ -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;
}

Expand Down
3 changes: 3 additions & 0 deletions components/sync/service/brave_sync_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -119,6 +120,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
Expand Down

0 comments on commit 9727b80

Please sign in to comment.