fix!: disable VirtualPageLoadTimer by default

docs/configuration.md

@@ -47,6 +47,7 @@ For example, the config object may look similar to the following:
 | eventCacheSize | Number | `1000` | The maximum number of events the cache can contain before dropping events. Each event is typically 1KB in size, so we recommend a default limit of 1000 events -> 1 MB to balance performance against capturing all observed events. If necessary, feel free to enable debug mode to get detailed logs on how to optimize cache size depending on your application behavior. |
 | sessionLengthSeconds | Number | `1800` | The duration of a session (in seconds). |
 | headers | Object | `{}` | The **headers** configuration is optional and allows you to include custom headers in an HTTP request. For example, you can use it to pass `Authorization` and `x-api-key` headers.<br/><br/>For more details, see: [MDN - Request Headers](https://developer.mozilla.org/en-US/docs/Glossary/Request_header). |
+| legacySPASupport | Boolean | `false` | When this field is `true`, then legacy NavigationEvents with initiatorType=route_change will be recorded for single page applications. **Warning:** This is a legacy feature that is no longer supported. Please enable with caution in debugging mode first to see if it is relevant for your web application before enabling it in prod. See [#723](https://github.com/aws-observability/aws-rum-web/issues/723) for more details. |
 
 ## CookieAttributes
 

src/orchestration/Orchestration.ts

@@ -98,6 +98,7 @@ export const defaultConfig = (cookieAttributes: CookieAttributes): Config => {
         useBeacon: true,
         userIdRetentionDays: 30,
         enableW3CTraceId: false,
+        legacySPASupport: false, // deprecated
         ...internalConfigOverrides
     };
 };
@@ -174,6 +175,9 @@ export interface Config {
     alias?: string;
     headers?: HeaderBag;
     enableW3CTraceId: boolean;
+
+    // Deprecated features; enable with caution
+    legacySPASupport: boolean; // See https://github.com/aws-observability/aws-rum-web/issues/723
 }
 
 export interface PartialConfig

src/orchestration/__tests__/Orchestration.test.ts

@@ -572,5 +572,6 @@ describe('defaultConfig tests', () => {
         expect(config.userIdRetentionDays).toBe(30);
         expect(config.enableW3CTraceId).toBe(false);
         expect(config.candidatesCacheSize).toBe(10);
+        expect(config.legacySPASupport).toBe(false);
     });
 });

src/sessions/PageManager.ts

@@ -50,7 +50,7 @@ export class PageManager {
     private page: Page | undefined;
     private resumed: boolean;
     private attributes: Attributes | undefined;
-    private virtualPageLoadTimer: VirtualPageLoadTimer;
+    private virtualPageLoadTimer: VirtualPageLoadTimer | undefined;
     private TIMEOUT = 1000;
     private timeOnParentPage: number | undefined;
 
@@ -68,11 +68,18 @@ export class PageManager {
         this.page = undefined;
         this.resumed = false;
         this.recordInteraction = false;
-        this.virtualPageLoadTimer = new VirtualPageLoadTimer(
-            this,
-            config,
-            record
-        );
+        if (config.legacySPASupport) {
+            if (config.debug) {
+                InternalLogger.warn(
+                    'VirtualPageLoadTiming (deprecated) is enabled and may result in innaccurate page load timing for Single Page Applications. Please use with caution after reviewing https://github.com/aws-observability/aws-rum-web/issues/723'
+                );
+            }
+            this.virtualPageLoadTimer = new VirtualPageLoadTimer(
+                this,
+                config,
+                record
+            );
+        }
     }
 
     public getPage(): Page | undefined {
@@ -99,28 +106,38 @@ export class PageManager {
             pageId = payload.pageId;
         }
 
+        if (this.config.debug) {
+            InternalLogger.info(`recordPageView called with pageId: ${pageId}`);
+        }
+
         if (this.useCookies()) {
             this.recordInteraction = true;
         }
 
         if (!this.page) {
             this.createLandingPage(pageId);
             if (this.config.debug) {
-                InternalLogger.info(`Landing page recorded: ${pageId}`);
+                InternalLogger.info(`Landing page created: ${pageId}`);
             }
         } else if (this.page.pageId !== pageId) {
             this.createNextPage(this.page, pageId);
             if (this.config.debug) {
-                InternalLogger.info(`Navigation to page: ${pageId}`);
+                InternalLogger.info(`Navigation to new page: ${pageId}`);
             }
         } else if (this.resumed) {
+            if (this.config.debug) {
+                InternalLogger.info(`Resumed session for page: ${pageId}`);
+            }
             // Update attributes state in PageManager for event metadata
             this.collectAttributes(
                 this.page as Page,
                 typeof payload === 'object' ? payload : undefined
             );
             return;
         } else {
+            if (this.config.debug) {
+                InternalLogger.info(`No page change detected for: ${pageId}`);
+            }
             // The view has not changed.
             return;
         }
@@ -139,7 +156,14 @@ export class PageManager {
 
     private createNextPage(currentPage: Page, pageId: string) {
         let startTime = Date.now();
-        const interactionTime = this.virtualPageLoadTimer.latestInteractionTime;
+
+        if (this.config.debug) {
+            InternalLogger.info(
+                `Creating next page ${pageId}, interaction: ${
+                    currentPage.interaction + 1
+                }`
+            );
+        }
 
         // The latest interaction time (latest) is not guaranteed to be the
         // interaction that triggered the route change (actual). There are two
@@ -160,9 +184,24 @@ export class PageManager {
         //
         // We do not believe that case (2) has a high risk of skewing route
         // change timing, and therefore ignore case (2).
-        if (!this.resumed && startTime - interactionTime <= this.TIMEOUT) {
-            startTime = interactionTime;
-            this.virtualPageLoadTimer.startTiming();
+        if (this.config.legacySPASupport && this.virtualPageLoadTimer) {
+            const interactionTime =
+                this.virtualPageLoadTimer.latestInteractionTime;
+            if (!this.resumed && startTime - interactionTime <= this.TIMEOUT) {
+                startTime = interactionTime;
+                this.virtualPageLoadTimer.startTiming();
+                if (this.config.debug) {
+                    InternalLogger.info(
+                        `Started virtual page load timing for ${pageId}`
+                    );
+                }
+            } else if (this.config.debug) {
+                InternalLogger.info(
+                    `Skipped virtual page load timing for ${pageId} (resumed: ${
+                        this.resumed
+                    }, timeDiff: ${startTime - interactionTime}ms)`
+                );
+            }
         }
 
         this.timeOnParentPage = startTime - currentPage.start;

src/sessions/VirtualPageLoadTimer.ts

@@ -4,6 +4,7 @@ import { MonkeyPatched } from '../plugins/MonkeyPatched';
 import { Config } from '../orchestration/Orchestration';
 import { RecordEvent } from '../plugins/types';
 import { PageManager, Page } from './PageManager';
+import { InternalLogger } from '../utils/InternalLogger';
 
 type Fetch = typeof fetch;
 type Send = () => void;
@@ -13,6 +14,10 @@ type Patching = Pick<XMLHttpRequest & Window, 'fetch' | 'send'>;
  * (1) Holds all virtual page load timing related resources
  * (2) Intercepts outgoing XMLHttpRequests and Fetch requests and listens for DOM changes
  * (3) Records virtual page load
+ *
+ * @deprecated This class is deprecated and will be removed in a future version.
+ * For now, it can still be enabled via the legacySPASupport configuration. If you would like to opt-in
+ * to this legacy feature, then please first carefully review https://github.com/aws-observability/aws-rum-web/issues/723
  */
 export class VirtualPageLoadTimer extends MonkeyPatched<
     Patching,
@@ -84,6 +89,11 @@ export class VirtualPageLoadTimer extends MonkeyPatched<
     /** Initializes timing related resources for current page. */
     public startTiming() {
         this.latestEndTime = Date.now();
+
+        if (this.config.debug) {
+            InternalLogger.info('Starting timing for virtual page load');
+        }
+
         // Clean up existing timer objects and mutationObserver
         if (this.periodicCheckerId) {
             clearInterval(this.periodicCheckerId);
@@ -114,6 +124,12 @@ export class VirtualPageLoadTimer extends MonkeyPatched<
         this.isPageLoaded = false;
         this.requestBuffer.forEach(this.moveItemsFromBuffer);
         this.requestBuffer.clear();
+
+        if (this.config.debug) {
+            InternalLogger.info(
+                `Moved ${this.requestBuffer.size} requests from buffer to ongoing`
+            );
+        }
     }
 
     private sendWrapper = (): ((original: Send) => Send) => {
@@ -132,8 +148,18 @@ export class VirtualPageLoadTimer extends MonkeyPatched<
         const page = this.pageManager.getPage();
         if (page && this.isPageLoaded === false) {
             this.ongoingRequests.add(item);
+            if (this.config.debug) {
+                InternalLogger.info(
+                    `Added XHR to ongoing requests (total: ${this.ongoingRequests.size})`
+                );
+            }
         } else {
             this.requestBuffer.add(item);
+            if (this.config.debug) {
+                InternalLogger.info(
+                    `VirtualPageLoadTimer: Added XHR to buffer (total: ${this.requestBuffer.size})`
+                );
+            }
         }
     }
 
@@ -204,7 +230,19 @@ export class VirtualPageLoadTimer extends MonkeyPatched<
      * (3) Indicate current page has finished loading
      */
     private checkLoadStatus = () => {
+        if (this.config.debug) {
+            InternalLogger.info(
+                `VirtualPageLoadTimer: Checking load status - XHR: ${this.ongoingRequests.size}, Fetch: ${this.fetchCounter}`
+            );
+        }
+
         if (this.ongoingRequests.size === 0 && this.fetchCounter === 0) {
+            if (this.config.debug) {
+                InternalLogger.info(
+                    'Page load completed, recording navigation event'
+                );
+            }
+
             clearInterval(this.periodicCheckerId);
             clearTimeout(this.timeoutCheckerId);
             this.domMutationObserver.disconnect();
@@ -219,6 +257,10 @@ export class VirtualPageLoadTimer extends MonkeyPatched<
 
     /** Clears timers and disconnects observer to stop page timing. */
     private declareTimeout = () => {
+        if (this.config.debug) {
+            InternalLogger.info('Page load timed out');
+        }
+
         clearInterval(this.periodicCheckerId);
         this.periodicCheckerId = undefined;
         this.timeoutCheckerId = undefined;
@@ -257,5 +299,10 @@ export class VirtualPageLoadTimer extends MonkeyPatched<
 
     private updateLatestInteractionTime = (e: Event) => {
         this.latestInteractionTime = Date.now();
+        if (this.config.debug) {
+            InternalLogger.info(
+                `Updated interaction time: ${this.latestInteractionTime}`
+            );
+        }
     };
 }

src/sessions/__tests__/PageManager.test.ts

@@ -479,7 +479,8 @@ describe('PageManager tests', () => {
         const pageManager: PageManager = new PageManager(
             {
                 ...DEFAULT_CONFIG,
-                allowCookies: true
+                allowCookies: true,
+                legacySPASupport: true
             },
             record
         );
@@ -519,7 +520,8 @@ describe('PageManager tests', () => {
         const pageManager: PageManager = new PageManager(
             {
                 ...DEFAULT_CONFIG,
-                allowCookies: true
+                allowCookies: true,
+                legacySPASupport: true
             },
             record
         );
@@ -554,7 +556,8 @@ describe('PageManager tests', () => {
         const pageManager: PageManager = new PageManager(
             {
                 ...DEFAULT_CONFIG,
-                allowCookies: true
+                allowCookies: true,
+                legacySPASupport: true
             },
             record
         );
@@ -584,7 +587,8 @@ describe('PageManager tests', () => {
         // Init
         const config: Config = {
             ...DEFAULT_CONFIG,
-            allowCookies: true
+            allowCookies: true,
+            legacySPASupport: true
         };
         const pageManager: PageManager = new PageManager(config, record);
         const helper = pageManager['virtualPageLoadTimer'];
@@ -608,11 +612,61 @@ describe('PageManager tests', () => {
         expect(pageManager.getPage().start).toEqual(3000);
     });
 
+    test('when legacySPASupport is false then virtualPageLoadTimer is not created', async () => {
+        // Init
+        const pageManager: PageManager = new PageManager(
+            {
+                ...DEFAULT_CONFIG,
+                allowCookies: true,
+                legacySPASupport: false
+            },
+            record
+        );
+
+        // Assert
+        expect(pageManager['virtualPageLoadTimer']).toBeUndefined();
+
+        window.removeEventListener(
+            'popstate',
+            (pageManager as any).popstateListener
+        );
+    });
+
+    test('when legacySPASupport is false then page navigation works without virtual timing', async () => {
+        // Init
+        const pageManager: PageManager = new PageManager(
+            {
+                ...DEFAULT_CONFIG,
+                allowCookies: true,
+                legacySPASupport: false
+            },
+            record
+        );
+
+        // Run
+        pageManager.recordPageView('/rum/home');
+        pageManager.recordPageView('/console/home');
+
+        // Assert - page view events are still recorded
+        expect(record.mock.calls.length).toEqual(2);
+        expect(record.mock.calls[1][0]).toEqual(PAGE_VIEW_EVENT_TYPE);
+        expect(record.mock.calls[1][1]).toMatchObject({
+            pageId: '/console/home',
+            interaction: 1
+        });
+
+        window.removeEventListener(
+            'popstate',
+            (pageManager as any).popstateListener
+        );
+    });
+
     test('when latestInteractionTime is within the scope of routeChangeTimeout then page.start is latestInteractionTime', async () => {
         // Init
         const config: Config = {
             ...DEFAULT_CONFIG,
-            allowCookies: true
+            allowCookies: true,
+            legacySPASupport: true
         };
         const pageManager: PageManager = new PageManager(config, record);
         const helper = pageManager['virtualPageLoadTimer'];