Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Create Metric Event for Form Views without iFrame #5882

Closed
davismtl opened this issue Feb 6, 2018 · 47 comments
Closed

Create Metric Event for Form Views without iFrame #5882

davismtl opened this issue Feb 6, 2018 · 47 comments

Comments

@davismtl
Copy link

davismtl commented Feb 6, 2018

Various teams plan to capture email first and send users to our login/registration flows after. (onboarding/firstrun, snippets and mozilla.org)

By reducing the use of our iFrame, we will lose the ability to track out top of funnel. This will also prevent these teams from measuring their entire funnel in Amplitude.

I propose that we provide a js snippet (or something similar) that would serve to track our top off funnel (i.e. email first view > email first engage).

To make this work, I presume that we would need to make sure that users have the same flow_id as the user moves between product teams (e.g. snippet to fxa form).

@rfk
Copy link
Contributor

rfk commented Feb 7, 2018

For planning purposes, what sort of schedule do we want this on?

A couple of thoughts on implementation possibilities here while I think of it...

The flowids have to be generated by fxa-content-server, because they have signed validation data included in them. The process might have to look something like this:

  • Firstrun page includes a hidden iframe from accounts.firefox.com
  • The content-server generates and sends a flowid in the HTML contents for the iframe
  • When the iframe loads, it sends the "flow.begin" metric back to the server
  • The hidden iframe somehow persists the flowid for use when the containing page redirects to our email-first form
    • stash it in a cookie or localStorage?
    • have the containing page pass it in via query parameter?

So...not terrible, but non-trivial.

This will also prevent these teams from measuring their entire funnel in Amplitude

I wonder if this is actually an opportunity for us to think more holistically about getting cross-product metrics into amplitude. If we can arrange for the first-run page to be submitting events to amplitude under a specific device-id, and can arrange for that device-id to be passed to FxA so we can use it for our own amplitude metrics, maybe we don't need to worry about passing our own custom "flow id" around everywhere. @davismtl let's try to pick up that thread of conversation when we next meet.

@davismtl
Copy link
Author

davismtl commented Feb 8, 2018

Per my discussion with @rfk :

  • I will move discussions forward for device and user id alignment. we identified next steps are to get a few technical people on a call from pipeline, data science, fxa and product.
  • Since that will be more of a longterm solution, we need a short-term solution to enable onboarding and marketing do optimization without our frequent help. This means, we need to provide them a JS snippet or something of the sort to align flow_ids.
  • Questions then become: how long would this take? and, what would @philbooth have to not do this quarter to do this?
  • I'll sync up with Andy on this.

@vladikoff
Copy link
Contributor

@davismtl @philbooth and everyone else to chat about that

@andymckay
Copy link

@davismtl do you have any timelines for the other authentication flows to go live so that we could plan for this?

I was chatting to @philbooth about this, would an API end point on the FxA server to generate the flow_id be sufficient? Before starting the email flow, the client would request the flow_id and send that along with the email. That could be a pretty straightforward change (I imagine).

I don't have information about what threats or concerns the flow_id protects against though, so I'll look to others for that kind of background.

@davismtl
Copy link
Author

@davismtl do you have any timelines for the other authentication flows to go live so that we could plan for this?

I have requested specific dates. I just know that they were planning tests for Q1 as part of their OKRs.

I was chatting to @philbooth about this, would an API end point on the FxA server to generate the flow_id be sufficient? Before starting the email flow, the client would request the flow_id and send that along with the email. That could be a pretty straightforward change (I imagine).

IIUC, this seems like a good solution. Can't think of any reason why it wouldn't work.

@philbooth
Copy link
Contributor

I just got round to properly reading this thread, as I said I'd give Alex an estimate today. Before I estimate anything though, there's a couple of questions I have about the requirements and one of the suggested approaches.

  1. Question for @davismtl (with apologies if you told me in our meeting yesterday, I forgot the answer already). Is it sufficient for us to just emit the flow.begin event ourselves at the top of the funnel, or do we also need to communicate event properties to the external page so that it can emit it's own events as the user interacts with it? (I'm assuming the latter is what we want but if the former is sufficient, everything becomes easier)

  2. Question for @rfk. With your iframe suggestion, did you have a plan for sending session_id (a.k.a. flowBeginTime) and device_id to the external resource (if we need to send them)? I presume there'll be a cross-domain hurdle but we can get over it using - waves hands - CORS? And we don't care about older, pre-CORS clients?

Of the two suggestions in the thread so far, adding an endpoint to return flow_id, session_id and device_id seems cleaner, simpler and easier to implement. The iframe one gives me the willies.

So, on to the estimates then:

I think adding an endpoint would take - made-up numbers alert - 2 days and doing iframe magic would take 2 weeks. Most of the 2 weeks is predicated on my uneasiness about cross-domain issues but someone who knows what they're doing would probably estimate a much smaller number. I am somewhat out of my depth on that score.

@davismtl
Copy link
Author

Is it sufficient for us to just emit the flow.begin event ourselves at the top of the funnel

I believe this to be sufficient. The marketing teams will use our tools to do their funnel and retention analysis. I don't see the value of communicating the event properties to the external page since they can probably capture some via their own metrics (which in contrast only have the top of funnel).

@philbooth
Copy link
Contributor

Cool, in that case the iframe one definitely isn't 2 weeks fwiw. Call that one a week maybe?

@davismtl
Copy link
Author

davismtl commented Feb 15, 2018

@davismtl do you have any timelines for the other authentication flows to go live so that we could plan for this?

Snippets team is launching their first test this month and into March so I am checking that they are tracking the top of funnel themselves. They'll have to consolidate metrics manually after.

/Firsrun tests won't start development for at least another 2 weeks.

@rfk
Copy link
Contributor

rfk commented Feb 15, 2018

Simplicity++ FWIW!

I'm probably missing context, but I'm confused by the proposal here.

Is it sufficient for us to just emit the flow.begin event ourselves at the top of the funnel. or do we
also need to communicate event properties to the external page so that it can emit it's own events
as the user interacts with it?

I believe this to be sufficient.

The initial comment suggests you were worried about losing visibility of the "email first view" and "email first engage" events, which we can no longer emit ourselves. Do we expect those events to still be emitted somehow, and show up in amplitude with appropriate flow_id/device_id attribution that matches the values used by FxA web content later on in the funnel?

IOW: if we're not going to let the /firstrun page emit its own metrics that are tagged with flow_id/device_id, then what are we trying to accomplish here?

Question for @rfk. With your iframe suggestion, did you have a plan for sending session_id
(a.k.a. flowBeginTime) and device_id to the external resource (if we need to send them)?
I presume there'll be a cross-domain hurdle but we can get over it using - waves hands - CORS?
And we don't care about older, pre-CORS clients?

I was imagining a postMessage API where the containing page could send us a message saying "log this event" and our iframe would annotate it with flow_id etc, then send it to our backend metrics endpoint. So it wouldn't have to communicate them out (and indeed the containing page wouldn't have to know what a "flow id" is at all).

Of the two suggestions in the thread so far, adding an endpoint to return flow_id, session_id
and device_id seems cleaner, simpler and easier to implement.

I agree, if this is the only API surface that we need to expose to the external page. Will that page then take these values and incorporate them into its own events that it submits directly to amplitude? Do we care about ensuring that those events are the correct shape or meet other expectations?

Mostly my concern here is about the API surface between two independent teams, the amount of complexity that gets split across it, and what we're locking ourselves into supporting on an ongoing basis.

The iframe one gives me the willies.

That's fair :-)

@davismtl
Copy link
Author

The initial comment suggests you were worried about losing visibility of the "email first view" and "email first engage" events, which we can no longer emit ourselves. Do we expect those events to still be emitted somehow, and show up in amplitude with appropriate flow_id/device_id attribution that matches the values used by FxA web content later on in the funnel?

Yes, ideally we can capture all of those. Apologies if I was unclear in a previous post.

IF we can't, then at a minimum, let's get flow_begin which is equal to fxa_email_first - view. We can assume the submit by reg/login view. We would just lose the engage event.

@rfk
Copy link
Contributor

rfk commented Apr 30, 2018

This will be relevant for https://bugzilla.mozilla.org/show_bug.cgi?id=1446023

@hoosteeno
Copy link

As one of the primary customers of the on-page analytics, I would not block this effort until we figure out engage events. We're not optimizing forms at that level of magnification; we need view tracking on an iframeless form much more.

@rfk
Copy link
Contributor

rfk commented Apr 30, 2018

I'm putting this in "next" to review now that this work is active on the moz.org side. @philbooth, has the way we use amplitude device ids, flows ids, etc changed much since the above discussions took place?

@hoosteeno
Copy link

The proof of concept for an iframeless form comes from Snippets: https://github.com/mozmeao/snippets/blob/master/activity-stream/fxa.html#L254.

In order to get this moving on www.mozilla.org, we'll need some kind of documentation of the API that a client of this data collection service should use. That'll give us a place to start for product owner and data steward sign-offs.

@rfk
Copy link
Contributor

rfk commented May 1, 2018

Based on #5882 (comment), the simplest thing I can imagine working here would be:

  • You make a GET request to an endpoint on https://accounts.firefox.com
  • That endpoint logs a "view" event into our pipeline on the server side, and returns an opaque token with some state in it
  • You include that token as an extra query param when opening FxA in a new tab

That would not allow us to capture any detailed events of user activity in the iframeless page, only a single "the user viewed a page with some custom FxA messaging on it". But it sounds like that would be enough to unblock work here.

@rfk
Copy link
Contributor

rfk commented May 1, 2018

(Note that this would also avoid the need to include the amplitude SDK in the first-run page, because we'd emit a single event and it would be server-side)

@shane-tomlinson
Copy link

shane-tomlinson commented May 2, 2018

I'm wondering if we can take care of everything with Cookies that way the reliers do not need to worry about passing around any extra info.

Here's how I can imagine it working, but I have not yet tried this:

  1. Relier makes a call to an endpoint on the FxA content-server (the domain is important because of domain restrictions for the cookies).
  2. The FxA content server provisions flow_id, session_id and device_id and maybe a flow_begin_time. A flow.begin event is emit.
  3. The FxA content-server sets an HttpOnly cookie with the info from step 2. The cookie would have a path of / to allow a redirection to any content-server endpoint. The cookie would also have domain set to https://accounts.firefox.com/ to prevent the cookie from being sent to all API endpoint calls. Finally, the cookie would have a short max-age, perhaps a few minutes or hours.
  4. The first-run page doesn't need to do anything else when it redirects to FxA.
  5. Before the content server renders the page, it looks for the existence of the cookie from step 3.
    • If the cookie exists, ensure the cookie is still valid (perhaps something went awry and it's malformed or expired?), and if so, use that information to populate the appropriate body element attributes. Remove the cookie.
    • If the cookie does not exist, provision info as is currently done.

@rfk
Copy link
Contributor

rfk commented May 2, 2018

That sounds pretty good to me, modulo any concerns about users having cookies disabled. But I guess those users were never going to succeed at signin in anyway :-)

Since such a cookie would not be for any direct user benefit, I wonder if there would be any problems from a policy standpoint.

@hoosteeno
Copy link

@jpetto can you review #5882 (comment) and #5882 (comment)? These are alternative API client implementations and as the first implementer you may have some feedback.

@shane-tomlinson
Copy link

shane-tomlinson commented May 3, 2018

That sounds pretty good to me, modulo any concerns about users having cookies disabled. But I guess those users were never going to succeed at signin in anyway :-)

This comment makes me realize this approach will fail if the user has 3rd party cookies disabled or set to "from visited" and they have not yet visited FxA or they've enabled first-party isolation.

I'm not sure what percentage of people have these features enabled, but I imagine more than we are willing to ignore.

@shane-tomlinson
Copy link

This comment makes me realize this approach will fail if the user has 3rd party cookies disabled or set to "from visited." and they have not yet visited FxA or they've enabled first-party isolation.

Of course, if firstrun is hosted by a system addon, I don't know if these policies and resulting problems apply.

@jpetto
Copy link
Contributor

jpetto commented May 3, 2018

If the cookie approach isn't viable (for whatever reason), it wouldn't be too much work to make an API call to the FxA content server (which I believe would need to happen even if we go the cookie route), store a token, and include that token in the querystring when redirecting to FxA.

How soon are we looking to implement this? I have some code that should be on a demo server tomorrow with a basic iframe-less approach (similar to that done recently in snippet).

@shane-tomlinson shane-tomlinson self-assigned this May 9, 2018
@vladikoff vladikoff self-assigned this May 15, 2018
@davismtl
Copy link
Author

davismtl commented Jun 21, 2018

Where can I find the details about the GET endpoint, params, etc? (and how it will show up in Amplitude)

@chrismore might need this for the about:welcome page that will be replacing /firstrun.
https://bugzilla.mozilla.org/show_bug.cgi?id=1448918

They've abolished the iFrame and will be passing us the user's email.

@chrismore
Copy link

the specific issue this is creating is documented in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1470336

@rfk
Copy link
Contributor

rfk commented Jun 22, 2018

I'm going to re-open this for the creation of some consumer docs, perhaps somewhere in https://github.com/mozilla/fxa-content-server/blob/master/docs/client-metrics.md

IIUC the intention is simply for the relier to GET https://accounts.firefox.com/metrics-flow and include the values from the JSON response in the query parameters when redirecting to FxA. However, I can't make that endpoint work on current production:

$ curl https://accounts.firefox.com/metrics-flow
{"error":"Unauthorized","message":"CORS Error","statusCode":401}

So it looks like we may need to make a config change in prod to allow the necessary origins here:

https://github.com/mozilla/fxa-content-server/pull/6227/files#diff-a7802dc9db070ba75283c9fc3199eb02R21

@rfk rfk reopened this Jun 22, 2018
@vladikoff
Copy link
Contributor

@rfk 👍 which origins are missing?

@rfk
Copy link
Contributor

rfk commented Jun 22, 2018

One potential wrinkle...Bug 1470336 involves the new about:welcome page, and about:welcome is browser content that sends Origin: null. But I guess we already handle that in some other places so it should be OK to accept it here..?

I think we should set the allowed origins here to match the existing ALLOWED_PARENT_ORIGINS settings, with the inclusion of "null" if that's not already in the list. @shane-tomlinson may want to weigh in on the specifics here.

@vladikoff
Copy link
Contributor

about:welcome is browser content that sends Origin: null

Yeah that is not the integration point that we tested during development of this feature

@shane-tomlinson
Copy link

I think we should set the allowed origins here to match the existing ALLOWED_PARENT_ORIGINS settings, with the inclusion of "null" if that's not already in the list. @shane-tomlinson may want to weigh in on the specifics here.

ALLOWED_PARENT_ORIGINS is meant to be for iframing purposes is the list of origins that can frame FxA, there are only a couple on prod and the list happens to match the list for ALLOWED_METRICS_FLOW_ORIGINS.

If the suggestion is to only add null to ALLOWED_METRICS_FLOW_ORIGINS I'm fine with that. TBH, I'm surprised we do a server side check, I'd expect us to return an Access-Control-Allow-Origin: header and let the browser finish the check. IIRC, XHR requests made within the browser context ignore CORS headers and should just work. Someone who wanted to to attack us would programatically set the Origin header anyways, so I don't fully understand the server side check.

If the suggestion is to add null to ALLOWED_PARENT_ORIGINS, I'm very reluctant to do so w/o a lot of due diligence to avoid security problems.

@rfk
Copy link
Contributor

rfk commented Jun 25, 2018

If the suggestion is to add null to ALLOWED_PARENT_ORIGINS, I'm very reluctant to do
so w/o a lot of due diligence to avoid security problems.

Definitely not suggesting this.

@rfk
Copy link
Contributor

rfk commented Jun 25, 2018

Somehow I had thought these were not configured in prod, but I must have been testing it wrong because this does indeed work:

$ curl -H "Origin: https://www.mozilla.org" https://accounts.firefox.com/metrics-flow
{"flowBeginTime":1529887567153,"flowId":"98850f14ad43991f07de9542ae002d6f8e42760e034d1256c0a6a7a9654b6bf1"}

But omitting the Origin header or sending Origin: null means the request will fail:

$ curl -H "Origin: null" https://accounts.firefox.com/metrics-flow
{"error":"Unauthorized","message":"CORS Error","statusCode":401}

This might be a cleaner solution than adding "null" to the list of allowed origins tho:

TBH, I'm surprised we do a server side check, I'd expect us to
return an Access-Control-Allow-Origin: header and let the browser finish the check.

@vladikoff vladikoff removed their assignment Jun 25, 2018
@vladikoff
Copy link
Contributor

@jpetto could you clarify how you want this integration to work? If you want this to work in about:welcome then it's a new requirement and we will have to update our servers.

I also looked at mozilla/bedrock#5713 and that was not merged yet.

@davismtl
Copy link
Author

@vladikoff I don't think @jpetto is working on the about:welcome.

That's the onboarding team who also did an iframeless integrations of FxA.

@hoosteeno
Copy link

@davismtl, do you expect (or will you advocate for) the FxA signup/signin form to land on about:welcome (and similar) as our onboarding experiences move into the product? If so, additional work to support funnel visibility in those channels seems like a good investment to me.

On the web side, it has been my hope that an email-only, iframeless FxA form could land in places it's never lived before (e.g. other web sites, or other places on www) and that we could measure its performance there. This might help mitigate the potential loss of one of our most important acquisition channels if the form does not land in about:welcome. This assumes that the affordance we've built above is generic enough to work from other pages and domains.

@davismtl
Copy link
Author

@hoosteeno It already has landed on about:welcome. You can go to it in Nightly.

@shane-tomlinson
Copy link

It sounds like there is some confusion. There are 2 iframeless flows:

  1. Hosted in bedrock by mozilla.org, done by @jpetto
  2. Hosted in about:wecome, done by @ericawright

The mozilla.org hosted version should always send an Origin header, the about:welcome variant will not since it's hosted in browser code.

I imagine we'll need about:welcome to make a call to https://accounts.firefox.com/metrics-flow to get the flowId and flowBeginTime, then propagate those to FxA as a hidden form element.

@ericawright - what is the best way for us to progress to get about:welcome updated so that we can gather metrics for users who arrive from about:welcome?

@ericawright
Copy link

@shane-tomlinson the best way would be to open an issue in bugzilla in the in the Activity Streams: Newtab component. But also you can ping me and I'll make sure to get it in.
At this point I don't fully understand what is missing or needs to be changed, is this still being decided?

@davismtl
Copy link
Author

@ericawright while the removal of the iframe is desired by all, it just prevents us from monitoring our top of login and registration funnel (i.e. form view). We just want to provide you with a means to trigger that first event so that we can more easily monitor the performance of FxA login and registration.

@shane-tomlinson should get back to you shortly with the technical details.

@shane-tomlinson
Copy link

@ericawright, @davismtl I'm at a conference the next two days, perhaps @philbooth or @vladikoff can provide support in my absence.

Alex provided the "why", we won't have top of funnel insights for users coming from about:welcome.
On to the "how" this can be fixed.

To seed the top of funnel, we require a call to FxA that sets a marker that says "This user's flow started NOW." In the call to FxA we return an ID and a begin time that needs to be propagated from about:welcome to FxA.

More concretely, about:welcome needs to make an XHR call to https://accounts.firefox.com/metrics-flow. The domain name of the request should match the FxA that is being redirected to. Bedrock uses fetch to get this info. The response to metrics-flow will be a JSON object that contains a flowId and flowBeginTime. These values need to be propagated to FxA as query parameters, which can be done using hidden form fields with the names "flow_id" and "flow_begin_time".

@philbooth - is this an accurate summary of what needs to happen?

@shane-tomlinson
Copy link

Opened https://bugzilla.mozilla.org/show_bug.cgi?id=1471514 to track this from the Activity Stream side.

@ericawright
Copy link

ericawright commented Jun 27, 2018

oh, I was misunderstanding that FXA doesn't use telemetry. Since all of the missing data is going to telemetry.

@vladikoff
Copy link
Contributor

Linked instructions for about about:welcome in #6267 (comment)

@vladikoff
Copy link
Contributor

Events should be coming in from both integrations now 🍰

@ghost ghost removed the waffle:backlog label Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants