Commit e2518d30 authored by Daniel Classon's avatar Daniel Classon Committed by Commit Bot

[OsSettingsMetrics] Add Metric for kAddAccount (Int setting example)

Adds a metric for the kAddAccount setting on the People Page. It records
the number of the account the user was trying to add, e.g. a sample of 3
on the histogram means the user attempted to add a 3rd account.

Bug: 1133553
Change-Id: I8befd66ce2ce78f1c07e6c5afb4f4a5fcb8ae09c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2454492
Commit-Queue: Daniel Classon <dclasson@google.com>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815867}
parent 31097e58
...@@ -34,6 +34,7 @@ js_type_check("closure_compile") { ...@@ -34,6 +34,7 @@ js_type_check("closure_compile") {
js_library("account_manager") { js_library("account_manager") {
deps = [ deps = [
"..:deep_linking_behavior", "..:deep_linking_behavior",
"..:metrics_recorder",
"..:os_route", "..:os_route",
"../..:router", "../..:router",
"../../people_page:account_manager_browser_proxy", "../../people_page:account_manager_browser_proxy",
...@@ -259,6 +260,7 @@ js_library("account_manager.m") { ...@@ -259,6 +260,7 @@ js_library("account_manager.m") {
sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/os_people_page/account_manager.m.js" ] sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/os_people_page/account_manager.m.js" ]
deps = [ deps = [
"..:deep_linking_behavior.m", "..:deep_linking_behavior.m",
"..:metrics_recorder.m",
"..:os_route.m", "..:os_route.m",
"../..:router.m", "../..:router.m",
"../../people_page:account_manager_browser_proxy.m", "../../people_page:account_manager_browser_proxy.m",
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
<link rel="import" href="../localized_link/localized_link.html"> <link rel="import" href="../localized_link/localized_link.html">
<link rel="import" href="../../i18n_setup.html"> <link rel="import" href="../../i18n_setup.html">
<link rel="import" href="../deep_linking_behavior.html"> <link rel="import" href="../deep_linking_behavior.html">
<link rel="import" href="../metrics_recorder.html">
<link rel="import" href="../os_route.html"> <link rel="import" href="../os_route.html">
<link rel="import" href="../../router.html"> <link rel="import" href="../../router.html">
<link rel="import" href="../../settings_shared_css.html"> <link rel="import" href="../../settings_shared_css.html">
......
...@@ -161,6 +161,9 @@ Polymer({ ...@@ -161,6 +161,9 @@ Polymer({
* @private * @private
*/ */
addAccount_(event) { addAccount_(event) {
settings.recordSettingChange(
chromeos.settings.mojom.Setting.kAddAccount,
{intValue: this.accounts_.length + 1});
this.browserProxy_.addAccount(); this.browserProxy_.addAccount();
}, },
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "ash/public/cpp/ash_features.h" #include "ash/public/cpp/ash_features.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/i18n/number_formatting.h" #include "base/i18n/number_formatting.h"
#include "base/metrics/histogram_functions.h"
#include "base/no_destructor.h" #include "base/no_destructor.h"
#include "base/strings/strcat.h" #include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
...@@ -920,8 +921,15 @@ std::string PeopleSection::GetSectionPath() const { ...@@ -920,8 +921,15 @@ std::string PeopleSection::GetSectionPath() const {
bool PeopleSection::LogMetric(mojom::Setting setting, bool PeopleSection::LogMetric(mojom::Setting setting,
base::Value& value) const { base::Value& value) const {
// Unimplemented. switch (setting) {
return false; case mojom::Setting::kAddAccount:
base::UmaHistogramCounts1000("ChromeOS.Settings.People.AddAccountCount",
value.GetInt());
return true;
default:
return false;
}
} }
void PeopleSection::RegisterHierarchy(HierarchyGenerator* generator) const { void PeopleSection::RegisterHierarchy(HierarchyGenerator* generator) const {
......
...@@ -39,7 +39,9 @@ class SettingsUserActionTracker : public mojom::UserActionRecorder { ...@@ -39,7 +39,9 @@ class SettingsUserActionTracker : public mojom::UserActionRecorder {
// SettingsUserActionTracker(); // SettingsUserActionTracker();
friend class SettingsUserActionTrackerTest; friend class SettingsUserActionTrackerTest;
FRIEND_TEST_ALL_PREFIXES(SettingsUserActionTrackerTest, FRIEND_TEST_ALL_PREFIXES(SettingsUserActionTrackerTest,
TestRecordSettingChanged); TestRecordSettingChangedBool);
FRIEND_TEST_ALL_PREFIXES(SettingsUserActionTrackerTest,
TestRecordSettingChangedInt);
// mojom::UserActionRecorder: // mojom::UserActionRecorder:
void RecordPageFocus() override; void RecordPageFocus() override;
......
...@@ -33,6 +33,8 @@ class SettingsUserActionTrackerTest : public testing::Test { ...@@ -33,6 +33,8 @@ class SettingsUserActionTrackerTest : public testing::Test {
void SetUp() override { void SetUp() override {
fake_hierarchy_.AddSettingMetadata(mojom::Section::kBluetooth, fake_hierarchy_.AddSettingMetadata(mojom::Section::kBluetooth,
mojom::Setting::kBluetoothOnOff); mojom::Setting::kBluetoothOnOff);
fake_hierarchy_.AddSettingMetadata(mojom::Section::kPeople,
mojom::Setting::kAddAccount);
} }
base::HistogramTester histogram_tester_; base::HistogramTester histogram_tester_;
...@@ -41,7 +43,7 @@ class SettingsUserActionTrackerTest : public testing::Test { ...@@ -41,7 +43,7 @@ class SettingsUserActionTrackerTest : public testing::Test {
SettingsUserActionTracker tracker_; SettingsUserActionTracker tracker_;
}; };
TEST_F(SettingsUserActionTrackerTest, TestRecordSettingChanged) { TEST_F(SettingsUserActionTrackerTest, TestRecordSettingChangedBool) {
// Record that the bluetooth enabled setting was toggled off. // Record that the bluetooth enabled setting was toggled off.
tracker_.RecordSettingChangeWithDetails( tracker_.RecordSettingChangeWithDetails(
mojom::Setting::kBluetoothOnOff, mojom::Setting::kBluetoothOnOff,
...@@ -63,5 +65,26 @@ TEST_F(SettingsUserActionTrackerTest, TestRecordSettingChanged) { ...@@ -63,5 +65,26 @@ TEST_F(SettingsUserActionTrackerTest, TestRecordSettingChanged) {
mojom::Setting::kBluetoothOnOff); mojom::Setting::kBluetoothOnOff);
} }
TEST_F(SettingsUserActionTrackerTest, TestRecordSettingChangedInt) {
// Record that the user tried to add a 3rd account.
tracker_.RecordSettingChangeWithDetails(
mojom::Setting::kAddAccount, mojom::SettingChangeValue::NewIntValue(3));
// The umbrella metric for which setting was changed should be updated. Note
// that kAddAccount has enum value of 300.
histogram_tester_.ExpectTotalCount("ChromeOS.Settings.SettingChanged",
/*count=*/1);
histogram_tester_.ExpectBucketCount("ChromeOS.Settings.SettingChanged",
/*sample=*/300,
/*count=*/1);
// The LogMetric fn in the People section should have been called.
const FakeOsSettingsSection* people_section =
static_cast<const FakeOsSettingsSection*>(
fake_sections_.GetSection(mojom::Section::kPeople));
EXPECT_TRUE(people_section->logged_metrics().back() ==
mojom::Setting::kAddAccount);
}
} // namespace settings. } // namespace settings.
} // namespace chromeos. } // namespace chromeos.
...@@ -712,6 +712,18 @@ incoming reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -712,6 +712,18 @@ incoming reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary> </summary>
</histogram> </histogram>
<histogram name="ChromeOS.Settings.People.AddAccountCount" units="accounts"
expires_after="2021-09-30">
<owner>khorimoto@chromium.org</owner>
<owner>hsuregan@chromium.org</owner>
<owner>cros-customization@google.com</owner>
<summary>
Records when users click the Add Account button on the People page. The
number of the account that would be added is saved, e.g. a sample of 2 means
the user entered the add account dialog for a 2nd account.
</summary>
</histogram>
<histogram name="ChromeOS.Settings.SearchLatency" units="ms" <histogram name="ChromeOS.Settings.SearchLatency" units="ms"
expires_after="2021-04-04"> expires_after="2021-04-04">
<owner>khorimoto@chromium.org</owner> <owner>khorimoto@chromium.org</owner>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment