OCPBUGS-67319: Fix duplicate telemetry events on page load

frontend/packages/console-dynamic-plugin-sdk/src/api/segment-analytics.ts

@@ -106,7 +106,6 @@ const initSegmentAnalytics = () => {
     options.integrations = { 'Segment.io': { apiHost: TELEMETRY_API_HOST } };
   }
   analytics.load(TELEMETRY_API_KEY, options);
-  analytics.page(); // Make the first page call to load the integrations
 };
 
 if (!SAMPLE_SESSION) {

frontend/packages/console-shared/src/hooks/__tests__/useTelemetry.spec.ts

@@ -12,6 +12,7 @@ import { testHook } from '@console/shared/src/test-utils/hooks-utils';
 import {
   getClusterProperties,
   updateClusterPropertiesFromTests,
+  clearTelemetryEventsForTests,
   useTelemetry,
 } from '../useTelemetry';
 
@@ -135,6 +136,7 @@ describe('useTelemetry', () => {
 
   beforeEach(() => {
     listener.mockReset();
+    clearTelemetryEventsForTests();
     const extensions: ResolvedExtension<TelemetryListener>[] = [
       {
         type: 'console.telemetry/listener',

frontend/packages/console-shared/src/hooks/useTelemetry.ts

@@ -1,4 +1,4 @@
-import { useEffect, useCallback } from 'react';
+import { useEffect, useCallback, useRef } from 'react';
 import {
   useResolvedExtensions,
   isTelemetryListener,
@@ -70,6 +70,10 @@ let clusterProperties = getClusterProperties();
 
 export const updateClusterPropertiesFromTests = () => (clusterProperties = getClusterProperties());
 
+export const clearTelemetryEventsForTests = () => {
+  telemetryEvents = [];
+};
+
 export const useTelemetry = () => {
   // TODO use usePluginInfo() hook to tell whether all dynamic plugins have been processed
   // to avoid firing telemetry events multiple times whenever a dynamic plugin loads asynchronously
@@ -85,24 +89,44 @@ export const useTelemetry = () => {
 
   const [extensions] = useResolvedExtensions<TelemetryListener>(isTelemetryListener);
 
+  // Store current values in refs so the callback can access them without being recreated
+  const extensionsRef = useRef(extensions);
+  extensionsRef.current = extensions;
+  const userPreferenceRef = useRef(currentUserPreferenceTelemetryValue);
+  userPreferenceRef.current = currentUserPreferenceTelemetryValue;
+  const userResourceRef = useRef(userResource);
+  userResourceRef.current = userResource;
+  const userResourceIsLoadedRef = useRef(userResourceIsLoaded);
+  userResourceIsLoadedRef.current = userResourceIsLoaded;
+
+  // Replay queued events when user explicitly opts in (OPT-IN cluster mode only)
   useEffect(() => {
     if (
       userIsOptedInToTelemetry(currentUserPreferenceTelemetryValue) &&
       clusterIsOptedInToTelemetry() &&
-      telemetryEvents.length > 0 &&
-      userResourceIsLoaded
+      extensions.length > 0 &&
+      userResourceIsLoaded &&
+      telemetryEvents.length > 0
     ) {
       telemetryEvents.forEach(({ eventType, event }) => {
         extensions.forEach((e) => e.properties.listener(eventType, { ...event, userResource }));
       });
       telemetryEvents = [];
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [currentUserPreferenceTelemetryValue, userResourceIsLoaded]);
+  }, [currentUserPreferenceTelemetryValue, userResourceIsLoaded, extensions, userResource]);
 
+  // Return a stable callback that reads current values from refs
+  // This prevents consumers from re-running effects when telemetry state changes
   return useCallback<TelemetryEventListener>(
     (eventType, properties: Record<string, any>) => {
-      if (isOptedOutFromTelemetry(currentUserPreferenceTelemetryValue)) return;
+      const currentUserPreference = userPreferenceRef.current;
+      const currentExtensions = extensionsRef.current;
+      const currentUserResource = userResourceRef.current;
+      const currentUserResourceIsLoaded = userResourceIsLoadedRef.current;
+
+      if (isOptedOutFromTelemetry(currentUserPreference)) {
+        return;
+      }
 
       const event = {
         ...clusterProperties,
@@ -112,20 +136,22 @@ export const useTelemetry = () => {
       };
 
       if (
-        (clusterIsOptedInToTelemetry() && !currentUserPreferenceTelemetryValue) ||
-        !userResourceIsLoaded
+        (clusterIsOptedInToTelemetry() && !currentUserPreference) ||
+        !currentUserResourceIsLoaded
       ) {
         telemetryEvents.push({ eventType, event });
 
         if (telemetryEvents.length > 10) {
-          telemetryEvents.shift(); // Remove the first element
+          telemetryEvents.shift();
         }
 
         return;
       }
 
-      extensions.forEach((e) => e.properties.listener(eventType, { ...event, userResource }));
+      currentExtensions.forEach((e) =>
+        e.properties.listener(eventType, { ...event, userResource: currentUserResource }),
+      );
     },
-    [extensions, currentUserPreferenceTelemetryValue, userResource, userResourceIsLoaded],
+    [], // Empty deps - callback is stable, reads current values from refs
   );
 };

frontend/public/components/app.tsx

@@ -360,26 +360,30 @@ const AppRouter: React.FC = () => {
 const CaptureTelemetry = memo(function CaptureTelemetry() {
   const [perspective] = useActivePerspective();
   const fireTelemetryEvent = useTelemetry();
-  const [debounceTime, setDebounceTime] = useState(5000);
   const [titleOnLoad, setTitleOnLoad] = useState('');
   // notify of identity change
   const user = useSelector(getUser);
   const telemetryTitle = getTelemetryTitle();
 
+  // Track if we've already fired the initial page event
+  const initialPageFiredRef = useRef(false);
+
+  // Wait 5 seconds before firing the first page event to allow initial redirects to settle
   useEffect(() => {
     setTimeout(() => {
       setTitleOnLoad(telemetryTitle);
-      setDebounceTime(500);
     }, 5000);
   }, [telemetryTitle]);
 
   useEffect(() => {
     if (user?.uid || user?.username) {
       fireTelemetryEvent('identify', { perspective, user });
     }
-    // Only trigger identify event when the user identifier changes
+    // Only trigger identify event when the user identifier changes.
+    // fireTelemetryEvent is stable (doesn't change identity) so it's safe to omit.
+    // We intentionally exclude perspective/user object - identify is for user identity, not navigation.
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [user?.uid || user?.username, fireTelemetryEvent]);
+  }, [user?.uid, user?.username]);
 
   // notify url change events
   // Debouncing the url change events so that redirects don't fire multiple events.
@@ -390,25 +394,36 @@ const CaptureTelemetry = memo(function CaptureTelemetry() {
       title: getTelemetryTitle(),
       ...withoutSensitiveInformations(location),
     });
-  }, debounceTime);
+  }, 500);
+
+  // Use a ref to always have the latest fireUrlChangeEvent without triggering effect re-runs
+  const fireUrlChangeEventRef = useRef(fireUrlChangeEvent);
+  fireUrlChangeEventRef.current = fireUrlChangeEvent;
+
   useEffect(() => {
     if (!titleOnLoad) {
       return;
     }
-    fireUrlChangeEvent(history.location);
+    // Only fire initial page event once
+    if (!initialPageFiredRef.current) {
+      initialPageFiredRef.current = true;
+      fireUrlChangeEventRef.current(history.location);
+    }
     let { pathname, search } = history.location;
     const unlisten = history.listen((location) => {
       const { pathname: nextPathname, search: nextSearch } = history.location;
       if (pathname !== nextPathname || search !== nextSearch) {
         pathname = nextPathname;
         search = nextSearch;
-        fireUrlChangeEvent(location);
+        fireUrlChangeEventRef.current(location);
       }
     });
     return () => {
       unlisten();
     };
-  }, [perspective, fireUrlChangeEvent, titleOnLoad]);
+    // fireUrlChangeEventRef is stable, so we don't need it in deps
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [perspective, titleOnLoad]);
 
   return null;
 });