suggested changes

Author: Abinet18

package-lock.json

@@ -55,7 +55,8 @@
     "karma-jquery": "0.2.4"
   },
   "dependencies": {
-    "@opentelemetry/instrumentation": "^0.208.0"
+    "@opentelemetry/instrumentation": "^0.208.0",
+    "@opentelemetry/semantic-conventions": "^1.27.0"
   },
   "homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/packages/instrumentation-browser-navigation#readme"
 }

packages/instrumentation-browser-navigation/package.json

@@ -16,18 +16,20 @@
 
 import { InstrumentationBase, isWrapped } from '@opentelemetry/instrumentation';
 import { LogRecord } from '@opentelemetry/api-logs';
+import { ATTR_URL_FULL } from '@opentelemetry/semantic-conventions';
 import { PACKAGE_NAME, PACKAGE_VERSION } from './version';
 import {
   BrowserNavigationInstrumentationConfig,
   NavigationType,
   ApplyCustomLogRecordDataFunction,
+  SanitizeUrlFunction,
 } from './types';
+import { isHashChange, defaultSanitizeUrl } from './utils';
 
 /**
  * This class represents a browser navigation instrumentation plugin
  */
 export const EVENT_NAME = 'browser.navigation';
-export const ATTR_URL_FULL = 'url.full';
 export const ATTR_BROWSER_NAVIGATION_SAME_DOCUMENT =
   'browser.navigation.same_document';
 export const ATTR_BROWSER_NAVIGATION_HASH_CHANGE =
@@ -37,10 +39,12 @@ export const ATTR_BROWSER_NAVIGATION_HASH_TYPE = 'browser.navigation.type';
 export class BrowserNavigationInstrumentation extends InstrumentationBase<BrowserNavigationInstrumentationConfig> {
   applyCustomLogRecordData: ApplyCustomLogRecordDataFunction | undefined =
     undefined;
+  sanitizeUrl: SanitizeUrlFunction = defaultSanitizeUrl; // Initialize with default
   private _onLoadHandler?: () => void;
   private _onPopStateHandler?: (ev: PopStateEvent) => void;
   private _onNavigateHandler?: (ev: Event) => void;
   private _lastUrl: string = location.href;
+  private _hasProcessedInitialLoad: boolean = false;
 
   /**
    *
@@ -49,6 +53,10 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
   constructor(config: BrowserNavigationInstrumentationConfig) {
     super(PACKAGE_NAME, PACKAGE_VERSION, config);
     this.applyCustomLogRecordData = config?.applyCustomLogRecordData;
+    // Override default with custom sanitizer if provided
+    if (config?.sanitizeUrl) {
+      this.sanitizeUrl = config.sanitizeUrl;
+    }
   }
 
   init() {}
@@ -60,7 +68,7 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
     const navLogRecord: LogRecord = {
       eventName: EVENT_NAME,
       attributes: {
-        [ATTR_URL_FULL]: this._sanitizeUrl(document.documentURI),
+        [ATTR_URL_FULL]: this.sanitizeUrl(document.documentURI),
         [ATTR_BROWSER_NAVIGATION_SAME_DOCUMENT]: false,
         [ATTR_BROWSER_NAVIGATION_HASH_CHANGE]: false,
       },
@@ -78,8 +86,8 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
   ) {
     const referrerUrl = this._lastUrl;
     const currentUrl =
-      changeState === 'navigate' && navigationEvent?.destination?.url
-        ? navigationEvent.destination.url
+      changeState === 'currententrychange' && navigationEvent?.target?.currentEntry?.url
+        ? navigationEvent.target.currentEntry.url
         : location.href;
 
     if (referrerUrl === currentUrl) {
@@ -102,7 +110,7 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
     const navLogRecord: LogRecord = {
       eventName: EVENT_NAME,
       attributes: {
-        [ATTR_URL_FULL]: this._sanitizeUrl(currentUrl),
+        [ATTR_URL_FULL]: this.sanitizeUrl(currentUrl),
         [ATTR_BROWSER_NAVIGATION_SAME_DOCUMENT]: sameDocument,
         [ATTR_BROWSER_NAVIGATION_HASH_CHANGE]: hashChange,
         ...(navType ? { [ATTR_BROWSER_NAVIGATION_HASH_TYPE]: navType } : {}),
@@ -119,11 +127,24 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
    * executes callback {_onDOMContentLoaded } when the page is viewed
    */
   private _waitForPageLoad() {
+    // Check if document has already loaded completely
+    if (document.readyState === 'complete' && !this._hasProcessedInitialLoad) {
+      this._hasProcessedInitialLoad = true;
+      // Use setTimeout to allow tests to reset exporter before this fires
+      setTimeout(() => this._onHardNavigation(), 0);
+      return;
+    }
+
     // Ensure previous handler is removed before adding a new one
     if (this._onLoadHandler) {
       document.removeEventListener('DOMContentLoaded', this._onLoadHandler);
     }
-    this._onLoadHandler = this._onHardNavigation.bind(this);
+    this._onLoadHandler = () => {
+      if (!this._hasProcessedInitialLoad) {
+        this._hasProcessedInitialLoad = true;
+        this._onHardNavigation();
+      }
+    };
     document.addEventListener('DOMContentLoaded', this._onLoadHandler);
   }
 
@@ -147,13 +168,13 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
 
     if (navigationApi) {
       if (this._onNavigateHandler) {
-        navigationApi.removeEventListener('navigate', this._onNavigateHandler);
+        navigationApi.removeEventListener('currententrychange', this._onNavigateHandler);
         this._onNavigateHandler = undefined;
       }
       this._onNavigateHandler = (event: any) => {
-        this._onSoftNavigation('navigate', event);
+        this._onSoftNavigation('currententrychange', event);
       };
-      navigationApi.addEventListener('navigate', this._onNavigateHandler);
+      navigationApi.addEventListener('currententrychange', this._onNavigateHandler);
     } else {
       if (this._onPopStateHandler) {
         window.removeEventListener('popstate', this._onPopStateHandler);
@@ -183,14 +204,16 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
       try {
         const navigationApi = (window as any).navigation as EventTarget;
         navigationApi?.removeEventListener?.(
-          'navigate',
+          'currententrychange',
           this._onNavigateHandler
         );
       } catch {
         // Ignore errors when removing Navigation API listeners
       }
       this._onNavigateHandler = undefined;
     }
+    // Reset the initial load flag so it can be processed again if re-enabled
+    this._hasProcessedInitialLoad = false;
   }
 
   /**
@@ -243,53 +266,28 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
     }
   }
 
-  private _isHashChange(fromUrl: string, toUrl: string): boolean {
-    try {
-      const a = new URL(fromUrl, window.location.origin);
-      const b = new URL(toUrl, window.location.origin);
-      // Only consider it a hash change if:
-      // 1. Base URL (origin + pathname + search) is identical
-      // 2. Both URLs have hashes and they're different, OR we're adding a hash
-      const sameBase =
-        a.origin === b.origin &&
-        a.pathname === b.pathname &&
-        a.search === b.search;
-      const fromHasHash = a.hash !== '';
-      const toHasHash = b.hash !== '';
-      const hashesAreDifferent = a.hash !== b.hash;
-
-      return (
-        sameBase &&
-        hashesAreDifferent &&
-        ((fromHasHash && toHasHash) || (!fromHasHash && toHasHash))
-      );
-    } catch {
-      // Fallback: check if base URLs are identical and we're changing/adding hash (not removing)
-      const fromBase = fromUrl.split('#')[0];
-      const toBase = toUrl.split('#')[0];
-      const fromHash = fromUrl.split('#')[1] || '';
-      const toHash = toUrl.split('#')[1] || '';
-
-      const sameBase = fromBase === toBase;
-      const hashesAreDifferent = fromHash !== toHash;
-      const notRemovingHash = toHash !== ''; // Only true if we're not removing the hash
-
-      return sameBase && hashesAreDifferent && notRemovingHash;
-    }
-  }
 
   private _determineSameDocument(
     changeState?: string | null,
     navigationEvent?: any,
     fromUrl?: string,
     toUrl?: string
   ): boolean {
-    // For Navigation API events, use the sameDocument property if available
-    if (
-      changeState === 'navigate' &&
-      navigationEvent?.destination?.sameDocument !== undefined
-    ) {
-      return navigationEvent.destination.sameDocument;
+    // For Navigation API currententrychange events
+    if (changeState === 'currententrychange') {
+      // For currententrychange, we can check if the navigation was same-document
+      // by comparing origins or checking if it's a SPA navigation
+      if (fromUrl && toUrl) {
+        try {
+          const fromURL = new URL(fromUrl);
+          const toURL = new URL(toUrl);
+          return fromURL.origin === toURL.origin;
+        } catch {
+          return true; // Fallback to same document
+        }
+      }
+      // Default to true for same-document navigations in SPAs
+      return true;
     }
 
     // For other navigation types, determine based on URL comparison
@@ -321,110 +319,47 @@ export class BrowserNavigationInstrumentation extends InstrumentationBase<Browse
     fromUrl?: string,
     toUrl?: string
   ): boolean {
-    // For Navigation API events, use the hashChange property if available
-    if (
-      changeState === 'navigate' &&
-      navigationEvent?.hashChange !== undefined
-    ) {
-      return navigationEvent.hashChange;
+    // For Navigation API currententrychange events, determine based on URL comparison
+    if (changeState === 'currententrychange') {
+      if (fromUrl && toUrl) {
+        return isHashChange(fromUrl, toUrl);
+      }
+      return false;
     }
 
     // For all other cases, determine based on URL comparison
     if (fromUrl && toUrl) {
-      return this._isHashChange(fromUrl, toUrl);
+      return isHashChange(fromUrl, toUrl);
     }
 
     return false;
   }
 
-  /**
-   * Sanitizes URL according to OpenTelemetry specification:
-   * - Redacts credentials (username:password)
-   * - Redacts sensitive query parameters
-   * - Preserves fragment when available
-   */
-  private _sanitizeUrl(url: string): string {
-    const sensitiveParams = [
-      'password',
-      'passwd',
-      'secret',
-      'api_key',
-      'apikey',
-      'auth',
-      'authorization',
-      'token',
-      'access_token',
-      'refresh_token',
-      'jwt',
-      'session',
-      'sessionid',
-      'key',
-      'private_key',
-      'client_secret',
-      'client_id',
-      'signature',
-      'hash',
-    ];
-    try {
-      const urlObj = new URL(url);
-
-      // Redact credentials if present
-      if (urlObj.username || urlObj.password) {
-        urlObj.username = 'REDACTED';
-        urlObj.password = 'REDACTED';
-      }
-
-      // Redact sensitive query parameters
-
-      for (const param of sensitiveParams) {
-        if (urlObj.searchParams.has(param)) {
-          urlObj.searchParams.set(param, 'REDACTED');
-        }
-      }
-
-      return urlObj.toString();
-    } catch {
-      // If URL parsing fails, redact credentials and sensitive query parameters
-      let sanitized = url.replace(/\/\/[^:]+:[^@]+@/, '//REDACTED:REDACTED@');
-
-      for (const param of sensitiveParams) {
-        // Match param=value or param%3Dvalue (URL encoded)
-        const regex = new RegExp(`([?&]${param}(?:%3D|=))[^&]*`, 'gi');
-        sanitized = sanitized.replace(regex, '$1REDACTED');
-      }
-
-      return sanitized;
-    }
-  }
 
   private _mapChangeStateToType(
     changeState?: string | null,
     navigationEvent?: any
   ): NavigationType | undefined {
-    // For Navigation API events, check if it's a hash change first
-    if (changeState === 'navigate' && navigationEvent?.hashChange) {
-      // Hash changes are always considered 'push' operations semantically
-      return 'push';
-    }
-
-    // For Navigation API events, determine type based on event properties
-    if (changeState === 'navigate') {
-      // Check if this is a back/forward navigation (traverse)
-      if (navigationEvent?.navigationType === 'traverse') {
-        return 'traverse';
-      }
-
-      // Check if this is a replace operation
-      if (navigationEvent?.navigationType === 'replace') {
-        return 'replace';
-      }
-
-      // Check if this is a reload
-      if (navigationEvent?.navigationType === 'reload') {
-        return 'reload';
+    // For Navigation API currententrychange events
+    if (changeState === 'currententrychange') {
+      // First, try to get navigation type from the currententrychange event itself
+      if (navigationEvent?.navigationType) {
+        const navType = navigationEvent.navigationType;
+        switch (navType) {
+          case 'traverse':
+            return 'traverse';
+          case 'replace':
+            return 'replace';
+          case 'reload':
+            return 'reload';
+          case 'push':
+          default:
+            return 'push';
+        }
       }
-
-      // Default to 'push' for new navigations (link clicks, programmatic navigation)
+      
+      // Fallback: For currententrychange events without type info, default to 'push'
+      // Most programmatic navigations (history.pushState, link clicks) are 'push' operations
       return 'push';
     }
 

packages/instrumentation-browser-navigation/src/instrumentation.ts

@@ -24,10 +24,16 @@ export interface BrowserNavigationInstrumentationConfig
   applyCustomLogRecordData?: ApplyCustomLogRecordDataFunction;
   /** Use the Navigation API navigate event if available (experimental) */
   useNavigationApiIfAvailable?: boolean;
+  /** Custom function to sanitize URLs before adding to log records */
+  sanitizeUrl?: SanitizeUrlFunction;
 }
 
 export interface ApplyCustomLogRecordDataFunction {
   (logRecord: LogRecord): void;
 }
 
+export interface SanitizeUrlFunction {
+  (url: string): string;
+}
+
 export type NavigationType = 'push' | 'replace' | 'reload' | 'traverse';