Skip to content
This repository has been archived by the owner. It is now read-only.

Accept and log utm_* metrics params on password change, password reset, and sign-in confirm #3932

Closed
rfk opened this issue Jul 15, 2016 · 7 comments
Assignees
Labels

Comments

@rfk
Copy link
Member

@rfk rfk commented Jul 15, 2016

In https://github.com/mozilla/fxa-auth-mailer/issues/182 we're going to include utm_* metrics parameters in links from our various emails. I'm kinda hopeful that the existing support for such params in this repo will mean that they just start showing up in datadog for free.

But we should confirm that this is the case for the ones we care about most urgently - the password-change, password-reset and confirm-signin links that form key success metrics for [1].

If I follow e.g. a password-reset link from an email, and the link contains utm_* params, will those params show up as event tags in datadog?

[1] https://github.com/mozilla/fxa/tree/master/features/proposed/FxA-99-improved-login-notification#metrics

@rfk rfk added this to the FxA-99: signin geo info milestone Jul 15, 2016
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Jul 18, 2016

@rfk - can you prioritize this one?

@rfk
Copy link
Member Author

@rfk rfk commented Jul 18, 2016

We need it for the sign-in geolocation milestone; -> next

@rfk
Copy link
Member Author

@rfk rfk commented Aug 4, 2016

So AFAICT, we do ingest utm_* params from the page and submit them as part of the metrics bundle to the sever:

https://github.com/mozilla/fxa-content-server/blob/master/app/scripts/lib/metrics.js#L36

But we don't include them in the events that we send to datadog:

https://github.com/mozilla/fxa-content-server/blob/master/server/lib/statsd-collector.js#L25

So we may have some amount of server-side work to do to ensure we can analyze the metrics from https://github.com/mozilla/fxa-auth-mailer/issues/182. @vladikoff would it be reasonable to add the utm_* values as tags on the events sent to datadog, or is this likely to cause Bad Things to happen? I recall there being some issues with specifying too many tags on an event.

@rfk
Copy link
Member Author

@rfk rfk commented Aug 4, 2016

From mtg: @vladikoff had some thoughts on how to manage this which he's going to record in this bug

@vladikoff vladikoff assigned vladikoff and unassigned vladikoff Aug 5, 2016
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Aug 5, 2016

@vbudhram ok so, we already pass the utm params as part of the metrics request:

So just add the utm values into https://github.com/mozilla/fxa-content-server/blob/master/server/lib/statsd-collector.js#L27 to attach them to the events.

@rfk rfk added the shipit label Aug 8, 2016
@vladikoff vladikoff self-assigned this Aug 8, 2016
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Aug 9, 2016

I think this was fixed, cc @vbudhram

@vbudhram
Copy link
Contributor

@vbudhram vbudhram commented Aug 9, 2016

Yup! Forgot to link issue, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants