-
Notifications
You must be signed in to change notification settings - Fork 120
fix(pw-strength): Report password strength metrics to amplitude #6353
Conversation
this.saveState('blocked'); | ||
this.saveState(state); | ||
logPasswordBlockedError (state) { | ||
this.logEvent('blocked'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed from saveState
to logEvent
because saveState
will allow at most one of each event type per browser, unless the user clears their localStorage.
I'd like to create a content-server point release for this PR to ship once train-116 goes to prod, and then a separate point release for #6355 once we know metrics are being reported to amplitude correctly. |
server/lib/amplitude.js
Outdated
@@ -54,6 +54,10 @@ const VIEW_ENGAGE_SUBMIT_EVENT_GROUPS = { | |||
// In the following regular expressions, the first match group is | |||
// exposed as `eventCategory` and the second as `eventTarget`. | |||
const FUZZY_EVENTS = new Map([ | |||
[ /^experiment\.(?:designF|designG)\.passwordStrength\.([\w]+)$/, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regexp needs to include control
6e0e95c
to
9ff6fc0
Compare
@@ -54,6 +54,10 @@ const VIEW_ENGAGE_SUBMIT_EVENT_GROUPS = { | |||
// In the following regular expressions, the first match group is | |||
// exposed as `eventCategory` and the second as `eventTarget`. | |||
const FUZZY_EVENTS = new Map([ | |||
[ /^experiment\.(?:control|designF|designG)\.passwordStrength\.([\w]+)$/, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@philbooth - will the F and G in designF and designG be a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the output, it looks like those are automagically converted to snake case.
Here's an example of the output for control: {"op":"amplitudeEvent","event_type":"fxa_reg - password_blocked","time":1531327011280,"device_id":"6ac896fd1ee14fa3a3157fe95cb0ed18","session_id":1531326844360,"app_version":"115","language":"en","os_name":"Mac OS X","os_version":"10.13","event_properties":{"service":"sync"},"user_properties":{"flow_id":"bd776c017a055f388afc73f1acf8eafe9ff0f57e91d5f215ae0b3c22c171d97b","ua_browser":"Firefox","ua_version":"61.0","$append":{"fxa_services_used":"sync","experiments":["password_strength_control"]}}}
{"op":"amplitudeEvent","event_type":"fxa_reg - password_missing","time":1531327011280,"device_id":"6ac896fd1ee14fa3a3157fe95cb0ed18","session_id":1531326844360,"app_version":"115","language":"en","os_name":"Mac OS X","os_version":"10.13","event_properties":{"service":"sync"},"user_properties":{"flow_id":"bd776c017a055f388afc73f1acf8eafe9ff0f57e91d5f215ae0b3c22c171d97b","ua_browser":"Firefox","ua_version":"61.0","$append":{"fxa_services_used":"sync","experiments":["password_strength_control"]}}} Here's an example of the output for "designF": {"op":"amplitudeEvent","event_type":"fxa_reg - password_blocked","time":1531327164748,"device_id":"b93dbb8a41f04f16a66db702d60b8c07","session_id":1531327159781,"app_version":"115","language":"en","os_name":"Mac OS X","os_version":"10.13","event_properties":{"service":"sync"},"user_properties":{"flow_id":"9997a3df1d474cfee7573f14d2367e26b41e92872c95506232d39812a7d65e5d","ua_browser":"Firefox","ua_version":"61.0","$append":{"fxa_services_used":"sync","experiments":["password_strength_design_f"]}}}
{"op":"amplitudeEvent","event_type":"fxa_reg - password_too_short","time":1531327164749,"device_id":"b93dbb8a41f04f16a66db702d60b8c07","session_id":1531327159781,"app_version":"115","language":"en","os_name":"Mac OS X","os_version":"10.13","event_properties":{"service":"sync"},"user_properties":{"flow_id":"9997a3df1d474cfee7573f14d2367e26b41e92872c95506232d39812a7d65e5d","ua_browser":"Firefox","ua_version":"61.0","$append":{"fxa_services_used":"sync","experiments":["password_strength_design_f"]}}} |
@philbooth is on PTO, removing his review request (do not review this @philbooth!). @mozilla/fxa-devs - r? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+
fixes #6349
@philbooth - r?