Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "day zero" experiment feature & metrics #22195

Merged
merged 2 commits into from
Mar 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion android/java/org/chromium/base/BraveFeatureList.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ public abstract class BraveFeatureList {
public static final String AI_CHAT = "AIChat";
public static final String BRAVE_SHOW_STRICT_FINGERPRINTING_MODE =
"BraveShowStrictFingerprintingMode";
public static final String BRAVE_ZERO_DAY_FLAG_ANDROID = "BraveZeroDayFlagAndroid";
public static final String BRAVE_DAY_ZERO_EXPERIMENT = "BraveDayZeroExperiment";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DJAndries the BRAVE_DAY_ZERO_EXPERIMENTvariable is the same variable cross platforms and as a replacement for the previous Android-specific one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, correct

public static final String BRAVE_FALLBACK_DOH_PROVIDER = "BraveFallbackDoHProvider";
}
10 changes: 0 additions & 10 deletions browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,18 +372,9 @@
kOsAndroid, \
FEATURE_VALUE_TYPE(safe_browsing::features::kBraveAndroidSafeBrowsing), \
})
#define BRAVE_ZERO_DAY_FLAG_ANDROID \
EXPAND_FEATURE_ENTRIES({ \
"brave-zero-day-flag-android", \
"ZeroDayFlag flag for product test", \
"This flag will be set through griffin to perform product testing", \
kOsAndroid, \
FEATURE_VALUE_TYPE(preferences::features::kBraveZeroDayFlagAndroid), \
})
#else
#define BRAVE_BACKGROUND_VIDEO_PLAYBACK_ANDROID
#define BRAVE_SAFE_BROWSING_ANDROID
#define BRAVE_ZERO_DAY_FLAG_ANDROID
#endif // BUILDFLAG(IS_ANDROID)

#if !BUILDFLAG(IS_ANDROID)
Expand Down Expand Up @@ -992,7 +983,6 @@
BRAVE_COMMANDS_FEATURE_ENTRIES \
BRAVE_BACKGROUND_VIDEO_PLAYBACK_ANDROID \
BRAVE_SAFE_BROWSING_ANDROID \
BRAVE_ZERO_DAY_FLAG_ANDROID \
BRAVE_CHANGE_ACTIVE_TAB_ON_SCROLL_EVENT_FEATURE_ENTRIES \
BRAVE_TABS_FEATURE_ENTRIES \
BRAVE_AI_CHAT \
Expand Down
4 changes: 0 additions & 4 deletions browser/android/preferences/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,5 @@ BASE_FEATURE(kBraveBackgroundVideoPlayback,
"BraveBackgroundVideoPlayback",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kBraveZeroDayFlagAndroid,
"BraveZeroDayFlagAndroid",
base::FEATURE_DISABLED_BY_DEFAULT);

} // namespace features
} // namespace preferences
1 change: 0 additions & 1 deletion browser/android/preferences/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ namespace preferences {
namespace features {

BASE_DECLARE_FEATURE(kBraveBackgroundVideoPlayback);
BASE_DECLARE_FEATURE(kBraveZeroDayFlagAndroid);

} // namespace features
} // namespace preferences
Expand Down
6 changes: 6 additions & 0 deletions browser/brave_browser_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,10 @@ BASE_FEATURE(kBraveWebViewRoundedCorners,
"brave-web-view-rounded-corners",
base::FEATURE_DISABLED_BY_DEFAULT);

// Enable "day zero" experiment modifications, for potential user
// retention improvements.
BASE_FEATURE(kBraveDayZeroExperiment,
"BraveDayZeroExperiment",
base::FEATURE_DISABLED_BY_DEFAULT);

} // namespace features
2 changes: 1 addition & 1 deletion browser/brave_browser_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ BASE_DECLARE_FEATURE(kBraveCleanupSessionCookiesOnSessionRestore);
BASE_DECLARE_FEATURE(kBraveCopyCleanLinkByDefault);
BASE_DECLARE_FEATURE(kBraveOverrideDownloadDangerLevel);
BASE_DECLARE_FEATURE(kBraveWebViewRoundedCorners);
BASE_DECLARE_FEATURE(kBraveZeroDayFlagAndroid);
BASE_DECLARE_FEATURE(kBraveDayZeroExperiment);

} // namespace features

Expand Down
8 changes: 7 additions & 1 deletion browser/brave_stats/brave_stats_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
#include "base/command_line.h"
#include "base/metrics/histogram_macros.h"
#include "base/system/sys_info.h"
#include "brave/browser/brave_browser_features.h"
#include "brave/browser/brave_stats/brave_stats_updater_params.h"
#include "brave/browser/brave_stats/buildflags.h"
#include "brave/browser/brave_stats/first_run_util.h"
#include "brave/browser/brave_stats/switches.h"
#include "brave/common/brave_channel_info.h"
#include "brave/components/brave_ads/core/public/prefs/pref_names.h"
Expand All @@ -26,6 +28,7 @@
#include "brave/components/rpill/common/rpill.h"
#include "brave/components/version_info/version_info.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/first_run/first_run.h"
#include "chrome/browser/net/system_network_context_manager.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/common/channel_info.h"
Expand Down Expand Up @@ -107,7 +110,10 @@ BraveStatsUpdater::BraveStatsUpdater(PrefService* pref_service,
}

general_browser_usage_p3a_ =
std::make_unique<misc_metrics::GeneralBrowserUsage>(pref_service);
std::make_unique<misc_metrics::GeneralBrowserUsage>(
pref_service,
base::FeatureList::IsEnabled(features::kBraveDayZeroExperiment),
first_run::IsChromeFirstRun(), GetFirstRunTime(pref_service));

if (profile_manager != nullptr) {
g_browser_process->profile_manager()->AddObserver(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
&brave_wallet::features::kNativeBraveWalletFeature, \
&playlist::features::kPlaylist, \
&preferences::features::kBraveBackgroundVideoPlayback, \
&preferences::features::kBraveZeroDayFlagAndroid, \
&request_otr::features::kBraveRequestOTRTab, \
&safe_browsing::features::kBraveAndroidSafeBrowsing, \
&speedreader::kSpeedreaderFeature, \
Expand Down
35 changes: 34 additions & 1 deletion components/misc_metrics/general_browser_usage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "brave/components/misc_metrics/general_browser_usage.h"

#include "base/metrics/histogram_macros.h"
#include "base/time/time.h"
#include "brave/components/misc_metrics/pref_names.h"
#include "brave/components/p3a_utils/bucket.h"
#include "brave/components/time_period_storage/iso_weekly_storage.h"
Expand All @@ -24,16 +25,31 @@ constexpr int kProfileCountBuckets[] = {0, 1, 2, 3, 5};

} // namespace

GeneralBrowserUsage::GeneralBrowserUsage(PrefService* local_state) {
GeneralBrowserUsage::GeneralBrowserUsage(PrefService* local_state,
bool day_zero_experiment_enabled,
bool is_first_run,
base::Time first_run_time)
: local_state_(local_state), first_run_time_(first_run_time) {
usage_storage_ = std::make_unique<ISOWeeklyStorage>(
local_state, kMiscMetricsBrowserUsageList);

if (is_first_run) {
if (day_zero_experiment_enabled) {
local_state->SetBoolean(kMiscMetricsDayZeroAtInstall, true);
}
if (first_run_time.is_null()) {
first_run_time_ = base::Time::Now();
}
}

Update();
}

GeneralBrowserUsage::~GeneralBrowserUsage() = default;

void GeneralBrowserUsage::RegisterPrefs(PrefRegistrySimple* registry) {
registry->RegisterListPref(kMiscMetricsBrowserUsageList);
registry->RegisterBooleanPref(kMiscMetricsDayZeroAtInstall, false);
}

void GeneralBrowserUsage::ReportWeeklyUse() {
Expand All @@ -42,6 +58,20 @@ void GeneralBrowserUsage::ReportWeeklyUse() {
usage_storage_->GetLastISOWeekSum(), 8);
}

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)
void GeneralBrowserUsage::ReportInstallTime() {
int days_since_install = (base::Time::Now() - first_run_time_).InDays();
if (days_since_install < 0 || days_since_install > 30) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DJAndries this condition is to just keep the histogram putting users into 0-30 day buckets and anyone outside of this range, gets returned to the previous function, right? e.g. negative or >=31 will not be included.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's right. users outside of the 0-30 day range will not report either metric.

return;
}
const char* histogram_name =
local_state_->GetBoolean(kMiscMetricsDayZeroAtInstall)
? kDayZeroOnInstallTime
: kDayZeroOffInstallTime;
base::UmaHistogramExactLinear(histogram_name, days_since_install, 31);
}
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)

void GeneralBrowserUsage::ReportProfileCount(size_t count) {
#if !BUILDFLAG(IS_ANDROID)
p3a_utils::RecordToHistogramBucket(kProfileCountHistogramName,
Expand All @@ -56,6 +86,9 @@ void GeneralBrowserUsage::SetUpUpdateTimer() {

void GeneralBrowserUsage::Update() {
ReportWeeklyUse();
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)
ReportInstallTime();
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)

SetUpUpdateTimer();
}
Expand Down
17 changes: 16 additions & 1 deletion components/misc_metrics/general_browser_usage.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <memory>

#include "base/memory/raw_ptr.h"
#include "base/timer/wall_clock_timer.h"

class PrefRegistrySimple;
Expand All @@ -19,9 +20,18 @@ namespace misc_metrics {
inline constexpr char kWeeklyUseHistogramName[] = "Brave.Core.WeeklyUsage";
inline constexpr char kProfileCountHistogramName[] = "Brave.Core.ProfileCount";

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)
inline constexpr char kDayZeroOnInstallTime[] = "Brave.DayZero.On.InstallTime";
inline constexpr char kDayZeroOffInstallTime[] =
"Brave.DayZero.Off.InstallTime";
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)

class GeneralBrowserUsage {
public:
explicit GeneralBrowserUsage(PrefService* local_state);
GeneralBrowserUsage(PrefService* local_state,
bool day_zero_experiment_enabled,
bool is_first_run,
base::Time first_run_time);
~GeneralBrowserUsage();

GeneralBrowserUsage(const GeneralBrowserUsage&) = delete;
Expand All @@ -33,11 +43,16 @@ class GeneralBrowserUsage {

private:
void ReportWeeklyUse();
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)
void ReportInstallTime();
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)

void SetUpUpdateTimer();

void Update();

raw_ptr<PrefService> local_state_;
base::Time first_run_time_;
std::unique_ptr<ISOWeeklyStorage> usage_storage_;

base::WallClockTimer report_timer_;
Expand Down
61 changes: 58 additions & 3 deletions components/misc_metrics/general_browser_usage_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,26 @@ class GeneralBrowserUsageUnitTest : public testing::Test {
days_until_monday = 1;
}
task_environment_.AdvanceClock(base::Days(days_until_monday));

general_browser_usage_ =
std::make_unique<GeneralBrowserUsage>(&local_state_);
}

protected:
void SetUpUsage(bool day_zero_experiment_enabled,
bool is_first_run,
base::Time first_run_time) {
general_browser_usage_ = std::make_unique<GeneralBrowserUsage>(
&local_state_, day_zero_experiment_enabled, is_first_run,
first_run_time);
}

content::BrowserTaskEnvironment task_environment_;
TestingPrefServiceSimple local_state_;
base::HistogramTester histogram_tester_;
std::unique_ptr<GeneralBrowserUsage> general_browser_usage_;
};

TEST_F(GeneralBrowserUsageUnitTest, WeeklyUsage) {
SetUpUsage(false, true, base::Time::Now());

histogram_tester_.ExpectUniqueSample(kWeeklyUseHistogramName, 0, 1);

task_environment_.FastForwardBy(base::Days(1));
Expand All @@ -61,6 +68,8 @@ TEST_F(GeneralBrowserUsageUnitTest, WeeklyUsage) {

#if !BUILDFLAG(IS_ANDROID)
TEST_F(GeneralBrowserUsageUnitTest, ProfileCount) {
SetUpUsage(false, true, base::Time::Now());

histogram_tester_.ExpectTotalCount(kProfileCountHistogramName, 0);

general_browser_usage_->ReportProfileCount(1);
Expand All @@ -73,4 +82,50 @@ TEST_F(GeneralBrowserUsageUnitTest, ProfileCount) {
}
#endif // !BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)
TEST_F(GeneralBrowserUsageUnitTest, InstallTimeOff) {
base::Time install_time = base::Time::Now();
SetUpUsage(false, true, install_time);

histogram_tester_.ExpectUniqueSample(kDayZeroOffInstallTime, 0, 1);

task_environment_.FastForwardBy(base::Days(15));

histogram_tester_.ExpectBucketCount(kDayZeroOffInstallTime, 15, 1);
histogram_tester_.ExpectTotalCount(kDayZeroOnInstallTime, 0);

SetUpUsage(true, false, install_time);
// Ensure histogram name does not change if "day zero" is enabled
// after install; we only want to report the "day zero on" metric
// if it was enabled at install time.
histogram_tester_.ExpectBucketCount(kDayZeroOffInstallTime, 15, 2);
histogram_tester_.ExpectTotalCount(kDayZeroOnInstallTime, 0);

task_environment_.FastForwardBy(base::Days(15));
histogram_tester_.ExpectBucketCount(kDayZeroOffInstallTime, 30, 1);

// Ensure there are no more reports past 30 days
task_environment_.FastForwardBy(base::Days(5));

histogram_tester_.ExpectTotalCount(kDayZeroOffInstallTime, 32);
histogram_tester_.ExpectTotalCount(kDayZeroOnInstallTime, 0);
}

TEST_F(GeneralBrowserUsageUnitTest, InstallTimeOn) {
base::Time install_time = base::Time::Now();
SetUpUsage(true, true, install_time);

histogram_tester_.ExpectUniqueSample(kDayZeroOnInstallTime, 0, 1);

task_environment_.FastForwardBy(base::Days(15));

histogram_tester_.ExpectBucketCount(kDayZeroOnInstallTime, 15, 1);
histogram_tester_.ExpectTotalCount(kDayZeroOffInstallTime, 0);

SetUpUsage(false, false, install_time);
histogram_tester_.ExpectBucketCount(kDayZeroOnInstallTime, 15, 2);
histogram_tester_.ExpectTotalCount(kDayZeroOffInstallTime, 0);
}
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN)

} // namespace misc_metrics
3 changes: 3 additions & 0 deletions components/misc_metrics/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ inline constexpr char kMiscMetricsNewTabLocationBarEntriesStorage[] =
"brave.misc_metrics.new_tab_location_bar_entries_storage";
inline constexpr char kMiscMetricsTotalLocationBarEntriesStorage[] =
"brave.misc_metrics.total_location_bar_entries_storage";

inline constexpr char kMiscMetricsDayZeroAtInstall[] =
"brave.misc_metrics.day_zero_at_install";
} // namespace misc_metrics

#endif // BRAVE_COMPONENTS_MISC_METRICS_PREF_NAMES_H_
11 changes: 8 additions & 3 deletions components/p3a/metric_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ inline constexpr auto kCollectedExpressHistograms =
base::MakeFixedFlatSet<std::string_view>(base::sorted_unique,{
"Brave.AIChat.UsageDaily.2",
"Brave.Core.UsageDaily",
"Brave.DayZero.Off.InstallTime",
"Brave.DayZero.On.InstallTime",
"Brave.Rewards.EnabledInstallationTime",
"Brave.Search.DefaultEngine.4",
"Brave.Today.IsEnabled",
Expand Down Expand Up @@ -284,6 +286,8 @@ inline constexpr auto kEphemeralHistograms =
"Brave.AIChat.UsageMonthly",
"Brave.AIChat.UsageWeekly",
"Brave.Core.FailedHTTPSUpgrades",
"Brave.DayZero.Off.InstallTime",
"Brave.DayZero.On.InstallTime",
"Brave.Playlist.FirstTimeOffset",
"Brave.Playlist.UsageDaysInWeek",
"Brave.PrivacyHub.Views",
Expand Down Expand Up @@ -325,15 +329,16 @@ inline constexpr auto kEphemeralHistograms =
inline constexpr auto kConstellationOnlyHistograms =
base::MakeFixedFlatSet<std::string_view>(base::sorted_unique,{
"Brave.Core.PrimaryLang",
"Brave.DayZero.Off.InstallTime",
"Brave.DayZero.On.InstallTime",
});

// List of metrics which will include the stored refcode when transmitted
// via the STAR/Constellation protocol.
inline constexpr auto kHistogramsWithRefcodeIncluded =
base::MakeFixedFlatSet<std::string_view>(base::sorted_unique,{
// TODO(djandries): Replace this metric with the first metric
// to include a refcode.
"Brave.ExampleMetric"
"Brave.DayZero.Off.InstallTime",
"Brave.DayZero.On.InstallTime",
});

// clang-format on
Expand Down
6 changes: 4 additions & 2 deletions components/p3a/star_randomness_test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ std::string HandleRandomnessRequest(const network::ResourceRequest& request,
rust::Vec<constellation::VecU8> req_points_rust;
const base::Value::List* points_list = req_parsed_val.FindList("points");

EXPECT_EQ(points_list->size(), 8U);
EXPECT_GE(points_list->size(), 8U);
EXPECT_LE(points_list->size(), 9U);

std::transform(
points_list->begin(), points_list->end(),
Expand All @@ -72,7 +73,8 @@ std::string HandleRandomnessRequest(const network::ResourceRequest& request,
auto rand_result =
constellation::generate_local_randomness(req_points_rust, expected_epoch);

EXPECT_EQ(rand_result.points.size(), 8U);
EXPECT_GE(rand_result.points.size(), 8U);
EXPECT_LE(rand_result.points.size(), 9U);

base::Value::List resp_points_list;
for (const constellation::VecU8& resp_point_rust : rand_result.points) {
Expand Down
Loading