chore: revert changes for disabling VirtualPageLoadTimer

CHANGELOG.md

@@ -12,7 +12,6 @@ All notable changes to this project will be documented in this file. See [standa
 
 ### Bug Fixes
 
-* disable VirtualPageLoadTimer by default ([#739](https://github.com/aws-observability/aws-rum-web/issues/739)) ([215d4c2](https://github.com/aws-observability/aws-rum-web/commit/215d4c230fd2eec879f2aef4b6d5693883733027))
 * ignore PutRumEvents for PerformanceResource events ([#712](https://github.com/aws-observability/aws-rum-web/issues/712)) ([8377316](https://github.com/aws-observability/aws-rum-web/commit/8377316c1189827d08708a48438c1f52a189c6e8))
 * purge invalid credentials on 403 ([#740](https://github.com/aws-observability/aws-rum-web/issues/740)) ([2329ecc](https://github.com/aws-observability/aws-rum-web/commit/2329ecc23f857226dc273d3b24ba23b836dc33c4))
 * resolve npe in web vitals attribution ([#716](https://github.com/aws-observability/aws-rum-web/issues/716)) ([6151a9b](https://github.com/aws-observability/aws-rum-web/commit/6151a9b2e82d9bb4e45db1c15c631690b07cf3bf))

docs/configuration.md

@@ -47,7 +47,6 @@ 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
 

smoke/smoke-test-application-CDN/smoke.html

@@ -40,8 +40,7 @@
                     'content-type': 'application/json'
                 },
                 useBeacon: false,
-                releaseId: '2.1.7',
-                legacySPASupport: true
+                releaseId: '2.1.7'
             });
         </script>
 

smoke/smoke-test-application-CDN/smoke_w3c_format_enabled.html

@@ -41,8 +41,7 @@
                 },
                 useBeacon: false,
                 releaseId: '2.1.7',
-                enableW3CTraceId: true,
-                legacySPASupport: true
+                enableW3CTraceId: true
             });
         </script>
 

smoke/smoke-test-application-NPM-CJS/src/loader-npm-rum-2.ts

@@ -14,8 +14,7 @@ try {
         cookieAttributes: {
             unique: true
         },
-        useBeacon: false,
-        legacySPASupport: true
+        useBeacon: false
     };
 
     const APPLICATION_ID: string = $MONITOR_ID_2;

smoke/smoke-test-application-NPM-CJS/src/loader-npm-rum-w3c-format-enabled.ts

@@ -22,8 +22,7 @@ try {
             'content-type': 'application/json'
         },
         useBeacon: false,
-        enableW3CTraceId: true,
-        legacySPASupport: true
+        enableW3CTraceId: true
     };
 
     const APPLICATION_ID: string = $MONITOR_ID;

smoke/smoke-test-application-NPM-CJS/src/loader-npm-rum.ts

@@ -21,8 +21,7 @@ try {
             'x-api-key': 'a1b2c3d4e5f6',
             'content-type': 'application/json'
         },
-        useBeacon: false,
-        legacySPASupport: true
+        useBeacon: false
     };
 
     const APPLICATION_ID: string = $MONITOR_ID;

smoke/smoke-test-application-NPM-ES/src/loader-npm-rum-2.ts

@@ -15,8 +15,7 @@ try {
             unique: true
         },
         useBeacon: false,
-        releaseId: '2.1.7',
-        legacySPASupport: true
+        releaseId: '2.1.7'
     };
 
     const APPLICATION_ID: string = $MONITOR_ID_2;

smoke/smoke-test-application-NPM-ES/src/loader-npm-rum-w3c-format-enabled.ts

@@ -22,8 +22,7 @@ try {
             'content-type': 'application/json'
         },
         useBeacon: false,
-        enableW3CTraceId: true,
-        legacySPASupport: true
+        enableW3CTraceId: true
     };
 
     const APPLICATION_ID: string = $MONITOR_ID;

smoke/smoke-test-application-NPM-ES/src/loader-npm-rum.ts

@@ -22,8 +22,7 @@ try {
             'content-type': 'application/json'
         },
         useBeacon: false,
-        releaseId: '2.1.7',
-        legacySPASupport: true
+        releaseId: '2.1.7'
     };
 
     const APPLICATION_ID: string = $MONITOR_ID;

src/loader/loader-page-event.js

@@ -7,8 +7,7 @@ loader('cwr', 'abc123', '1.0', 'us-west-2', './rum_javascript_telemetry.js', {
     telemetries: [],
     pagesToInclude: [/\/(page_event.html|page_view_one|page_view_two)/],
     pagesToExclude: [/\/page_view_do_not_record/],
-    clientBuilder: showRequestClientBuilder,
-    legacySPASupport: true
+    clientBuilder: showRequestClientBuilder
 });
 window.cwr('setAwsCredentials', {
     accessKeyId: 'a',

src/orchestration/Orchestration.ts

@@ -98,7 +98,6 @@ export const defaultConfig = (cookieAttributes: CookieAttributes): Config => {
         useBeacon: true,
         userIdRetentionDays: 30,
         enableW3CTraceId: false,
-        legacySPASupport: false, // deprecated
         ...internalConfigOverrides
     };
 };
@@ -175,9 +174,6 @@ 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

@@ -636,6 +636,5 @@ 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 | undefined;
+    private virtualPageLoadTimer: VirtualPageLoadTimer;
     private TIMEOUT = 1000;
     private timeOnParentPage: number | undefined;
 
@@ -68,18 +68,11 @@ export class PageManager {
         this.page = undefined;
         this.resumed = false;
         this.recordInteraction = false;
-        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
-            );
-        }
+        this.virtualPageLoadTimer = new VirtualPageLoadTimer(
+            this,
+            config,
+            record
+        );
     }
 
     public getPage(): Page | undefined {
@@ -106,38 +99,28 @@ 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 created: ${pageId}`);
+                InternalLogger.info(`Landing page recorded: ${pageId}`);
             }
         } else if (this.page.pageId !== pageId) {
             this.createNextPage(this.page, pageId);
             if (this.config.debug) {
-                InternalLogger.info(`Navigation to new page: ${pageId}`);
+                InternalLogger.info(`Navigation to 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;
         }
@@ -156,14 +139,7 @@ export class PageManager {
 
     private createNextPage(currentPage: Page, pageId: string) {
         let startTime = Date.now();
-
-        if (this.config.debug) {
-            InternalLogger.info(
-                `Creating next page ${pageId}, interaction: ${
-                    currentPage.interaction + 1
-                }`
-            );
-        }
+        const interactionTime = this.virtualPageLoadTimer.latestInteractionTime;
 
         // The latest interaction time (latest) is not guaranteed to be the
         // interaction that triggered the route change (actual). There are two
@@ -184,24 +160,9 @@ 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.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)`
-                );
-            }
+        if (!this.resumed && startTime - interactionTime <= this.TIMEOUT) {
+            startTime = interactionTime;
+            this.virtualPageLoadTimer.startTiming();
         }
 
         this.timeOnParentPage = startTime - currentPage.start;

src/sessions/VirtualPageLoadTimer.ts

@@ -4,7 +4,6 @@ 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;
@@ -14,10 +13,6 @@ 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,
@@ -89,11 +84,6 @@ 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);
@@ -124,12 +114,6 @@ 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) => {
@@ -148,18 +132,8 @@ 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})`
-                );
-            }
         }
     }
 
@@ -230,19 +204,7 @@ 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();
@@ -257,10 +219,6 @@ 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;
@@ -299,10 +257,5 @@ export class VirtualPageLoadTimer extends MonkeyPatched<
 
     private updateLatestInteractionTime = (e: Event) => {
         this.latestInteractionTime = Date.now();
-        if (this.config.debug) {
-            InternalLogger.info(
-                `Updated interaction time: ${this.latestInteractionTime}`
-            );
-        }
     };
 }