Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.41k stars 1.99k forks source link

Stepper: Seah's onboarding flow 2nd test results #95106

Open skim1220 opened 1 week ago

skim1220 commented 1 week ago

I tested the Stepper onboarding flow for a few different scenarios and checked if the key signup events are firing and working as expected. I also checked it against the Classic onboarding flow to verify if there is any difference between two flows.

All the information and details are in this Google Doc, but I am sharing it as an issue in our Github board as well.

(cc: @escapemanuele , @MicBosi )

Summary

In all test scenarios, the key signup events fired correctly, without duplication and in the proper order. The main event properties used in analytics, including the flow and step properties, were accurate and consistent throughout the user journey. I also verified the events triggered by interactions with dropdown menus.

Things that are not blockers for our experiment but ideal to be fixed soon

1 [Stepper Tracking] calypso_signup_complete event is incorrectly logging signup_domain_origin property, which always appears as not-set:

2 [Stepper Tracking] Upon clicking “Use my own domain” option at the Domain Selection step in the Stepper flow, calypso_signup_start event fires but it shouldn’t. See this screenshot. The detailed event log is available in the Test #02 results. This does not happen in the Classic flow.

→ We could filter out this false calypso_signup_start event by checking user_id. This event will have a numeric user_id attached whereas the true calypso_signup_start event would have anonid. However, this type of filtering could get complicated when we start considering the existing users entering the signup flows.. So, it would be great to fix this soon so that we do not need to do any extra filtering.

3 [Stepper Tracking] calypso_signup_actions_submit_step (step: use-my-domain) fires two times. See the details in the Test #02 results.

Other observations:

1 [Stepper Tracking] On the Checkout page, the Stepper flow logs a step called create-site and a step called site-creation-step with the following events. These steps however don’t seem to exist in the Classic flow.

calypso_signup_step_start (step: create-site; step: site-creation-step) calypso_signup_actions_submit_step (step: create-site; step: site-creation-step) calypso_signup_actions_complete_step (step: create-site)

2 [Stepper Tracking] The checkout page in the Stepper flow fires multiple calypso_page_view events and calypso_signup_step_start events for three different steps (that seem to be happening behind the scenes): create-site, site-creation-step, processing. This doesn't affect signup analytics, as we don't track these steps—just noting for reference.

3 [Stepper Tracking] This PR introduced skip_step_render property to calypso_signup_step_start, calypso_page_view, and calypso_signup_actions_complete_step events. However, I haven’t seen this property with these events during my audit. (cc: @chriskmnds)

4 [Classic vs. Stepper] When I landed on the Account Creation step, the Classic flow displayed the message “Also available in English” (screenshot), but it didn’t appear in the Stepper flow.

5 [Classic vs. Stepper] I observed three instances where Stepper tracking is more accurate than Classic! 😄

6 On the right pane of the checkout page, clicking the upsell button (screenshot) didn’t trigger any Tracks event. This happens in both Classic and Stepper. It would be nice to track this interaction as well.

7 On the checkout page, changing the domain renewal interval triggers calypso_checkout_composite_plan_length_change event. However, its new_product_slug property doesn’t include any information about the renewal interval (e.g., dotblog_domain). This occurs in both the Classic flow and the Stepper flow.

Results

Please find all the test details in the Google Doc links below. For each test scenario, you can see the steps taken as well as the key Tracks events and properties checked in the chronological order (in the order of firing).

chriskmnds commented 1 week ago

[Stepper Tracking] Upon clicking “Use my own domain” option at the Domain Selection step in the Stepper flow, calypso_signup_start event fires but it shouldn’t. See [this screenshot](). The detailed event log is available in the 1BPDrnHB2lA3TBfVOrDIp-arsOCCUZ6LAWXVoT1xCy-s-gdoc results. This does not happen in the Classic flow.

I can also investigate here. However, I want to highlight something:

→ We could filter out this false calypso_signup_start event by checking user_id. This event will have a numeric user_id attached whereas the true calypso_signup_start event would have anonid. However, this type of filtering could get complicated when we start considering the existing users entering the signup flows.. So, it would be great to fix this soon so that we do not need to do any extra filtering.

Filtering out on the analytics ends should be the way forward. Unless we are looking at a bug (like the 50-times bug from the other day), then we need to accept that Stepper may record the same event multiple times given whatever logic may be causing a rerender. I am not sure why I feel the need to repeat this over and over, it doesn't excuse fixing potential bugs either (the above could be a bug), but it is a decision that we agreed on in https://github.com/Automattic/wp-calypso/issues/94175, and it will become especially prevalent once https://github.com/Automattic/wp-calypso/issues/94108 is fully deployed.

@skim1220 / @MicBosi / @escapemanuele Just saying, we need to make sure we are properly aligned behind this, otherwise detract from it. Otherwise, it will look weird. Just want to make sure we have agreement (especially @skim1220 since you bring it up without any sort of footnote). "extra filtering" will be the norm going forward, and "no filtering" the exception e.g. for flows that want to see the whole spectrum/variation of props being passed through.

skim1220 commented 1 week ago

Hey @chriskmnds , sorry that my intention wasn't clear. I'd like to clarify the reasoning behind the request you mentioned.

"Extra filtering" will be the norm going forward, with "no filtering" being the exception (e.g., for flows that need to see the full range of properties being passed through).

I understand we’ll never reach a point where no filtering is needed—it’s simply not feasible, and it’s not our goal. Instead, we're aiming to standardize tracking so that filtering is applied as consistently as possible throughout the user signup journey, preferably across all signup flows and steps.

As mentioned in various places, including this post and this FG page, calypso_signup_start event is used as a marker for when users begin the signup process. However, the instance I reported is clearly not at the start of the signup flow; it's in the middle, when the user submits their domain choice. This goes against the intended use of calypso_signup_start event - at least from the analytics standpoint - so I believe it needs to be corrected.

If we don’t address the issue, we can introduce extra filtering just to handle this "Use my own domain" case at the Domain Step—something that isn't necessary for other domain choices as well as for other signup steps. Moreover, this issue may not even exist in other signup flows, making the filtering overly specific to this one case.

My point was that the issue needs to be resolved because it is not aligned with our primary usage of calypso_signup_start event in the analytics. We could implement extra filtering as a temporary fix whenever it is needed until the issue is resolved, rather than relying on extra filtering as the long-term solution. However, if the engineering solution to fix this turns out to be very challenging and complicate many other Stepper tracking logic implementations, we will of course have to use extra filtering as the long-term solution - which will require the analytics team to assess various situations and come up with the filtering logic.

Does this make sense? Let me know what you think.

chriskmnds commented 1 week ago

Thanks @skim1220 for replying.

I think we need to approach things a little differently - and I feel some of the messaging may not have been clear through https://github.com/Automattic/wp-calypso/issues/94175

I am not arguing that this instance of calypso_signup_start firing in the middle of a flow isn't due to a bug. However, what I am arguing for is that it shouldn't surprise us.

My point was that the issue needs to be resolved because it is not aligned with our primary usage of calypso_signup_start event in the analytics.

The way we are encoding things right now, as I explained above and in the various discussions, is that any event may fire at any interval throughout a flow. This was the decision and the flexibility we are giving flow implementations. No exceptions to this rule. So you may find calypso_signup_start event being recorded over and over as long as the flow decided to wrap it with any form of "changing" or "async" data to record. It should not surprise you, and I think the only to run with this is to embrace it. If we keep flagging exceptions, then we'll never rest. I explain more below.

What matters to you, from a "typical" analytics POV, is whether the first instance of that event matches your expectations i.e. whether it's as accurate and as semantically meaningful as you would expect it. This is the guarantee the code gives while remaining flexible to keep things SIMPLE for the API we are building for unconditionally (double empasis) extending the event props.

This will be the "new way" of handling events in Stepper. You will need to embody whatever filtering is necessary, update P2s and FG pages, make it part of the "new standard". There's just no other way around this to avoid ambiguities.

For posterity, I didn't come up with any of the above - I merely took a suggestion and ran with it (it started with @MicBosi 's suggestion here, which I carried on discussing in that issue, also here, and a little bit here). Allowing events to be recorded multiple times is a workaround to investing in a more ambiguous and complex system to allow flows to extend props unconditionally (see https://github.com/Automattic/wp-calypso/issues/94108 ). IMO we either accept and embrace or leave it.

MicBosi commented 1 week ago

I'll expand on my position regarding my commend here to make it 100% crystal clear:

Example cases where given the complexities is OK to see multiple events fire and we'll handle at data analytics time:

Gray cases:

In cases such as the ones above I don't think it's worth/simple/safe for Calypso to try and synthesise in real time a "cleaned" up version of the users's journey through Tracks events, removing "duplicate" step start events, providing an overall final version of what the user did etc. Going down this path is complex and just asking for trouble I think.

Example cases that are Not Ok:

In my mind these are some useful north starts to keep in mind:


@chriskmnds my understanding is that this issue falls under the "Tracks events firing out of order" category, which I don't see why it should be surprising that @skim1220 is reporting it, unless you think Tracks events order can be random and disjoint from the order in which users experiences things?

chriskmnds commented 1 week ago

@MicBosi I will reply tomorrow - but want to just emphasise something concerning:

@chriskmnds my understanding is that this issue falls under the "Tracks events firing out of order" category, which I don't see why it should be surprising that @skim1220 is reporting it, unless you think Tracks events order can be random and disjoint from the order in which users experiences things?

I mentioned above:

What matters to you, from a "typical" analytics POV, is whether the first instance of that event matches your expectations i.e. whether it's as accurate and as semantically meaningful as you would expect it. This is the guarantee the code gives while remaining flexible to keep things SIMPLE for the API we are building for unconditionally (double empasis) extending the event props.

So the guarantee is that events will fire in the correct order, irrespective if they fire multiple times (for whatever reason e.g. some async property changes value). I'm trying to keep things simple i.e. we accept a (any) scenario that a given event can fire multiple times and have a guarantee that the first time it fires is the correct from an "order" point of view e.g. you won't see the first instance of calypso_signup_start get recorded after a calypso_signup_step_submit. This is the simplicity that I'm referring to. Allowing the scenario of multiple records only on exclusive cases is a can of worms. I've seen it over and over - some dependency is introduced, not ignored, some other ignored, etc. At the basis, I think it should be a pretty boolean/discrete semantic.

MicBosi commented 1 week ago

@chriskmnds Thanks for clarifying your position. I think I get it now. Unfortunately I don't think that is a blank check that we can sign for Stepper, accepting that the same event can be fired both at its expected place and also at any random place after that.

Even for things as simple as calypso_page_views this would be a pretty outrageous expectation to have.

If the user hasn't started a new signup why in the world would it be acceptable to fire it again? Same goes for any other event, especially key ones during onboarding.

If Classic could do it, I don't think is too much to ask for Stepper to just not spam us with spurious Tracks events and then assume we'll have to cleanup the mess at analytics time, with no recourse, regardless of the cause of the duplicate events.

We are willing to work with some reasonable degree of noise, especially if that noise is a reflection of the true user behaviour, definitely not if it's a regression from Classic due to some implementation detail - that, if anything, is hinting at fundamental flaws in the design.

I think is also worth mentioning that we don't care about all events in the same way. Maybe for some events it does makes sense or is more practical doing what you are proposing but things like calypso_signup_start/complete,calypso_signup_step_start/calypso_signup_actions_submit_step/calypso_signup_actions_complete_step we'd expect them to not regress from Classic, especially for the key steps, like domain, plan, user.

escapemanuele commented 1 week ago

Opened a few issues to keep track/work on the problems:

chriskmnds commented 1 week ago

Thanks for clarifying @MicBosi

@chriskmnds Thanks for clarifying your position. I think I get it now. Unfortunately I don't think that is a blank check that we can sign for Stepper, accepting that the same event can be fired both at its expected place and also at any random place after that.

Even for things as simple as calypso_page_views this would be a pretty outrageous expectation to have.

If the user hasn't started a new signup why in the world would it be acceptable to fire it again? Same goes for any other event, especially key ones during onboarding.

If Classic could do it, I don't think is too much to ask for Stepper to just not spam us with spurious Tracks events and then assume we'll have to cleanup the mess at analytics time, with no recourse, regardless of the cause of the duplicate events.

We are willing to work with some reasonable degree of noise, especially if that noise is a reflection of the true user behaviour, definitely not if it's a regression from Classic due to some implementation detail - that, if anything, is hinting at fundamental flaws in the design.

Ok. Let's just put that to rest then. I'm glad to be out of this discussion with whatever resolution. What suddens me is this spurred from your proposal, I've expressed how weird calypso_signup_start getting recorded multiple times may look, I've pinged over and over in discussions. I feel like we've done a bit of a circle, at least what looks to me 🤯

I think is also worth mentioning that we don't care about all events in the same way. Maybe for some events it does makes sense or is more practical doing what you are proposing but things like calypso_signup_start/complete,calypso_signup_step_start/calypso_signup_actions_submit_step/calypso_signup_actions_complete_step we'd expect them to not regress from Classic, especially for the key steps, like domain, plan, user.

All the same to me :-)

MicBosi commented 1 week ago

What suddens me is this spurred from your proposal

Thanks @chriskmnds, I am flattered by the credit you give me based on that 1 comment 😄 where I'm asking your (plural) expert opinion on the pros and cons of chasing every rabbit hole and corner case to avoid "duplicate" events.

I was trying to make your life a bit easier so you don't feel pressured to implement and maintain some complex new system that Classic does not have, even before we are on par with Classic. As a corollary, I also wanted to make it clear that this requirement is not coming from us in Data.

However, Classic still remains the baseline. Let's not do worse than Classic™.

Saying you don't need to fix today all the corner case flaws that Classic had, does not imply we can now introduce a host of new ones, or we can run for the hills with a radically different approach that is not compatible with the existing infrastructure.

I'm not even sure how relevant or theoretical this discussion is, since, also thanks to all your great work so far, it feels like we are pretty close to match Classic and the issues Seah reported are relatively minor at this point.

This is why is important to have these discussions, to find alignment, test in real life conditions, and figure out a reasonable path forward.

I hope tomorrow we can approach this with fresh minds and I also want to reiterate how much we appreciate your help in figuring these things out and in getting Stepper over the finish line so we can all celebrate!

agrullon95 commented 3 days ago

Thanks for the test results @skim1220.

2 [Stepper Tracking] Upon clicking “Use my own domain” option at the Domain Selection step in the Stepper flow, calypso_signup_start event fires but it shouldn’t. See [this screenshot](). The detailed event log is available in the 1BPDrnHB2lA3TBfVOrDIp-arsOCCUZ6LAWXVoT1xCy-s-gdoc results. This does not happen in the Classic flow.

I don't see the calypso_signup_start event triggered when I click the "Use my own domain" option on the Domain selection step. Can you test this again when you get a chance?

skim1220 commented 2 days ago

👋 @agrullon95 , thanks for taking a look at this! I tested it on https://wordpress.com/setup/onboarding/es/?ref=seahtest, and it still happens to me. Please see the recording:

https://github.com/user-attachments/assets/33eca525-1426-49f6-85a5-2326a5620a8f

calypso_signup_start fires correctly once when the flow is initiated. However, when I select the "Use your domain" option at the Domain Selection step, calypso_signup_start event fires again.

escapemanuele commented 2 days ago

@skim1220 just did some testing and... It's because of the ref parameter! We'll take a look