aksd: expand telemetry coverage for core workflows

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.
+

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<string, string>) =>
+      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;
+  }) => (
+    <div>
+      <button onClick={event => onSubscriptionChange(event, subscription)}>
+        Select subscription
+      </button>
+      <button onClick={event => onClusterChange(event, cluster)}>Select cluster</button>
+      <button onClick={onRegister}>Register</button>
+    </div>
+  ),
+}));
+
+import RegisterAKSClusterDialog from './RegisterAKSClusterDialog';
+
+function renderDialog() {
+  return render(
+    <RegisterAKSClusterDialog
+      open
+      onClose={mocks.onClose}
+      onClusterRegistered={mocks.onClusterRegistered}
+      onRegistrationFinished={mocks.onRegistrationFinished}
+      onRegistrationStarted={mocks.onRegistrationStarted}
+    />
+  );
+}
+
+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();
+  });
+});