If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Add quality metrics for FxA Verification via Push

RESOLVED FIXED in Firefox 48

Status

()

Core
FxAccounts
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: vladikoff, Assigned: vfilippov)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox47 wontfix, firefox48 fixed)

Details

(Whiteboard: [fxa-waffle])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
We need to be able to measure if FxA Verification via Push works properly. 

This issue is related to: https://bugzilla.mozilla.org/show_bug.cgi?id=1235607
It does not block the issue above.
(Reporter)

Updated

2 years ago
Depends on: 1235607
Component: Sync → FxAccounts
Product: Firefox → Core
Version: 47 Branch → unspecified
(Reporter)

Updated

2 years ago
Whiteboard: [fxa-waffle]
Comment from markh in Bug 1254404

"Bug 1247786 landed basic support for a push notification to detect when an account is verified. Polling is still enabled as a backup, so we should try and measure how effective push is, with an eye to either fixing it if it doesn't get most users, or removing (or severely limiting) the polling if it does."
Duplicate of this bug: 1254404
According to the feature card at [1] our current "success criteria" are:

"""
This feature will be successful if we successfully eliminate polling for account verification from the browser code on Desktop. Mobile browsers are not in scope for the initial version of this feature.

As a concrete metric we can measure the number of calls to /recovery_email/status and check that it decreases as this feature rolls out.
"""

This doesn't seem to capture the current state of things.  At the very least, we should de-scope to remove the word "eliminate" ;-)

If I understand correctly, the patch for this at [2] reduces the frequency with which we poll /recovery_email/status.  I definitely expect this to decrease the number of calls to that endpoint, but it will do so even if push is not working.  So what's the new strategy for measuring success of this feature?

Vlad, in Bug 1254404 Comment 1 you mentioned a server-side strategy using DataDog, when you get a chance can you please update the feature card to include some details of it?

[1] https://github.com/mozilla/fxa/tree/master/features/FxA-54-push-on-status-change
[2] https://hg.mozilla.org/integration/mozilla-inbound/rev/4221fa1f45c3
(In reply to Ryan Kelly [:rfkelly] from comment #3)
> If I understand correctly, the patch for this at [2] reduces the frequency
> with which we poll /recovery_email/status.  I definitely expect this to
> decrease the number of calls to that endpoint, but it will do so even if
> push is not working.  So what's the new strategy for measuring success of
> this feature?

Note also that in the patch as landed, we still check /status after receiving the push notification (ie, we don't treat the message itself as "user is verified" but instead as "now would be a great time to check if they are verified".) So without the client sending a specific signal to the server (eg, query param) I'm struggling to see how the server can help work out whether the client hit that endpoint due to normal polling, or due to the push message. (I guess if the server knows the time the message was delivered and the time the /status call was made it could infer this, but that sounds awkward)
(Reporter)

Comment 5

2 years ago
Created attachment 8731000 [details]
MozReview Request: Bug 1249029 - Add quality metrics for FxA Verification via Push r=markh

Review commit: https://reviewboard.mozilla.org/r/40301/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40301/
Attachment #8731000 - Flags: review?(markh)
(Reporter)

Comment 6

2 years ago
Comment on attachment 8731000 [details]
MozReview Request: Bug 1249029 - Add quality metrics for FxA Verification via Push r=markh

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40301/diff/1-2/
(Reporter)

Comment 7

2 years ago
https://reviewboard.mozilla.org/r/40301/#review36811

@markh, do you agree with this approach? I shall add tests if this is on the right track
Comment on attachment 8731000 [details]
MozReview Request: Bug 1249029 - Add quality metrics for FxA Verification via Push r=markh

We discussed a slightly different strategy on IRC - clearing the review flag to make things clear.
Attachment #8731000 - Flags: review?(markh)
(Reporter)

Comment 9

2 years ago
Comment on attachment 8731000 [details]
MozReview Request: Bug 1249029 - Add quality metrics for FxA Verification via Push r=markh

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40301/diff/2-3/
Attachment #8731000 - Flags: review?(markh)
(Reporter)

Comment 10

2 years ago
Comment on attachment 8731000 [details]
MozReview Request: Bug 1249029 - Add quality metrics for FxA Verification via Push r=markh

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40301/diff/3-4/
Comment on attachment 8731000 [details]
MozReview Request: Bug 1249029 - Add quality metrics for FxA Verification via Push r=markh

https://reviewboard.mozilla.org/r/40301/#review38027
Attachment #8731000 - Flags: review?(markh) → review+

Comment 12

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5d05c652d6b
(Reporter)

Comment 13

2 years ago
Thank you Mark!!

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d5d05c652d6b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Reporter)

Updated

2 years ago
status-firefox47: affected → wontfix
You need to log in before you can comment on or make changes to this bug.