console icon indicating copy to clipboard operation
console copied to clipboard

OCPBUGS-67319: Fix duplicate telemetry events on page load

Open luciddreamz opened this issue 1 month ago • 7 comments

Stabilize the useTelemetry hook callback using the "latest ref" pattern to prevent re-renders from causing duplicate identify and page events. Previously, the callback had four dependencies that changed frequently, triggering multiple effect re-runs.

Key changes:

  • Use refs to store current values, allowing a stable callback
  • Fix CaptureTelemetry to track fired events and prevent duplicates
  • Remove duplicate analytics.page() call from Segment initialization
  • Add test isolation for module-level telemetry event queue

Root cause: The fireTelemetryEvent callback had 4 dependencies (extensions, currentUserPreferenceTelemetryValue, userResource, userResourceIsLoaded) that changed frequently, causing effects to re-run and fire duplicate events.

Fixes https://issues.redhat.com/browse/OCPBUGS-67319

luciddreamz avatar Dec 14 '25 19:12 luciddreamz

@luciddreamz: This pull request references Jira Issue OCPBUGS-67319, which is invalid:

  • expected the bug to target the "4.22.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Stabilize the useTelemetry hook callback using the "latest ref" pattern to prevent re-renders from causing duplicate identify and page events. Previously, the callback had four dependencies that changed frequently, triggering multiple effect re-runs.

Key changes:

  • Use refs to store current values, allowing a stable callback
  • Fix CaptureTelemetry to track fired events and prevent duplicates
  • Remove duplicate analytics.page() call from Segment initialization
  • Add test isolation for module-level telemetry event queue

Root cause: The fireTelemetryEvent callback had 4 dependencies (extensions, currentUserPreferenceTelemetryValue, userResource, userResourceIsLoaded) that changed frequently, causing effects to re-run and fire duplicate events.

Fixes https://issues.redhat.com/browse/OCPBUGS-67319

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot avatar Dec 14 '25 19:12 openshift-ci-robot

Walkthrough

These changes modify telemetry event tracking by removing automatic page calls from Segment analytics initialization, refactoring telemetry event queuing and replay logic to use refs for stable callback references, introducing test utilities for clearing telemetry state, and gating the initial page telemetry event with a one-time check.

Changes

Cohort / File(s) Summary
Segment Analytics Configuration
frontend/packages/console-dynamic-plugin-sdk/src/api/segment-analytics.ts
Removed the automatic analytics.page() call that was invoked during initialization after loading the Segment script. The initial page-tracking behavior is no longer eager.
Telemetry Hook Testing
frontend/packages/console-shared/src/hooks/__tests__/useTelemetry.spec.ts
Added import of clearTelemetryEventsForTests and integrated it into the test suite's beforeEach hook to reset telemetry state between tests.
Telemetry Hook Core Logic
frontend/packages/console-shared/src/hooks/useTelemetry.ts
Refactored to use refs (useRef) for mirroring current values of extensions, user preference, userResource, and loading state. Introduced clearTelemetryEventsForTests() export. Updated replay logic to check ref-based values. Stabilized the returned telemetry callback to avoid recreating it on state changes. Pre-flight checks and event dispatch now read from refs instead of relying on closure over state.
Application Page Telemetry Integration
frontend/public/components/app.tsx
Replaced stateful debounceTime with fixed 500ms useDebounceCallback. Introduced initialPageFiredRef to gate the initial telemetry page event to fire once. Added fireUrlChangeEventRef to hold the latest callback reference. Simplified effect dependencies by removing callback dependencies in favor of stable refs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Attention areas:
    • useTelemetry.ts replay and queuing logic: Verify that the ref-based state access correctly reflects opt-in status and loading states during event queuing and replaying.
    • Ref dependency patterns: Confirm that the use of useRef for stable callback references in both useTelemetry.ts and app.tsx correctly avoids unintended effect re-runs while maintaining up-to-date data.
    • Initialization sequence: Ensure the removal of automatic analytics.page() in segment-analytics.ts and the initialPageFiredRef gate in app.tsx are coordinated so that the first page event fires correctly once.
    • Test state isolation: Verify that clearTelemetryEventsForTests() properly resets all shared state and that the beforeEach hook is sufficient for test isolation.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 14 '25 19:12 coderabbitai[bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: luciddreamz Once this PR has been reviewed and has the lgtm label, please assign rhamilto for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci[bot] avatar Dec 14 '25 19:12 openshift-ci[bot]

/jira refresh

jhadvig avatar Dec 15 '25 10:12 jhadvig

@jhadvig: This pull request references Jira Issue OCPBUGS-67319, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.22.0) matches configured target version for branch (4.22.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact: /cc @yapei

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot avatar Dec 15 '25 10:12 openshift-ci-robot

/retest

jhadvig avatar Dec 15 '25 10:12 jhadvig

@luciddreamz: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gcp-console 591693b0d3a4d2cc0394b36e2902b3dd200dfc99 link true /test e2e-gcp-console

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci[bot] avatar Dec 15 '25 17:12 openshift-ci[bot]