fix(metrics): don't force utm_source=email on links in emails #2505

philbooth · 2018-07-05T13:41:28Z

I'm putting this forward as a blunt-instrument proposal to fix mozilla/fxa-content-server#6258. It completely removes utm_source=email from the links we send out, which has the obvious knock-on effect of not overwriting utm_source in Amplitude:

{"op":"amplitudeEvent","event_type":"fxa_email - click","time":1530797664542,"user_id":"1f705b2692bc4565bf6a3f25d610d841","device_id":"81a9e7d7aa8c4de0a1cbcb3d3b64068c","session_id":1530797640628,"app_version":"115","language":"en","os_name":"Mac OS X","os_version":"10.11","event_properties":{"service":"sync","email_type":"registration"},"user_properties":{"entrypoint":"menupanel","flow_id":"c4bd39952cae9955fd90b3e892f5a4941a9ccea993e1b70aee76a4fa1a83dcb0","ua_browser":"Firefox","ua_version":"63.0","utm_campaign":"fx-welcome","utm_content":"fx-activate","utm_medium":"email","$append":{"fxa_services_used":"sync"}}}
{"op":"amplitudeEvent","event_type":"fxa_connect_device - view","time":1530797664760,"user_id":"1f705b2692bc4565bf6a3f25d610d841","device_id":"81a9e7d7aa8c4de0a1cbcb3d3b64068c","session_id":1530797640628,"app_version":"115","language":"en","os_name":"Mac OS X","os_version":"10.11","event_properties":{"service":"sync","connect_device_flow":"signin"},"user_properties":{"entrypoint":"menupanel","flow_id":"c4bd39952cae9955fd90b3e892f5a4941a9ccea993e1b70aee76a4fa1a83dcb0","ua_browser":"Firefox","ua_version":"63.0","utm_campaign":"fx-welcome","utm_content":"fx-activate","utm_medium":"email","$append":{"fxa_services_used":"sync"}}}

Compare those events, generated with this change, to the ones logged in #2496 (comment).

Is this too blunt an instrument to fix the problem with? Are we relying on utm_source=email for some other reason? Forgive me if that got discussed in the meeting on Tuesday, I wasn't paying attention properly at the time.

If we need something more fine-grained, I can make some kind of change in the content server that specifically disables utm_source just for those events, but that approach feels more hackish to me.

@mozilla/fxa-devs r?

vbudhram

@philbooth I wonder if we can use a semi-blunt instrument. Possible updating _generateUTMLink to take an optional param forceEmailUtm (something along those lines). It seems like the only link that needs to keep the original utm_source is the primary link of the email.

vbudhram · 2018-07-05T14:21:21Z

test/local/senders/email.js

@@ -376,6 +389,7 @@ describe(

              mailer.mailer.sendMail = function (emailConfig) {
                assert.ok(includes(emailConfig.html, androidStoreLink))
+                assert.ok(! includes(emailConfig.html, 'utm_source=email'))


I think this was one of the scenarios we mentioned that the utm_source was actually an email. Would we lose any downstream events in amplitude or adjust that track app installs from our email?

Does it make sense to enable "utm_source=email" for the app store links only so we don't lose these metrics?

Genuinely I have no idea. That wasn't what I took from the meeting on Friday though. Let's check with @davismtl.

Fwiw, I've reinstated utm_source=email on the app store links directly in config.

shane-tomlinson · 2018-07-05T15:29:15Z

Are we relying on utm_source=email for some other reason?

utm_source=email is used in when the user verifies a signin/signup on a second device. If the user verifies on a 2nd device, the connect another device logic determines whether the user can sign in, and if so, we consider this a new flow.

This situation is a bit strange, because there are two potential flows involved. We have the flow being completed, and the potential new flow. The new flow should have a utm_source=email and we propagate that field here. What if all the utm_ fields sent by the email were the same as the utm_ params at the beginning of the flow, but we added an additional field that indicates the user is coming from an email, perhaps entrypoint:email. Whenever we create the CAD URL for the signin button, we could look at the entrypoint, see it's email, and set the utm_source to email at that point?

philbooth · 2018-07-05T16:12:08Z

What if all the utm_ fields sent by the email were the same as the utm_ params at the beginning of the flow, but we added an additional field that indicates the user is coming from an email, perhaps entrypoint:email.

I feel a bit out of my depth discussing the possibilities here, because there's so much of the surrounding framework that I'm not familiar with.

@davismtl or @irrationalagent have you got some time today or tomorrow to chat with us about the correct solution to this issue / mozilla/fxa-content-server#6258?

shane-tomlinson · 2018-07-05T16:48:04Z

@davismtl or @irrationalagent have you got some time today or tomorrow to chat with us about the correct solution to this issue / mozilla/fxa-content-server#6258?

I'm happy to be part of this chat too. @davismtl and I are already meeting at 16.00 BST, want to join us to discuss?

philbooth · 2018-07-05T16:50:53Z

@davismtl and I are already meeting at 16.00 BST, want to join us to discuss?

👍

Currently, the vast majority of our `fxa_reg - complete` events in Amplitude are incorrectly showing up with `utm_source` set to `email`. This is because the auth server forces it on all links in email addresses, so when the user clicks the verification link the original `utm_source` for that flow is overwritten. A separate PR for the auth server [1] has been opened to prevent this from happening. But in order for that PR not to break legitimate cases where we want `utm_source` set to `email`, we must first handle those legitimate cases elsewhere. Hence this PR. The connect-another-device view can only be reached from a verification email. When a user signs in from that view, we always want the new flow to have `utm_source` set to `email`. This change simply hard-codes the `utm_source` on signin links in that view. Related issues: * #6258 * mozilla/fxa-auth-server#2496 [1]: mozilla/fxa-auth-server#2505

philbooth · 2018-07-09T16:26:15Z

Tested on pbooth.dev.lcip.org in tandem with mozilla/fxa-content-server#6342. If there's no objections it would be good to ship both of these on train 116.

Instructions for testing in mozilla/fxa-content-server#6342 (comment).

@mozilla/fxa-devs r?

shane-tomlinson

Looks great, only question is how to handle app store links. If we decide we don't care about those, r+. Maybe we can add utm_source=email for just the app store links?

Currently, the vast majority of our `fxa_reg - complete` events in Amplitude are incorrectly showing up with `utm_source` set to `email`. This is because the auth server forces it on all links in email addresses, so when the user clicks the verification link the original `utm_source` for that flow is overwritten. A separate PR for the auth server [1] has been opened to prevent this from happening. But in order for that PR not to break legitimate cases where we want `utm_source` set to `email`, we must first handle those legitimate cases elsewhere. Hence this PR. The connect-another-device view can only be reached from a verification email. When a user signs in from that view, we always want the new flow to have `utm_source` set to `email`. This change simply hard-codes the `utm_source` on signin links in that view. [1]: mozilla/fxa-auth-server#2505 Related issues: * #6258 * mozilla/fxa-auth-server#2496 #6342 r=shane-tomlinson

davismtl · 2018-07-17T18:51:18Z

Looks great, only question is how to handle app store links. If we decide we don't care about those, r+. Maybe we can add utm_source=email for just the app store links?

I'm a bit confused by this. Which app store links are you referring to? The ones in the emails?

fix(metrics): don't force utm_source=email on links in emails

90dac65

philbooth added the waffle:review label Jul 5, 2018

philbooth self-assigned this Jul 5, 2018

philbooth requested a review from a team July 5, 2018 13:41

vbudhram reviewed Jul 5, 2018

View reviewed changes

philbooth mentioned this pull request Jul 6, 2018

fix(metrics): force utm_source=email when signing in from CAD mozilla/fxa-content-server#6342

Merged

philbooth added the shipit label Jul 9, 2018

shane-tomlinson approved these changes Jul 9, 2018

View reviewed changes

fix(metrics): reinstate utm_source=email on app store links

9db2c22

philbooth merged commit e47b710 into master Jul 9, 2018

ghost removed the waffle:review label Jul 9, 2018

philbooth deleted the pb/content-6258 branch July 9, 2018 20:59

davismtl mentioned this pull request Jul 19, 2018

Since Release of Train 116, Growing Number of utm_source = (none) mozilla/fxa-amplitude-send#72

Closed

shane-tomlinson added this to the FxA-0: quality milestone Aug 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(metrics): don't force utm_source=email on links in emails #2505

fix(metrics): don't force utm_source=email on links in emails #2505

philbooth commented Jul 5, 2018 •

edited

vbudhram left a comment •

edited

vbudhram Jul 5, 2018

shane-tomlinson Jul 9, 2018

philbooth Jul 9, 2018

philbooth Jul 9, 2018

shane-tomlinson commented Jul 5, 2018

philbooth commented Jul 5, 2018

shane-tomlinson commented Jul 5, 2018

philbooth commented Jul 5, 2018

philbooth commented Jul 9, 2018

shane-tomlinson left a comment

davismtl commented Jul 17, 2018

fix(metrics): don't force utm_source=email on links in emails #2505

fix(metrics): don't force utm_source=email on links in emails #2505

Conversation

philbooth commented Jul 5, 2018 • edited

vbudhram left a comment • edited

Choose a reason for hiding this comment

vbudhram Jul 5, 2018

Choose a reason for hiding this comment

shane-tomlinson Jul 9, 2018

Choose a reason for hiding this comment

philbooth Jul 9, 2018

Choose a reason for hiding this comment

philbooth Jul 9, 2018

Choose a reason for hiding this comment

shane-tomlinson commented Jul 5, 2018

philbooth commented Jul 5, 2018

shane-tomlinson commented Jul 5, 2018

philbooth commented Jul 5, 2018

philbooth commented Jul 9, 2018

shane-tomlinson left a comment

Choose a reason for hiding this comment

davismtl commented Jul 17, 2018

philbooth commented Jul 5, 2018 •

edited

vbudhram left a comment •

edited