Commit a9fafab1 authored by Yi Gu's avatar Yi Gu Committed by Commit Bot

[WebOTP] Record timing related UKMs

There are UMA metrics to measure the duration from when the API is
called to when the SMS is received / a user clicks "Allow" to complete
the flow. We want to add corresponding UKMs to help with the feature
adoption.

Note that the UKM that records the duration from when the API is called
to when a user clicks "Deny" to abort the flow will be added in a
follow up patch after crrev.com/c/2428825.

UKM collection review:
https://docs.google.com/document/d/15orQiLx06QSuZr4ShBaozY0dvPzXiTWwJZ9Xk5cgzAA/edit?usp=sharing

Bug: 1127402
Change-Id: I204bd469ca203182eba6a37f3522d7a492cc937d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429236Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Reviewed-by: default avatarSam Goto <goto@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814701}
parent 82d1c5d8
...@@ -63,7 +63,7 @@ class SmsBrowserTest : public ContentBrowserTest { ...@@ -63,7 +63,7 @@ class SmsBrowserTest : public ContentBrowserTest {
for (const auto* const entry : entries) { for (const auto* const entry : entries) {
const int64_t* metric = ukm_recorder()->GetEntryMetric(entry, "Outcome"); const int64_t* metric = ukm_recorder()->GetEntryMetric(entry, "Outcome");
if (*metric == static_cast<int>(outcome)) { if (metric && *metric == static_cast<int>(outcome)) {
SUCCEED(); SUCCEED();
return; return;
} }
...@@ -71,6 +71,20 @@ class SmsBrowserTest : public ContentBrowserTest { ...@@ -71,6 +71,20 @@ class SmsBrowserTest : public ContentBrowserTest {
FAIL() << "Expected SMSReceiverOutcome was not recorded"; FAIL() << "Expected SMSReceiverOutcome was not recorded";
} }
void ExpectTimingUKM(const std::string& metric_name) {
auto entries = ukm_recorder()->GetEntriesByName(Entry::kEntryName);
ASSERT_FALSE(entries.empty());
for (const auto* const entry : entries) {
if (ukm_recorder()->GetEntryMetric(entry, metric_name)) {
SUCCEED();
return;
}
}
FAIL() << "Expected UKM was not recorded";
}
void ExpectNoOutcomeUKM() { void ExpectNoOutcomeUKM() {
EXPECT_TRUE(ukm_recorder()->GetEntriesByName(Entry::kEntryName).empty()); EXPECT_TRUE(ukm_recorder()->GetEntriesByName(Entry::kEntryName).empty());
} }
...@@ -177,6 +191,8 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Receive) { ...@@ -177,6 +191,8 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Receive) {
content::FetchHistogramsFromChildProcesses(); content::FetchHistogramsFromChildProcesses();
ExpectOutcomeUKM(url, blink::SMSReceiverOutcome::kSuccess); ExpectOutcomeUKM(url, blink::SMSReceiverOutcome::kSuccess);
ExpectTimingUKM("TimeSmsReceiveMs");
ExpectTimingUKM("TimeSuccessMs");
histogram_tester.ExpectTotalCount("Blink.Sms.Receive.TimeSuccess", 1); histogram_tester.ExpectTotalCount("Blink.Sms.Receive.TimeSuccess", 1);
} }
......
...@@ -5,10 +5,16 @@ ...@@ -5,10 +5,16 @@
#include "content/browser/sms/sms_metrics.h" #include "content/browser/sms/sms_metrics.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "services/metrics/public/cpp/metrics_utils.h"
namespace content { namespace content {
void RecordSmsReceiveTime(base::TimeDelta duration) { void RecordSmsReceiveTime(base::TimeDelta duration, ukm::SourceId source_id) {
ukm::builders::SMSReceiver builder(source_id);
builder.SetTimeSmsReceiveMs(
ukm::GetExponentialBucketMinForUserTiming(duration.InMilliseconds()));
builder.Record(ukm::UkmRecorder::Get());
UMA_HISTOGRAM_MEDIUM_TIMES("Blink.Sms.Receive.TimeSmsReceive", duration); UMA_HISTOGRAM_MEDIUM_TIMES("Blink.Sms.Receive.TimeSmsReceive", duration);
} }
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef CONTENT_BROWSER_SMS_SMS_METRICS_H_ #ifndef CONTENT_BROWSER_SMS_SMS_METRICS_H_
#define CONTENT_BROWSER_SMS_SMS_METRICS_H_ #define CONTENT_BROWSER_SMS_SMS_METRICS_H_
#include "services/metrics/public/cpp/ukm_builders.h"
#include "third_party/blink/public/common/sms/sms_receiver_destroyed_reason.h" #include "third_party/blink/public/common/sms/sms_receiver_destroyed_reason.h"
namespace base { namespace base {
...@@ -15,7 +16,7 @@ namespace content { ...@@ -15,7 +16,7 @@ namespace content {
// Records the time from when a call to the API was made to when an SMS has been // Records the time from when a call to the API was made to when an SMS has been
// successfully received. // successfully received.
void RecordSmsReceiveTime(base::TimeDelta duration); void RecordSmsReceiveTime(base::TimeDelta duration, ukm::SourceId source_id);
// Records the time from when a successful SMS was retrieved to when the user // Records the time from when a successful SMS was retrieved to when the user
// presses the Cancel button. // presses the Cancel button.
......
...@@ -131,7 +131,8 @@ void SmsService::OnReceive(const std::string& one_time_code) { ...@@ -131,7 +131,8 @@ void SmsService::OnReceive(const std::string& one_time_code) {
DCHECK(!start_time_.is_null()); DCHECK(!start_time_.is_null());
receive_time_ = base::TimeTicks::Now(); receive_time_ = base::TimeTicks::Now();
RecordSmsReceiveTime(receive_time_ - start_time_); RecordSmsReceiveTime(receive_time_ - start_time_,
render_frame_host()->GetPageUkmSourceId());
one_time_code_ = one_time_code; one_time_code_ = one_time_code;
......
include_rules = [ include_rules = [
"+mojo/public", "+mojo/public",
"+services/metrics/public/cpp/metrics_utils.h",
"+services/metrics/public/cpp/ukm_builders.h", "+services/metrics/public/cpp/ukm_builders.h",
"+services/metrics/public/cpp/ukm_source_id.h", "+services/metrics/public/cpp/ukm_source_id.h",
"-third_party/blink/renderer/modules", "-third_party/blink/renderer/modules",
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/modules/credentialmanager/credential_metrics.h" #include "third_party/blink/renderer/modules/credentialmanager/credential_metrics.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "services/metrics/public/cpp/metrics_utils.h"
#include "services/metrics/public/cpp/ukm_builders.h" #include "services/metrics/public/cpp/ukm_builders.h"
namespace blink { namespace blink {
...@@ -22,7 +23,18 @@ void RecordSmsOutcome(SMSReceiverOutcome outcome, ...@@ -22,7 +23,18 @@ void RecordSmsOutcome(SMSReceiverOutcome outcome,
UMA_HISTOGRAM_ENUMERATION("Blink.Sms.Receive.Outcome", outcome); UMA_HISTOGRAM_ENUMERATION("Blink.Sms.Receive.Outcome", outcome);
} }
void RecordSmsSuccessTime(base::TimeDelta duration) { void RecordSmsSuccessTime(base::TimeDelta duration,
ukm::SourceId source_id,
ukm::UkmRecorder* ukm_recorder) {
DCHECK_NE(source_id, ukm::kInvalidSourceId);
DCHECK(ukm_recorder);
ukm::builders::SMSReceiver builder(source_id);
// Uses exponential bucketing for datapoints reflecting user activity.
builder.SetTimeSuccessMs(
ukm::GetExponentialBucketMinForUserTiming(duration.InMilliseconds()));
builder.Record(ukm_recorder);
UMA_HISTOGRAM_MEDIUM_TIMES("Blink.Sms.Receive.TimeSuccess", duration); UMA_HISTOGRAM_MEDIUM_TIMES("Blink.Sms.Receive.TimeSuccess", duration);
} }
......
...@@ -28,7 +28,9 @@ void RecordSmsOutcome(SMSReceiverOutcome outcome, ...@@ -28,7 +28,9 @@ void RecordSmsOutcome(SMSReceiverOutcome outcome,
// receives the SMS and presses verify to move on with the verification flow. // receives the SMS and presses verify to move on with the verification flow.
// This uses the same histogram as SmsReceiver API to provide continuity with // This uses the same histogram as SmsReceiver API to provide continuity with
// previous iterations of the API. // previous iterations of the API.
void RecordSmsSuccessTime(base::TimeDelta duration); void RecordSmsSuccessTime(base::TimeDelta duration,
ukm::SourceId source_id,
ukm::UkmRecorder* ukm_recorder);
// Records the time from when the API is called to when the user dismisses the // Records the time from when the API is called to when the user dismisses the
// infobar to abort SMS retrieval. This uses the same histogram as SmsReceiver // infobar to abort SMS retrieval. This uses the same histogram as SmsReceiver
......
...@@ -565,7 +565,8 @@ void OnSmsReceive(ScriptPromiseResolver* resolver, ...@@ -565,7 +565,8 @@ void OnSmsReceive(ScriptPromiseResolver* resolver,
DOMExceptionCode::kAbortError, "OTP retrieval was cancelled.")); DOMExceptionCode::kAbortError, "OTP retrieval was cancelled."));
return; return;
} }
RecordSmsSuccessTime(base::TimeTicks::Now() - start_time); RecordSmsSuccessTime(base::TimeTicks::Now() - start_time, source_id,
recorder);
RecordSmsOutcome(SMSReceiverOutcome::kSuccess, source_id, recorder); RecordSmsOutcome(SMSReceiverOutcome::kSuccess, source_id, recorder);
resolver->Resolve(MakeGarbageCollected<OTPCredential>(otp)); resolver->Resolve(MakeGarbageCollected<OTPCredential>(otp));
} }
......
...@@ -11452,6 +11452,19 @@ be describing additional metrics about the same event. ...@@ -11452,6 +11452,19 @@ be describing additional metrics about the same event.
Records the result of a call to the SmsReceiver API. Records the result of a call to the SmsReceiver API.
</summary> </summary>
</metric> </metric>
<metric name="TimeSmsReceiveMs">
<summary>
Records the duration from when the API is called to when an SMS has been
successfully received.
</summary>
</metric>
<metric name="TimeSuccessMs">
<summary>
Records the duration from when the API is called to when the user presses
&quot;Allow&quot; to pass the incoming SMS to the site and proceed with
the SMS verification flow.
</summary>
</metric>
</event> </event>
<event name="SSL.MixedContentShown"> <event name="SSL.MixedContentShown">
......
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