Implement the total elapsed time tracking between `calypso_signup_start` and `calypso_signup_complete` #83944

southp · 2023-11-07T05:41:37Z

Proposed Changes

This PR implements the request of Automattic/martech#2383, adding the tracking to the total elapsed time for completing a sign-up flow. The time is computed as between calypso_signup_start and calypso_signup_complete. It's implemented using the session storage along side other signup storage utilities for the following reasons:

Tracking elapsed time per session would make the most sense, since it should cover majority of the cases and resuming a flow after closing a tab is very fragile anyway. If we want to cover the case outside of a session, we should fix the case of resuming a flow between sessions first.
The newly added storage utilities are called inside the lib/analytics/signup module directly. While this dependency relationship is questionable, it's the easiest way to get this functionality working across the classic signup framework and the Stepper framework that I can think of for now.

Testing Instructions

Open the network inspector, turn on "Preserve log", and filter for "elapsed_time". That will filter out calypso_signup_complete event with the new prop elapsed_time.
Completing a new signup from /start, calypso_signup_complete should contain elaped_time field indicating how many seconds has been taken.
Completing /start as a logged-in user, the same
Pick a few other classic signup flows to make sure it applies, e.g. /start/do-it-for-me, /start/domain
Pick a few Stepper flows to make sure it applies, e.g. /setup/newsletter

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

and `calypso_signup_complete`

in the analysis

github-actions · 2023-11-07T05:41:50Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-87420

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-87420&env=jetpack

matticbot · 2023-11-07T05:42:20Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~154 bytes added 📈 [gzipped])

name           parsed_size           gzip_size
entry-stepper       +498 B  (+0.0%)     +143 B  (+0.0%)
entry-main           +56 B  (+0.0%)      +11 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~6213 bytes removed 📉 [gzipped])

name                  parsed_size            gzip_size
accept-invite             +1960 B   (+0.5%)     +638 B   (+0.7%)
lib-analytics-signup      +1867 B  (+19.0%)     +528 B  (+13.8%)
jetpack-connect           +1867 B   (+0.2%)     +509 B   (+0.2%)
signup                     +483 B   (+0.2%)     +506 B   (+0.7%)
home                       +442 B   (+0.0%)     +125 B   (+0.0%)
sites-dashboard            +295 B   (+0.1%)      +73 B   (+0.0%)
settings                   +295 B   (+0.0%)      +61 B   (+0.0%)
checkout                   +295 B   (+0.0%)      +51 B   (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~251 bytes added 📈 [gzipped])

name                                              parsed_size           gzip_size
async-load-calypso-my-sites-checkout-modal            -1425 B  (-0.1%)     -367 B  (-0.1%)
async-load-design                                      +295 B  (+0.0%)      +72 B  (+0.0%)
async-load-calypso-layout-masterbar-checkout-tsx       +295 B  (+0.3%)      +53 B  (+0.1%)
async-load-calypso-layout-masterbar-checkout           +295 B  (+0.4%)      +59 B  (+0.2%)
async-load-calypso-blocks-editor-checkout-modal        +295 B  (+0.0%)      +67 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot · 2023-11-07T06:00:21Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

blaze-dashboard
editing-toolkit
odyssey-stats

To test WordPress.com changes, run install-plugin.sh $pluginSlug add/signup-funnel-completion-time on your sandbox.

southp · 2023-11-07T06:04:26Z

client/lib/analytics/signup.js

@@ -81,6 +93,7 @@ export function recordSignupComplete(
 	// blog_id instead of site_id here. We keep using "siteId" otherwise since
 	// all the other fields still refer with "site". e.g. isNewSite
 	recordTracksEvent( 'calypso_signup_complete', {
+		elapsed_time: elapsedTime ?? getSignupCompleteElapsedTimeInSeconds(),


The reason that getSignupCompleteElapsedTimeInSeconds() are conditionally called in two places is because recordSignupComplete() has an internal queue. If now flag is absent, what it does is to queue the function and then let the internal task consumer make the real call. Thus, the elapsedTime ?? ... above is for queuing the real elapsed time to the queue. That way when this function is called by the task consumer, elapsedTime here will be the correct elapsed time being queued previously. In the meantime, if now flag is enabled, this line will also guarantee that getSignupCompleteElapsedTimeInSeconds() is called since elapsedTime should be null in that case.

Can't we just call getSignupCompleteElapsedTimeInSeconds() on line 64 and use that value. The closure will preserve the correct value.

We do not want to add the Queuing time to the actual metric do we? since it's not relevant to the elapsed time of the UX ?

Also do you think we can rename the prop to time_since_signup_start I have a feeling that we might need to add this time stamp across multiple events to make sense of the data and the above event name is a bit more descriptive.

Actually I would prefer if we add this event prop to all other major signup events like calypso_signup_actions_submit_step, calypso_new_user_site_creation, calypso_user_registration_complete from the get go. But this can be followed up in another PR if you prefer 👍.

I've updated the prop name as a slightly different elapsed_time_since_start, since I somehow like to keep elapsed and it's already under a signup event so I only include start. Does that make sense to you?

Also:

Actually I would prefer if we add this event prop to all other major signup events like calypso_signup_actions_submit_step, calypso_new_user_site_creation, calypso_user_registration_complete from the get go. But this can be followed up in another PR if you prefer

Yeah, let's do it as a follow-up when the need arise :)

Can't we just call getSignupCompleteElapsedTimeInSeconds() on line 64 and use that value. The closure will preserve the correct value.
We do not want to add the Queuing time to the actual metric do we? since it's not relevant to the elapsed time of the UX ?

Good question and that's the tricky part. First of all, no, we don't want to add the queuing time to the actual metric. That's why getSignupCompleteElapsedTime() has to be called before the values go into the queue.

The way that "trigger queue" works is fairly simple(see addToQueue() and runTrigger() ). It pushes the function name and the arguments into the queue through addToQueue(), and then invoke them accordingly by runTrigger(). Thus, there

Thus, let's say we put getSignupCompletionTime() call on Line 64, it'd actually be called twice:

The 1st time: when recordSignupComplete() is called with now parameter as null

The 2nd time: when recordSignupComplete() is called again by runTrigger()

It's fine, but I don't want it to be called twice like this. Does that make sense? :)

ddc22

@southp have you thought about introducing the elapsed time in milliseconds? Doing math in FE/JS in generally a bit iffy. And we can do calculation with far higher precision in our query tools?

Left a few more comment below related to the implementation.

ddc22 · 2023-11-07T22:44:10Z

client/signup/storageUtils.js

@@ -83,3 +83,25 @@ export const getSignupCompleteStepNameAndClear = () => {
 	clearSignupCompleteStepName();
 	return value;
 };
+export const setSignupStartTimeInSeconds = () =>
+	ignoreFatalsForSessionStorage( () =>
+		sessionStorage?.setItem( 'wpcom_signup_start_time_in_second', Math.floor( Date.now() / 1000 ) )


I think we can use performance.now() which is more precise.

Good idea. I've updated it into performance.now() instead. Other than the precision, it's immune to any potential system clock changes, so it's the better choice for measuring elapsed time by design.

ddc22 · 2023-11-07T22:51:23Z

client/lib/analytics/signup.js

@@ -81,6 +93,7 @@ export function recordSignupComplete(
 	// blog_id instead of site_id here. We keep using "siteId" otherwise since
 	// all the other fields still refer with "site". e.g. isNewSite
 	recordTracksEvent( 'calypso_signup_complete', {
+		elapsed_time: elapsedTime ?? getSignupCompleteElapsedTimeInSeconds(),


Can't we just call getSignupCompleteElapsedTimeInSeconds() on line 64 and use that value. The closure will preserve the correct value.

We do not want to add the Queuing time to the actual metric do we? since it's not relevant to the elapsed time of the UX ?

ddc22 · 2023-11-07T22:54:52Z

client/lib/analytics/signup.js

@@ -81,6 +93,7 @@ export function recordSignupComplete(
 	// blog_id instead of site_id here. We keep using "siteId" otherwise since
 	// all the other fields still refer with "site". e.g. isNewSite
 	recordTracksEvent( 'calypso_signup_complete', {
+		elapsed_time: elapsedTime ?? getSignupCompleteElapsedTimeInSeconds(),


Also do you think we can rename the prop to time_since_signup_start I have a feeling that we might need to add this time stamp across multiple events to make sense of the data and the above event name is a bit more descriptive.

Actually I would prefer if we add this event prop to all other major signup events like calypso_signup_actions_submit_step, calypso_new_user_site_creation, calypso_user_registration_complete from the get go. But this can be followed up in another PR if you prefer 👍.

ddc22 · 2023-11-07T22:57:29Z

client/lib/analytics/signup.js

@@ -57,6 +68,7 @@ export function recordSignupComplete(
 			'signup',
 			'recordSignupComplete',
 			{
+				elapsedTime: elapsedTime ?? getSignupCompleteElapsedTimeInSeconds(),


Nit. Ignorable. Maybe we can do getSignupCompletionDuration and add a comment regarding the metric and more context :)

I'm not sure I fully understand your meaning. Do you mean getSignupCompleteElapsedTime() and a comment regarding what this prop means?

southp · 2023-11-10T08:48:26Z

@southp have you thought about introducing the elapsed time in milliseconds? Doing math in FE/JS in generally a bit iffy. And we can do calculation with far higher precision in our query tools?

Good suggestion, @jdc91 . I did that simply because the original request was in seconds, but it's simpler and even better to record the elapsed time in milliseconds. I've updated it accordingly, along with other inline comments. This PR should be ready for your revisit now :)

southp · 2023-11-21T09:43:16Z

I'm going ahead and deploying it. Any follow-up suggestions is still welcome :)

…rt` and `calypso_signup_complete` (#83944) * Implement the total elapsed time tracking between `calypso_signup_start` and `calypso_signup_complete` * Remove start_time from `calypso_signup_start` since it's likely not used in the analysis * Take the implicit queuing of `recordSignupCompleteEvent` into account. * `elapsedTime` should be mark as optional * Use the performance clock to compute the elapsed time instead

southp added 2 commits November 7, 2023 13:23

Implement the total elapsed time tracking between calypso_signup_start

925ff14

and `calypso_signup_complete`

Remove start_time from calypso_signup_start since it's likely not used

5cf1498

in the analysis

Take the implicit queuing of recordSignupCompleteEvent into account.

f68dda9

southp self-assigned this Nov 7, 2023

southp added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 7, 2023

southp commented Nov 7, 2023

View reviewed changes

elapsedTime should be mark as optional

d6d3292

southp marked this pull request as ready for review November 7, 2023 06:22

southp requested review from dzver, a team and chriskmnds November 7, 2023 06:22

ddc22 reviewed Nov 7, 2023

View reviewed changes

Use the performance clock to compute the elapsed time instead

2007976

southp merged commit d096763 into trunk Nov 21, 2023
12 checks passed

southp deleted the add/signup-funnel-completion-time branch November 21, 2023 09:43

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the total elapsed time tracking between `calypso_signup_start` and `calypso_signup_complete` #83944

Implement the total elapsed time tracking between `calypso_signup_start` and `calypso_signup_complete` #83944

southp commented Nov 7, 2023 •

edited

Loading

github-actions bot commented Nov 7, 2023 •

edited by matticbot

Loading

matticbot commented Nov 7, 2023 •

edited

Loading

matticbot commented Nov 7, 2023

southp Nov 7, 2023

ddc22 Nov 7, 2023

ddc22 Nov 7, 2023 •

edited

Loading

southp Nov 10, 2023

southp Nov 10, 2023

ddc22 left a comment

ddc22 Nov 7, 2023

southp Nov 10, 2023

ddc22 Nov 7, 2023

ddc22 Nov 7, 2023 •

edited

Loading

ddc22 Nov 7, 2023

southp Nov 10, 2023

southp commented Nov 10, 2023

southp commented Nov 21, 2023

Implement the total elapsed time tracking between calypso_signup_start and calypso_signup_complete #83944

Implement the total elapsed time tracking between calypso_signup_start and calypso_signup_complete #83944

Conversation

southp commented Nov 7, 2023 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Nov 7, 2023 • edited by matticbot Loading

matticbot commented Nov 7, 2023 • edited Loading

matticbot commented Nov 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddc22 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddc22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddc22 Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

southp commented Nov 10, 2023

southp commented Nov 21, 2023

Implement the total elapsed time tracking between `calypso_signup_start` and `calypso_signup_complete` #83944

Implement the total elapsed time tracking between `calypso_signup_start` and `calypso_signup_complete` #83944

southp commented Nov 7, 2023 •

edited

Loading

github-actions bot commented Nov 7, 2023 •

edited by matticbot

Loading

matticbot commented Nov 7, 2023 •

edited

Loading

ddc22 Nov 7, 2023 •

edited

Loading

ddc22 Nov 7, 2023 •

edited

Loading