Add new Timing block to app loading track message (#3796)

* + Add new Timing block to app loading track message. + New state for 'Authenticating' + Misc cleanups * Changelog tweaks * Show mask during new AUTHENTICATING appState * + Comments from ATM code review --------- Co-authored-by: Anselm McClain <atm@xh.io>
xh · Oct 3, 2024 · 9a94f56 · 9a94f56
1 parent eb3fcc3
commit 9a94f56
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,17 @@
 
 ## 69.0.0-SNAPSHOT - unreleased
 
+
+### 💥 Breaking Changes (upgrade difficulty: 🟢 LOW )
+* The `INITIALIZING` AppState has been replaced with more fine-grained states (see below).  This
+is not expected to effect any applications.
+
+### 🎁 New Features
+
+* Added new AppStates `AUTHENTICATING`, `INITIALIZING_HOIST`, and `INITIALIZING_APP` to support
+more granular tracking and timing of app startup lifecycle.
+* Improved the default "Loaded App" activity tracking entry with more granular data on load timing.
+
 ## 68.1.0 - 2024-09-27
 
 ### 🎁 New Features

diff --git a/appcontainer/AppContainerModel.ts b/appcontainer/AppContainerModel.ts
@@ -99,7 +99,7 @@ export class AppContainerModel extends HoistModel {
     @managed userAgentModel = new UserAgentModel();
 
     /**
-     * Message shown on spinner while the application is in the INITIALIZING state.
+     * Message shown on spinner while the application is in a pre-running state.
      * Update within `AppModel.initAsync()` to relay app-specific initialization status.
      */
     @bindable initializingLoadMaskMessage: ReactNode;
@@ -108,7 +108,7 @@ export class AppContainerModel extends HoistModel {
      * Main entry point. Initialize and render application code.
      */
     renderApp<T extends HoistAppModel>(appSpec: AppSpec<T>) {
-        // Remove the pre-load exception handler installed by preflight.js
+        // Remove the preload exception handler installed by preflight.js
         window.onerror = null;
         const spinner = document.getElementById('xh-preload-spinner');
         if (spinner) spinner.style.display = 'none';
@@ -180,6 +180,7 @@ export class AppContainerModel extends HoistModel {
             await installServicesAsync([FetchService]);
 
             // Check auth, locking out, or showing login if possible
+            this.setAppState('AUTHENTICATING');
             XH.authModel = new this.appSpec.authModelClass();
             const isAuthenticated = await XH.authModel.completeAuthAsync();
             if (!isAuthenticated) {
@@ -206,6 +207,7 @@ export class AppContainerModel extends HoistModel {
      */
     @action
     async completeInitAsync() {
+        this.setAppState('INITIALIZING_HOIST');
         try {
             // Install identity service and confirm access
             await installServicesAsync(IdentityService);
@@ -215,7 +217,6 @@ export class AppContainerModel extends HoistModel {
             }
 
             // Complete initialization process
-            this.setAppState('INITIALIZING');
             await installServicesAsync([ConfigService, LocalStorageService]);
             await installServicesAsync(TrackService);
             await installServicesAsync([EnvironmentService, PrefService, JsonBlobService]);
@@ -272,6 +273,7 @@ export class AppContainerModel extends HoistModel {
             // Delay to workaround hot-reload styling issues in dev.
             await wait(XH.isDevelopmentMode ? 300 : 1);
 
+            this.setAppState('INITIALIZING_APP');
             const modelClass: any = this.appSpec.modelClass;
             this.appModel = modelClass.instance = new modelClass();
             await this.appModel.initAsync();

diff --git a/appcontainer/AppStateModel.ts b/appcontainer/AppStateModel.ts
@@ -5,10 +5,10 @@
  * Copyright © 2024 Extremely Heavy Industries Inc.
  */
 import {AppState, AppSuspendData, HoistModel, XH} from '@xh/hoist/core';
-import {action, makeObservable, observable, reaction} from '@xh/hoist/mobx';
+import {action, makeObservable, observable} from '@xh/hoist/mobx';
 import {Timer} from '@xh/hoist/utils/async';
 import {getClientDeviceInfo} from '@xh/hoist/utils/js';
-import {isBoolean, isString} from 'lodash';
+import {camelCase, isBoolean, isString, mapKeys} from 'lodash';
 
 /**
  * Support for Core Hoist Application state and loading.
@@ -24,6 +24,10 @@ export class AppStateModel extends HoistModel {
     suspendData: AppSuspendData;
     accessDeniedMessage: string = 'Access Denied';
 
+    private timings: Record<AppState, number> = {} as Record<AppState, number>;
+    private loadStarted: number = window['_xhLoadTimestamp']; // set in index.html
+    private lastStateChangeTime: number = this.loadStarted;
+
     constructor() {
         super();
         makeObservable(this);
@@ -33,10 +37,14 @@ export class AppStateModel extends HoistModel {
 
     @action
     setAppState(nextState: AppState) {
-        if (this.state !== nextState) {
-            this.logDebug(`AppState change`, `${this.state} → ${nextState}`);
-        }
+        if (this.state === nextState) return;
+
+        const {state, timings, lastStateChangeTime} = this,
+            now = Date.now();
+        timings[state] = (timings[state] ?? 0) + now - lastStateChangeTime;
+        this.lastStateChangeTime = now;
         this.state = nextState;
+        this.logDebug(`AppState change`, `${state} → ${nextState}`);
     }
 
     suspendApp(suspendData: AppSuspendData) {
@@ -70,39 +78,25 @@ export class AppStateModel extends HoistModel {
     // Implementation
     //------------------
     private trackLoad() {
-        let loadStarted = window['_xhLoadTimestamp'], // set in index.html
-            loginStarted = null,
-            loginElapsed = 0;
-
-        const disposer = reaction(
-            () => this.state,
-            state => {
-                const now = Date.now();
-                switch (state) {
-                    case 'RUNNING':
-                        XH.track({
-                            category: 'App',
-                            message: `Loaded ${XH.clientAppCode}`,
-                            elapsed: now - loadStarted - loginElapsed,
-                            data: {
-                                appVersion: XH.appVersion,
-                                appBuild: XH.appBuild,
-                                locationHref: window.location.href,
-                                ...getClientDeviceInfo()
-                            },
-                            logData: ['appVersion', 'appBuild'],
-                            omit: !XH.appSpec.trackAppLoad
-                        });
-                        disposer();
-                        break;
-                    case 'LOGIN_REQUIRED':
-                        loginStarted = now;
-                        break;
-                    default:
-                        if (loginStarted) loginElapsed = now - loginStarted;
-                }
-            }
-        );
+        const {timings, loadStarted} = this;
+        this.addReaction({
+            when: () => this.state === 'RUNNING',
+            run: () =>
+                XH.track({
+                    category: 'App',
+                    message: `Loaded ${XH.clientAppCode}`,
+                    elapsed: Date.now() - loadStarted - (timings.LOGIN_REQUIRED ?? 0),
+                    data: {
+                        appVersion: XH.appVersion,
+                        appBuild: XH.appBuild,
+                        locationHref: window.location.href,
+                        timings: mapKeys(timings, (v, k) => camelCase(k)),
+                        ...getClientDeviceInfo()
+                    },
+                    logData: ['appVersion', 'appBuild'],
+                    omit: !XH.appSpec.trackAppLoad
+                })
+        });
     }
 
     //---------------------

diff --git a/core/types/AppState.ts b/core/types/AppState.ts
@@ -9,13 +9,18 @@
  * Enumeration of possible App States
  */
 export const AppState = Object.freeze({
+    // Main Flow
     PRE_AUTH: 'PRE_AUTH',
+    AUTHENTICATING: 'AUTHENTICATING',
     LOGIN_REQUIRED: 'LOGIN_REQUIRED',
-    ACCESS_DENIED: 'ACCESS_DENIED',
-    INITIALIZING: 'INITIALIZING',
+    INITIALIZING_HOIST: 'INITIALIZING_HOIST',
+    INITIALIZING_APP: 'INITIALIZING_APP',
     RUNNING: 'RUNNING',
     SUSPENDED: 'SUSPENDED',
-    LOAD_FAILED: 'LOAD_FAILED'
+
+    // Terminal Error States.
+    LOAD_FAILED: 'LOAD_FAILED',
+    ACCESS_DENIED: 'ACCESS_DENIED'
 });
 
 // eslint-disable-next-line

diff --git a/desktop/appcontainer/AppContainer.ts b/desktop/appcontainer/AppContainer.ts
@@ -104,7 +104,9 @@ export const AppContainer = hoistCmp({
 function viewForState({model}) {
     switch (XH.appState) {
         case 'PRE_AUTH':
-        case 'INITIALIZING':
+        case 'AUTHENTICATING':
+        case 'INITIALIZING_HOIST':
+        case 'INITIALIZING_APP':
             return viewport(
                 mask({spinner: true, isDisplayed: true, message: model.initializingLoadMaskMessage})
             );

diff --git a/mobile/appcontainer/AppContainer.ts b/mobile/appcontainer/AppContainer.ts
@@ -89,7 +89,9 @@ export const AppContainer = hoistCmp({
 function viewForState({model}) {
     switch (XH.appState) {
         case 'PRE_AUTH':
-        case 'INITIALIZING':
+        case 'AUTHENTICATING':
+        case 'INITIALIZING_HOIST':
+        case 'INITIALIZING_APP':
             return viewport(
                 mask({spinner: true, isDisplayed: true, message: model.initializingLoadMaskMessage})
             );