diff --git a/docs/superpowers/plans/2026-07-19-telemetry-coverage-expansion.md b/docs/superpowers/plans/2026-07-19-telemetry-coverage-expansion.md new file mode 100644 index 000000000..23dff950d --- /dev/null +++ b/docs/superpowers/plans/2026-07-19-telemetry-coverage-expansion.md @@ -0,0 +1,641 @@ +# Telemetry Coverage Expansion Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add privacy-safe lifecycle telemetry for Azure login/logout, AKS cluster registration, namespace creation, and project deletion. + +**Architecture:** Keep `plugins/aks-desktop/src/telemetry/index.ts` as the only transport chokepoint. Add a focused typed helper for direct `aksd.*` feature events and a shared hook for one-time `opened` signals, then instrument explicit workflow boundaries without adding properties or identifiers. Each workflow emits categorical feature and error events while preserving its existing behavior when telemetry is disabled or fails. + +**Tech Stack:** TypeScript, React, Vitest, Testing Library, Application Insights through the existing telemetry module. + +--- + +Implementation checkpoints should remain uncommitted unless the user explicitly asks for commits. + +## File Map + +- Modify `plugins/aks-desktop/src/telemetry/schema.ts` to define the new closed feature names and error areas. +- Create `plugins/aks-desktop/src/telemetry/aksFeature.ts` for the typed direct-feature helper. +- Create `plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.ts` for dedicated-surface `opened` events. +- Modify the four workflow areas under `src/components/AzureAuth/`, `src/components/AKS/`, `src/components/CreateNamespace/`, and `src/components/DeleteAKSProject/`. +- Add or extend co-located tests for every new helper and workflow boundary. + +### Task 1: Extend Closed Telemetry Vocabularies + +**Files:** +- Modify: `plugins/aks-desktop/src/telemetry/schema.ts` +- Modify: `plugins/aks-desktop/src/telemetry/vocabulary.test.ts` + +- [ ] **Step 1: Write failing vocabulary tests** + +Update the existing exhaustive `KNOWN_FEATURE_TYPES` assertion to include the five new members after `aksd.deploy`: + +```ts +expect([...KNOWN_FEATURE_TYPES]).toEqual([ + 'headlamp.delete-resource', + 'headlamp.delete-resources', + 'headlamp.create-resource', + 'headlamp.edit-resource', + 'headlamp.scale-resource', + 'headlamp.restart-resource', + 'headlamp.restart-resources', + 'headlamp.rollback-resource', + 'headlamp.logs', + 'headlamp.terminal', + 'headlamp.pod-attach', + 'headlamp.plugin-loading-error', + 'headlamp.details-view', + 'headlamp.list-view', + 'headlamp.object-events', + 'aksd.project-create', + 'aksd.project-import', + 'aksd.deploy', + 'aksd.auth-login', + 'aksd.auth-logout', + 'aksd.cluster-add', + 'aksd.namespace-create', + 'aksd.project-delete', +]); +``` + +Update the existing exhaustive `ERROR_AREAS` assertion: + +```ts +expect([...ERROR_AREAS]).toEqual([ + 'project-create', + 'project-import', + 'deploy', + 'kubernetes', + 'plugin-ui', + 'auth-login', + 'auth-logout', + 'cluster-add', + 'namespace-create', + 'project-delete', +]); +``` + +Also add an assertion that the exported direct AKS feature tuple is the exact approved list: + +```ts +import { AKS_FEATURE_TYPES, ERROR_AREAS, KNOWN_FEATURE_TYPES } from './schema'; + +const expectedAksFeatures = [ + 'aksd.project-create', + 'aksd.project-import', + 'aksd.deploy', + 'aksd.auth-login', + 'aksd.auth-logout', + 'aksd.cluster-add', + 'aksd.namespace-create', + 'aksd.project-delete', +] as const; + +test('enumerates every direct AKS feature type', () => { + expect(AKS_FEATURE_TYPES).toEqual(expectedAksFeatures); +}); +``` + +- [ ] **Step 2: Run the tests and verify RED** + +Run from `plugins/aks-desktop`: + +```bash +npm run test:unit -- src/telemetry/vocabulary.test.ts +``` + +Expected: FAIL because `AKS_FEATURE_TYPES` does not exist and the new values are absent. + +- [ ] **Step 3: Add typed feature and error-area constants** + +Replace the direct `aksd.*` literals in `KNOWN_FEATURE_TYPES` with: + +```ts +export const AKS_FEATURE_TYPES = [ + 'aksd.project-create', + 'aksd.project-import', + 'aksd.deploy', + 'aksd.auth-login', + 'aksd.auth-logout', + 'aksd.cluster-add', + 'aksd.namespace-create', + 'aksd.project-delete', +] as const; + +export type AksFeatureType = (typeof AKS_FEATURE_TYPES)[number]; + +export const KNOWN_FEATURE_TYPES: ReadonlySet = new Set([ + 'headlamp.delete-resource', + 'headlamp.delete-resources', + 'headlamp.create-resource', + 'headlamp.edit-resource', + 'headlamp.scale-resource', + 'headlamp.restart-resource', + 'headlamp.restart-resources', + 'headlamp.rollback-resource', + 'headlamp.logs', + 'headlamp.terminal', + 'headlamp.pod-attach', + 'headlamp.plugin-loading-error', + 'headlamp.details-view', + 'headlamp.list-view', + 'headlamp.object-events', + ...AKS_FEATURE_TYPES, +]); +``` + +Extend `ERROR_AREA_VALUES`: + +```ts +const ERROR_AREA_VALUES = [ + 'project-create', + 'project-import', + 'deploy', + 'kubernetes', + 'plugin-ui', + 'auth-login', + 'auth-logout', + 'cluster-add', + 'namespace-create', + 'project-delete', +] as const; +``` + +- [ ] **Step 4: Run the tests and verify GREEN** + +```bash +npm run test:unit -- src/telemetry/vocabulary.test.ts +``` + +Expected: PASS. + +### Task 2: Add Typed Direct-Feature Helpers + +**Files:** +- Create: `plugins/aks-desktop/src/telemetry/aksFeature.ts` +- Create: `plugins/aks-desktop/src/telemetry/aksFeature.test.ts` +- Create: `plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.ts` +- Create: `plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.test.ts` + +- [ ] **Step 1: Write failing helper tests** + +Create `src/telemetry/aksFeature.test.ts`: + +```ts +import { beforeEach, describe, expect, test, vi } from 'vitest'; + +const mockTrackFeature = vi.hoisted(() => vi.fn()); + +vi.mock('./index', () => ({ trackFeature: mockTrackFeature })); + +import { trackAksFeature } from './aksFeature'; + +describe('trackAksFeature', () => { + beforeEach(() => vi.clearAllMocks()); + + test('forwards an allowlisted AKS lifecycle event', () => { + trackAksFeature('aksd.namespace-create', 'started'); + expect(mockTrackFeature).toHaveBeenCalledWith({ + feature: 'aksd.namespace-create', + status: 'started', + }); + }); + + test('does not propagate a telemetry failure', () => { + mockTrackFeature.mockImplementationOnce(() => { + throw new Error('transport failure'); + }); + expect(() => trackAksFeature('aksd.project-delete', 'failed')).not.toThrow(); + }); +}); +``` + +This test intentionally verifies the wrapper's synchronous no-throw contract by making the mocked `trackFeature` throw. Real SDK transport failure remains covered by `src/telemetry/index.test.ts`. + +Create `src/hooks/useTelemetryFeatureOpened.test.ts`: + +```ts +// @vitest-environment jsdom + +import { renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; + +const mockTrackAksFeature = vi.hoisted(() => vi.fn()); + +vi.mock('../telemetry/aksFeature', () => ({ trackAksFeature: mockTrackAksFeature })); + +import { useTelemetryFeatureOpened } from './useTelemetryFeatureOpened'; + +describe('useTelemetryFeatureOpened', () => { + beforeEach(() => vi.clearAllMocks()); + + test('emits one opened event for the mounted surface', () => { + const { rerender } = renderHook(() => useTelemetryFeatureOpened('aksd.cluster-add')); + rerender(); + expect(mockTrackAksFeature).toHaveBeenCalledTimes(1); + expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.cluster-add', 'opened'); + }); +}); +``` + +- [ ] **Step 2: Run the tests and verify RED** + +```bash +npm run test:unit -- src/telemetry/aksFeature.test.ts src/hooks/useTelemetryFeatureOpened.test.ts +``` + +Expected: FAIL because both modules are missing. + +- [ ] **Step 3: Implement the minimal typed helper** + +Create `src/telemetry/aksFeature.ts`: + +```ts +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +import { trackFeature } from './index'; +import type { AksFeatureType, TelemetryStatus } from './schema'; + +export type AksFeatureLifecycleStatus = Extract< + TelemetryStatus, + 'opened' | 'started' | 'succeeded' | 'failed' | 'cancelled' +>; + +export function trackAksFeature( + feature: AksFeatureType, + status: AksFeatureLifecycleStatus +): void { + try { + trackFeature({ feature, status }); + } catch { + // Telemetry must never affect the workflow being measured. + } +} +``` + +Create `src/hooks/useTelemetryFeatureOpened.ts`: + +```ts +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +import { useEffect } from 'react'; +import { trackAksFeature } from '../telemetry/aksFeature'; +import type { AksFeatureType } from '../telemetry/schema'; + +export function useTelemetryFeatureOpened(feature: AksFeatureType): void { + useEffect(() => { + trackAksFeature(feature, 'opened'); + }, [feature]); +} +``` + +- [ ] **Step 4: Run the tests and verify GREEN** + +```bash +npm run test:unit -- src/telemetry/aksFeature.test.ts src/hooks/useTelemetryFeatureOpened.test.ts +``` + +Expected: PASS. + +### Task 3: Instrument Azure Login and Logout + +**Files:** +- Create: `plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.test.tsx` +- Modify: `plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.tsx` +- Modify: `plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.test.ts` +- Modify: `plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.ts` + +- [ ] **Step 1: Add failing login lifecycle tests** + +Create a jsdom test that mocks `initiateLogin`, `getLoginStatus`, `trackAksFeature`, `trackError`, and `useTelemetryFeatureOpened`. Cover these exact assertions: + +```ts +expect(mockUseTelemetryFeatureOpened).toHaveBeenCalledWith('aksd.auth-login'); +expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'started'); +expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'succeeded'); +expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'failed'); +expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'cancelled'); +``` + +For an unsuccessful initiation, assert only categorical error data: + +```ts +expect(mockTrackError).toHaveBeenCalledWith({ + area: 'auth-login', + errorClass: 'UnknownError', + phase: 'failed', +}); +``` + +Use fake timers for polling success and timeout; timeout must use `TimeoutError`. In the success test, advance through both `LOGIN_POLL_INTERVAL_MS` and `LOGIN_REDIRECT_DELAY_MS`, then assert the success event was emitted before navigation. The existing transient polling catch continues polling and does not emit a terminal result. + +- [ ] **Step 2: Extend failing logout tests** + +Add telemetry mocks to `useAzureProfilePage.test.ts`: + +```ts +const mockTrackAksFeature = vi.hoisted(() => vi.fn()); +const mockTrackError = vi.hoisted(() => vi.fn()); + +vi.mock('../../../telemetry/aksFeature', () => ({ trackAksFeature: mockTrackAksFeature })); +vi.mock('../../../telemetry', () => ({ trackError: mockTrackError })); +``` + +Extend the success, stderr-error, and thrown-error tests to assert `started`, `succeeded`, or `failed`, plus: + +```ts +expect(mockTrackError).toHaveBeenCalledWith({ + area: 'auth-logout', + errorClass: 'UnknownError', + phase: 'failed', +}); +``` + +- [ ] **Step 3: Run focused tests and verify RED** + +```bash +npm run test:unit -- src/components/AzureAuth/AzureLoginPage.test.tsx src/components/AzureAuth/hooks/useAzureProfilePage.test.ts +``` + +Expected: FAIL because the workflow code does not emit telemetry. + +- [ ] **Step 4: Instrument login control-flow boundaries** + +Import the opened hook, typed helper, and `trackError`. Call `useTelemetryFeatureOpened('aksd.auth-login')`. Emit `started` at the start of `handleLogin`; `succeeded` when polling confirms login; `failed` for unsuccessful initiation, thrown initiation errors, and timeout; and `cancelled` in `handleCancel` only while `loading` is true. Do not emit a terminal event from the existing transient polling catch because the interval continues. + +Use `UnknownError` for initiation failures because the current structured result does not distinguish CLI, network, and authentication causes. Use `TimeoutError` for the existing timeout branch. Never parse or pass `result.message` or an exception into telemetry. + +- [ ] **Step 5: Instrument logout control-flow boundaries** + +Emit `started` before `runCommandAsync`, `succeeded` after the command passes `isAzError`, and `failed` plus an `UnknownError` `auth-logout` event in both failure branches. Do not add `opened` or `cancelled` for logout. + +- [ ] **Step 6: Run focused tests and verify GREEN** + +```bash +npm run test:unit -- src/components/AzureAuth/AzureLoginPage.test.tsx src/components/AzureAuth/hooks/useAzureProfilePage.test.ts +``` + +Expected: PASS. + +### Task 4: Instrument AKS Cluster Registration + +**Files:** +- Create: `plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.test.tsx` +- Create: `plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.test.tsx` +- Modify: `plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.tsx` +- Modify: `plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.tsx` + +- [ ] **Step 1: Write failing page-level opened/cancelled tests** + +Mock `RegisterAKSClusterDialog` so the test can invoke `onClose` and `onClusterRegistered`. Assert the page calls: + +```ts +expect(mockUseTelemetryFeatureOpened).toHaveBeenCalledWith('aksd.cluster-add'); +``` + +Closing before registration emits `cancelled`. Calling `onClusterRegistered()` before `onClose()` suppresses `cancelled`. + +- [ ] **Step 2: Write failing registration outcome tests** + +Mock `RegisterAKSClusterDialogPure` with controls that invoke `onSubscriptionChange`, `onClusterChange`, and `onRegister`. Mock `registerAKSCluster` and data-loading utilities. Assert successful registration emits `started` then `succeeded`. Unsuccessful and thrown results emit `failed` plus: + +```ts +expect(mockTrackError).toHaveBeenCalledWith({ + area: 'cluster-add', + errorClass: 'UnknownError', + phase: 'failed', +}); +``` + +- [ ] **Step 3: Run focused tests and verify RED** + +```bash +npm run test:unit -- src/components/AKS/RegisterAKSClusterPage.test.tsx src/components/AKS/RegisterAKSClusterDialog.test.tsx +``` + +Expected: FAIL because cluster-registration telemetry is absent. + +- [ ] **Step 4: Add opened/cancelled state to the page** + +Add `const registrationCompletedRef = useRef(false)` and call `useTelemetryFeatureOpened('aksd.cluster-add')`. Set the ref in `handleClusterRegistered`. In `handleClose`, emit `cancelled` only when the ref is false, then preserve delayed navigation. + +- [ ] **Step 5: Add registration outcomes to the dialog** + +Emit `started` only after subscription and cluster validation. Emit `failed` plus the categorical error event for both failure branches. Emit `succeeded` after registration succeeds and before the non-critical capability query. Capability-query failure must not become registration failure telemetry. + +- [ ] **Step 6: Run focused tests and verify GREEN** + +```bash +npm run test:unit -- src/components/AKS/RegisterAKSClusterPage.test.tsx src/components/AKS/RegisterAKSClusterDialog.test.tsx +``` + +Expected: PASS. + +### Task 5: Instrument Namespace Creation + +**Files:** +- Create: `plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.test.tsx` +- Modify: `plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.tsx` + +- [ ] **Step 1: Write failing lifecycle tests** + +Mock cluster configuration, `createNamespaceAsProject`, routing, the telemetry helper, `trackError`, and the opened hook. Assert: + +```ts +expect(mockUseTelemetryFeatureOpened).toHaveBeenCalledWith('aksd.namespace-create'); +expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'started'); +expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'succeeded'); +expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'failed'); +expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'cancelled'); +``` + +The failure test must assert: + +```ts +expect(mockTrackError).toHaveBeenCalledWith({ + area: 'namespace-create', + errorClass: 'UnknownError', + phase: 'failed', +}); +``` + +- [ ] **Step 2: Run the test and verify RED** + +```bash +npm run test:unit -- src/components/CreateNamespace/CreateNamespace.test.tsx +``` + +Expected: FAIL because namespace telemetry is absent. + +- [ ] **Step 3: Implement namespace lifecycle tracking** + +Call `useTelemetryFeatureOpened('aksd.namespace-create')`. Add `const terminalTrackedRef = useRef(false)`. Emit `started` when submission begins; `succeeded` after namespace creation and local settings updates complete; `failed` plus the categorical error event in the catch branch; and `cancelled` from the explicit cancel/back-to-home handler only when no terminal result was tracked. Do not instrument Back between wizard steps. + +- [ ] **Step 4: Run the test and verify GREEN** + +```bash +npm run test:unit -- src/components/CreateNamespace/CreateNamespace.test.tsx +``` + +Expected: PASS. + +### Task 6: Instrument Project Deletion + +**Files:** +- Create: `plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.test.tsx` +- Modify: `plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.tsx` +- Modify: `plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.test.ts` +- Modify: `plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.ts` + +- [ ] **Step 1: Write failing dialog opened/cancelled tests** + +Mock permission and deletion hooks. Assert clicking the delete icon emits `opened`, closing without confirming emits `cancelled`, and confirming does not emit `cancelled` through its close callback. + +- [ ] **Step 2: Extend failing deletion outcome tests** + +Mock `trackAksFeature` and `trackError` in `useProjectDeletion.test.ts`. Successful execution must emit `started` and `succeeded`. A rejected action must emit `failed` and: + +```ts +expect(mockTrackError).toHaveBeenCalledWith({ + area: 'project-delete', + errorClass: 'UnknownError', + phase: 'failed', +}); +``` + +Assert the original error is rethrown so `clusterAction` retains its notification behavior. + +- [ ] **Step 3: Run focused tests and verify RED** + +```bash +npm run test:unit -- src/components/DeleteAKSProject/AKSProjectDeleteButton.test.tsx src/components/DeleteAKSProject/hooks/useProjectDeletion.test.ts +``` + +Expected: FAIL because deletion telemetry is absent. + +- [ ] **Step 4: Track dialog discovery and cancellation** + +Replace inline state setters with explicit handlers: + +```ts +const handleOpen = () => { + trackAksFeature('aksd.project-delete', 'opened'); + setOpen(true); +}; + +const handleCancel = () => { + trackAksFeature('aksd.project-delete', 'cancelled'); + setOpen(false); +}; + +const handleConfirm = () => { + handleDelete(project, deleteNamespaces, () => setOpen(false)); +}; +``` + +Use `handleCancel` only for cancel/close and `handleConfirm` for confirmation. + +- [ ] **Step 5: Track deletion outcomes inside the action** + +Emit `started` before `clusterAction`. Immediately inside the existing async callback, open a `try` block before `const namespacePromises`. Immediately after the existing namespace-processing loop, add the success event and close with this catch block: + +```ts +trackAksFeature('aksd.project-delete', 'succeeded'); +} catch (error) { + trackAksFeature('aksd.project-delete', 'failed'); + trackError({ + area: 'project-delete', + errorClass: 'UnknownError', + phase: 'failed', + }); + throw error; +} +``` + +Preserve the existing `clusterAction` messages, navigation, and `onClose()` behavior. + +The current `clusterAction` API exposes a `cancelledMessage` but no cancellation callback to this hook. Therefore this slice intentionally records `cancelled` only when the confirmation dialog closes before deletion starts. Cancelling the underlying cluster action may leave a `started` event without a terminal feature event; adding an action-level cancellation bridge is deferred rather than inferred from UI or error text. + +- [ ] **Step 6: Run focused tests and verify GREEN** + +```bash +npm run test:unit -- src/components/DeleteAKSProject/AKSProjectDeleteButton.test.tsx src/components/DeleteAKSProject/hooks/useProjectDeletion.test.ts +``` + +Expected: PASS. + +### Task 7: Run Telemetry and Plugin Verification + +**Files:** +- Verify all modified and created files above. + +- [ ] **Step 1: Run the complete telemetry suite** + +```bash +npm run test:unit -- src/telemetry src/hooks/useTelemetryFeatureOpened.test.ts +``` + +Expected: PASS with no warnings. + +- [ ] **Step 2: Run all affected workflow tests together** + +```bash +npm run test:unit -- \ + src/components/AzureAuth/AzureLoginPage.test.tsx \ + src/components/AzureAuth/hooks/useAzureProfilePage.test.ts \ + src/components/AKS/RegisterAKSClusterPage.test.tsx \ + src/components/AKS/RegisterAKSClusterDialog.test.tsx \ + src/components/CreateNamespace/CreateNamespace.test.tsx \ + src/components/DeleteAKSProject/AKSProjectDeleteButton.test.tsx \ + src/components/DeleteAKSProject/hooks/useProjectDeletion.test.ts +``` + +Expected: PASS. + +- [ ] **Step 3: Run static validation** + +```bash +npm run tsc +npm run lint +npx prettier --check \ + src/telemetry/schema.ts \ + src/telemetry/aksFeature.ts \ + src/telemetry/aksFeature.test.ts \ + src/hooks/useTelemetryFeatureOpened.ts \ + src/hooks/useTelemetryFeatureOpened.test.ts \ + src/components/AzureAuth \ + src/components/AKS/RegisterAKSClusterPage.tsx \ + src/components/AKS/RegisterAKSClusterPage.test.tsx \ + src/components/AKS/RegisterAKSClusterDialog.tsx \ + src/components/AKS/RegisterAKSClusterDialog.test.tsx \ + src/components/CreateNamespace/CreateNamespace.tsx \ + src/components/CreateNamespace/CreateNamespace.test.tsx \ + src/components/DeleteAKSProject +``` + +Expected: all commands exit successfully. + +- [ ] **Step 4: Run the full plugin unit suite** + +```bash +npm run test:unit +``` + +Expected: PASS. Do not fix unrelated failures; report them separately if present. + +- [ ] **Step 5: Review the final diff for privacy and scope** + +```bash +git diff --check +git diff -- plugins/aks-desktop/src/telemetry plugins/aks-desktop/src/hooks plugins/aks-desktop/src/components +``` + +Confirm: + +- No new telemetry property keys were added. +- No identifiers or raw errors are passed to telemetry. +- Every new feature and error area is closed and typed. +- Only the four approved workflow areas were instrumented. +- Existing workflow behavior remains unchanged when telemetry is unavailable. diff --git a/docs/superpowers/specs/2026-07-19-telemetry-coverage-expansion-design.md b/docs/superpowers/specs/2026-07-19-telemetry-coverage-expansion-design.md new file mode 100644 index 000000000..ed3858754 --- /dev/null +++ b/docs/superpowers/specs/2026-07-19-telemetry-coverage-expansion-design.md @@ -0,0 +1,143 @@ +# Telemetry Coverage Expansion Design + +## Summary + +Expand AKS Desktop telemetry across a small set of high-value management workflows while preserving the privacy contract introduced by PRs #718 and #773. The first implementation covers Azure authentication, AKS cluster registration, namespace creation, and project deletion. + +The change reuses the existing `headlamp.feature` and `headlamp.exception` envelopes. It adds no free-form dimensions, identifiers, automatic collection, or production-ingestion dependencies. + +## Goals + +- Measure discovery and completion of core AKS Desktop management workflows. +- Record clear operation outcomes using the existing lifecycle statuses. +- Keep direct AKS-specific event names type-safe and centrally allowlisted. +- Reduce repeated telemetry boilerplate at feature call sites. +- Maintain the existing fail-closed, no-throw telemetry behavior. +- Keep the implementation small enough for one focused pull request. + +## Non-Goals + +- Instrumenting Access, Scaling, GitHub pipelines, Metrics, or Logs in this pull request. +- Capturing field edits, wizard navigation, filters, selections, refreshes, or generic button clicks. +- Adding duration, workflow identifiers, resource identifiers, error messages, or new telemetry properties. +- Changing consent behavior, install correlation, Application Insights configuration, or ingestion infrastructure. +- Refactoring the entire telemetry module. + +## Event Model + +Continue sending direct product telemetry through `headlamp.feature` with the existing `feature` and `status` properties. + +Add these closed feature names: + +- `aksd.auth-login` +- `aksd.auth-logout` +- `aksd.cluster-add` +- `aksd.namespace-create` +- `aksd.project-delete` + +Use only existing lifecycle statuses: + +- `opened`: the user reached a dedicated workflow surface. +- `started`: the user initiated the operation. +- `succeeded`: the operation completed successfully. +- `failed`: the operation reached a definitive failure. +- `cancelled`: the user explicitly abandoned an opened or started workflow before completion. + +Not every workflow must emit every status. For example, logout begins from the profile page and therefore does not need a separate `opened` event. + +## Shared API + +Introduce a typed AKS feature helper alongside the existing generic `trackFeature` API. The helper accepts only allowlisted `aksd.*` feature names and lifecycle statuses, then delegates to the existing telemetry chokepoint. + +Add a shared `useTelemetryFeatureOpened` hook under `src/hooks/` for dedicated workflow surfaces. The hook emits one `opened` event when the surface mounts. Operation handlers continue emitting `started`, `succeeded`, `failed`, and `cancelled` at explicit control-flow boundaries. + +The telemetry APIs remain no-throw. New call sites should invoke them directly rather than adding local `safelyTrack*` wrappers. Telemetry must never alter navigation, loading state, error handling, or operation results. + +## Workflow Instrumentation + +### Azure Login + +- Emit `opened` when the login page mounts. +- Emit `started` immediately before initiating Azure login. +- Emit `succeeded` after authentication is confirmed, before redirecting. +- Emit `failed` when login initiation or polling reaches a definitive failure. +- Emit `cancelled` when the user explicitly cancels an active login attempt. +- Emit `headlamp.exception` using area `auth-login` for failures. + +### Azure Logout + +- Emit `started` immediately before invoking Azure logout. +- Emit `succeeded` after the logout command succeeds. +- Emit `failed` when logout fails. +- Emit `headlamp.exception` using area `auth-logout` for failures. +- Do not emit `opened` merely because the profile page or logout button is visible. + +### Add/Register Cluster + +- Emit `opened` when the AKS cluster-registration surface mounts. +- Emit `started` when registration is submitted. +- Emit `succeeded` after the cluster is successfully registered. +- Emit `failed` when registration definitively fails. +- Emit `cancelled` for an explicit cancel or back action before a terminal result. +- Emit `headlamp.exception` using area `cluster-add` for failures. + +### Namespace Creation + +- Emit `opened` when the namespace-creation surface mounts. +- Emit `started` when creation is submitted. +- Emit `succeeded` after all required namespace setup completes. +- Emit `failed` when the workflow definitively fails. +- Emit `cancelled` for an explicit cancel or back action before a terminal result. +- Emit `headlamp.exception` using area `namespace-create` for failures. + +### Project Deletion + +- Emit `opened` when the delete confirmation workflow opens. +- Emit `started` when deletion is confirmed. +- Emit `succeeded` after deletion completes. +- Emit `failed` when deletion definitively fails. +- Emit `cancelled` when the confirmation workflow closes without starting deletion. +- Emit `headlamp.exception` using area `project-delete` for failures. + +## Error Classification + +Use only the existing categorical error classes: + +- `AuthenticationError` for explicit authentication failures. +- `PermissionError` for explicit authorization or role failures. +- `ValidationError` for rejected local or service validation. +- `NetworkError` for explicit connectivity failures. +- `TimeoutError` for existing timeout branches. +- `UnknownError` when the code cannot safely determine a category. + +Classification must be based on existing structured branches or result types. This pull request will not inspect or transmit raw error messages to derive categories. + +## Privacy Requirements + +- Feature and error values must be members of closed vocabularies. +- No subscription, tenant, resource group, cluster, namespace, project, user, or file identifiers may be added. +- No raw error messages, command output, URLs, routes, or form values may be transmitted. +- Existing property filtering and final envelope scrubbing remain unchanged. +- Existing consent and pre-initialization buffering behavior remain unchanged. + +## Testing Strategy + +Development follows test-driven development for each workflow: + +1. Extend vocabulary tests for the new feature names and error areas. +2. Add tests for the typed AKS feature helper and opened-event hook. +3. Add or extend workflow tests to verify lifecycle events at success, failure, and cancellation boundaries. +4. Verify telemetry failures do not change workflow behavior. +5. Run focused telemetry and affected component tests, followed by TypeScript, lint, formatting, and the plugin unit suite. + +Tests assert categorical telemetry properties only. They must not snapshot or expose user-controlled values. + +## Follow-Up Slices + +After this pull request, expand coverage in separate reviews: + +1. Access and Scaling operations. +2. GitHub pipeline configuration and workflow dispatch. +3. Metrics and Logs feature-open signals and meaningful configuration failures. +4. Optional coarse duration buckets and improved structured error classification. + diff --git a/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.test.tsx b/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.test.tsx new file mode 100644 index 000000000..32dc444fe --- /dev/null +++ b/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.test.tsx @@ -0,0 +1,238 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +// @vitest-environment jsdom + +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + getAKSClusters: vi.fn(), + getClusterCapabilities: vi.fn(), + getSubscriptions: vi.fn(), + onClose: vi.fn(), + onClusterRegistered: vi.fn(), + onRegistrationFinished: vi.fn(), + onRegistrationStarted: vi.fn(), + registerAKSCluster: vi.fn(), + replace: vi.fn(), + trackAksFeature: vi.fn(), + trackError: vi.fn(), +})); + +const subscription = { + id: 'sensitive-subscription-id', + name: 'Sensitive Subscription', + state: 'Enabled', + tenantId: 'sensitive-tenant-id', +}; + +const cluster = { + name: 'sensitive-cluster-name', + resourceGroup: 'sensitive-resource-group', + location: 'eastus', + kubernetesVersion: '1.32.0', + provisioningState: 'Succeeded', +}; + +vi.mock('@kinvolk/headlamp-plugin/lib', () => ({ + useTranslation: () => ({ + t: (key: string, values?: Record) => + values + ? Object.entries(values).reduce( + (message, [name, value]) => message.replace(`{{${name}}}`, value), + key + ) + : key, + }), +})); + +vi.mock('react-router-dom', () => ({ + useHistory: () => ({ replace: mocks.replace }), +})); + +vi.mock('../../hooks/useAzureAuth', () => ({ + useAzureAuth: () => ({ isChecking: false, isLoggedIn: true }), +})); + +vi.mock('../../utils/azure/aks', () => ({ + getAKSClusters: mocks.getAKSClusters, + getSubscriptions: mocks.getSubscriptions, + registerAKSCluster: mocks.registerAKSCluster, +})); + +vi.mock('../../utils/azure/az-clusters', () => ({ + getClusterCapabilities: mocks.getClusterCapabilities, +})); + +vi.mock('../../telemetry/aksFeature', () => ({ + trackAksFeature: mocks.trackAksFeature, +})); + +vi.mock('../../telemetry', () => ({ + trackError: mocks.trackError, +})); + +vi.mock('./RegisterAKSClusterDialogPure', () => ({ + default: ({ + onClusterChange, + onRegister, + onSubscriptionChange, + }: { + onClusterChange: (event: React.SyntheticEvent, value: typeof cluster) => void; + onRegister: () => void; + onSubscriptionChange: (event: React.SyntheticEvent, value: typeof subscription) => void; + }) => ( +
+ + + +
+ ), +})); + +import RegisterAKSClusterDialog from './RegisterAKSClusterDialog'; + +function renderDialog() { + return render( + + ); +} + +function selectRequiredValues() { + fireEvent.click(screen.getByRole('button', { name: 'Select subscription' })); + fireEvent.click(screen.getByRole('button', { name: 'Select cluster' })); +} + +function telemetryCallsAsJson() { + return JSON.stringify([mocks.trackAksFeature.mock.calls, mocks.trackError.mock.calls]); +} + +describe('RegisterAKSClusterDialog telemetry', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getAKSClusters.mockResolvedValue({ success: true, clusters: [] }); + mocks.getSubscriptions.mockResolvedValue({ success: true, subscriptions: [] }); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + }); + + test('emits started only after required selection validation', async () => { + mocks.registerAKSCluster.mockReturnValue(new Promise(() => {})); + renderDialog(); + + fireEvent.click(screen.getByRole('button', { name: 'Register' })); + expect(mocks.trackAksFeature).not.toHaveBeenCalled(); + + selectRequiredValues(); + fireEvent.click(screen.getByRole('button', { name: 'Register' })); + + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.cluster-add', 'started'); + expect(mocks.onRegistrationStarted).toHaveBeenCalledTimes(1); + expect(mocks.trackAksFeature.mock.invocationCallOrder[0]).toBeLessThan( + mocks.registerAKSCluster.mock.invocationCallOrder[0] + ); + }); + + test('emits succeeded immediately after registration before the capability query settles', async () => { + let rejectCapabilities!: (error: Error) => void; + mocks.registerAKSCluster.mockResolvedValue({ + success: true, + message: 'sensitive registration result', + }); + mocks.getClusterCapabilities.mockReturnValue( + new Promise((_resolve, reject) => { + rejectCapabilities = reject; + }) + ); + renderDialog(); + selectRequiredValues(); + + fireEvent.click(screen.getByRole('button', { name: 'Register' })); + + await waitFor(() => + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.cluster-add', 'succeeded') + ); + expect(mocks.trackAksFeature.mock.invocationCallOrder[1]).toBeLessThan( + mocks.getClusterCapabilities.mock.invocationCallOrder[0] + ); + expect(mocks.onClusterRegistered).toHaveBeenCalledTimes(1); + expect(mocks.onRegistrationFinished).toHaveBeenCalledWith('succeeded'); + expect(telemetryCallsAsJson()).not.toContain('sensitive'); + + await act(async () => { + rejectCapabilities(new Error('sensitive capability failure')); + }); + }); + + test('emits failed and a privacy-safe error for an unsuccessful result', async () => { + mocks.registerAKSCluster.mockResolvedValue({ + success: false, + message: 'sensitive unsuccessful result', + }); + renderDialog(); + selectRequiredValues(); + + fireEvent.click(screen.getByRole('button', { name: 'Register' })); + + await waitFor(() => + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.cluster-add', 'failed') + ); + expect(mocks.trackError).toHaveBeenCalledWith({ + area: 'cluster-add', + errorClass: 'UnknownError', + phase: 'failed', + }); + expect(mocks.onRegistrationFinished).toHaveBeenCalledWith('failed'); + expect(mocks.getClusterCapabilities).not.toHaveBeenCalled(); + expect(telemetryCallsAsJson()).not.toContain('sensitive'); + }); + + test('emits failed and a privacy-safe error for a thrown exception', async () => { + mocks.registerAKSCluster.mockRejectedValue(new Error('sensitive thrown exception')); + renderDialog(); + selectRequiredValues(); + + fireEvent.click(screen.getByRole('button', { name: 'Register' })); + + await waitFor(() => + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.cluster-add', 'failed') + ); + expect(mocks.trackError).toHaveBeenCalledWith({ + area: 'cluster-add', + errorClass: 'UnknownError', + phase: 'failed', + }); + expect(mocks.onRegistrationFinished).toHaveBeenCalledWith('failed'); + expect(telemetryCallsAsJson()).not.toContain('sensitive'); + }); + + test('does not classify a non-critical capability failure as registration failure', async () => { + mocks.registerAKSCluster.mockResolvedValue({ success: true, message: 'registered' }); + mocks.getClusterCapabilities.mockRejectedValue(new Error('capability unavailable')); + renderDialog(); + selectRequiredValues(); + + fireEvent.click(screen.getByRole('button', { name: 'Register' })); + + await waitFor(() => expect(mocks.getClusterCapabilities).toHaveBeenCalledTimes(1)); + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.cluster-add', 'started'], + ['aksd.cluster-add', 'succeeded'], + ]); + expect(mocks.trackError).not.toHaveBeenCalled(); + }); +}); diff --git a/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.tsx b/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.tsx index cf75c6aa5..b849fbe9d 100644 --- a/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.tsx +++ b/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterDialog.tsx @@ -5,6 +5,8 @@ import { useTranslation } from '@kinvolk/headlamp-plugin/lib'; import React, { useEffect, useRef, useState } from 'react'; import { useHistory } from 'react-router-dom'; import { useAzureAuth } from '../../hooks/useAzureAuth'; +import { trackError } from '../../telemetry'; +import { trackAksFeature } from '../../telemetry/aksFeature'; import type { ClusterCapabilities } from '../../types/ClusterCapabilities'; import { getAKSClusters, getSubscriptions, registerAKSCluster } from '../../utils/azure/aks'; import { getClusterCapabilities } from '../../utils/azure/az-clusters'; @@ -15,12 +17,16 @@ interface RegisterAKSClusterDialogProps { open: boolean; onClose: () => void; onClusterRegistered?: () => void; + onRegistrationFinished?: (outcome: 'failed' | 'succeeded') => void; + onRegistrationStarted?: () => void; } export default function RegisterAKSClusterDialog({ open, onClose, onClusterRegistered, + onRegistrationFinished, + onRegistrationStarted, }: RegisterAKSClusterDialogProps) { const history = useHistory(); const { t } = useTranslation(); @@ -236,55 +242,22 @@ export default function RegisterAKSClusterDialog({ return; } + onRegistrationStarted?.(); + trackAksFeature('aksd.cluster-add', 'started'); setLoading(true); setError(''); setSuccess(''); + let result: Awaited>; try { // Register the cluster by running az aks get-credentials and setting up kubeconfig - const result = await registerAKSCluster( + result = await registerAKSCluster( selectedSubscription.id, selectedCluster.resourceGroup, selectedCluster.name, undefined, // managedNamespace selectedSubscription.tenantId ); - - if (!result.success) { - setError(result.message); - setLoading(false); - return; - } - - setLoading(false); - - // Show success message with cluster name - setSuccess( - t("Cluster '{{cluster}}' successfully merged in kubeconfig", { - cluster: selectedCluster.name, - }) - ); - - onClusterRegistered?.(); - - // Check cluster capabilities (non-blocking) - setCapabilitiesLoading(true); - try { - const caps = await getClusterCapabilities({ - subscriptionId: selectedSubscription.id, - resourceGroup: selectedCluster.resourceGroup, - clusterName: selectedCluster.name, - }); - if (isMountedRef.current) { - setCapabilities(caps); - } - } catch { - // Non-critical — just don't show capabilities - } finally { - if (isMountedRef.current) { - setCapabilitiesLoading(false); - } - } } catch (err) { console.error('Error registering AKS cluster:', err); setError( @@ -293,6 +266,51 @@ export default function RegisterAKSClusterDialog({ }) ); setLoading(false); + trackAksFeature('aksd.cluster-add', 'failed'); + trackError({ area: 'cluster-add', errorClass: 'UnknownError', phase: 'failed' }); + onRegistrationFinished?.('failed'); + return; + } + + if (!result.success) { + setError(result.message); + setLoading(false); + trackAksFeature('aksd.cluster-add', 'failed'); + trackError({ area: 'cluster-add', errorClass: 'UnknownError', phase: 'failed' }); + onRegistrationFinished?.('failed'); + return; + } + + trackAksFeature('aksd.cluster-add', 'succeeded'); + onRegistrationFinished?.('succeeded'); + setLoading(false); + + // Show success message with cluster name + setSuccess( + t("Cluster '{{cluster}}' successfully merged in kubeconfig", { + cluster: selectedCluster.name, + }) + ); + + onClusterRegistered?.(); + + // Check cluster capabilities (non-blocking) + setCapabilitiesLoading(true); + try { + const caps = await getClusterCapabilities({ + subscriptionId: selectedSubscription.id, + resourceGroup: selectedCluster.resourceGroup, + clusterName: selectedCluster.name, + }); + if (isMountedRef.current) { + setCapabilities(caps); + } + } catch { + // Non-critical — just don't show capabilities + } finally { + if (isMountedRef.current) { + setCapabilitiesLoading(false); + } } }; diff --git a/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.test.tsx b/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.test.tsx new file mode 100644 index 000000000..a5dd2dfa4 --- /dev/null +++ b/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.test.tsx @@ -0,0 +1,128 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +// @vitest-environment jsdom + +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import React from 'react'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + push: vi.fn(), + trackAksFeature: vi.fn(), + useTelemetryFeatureOpened: vi.fn(), +})); + +vi.mock('react-router-dom', () => ({ + useHistory: () => ({ push: mocks.push }), +})); + +vi.mock('../../telemetry/aksFeature', () => ({ + trackAksFeature: mocks.trackAksFeature, +})); + +vi.mock('../../hooks/useTelemetryFeatureOpened', () => ({ + useTelemetryFeatureOpened: mocks.useTelemetryFeatureOpened, +})); + +vi.mock('./RegisterAKSClusterDialog', () => ({ + default: ({ + onClose, + onClusterRegistered, + onRegistrationFinished, + onRegistrationStarted, + }: { + onClose: () => void; + onClusterRegistered?: () => void; + onRegistrationFinished?: (outcome: 'failed' | 'succeeded') => void; + onRegistrationStarted?: () => void; + }) => ( +
+ + + + +
+ ), +})); + +import RegisterAKSClusterPage from './RegisterAKSClusterPage'; + +describe('RegisterAKSClusterPage telemetry', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + }); + + afterEach(() => { + cleanup(); + vi.useRealTimers(); + }); + + test('records the cluster-add page opening through the shared hook', () => { + render(); + + expect(mocks.useTelemetryFeatureOpened).toHaveBeenCalledWith('aksd.cluster-add'); + }); + + test('emits cancelled once when closing before registration succeeds', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + + expect(mocks.trackAksFeature.mock.calls).toEqual([['aksd.cluster-add', 'cancelled']]); + + vi.advanceTimersByTime(100); + expect(mocks.push).toHaveBeenCalledWith('/'); + }); + + test('suppresses cancellation when close follows terminal registration success', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Registered' })); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + + expect(mocks.trackAksFeature).not.toHaveBeenCalledWith('aksd.cluster-add', 'cancelled'); + }); + + test('does not emit cancelled when close follows a failed registration attempt', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Fail registration' })); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + + expect(mocks.trackAksFeature.mock.calls).toEqual([['aksd.cluster-add', 'failed']]); + }); + + test('emits cancelled when closing during a retry after failure', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Fail registration' })); + fireEvent.click(screen.getByRole('button', { name: 'Start registration' })); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.cluster-add', 'failed'], + ['aksd.cluster-add', 'cancelled'], + ]); + }); + + test('clears pending navigation when unmounted', () => { + const rendered = render(); + + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + rendered.unmount(); + vi.advanceTimersByTime(100); + + expect(mocks.push).not.toHaveBeenCalled(); + }); +}); diff --git a/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.tsx b/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.tsx index 91cb167fa..188dc0fd4 100644 --- a/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.tsx +++ b/plugins/aks-desktop/src/components/AKS/RegisterAKSClusterPage.tsx @@ -1,8 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the Apache 2.0. -import React, { useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import { useHistory } from 'react-router-dom'; +import { useTelemetryFeatureOpened } from '../../hooks/useTelemetryFeatureOpened'; +import { trackAksFeature } from '../../telemetry/aksFeature'; import RegisterAKSClusterDialog from './RegisterAKSClusterDialog'; /** @@ -12,24 +14,57 @@ import RegisterAKSClusterDialog from './RegisterAKSClusterDialog'; export default function RegisterAKSClusterPage() { const [open, setOpen] = useState(true); const history = useHistory(); + const terminalStatusRef = useRef<'active' | 'cancelled' | 'failed' | 'succeeded'>('active'); + const navigationTimerRef = useRef | null>(null); + + useTelemetryFeatureOpened('aksd.cluster-add'); + + useEffect(() => { + return () => { + if (navigationTimerRef.current !== null) { + clearTimeout(navigationTimerRef.current); + } + }; + }, []); const handleClose = () => { + if (terminalStatusRef.current === 'active') { + terminalStatusRef.current = 'cancelled'; + trackAksFeature('aksd.cluster-add', 'cancelled'); + } + setOpen(false); // Navigate back to home/clusters page - setTimeout(() => { - history.push('/'); - }, 100); + if (navigationTimerRef.current === null) { + navigationTimerRef.current = setTimeout(() => { + navigationTimerRef.current = null; + history.push('/'); + }, 100); + } }; const handleClusterRegistered = () => { + if (terminalStatusRef.current === 'active') { + terminalStatusRef.current = 'succeeded'; + } // Dialog will handle reload, so no need to do anything here }; + const handleRegistrationStarted = () => { + terminalStatusRef.current = 'active'; + }; + + const handleRegistrationFinished = (outcome: 'failed' | 'succeeded') => { + terminalStatusRef.current = outcome; + }; + return ( ); } diff --git a/plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.test.tsx b/plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.test.tsx new file mode 100644 index 000000000..7ab9d6a1f --- /dev/null +++ b/plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.test.tsx @@ -0,0 +1,424 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +// @vitest-environment jsdom + +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { + LOGIN_POLL_INTERVAL_MS, + LOGIN_REDIRECT_DELAY_MS, + LOGIN_TIMEOUT_MS, +} from '../../utils/constants/timing'; + +const mocks = vi.hoisted(() => ({ + getLoginStatus: vi.fn(), + initiateLogin: vi.fn(), + push: vi.fn(), + trackAksFeature: vi.fn(), + trackError: vi.fn(), + useTelemetryFeatureOpened: vi.fn(), +})); + +vi.mock('@iconify/react', () => ({ Icon: () => null })); + +vi.mock('@kinvolk/headlamp-plugin/lib', () => ({ + useTranslation: () => ({ + t: (key: string, values?: Record) => + values + ? Object.entries(values).reduce( + (message, [name, value]) => message.replace(`{{${name}}}`, value), + key + ) + : key, + }), +})); + +vi.mock('react-router-dom', () => ({ + useHistory: () => ({ push: mocks.push }), + useLocation: () => ({ search: '' }), +})); + +vi.mock('../../utils/azure/az-auth', () => ({ + getLoginStatus: mocks.getLoginStatus, + initiateLogin: mocks.initiateLogin, +})); + +vi.mock('../../telemetry/aksFeature', () => ({ + trackAksFeature: mocks.trackAksFeature, +})); + +vi.mock('../../telemetry', () => ({ + trackError: mocks.trackError, +})); + +vi.mock('../../hooks/useTelemetryFeatureOpened', () => ({ + useTelemetryFeatureOpened: mocks.useTelemetryFeatureOpened, +})); + +import AzureLoginPage from './AzureLoginPage'; + +function createDeferred() { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((promiseResolve, promiseReject) => { + resolve = promiseResolve; + reject = promiseReject; + }); + return { promise, reject, resolve }; +} + +async function renderLoggedOutPage() { + mocks.getLoginStatus.mockResolvedValueOnce({ isLoggedIn: false }); + const rendered = render(); + await screen.findByRole('button', { name: 'Sign in with Azure' }); + return rendered; +} + +describe('AzureLoginPage telemetry', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers({ shouldAdvanceTime: true }); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + test('records the login page opening through the shared hook', async () => { + await renderLoggedOutPage(); + + expect(mocks.useTelemetryFeatureOpened).toHaveBeenCalledWith('aksd.auth-login'); + }); + + test('emits started at the beginning of an explicit login attempt', async () => { + mocks.initiateLogin.mockReturnValue(new Promise(() => {})); + await renderLoggedOutPage(); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'started'); + expect(mocks.trackAksFeature.mock.invocationCallOrder[0]).toBeLessThan( + mocks.initiateLogin.mock.invocationCallOrder[0] + ); + }); + + test('ignores an unsuccessful initiation result after the attempt is cancelled', async () => { + const deferred = createDeferred<{ success: boolean; message: string }>(); + mocks.initiateLogin.mockReturnValue(deferred.promise); + await renderLoggedOutPage(); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + fireEvent.click(await screen.findByRole('button', { name: 'Cancel' })); + await act(async () => { + deferred.resolve({ success: false, message: 'late failure' }); + await deferred.promise; + }); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.auth-login', 'started'], + ['aksd.auth-login', 'cancelled'], + ]); + expect(mocks.trackError).not.toHaveBeenCalled(); + }); + + test('does not start polling when successful initiation resolves after cancellation', async () => { + const deferred = createDeferred<{ success: boolean; message: string }>(); + mocks.initiateLogin.mockReturnValue(deferred.promise); + await renderLoggedOutPage(); + mocks.getLoginStatus.mockResolvedValue({ isLoggedIn: false }); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + fireEvent.click(await screen.findByRole('button', { name: 'Cancel' })); + await act(async () => { + deferred.resolve({ success: true, message: 'late success' }); + await deferred.promise; + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_TIMEOUT_MS); + }); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.auth-login', 'started'], + ['aksd.auth-login', 'cancelled'], + ]); + expect(mocks.trackError).not.toHaveBeenCalled(); + expect(mocks.getLoginStatus).toHaveBeenCalledTimes(1); + }); + + test('ignores an initiation rejection after the attempt is cancelled', async () => { + const deferred = createDeferred<{ success: boolean; message: string }>(); + mocks.initiateLogin.mockReturnValue(deferred.promise); + await renderLoggedOutPage(); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + fireEvent.click(await screen.findByRole('button', { name: 'Cancel' })); + await act(async () => { + deferred.reject(new Error('late rejection')); + await deferred.promise.catch(() => {}); + }); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.auth-login', 'started'], + ['aksd.auth-login', 'cancelled'], + ]); + expect(mocks.trackError).not.toHaveBeenCalled(); + }); + + test('a stale cancelled attempt cannot interfere with a new login attempt', async () => { + const firstAttempt = createDeferred<{ success: boolean; message: string }>(); + const secondAttempt = createDeferred<{ success: boolean; message: string }>(); + mocks.initiateLogin + .mockReturnValueOnce(firstAttempt.promise) + .mockReturnValueOnce(secondAttempt.promise); + await renderLoggedOutPage(); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + fireEvent.click(await screen.findByRole('button', { name: 'Cancel' })); + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + + await act(async () => { + firstAttempt.resolve({ success: false, message: 'stale failure' }); + await firstAttempt.promise; + }); + expect(screen.getByRole('button', { name: 'Cancel' })).toBeTruthy(); + + mocks.getLoginStatus.mockResolvedValueOnce({ isLoggedIn: true }); + await act(async () => { + secondAttempt.resolve({ success: true, message: 'current success' }); + await secondAttempt.promise; + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_POLL_INTERVAL_MS); + }); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.auth-login', 'started'], + ['aksd.auth-login', 'cancelled'], + ['aksd.auth-login', 'started'], + ['aksd.auth-login', 'succeeded'], + ]); + expect(mocks.trackError).not.toHaveBeenCalled(); + }); + + test('emits succeeded before navigating after polling confirms login', async () => { + mocks.initiateLogin.mockResolvedValue({ success: true, message: 'browser opened' }); + await renderLoggedOutPage(); + mocks.getLoginStatus.mockResolvedValueOnce({ + isLoggedIn: true, + username: 'sensitive-user@contoso.com', + tenantId: 'sensitive-tenant', + subscriptionId: 'sensitive-subscription', + }); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + await act(async () => {}); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_POLL_INTERVAL_MS); + }); + + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'succeeded'); + expect(mocks.push).not.toHaveBeenCalled(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_REDIRECT_DELAY_MS); + }); + + expect(mocks.push).toHaveBeenCalledWith('/azure/profile'); + const successCall = mocks.trackAksFeature.mock.calls.findIndex( + call => call[0] === 'aksd.auth-login' && call[1] === 'succeeded' + ); + expect(mocks.trackAksFeature.mock.invocationCallOrder[successCall]).toBeLessThan( + mocks.push.mock.invocationCallOrder[0] + ); + expect(JSON.stringify(mocks.trackAksFeature.mock.calls)).not.toContain('sensitive-'); + }); + + test('does not emit cancelled after polling has already emitted succeeded', async () => { + mocks.initiateLogin.mockResolvedValue({ success: true, message: 'browser opened' }); + await renderLoggedOutPage(); + mocks.getLoginStatus.mockResolvedValueOnce({ isLoggedIn: true }); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + await act(async () => {}); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_POLL_INTERVAL_MS); + }); + + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'succeeded'); + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(mocks.trackAksFeature).not.toHaveBeenCalledWith('aksd.auth-login', 'cancelled'); + expect(mocks.push).not.toHaveBeenCalled(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_REDIRECT_DELAY_MS); + }); + expect(mocks.push).toHaveBeenCalledWith('/azure/profile'); + }); + + test('unmount during an in-flight poll prevents all later effects', async () => { + const statusDeferred = createDeferred<{ isLoggedIn: boolean }>(); + mocks.initiateLogin.mockResolvedValue({ success: true, message: 'browser opened' }); + const { unmount } = await renderLoggedOutPage(); + mocks.getLoginStatus.mockReturnValueOnce(statusDeferred.promise); + const dispatchSpy = vi.spyOn(window, 'dispatchEvent'); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + await act(async () => {}); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_POLL_INTERVAL_MS); + }); + expect(mocks.getLoginStatus).toHaveBeenCalledTimes(2); + + unmount(); + await act(async () => { + statusDeferred.resolve({ isLoggedIn: true }); + await statusDeferred.promise; + await vi.advanceTimersByTimeAsync(LOGIN_TIMEOUT_MS + LOGIN_REDIRECT_DELAY_MS); + }); + + expect(mocks.trackAksFeature.mock.calls).toEqual([['aksd.auth-login', 'started']]); + expect(mocks.trackError).not.toHaveBeenCalled(); + expect(dispatchSpy).not.toHaveBeenCalled(); + expect(mocks.push).not.toHaveBeenCalled(); + expect(console.error).not.toHaveBeenCalled(); + }); + + test('unmount after success clears the pending redirect without later effects', async () => { + mocks.initiateLogin.mockResolvedValue({ success: true, message: 'browser opened' }); + const { unmount } = await renderLoggedOutPage(); + mocks.getLoginStatus.mockResolvedValueOnce({ isLoggedIn: true }); + const dispatchSpy = vi.spyOn(window, 'dispatchEvent'); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + await act(async () => {}); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_POLL_INTERVAL_MS); + }); + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.auth-login', 'started'], + ['aksd.auth-login', 'succeeded'], + ]); + expect(dispatchSpy).toHaveBeenCalledTimes(1); + + unmount(); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_REDIRECT_DELAY_MS); + }); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.auth-login', 'started'], + ['aksd.auth-login', 'succeeded'], + ]); + expect(dispatchSpy).toHaveBeenCalledTimes(1); + expect(mocks.push).not.toHaveBeenCalled(); + expect(console.error).not.toHaveBeenCalled(); + }); + + test('emits a privacy-safe failure when login initiation is unsuccessful', async () => { + mocks.initiateLogin.mockResolvedValue({ + success: false, + message: 'sensitive result message', + }); + await renderLoggedOutPage(); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + + await screen.findByText('sensitive result message'); + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'failed'); + expect(mocks.trackError).toHaveBeenCalledWith({ + area: 'auth-login', + errorClass: 'UnknownError', + phase: 'failed', + }); + expect( + JSON.stringify([mocks.trackAksFeature.mock.calls, mocks.trackError.mock.calls]) + ).not.toContain('sensitive result message'); + }); + + test('emits a privacy-safe failure when login initiation throws', async () => { + mocks.initiateLogin.mockRejectedValue(new Error('sensitive thrown exception')); + await renderLoggedOutPage(); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + + await screen.findByText('Failed to initiate login: sensitive thrown exception'); + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'failed'); + expect(mocks.trackError).toHaveBeenCalledWith({ + area: 'auth-login', + errorClass: 'UnknownError', + phase: 'failed', + }); + expect( + JSON.stringify([mocks.trackAksFeature.mock.calls, mocks.trackError.mock.calls]) + ).not.toContain('sensitive thrown exception'); + }); + + test('emits failed and TimeoutError when polling times out', async () => { + mocks.initiateLogin.mockResolvedValue({ success: true, message: 'browser opened' }); + await renderLoggedOutPage(); + mocks.getLoginStatus.mockResolvedValue({ isLoggedIn: false }); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + await act(async () => {}); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_TIMEOUT_MS); + }); + + expect(await screen.findByText('Login timeout. Please try again.')).toBeTruthy(); + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'failed'); + expect(mocks.trackError).toHaveBeenCalledWith({ + area: 'auth-login', + errorClass: 'TimeoutError', + phase: 'failed', + }); + }); + + test('does not emit terminal telemetry for the passive initial status check', async () => { + mocks.getLoginStatus.mockResolvedValueOnce({ + isLoggedIn: true, + username: 'sensitive-user@contoso.com', + }); + + render(); + await waitFor(() => expect(mocks.push).toHaveBeenCalledWith('/clusters')); + + expect(mocks.trackAksFeature).not.toHaveBeenCalled(); + expect(mocks.trackError).not.toHaveBeenCalled(); + }); + + test('does not emit terminal telemetry for a transient polling error', async () => { + mocks.initiateLogin.mockResolvedValue({ success: true, message: 'browser opened' }); + await renderLoggedOutPage(); + mocks.getLoginStatus.mockRejectedValueOnce(new Error('transient sensitive error')); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + await act(async () => {}); + await act(async () => { + await vi.advanceTimersByTimeAsync(LOGIN_POLL_INTERVAL_MS); + }); + + expect(mocks.trackAksFeature).toHaveBeenCalledTimes(1); + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'started'); + expect(mocks.trackError).not.toHaveBeenCalled(); + expect(screen.getByRole('button', { name: 'Cancel' })).toBeTruthy(); + }); + + test('emits cancelled only when cancelling an active login attempt', async () => { + mocks.initiateLogin.mockResolvedValue({ success: true, message: 'browser opened' }); + await renderLoggedOutPage(); + expect(mocks.trackAksFeature).not.toHaveBeenCalledWith('aksd.auth-login', 'cancelled'); + + fireEvent.click(screen.getByRole('button', { name: 'Sign in with Azure' })); + await screen.findByRole('button', { name: 'Cancel' }); + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-login', 'cancelled'); + expect(screen.getByRole('button', { name: 'Sign in with Azure' })).toBeTruthy(); + }); +}); diff --git a/plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.tsx b/plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.tsx index 7fdb9d5d8..9946fb99f 100644 --- a/plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.tsx +++ b/plugins/aks-desktop/src/components/AzureAuth/AzureLoginPage.tsx @@ -12,8 +12,11 @@ import { Container, Typography, } from '@mui/material'; -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import { useHistory, useLocation } from 'react-router-dom'; +import { useTelemetryFeatureOpened } from '../../hooks/useTelemetryFeatureOpened'; +import { trackError } from '../../telemetry'; +import { trackAksFeature } from '../../telemetry/aksFeature'; import { getLoginStatus, initiateLogin } from '../../utils/azure/az-auth'; import { LOGIN_POLL_INTERVAL_MS, @@ -25,7 +28,17 @@ interface AzureLoginPageProps { redirectTo?: string; } +type LoginAttemptOutcome = 'idle' | 'active' | 'succeeded' | 'failed' | 'cancelled'; + +function trackLoginFailure(errorClass: 'TimeoutError' | 'UnknownError') { + trackAksFeature('aksd.auth-login', 'failed'); + try { + trackError({ area: 'auth-login', errorClass, phase: 'failed' }); + } catch {} +} + export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { + useTelemetryFeatureOpened('aksd.auth-login'); const history = useHistory(); const { t } = useTranslation(); const location = useLocation(); @@ -33,7 +46,17 @@ export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { const [checking, setChecking] = useState(true); const [statusMessage, setStatusMessage] = useState(''); const [errorMessage, setErrorMessage] = useState(''); - const [pollingInterval, setPollingInterval] = useState(null); + const pollingIntervalRef = useRef | null>(null); + const redirectTimeoutRef = useRef | null>(null); + const loginAttemptOutcomeRef = useRef('idle'); + const loginAttemptGenerationRef = useRef(0); + const mountedRef = useRef(true); + + const isCurrentAttempt = (attemptGeneration: number) => + mountedRef.current && loginAttemptGenerationRef.current === attemptGeneration; + + const isActiveAttempt = (attemptGeneration: number) => + isCurrentAttempt(attemptGeneration) && loginAttemptOutcomeRef.current === 'active'; // Get redirect target from URL query parameter or prop, fallback to profile page const getRedirectTarget = () => { @@ -44,10 +67,18 @@ export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { // Check if already logged in on mount useEffect(() => { + mountedRef.current = true; checkLoginStatus(); return () => { - if (pollingInterval) { - clearInterval(pollingInterval); + mountedRef.current = false; + loginAttemptGenerationRef.current++; + if (pollingIntervalRef.current) { + clearInterval(pollingIntervalRef.current); + pollingIntervalRef.current = null; + } + if (redirectTimeoutRef.current) { + clearTimeout(redirectTimeoutRef.current); + redirectTimeoutRef.current = null; } }; }, []); @@ -55,6 +86,9 @@ export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { const checkLoginStatus = async () => { try { const status = await getLoginStatus(); + if (!mountedRef.current) { + return; + } if (status.isLoggedIn) { // Trigger update event for sidebar label window.dispatchEvent(new CustomEvent('azure-auth-update')); @@ -63,13 +97,20 @@ export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { history.push(target); } } catch (error) { - console.error('Error checking login status:', error); + if (mountedRef.current) { + console.error('Error checking login status:', error); + } } finally { - setChecking(false); + if (mountedRef.current) { + setChecking(false); + } } }; const handleLogin = async () => { + const attemptGeneration = ++loginAttemptGenerationRef.current; + loginAttemptOutcomeRef.current = 'active'; + trackAksFeature('aksd.auth-login', 'started'); setLoading(true); setErrorMessage(''); setStatusMessage(`${t('Initiating Azure login')}...`); @@ -77,7 +118,13 @@ export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { try { const result = await initiateLogin(); + if (!isActiveAttempt(attemptGeneration)) { + return; + } + if (!result.success) { + loginAttemptOutcomeRef.current = 'failed'; + trackLoginFailure('UnknownError'); setErrorMessage(result.message); setLoading(false); return; @@ -94,25 +141,45 @@ export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { const maxPolls = Math.ceil(LOGIN_TIMEOUT_MS / LOGIN_POLL_INTERVAL_MS); const interval = setInterval(async () => { + if (!isActiveAttempt(attemptGeneration)) { + clearInterval(interval); + return; + } pollCount++; try { const status = await getLoginStatus(); + if (!isActiveAttempt(attemptGeneration)) { + clearInterval(interval); + return; + } + if (status.isLoggedIn) { clearInterval(interval); + pollingIntervalRef.current = null; + loginAttemptOutcomeRef.current = 'succeeded'; + trackAksFeature('aksd.auth-login', 'succeeded'); setStatusMessage(`${t('Login successful! Redirecting')}...`); // Trigger update event for sidebar label window.dispatchEvent(new CustomEvent('azure-auth-update')); // Wait a moment before redirecting - setTimeout(() => { - const target = getRedirectTarget(); - history.push(target); + redirectTimeoutRef.current = setTimeout(() => { + if ( + isCurrentAttempt(attemptGeneration) && + loginAttemptOutcomeRef.current === 'succeeded' + ) { + const target = getRedirectTarget(); + history.push(target); + } }, LOGIN_REDIRECT_DELAY_MS); } else if (pollCount >= maxPolls) { clearInterval(interval); + pollingIntervalRef.current = null; + loginAttemptOutcomeRef.current = 'failed'; + trackLoginFailure('TimeoutError'); setErrorMessage(t('Login timeout. Please try again.')); setLoading(false); } else { @@ -124,13 +191,22 @@ export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { ); } } catch (error) { + if (!isActiveAttempt(attemptGeneration)) { + clearInterval(interval); + return; + } console.error('Error polling login status:', error); } }, LOGIN_POLL_INTERVAL_MS); - setPollingInterval(interval); + pollingIntervalRef.current = interval; } catch (error) { + if (!isActiveAttempt(attemptGeneration)) { + return; + } console.error('Error initiating login:', error); + loginAttemptOutcomeRef.current = 'failed'; + trackLoginFailure('UnknownError'); setErrorMessage( t('Failed to initiate login: {{message}}', { message: error instanceof Error ? error.message : t('Unknown error'), @@ -141,8 +217,15 @@ export default function AzureLoginPage({ redirectTo }: AzureLoginPageProps) { }; const handleCancel = () => { - if (pollingInterval) { - clearInterval(pollingInterval); + if (loginAttemptOutcomeRef.current !== 'active') { + return; + } + loginAttemptGenerationRef.current++; + loginAttemptOutcomeRef.current = 'cancelled'; + trackAksFeature('aksd.auth-login', 'cancelled'); + if (pollingIntervalRef.current) { + clearInterval(pollingIntervalRef.current); + pollingIntervalRef.current = null; } setLoading(false); setStatusMessage(''); diff --git a/plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.test.ts b/plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.test.ts index f0cec3aee..9864d4177 100644 --- a/plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.test.ts +++ b/plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.test.ts @@ -3,12 +3,24 @@ // @vitest-environment jsdom -import { act, renderHook } from '@testing-library/react'; +import { act, renderHook, waitFor } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; const mockPush = vi.hoisted(() => vi.fn()); const mockUseAzureAuth = vi.hoisted(() => vi.fn()); const mockRunCommandAsync = vi.hoisted(() => vi.fn()); +const azureCliImportControl = vi.hoisted(() => { + let resolveImport!: () => void; + const importPromise = new Promise(resolve => { + resolveImport = resolve; + }); + + return { deferImport: false, importPromise, resolveImport }; +}); +const telemetryMocks = vi.hoisted(() => ({ + trackAksFeature: vi.fn(), + trackError: vi.fn(), +})); vi.mock('react-router-dom', () => ({ useHistory: () => ({ push: mockPush }), @@ -19,9 +31,23 @@ vi.mock('../../../hooks/useAzureAuth', () => ({ })); // Covers the dynamic import inside handleLogout. -vi.mock('../../../utils/azure/az-cli-core', () => ({ - runCommandAsync: mockRunCommandAsync, - isAzError: (stderr: string) => stderr.includes('ERROR:'), +vi.mock('../../../utils/azure/az-cli-core', async () => { + if (azureCliImportControl.deferImport) { + await azureCliImportControl.importPromise; + } + + return { + runCommandAsync: mockRunCommandAsync, + isAzError: (stderr: string) => stderr.includes('ERROR:'), + }; +}); + +vi.mock('../../../telemetry/aksFeature', () => ({ + trackAksFeature: telemetryMocks.trackAksFeature, +})); + +vi.mock('../../../telemetry', () => ({ + trackError: telemetryMocks.trackError, })); vi.mock('@kinvolk/headlamp-plugin/lib', () => ({ @@ -31,6 +57,14 @@ vi.mock('@kinvolk/headlamp-plugin/lib', () => ({ // Import after mocks are in place import { useAzureProfilePage } from './useAzureProfilePage'; +function createDeferred() { + let resolve!: (value: T) => void; + const promise = new Promise(promiseResolve => { + resolve = promiseResolve; + }); + return { promise, resolve }; +} + const AUTH_LOGGED_IN = { isChecking: false, isLoggedIn: true, @@ -106,6 +140,43 @@ describe('useAzureProfilePage', () => { expect(mockPush).toHaveBeenCalledWith('/add-cluster-aks'); }); + test('unmount while loading logout command still executes it without later effects', async () => { + const commandDeferred = createDeferred<{ stderr: string }>(); + azureCliImportControl.deferImport = true; + mockRunCommandAsync.mockReturnValue(commandDeferred.promise); + const dispatchSpy = vi.spyOn(window, 'dispatchEvent'); + const { result, unmount } = renderHook(() => useAzureProfilePage()); + let logoutPromise!: Promise; + + act(() => { + logoutPromise = result.current.handleLogout(); + }); + expect(result.current.loggingOut).toBe(true); + expect(telemetryMocks.trackAksFeature.mock.calls).toEqual([['aksd.auth-logout', 'started']]); + expect(mockRunCommandAsync).not.toHaveBeenCalled(); + + unmount(); + await act(async () => { + azureCliImportControl.resolveImport(); + await Promise.resolve(); + }); + + expect(mockRunCommandAsync).toHaveBeenCalledWith('az', ['logout']); + + await act(async () => { + commandDeferred.resolve({ stderr: '' }); + await logoutPromise; + }); + + expect(vi.getTimerCount()).toBe(0); + act(() => vi.runAllTimers()); + expect(telemetryMocks.trackAksFeature.mock.calls).toEqual([['aksd.auth-logout', 'started']]); + expect(telemetryMocks.trackError).not.toHaveBeenCalled(); + expect(dispatchSpy).not.toHaveBeenCalled(); + expect(mockPush).not.toHaveBeenCalled(); + expect(console.error).not.toHaveBeenCalled(); + }); + test('handleLogout dispatches azure-auth-update and redirects on success', async () => { mockRunCommandAsync.mockResolvedValue({ stderr: '' }); const dispatchSpy = vi.spyOn(window, 'dispatchEvent'); @@ -113,6 +184,17 @@ describe('useAzureProfilePage', () => { const { result } = renderHook(() => useAzureProfilePage()); await act(() => result.current.handleLogout()); + expect(telemetryMocks.trackAksFeature).toHaveBeenNthCalledWith( + 1, + 'aksd.auth-logout', + 'started' + ); + expect(telemetryMocks.trackAksFeature).toHaveBeenNthCalledWith( + 2, + 'aksd.auth-logout', + 'succeeded' + ); + expect(telemetryMocks.trackError).not.toHaveBeenCalled(); expect(dispatchSpy).toHaveBeenCalledWith( expect.objectContaining({ type: 'azure-auth-update' }) ); @@ -142,7 +224,7 @@ describe('useAzureProfilePage', () => { }); test('handleLogout does not redirect when stderr contains ERROR:', async () => { - mockRunCommandAsync.mockResolvedValue({ stderr: 'ERROR: not logged in' }); + mockRunCommandAsync.mockResolvedValue({ stderr: 'ERROR: sensitive logout result' }); const { result } = renderHook(() => useAzureProfilePage()); await act(() => result.current.handleLogout()); @@ -150,15 +232,66 @@ describe('useAzureProfilePage', () => { act(() => vi.runAllTimers()); expect(mockPush).not.toHaveBeenCalled(); expect(result.current.loggingOut).toBe(false); + expect(telemetryMocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-logout', 'failed'); + expect(telemetryMocks.trackError).toHaveBeenCalledWith({ + area: 'auth-logout', + errorClass: 'UnknownError', + phase: 'failed', + }); + expect( + JSON.stringify([ + telemetryMocks.trackAksFeature.mock.calls, + telemetryMocks.trackError.mock.calls, + ]) + ).not.toContain('sensitive logout result'); }); test('handleLogout sets loggingOut false when an error is thrown', async () => { - mockRunCommandAsync.mockRejectedValue(new Error('CLI not found')); + mockRunCommandAsync.mockRejectedValue(new Error('sensitive thrown logout error')); const { result } = renderHook(() => useAzureProfilePage()); await act(() => result.current.handleLogout()); expect(result.current.loggingOut).toBe(false); + expect(telemetryMocks.trackAksFeature).toHaveBeenCalledWith('aksd.auth-logout', 'failed'); + expect(telemetryMocks.trackError).toHaveBeenCalledWith({ + area: 'auth-logout', + errorClass: 'UnknownError', + phase: 'failed', + }); + expect( + JSON.stringify([ + telemetryMocks.trackAksFeature.mock.calls, + telemetryMocks.trackError.mock.calls, + ]) + ).not.toContain('sensitive thrown logout error'); + }); + + test('emits started before invoking the logout command', async () => { + mockRunCommandAsync.mockResolvedValue({ stderr: '' }); + + const { result } = renderHook(() => useAzureProfilePage()); + await act(() => result.current.handleLogout()); + + const startedCall = telemetryMocks.trackAksFeature.mock.calls.findIndex( + call => call[0] === 'aksd.auth-logout' && call[1] === 'started' + ); + expect(telemetryMocks.trackAksFeature.mock.invocationCallOrder[startedCall]).toBeLessThan( + mockRunCommandAsync.mock.invocationCallOrder[0] + ); + }); + + test('does not emit opened or cancelled events for logout', async () => { + mockRunCommandAsync.mockResolvedValue({ stderr: '' }); + + const { result } = renderHook(() => useAzureProfilePage()); + await act(() => result.current.handleLogout()); + + expect(telemetryMocks.trackAksFeature).not.toHaveBeenCalledWith('aksd.auth-logout', 'opened'); + expect(telemetryMocks.trackAksFeature).not.toHaveBeenCalledWith( + 'aksd.auth-logout', + 'cancelled' + ); }); test('clears redirect timer on unmount to prevent stray navigation', async () => { @@ -171,4 +304,32 @@ describe('useAzureProfilePage', () => { act(() => vi.runAllTimers()); expect(mockPush).not.toHaveBeenCalledWith('/azure/login'); }); + + test('unmount during logout command prevents all later effects', async () => { + const commandDeferred = createDeferred<{ stderr: string }>(); + mockRunCommandAsync.mockReturnValue(commandDeferred.promise); + const dispatchSpy = vi.spyOn(window, 'dispatchEvent'); + const { result, unmount } = renderHook(() => useAzureProfilePage()); + let logoutPromise!: Promise; + + act(() => { + logoutPromise = result.current.handleLogout(); + }); + await waitFor(() => expect(mockRunCommandAsync).toHaveBeenCalled()); + expect(telemetryMocks.trackAksFeature.mock.calls).toEqual([['aksd.auth-logout', 'started']]); + + unmount(); + await act(async () => { + commandDeferred.resolve({ stderr: 'ERROR: sensitive logout result' }); + await logoutPromise; + }); + + expect(vi.getTimerCount()).toBe(0); + act(() => vi.runAllTimers()); + expect(telemetryMocks.trackAksFeature.mock.calls).toEqual([['aksd.auth-logout', 'started']]); + expect(telemetryMocks.trackError).not.toHaveBeenCalled(); + expect(dispatchSpy).not.toHaveBeenCalled(); + expect(mockPush).not.toHaveBeenCalled(); + expect(console.error).not.toHaveBeenCalled(); + }); }); diff --git a/plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.ts b/plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.ts index 88a5073ec..e472f8831 100644 --- a/plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.ts +++ b/plugins/aks-desktop/src/components/AzureAuth/hooks/useAzureProfilePage.ts @@ -4,8 +4,17 @@ import { useEffect, useRef, useState } from 'react'; import { useHistory } from 'react-router-dom'; import { useAzureAuth } from '../../../hooks/useAzureAuth'; +import { trackError } from '../../../telemetry'; +import { trackAksFeature } from '../../../telemetry/aksFeature'; import { PROFILE_REDIRECT_DELAY_MS } from '../../../utils/constants/timing'; +function trackLogoutFailure() { + trackAksFeature('aksd.auth-logout', 'failed'); + try { + trackError({ area: 'auth-logout', errorClass: 'UnknownError', phase: 'failed' }); + } catch {} +} + /** * Return type for {@link useAzureProfilePage}. */ @@ -48,11 +57,20 @@ export function useAzureProfilePage(): UseAzureProfilePageResult { const authStatus = useAzureAuth(); const [loggingOut, setLoggingOut] = useState(false); const redirectTimerRef = useRef | null>(null); + const mountedRef = useRef(true); + const logoutAttemptGenerationRef = useRef(0); + + const isCurrentLogoutAttempt = (attemptGeneration: number) => + mountedRef.current && logoutAttemptGenerationRef.current === attemptGeneration; useEffect(() => { + mountedRef.current = true; return () => { + mountedRef.current = false; + logoutAttemptGenerationRef.current++; if (redirectTimerRef.current) { clearTimeout(redirectTimerRef.current); + redirectTimerRef.current = null; } }; }, []); @@ -74,27 +92,41 @@ export function useAzureProfilePage(): UseAzureProfilePageResult { }; const handleLogout = async () => { + const attemptGeneration = ++logoutAttemptGenerationRef.current; + trackAksFeature('aksd.auth-logout', 'started'); setLoggingOut(true); try { // Dynamic import avoids circular dependencies at module load time. const { runCommandAsync, isAzError } = await import('../../../utils/azure/az-cli-core'); const result = await runCommandAsync('az', ['logout']); + if (!isCurrentLogoutAttempt(attemptGeneration)) { + return; + } if (result.stderr && isAzError(result.stderr)) { console.error('Azure CLI logout error:', result.stderr); + trackLogoutFailure(); setLoggingOut(false); return; } + trackAksFeature('aksd.auth-logout', 'succeeded'); + // Notify the sidebar label to refresh its auth state. window.dispatchEvent(new CustomEvent('azure-auth-update')); // Stay in loggingOut=true state until the component unmounts on redirect. redirectTimerRef.current = setTimeout(() => { - history.push('/azure/login'); + if (isCurrentLogoutAttempt(attemptGeneration)) { + history.push('/azure/login'); + } }, PROFILE_REDIRECT_DELAY_MS); } catch (error) { + if (!isCurrentLogoutAttempt(attemptGeneration)) { + return; + } console.error('Error logging out:', error); + trackLogoutFailure(); setLoggingOut(false); } }; diff --git a/plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.test.tsx b/plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.test.tsx new file mode 100644 index 000000000..25b88b87c --- /dev/null +++ b/plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.test.tsx @@ -0,0 +1,356 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +// @vitest-environment jsdom + +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + createNamespaceAsProject: vi.fn(), + getClusterSettings: vi.fn(), + push: vi.fn(), + setClusterSettings: vi.fn(), + trackAksFeature: vi.fn(), + trackError: vi.fn(), + useTelemetryFeatureOpened: vi.fn(), +})); + +vi.mock('@iconify/react', () => ({ Icon: () => null })); + +vi.mock('@kinvolk/headlamp-plugin/lib', () => ({ + K8s: { + useClustersConf: () => ({ 'private-cluster': {} }), + }, + useTranslation: () => ({ + t: (key: string, values?: Record) => + values + ? Object.entries(values).reduce( + (message, [name, value]) => message.replace(`{{${name}}}`, value), + key + ) + : key, + }), +})); + +vi.mock('@kinvolk/headlamp-plugin/lib/CommonComponents', () => ({ + PageGrid: ({ children }: React.PropsWithChildren) => <>{children}, + SectionBox: ({ + backLink, + children, + }: React.PropsWithChildren<{ backLink?: string | boolean }>) => ( + <> + {typeof backLink === 'string' ? ( + + ) : null} + {children} + + ), +})); + +vi.mock('react-router-dom', () => ({ + useHistory: () => ({ push: mocks.push }), +})); + +vi.mock('../../hooks/useTelemetryFeatureOpened', () => ({ + useTelemetryFeatureOpened: mocks.useTelemetryFeatureOpened, +})); + +vi.mock('../../telemetry/aksFeature', () => ({ + trackAksFeature: mocks.trackAksFeature, +})); + +vi.mock('../../telemetry', () => ({ + trackError: mocks.trackError, +})); + +vi.mock('../../utils/kubernetes/namespaceUtils', () => ({ + createNamespaceAsProject: mocks.createNamespaceAsProject, +})); + +vi.mock('../../utils/shared/clusterSettings', () => ({ + getClusterSettings: mocks.getClusterSettings, + setClusterSettings: mocks.setClusterSettings, +})); + +vi.mock('../CreateAKSProject/components/Breadcrumb', () => ({ + Breadcrumb: () => null, +})); + +vi.mock('../CreateAKSProject/components/SearchableSelect', () => ({ + SearchableSelect: ({ + label, + onChange, + options, + value, + }: { + label: string; + onChange: (value: string) => void; + options: Array<{ label: string; value: string }>; + value: string; + }) => ( + + ), +})); + +vi.mock('../shared/FormField', () => ({ + FormField: ({ + label, + onChange, + value, + }: { + label: string; + onChange: (value: string) => void; + value: string; + }) => onChange(event.target.value)} value={value} />, +})); + +import CreateNamespace from './CreateNamespace'; + +function createDeferred() { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((promiseResolve, promiseReject) => { + resolve = promiseResolve; + reject = promiseReject; + }); + return { promise, reject, resolve }; +} + +function completeBasics() { + fireEvent.change(screen.getByLabelText('Cluster'), { + target: { value: 'private-cluster' }, + }); + fireEvent.change(screen.getByLabelText('Namespace Name'), { + target: { value: 'private-namespace' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Next' })); +} + +function submitNamespace() { + fireEvent.click(screen.getByRole('button', { name: 'Create Namespace' })); +} + +function goBackOneWizardStep() { + const backButtons = screen.getAllByRole('button', { name: 'Back' }); + fireEvent.click(backButtons[backButtons.length - 1]); +} + +function clickHeaderBack() { + fireEvent.click(screen.getAllByRole('button', { name: 'Back' })[0]); +} + +describe('CreateNamespace telemetry', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers({ shouldAdvanceTime: true }); + vi.spyOn(console, 'error').mockImplementation(() => {}); + mocks.getClusterSettings.mockReturnValue({ allowedNamespaces: ['existing-namespace'] }); + }); + + afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + test('records the namespace creation surface opening through the shared hook', () => { + render(); + + expect(mocks.useTelemetryFeatureOpened).toHaveBeenCalledTimes(1); + expect(mocks.useTelemetryFeatureOpened).toHaveBeenCalledWith('aksd.namespace-create'); + }); + + test('emits started and succeeded after namespace and settings updates complete', async () => { + const deferred = createDeferred(); + mocks.createNamespaceAsProject.mockReturnValue(deferred.promise); + render(); + completeBasics(); + + submitNamespace(); + + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'started'); + expect(mocks.trackAksFeature.mock.invocationCallOrder[0]).toBeLessThan( + mocks.createNamespaceAsProject.mock.invocationCallOrder[0] + ); + + await act(async () => { + deferred.resolve(); + await deferred.promise; + }); + + await waitFor(() => { + expect(mocks.setClusterSettings).toHaveBeenCalledTimes(1); + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.namespace-create', 'started'], + ['aksd.namespace-create', 'succeeded'], + ]); + }); + expect(mocks.setClusterSettings.mock.invocationCallOrder[0]).toBeLessThan( + mocks.trackAksFeature.mock.invocationCallOrder[1] + ); + expect(JSON.stringify(mocks.trackAksFeature.mock.calls)).not.toContain('private-'); + }); + + test('emits failed and a categorical UnknownError without transmitting the error message', async () => { + mocks.createNamespaceAsProject.mockRejectedValue( + new Error('private-namespace failed on private-cluster') + ); + render(); + completeBasics(); + + submitNamespace(); + + await waitFor(() => { + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.namespace-create', 'started'], + ['aksd.namespace-create', 'failed'], + ]); + expect(mocks.trackError).toHaveBeenCalledWith({ + area: 'namespace-create', + errorClass: 'UnknownError', + phase: 'failed', + }); + }); + expect(JSON.stringify(mocks.trackError.mock.calls)).not.toContain('private-'); + }); + + test('emits cancelled from the explicit idle cancel action', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(mocks.trackAksFeature.mock.calls).toEqual([['aksd.namespace-create', 'cancelled']]); + expect(mocks.push).toHaveBeenCalledWith('/'); + }); + + test('emits cancelled from the accessible header back-to-home control', () => { + render(); + + clickHeaderBack(); + + expect(mocks.trackAksFeature.mock.calls).toEqual([['aksd.namespace-create', 'cancelled']]); + expect(mocks.push.mock.calls).toEqual([['/']]); + }); + + test('does not instrument wizard Back between visible steps', () => { + render(); + completeBasics(); + + goBackOneWizardStep(); + + expect(mocks.trackAksFeature).not.toHaveBeenCalled(); + }); + + test('does not emit cancelled from the reachable error-overlay Cancel after failure', async () => { + mocks.createNamespaceAsProject.mockRejectedValue(new Error('private failure')); + render(); + completeBasics(); + submitNamespace(); + await waitFor(() => + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'failed') + ); + + fireEvent.click(screen.getAllByRole('button', { name: 'Cancel' })[0]); + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.namespace-create', 'started'], + ['aksd.namespace-create', 'failed'], + ]); + }); + + test('does not emit cancelled from back-to-home after success', async () => { + mocks.createNamespaceAsProject.mockResolvedValue(undefined); + render(); + completeBasics(); + submitNamespace(); + await waitFor(() => + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'succeeded') + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(1500); + }); + fireEvent.click(screen.getByRole('button', { name: 'Go To Projects' })); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.namespace-create', 'started'], + ['aksd.namespace-create', 'succeeded'], + ]); + }); + + test('cancels an active attempt from the reachable header Back and ignores late completion', async () => { + const attempt = createDeferred(); + mocks.createNamespaceAsProject.mockReturnValue(attempt.promise); + render(); + completeBasics(); + submitNamespace(); + clickHeaderBack(); + await act(async () => { + attempt.resolve(); + await attempt.promise; + }); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.namespace-create', 'started'], + ['aksd.namespace-create', 'cancelled'], + ]); + expect(mocks.push.mock.calls).toEqual([['/']]); + expect(mocks.setClusterSettings).not.toHaveBeenCalled(); + }); + + test('cancels a retry after failure and ignores its late completion', async () => { + const retry = createDeferred(); + mocks.createNamespaceAsProject + .mockRejectedValueOnce(new Error('private failure')) + .mockReturnValueOnce(retry.promise); + render(); + completeBasics(); + submitNamespace(); + await waitFor(() => + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'failed') + ); + + submitNamespace(); + clickHeaderBack(); + await act(async () => { + retry.resolve(); + await retry.promise; + }); + + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.namespace-create', 'started'], + ['aksd.namespace-create', 'failed'], + ['aksd.namespace-create', 'started'], + ['aksd.namespace-create', 'cancelled'], + ]); + expect(mocks.push.mock.calls).toEqual([['/']]); + expect(mocks.setClusterSettings).not.toHaveBeenCalled(); + }); + + test('clears the success-dialog timer on unmount', async () => { + const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout'); + const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout'); + mocks.createNamespaceAsProject.mockResolvedValue(undefined); + const rendered = render(); + completeBasics(); + submitNamespace(); + await waitFor(() => + expect(mocks.trackAksFeature).toHaveBeenCalledWith('aksd.namespace-create', 'succeeded') + ); + const successTimerIndex = setTimeoutSpy.mock.calls.findIndex(([, delay]) => delay === 1500); + const successTimer = setTimeoutSpy.mock.results[successTimerIndex]?.value; + + rendered.unmount(); + + expect(successTimer).toBeDefined(); + expect(clearTimeoutSpy.mock.calls.some(([timer]) => timer === successTimer)).toBe(true); + }); +}); diff --git a/plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.tsx b/plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.tsx index fa8e4d960..42b7de010 100644 --- a/plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.tsx +++ b/plugins/aks-desktop/src/components/CreateNamespace/CreateNamespace.tsx @@ -17,6 +17,9 @@ import { } from '@mui/material'; import React, { useEffect, useMemo, useRef, useState } from 'react'; import { useHistory } from 'react-router-dom'; +import { useTelemetryFeatureOpened } from '../../hooks/useTelemetryFeatureOpened'; +import { trackError } from '../../telemetry'; +import { trackAksFeature } from '../../telemetry/aksFeature'; import { createNamespaceAsProject } from '../../utils/kubernetes/namespaceUtils'; import { getClusterSettings, setClusterSettings } from '../../utils/shared/clusterSettings'; import { Breadcrumb } from '../CreateAKSProject/components/Breadcrumb'; @@ -45,6 +48,48 @@ function getStepLabel(t: (key: string) => string, step: NamespaceStepName): stri const NAMESPACE_NAME_REGEX = /^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$/; +type NamespaceCreationAttemptOutcome = 'active' | 'cancelled' | 'failed' | 'succeeded'; + +interface NamespaceCreationAttemptState { + cancel: () => boolean; + finish: ( + generation: number, + outcome: Extract + ) => boolean; + invalidate: () => void; + is: (generation: number, outcome: NamespaceCreationAttemptOutcome) => boolean; + start: () => number; +} + +function createNamespaceCreationAttemptState(): NamespaceCreationAttemptState { + let generation = 0; + let outcome: NamespaceCreationAttemptOutcome = 'active'; + + return { + cancel: () => { + if (outcome !== 'active') return false; + outcome = 'cancelled'; + generation += 1; + return true; + }, + finish: (attemptGeneration, attemptOutcome) => { + if (generation !== attemptGeneration || outcome !== 'active') return false; + outcome = attemptOutcome; + return true; + }, + invalidate: () => { + generation += 1; + }, + is: (attemptGeneration, attemptOutcome) => + generation === attemptGeneration && outcome === attemptOutcome, + start: () => { + generation += 1; + outcome = 'active'; + return generation; + }, + }; +} + function CreateNamespaceContent() { const history = useHistory(); const { t } = useTranslation(); @@ -59,6 +104,29 @@ function CreateNamespaceContent() { const [showSuccessDialog, setShowSuccessDialog] = useState(false); const [applicationName, setApplicationName] = useState(''); const stepContentRef = useRef(null); + const attemptStateRef = useRef(null); + const isMountedRef = useRef(true); + const submissionInFlightRef = useRef(false); + const successTimerRef = useRef | null>(null); + + if (attemptStateRef.current === null) { + attemptStateRef.current = createNamespaceCreationAttemptState(); + } + const attemptState = attemptStateRef.current; + + useTelemetryFeatureOpened('aksd.namespace-create'); + + useEffect(() => { + isMountedRef.current = true; + return () => { + isMountedRef.current = false; + attemptState.invalidate(); + if (successTimerRef.current !== null) { + clearTimeout(successTimerRef.current); + successTimerRef.current = null; + } + }; + }, [attemptState]); // Focus the first form input when the active step changes. // Skip if the user already has focus inside the step content. @@ -110,12 +178,26 @@ function CreateNamespaceContent() { }, [selectedCluster, namespaceName]); const handleSubmit = async () => { + if (!validation.isValid || submissionInFlightRef.current) return; + + const attemptGeneration = attemptState.start(); + submissionInFlightRef.current = true; + if (successTimerRef.current !== null) { + clearTimeout(successTimerRef.current); + successTimerRef.current = null; + } + trackAksFeature('aksd.namespace-create', 'started'); + + const isCurrentActiveAttempt = () => + isMountedRef.current && attemptState.is(attemptGeneration, 'active'); + try { setIsCreating(true); setCreationError(null); setCreationProgress(`${t('Creating namespace')}...`); await createNamespaceAsProject(namespaceName, selectedCluster); + if (!isCurrentActiveAttempt()) return; setCreationProgress(`${t('Updating local settings')}...`); // Only append to allowedNamespaces if it's already configured. Appending @@ -127,12 +209,22 @@ function CreateNamespaceContent() { setClusterSettings(selectedCluster, settings); } + if (!isMountedRef.current || !attemptState.finish(attemptGeneration, 'succeeded')) return; + trackAksFeature('aksd.namespace-create', 'succeeded'); setCreationProgress(t('Namespace created successfully!')); - setTimeout(() => { + successTimerRef.current = setTimeout(() => { + successTimerRef.current = null; + if (!isMountedRef.current || !attemptState.is(attemptGeneration, 'succeeded')) { + return; + } setShowSuccessDialog(true); setIsCreating(false); }, 1500); } catch (error) { + if (!isMountedRef.current || !attemptState.finish(attemptGeneration, 'failed')) return; + submissionInFlightRef.current = false; + trackAksFeature('aksd.namespace-create', 'failed'); + trackError({ area: 'namespace-create', errorClass: 'UnknownError', phase: 'failed' }); console.error('Error creating namespace:', error); setCreationError(error instanceof Error ? error.message : t('Failed to create namespace')); setIsCreating(false); @@ -141,6 +233,14 @@ function CreateNamespaceContent() { }; const onBack = () => { + if (attemptState.cancel()) { + submissionInFlightRef.current = false; + if (successTimerRef.current !== null) { + clearTimeout(successTimerRef.current); + successTimerRef.current = null; + } + trackAksFeature('aksd.namespace-create', 'cancelled'); + } history.push('/'); }; @@ -230,10 +330,18 @@ function CreateNamespaceContent() { return ( + {/* Loading / Success / Error Overlay */} diff --git a/plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.test.tsx b/plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.test.tsx new file mode 100644 index 000000000..006136fde --- /dev/null +++ b/plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.test.tsx @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +// @vitest-environment jsdom + +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import React from 'react'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + handleDelete: vi.fn(), + trackAksFeature: vi.fn(), +})); + +vi.mock('@iconify/react', () => ({ + Icon: () => null, +})); + +vi.mock('@kinvolk/headlamp-plugin/lib', () => ({ + useTranslation: () => ({ t: (message: string) => message }), +})); + +vi.mock('./hooks/useProjectDeletion', () => ({ + useProjectDeletion: () => ({ handleDelete: mocks.handleDelete }), +})); + +vi.mock('./hooks/useProjectPermissions', () => ({ + useProjectPermissions: () => ({ canDelete: true, isLoading: false }), +})); + +vi.mock('../../telemetry/aksFeature', () => ({ + trackAksFeature: mocks.trackAksFeature, +})); + +vi.mock('./components/AKSProjectDeleteDialog', () => ({ + AKSProjectDeleteDialog: ({ + onClose, + onDelete, + open, + }: { + onClose: () => void; + onDelete: () => void; + open: boolean; + }) => + open ? ( +
+ + +
+ ) : null, +})); + +import AKSProjectDeleteButton from './AKSProjectDeleteButton'; + +const project = { + clusters: ['sensitive-cluster'], + id: 'sensitive-project', + namespaces: ['sensitive-namespace'], +}; + +describe('AKSProjectDeleteButton telemetry', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.handleDelete.mockImplementation((_project, _deleteNamespaces, onClose) => onClose()); + }); + + afterEach(() => { + cleanup(); + }); + + test('tracks opened and explicit pre-confirm cancellation for each interaction', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Delete project' })); + expect(mocks.trackAksFeature).toHaveBeenLastCalledWith('aksd.project-delete', 'opened'); + + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); + expect(mocks.trackAksFeature).toHaveBeenLastCalledWith('aksd.project-delete', 'cancelled'); + + fireEvent.click(screen.getByRole('button', { name: 'Delete project' })); + expect(mocks.trackAksFeature.mock.calls).toEqual([ + ['aksd.project-delete', 'opened'], + ['aksd.project-delete', 'cancelled'], + ['aksd.project-delete', 'opened'], + ]); + }); + + test('confirmation closes without tracking cancellation', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Delete project' })); + fireEvent.click(screen.getByRole('button', { name: 'Confirm' })); + + expect(mocks.handleDelete).toHaveBeenCalledWith(project, false, expect.any(Function)); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(mocks.trackAksFeature.mock.calls).toEqual([['aksd.project-delete', 'opened']]); + }); +}); diff --git a/plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.tsx b/plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.tsx index e2ca0c064..62cbd5366 100644 --- a/plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.tsx +++ b/plugins/aks-desktop/src/components/DeleteAKSProject/AKSProjectDeleteButton.tsx @@ -4,6 +4,7 @@ import { Icon } from '@iconify/react'; import { useTranslation } from '@kinvolk/headlamp-plugin/lib'; import { IconButton, Tooltip } from '@mui/material'; import React, { useState } from 'react'; +import { trackAksFeature } from '../../telemetry/aksFeature'; import { AKSProjectDeleteDialog } from './components/AKSProjectDeleteDialog'; import { useProjectDeletion } from './hooks/useProjectDeletion'; import { useProjectPermissions } from './hooks/useProjectPermissions'; @@ -26,6 +27,16 @@ const AKSProjectDeleteButton: React.FC = ({ project const { isLoading, canDelete } = useProjectPermissions(project); const { handleDelete } = useProjectDeletion(); + const handleOpen = () => { + trackAksFeature('aksd.project-delete', 'opened'); + setOpen(true); + }; + + const handleCancel = () => { + trackAksFeature('aksd.project-delete', 'cancelled'); + setOpen(false); + }; + if (isLoading) { return ( @@ -43,14 +54,14 @@ const AKSProjectDeleteButton: React.FC = ({ project return ( <> - setOpen(true)} size="medium"> + setOpen(false)} + onClose={handleCancel} project={project} deleteNamespaces={deleteNamespaces} setDeleteNamespaces={setDeleteNamespaces} diff --git a/plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.test.ts b/plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.test.ts index d4e6915c3..99c7fde87 100644 --- a/plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.test.ts +++ b/plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.test.ts @@ -9,6 +9,8 @@ const mockApiGet = vi.hoisted(() => vi.fn()); const mockApiEndpointDelete = vi.hoisted(() => vi.fn()); const mockApiEndpointPut = vi.hoisted(() => vi.fn()); const mockDeleteManagedNamespace = vi.hoisted(() => vi.fn()); +const mockTrackAksFeature = vi.hoisted(() => vi.fn()); +const mockTrackError = vi.hoisted(() => vi.fn()); vi.mock('@kinvolk/headlamp-plugin/lib', () => ({ clusterAction: mockClusterAction, @@ -30,6 +32,14 @@ vi.mock('../../../utils/azure/az-namespaces', () => ({ deleteManagedNamespace: mockDeleteManagedNamespace, })); +vi.mock('../../../telemetry/aksFeature', () => ({ + trackAksFeature: mockTrackAksFeature, +})); + +vi.mock('../../../telemetry', () => ({ + trackError: mockTrackError, +})); + import { useProjectDeletion } from './useProjectDeletion'; const baseProject = { @@ -96,6 +106,58 @@ describe('useProjectDeletion', () => { }) ); expect(onClose).toHaveBeenCalled(); + expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.project-delete', 'started'); + expect(mockTrackAksFeature.mock.invocationCallOrder[0]).toBeLessThan( + mockClusterAction.mock.invocationCallOrder[0] + ); + }); + + test('tracks success only after all deletion work completes', async () => { + const ns = makeMockNs(regularLabels); + let resolveDelete!: () => void; + ns.delete.mockImplementation( + () => + new Promise(resolve => { + resolveDelete = resolve; + }) + ); + setupApiGet(ns); + + const { result } = renderHook(() => useProjectDeletion()); + result.current.handleDelete(baseProject, true, vi.fn()); + + const actionPromise = executeClusterAction(); + await vi.waitFor(() => expect(ns.delete).toHaveBeenCalled()); + expect(mockTrackAksFeature).not.toHaveBeenCalledWith('aksd.project-delete', 'succeeded'); + + resolveDelete(); + await actionPromise; + + expect(mockTrackAksFeature.mock.calls).toEqual([ + ['aksd.project-delete', 'started'], + ['aksd.project-delete', 'succeeded'], + ]); + }); + + test('tracks categorical failure and rethrows the original error', async () => { + const originalError = new Error('sensitive failure details'); + const ns = makeMockNs(regularLabels); + ns.delete.mockRejectedValue(originalError); + setupApiGet(ns); + + const { result } = renderHook(() => useProjectDeletion()); + result.current.handleDelete(baseProject, true, vi.fn()); + + await expect(executeClusterAction()).rejects.toBe(originalError); + expect(mockTrackAksFeature.mock.calls).toEqual([ + ['aksd.project-delete', 'started'], + ['aksd.project-delete', 'failed'], + ]); + expect(mockTrackError).toHaveBeenCalledWith({ + area: 'project-delete', + errorClass: 'UnknownError', + phase: 'failed', + }); }); /** AKS Managed Namespaces */ diff --git a/plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.ts b/plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.ts index d3d74a9b7..549deea13 100644 --- a/plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.ts +++ b/plugins/aks-desktop/src/components/DeleteAKSProject/hooks/useProjectDeletion.ts @@ -5,6 +5,8 @@ import { clusterAction, K8s, useTranslation } from '@kinvolk/headlamp-plugin/lib import type { ApiClient } from '@kinvolk/headlamp-plugin/lib/lib/k8s/api/v1/factories'; import type { KubeNamespace } from '@kinvolk/headlamp-plugin/lib/lib/k8s/namespace'; import Namespace from '@kinvolk/headlamp-plugin/lib/lib/k8s/namespace'; +import { trackError } from '../../../telemetry'; +import { trackAksFeature } from '../../../telemetry/aksFeature'; import { deleteManagedNamespace } from '../../../utils/azure/az-namespaces'; import { PROJECT_ID_LABEL, @@ -27,103 +29,110 @@ export function useProjectDeletion() { deleteNamespaces: boolean, onClose: () => void ) => { + trackAksFeature('aksd.project-delete', 'started'); clusterAction( async () => { - const namespacePromises = project.namespaces.map( - nsName => - new Promise(resolve => { - K8s.ResourceClasses.Namespace.apiGet( - (ns: Namespace) => resolve(ns), - nsName, - undefined, - () => resolve(null), - { cluster: project.clusters[0] } - )(); - }) - ); - - const namespaces = (await Promise.all(namespacePromises)).filter( - (ns): ns is Namespace => ns !== null - ); - - for (const ns of namespaces) { - const labels = ns.metadata?.labels || {}; - const isAKSManaged = labels[PROJECT_MANAGED_BY_LABEL] === PROJECT_MANAGED_BY_VALUE; - const nsName = ns.metadata?.name || ''; - - if (isAKSManaged) { - const resourceGroup = labels[RESOURCE_GROUP_LABEL]; - const subscriptionId = labels[SUBSCRIPTION_LABEL]; - - if (!resourceGroup || !subscriptionId) { - throw new Error( - `Missing required Azure labels on namespace '${nsName}' for managed deletion.` - ); - } - - // Delete ARM managed namespace - const result = await deleteManagedNamespace({ - clusterName: project.clusters[0], - resourceGroup, - namespaceName: nsName, - subscriptionId, - }); - - if (!result.success) { - throw new Error(result.error || 'Failed to delete managed namespace'); - } - - if (deleteNamespaces) { - // Delete the Kubernetes namespace - await (K8s.ResourceClasses.Namespace.apiEndpoint as ApiClient).delete( - nsName, - {}, - project.clusters[0] - ); - } else { - // Re-fetch namespace to get latest resourceVersion after ARM call modified it - const freshNs = await new Promise((resolve, reject) => { + try { + const namespacePromises = project.namespaces.map( + nsName => + new Promise(resolve => { K8s.ResourceClasses.Namespace.apiGet( (ns: Namespace) => resolve(ns), nsName, undefined, - (err: any) => reject(err), + () => resolve(null), { cluster: project.clusters[0] } )(); + }) + ); + + const namespaces = (await Promise.all(namespacePromises)).filter( + (ns): ns is Namespace => ns !== null + ); + + for (const ns of namespaces) { + const labels = ns.metadata?.labels || {}; + const isAKSManaged = labels[PROJECT_MANAGED_BY_LABEL] === PROJECT_MANAGED_BY_VALUE; + const nsName = ns.metadata?.name || ''; + + if (isAKSManaged) { + const resourceGroup = labels[RESOURCE_GROUP_LABEL]; + const subscriptionId = labels[SUBSCRIPTION_LABEL]; + + if (!resourceGroup || !subscriptionId) { + throw new Error( + `Missing required Azure labels on namespace '${nsName}' for managed deletion.` + ); + } + + // Delete ARM managed namespace + const result = await deleteManagedNamespace({ + clusterName: project.clusters[0], + resourceGroup, + namespaceName: nsName, + subscriptionId, }); - // Remove project labels from namespace - const updatedData = { ...freshNs.jsonData }; - if (updatedData.metadata?.labels) { - delete updatedData.metadata.labels[PROJECT_ID_LABEL]; - delete updatedData.metadata.labels[PROJECT_MANAGED_BY_LABEL]; - delete updatedData.metadata.labels[SUBSCRIPTION_LABEL]; - delete updatedData.metadata.labels[RESOURCE_GROUP_LABEL]; + if (!result.success) { + throw new Error(result.error || 'Failed to delete managed namespace'); + } + + if (deleteNamespaces) { + // Delete the Kubernetes namespace + await ( + K8s.ResourceClasses.Namespace.apiEndpoint as ApiClient + ).delete(nsName, {}, project.clusters[0]); + } else { + // Re-fetch namespace to get latest resourceVersion after ARM call modified it + const freshNs = await new Promise((resolve, reject) => { + K8s.ResourceClasses.Namespace.apiGet( + (ns: Namespace) => resolve(ns), + nsName, + undefined, + (err: any) => reject(err), + { cluster: project.clusters[0] } + )(); + }); + + // Remove project labels from namespace + const updatedData = { ...freshNs.jsonData }; + if (updatedData.metadata?.labels) { + delete updatedData.metadata.labels[PROJECT_ID_LABEL]; + delete updatedData.metadata.labels[PROJECT_MANAGED_BY_LABEL]; + delete updatedData.metadata.labels[SUBSCRIPTION_LABEL]; + delete updatedData.metadata.labels[RESOURCE_GROUP_LABEL]; + } + await K8s.ResourceClasses.Namespace.apiEndpoint.put( + updatedData, + {}, + project.clusters[0] + ); } - await K8s.ResourceClasses.Namespace.apiEndpoint.put( - updatedData, - {}, - project.clusters[0] - ); - } - } else { - // Regular namespace (not AKS managed) - if (deleteNamespaces) { - await ns.delete(); } else { - // Remove project labels - const updatedData = { ...ns.jsonData }; - if (updatedData.metadata?.labels) { - delete updatedData.metadata.labels[PROJECT_ID_LABEL]; - delete updatedData.metadata.labels[PROJECT_MANAGED_BY_LABEL]; + // Regular namespace (not AKS managed) + if (deleteNamespaces) { + await ns.delete(); + } else { + // Remove project labels + const updatedData = { ...ns.jsonData }; + if (updatedData.metadata?.labels) { + delete updatedData.metadata.labels[PROJECT_ID_LABEL]; + delete updatedData.metadata.labels[PROJECT_MANAGED_BY_LABEL]; + } + await K8s.ResourceClasses.Namespace.apiEndpoint.put( + updatedData, + {}, + project.clusters[0] + ); } - await K8s.ResourceClasses.Namespace.apiEndpoint.put( - updatedData, - {}, - project.clusters[0] - ); } } + + trackAksFeature('aksd.project-delete', 'succeeded'); + } catch (error) { + trackAksFeature('aksd.project-delete', 'failed'); + trackError({ area: 'project-delete', errorClass: 'UnknownError', phase: 'failed' }); + throw error; } }, { diff --git a/plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.test.ts b/plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.test.ts new file mode 100644 index 000000000..b22c37dfe --- /dev/null +++ b/plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.test.ts @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +// @vitest-environment jsdom + +import { renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; + +const mockTrackAksFeature = vi.hoisted(() => vi.fn()); + +vi.mock('../telemetry/aksFeature', () => ({ trackAksFeature: mockTrackAksFeature })); + +import { useTelemetryFeatureOpened } from './useTelemetryFeatureOpened'; + +describe('useTelemetryFeatureOpened', () => { + beforeEach(() => vi.clearAllMocks()); + + test('emits one opened event for the mounted surface', () => { + const { rerender } = renderHook(() => useTelemetryFeatureOpened('aksd.cluster-add')); + + rerender(); + + expect(mockTrackAksFeature).toHaveBeenCalledTimes(1); + expect(mockTrackAksFeature).toHaveBeenCalledWith('aksd.cluster-add', 'opened'); + }); +}); diff --git a/plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.ts b/plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.ts new file mode 100644 index 000000000..8155dd51e --- /dev/null +++ b/plugins/aks-desktop/src/hooks/useTelemetryFeatureOpened.ts @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +import { useEffect } from 'react'; +import { trackAksFeature } from '../telemetry/aksFeature'; +import type { AksFeatureType } from '../telemetry/schema'; + +export function useTelemetryFeatureOpened(feature: AksFeatureType): void { + useEffect(() => { + trackAksFeature(feature, 'opened'); + }, [feature]); +} diff --git a/plugins/aks-desktop/src/telemetry/aksFeature.test.ts b/plugins/aks-desktop/src/telemetry/aksFeature.test.ts new file mode 100644 index 000000000..ab4085123 --- /dev/null +++ b/plugins/aks-desktop/src/telemetry/aksFeature.test.ts @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +import { beforeEach, describe, expect, test, vi } from 'vitest'; + +const mockTrackFeature = vi.hoisted(() => vi.fn()); + +vi.mock('./index', () => ({ trackFeature: mockTrackFeature })); + +import { trackAksFeature } from './aksFeature'; + +describe('trackAksFeature', () => { + beforeEach(() => vi.clearAllMocks()); + + test('forwards an allowlisted AKS lifecycle event', () => { + trackAksFeature('aksd.namespace-create', 'started'); + expect(mockTrackFeature).toHaveBeenCalledWith({ + feature: 'aksd.namespace-create', + status: 'started', + }); + }); + + test('synchronously protects callers from telemetry failures', () => { + // Real SDK transport failure behavior is covered by telemetry/index.test.ts. + mockTrackFeature.mockImplementationOnce(() => { + throw new Error('transport failure'); + }); + + expect(() => trackAksFeature('aksd.project-delete', 'failed')).not.toThrow(); + }); +}); diff --git a/plugins/aks-desktop/src/telemetry/aksFeature.ts b/plugins/aks-desktop/src/telemetry/aksFeature.ts new file mode 100644 index 000000000..0c464f54b --- /dev/null +++ b/plugins/aks-desktop/src/telemetry/aksFeature.ts @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache 2.0. + +import { trackFeature } from './index'; +import type { AksFeatureType, TelemetryStatus } from './schema'; + +export type AksFeatureLifecycleStatus = Extract< + TelemetryStatus, + 'opened' | 'started' | 'succeeded' | 'failed' | 'cancelled' +>; + +export function trackAksFeature(feature: AksFeatureType, status: AksFeatureLifecycleStatus): void { + try { + trackFeature({ feature, status }); + } catch { + // Telemetry must never affect the workflow being measured. + } +} diff --git a/plugins/aks-desktop/src/telemetry/schema.ts b/plugins/aks-desktop/src/telemetry/schema.ts index d7e913f4b..05b6e5e02 100644 --- a/plugins/aks-desktop/src/telemetry/schema.ts +++ b/plugins/aks-desktop/src/telemetry/schema.ts @@ -94,6 +94,19 @@ export const TELEMETRY_PROPERTY_KEYS: ReadonlySet = new Set([ 'thirdPartyCount', ]); +export const AKS_FEATURE_TYPES = [ + 'aksd.project-create', + 'aksd.project-import', + 'aksd.deploy', + 'aksd.auth-login', + 'aksd.auth-logout', + 'aksd.cluster-add', + 'aksd.namespace-create', + 'aksd.project-delete', +] as const; + +export type AksFeatureType = (typeof AKS_FEATURE_TYPES)[number]; + /** Redux event types forwarded as `headlamp.feature` envelopes. */ export const KNOWN_FEATURE_TYPES: ReadonlySet = new Set([ 'headlamp.delete-resource', @@ -111,9 +124,7 @@ export const KNOWN_FEATURE_TYPES: ReadonlySet = new Set([ 'headlamp.details-view', 'headlamp.list-view', 'headlamp.object-events', - 'aksd.project-create', - 'aksd.project-import', - 'aksd.deploy', + ...AKS_FEATURE_TYPES, // PLUGINS_LOADED routes to trackPluginsLoaded; ERROR_BOUNDARY is captured // by TelemetryErrorBoundary directly. Both intentionally omitted. ]); @@ -166,6 +177,11 @@ const ERROR_AREA_VALUES = [ 'deploy', 'kubernetes', 'plugin-ui', + 'auth-login', + 'auth-logout', + 'cluster-add', + 'namespace-create', + 'project-delete', ] as const; export type TelemetryErrorArea = (typeof ERROR_AREA_VALUES)[number]; diff --git a/plugins/aks-desktop/src/telemetry/vocabulary.test.ts b/plugins/aks-desktop/src/telemetry/vocabulary.test.ts index f0f006217..2266e6cef 100644 --- a/plugins/aks-desktop/src/telemetry/vocabulary.test.ts +++ b/plugins/aks-desktop/src/telemetry/vocabulary.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import { + AKS_FEATURE_TYPES, ERROR_AREAS, ERROR_CLASSES, EVENT_STATUSES, @@ -72,6 +73,24 @@ describe('telemetry privacy vocabularies', () => { 'aksd.project-create', 'aksd.project-import', 'aksd.deploy', + 'aksd.auth-login', + 'aksd.auth-logout', + 'aksd.cluster-add', + 'aksd.namespace-create', + 'aksd.project-delete', + ]); + }); + + it('enumerates every approved AKS feature type', () => { + expect(AKS_FEATURE_TYPES).toEqual([ + 'aksd.project-create', + 'aksd.project-import', + 'aksd.deploy', + 'aksd.auth-login', + 'aksd.auth-logout', + 'aksd.cluster-add', + 'aksd.namespace-create', + 'aksd.project-delete', ]); }); @@ -121,6 +140,11 @@ describe('telemetry privacy vocabularies', () => { 'deploy', 'kubernetes', 'plugin-ui', + 'auth-login', + 'auth-logout', + 'cluster-add', + 'namespace-create', + 'project-delete', ]); }); });