diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 473d733d08..ef55bbd2d8 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -58,6 +58,11 @@ jobs: - name: Package extension run: npm run package + env: + POSTHOG_API_KEY: ${{ secrets.POSTHOG_API_KEY }} + # workflow_dispatch accepts any ref, so gate `stable` on the ref rather than the event to keep + # manually packaged feature branches out of stable analytics. + POSTHOG_CHANNEL: ${{ github.event_name == 'pull_request' && 'pr' || (github.ref == 'refs/heads/main' && 'stable' || 'development') }} - name: Rename VSIX file run: | diff --git a/build/esbuild/build.ts b/build/esbuild/build.ts index c313ce8cf8..76fb76ad68 100644 --- a/build/esbuild/build.ts +++ b/build/esbuild/build.ts @@ -231,6 +231,13 @@ function createConfig( inject.push(path.join(__dirname, isDevbuild ? 'process.development.js' : 'process.production.js')); } } + if (target === 'desktop') { + // Bake the PostHog key and channel in from CI at build time; both fall back to safe defaults locally (see constants.ts). + define = { + POSTHOG_API_KEY_BUILD: JSON.stringify(process.env.POSTHOG_API_KEY ?? ''), + POSTHOG_CHANNEL_BUILD: JSON.stringify(process.env.POSTHOG_CHANNEL ?? '') + }; + } if (source.endsWith(path.join('data-explorer', 'index.tsx'))) { inject.push(path.join(__dirname, 'jquery.js')); } diff --git a/package-lock.json b/package-lock.json index f8e9ef939b..22cbeaa779 100644 --- a/package-lock.json +++ b/package-lock.json @@ -64,6 +64,7 @@ "pidtree": "^0.6.0", "plotly.js-dist": "^3.0.1", "portfinder": "^1.0.25", + "posthog-node": "^4.18.0", "re-resizable": "^6.5.5", "react": "^16.5.2", "react-data-grid": "^6.0.2-0", @@ -11009,6 +11010,18 @@ "node": ">=4" } }, + "node_modules/axios": { + "version": "1.18.1", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.18.1.tgz", + "integrity": "sha512-3nTvFlvpn9Zu/RkHUqtc7/+al4UpRW5az71ap5zccp6e8RAYEzhMTecX8Dz1wWDYrPpUoB1HAQEGEAEvUr7S9g==", + "license": "MIT", + "dependencies": { + "follow-redirects": "^1.16.0", + "form-data": "^4.0.5", + "https-proxy-agent": "^5.0.1", + "proxy-from-env": "^2.1.0" + } + }, "node_modules/axobject-query": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/axobject-query/-/axobject-query-3.2.1.tgz", @@ -17771,6 +17784,26 @@ "dev": true, "license": "ISC" }, + "node_modules/follow-redirects": { + "version": "1.16.0", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.0.tgz", + "integrity": "sha512-y5rN/uOsadFT/JfYwhxRS5R7Qce+g3zG97+JrtFZlC9klX/W5hD7iiLzScI4nZqUS7DNUdhPgw4xI8W2LuXlUw==", + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/RubenVerborgh" + } + ], + "license": "MIT", + "engines": { + "node": ">=4.0" + }, + "peerDependenciesMeta": { + "debug": { + "optional": true + } + } + }, "node_modules/font-awesome": { "version": "4.7.0", "resolved": "https://registry.npmjs.org/font-awesome/-/font-awesome-4.7.0.tgz", @@ -27804,6 +27837,18 @@ "node": ">=0.10.0" } }, + "node_modules/posthog-node": { + "version": "4.18.0", + "resolved": "https://registry.npmjs.org/posthog-node/-/posthog-node-4.18.0.tgz", + "integrity": "sha512-XROs1h+DNatgKh/AlIlCtDxWzwrKdYDb2mOs58n4yN8BkGN9ewqeQwG5ApS4/IzwCb7HPttUkOVulkYatd2PIw==", + "license": "MIT", + "dependencies": { + "axios": "^1.8.2" + }, + "engines": { + "node": ">=15.0.0" + } + }, "node_modules/postinstall-build": { "version": "5.0.3", "resolved": "https://registry.npmjs.org/postinstall-build/-/postinstall-build-5.0.3.tgz", @@ -28135,6 +28180,15 @@ "node": ">= 0.10" } }, + "node_modules/proxy-from-env": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-2.1.0.tgz", + "integrity": "sha512-cJ+oHTW1VAEa8cJslgmUZrc+sjRKgAKl3Zyse6+PV38hZe/V6Z14TbCuXcan9F9ghlz4QrFr2c92TNF82UkYHA==", + "license": "MIT", + "engines": { + "node": ">=10" + } + }, "node_modules/prr": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/prr/-/prr-1.0.1.tgz", @@ -43318,6 +43372,17 @@ "integrity": "sha512-/dlp0fxyM3R8YW7MFzaHWXrf4zzbr0vaYb23VBFCl83R7nWNPg/yaQw2Dc8jzCMmDVLhSdzH8MjrsuIUuvX+6g==", "dev": true }, + "axios": { + "version": "1.18.1", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.18.1.tgz", + "integrity": "sha512-3nTvFlvpn9Zu/RkHUqtc7/+al4UpRW5az71ap5zccp6e8RAYEzhMTecX8Dz1wWDYrPpUoB1HAQEGEAEvUr7S9g==", + "requires": { + "follow-redirects": "^1.16.0", + "form-data": "4.0.6", + "https-proxy-agent": "^5.0.1", + "proxy-from-env": "^2.1.0" + } + }, "axobject-query": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/axobject-query/-/axobject-query-3.2.1.tgz", @@ -48179,6 +48244,11 @@ "integrity": "sha512-PjDse7RzhcPkIJwy5t7KPWQSZ9cAbzQXcafsetQoD7sOJRQlGikNbx7yZp2OotDnJyrDcbyRq3Ttb18iYOqkxA==", "dev": true }, + "follow-redirects": { + "version": "1.16.0", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.0.tgz", + "integrity": "sha512-y5rN/uOsadFT/JfYwhxRS5R7Qce+g3zG97+JrtFZlC9klX/W5hD7iiLzScI4nZqUS7DNUdhPgw4xI8W2LuXlUw==" + }, "font-awesome": { "version": "4.7.0", "resolved": "https://registry.npmjs.org/font-awesome/-/font-awesome-4.7.0.tgz", @@ -55117,6 +55187,14 @@ "xtend": "^4.0.0" } }, + "posthog-node": { + "version": "4.18.0", + "resolved": "https://registry.npmjs.org/posthog-node/-/posthog-node-4.18.0.tgz", + "integrity": "sha512-XROs1h+DNatgKh/AlIlCtDxWzwrKdYDb2mOs58n4yN8BkGN9ewqeQwG5ApS4/IzwCb7HPttUkOVulkYatd2PIw==", + "requires": { + "axios": "^1.8.2" + } + }, "postinstall-build": { "version": "5.0.3", "resolved": "https://registry.npmjs.org/postinstall-build/-/postinstall-build-5.0.3.tgz", @@ -55352,6 +55430,11 @@ "ipaddr.js": "1.9.1" } }, + "proxy-from-env": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-2.1.0.tgz", + "integrity": "sha512-cJ+oHTW1VAEa8cJslgmUZrc+sjRKgAKl3Zyse6+PV38hZe/V6Z14TbCuXcan9F9ghlz4QrFr2c92TNF82UkYHA==" + }, "prr": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/prr/-/prr-1.0.1.tgz", diff --git a/package.json b/package.json index 8dc22af21d..e44e44ecb5 100644 --- a/package.json +++ b/package.json @@ -1643,6 +1643,12 @@ "description": "Disable SSL certificate verification (for development only)", "scope": "application" }, + "deepnote.telemetry.enabled": { + "type": "boolean", + "default": true, + "description": "%deepnote.configuration.deepnote.telemetry.enabled.description%", + "scope": "application" + }, "deepnote.snapshots.enabled": { "type": "boolean", "default": true, @@ -2726,6 +2732,7 @@ "pidtree": "^0.6.0", "plotly.js-dist": "^3.0.1", "portfinder": "^1.0.25", + "posthog-node": "^4.18.0", "re-resizable": "^6.5.5", "react": "^16.5.2", "react-data-grid": "^6.0.2-0", diff --git a/package.nls.json b/package.nls.json index f3b21dc525..58ea84148d 100644 --- a/package.nls.json +++ b/package.nls.json @@ -122,6 +122,7 @@ "deepnote.debuggers.kernel": "Python Kernel Debug Adapter", "deepnote.debuggers.interactive": "Python Interactive Window", "deepnote.configuration.deepnote.experiments.enabled.description": "Enables/disables A/B tests.", + "deepnote.configuration.deepnote.telemetry.enabled.description": "Enable anonymous usage telemetry to help improve Deepnote for VS Code.", "deepnote.configuration.deepnote.showVariableViewWhenDebugging.description": "Bring up the Variable View when starting a Run by Line session.", "deepnote.configuration.deepnote.logging.level.off": "No messages are logged with this level.", "deepnote.configuration.deepnote.logging.level.trace": "All messages are logged with this level.", diff --git a/src/commands.ts b/src/commands.ts index 0ec0d1ad52..363547f50f 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -57,7 +57,6 @@ export interface ICommandNameArgumentTypeMapping { [DSCommands.RestartKernelAndRunAllCells]: [{ notebookEditor: { notebookUri: Uri } } | undefined]; [DSCommands.RestartKernelAndRunUpToSelectedCell]: [{ notebookEditor: { notebookUri: Uri } } | undefined]; [DSCommands.NotebookEditorRemoveAllCells]: []; - [DSCommands.NotebookEditorRunAllCells]: []; [DSCommands.NotebookEditorRunFocusedCell]: []; [DSCommands.NotebookEditorAddCellBelow]: []; [DSCommands.ExpandAllCells]: []; diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts index 0b5ab50948..e04c9d4d59 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts @@ -13,6 +13,7 @@ import { import { IPythonApiProvider } from '../../../platform/api/types'; import { STANDARD_OUTPUT_CHANNEL } from '../../../platform/common/constants'; import { getDisplayPath } from '../../../platform/common/platform/fs-paths.node'; +import { ITelemetryService } from '../../../platform/analytics/types'; import { IDisposableRegistry, IOutputChannel } from '../../../platform/common/types'; import { createDeepnoteServerConfigHandle } from '../../../platform/deepnote/deepnoteServerUtils.node'; import { DeepnoteToolkitMissingError } from '../../../platform/errors/deepnoteKernelErrors'; @@ -54,7 +55,8 @@ export class DeepnoteEnvironmentsView implements Disposable { private readonly notebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper, @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, - @inject(IDeepnoteServerStarter) private readonly serverStarter: IDeepnoteServerStarter + @inject(IDeepnoteServerStarter) private readonly serverStarter: IDeepnoteServerStarter, + @inject(ITelemetryService) private readonly analytics: ITelemetryService ) { // Create tree data provider @@ -195,6 +197,14 @@ export class DeepnoteEnvironmentsView implements Disposable { const config = await this.environmentManager.createEnvironment(options, token); logger.info(`Created environment: ${config.id} (${config.name})`); + this.analytics.trackEvent({ + eventName: 'create_environment', + properties: { + hasDescription: !!options.description, + packageCount: options.packages?.length ?? 0 + } + }); + void window.showInformationMessage( l10n.t('Environment "{0}" created successfully!', config.name) ); @@ -328,6 +338,7 @@ export class DeepnoteEnvironmentsView implements Disposable { } ); + this.analytics.trackEvent({ eventName: 'delete_environment' }); void window.showInformationMessage(l10n.t('Environment "{0}" deleted', config.name)); } catch (error) { logger.error('Failed to delete environment', error); @@ -494,6 +505,7 @@ export class DeepnoteEnvironmentsView implements Disposable { } ); + this.analytics.trackEvent({ eventName: 'select_environment' }); void window.showInformationMessage(l10n.t('Environment switched successfully')); } catch (error) { if (error instanceof DeepnoteToolkitMissingError) { @@ -544,6 +556,7 @@ export class DeepnoteEnvironmentsView implements Disposable { logger.info(`Renamed environment ${environmentId} to "${newName}"`); void window.showInformationMessage(l10n.t('Environment renamed to "{0}"', newName)); + this.analytics.trackEvent({ eventName: 'update_environment', properties: { field: 'name' } }); } catch (error) { logger.error('Failed to rename environment', error); void window.showErrorMessage(l10n.t('Failed to rename environment. See output for details.')); @@ -602,6 +615,10 @@ export class DeepnoteEnvironmentsView implements Disposable { ); void window.showInformationMessage(l10n.t('Packages updated for "{0}"', config.name)); + this.analytics.trackEvent({ + eventName: 'update_environment', + properties: { field: 'packages', packageCount: packages.length } + }); } catch (error) { logger.error('Failed to update packages', error); void window.showErrorMessage(l10n.t('Failed to update packages. See output for details.')); diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts index e2a9cf0edc..9d59edc279 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts @@ -10,6 +10,7 @@ import { IDeepnoteServerStarter } from '../types'; import { IPythonApiProvider } from '../../../platform/api/types'; +import { ITelemetryService } from '../../../platform/analytics/types'; import { IDisposableRegistry, IOutputChannel } from '../../../platform/common/types'; import { IKernelProvider } from '../../../kernels/types'; import { DeepnoteEnvironment } from './deepnoteEnvironment'; @@ -30,6 +31,7 @@ suite('DeepnoteEnvironmentsView', () => { let mockNotebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper; let mockKernelProvider: IKernelProvider; let mockOutputChannel: IOutputChannel; + let mockTelemetryService: ITelemetryService; let mockServerStarter: IDeepnoteServerStarter; let disposables: Disposable[] = []; let pythonEnvironments: PythonExtension['environments']; @@ -49,6 +51,7 @@ suite('DeepnoteEnvironmentsView', () => { mockNotebookEnvironmentMapper = mock(); mockKernelProvider = mock(); mockOutputChannel = mock(); + mockTelemetryService = mock(); mockServerStarter = mock(); // stopServer is a safe no-op when a notebook has no running server @@ -72,7 +75,8 @@ suite('DeepnoteEnvironmentsView', () => { instance(mockNotebookEnvironmentMapper), instance(mockKernelProvider), instance(mockOutputChannel), - instance(mockServerStarter) + instance(mockServerStarter), + instance(mockTelemetryService) ); }); diff --git a/src/notebooks/controllers/vscodeNotebookController.ts b/src/notebooks/controllers/vscodeNotebookController.ts index 9ab88bd119..a07eceba48 100644 --- a/src/notebooks/controllers/vscodeNotebookController.ts +++ b/src/notebooks/controllers/vscodeNotebookController.ts @@ -56,6 +56,7 @@ import { } from '../../kernels/types'; import { IJupyterVariablesProvider } from '../../kernels/variables/types'; import { IPyWidgetMessages } from '../../messageTypes'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IPythonExtensionChecker } from '../../platform/api/types'; import { isCancellationError } from '../../platform/common/cancellation'; import { @@ -462,6 +463,18 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont cts.dispose(); } } + + // A batch covering every code cell is the only signal core gives us that the user ran the + // whole notebook; the built-in Run All button never routes through an extension command. + // deliberate: a single-code-cell notebook and "Run All Above" from the last cell both + // match this shape and are counted — unavoidable without a core API naming the command. + const codeCellCount = notebook.getCells().filter((cell) => cell.kind === NotebookCellKind.Code).length; + + if (codeCellCount > 0 && cells.length === codeCellCount) { + this.serviceContainer + .get(ITelemetryService) + .trackEvent({ eventName: 'execute_notebook' }); + } } logger.debug(`Handle Execution of Cells ${cells.map((c) => c.index)} for ${getDisplayPath(notebook.uri)}`); diff --git a/src/notebooks/controllers/vscodeNotebookController.unit.test.ts b/src/notebooks/controllers/vscodeNotebookController.unit.test.ts index c03296abb6..0f55f5d9d1 100644 --- a/src/notebooks/controllers/vscodeNotebookController.unit.test.ts +++ b/src/notebooks/controllers/vscodeNotebookController.unit.test.ts @@ -7,7 +7,15 @@ /* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */ import { assert } from 'chai'; import * as fakeTimers from '@sinonjs/fake-timers'; -import { NotebookDocument, EventEmitter, NotebookController, Uri, Disposable } from 'vscode'; +import { + NotebookDocument, + EventEmitter, + NotebookCell, + NotebookCellKind, + NotebookController, + Uri, + Disposable +} from 'vscode'; import { VSCodeNotebookController, warnWhenUsingOutdatedPython } from './vscodeNotebookController'; import { IKernel, @@ -18,7 +26,8 @@ import { LocalKernelSpecConnectionMetadata, RemoteKernelSpecConnectionMetadata } from '../../kernels/types'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IConfigurationService, IDisposable, @@ -37,7 +46,7 @@ import { KernelConnector } from './kernelConnector'; import { ITrustedKernelPaths } from '../../kernels/raw/finder/types'; import { IInterpreterService } from '../../platform/interpreter/contracts'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; -import { IConnectionDisplayData, IConnectionDisplayDataProvider } from './types'; +import { IConnectionDisplayData, IConnectionDisplayDataProvider, IVSCodeNotebookController } from './types'; import { ConnectionDisplayDataProvider } from './connectionDisplayData.node'; import { mockedVSCode, mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; import { Environment, PythonExtension } from '@vscode/python-extension'; @@ -825,5 +834,73 @@ suite(`Notebook Controller`, function () { // Assert assert.isDefined(result); }); + + suite('execute_notebook telemetry', () => { + let telemetry: ITelemetryService; + + function createControllerForExecution(): IVSCodeNotebookController { + return VSCodeNotebookController.create( + instance(kernelConnection), + 'test-id', + 'jupyter-notebook', + instance(kernelProvider), + instance(context), + disposables, + instance(languageService), + instance(configService), + instance(extensionChecker), + instance(serviceContainer), + instance(displayDataProvider), + instance(jupyterVariablesProvider) + ); + } + + function codeCell(index: number): NotebookCell { + return { index, kind: NotebookCellKind.Code, document: { getText: () => '' } } as never; + } + + function markdownCell(index: number): NotebookCell { + return { index, kind: NotebookCellKind.Markup, document: { getText: () => '' } } as never; + } + + function deepnoteNotebook(cells: NotebookCell[]): NotebookDocument { + return { notebookType: 'deepnote', uri: Uri.file('/ws/exec.deepnote'), getCells: () => cells } as never; + } + + setup(() => { + telemetry = mock(); + when(mockedVSCodeNamespaces.workspace.isTrusted).thenReturn(true); + when(serviceContainer.get(ITelemetryService)).thenReturn(instance(telemetry)); + }); + + async function handleExecution(cells: NotebookCell[], notebook: NotebookDocument): Promise { + // Kernel startup after the tracking point fails on these bare mocks; that failure is + // irrelevant to what these tests assert. + await (createControllerForExecution() as any).handleExecution(cells, notebook).catch(() => undefined); + } + + test('a batch covering every code cell reports execute_notebook', async () => { + const cells = [codeCell(0), codeCell(1)]; + const notebook = deepnoteNotebook([...cells, markdownCell(2)]); + + await handleExecution(cells, notebook); + + verify(telemetry.trackEvent(deepEqual({ eventName: 'execute_notebook' }))).once(); + verify(telemetry.trackEvent(anything())).once(); + }); + + test('a partial batch or a non-Deepnote notebook does not report execute_notebook', async () => { + const allCells = [codeCell(0), codeCell(1)]; + + await handleExecution([allCells[0]], deepnoteNotebook(allCells)); + await handleExecution(allCells, { + notebookType: 'jupyter-notebook', + uri: Uri.file('/ws/n.ipynb'), + getCells: () => allCells + } as never as NotebookDocument); + + verify(telemetry.trackEvent(anything())).never(); + }); + }); }); }); diff --git a/src/notebooks/deepnote/deepnoteActivationService.ts b/src/notebooks/deepnote/deepnoteActivationService.ts index 07d65abdf9..cb2144b9fd 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.ts @@ -2,6 +2,7 @@ import { inject, injectable, optional } from 'inversify'; import { commands, l10n, workspace, window, type Disposable, type NotebookDocumentContentOptions } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { ILogger } from '../../platform/logging/types'; import { IDeepnoteNotebookEnvironmentMapper } from '../../kernels/deepnote/types'; @@ -40,6 +41,7 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, @inject(IIntegrationManager) integrationManager: IIntegrationManager, @inject(ILogger) private readonly logger: ILogger, + @inject(ITelemetryService) private readonly analytics: ITelemetryService, @inject(SnapshotService) @optional() private readonly snapshotService?: SnapshotService, @inject(IDeepnoteNotebookEnvironmentMapper) @optional() @@ -57,7 +59,8 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic this.explorerView = new DeepnoteExplorerView( this.extensionContext, this.logger, - new DeepnoteTreeDataProvider(this.logger) + new DeepnoteTreeDataProvider(this.logger), + this.analytics ); this.editProtection = new DeepnoteInputBlockEditProtection(this.logger); this.snapshotsEnabled = this.isSnapshotsEnabled(); @@ -86,7 +89,8 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic this.environmentMapper, () => this.explorerView.refresh(), this.logger, - deepnoteFileExists + deepnoteFileExists, + this.analytics ); this.extensionContext.subscriptions.push(...this.multiNotebookSplitter.activate()); this.extensionContext.subscriptions.push(this.multiNotebookSplitter); diff --git a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts index a8068d5f64..e4409af9ad 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts @@ -1,6 +1,7 @@ import { assert } from 'chai'; -import { anything, verify, when } from 'ts-mockito'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { ITelemetryService } from '../../platform/analytics/types'; import { DeepnoteActivationService } from './deepnoteActivationService'; import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; import { IExtensionContext } from '../../platform/common/types'; @@ -25,6 +26,7 @@ suite('DeepnoteActivationService', () => { let manager: DeepnoteNotebookManager; let mockIntegrationManager: IIntegrationManager; let mockLogger: ILogger; + let mockAnalytics: ITelemetryService; setup(() => { mockExtensionContext = { @@ -38,11 +40,13 @@ suite('DeepnoteActivationService', () => { } }; mockLogger = createMockLogger(); + mockAnalytics = instance(mock()); activationService = new DeepnoteActivationService( mockExtensionContext, manager, mockIntegrationManager, - mockLogger + mockLogger, + mockAnalytics ); }); @@ -103,6 +107,7 @@ suite('DeepnoteActivationService', () => { manager, mockIntegrationManager, mockLogger, + mockAnalytics, mockSnapshotService ); @@ -150,6 +155,7 @@ suite('DeepnoteActivationService', () => { manager, mockIntegrationManager, mockLogger, + mockAnalytics, mockSnapshotService ); @@ -206,8 +212,20 @@ suite('DeepnoteActivationService', () => { }; const mockLogger1 = createMockLogger(); const mockLogger2 = createMockLogger(); - const service1 = new DeepnoteActivationService(context1, manager1, mockIntegrationManager1, mockLogger1); - const service2 = new DeepnoteActivationService(context2, manager2, mockIntegrationManager2, mockLogger2); + const service1 = new DeepnoteActivationService( + context1, + manager1, + mockIntegrationManager1, + mockLogger1, + mockAnalytics + ); + const service2 = new DeepnoteActivationService( + context2, + manager2, + mockIntegrationManager2, + mockLogger2, + mockAnalytics + ); // Verify each service has its own context assert.strictEqual((service1 as any).extensionContext, context1); @@ -244,8 +262,8 @@ suite('DeepnoteActivationService', () => { }; const mockLogger3 = createMockLogger(); const mockLogger4 = createMockLogger(); - new DeepnoteActivationService(context1, manager1, mockIntegrationManager1, mockLogger3); - new DeepnoteActivationService(context2, manager2, mockIntegrationManager2, mockLogger4); + new DeepnoteActivationService(context1, manager1, mockIntegrationManager1, mockLogger3, mockAnalytics); + new DeepnoteActivationService(context2, manager2, mockIntegrationManager2, mockLogger4, mockAnalytics); assert.strictEqual(context1.subscriptions.length, 0); assert.strictEqual(context2.subscriptions.length, 1); diff --git a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts new file mode 100644 index 0000000000..ba8408661c --- /dev/null +++ b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts @@ -0,0 +1,102 @@ +import { inject, injectable } from 'inversify'; +import { Disposable, NotebookCellKind, workspace } from 'vscode'; + +import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { ITelemetryService } from '../../platform/analytics/types'; +import { IDisposableRegistry } from '../../platform/common/types'; +import { isDeepnoteNotebook } from '../../platform/common/utils'; +import { NotebookCellExecutionState, notebookCellExecutions } from '../../platform/notebooks/cellExecutionStateService'; +import { + DATAFRAME_SQL_INTEGRATION_ID, + toTelemetryIntegrationType +} from '../../platform/notebooks/deepnote/integrationTypes'; +import { IDeepnoteNotebookManager } from '../types'; + +/** + * Tracks notebook usage events for telemetry: cell executions, and plain code/markdown cell + * insertions from VS Code's built-in "+ Code" / "+ Markdown" controls that do not go through + * any extension command. + */ +@injectable() +export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationService { + constructor( + @inject(ITelemetryService) private readonly analytics: ITelemetryService, + @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, + @inject(IDisposableRegistry) private readonly disposables: Disposable[] + ) {} + + public activate(): void { + this.disposables.push( + workspace.onDidChangeNotebookDocument((e) => { + if (!isDeepnoteNotebook(e.notebook)) { + return; + } + + for (const change of e.contentChanges) { + // Only pure insertions are user-authored. Every reload/replace path in the + // extension (file-change watcher, remove-all-cells, input-block protection) uses + // replaceCells, which reports the displaced cells in removedCells. + if (change.removedCells.length > 0) { + continue; + } + + for (const cell of change.addedCells) { + // Typed Deepnote blocks stamp their pocket type on insert and are already + // counted by DeepnoteNotebookCommandListener; only plain cells are missing. + if (cell.metadata?.__deepnotePocket?.type) { + continue; + } + + this.analytics.trackEvent({ + eventName: 'add_block', + properties: { blockType: cell.kind === NotebookCellKind.Code ? 'code' : 'markdown' } + }); + } + } + }) + ); + + this.disposables.push( + notebookCellExecutions.onDidChangeNotebookCellExecutionState((e) => { + if (e.state !== NotebookCellExecutionState.Executing) { + return; + } + + if (!isDeepnoteNotebook(e.cell.notebook)) { + return; + } + + const languageId = e.cell.document.languageId; + const cellType = languageId === 'sql' ? 'sql' : languageId === 'markdown' ? 'markdown' : 'code'; + + const properties: { cellType: 'sql' | 'markdown' | 'code'; integrationType?: string } = { cellType }; + + if (cellType === 'sql') { + // Read the authoritative top-level key only; the status-bar switch updates only this + // key (the __deepnotePocket copy can go stale after an in-session integration switch). + const integrationId = e.cell.metadata?.sql_integration_id; + + if (integrationId === DATAFRAME_SQL_INTEGRATION_ID) { + // The built-in DataFrame SQL integration is a pseudo-id never present in + // project.integrations; map it the same way switch_sql_integration does. + properties.integrationType = 'duckdb'; + } else if (integrationId) { + const projectId = e.cell.notebook.metadata?.deepnoteProjectId; + const notebookId = e.cell.notebook.metadata?.deepnoteNotebookId; + + if (projectId && notebookId) { + const project = this.notebookManager.getProjectForNotebook(projectId, notebookId); + const integration = project?.project.integrations?.find((i) => i.id === integrationId); + + if (integration?.type) { + properties.integrationType = toTelemetryIntegrationType(integration.type); + } + } + } + } + + this.analytics.trackEvent({ eventName: 'execute_cell', properties }); + }) + ); + } +} diff --git a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.unit.test.ts b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.unit.test.ts new file mode 100644 index 0000000000..8a83dd8a5c --- /dev/null +++ b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.unit.test.ts @@ -0,0 +1,195 @@ +import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; +import { + Disposable, + EventEmitter, + NotebookCell, + NotebookCellKind, + NotebookDocument, + NotebookDocumentChangeEvent, + Uri +} from 'vscode'; + +import { ITelemetryService } from '../../platform/analytics/types'; +import { NotebookCellExecutionState, notebookCellExecutions } from '../../platform/notebooks/cellExecutionStateService'; +import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes'; +import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; +import { DeepnoteCellExecutionAnalytics } from './deepnoteCellExecutionAnalytics'; +import { IDeepnoteNotebookManager } from '../types'; + +suite('DeepnoteCellExecutionAnalytics', () => { + let telemetry: ITelemetryService; + let notebookManager: IDeepnoteNotebookManager; + let disposables: Disposable[]; + let onDidChangeNotebookDocument: EventEmitter; + + const JUPYTER = 'jupyter-notebook'; + + function addedCell(kind: NotebookCellKind, pocketType?: string): NotebookCell { + return { + kind, + metadata: pocketType ? { __deepnotePocket: { type: pocketType } } : {} + } as unknown as NotebookCell; + } + + function executingCell(languageId: string, sqlIntegrationId?: string, notebookType = 'deepnote'): NotebookCell { + return { + document: { languageId }, + kind: NotebookCellKind.Code, + metadata: sqlIntegrationId ? { sql_integration_id: sqlIntegrationId } : {}, + notebook: { + metadata: { deepnoteNotebookId: 'notebook-1', deepnoteProjectId: 'project-1' }, + notebookType, + uri: Uri.file('/ws/p.deepnote') + } + } as unknown as NotebookCell; + } + + function fireContentChange(added: NotebookCell[], removed: NotebookCell[], notebookType = 'deepnote'): void { + onDidChangeNotebookDocument.fire({ + notebook: { notebookType, uri: Uri.file('/ws/p.deepnote') } as NotebookDocument, + metadata: undefined, + cellChanges: [], + contentChanges: [{ range: undefined, addedCells: added, removedCells: removed }] + } as unknown as NotebookDocumentChangeEvent); + } + + function stubProjectIntegrations(integrations: Array<{ id: string; name: string; type: string }>): void { + when(notebookManager.getProjectForNotebook('project-1', 'notebook-1')).thenReturn({ + project: { integrations } + } as never); + } + + setup(() => { + resetVSCodeMocks(); + disposables = []; + onDidChangeNotebookDocument = new EventEmitter(); + when(mockedVSCodeNamespaces.workspace.onDidChangeNotebookDocument).thenReturn( + onDidChangeNotebookDocument.event + ); + + telemetry = mock(); + notebookManager = mock(); + new DeepnoteCellExecutionAnalytics(instance(telemetry), instance(notebookManager), disposables).activate(); + }); + + teardown(() => { + onDidChangeNotebookDocument.dispose(); + disposables.forEach((d) => d.dispose()); + resetVSCodeMocks(); + }); + + suite('add_block', () => { + ( + [ + { + // Typed blocks stamp a pocket and are already counted by + // DeepnoteNotebookCommandListener; the built-in "+ Code"/"+ Markdown" don't. + name: 'counts the untyped cells of an insertion, by kind', + added: [ + addedCell(NotebookCellKind.Code, 'sql'), + addedCell(NotebookCellKind.Code), + addedCell(NotebookCellKind.Markup) + ], + removed: [], + expected: ['code', 'markdown'] + }, + { + // Every replace path (file-change watcher, remove-all-cells, input-block + // protection) rebuilds cells without a pocket, so removedCells is the only signal. + name: 'a replace is not an insertion', + added: [addedCell(NotebookCellKind.Code), addedCell(NotebookCellKind.Markup)], + removed: [addedCell(NotebookCellKind.Code), addedCell(NotebookCellKind.Markup)], + expected: [] + } + ] as const + ).forEach((row) => { + test(row.name, () => { + fireContentChange([...row.added], [...row.removed]); + + row.expected.forEach((blockType) => + verify( + telemetry.trackEvent(deepEqual({ eventName: 'add_block', properties: { blockType } })) + ).once() + ); + verify(telemetry.trackEvent(anything())).times(row.expected.length); + }); + }); + + test('ignores non-Deepnote notebooks', () => { + fireContentChange([addedCell(NotebookCellKind.Code)], [], JUPYTER); + + verify(telemetry.trackEvent(anything())).never(); + }); + }); + + suite('execute_cell', () => { + const rows: Array<{ + name: string; + languageId: string; + integrationId?: string; + integrations?: Array<{ id: string; name: string; type: string }>; + expected: { cellType: 'sql' | 'markdown' | 'code'; integrationType?: string }; + }> = [ + { name: 'python cell reports cellType code', languageId: 'python', expected: { cellType: 'code' } }, + { + name: 'the DataFrame SQL pseudo-integration maps to duckdb', + languageId: 'sql', + integrationId: DATAFRAME_SQL_INTEGRATION_ID, + expected: { cellType: 'sql', integrationType: 'duckdb' } + }, + { + name: 'a project integration id resolves to its type', + languageId: 'sql', + integrationId: 'int-1', + integrations: [{ id: 'int-1', name: 'PG', type: 'pgsql' }], + expected: { cellType: 'sql', integrationType: 'pgsql' } + }, + { + // integrations[].type is a free-form string in the .deepnote schema. + name: 'an unrecognized integration type reports unknown', + languageId: 'sql', + integrationId: 'int-1', + integrations: [{ id: 'int-1', name: 'X', type: 'not-a-real-integration-type' }], + expected: { cellType: 'sql', integrationType: 'unknown' } + }, + { + name: 'an id absent from the project omits integrationType', + languageId: 'sql', + integrationId: 'gone', + integrations: [], + expected: { cellType: 'sql' } + } + ]; + + rows.forEach((row) => { + test(row.name, () => { + if (row.integrations) { + stubProjectIntegrations(row.integrations); + } + + notebookCellExecutions.changeCellState( + executingCell(row.languageId, row.integrationId), + NotebookCellExecutionState.Executing + ); + + verify( + telemetry.trackEvent(deepEqual({ eventName: 'execute_cell', properties: { ...row.expected } })) + ).once(); + verify(telemetry.trackEvent(anything())).once(); + }); + }); + + test('ignores Pending and Idle transitions, and non-Deepnote notebooks', () => { + const cell = executingCell('python'); + + notebookCellExecutions.changeCellState(cell, NotebookCellExecutionState.Pending); + notebookCellExecutions.changeCellState(cell, NotebookCellExecutionState.Idle); + notebookCellExecutions.changeCellState( + executingCell('python', undefined, JUPYTER), + NotebookCellExecutionState.Executing + ); + + verify(telemetry.trackEvent(anything())).never(); + }); + }); +}); diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index c8d082ce46..5594b596e5 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -3,6 +3,7 @@ import { commands, window, workspace, type TreeView, RelativePattern, Uri, l10n import { serializeDeepnoteFile, type DeepnoteBlock, type DeepnoteFile } from '@deepnote/blocks'; import { convertDeepnoteToJupyterNotebooks, convertIpynbFileToDeepnoteFile } from '@deepnote/convert'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { DeepnoteTreeDataProvider } from './deepnoteTreeDataProvider'; import { @@ -24,6 +25,9 @@ import { buildSingleNotebookFile, buildSiblingNotebookFileUri } from './deepnote import { deepnoteFileExists } from './deepnoteSiblingFileAllocator'; import { isSnapshotFile } from './snapshots/snapshotFiles'; +/** Outcome of a tracked explorer command, so telemetry can separate user drop-off from real failures. */ +type CommandOutcome = 'completed' | 'cancelled' | 'failed'; + /** * Manages the Deepnote explorer tree view and its commands. Sibling `.deepnote` files are grouped * by `project.id`; project-scoped commands span the group, notebook-scoped ones a single leaf/child. @@ -36,7 +40,8 @@ export class DeepnoteExplorerView { constructor( @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, @inject(ILogger) private readonly logger: ILogger, - private readonly treeDataProvider: DeepnoteTreeDataProvider + private readonly treeDataProvider: DeepnoteTreeDataProvider, + private readonly analytics: ITelemetryService ) {} public activate(): void { @@ -109,7 +114,8 @@ export class DeepnoteExplorerView { * Never appends to `project.notebooks`. * @param sourceUri A sibling file used as the source for project-level metadata * @param existingNames Notebook names already in use across the project group (for uniqueness) - * @returns Object with notebook id and name if successful, or null if aborted/failed + * @returns Object with notebook id and name, or null if the user cancelled the name prompt + * @throws If `sourceUri` is not a readable Deepnote project file */ public async createNotebookSiblingFile( sourceUri: Uri, @@ -118,9 +124,7 @@ export class DeepnoteExplorerView { const sourceProject = await readDeepnoteProjectFile(sourceUri); if (!sourceProject?.project) { - await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - - return null; + throw new Error(l10n.t('Invalid Deepnote file format')); } const suggestedName = this.generateSuggestedNotebookName(existingNames); @@ -144,9 +148,9 @@ export class DeepnoteExplorerView { return { id: newNotebook.id, name: notebookName }; } - public async renameNotebook(treeItem: DeepnoteTreeItem): Promise { + public async renameNotebook(treeItem: DeepnoteTreeItem): Promise { if (!this.itemIsNotebookScoped(treeItem)) { - return; + return 'cancelled'; } try { @@ -156,7 +160,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return 'failed'; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -164,7 +168,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return 'failed'; } const currentName = targetNotebook.name; @@ -173,7 +177,7 @@ export class DeepnoteExplorerView { const newName = await this.promptForNotebookName(currentName, existingNames); if (!newName || newName === currentName) { - return; + return 'cancelled'; } // Flush the open document and re-read before rewriting, so we serialize the user's live cell @@ -183,7 +187,7 @@ export class DeepnoteExplorerView { l10n.t('Could not save "{0}" before renaming. The notebook was left unchanged.', currentName) ); - return; + return 'failed'; } const freshData = await readDeepnoteProjectFile(fileUri); @@ -192,7 +196,7 @@ export class DeepnoteExplorerView { if (!freshTarget) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return 'failed'; } freshTarget.name = newName; @@ -201,15 +205,19 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook renamed to: {0}', newName)); + + return 'completed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to rename notebook: {0}', errorMessage)); + + return 'failed'; } } - public async deleteNotebook(treeItem: DeepnoteTreeItem): Promise { + public async deleteNotebook(treeItem: DeepnoteTreeItem): Promise { if (!this.itemIsNotebookScoped(treeItem)) { - return; + return 'cancelled'; } try { @@ -219,7 +227,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return 'failed'; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -227,7 +235,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return 'failed'; } const notebookName = targetNotebook.name; @@ -239,7 +247,7 @@ export class DeepnoteExplorerView { ); if (confirmation !== l10n.t('Delete')) { - return; + return 'cancelled'; } // A single-notebook file's only non-init notebook is the file itself: delete the file. @@ -248,7 +256,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); await window.showInformationMessage(l10n.t('Notebook deleted: {0}', notebookName)); - return; + return 'completed'; } // Legacy multi-notebook file: remove the notebook from the array. @@ -260,9 +268,13 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook deleted: {0}', notebookName)); + + return 'completed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to delete notebook: {0}', errorMessage)); + + return 'failed'; } } @@ -276,9 +288,9 @@ export class DeepnoteExplorerView { await workspace.fs.delete(fileUri, { useTrash }); } - public async duplicateNotebook(treeItem: DeepnoteTreeItem): Promise { + public async duplicateNotebook(treeItem: DeepnoteTreeItem): Promise { if (!this.itemIsNotebookScoped(treeItem)) { - return; + return 'cancelled'; } try { @@ -288,7 +300,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return 'failed'; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -296,7 +308,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return 'failed'; } const existingNames = await this.collectNotebookNamesForProject(treeItem.context.projectId); @@ -317,7 +329,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook duplicated: {0}', newName)); - return; + return 'completed'; } // Legacy multi-notebook file: append the duplicate in place (existing behavior). @@ -334,15 +346,19 @@ export class DeepnoteExplorerView { }); await window.showInformationMessage(l10n.t('Notebook duplicated: {0}', newName)); + + return 'completed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to duplicate notebook: {0}', errorMessage)); + + return 'failed'; } } - public async renameProject(treeItem: DeepnoteTreeItem): Promise { + public async renameProject(treeItem: DeepnoteTreeItem): Promise { if (treeItem.extra.type !== DeepnoteTreeItemType.ProjectGroup) { - return; + return 'cancelled'; } const group = treeItem.extra.data; @@ -361,7 +377,7 @@ export class DeepnoteExplorerView { }); if (!newName || newName === currentName) { - return; + return 'cancelled'; } try { @@ -378,7 +394,7 @@ export class DeepnoteExplorerView { ) ); - return; + return 'failed'; } } @@ -411,9 +427,13 @@ export class DeepnoteExplorerView { } else { await window.showInformationMessage(l10n.t('Project renamed to: {0}', newName)); } + + return failedCount === 0 ? 'completed' : 'failed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to rename project: {0}', errorMessage)); + + return 'failed'; } } @@ -423,9 +443,10 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.OpenDeepnoteNotebook, (context: DeepnoteTreeItemContext) => - this.openNotebook(context) - ) + commands.registerCommand(Commands.OpenDeepnoteNotebook, async (context: DeepnoteTreeItemContext) => { + const outcome = await this.openNotebook(context); + this.analytics.trackEvent({ eventName: 'open_notebook', properties: { outcome } }); + }) ); this.extensionContext.subscriptions.push( @@ -437,56 +458,83 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.NewProject, () => this.newProject()) + commands.registerCommand(Commands.NewProject, async () => { + const outcome = await this.newProject(); + this.analytics.trackEvent({ eventName: 'create_project', properties: { outcome } }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ImportNotebook, () => this.importNotebook()) + commands.registerCommand(Commands.ImportNotebook, async () => { + const outcome = await this.importNotebook(); + this.analytics.trackEvent({ + eventName: 'import_notebook', + properties: { outcome, source: 'deepnote' } + }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ImportJupyterNotebook, () => this.importJupyterNotebook()) + commands.registerCommand(Commands.ImportJupyterNotebook, async () => { + const outcome = await this.importJupyterNotebook(); + this.analytics.trackEvent({ eventName: 'import_notebook', properties: { outcome, source: 'jupyter' } }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.NewNotebook, () => this.newNotebook()) + commands.registerCommand(Commands.NewNotebook, async () => { + const outcome = await this.newNotebook(); + this.analytics.trackEvent({ eventName: 'create_notebook', properties: { outcome, source: 'toolbar' } }); + }) ); // Context menu commands for tree items this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.RenameProject, (treeItem: DeepnoteTreeItem) => - this.renameProject(treeItem) - ) + commands.registerCommand(Commands.RenameProject, async (treeItem: DeepnoteTreeItem) => { + const outcome = await this.renameProject(treeItem); + this.analytics.trackEvent({ eventName: 'rename_project', properties: { outcome } }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.RenameNotebook, (treeItem: DeepnoteTreeItem) => - this.renameNotebook(treeItem) - ) + commands.registerCommand(Commands.RenameNotebook, async (treeItem: DeepnoteTreeItem) => { + const outcome = await this.renameNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'rename_notebook', properties: { outcome } }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.DeleteNotebook, (treeItem: DeepnoteTreeItem) => - this.deleteNotebook(treeItem) - ) + commands.registerCommand(Commands.DeleteNotebook, async (treeItem: DeepnoteTreeItem) => { + const outcome = await this.deleteNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'delete_notebook', properties: { outcome } }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.DuplicateNotebook, (treeItem: DeepnoteTreeItem) => - this.duplicateNotebook(treeItem) - ) + commands.registerCommand(Commands.DuplicateNotebook, async (treeItem: DeepnoteTreeItem) => { + const outcome = await this.duplicateNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'duplicate_notebook', properties: { outcome } }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.AddNotebookToProject, (treeItem: DeepnoteTreeItem) => - this.addNotebookToProject(treeItem) - ) + commands.registerCommand(Commands.AddNotebookToProject, async (treeItem: DeepnoteTreeItem) => { + const outcome = await this.addNotebookToProject(treeItem); + this.analytics.trackEvent({ + eventName: 'create_notebook', + properties: { outcome, source: 'project_menu' } + }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ExportNotebook, (treeItem: DeepnoteTreeItem) => - this.exportNotebook(treeItem) - ) + commands.registerCommand(Commands.ExportNotebook, async (treeItem: DeepnoteTreeItem) => { + const { outcome, format } = await this.exportNotebook(treeItem); + this.analytics.trackEvent({ + eventName: 'export_notebook', + properties: { outcome, ...(format ? { format } : {}) } + }); + }) ); } @@ -664,7 +712,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); } - private async openNotebook(context: DeepnoteTreeItemContext): Promise { + private async openNotebook(context: DeepnoteTreeItemContext): Promise { try { const fileUri = Uri.file(context.filePath); const document = await workspace.openNotebookDocument(fileUri); @@ -673,10 +721,14 @@ export class DeepnoteExplorerView { preview: false, preserveFocus: false }); + + return 'completed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(`Failed to open notebook: ${errorMessage}`); + + return 'failed'; } } @@ -740,7 +792,7 @@ export class DeepnoteExplorerView { } } - private async newProject(): Promise { + private async newProject(): Promise { if (!workspace.workspaceFolders || workspace.workspaceFolders.length === 0) { const selection = await window.showInformationMessage( l10n.t('No workspace folder is open. Would you like to open a folder?'), @@ -752,7 +804,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return; + return 'cancelled'; } const projectName = await window.showInputBox({ @@ -768,7 +820,7 @@ export class DeepnoteExplorerView { }); if (!projectName) { - return; + return 'cancelled'; } try { @@ -781,7 +833,7 @@ export class DeepnoteExplorerView { await workspace.fs.stat(fileUri); await window.showErrorMessage(l10n.t('A file named "{0}" already exists in this workspace.', fileName)); - return; + return 'failed'; } catch { // File doesn't exist, continue } @@ -835,20 +887,24 @@ export class DeepnoteExplorerView { preserveFocus: false, preview: false }); + + return 'completed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t(`Failed to create project: {0}`, errorMessage)); + + return 'failed'; } } - private async newNotebook(): Promise { + private async newNotebook(): Promise { const activeEditor = window.activeNotebookEditor; if (!activeEditor || activeEditor.notebook.notebookType !== 'deepnote') { await window.showErrorMessage(l10n.t('No active Deepnote file opened. Please open a Deepnote file first.')); - return; + return 'failed'; } const document = activeEditor.notebook; @@ -870,9 +926,13 @@ export class DeepnoteExplorerView { await window.showInformationMessage(l10n.t('Created new notebook: {0}', result.name)); } + + return result !== null ? 'completed' : 'cancelled'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to add notebook: {0}', errorMessage)); + + return 'failed'; } } @@ -952,7 +1012,7 @@ export class DeepnoteExplorerView { }; } - private async importNotebook(): Promise { + private async importNotebook(): Promise { if (!workspace.workspaceFolders || workspace.workspaceFolders.length === 0) { const selection = await window.showInformationMessage( l10n.t('No workspace folder is open. Would you like to open a folder?'), @@ -964,7 +1024,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return; + return 'cancelled'; } const fileUris = await window.showOpenDialog({ @@ -978,7 +1038,7 @@ export class DeepnoteExplorerView { }); if (!fileUris || fileUris.length === 0) { - return; + return 'cancelled'; } try { @@ -998,14 +1058,14 @@ export class DeepnoteExplorerView { l10n.t('A file named "{0}" already exists in this workspace.', fileName) ); - return; + return 'failed'; } catch { // File doesn't exist, continue } } if (!(await this.checkJupyterImportTargetsAvailable(jupyterUris, workspaceFolder.uri))) { - return; + return 'failed'; } // Import deepnote files @@ -1029,14 +1089,18 @@ export class DeepnoteExplorerView { } this.treeDataProvider.refresh(); + + return numberOfNotebooks > 0 ? 'completed' : 'failed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; - await window.showErrorMessage(`Failed to import notebook: ${errorMessage}`); + await window.showErrorMessage(l10n.t('Failed to import notebook: {0}', errorMessage)); + + return 'failed'; } } - private async importJupyterNotebook(): Promise { + private async importJupyterNotebook(): Promise { if (!workspace.workspaceFolders || workspace.workspaceFolders.length === 0) { const selection = await window.showInformationMessage( l10n.t('No workspace folder is open. Would you like to open a folder?'), @@ -1048,7 +1112,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return; + return 'cancelled'; } const fileUris = await window.showOpenDialog({ @@ -1062,14 +1126,14 @@ export class DeepnoteExplorerView { }); if (!fileUris || fileUris.length === 0) { - return; + return 'cancelled'; } try { const workspaceFolder = workspace.workspaceFolders[0]; if (!(await this.checkJupyterImportTargetsAvailable(fileUris, workspaceFolder.uri))) { - return; + return 'failed'; } const failedCount = await this.convertJupyterUrisToDeepnoteFiles(fileUris, workspaceFolder.uri); @@ -1085,16 +1149,20 @@ export class DeepnoteExplorerView { } this.treeDataProvider.refresh(); + + return numberOfNotebooks > 0 ? 'completed' : 'failed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t(`Failed to import Jupyter notebook: {0}`, errorMessage)); + + return 'failed'; } } - private async addNotebookToProject(treeItem: DeepnoteTreeItem): Promise { + private async addNotebookToProject(treeItem: DeepnoteTreeItem): Promise { if (treeItem.extra.type !== DeepnoteTreeItemType.ProjectGroup) { - return; + return 'cancelled'; } const group = treeItem.extra.data; @@ -1103,7 +1171,7 @@ export class DeepnoteExplorerView { if (!sourceFile) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return 'failed'; } try { @@ -1116,16 +1184,20 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Created new notebook: {0}', result.name)); } + + return result !== null ? 'completed' : 'cancelled'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to add notebook: {0}', errorMessage)); + + return 'failed'; } } /** Exports a single notebook (single-notebook leaf or legacy in-file notebook) to Jupyter. */ - private async exportNotebook(treeItem: DeepnoteTreeItem): Promise { + private async exportNotebook(treeItem: DeepnoteTreeItem): Promise<{ outcome: CommandOutcome; format?: string }> { if (!this.itemIsNotebookScoped(treeItem)) { - return; + return { outcome: 'cancelled' }; } try { @@ -1134,7 +1206,7 @@ export class DeepnoteExplorerView { }); if (!format) { - return; + return { outcome: 'cancelled' }; } const fileUri = Uri.file(treeItem.context.filePath); @@ -1143,7 +1215,7 @@ export class DeepnoteExplorerView { if (!projectData?.project) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return { outcome: 'failed' }; } const outputFolder = await window.showOpenDialog({ @@ -1155,7 +1227,7 @@ export class DeepnoteExplorerView { }); if (!outputFolder?.length) { - return; + return { outcome: 'cancelled' }; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -1163,7 +1235,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return { outcome: 'failed' }; } const filteredProject = { @@ -1198,7 +1270,7 @@ export class DeepnoteExplorerView { ); if (result !== overwrite) { - return; + return { outcome: 'cancelled' }; } } @@ -1208,9 +1280,13 @@ export class DeepnoteExplorerView { ); await window.showInformationMessage(l10n.t('Exported 1 notebook successfully')); + + return { outcome: 'completed', format: format.value }; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to export: {0}', errorMessage)); + + return { outcome: 'failed' }; } } } diff --git a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts index 9d0df7b06b..fb28924a41 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts @@ -2,7 +2,7 @@ import { deserializeDeepnoteFile, ExecutableBlock, serializeDeepnoteFile, type D import { assert, expect } from 'chai'; import esmock from 'esmock'; import * as sinon from 'sinon'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; import { FileType, Uri, @@ -19,6 +19,7 @@ import { DeepnoteExplorerView } from './deepnoteExplorerView'; import { DeepnoteTreeDataProvider } from './deepnoteTreeDataProvider'; import { createWorkspaceFolder } from './deepnoteTestHelpers'; import { DeepnoteTreeItem, DeepnoteTreeItemType, type DeepnoteTreeItemContext } from './deepnoteTreeItem'; +import { ITelemetryService } from '../../platform/analytics/types'; import { Commands } from '../../platform/common/constants'; import type { IExtensionContext } from '../../platform/common/types'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; @@ -116,6 +117,7 @@ suite('DeepnoteExplorerView', () => { let explorerView: DeepnoteExplorerView; let mockExtensionContext: IExtensionContext; let mockLogger: ILogger; + let mockAnalytics: ITelemetryService; setup(() => { resetVSCodeMocks(); @@ -124,10 +126,12 @@ suite('DeepnoteExplorerView', () => { mockExtensionContext = makeExtensionContext(); mockLogger = createMockLogger(); + mockAnalytics = instance(mock()); explorerView = new DeepnoteExplorerView( mockExtensionContext, mockLogger, - new DeepnoteTreeDataProvider(mockLogger) + new DeepnoteTreeDataProvider(mockLogger), + mockAnalytics ); explorerView.activate(); }); @@ -244,6 +248,8 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { let mockContext: IExtensionContext; let sandbox: sinon.SinonSandbox; let uuidStubs: sinon.SinonStub[] = []; + let mockAnalyticsMock: ITelemetryService; + let mockAnalytics: ITelemetryService; setup(() => { sandbox = sinon.createSandbox(); @@ -254,7 +260,14 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { mockContext = makeExtensionContext(); const mockLogger = createMockLogger(); - explorerView = new DeepnoteExplorerView(mockContext, mockLogger, new DeepnoteTreeDataProvider(mockLogger)); + mockAnalyticsMock = mock(); + mockAnalytics = instance(mockAnalyticsMock); + explorerView = new DeepnoteExplorerView( + mockContext, + mockLogger, + new DeepnoteTreeDataProvider(mockLogger), + mockAnalytics + ); explorerView.activate(); }); @@ -265,6 +278,25 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { resetVSCodeMocks(); }); + suite('newNotebook', () => { + test('reports failed, not cancelled, when no Deepnote file is open', async () => { + // Reachable from the Command Palette, where the command is not gated on an active editor. + when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn(undefined); + when(mockedVSCodeNamespaces.window.showErrorMessage(anything())).thenReturn(Promise.resolve(undefined)); + + await handlerFor(Commands.NewNotebook)(); + + verify( + mockAnalyticsMock.trackEvent( + deepEqual({ + eventName: 'create_notebook', + properties: { outcome: 'failed', source: 'toolbar' } + }) + ) + ).once(); + }); + }); + suite('newProject', () => { test('should create a new project with valid input', async () => { const projectName = 'My Test Project'; @@ -484,7 +516,8 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { explorerView = new importModule.DeepnoteExplorerView( mockContext, createMockLogger(), - new DeepnoteTreeDataProvider(createMockLogger()) + new DeepnoteTreeDataProvider(createMockLogger()), + instance(mock()) ); explorerView.activate(); }); @@ -674,7 +707,8 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { explorerView = new importModule.DeepnoteExplorerView( mockContext, createMockLogger(), - new DeepnoteTreeDataProvider(createMockLogger()) + new DeepnoteTreeDataProvider(createMockLogger()), + instance(mock()) ); explorerView.activate(); }); @@ -742,7 +776,8 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { const partialExplorer = new failingModule.DeepnoteExplorerView( mockContext, createMockLogger(), - new DeepnoteTreeDataProvider(createMockLogger()) + new DeepnoteTreeDataProvider(createMockLogger()), + instance(mock()) ); partialExplorer.activate(); @@ -797,7 +832,8 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { const failedExplorer = new failingModule.DeepnoteExplorerView( mockContext, createMockLogger(), - new DeepnoteTreeDataProvider(createMockLogger()) + new DeepnoteTreeDataProvider(createMockLogger()), + instance(mock()) ); failedExplorer.activate(); @@ -2383,7 +2419,8 @@ suite('DeepnoteExplorerView - Sibling-file command semantics', () => { explorerView = new DeepnoteExplorerView( mockContext, createMockLogger(), - new DeepnoteTreeDataProvider(createMockLogger()) + new DeepnoteTreeDataProvider(createMockLogger()), + instance(mock()) ); explorerView.activate(); }); @@ -2459,7 +2496,12 @@ suite('DeepnoteExplorerView - Sibling-file command semantics', () => { uuidStubs.push(createUuidMock(['new-nb', 'new-group', 'new-block'])); const mockProvider = mock(); - explorerView = new DeepnoteExplorerView(mockContext, createMockLogger(), instance(mockProvider)); + explorerView = new DeepnoteExplorerView( + mockContext, + createMockLogger(), + instance(mockProvider), + instance(mock()) + ); explorerView.activate(); const treeItem: Partial = { diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index 8c0b93cc4a..fb71649919 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts @@ -44,6 +44,7 @@ import { } from '../../kernels/jupyter/types'; import { IJupyterKernelSpec, IKernelProvider } from '../../kernels/types'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IPythonExtensionChecker } from '../../platform/api/types'; import { Cancellation, isCancellationError } from '../../platform/common/cancellation'; import { JVSC_EXTENSION_ID, STANDARD_OUTPUT_CHANNEL } from '../../platform/common/constants'; @@ -98,7 +99,8 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, private readonly notebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, @inject(IDeepnoteToolkitInstaller) private readonly toolkitInstaller: IDeepnoteToolkitInstaller, - @inject(IServerHandleRegistry) private readonly serverHandleRegistry: IServerHandleRegistry + @inject(IServerHandleRegistry) private readonly serverHandleRegistry: IServerHandleRegistry, + @inject(ITelemetryService) private readonly analytics: ITelemetryService ) {} public activate() { @@ -770,6 +772,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, const result = await this.setupKernelForEnvironment(notebook, selectedEnvironment, notebookKey, token); if (result) { + this.analytics.trackEvent({ eventName: 'select_environment' }); logger.info(`Environment "${selectedEnvironment.name}" configured for ${getDisplayPath(notebook.uri)}`); } diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts index d5f71bc77c..90ef280a22 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts @@ -1,6 +1,6 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; import { DeepnoteKernelAutoSelector } from './deepnoteKernelAutoSelector.node'; import { createMockChildProcess } from '../../kernels/deepnote/deepnoteTestHelpers.node'; import { ServerHandleRegistry } from '../../kernels/deepnote/deepnoteServerHandleRegistry.node'; @@ -13,6 +13,7 @@ import { IDeepnoteToolkitInstaller } from '../../kernels/deepnote/types'; import { IControllerRegistration, IVSCodeNotebookController } from '../controllers/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IDisposableRegistry, IOutputChannel } from '../../platform/common/types'; import { IPythonExtensionChecker } from '../../platform/api/types'; import { IJupyterRequestCreator } from '../../kernels/jupyter/types'; @@ -45,6 +46,7 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { let mockNotebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper; let mockOutputChannel: IOutputChannel; let mockToolkitInstaller: IDeepnoteToolkitInstaller; + let mockTelemetryService: ITelemetryService; let mockProgress: { report(value: { message?: string; increment?: number }): void }; let mockCancellationToken: CancellationToken; @@ -74,6 +76,7 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { mockToolkitInstaller = mock(); mockNotebookEnvironmentMapper = mock(); mockOutputChannel = mock(); + mockTelemetryService = mock(); mockProgress = { report: sandbox.stub() }; mockCancellationToken = mock(); @@ -141,7 +144,8 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { instance(mockNotebookEnvironmentMapper), instance(mockOutputChannel), instance(mockToolkitInstaller), - registry + registry, + instance(mockTelemetryService) ); }); @@ -376,6 +380,60 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { }); }); + suite('ensureEnvironmentConfiguredBeforeExecution', () => { + // The first-run picker is a second environment-selection path alongside the environments view; + // both must report select_environment or the metric only counts users who switch. + function arrangeFirstRunPicker(picked: DeepnoteEnvironment | undefined) { + when(mockCancellationToken.isCancellationRequested).thenReturn(false); + when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).thenReturn(undefined); + when(mockNotebookEnvironmentMapper.setEnvironmentForNotebook(anything(), anything())).thenResolve(); + when(mockEnvironmentManager.waitForInitialization()).thenResolve(); + when(mockEnvironmentManager.listEnvironments()).thenReturn(picked ? [picked] : []); + when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenResolve( + picked ? ({ label: picked.name, environment: picked } as any) : undefined + ); + } + + test('should track select_environment after the execution picker configures an environment', async () => { + const mockEnvironment = createMockEnvironment('env-1', 'Environment 1'); + arrangeFirstRunPicker(mockEnvironment); + const setupStub = sandbox.stub(selector as any, 'setupKernelForEnvironment').resolves(true); + + const result = await selector.ensureEnvironmentConfiguredBeforeExecution( + mockNotebook, + instance(mockCancellationToken) + ); + + assert.isTrue(result, 'Environment should be reported as configured'); + assert.isTrue(setupStub.calledOnce, 'Kernel setup should have run'); + verify(mockTelemetryService.trackEvent(deepEqual({ eventName: 'select_environment' }))).once(); + }); + + test('should not track select_environment when the picker is cancelled or setup fails', async () => { + arrangeFirstRunPicker(undefined); + + assert.isFalse( + await selector.ensureEnvironmentConfiguredBeforeExecution( + mockNotebook, + instance(mockCancellationToken) + ), + 'Cancelling the picker should not configure an environment' + ); + + arrangeFirstRunPicker(createMockEnvironment('env-1', 'Environment 1')); + sandbox.stub(selector as any, 'setupKernelForEnvironment').resolves(false); + + assert.isFalse( + await selector.ensureEnvironmentConfiguredBeforeExecution( + mockNotebook, + instance(mockCancellationToken) + ), + 'Failed setup should not report success' + ); + verify(mockTelemetryService.trackEvent(anything())).never(); + }); + }); + suite('ensureKernelSelected', () => { test('should return false when no environment ID is assigned to the notebook', async () => { // Mock environment mapper to return null (no environment assigned) diff --git a/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts b/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts index 49da7ee7c6..524030603e 100644 --- a/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts +++ b/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts @@ -2,6 +2,7 @@ import { l10n, TabInputNotebook, Uri, window, workspace, type Disposable, type N import { serializeDeepnoteFile } from '@deepnote/blocks'; import { isSingleNotebookDeepnoteFile, splitByNotebooks } from '@deepnote/convert'; +import { ITelemetryService } from '../../platform/analytics/types'; import { ILogger } from '../../platform/logging/types'; import type { IDeepnoteNotebookEnvironmentMapper } from '../../kernels/deepnote/types'; import { DEEPNOTE_NOTEBOOK_TYPE } from '../../kernels/deepnote/types'; @@ -23,6 +24,8 @@ const MAX_LEGACY_ALLOCATION_ATTEMPTS = 10_000; * The environment mapper is undefined on the web target, where env migration is a desktop-only no-op. */ export class DeepnoteMultiNotebookSplitter { + private readonly analytics: ITelemetryService; + private readonly disposables: Disposable[] = []; private readonly envMapper: IDeepnoteNotebookEnvironmentMapper | undefined; @@ -39,12 +42,14 @@ export class DeepnoteMultiNotebookSplitter { envMapper: IDeepnoteNotebookEnvironmentMapper | undefined, refreshTree: () => void, logger: ILogger, - exists: (uri: Uri) => Promise + exists: (uri: Uri) => Promise, + analytics: ITelemetryService ) { this.envMapper = envMapper; this.refreshTree = refreshTree; this.logger = logger; this.exists = exists; + this.analytics = analytics; } public activate(): Disposable[] { @@ -102,14 +107,27 @@ export class DeepnoteMultiNotebookSplitter { ); if (selection === SPLIT_ACTION) { - await this.splitFile(fileUri); + // splitFile reports 0 when the split failed and was rolled back. + const notebookCount = await this.splitFile(fileUri); + this.analytics.trackEvent({ + eventName: 'split_notebook', + properties: + notebookCount > 0 + ? { notebookCount, outcome: 'completed' } + : { notebookCount: file.project.notebooks.length, outcome: 'failed' } + }); + } else { + this.analytics.trackEvent({ + eventName: 'split_notebook', + properties: { notebookCount: file.project.notebooks.length, outcome: 'cancelled' } + }); } } catch (error) { this.logger.error(`Failed to inspect Deepnote file for multi-notebook split: ${fileUri.toString()}`, error); } } - private async splitFile(fileUri: Uri): Promise { + private async splitFile(fileUri: Uri): Promise { // Compensations for each applied step, unwound in reverse on any failure so the split is all-or-nothing. const rollbacks: Array<() => Thenable> = []; let renamed = false; @@ -134,7 +152,7 @@ export class DeepnoteMultiNotebookSplitter { l10n.t('Could not save the file before splitting. The file was left unchanged.') ); - return; + return 0; } } @@ -145,6 +163,13 @@ export class DeepnoteMultiNotebookSplitter { // Write all children before retiring the original (see step below). const entries = splitByNotebooks(deepnoteFile, getFileStem(fileUri)); + + // Guards the retire below: with no entries the rename would strand the original with no + // replacement while still reporting success. + if (entries.length === 0) { + throw new Error(l10n.t('The file has no notebooks that can be split into separate files.')); + } + const reserved = new Set(); const newUris: Uri[] = []; const encoder = new TextEncoder(); @@ -193,6 +218,8 @@ export class DeepnoteMultiNotebookSplitter { this.refreshTree(); await window.showInformationMessage(l10n.t('Split into {0} files.', newUris.length)); + + return newUris.length; } catch (error) { // Unwind every applied step so the original is left as it was found (or an honest message if it can't be). this.logger.error(`Failed to split Deepnote file: ${fileUri.toString()}`, error); @@ -201,6 +228,8 @@ export class DeepnoteMultiNotebookSplitter { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(this.describeSplitFailure({ errorMessage, renamed, restored })); + + return 0; } } diff --git a/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.ts b/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.ts index dac3395cf0..0418541edd 100644 --- a/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.ts @@ -1,8 +1,9 @@ import { deserializeDeepnoteFile, serializeDeepnoteFile, type DeepnoteFile } from '@deepnote/blocks'; import { assert } from 'chai'; -import { anything, instance, mock, when } from 'ts-mockito'; +import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; import { EventEmitter, FileType, NotebookDocument, TabGroups, TabInputNotebook, Uri } from 'vscode'; +import { ITelemetryService } from '../../platform/analytics/types'; import type { IDeepnoteNotebookEnvironmentMapper } from '../../kernels/deepnote/types'; import type { ILogger } from '../../platform/logging/types'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; @@ -64,6 +65,7 @@ suite('DeepnoteMultiNotebookSplitter', () => { let failWriteAfterCreateFor: string | undefined; // Filenames passed to workspace.fs.delete (rollback cleanup), in call order. let deleteTargets: string[]; + let mockTelemetryService: ITelemetryService; const logger: ILogger = { error: () => undefined, @@ -154,6 +156,16 @@ suite('DeepnoteMultiNotebookSplitter', () => { } as unknown as NotebookDocument & { readonly _saved: boolean }; } + function hasTrackedAnEvent(): boolean { + try { + verify(mockTelemetryService.trackEvent(anything())).atLeast(1); + + return true; + } catch { + return false; + } + } + setup(() => { resetVSCodeMocks(); callLog = []; @@ -165,6 +177,7 @@ suite('DeepnoteMultiNotebookSplitter', () => { failWriteFor = undefined; failWriteAfterCreateFor = undefined; deleteTargets = []; + mockTelemetryService = mock(); // Re-stub the open-notebook event with our own emitter so tests can fire opens. onDidOpen = new EventEmitter(); @@ -193,7 +206,8 @@ suite('DeepnoteMultiNotebookSplitter', () => { }, logger, // `exists` probe injected directly (mirrors deepnoteFileExists, but synchronous-set-backed). - (uri: Uri) => Promise.resolve(existingOnDisk.has(basename(uri))) + (uri: Uri) => Promise.resolve(existingOnDisk.has(basename(uri))), + instance(mockTelemetryService) ); splitter.activate(); }); @@ -403,7 +417,8 @@ suite('DeepnoteMultiNotebookSplitter', () => { refreshTreeCount++; }, logger, - (uri: Uri) => Promise.resolve(existingOnDisk.has(basename(uri))) + (uri: Uri) => Promise.resolve(existingOnDisk.has(basename(uri))), + instance(mock()) ); splitterWithEnv.activate(); @@ -582,7 +597,8 @@ suite('DeepnoteMultiNotebookSplitter', () => { refreshTreeCount++; }, logger, - (uri: Uri) => Promise.resolve(existingOnDisk.has(basename(uri))) + (uri: Uri) => Promise.resolve(existingOnDisk.has(basename(uri))), + instance(mock()) ); envSplitter.activate(); @@ -713,6 +729,104 @@ suite('DeepnoteMultiNotebookSplitter', () => { }); }); + suite('telemetry outcomes', () => { + test('a dismissed prompt reports split_notebook cancelled with the parsed notebook count', async () => { + const file = makeFile([ + makeNotebook('n1', 'Alpha', 'a'), + makeNotebook('n2', 'Beta', 'b'), + makeNotebook('n3', 'Gamma', 'c') + ]); + stubReadFile(file); + // Default prompt resolves to dismiss. + + onDidOpen.fire(notebookDoc(Uri.file('/ws/multi.deepnote'))); + await waitFor(hasTrackedAnEvent); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'split_notebook', + properties: { notebookCount: 3, outcome: 'cancelled' } + }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + + test('a successful split reports split_notebook completed with the number of files created', async () => { + const file = makeFile([makeNotebook('n1', 'Alpha', 'a'), makeNotebook('n2', 'Beta', 'b')]); + stubReadFile(file); + acceptSplit(); + + onDidOpen.fire(notebookDoc(Uri.file('/ws/multi.deepnote'))); + await waitFor(hasTrackedAnEvent); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'split_notebook', + properties: { notebookCount: 2, outcome: 'completed' } + }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + + test('a failed split (child write rejects, rollback) reports split_notebook failed with the parsed notebook count', async () => { + const file = makeFile([ + makeNotebook('n1', 'Alpha', 'a'), + makeNotebook('n2', 'Beta', 'b'), + makeNotebook('n3', 'Gamma', 'c') + ]); + stubReadFile(file); + acceptSplit(); + failWriteFor = 'multi-beta.deepnote'; + + onDidOpen.fire(notebookDoc(Uri.file('/ws/multi.deepnote'))); + await waitFor(hasTrackedAnEvent); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'split_notebook', + properties: { notebookCount: 3, outcome: 'failed' } + }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + + test('a split that yields no files leaves the original in place and reports failed', async () => { + // Every notebook is the init notebook, so splitByNotebooks produces nothing. Retiring the + // original here would strand the user with no replacement file. + const file = makeFile( + [makeNotebook('init-1', 'Init', 'a'), makeNotebook('init-1', 'Also Init', 'b')], + 'init-1' + ); + stubReadFile(file); + acceptSplit(); + + onDidOpen.fire(notebookDoc(Uri.file('/ws/multi.deepnote'))); + await waitFor(hasTrackedAnEvent); + + assert.strictEqual(writeTargets.length, 0, 'must not write any child file'); + assert.strictEqual( + renameOps.length, + 0, + 'the original must NEVER be retired when no children were produced' + ); + verify(mockedVSCodeNamespaces.window.showErrorMessage(anything())).atLeast(1); + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'split_notebook', + properties: { notebookCount: 2, outcome: 'failed' } + }) + ) + ).once(); + }); + }); + suite('init shape', () => { test('a legacy [init, main] file splits into an init file + a main file that still references initNotebookId', async () => { const file = makeFile( diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts index d91608c15a..27865247d0 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts @@ -17,6 +17,7 @@ import z from 'zod'; import { logger } from '../../platform/logging'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IConfigurationService, IDisposableRegistry } from '../../platform/common/types'; import { Commands } from '../../platform/common/constants'; import { notebookUpdaterUtils } from '../../kernels/execution/notebookUpdater'; @@ -151,6 +152,7 @@ export function getNextDeepnoteVariableName(cells: NotebookCell[], prefix: 'df' @injectable() export class DeepnoteNotebookCommandListener implements IExtensionSyncActivationService { constructor( + @inject(ITelemetryService) private readonly analytics: ITelemetryService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry ) {} @@ -264,6 +266,8 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert SQL block')); } + this.trackAddBlock('sql'); + const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); editor.selection = notebookRange; @@ -305,6 +309,8 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert big number chart block')); } + this.trackAddBlock('big-number'); + const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); editor.selection = notebookRange; @@ -359,6 +365,8 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new WrappedError(l10n.t('Failed to insert chart block')); } + this.trackAddBlock('visualization'); + const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -406,6 +414,8 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert input block')); } + this.trackAddBlock(blockType); + const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); editor.selection = notebookRange; @@ -539,6 +549,8 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert text block')); } + this.trackAddBlock(textBlockType); + const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); editor.selection = notebookRange; @@ -554,6 +566,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation undefined, ConfigurationTarget.Workspace ); + this.analytics.trackEvent({ eventName: 'toggle_snapshots', properties: { enabled: false } }); void window.showInformationMessage(l10n.t('Snapshots disabled for this workspace.')); } catch (error) { logger.error('Failed to disable snapshots', error); @@ -569,9 +582,14 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation undefined, ConfigurationTarget.Workspace ); + this.analytics.trackEvent({ eventName: 'toggle_snapshots', properties: { enabled: true } }); } catch (error) { logger.error('Failed to enable snapshots', error); void window.showErrorMessage(l10n.t('Failed to enable snapshots.')); } } + + private trackAddBlock(blockType: string): void { + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType } }); + } } diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts index ff600a2bf9..dd54a44e5e 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts @@ -1,6 +1,6 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; -import { when, reset, anything } from 'ts-mockito'; +import { when, reset, anything, mock, instance } from 'ts-mockito'; import { NotebookCell, NotebookDocument, @@ -18,6 +18,7 @@ import { InputBlockType } from './deepnoteNotebookCommandListener'; import { formatInputBlockCellContent, getInputBlockLanguage } from './inputBlockContentFormatter'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IConfigurationService, IDisposable } from '../../platform/common/types'; import * as notebookUpdater from '../../kernels/execution/notebookUpdater'; import { createMockedNotebookDocument } from '../../test/datascience/editor-integration/helpers'; @@ -31,6 +32,7 @@ suite('DeepnoteNotebookCommandListener', () => { let disposables: IDisposable[]; let sandbox: sinon.SinonSandbox; let mockConfigService: IConfigurationService; + let mockTelemetryService: ITelemetryService; function createMockConfigService(): IConfigurationService { return { @@ -44,7 +46,12 @@ suite('DeepnoteNotebookCommandListener', () => { sandbox = sinon.createSandbox(); disposables = []; mockConfigService = createMockConfigService(); - commandListener = new DeepnoteNotebookCommandListener(mockConfigService, disposables); + mockTelemetryService = mock(); + commandListener = new DeepnoteNotebookCommandListener( + instance(mockTelemetryService), + mockConfigService, + disposables + ); }); teardown(() => { @@ -89,7 +96,11 @@ suite('DeepnoteNotebookCommandListener', () => { // Create new instance and activate again const disposables2: IDisposable[] = []; - const commandListener2 = new DeepnoteNotebookCommandListener(createMockConfigService(), disposables2); + const commandListener2 = new DeepnoteNotebookCommandListener( + instance(mockTelemetryService), + createMockConfigService(), + disposables2 + ); commandListener2.activate(); // Both should register the same number of commands diff --git a/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.ts b/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.ts index 0e970ff2a5..2e594c327d 100644 --- a/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.ts +++ b/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.ts @@ -14,6 +14,7 @@ import { } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IDisposableRegistry } from '../../platform/common/types'; import { Commands } from '../../platform/common/constants'; @@ -32,7 +33,10 @@ export class DeepnoteNotebookInfoStatusBar implements IExtensionSyncActivationSe private statusBarItem: StatusBarItem | undefined; - constructor(@inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry) { + constructor( + @inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry, + @inject(ITelemetryService) private readonly analytics: ITelemetryService + ) { disposableRegistry.push(this); } @@ -115,6 +119,7 @@ export class DeepnoteNotebookInfoStatusBar implements IExtensionSyncActivationSe const details = this.formatNotebookDetails(notebook); await env.clipboard.writeText(details); + this.analytics.trackEvent({ eventName: 'copy_notebook_details' }); await window.showInformationMessage(l10n.t('Copied Deepnote notebook details to clipboard.')); } diff --git a/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.unit.test.ts index a78f6b647d..4f67c966f0 100644 --- a/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.unit.test.ts @@ -1,5 +1,5 @@ import { assert, expect } from 'chai'; -import { anything, capture, verify, when } from 'ts-mockito'; +import { anything, capture, deepEqual, instance, mock, verify, when } from 'ts-mockito'; import { EventEmitter, NotebookDocument, @@ -12,6 +12,7 @@ import { import { DeepnoteNotebookInfoStatusBar } from './deepnoteNotebookInfoStatusBar'; import { Commands } from '../../platform/common/constants'; import { mockedVSCode, mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; +import type { ITelemetryService } from '../../platform/analytics/types'; import type { IDisposableRegistry } from '../../platform/common/types'; /** @@ -81,12 +82,14 @@ suite('DeepnoteNotebookInfoStatusBar', () => { let disposableRegistry: IDisposableRegistry; let activeEditorEmitter: EventEmitter; let docChangeEmitter: EventEmitter; + let mockTelemetryService: ITelemetryService; setup(() => { resetVSCodeMocks(); fakeItem = createFakeStatusBarItem(); disposableRegistry = []; + mockTelemetryService = mock(); // Real emitters drive the active-editor / document-change subscriptions; the status bar // subscribes through `.event` (which honours thisArg + the disposables array it passes). @@ -99,7 +102,7 @@ suite('DeepnoteNotebookInfoStatusBar', () => { when(mockedVSCodeNamespaces.window.onDidChangeActiveNotebookEditor).thenReturn(activeEditorEmitter.event); when(mockedVSCodeNamespaces.workspace.onDidChangeNotebookDocument).thenReturn(docChangeEmitter.event); - statusBar = new DeepnoteNotebookInfoStatusBar(disposableRegistry); + statusBar = new DeepnoteNotebookInfoStatusBar(disposableRegistry, instance(mockTelemetryService)); }); teardown(() => { @@ -210,6 +213,8 @@ suite('DeepnoteNotebookInfoStatusBar', () => { ].join('\n'); assert.strictEqual(clipboardText, expected, 'clipboard must contain the full notebook detail block'); + verify(mockTelemetryService.trackEvent(deepEqual({ eventName: 'copy_notebook_details' }))).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); }); test('CopyNotebookDetails warns and writes nothing when there is no active deepnote notebook', async () => { @@ -223,6 +228,7 @@ suite('DeepnoteNotebookInfoStatusBar', () => { verify(mockedVSCodeNamespaces.window.showWarningMessage(anything())).once(); const clipboardText = await mockedVSCode.env!.clipboard.readText(); assert.strictEqual(clipboardText, '', 'nothing should be copied when there is no active deepnote notebook'); + verify(mockTelemetryService.trackEvent(anything())).never(); }); test('dispose() disposes the status bar item and clears its subscriptions', () => { diff --git a/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.ts b/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.ts index cc696a395c..45a14eebd2 100644 --- a/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.ts +++ b/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.ts @@ -4,6 +4,7 @@ import { CancellationError, CancellationToken, ProgressLocation, Uri, commands, import { BigQueryAuthMethods } from '@deepnote/database-integrations'; import { IExtensionSyncActivationService } from '../../../../platform/activation/types'; +import type { CommandOutcome } from '../../../../platform/analytics/types'; import { Commands } from '../../../../platform/common/constants'; import { IExtensionContext } from '../../../../platform/common/types'; import { Integrations } from '../../../../platform/common/utils/localize'; @@ -43,22 +44,25 @@ export class FederatedAuthCommandHandlerNode implements IExtensionSyncActivation ); } - /** Core flow. Public so tests can drive the handler without `commands.executeCommand`. */ - public async authenticate(integrationId: string): Promise { + /** + * Core flow. Public so tests can drive the handler without `commands.executeCommand`. Returns the + * outcome so the invoking UI layer can attach it to its telemetry. + */ + public async authenticate(integrationId: string): Promise { if (typeof integrationId !== 'string' || integrationId.length === 0) { logger.warn( `FederatedAuthCommandHandlerNode: invoked without a valid integrationId (received: ${String( integrationId )})` ); - return; + return 'failed'; } const integration = await this.integrationStorage.getIntegrationConfig(integrationId); if (!integration) { logger.warn(`FederatedAuthCommandHandlerNode: integration "${integrationId}" not found.`); void window.showErrorMessage(Integrations.federatedAuthIntegrationNotFound(integrationId)); - return; + return 'failed'; } if (integration.type !== 'big-query' || integration.metadata.authMethod !== BigQueryAuthMethods.GoogleOauth) { @@ -66,7 +70,7 @@ export class FederatedAuthCommandHandlerNode implements IExtensionSyncActivation `FederatedAuthCommandHandlerNode: integration "${integration.name}" is not configured for Google OAuth.` ); void window.showErrorMessage(Integrations.federatedAuthIntegrationNotConfiguredForOAuth(integration.name)); - return; + return 'failed'; } const { clientId, clientSecret, project } = integration.metadata; @@ -129,14 +133,18 @@ export class FederatedAuthCommandHandlerNode implements IExtensionSyncActivation await this.tokenStorage.save(entry); void window.showInformationMessage(Integrations.authenticationSucceeded(integration.name)); + + return 'completed'; } catch (err) { if (err instanceof CancellationError) { logger.info(`FederatedAuthCommandHandlerNode: authentication cancelled for "${integration.name}".`); - return; + return 'cancelled'; } const message = err instanceof Error ? err.message : String(err); logger.error(`FederatedAuthCommandHandlerNode: authentication failed for "${integration.name}".`, err); void window.showErrorMessage(Integrations.authenticationFailed(message)); + + return 'failed'; } } } diff --git a/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.unit.test.ts b/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.unit.test.ts index b9026bb7df..043a9e8448 100644 --- a/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.unit.test.ts +++ b/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.node.unit.test.ts @@ -59,20 +59,22 @@ suite('FederatedAuthCommandHandlerNode', () => { const id = lookupId ?? FED_AUTH_FIXTURE.INTEGRATION_ID; when(integrationStorage.getIntegrationConfig(id)).thenResolve(config); - await handler.authenticate(id); + const outcome = await handler.authenticate(id); + assert.strictEqual(outcome, 'failed'); assert.strictEqual(runOAuthFlowStub.callCount, 0); verify(tokenStorage.save(anything())).never(); }); }); - test('happy path: saves the captured refresh token with a fresh fingerprint', async () => { + test('happy path: saves the captured refresh token with a fresh fingerprint and reports completed', async () => { when(integrationStorage.getIntegrationConfig(FED_AUTH_FIXTURE.INTEGRATION_ID)).thenResolve( buildGoogleOauthIntegration() ); - await handler.authenticate(FED_AUTH_FIXTURE.INTEGRATION_ID); + const outcome = await handler.authenticate(FED_AUTH_FIXTURE.INTEGRATION_ID); + assert.strictEqual(outcome, 'completed'); assert.strictEqual(runOAuthFlowStub.callCount, 1); verify( tokenStorage.save( @@ -140,26 +142,28 @@ suite('FederatedAuthCommandHandlerNode', () => { assert.strictEqual(params.get('state'), callArg.state); }); - test('silently returns when the user cancels the flow', async () => { + test('reports cancelled when the user cancels the flow', async () => { when(integrationStorage.getIntegrationConfig(FED_AUTH_FIXTURE.INTEGRATION_ID)).thenResolve( buildGoogleOauthIntegration() ); runOAuthFlowStub.rejects(new CancellationError()); - await handler.authenticate(FED_AUTH_FIXTURE.INTEGRATION_ID); + const outcome = await handler.authenticate(FED_AUTH_FIXTURE.INTEGRATION_ID); + assert.strictEqual(outcome, 'cancelled'); assert.strictEqual(runOAuthFlowStub.callCount, 1); verify(tokenStorage.save(anything())).never(); }); - test('surfaces a generic OAuth error via the failure toast and does not save a token', async () => { + test('surfaces a generic OAuth error via the failure toast, reports failed, and does not save a token', async () => { when(integrationStorage.getIntegrationConfig(FED_AUTH_FIXTURE.INTEGRATION_ID)).thenResolve( buildGoogleOauthIntegration() ); runOAuthFlowStub.rejects(new Error('boom')); - await handler.authenticate(FED_AUTH_FIXTURE.INTEGRATION_ID); + const outcome = await handler.authenticate(FED_AUTH_FIXTURE.INTEGRATION_ID); + assert.strictEqual(outcome, 'failed'); assert.strictEqual(runOAuthFlowStub.callCount, 1); verify(tokenStorage.save(anything())).never(); }); diff --git a/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.ts b/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.ts index e71eec910d..62b735e70a 100644 --- a/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.ts +++ b/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.ts @@ -2,6 +2,7 @@ import { inject, injectable } from 'inversify'; import { commands, window } from 'vscode'; import { IExtensionSyncActivationService } from '../../../../platform/activation/types'; +import type { CommandOutcome } from '../../../../platform/analytics/types'; import { Commands } from '../../../../platform/common/constants'; import { IExtensionContext } from '../../../../platform/common/types'; import { Integrations } from '../../../../platform/common/utils/localize'; @@ -13,8 +14,10 @@ export class FederatedAuthCommandHandlerWeb implements IExtensionSyncActivationS public activate(): void { this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.AuthenticateIntegration, () => { + commands.registerCommand(Commands.AuthenticateIntegration, (): CommandOutcome => { void window.showInformationMessage(Integrations.federatedAuthNotSupportedInWeb); + + return 'failed'; }) ); } diff --git a/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.unit.test.ts b/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.unit.test.ts index 3aa06015f0..2cee394d80 100644 --- a/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.unit.test.ts +++ b/src/notebooks/deepnote/integrations/federatedAuth/federatedAuthCommandHandler.web.unit.test.ts @@ -37,13 +37,14 @@ suite('FederatedAuthCommandHandlerWeb', () => { assert.strictEqual(subscriptions.length, 1); }); - test('the registered command surfaces the not-supported-in-web information toast', () => { + test('the registered command surfaces the not-supported-in-web information toast and reports failed', () => { handler.activate(); assert.isDefined(registeredCallback, 'command callback should have been captured'); // Invoke the command — should not throw and should show the toast. - registeredCallback!('some-integration-id'); + const outcome = registeredCallback!('some-integration-id'); + assert.strictEqual(outcome, 'failed', 'the web stub must report a failed outcome for telemetry'); verify(mockedVSCodeNamespaces.window.showInformationMessage(anything())).once(); }); }); diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index 0ba25f8c5f..777044e05d 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -3,6 +3,7 @@ import { commands, Disposable, l10n, Uri, ViewColumn, WebviewPanel, window } fro import { BigQueryAuthMethods } from '@deepnote/database-integrations'; +import { type CommandOutcome, ITelemetryService } from '../../../platform/analytics/types'; import { Commands } from '../../../platform/common/constants'; import { IDisposableRegistry, IExtensionContext } from '../../../platform/common/types'; import * as localize from '../../../platform/common/utils/localize'; @@ -42,6 +43,7 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, @inject(IIntegrationStorage) private readonly integrationStorage: IIntegrationStorage, @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, + @inject(ITelemetryService) private readonly analytics: ITelemetryService, @inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry, @inject(IFederatedAuthTokenStorage) @optional() @@ -562,6 +564,15 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } } + private trackIntegrationEvent(event: { + eventName: 'configure_integration' | 'delete_integration' | 'reset_integration'; + integrationType: string | undefined; + }): void { + const properties = { integrationType: event.integrationType ?? 'unknown' }; + + this.analytics.trackEvent({ eventName: event.eventName, properties }); + } + /** Handle messages from the webview; mirrors the `WebviewOutboundMessage` union in `src/webviews/webview-side/integrations/types.ts`. */ private async handleMessage(message: { type: string; @@ -576,23 +587,56 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { break; case 'save': if (message.integrationId && message.config) { - await this.saveConfiguration(message.integrationId, message.config); + const saved = await this.saveConfiguration(message.integrationId, message.config); + + if (saved) { + // Legacy big-query configs may omit authMethod, which means service-account. + const authMethod = + message.config.type === 'big-query' + ? message.config.metadata.authMethod ?? BigQueryAuthMethods.ServiceAccount + : undefined; + + this.analytics.trackEvent({ + eventName: 'save_integration', + properties: { + integrationType: message.config.type, + ...(authMethod ? { authMethod } : {}) + } + }); + } } break; case 'reset': if (message.integrationId) { - await this.resetConfiguration(message.integrationId); + const integrationType = this.integrations.get(message.integrationId)?.integrationType; + const reset = await this.resetConfiguration(message.integrationId); + + if (reset) { + this.trackIntegrationEvent({ eventName: 'reset_integration', integrationType }); + } } break; case 'delete': if (message.integrationId) { - await this.deleteConfiguration(message.integrationId); + const integrationType = this.integrations.get(message.integrationId)?.integrationType; + const deleted = await this.deleteConfiguration(message.integrationId); + + if (deleted) { + this.trackIntegrationEvent({ eventName: 'delete_integration', integrationType }); + } } break; case 'authenticate': if (message.integrationId) { + const integrationType = this.integrations.get(message.integrationId)?.integrationType; + let outcome: CommandOutcome = 'failed'; + try { - await commands.executeCommand(Commands.AuthenticateIntegration, message.integrationId); + outcome = + (await commands.executeCommand( + Commands.AuthenticateIntegration, + message.integrationId + )) ?? 'failed'; } catch (error) { // Command handler shows its own toasts; log here to avoid an unhandled-rejection. logger.error( @@ -600,13 +644,20 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { error ); } + + this.analytics.trackEvent({ + eventName: 'authenticate_integration', + properties: { integrationType: integrationType ?? 'unknown', outcome } + }); } break; } } /** - * Show the configuration form for an integration + * Show the configuration form for an integration. Tracking lives here rather than in the webview + * `configure` handler so the SQL status bar's "Configure current integration" entry point, which + * opens the form directly via `show()`, is counted too. */ private async showConfigurationForm(integrationId: string): Promise { const integration = this.integrations.get(integrationId); @@ -621,6 +672,11 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { integrationType: integration.integrationType, type: 'showForm' }); + + this.trackIntegrationEvent({ + eventName: 'configure_integration', + integrationType: integration.integrationType + }); } /** @@ -629,7 +685,7 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { private async saveConfiguration( integrationId: string, config: ConfigurableDatabaseIntegrationConfig - ): Promise { + ): Promise { try { // Invalidate stale federated tokens before saving (fingerprint change or auth-method switch). await this.invalidateStaleFederatedToken(integrationId, config); @@ -665,6 +721,10 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { type: 'success' }); } + + // The credential save above is the operation being tracked; a skipped/failed + // project-YAML sync must not report the whole operation as a failure. + return true; } catch (error) { logger.error('Failed to save integration configuration', error); await this.currentPanel?.webview.postMessage({ @@ -674,13 +734,15 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { ), type: 'error' }); + + return false; } } /** * Reset the configuration for an integration (clears credentials but keeps the integration entry) */ - private async resetConfiguration(integrationId: string): Promise { + private async resetConfiguration(integrationId: string): Promise { try { await this.integrationStorage.delete(integrationId); await this.tokenStorage?.delete(integrationId).catch((error) => { @@ -705,6 +767,10 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { type: 'success' }); } + + // The credential reset above is the operation being tracked; a skipped/failed + // project-YAML sync must not report the whole operation as a failure. + return true; } catch (error) { logger.error('Failed to reset integration configuration', error); await this.currentPanel?.webview.postMessage({ @@ -714,13 +780,15 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { ), type: 'error' }); + + return false; } } /** * Delete the integration completely (removes credentials and integration entry) */ - private async deleteConfiguration(integrationId: string): Promise { + private async deleteConfiguration(integrationId: string): Promise { try { await this.integrationStorage.delete(integrationId); await this.tokenStorage?.delete(integrationId).catch((error) => { @@ -740,6 +808,10 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { type: 'success' }); } + + // The credential delete above is the operation being tracked; a skipped/failed + // project-YAML sync must not report the whole operation as a failure. + return true; } catch (error) { logger.error('Failed to delete integration', error); await this.currentPanel?.webview.postMessage({ @@ -749,6 +821,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { ), type: 'error' }); + + return false; } } diff --git a/src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts b/src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts index 51d2b67bbb..f3c3ca26f3 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts @@ -1,8 +1,9 @@ import { assert } from 'chai'; import sinon from 'sinon'; import { EventEmitter, Uri } from 'vscode'; -import { anyString, anything, instance, mock, reset, verify, when } from 'ts-mockito'; +import { anyString, anything, deepEqual, instance, mock, reset, resetCalls, verify, when } from 'ts-mockito'; +import { ITelemetryService } from '../../../platform/analytics/types'; import { IExtensionContext, IDisposable } from '../../../platform/common/types'; import { Commands } from '../../../platform/common/constants'; import { IDeepnoteNotebookManager } from '../../types'; @@ -95,6 +96,7 @@ suite('IntegrationWebviewProvider', () => { let extensionContext: IExtensionContext; let integrationStorage: IIntegrationStorage; let notebookManager: IDeepnoteNotebookManager; + let mockTelemetryService: ITelemetryService; let tokens: Map; let onDidChangeTokens: EventEmitter; let tokenSaveSpy: sinon.SinonSpy<[FederatedAuthTokenEntry, { silent?: boolean }?], Promise>; @@ -108,6 +110,7 @@ suite('IntegrationWebviewProvider', () => { extensionContext = mock(); integrationStorage = mock(); notebookManager = mock(); + mockTelemetryService = mock(); extensionSubscriptions = []; when(extensionContext.subscriptions).thenReturn(extensionSubscriptions); when(extensionContext.extensionUri).thenReturn(Uri.file('/ext')); @@ -156,6 +159,7 @@ suite('IntegrationWebviewProvider', () => { instance(extensionContext), instance(integrationStorage), instance(notebookManager), + instance(mockTelemetryService), extensionSubscriptions, opts.tokenStorage ); @@ -165,7 +169,17 @@ suite('IntegrationWebviewProvider', () => { id: string, config: ConfigurableDatabaseIntegrationConfig ): Map { - return new Map([[id, { config, status: IntegrationStatus.Connected }]]); + return new Map([ + [ + id, + { + config, + status: IntegrationStatus.Connected, + integrationName: config.name, + integrationType: config.type + } + ] + ]); } async function show(provider: IntegrationWebviewProvider, integrations: Map) { @@ -240,6 +254,43 @@ suite('IntegrationWebviewProvider', () => { }); }); + suite('configure_integration telemetry', () => { + test('tracks when the form is opened directly via show() (SQL status bar entry point)', async () => { + const provider = buildProvider({ tokenStorage }); + const id = 'pg-preselected'; + + await provider.show( + PROJECT_ID, + singleIntegrationMap(id, buildPostgresIntegration({ id })), + Uri.file('/ws/active.deepnote'), + id + ); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ eventName: 'configure_integration', properties: { integrationType: 'pgsql' } }) + ) + ).once(); + }); + + test('tracks the webview configure message exactly once, and not for an unknown id', async () => { + const provider = buildProvider({ tokenStorage }); + const id = 'pg-configure'; + await show(provider, singleIntegrationMap(id, buildPostgresIntegration({ id }))); + resetCalls(mockTelemetryService); + + await fakePanel.onDidReceiveMessage({ type: 'configure', integrationId: id }); + await fakePanel.onDidReceiveMessage({ type: 'configure', integrationId: 'does-not-exist' }); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ eventName: 'configure_integration', properties: { integrationType: 'pgsql' } }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + }); + test('handleMessage: "authenticate" → commands.executeCommand(AuthenticateIntegration, integrationId)', async () => { const executeCommandStub = sinon.stub().resolves(undefined); when(mockedVSCodeNamespaces.commands.executeCommand(anyString(), anything())).thenCall((command, arg) => @@ -261,6 +312,121 @@ suite('IntegrationWebviewProvider', () => { ); }); + suite('handleMessage: "authenticate" telemetry outcome', () => { + async function authenticate(commandResult: Promise): Promise { + when(mockedVSCodeNamespaces.commands.executeCommand(anyString(), anything())).thenReturn(commandResult); + + const provider = buildProvider({ tokenStorage }); + const integrationId = 'bq-auth-outcome'; + await show( + provider, + singleIntegrationMap(integrationId, buildGoogleOauthIntegration({ id: integrationId })) + ); + resetCalls(mockTelemetryService); + + await fakePanel.onDidReceiveMessage({ type: 'authenticate', integrationId }); + } + + test('reports the outcome returned by the command, after it settles', async () => { + await authenticate(Promise.resolve('cancelled')); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'authenticate_integration', + properties: { integrationType: 'big-query', outcome: 'cancelled' } + }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + + test('reports failed when the command returns nothing (web stub / unexpected undefined)', async () => { + await authenticate(Promise.resolve(undefined)); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'authenticate_integration', + properties: { integrationType: 'big-query', outcome: 'failed' } + }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + + test('reports failed when the command rejects', async () => { + const rejection = Promise.reject(new Error('boom')); + rejection.catch(() => undefined); // avoid an unhandled-rejection warning before the handler awaits it + + await authenticate(rejection); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'authenticate_integration', + properties: { integrationType: 'big-query', outcome: 'failed' } + }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + }); + + suite('handleMessage: "save" telemetry authMethod', () => { + async function save(config: ConfigurableDatabaseIntegrationConfig): Promise { + when(integrationStorage.save(anything())).thenResolve(); + + const provider = buildProvider({ tokenStorage }); + await show(provider, singleIntegrationMap(config.id, config)); + resetCalls(mockTelemetryService); + + await fakePanel.onDidReceiveMessage({ type: 'save', integrationId: config.id, config }); + } + + test('reports authMethod google-oauth for an OAuth BigQuery config', async () => { + await save(buildGoogleOauthIntegration({ id: 'bq-save-oauth' })); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'save_integration', + properties: { integrationType: 'big-query', authMethod: 'google-oauth' } + }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + + test('reports authMethod service-account for a legacy BigQuery config that omits authMethod', async () => { + const config = buildServiceAccountIntegration({ id: 'bq-save-legacy' }); + delete (config.metadata as { authMethod?: string }).authMethod; + + await save(config); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ + eventName: 'save_integration', + properties: { integrationType: 'big-query', authMethod: 'service-account' } + }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + + test('omits authMethod for non-BigQuery configs', async () => { + await save(buildPostgresIntegration({ id: 'pg-save' })); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ eventName: 'save_integration', properties: { integrationType: 'pgsql' } }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); + }); + (['reset', 'delete'] as const).forEach((messageType) => { test(`${messageType}Configuration: deletes the federated token in addition to the integration config`, async () => { when(integrationStorage.delete(anyString())).thenResolve(); diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts index 1d21340507..6b2519f331 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts @@ -4,6 +4,7 @@ import { injectable, inject } from 'inversify'; import { commands, window, Uri, env, l10n } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { Commands } from '../../platform/common/constants'; import { logger } from '../../platform/logging'; @@ -13,15 +14,26 @@ import { initImport, uploadFile, getErrorMessage, MAX_FILE_SIZE, getDeepnoteDoma @injectable() export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { - constructor(@inject(IExtensionContext) private readonly extensionContext: IExtensionContext) {} + constructor( + @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, + @inject(ITelemetryService) private readonly analytics: ITelemetryService + ) {} public activate(): void { this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.OpenInDeepnote, () => this.handleOpenInDeepnote()) + commands.registerCommand(Commands.OpenInDeepnote, async () => { + // Every falsy return from the handler is preceded by an error message; there is no + // user-cancel path, so `false` maps to 'failed' rather than 'cancelled'. + const completed = await this.handleOpenInDeepnote(); + this.analytics.trackEvent({ + eventName: 'open_in_deepnote', + properties: { outcome: completed ? 'completed' : 'failed' } + }); + }) ); } - private async handleOpenInDeepnote(): Promise { + private async handleOpenInDeepnote(): Promise { try { let fileUri: Uri | undefined; let isNotebook = false; @@ -39,7 +51,8 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { const activeEditor = window.activeTextEditor; if (!activeEditor) { void window.showErrorMessage('Please open a .deepnote file first'); - return; + + return false; } fileUri = activeEditor.document.uri; @@ -47,7 +60,8 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { if (!fileUri.fsPath.endsWith('.deepnote')) { void window.showErrorMessage('This command only works with .deepnote files'); - return; + + return false; } if (isNotebook) { @@ -58,7 +72,8 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { const saved = await activeEditor.document.save(); if (!saved) { void window.showErrorMessage('Please save the file before opening in Deepnote'); - return; + + return false; } } } @@ -71,12 +86,13 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { const stats = await fs.promises.stat(filePath); if (stats.size > MAX_FILE_SIZE) { void window.showErrorMessage(`File exceeds ${MAX_FILE_SIZE / (1024 * 1024)}MB limit`); - return; + + return false; } const fileBuffer = await fs.promises.readFile(filePath); - await window.withProgress( + return await window.withProgress( { location: { viewId: 'workbench.view.extension.deepnoteExplorer' }, title: l10n.t('Opening in Deepnote'), @@ -105,10 +121,14 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { void window.showInformationMessage('Opening in Deepnote...'); logger.info('Successfully opened file in Deepnote'); + + return true; } catch (error) { logger.error('Failed to open in Deepnote', error); const errorMessage = getErrorMessage(error); void window.showErrorMessage(`Failed to open in Deepnote: ${errorMessage}`); + + return false; } } ); @@ -116,6 +136,8 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { logger.error('Error in handleOpenInDeepnote', error); const errorMessage = getErrorMessage(error); void window.showErrorMessage(`Failed to open in Deepnote: ${errorMessage}`); + + return false; } } } diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts index 8b7d0240d5..d9acef674d 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts @@ -1,11 +1,12 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; -import { instance, mock, when, anything } from 'ts-mockito'; +import { deepEqual, instance, mock, verify, when, anything } from 'ts-mockito'; import { Uri, TextDocument, TextEditor, NotebookDocument, NotebookEditor } from 'vscode'; import * as fs from 'fs'; import esmock from 'esmock'; import type { OpenInDeepnoteHandler } from './openInDeepnoteHandler.node'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; import { MAX_FILE_SIZE } from './importClient.node'; @@ -13,6 +14,7 @@ import { MAX_FILE_SIZE } from './importClient.node'; suite('OpenInDeepnoteHandler', () => { let handler: OpenInDeepnoteHandler; let mockExtensionContext: IExtensionContext; + let mockTelemetryService: ITelemetryService; let sandbox: sinon.SinonSandbox; let initImportStub: sinon.SinonStub; let uploadFileStub: sinon.SinonStub; @@ -49,7 +51,8 @@ suite('OpenInDeepnoteHandler', () => { subscriptions: [] } as any; - handler = new OpenInDeepnoteHandlerClass(mockExtensionContext); + mockTelemetryService = mock(); + handler = new OpenInDeepnoteHandlerClass(mockExtensionContext, instance(mockTelemetryService)); }); teardown(() => { @@ -103,6 +106,29 @@ suite('OpenInDeepnoteHandler', () => { ); assert.isFunction(registeredCallback, 'Second argument should be a function'); }); + + test('the registered command tracks open_in_deepnote failed when there is nothing to open', async () => { + let registeredCallback: (() => Promise) | undefined; + + when(mockedVSCodeNamespaces.commands.registerCommand(anything(), anything())).thenCall((_id, callback) => { + registeredCallback = callback; + + return { dispose: () => undefined }; + }); + when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn(undefined); + when(mockedVSCodeNamespaces.window.activeTextEditor).thenReturn(undefined); + when(mockedVSCodeNamespaces.window.showErrorMessage(anything())).thenReturn(Promise.resolve(undefined)); + + handler.activate(); + await registeredCallback!(); + + verify( + mockTelemetryService.trackEvent( + deepEqual({ eventName: 'open_in_deepnote', properties: { outcome: 'failed' } }) + ) + ).once(); + verify(mockTelemetryService.trackEvent(anything())).once(); + }); }); suite('handleOpenInDeepnote', () => { diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts index 5bf312977c..d867947170 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts @@ -19,12 +19,14 @@ import { import { inject, injectable } from 'inversify'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IDisposableRegistry } from '../../platform/common/types'; import { IIntegrationStorage } from './integrations/types'; import { Commands } from '../../platform/common/constants'; import { ConfigurableDatabaseIntegrationType, - DATAFRAME_SQL_INTEGRATION_ID + DATAFRAME_SQL_INTEGRATION_ID, + toTelemetryIntegrationType } from '../../platform/notebooks/deepnote/integrationTypes'; import { IDeepnoteNotebookManager } from '../types'; import { DatabaseIntegrationType, databaseIntegrationTypes } from '@deepnote/database-integrations'; @@ -69,7 +71,8 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid constructor( @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IIntegrationStorage) private readonly integrationStorage: IIntegrationStorage, - @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager + @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, + @inject(ITelemetryService) private readonly analytics: ITelemetryService ) {} public activate(): void { @@ -459,5 +462,16 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid // Trigger status bar update this._onDidChangeCellStatusBarItems.fire(); + + const selectedIntegration = projectIntegrations.find((i) => i.id === selectedId); + const integrationType = + selectedId === DATAFRAME_SQL_INTEGRATION_ID + ? 'duckdb' + : toTelemetryIntegrationType(selectedIntegration?.type); + + this.analytics.trackEvent({ + eventName: 'switch_sql_integration', + properties: { integrationType } + }); } } diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts index f729453582..2e05a00283 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts @@ -1,9 +1,10 @@ import { assert } from 'chai'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; import { CancellationToken, CancellationTokenSource, EventEmitter, NotebookCell } from 'vscode'; import { IDisposableRegistry } from '../../platform/common/types'; import { IIntegrationStorage } from './integrations/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { SqlCellStatusBarProvider } from './sqlCellStatusBarProvider'; import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; @@ -23,7 +24,12 @@ suite('SqlCellStatusBarProvider', () => { disposables = []; integrationStorage = mock(); notebookManager = mock(); - provider = new SqlCellStatusBarProvider(disposables, instance(integrationStorage), instance(notebookManager)); + provider = new SqlCellStatusBarProvider( + disposables, + instance(integrationStorage), + instance(notebookManager), + instance(mock()) + ); const tokenSource = new CancellationTokenSource(); cancellationToken = tokenSource.token; @@ -304,7 +310,8 @@ suite('SqlCellStatusBarProvider', () => { activateProvider = new SqlCellStatusBarProvider( activateDisposables, instance(activateIntegrationStorage), - instance(activateNotebookManager) + instance(activateNotebookManager), + instance(mock()) ); }); @@ -540,7 +547,8 @@ suite('SqlCellStatusBarProvider', () => { eventProvider = new SqlCellStatusBarProvider( eventDisposables, instance(eventIntegrationStorage), - instance(eventNotebookManager) + instance(eventNotebookManager), + instance(mock()) ); }); @@ -668,7 +676,8 @@ suite('SqlCellStatusBarProvider', () => { commandProvider = new SqlCellStatusBarProvider( commandDisposables, instance(commandIntegrationStorage), - instance(commandNotebookManager) + instance(commandNotebookManager), + instance(mock()) ); // Capture the command handler @@ -799,6 +808,7 @@ suite('SqlCellStatusBarProvider', () => { let commandProvider: SqlCellStatusBarProvider; let commandIntegrationStorage: IIntegrationStorage; let commandNotebookManager: IDeepnoteNotebookManager; + let commandTelemetry: ITelemetryService; let switchIntegrationHandler: Function; setup(() => { @@ -806,10 +816,12 @@ suite('SqlCellStatusBarProvider', () => { commandDisposables = []; commandIntegrationStorage = mock(); commandNotebookManager = mock(); + commandTelemetry = mock(); commandProvider = new SqlCellStatusBarProvider( commandDisposables, instance(commandIntegrationStorage), - instance(commandNotebookManager) + instance(commandNotebookManager), + instance(commandTelemetry) ); // Capture the command handler @@ -862,6 +874,49 @@ suite('SqlCellStatusBarProvider', () => { verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).once(); verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).once(); + verify( + commandTelemetry.trackEvent( + deepEqual({ eventName: 'switch_sql_integration', properties: { integrationType: 'pgsql' } }) + ) + ).once(); + }); + + test('reports an unrecognized integration type as unknown', async () => { + // integrations[].type is a free-form string in the .deepnote schema; unbounded values must not + // reach analytics verbatim. + const notebookMetadata = { deepnoteProjectId: 'project-1', deepnoteNotebookId: 'notebook-1' }; + const cell = createMockCell({ + languageId: 'sql', + metadata: { sql_integration_id: 'old-integration' }, + notebookMetadata + }); + const newIntegrationId = 'new-integration'; + + when(commandNotebookManager.getProjectForNotebook('project-1', 'notebook-1')).thenReturn({ + project: { + integrations: [ + { + id: newIntegrationId, + name: 'New Integration', + type: 'not-a-real-integration-type' + } + ] + } + } as any); + + when(mockedVSCodeNamespaces.window.showErrorMessage(anything())).thenReturn(Promise.resolve(undefined)); + when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenReturn( + Promise.resolve({ id: newIntegrationId, label: 'New Integration' } as any) + ); + when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenReturn(Promise.resolve(true)); + + await switchIntegrationHandler(cell); + + verify( + commandTelemetry.trackEvent( + deepEqual({ eventName: 'switch_sql_integration', properties: { integrationType: 'unknown' } }) + ) + ).once(); }); test('does not update if user cancels quick pick', async () => { diff --git a/src/notebooks/notebookCommandListener.ts b/src/notebooks/notebookCommandListener.ts index 04907cb1ca..dbfba2a77f 100644 --- a/src/notebooks/notebookCommandListener.ts +++ b/src/notebooks/notebookCommandListener.ts @@ -65,9 +65,6 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens this.disposableRegistry.push( commands.registerCommand(Commands.NotebookEditorRemoveAllCells, () => this.removeAllCells()) ); - this.disposableRegistry.push( - commands.registerCommand(Commands.NotebookEditorRunAllCells, () => this.runAllCells()) - ); this.disposableRegistry.push( commands.registerCommand(Commands.NotebookEditorRunFocusedCell, () => this.runFocusedCell()) ); diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index eb87ce58a3..b429263154 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -92,6 +92,7 @@ import { DeepnoteEnvironmentsView } from '../kernels/deepnote/environments/deepn import { DeepnoteEnvironmentsActivationService } from '../kernels/deepnote/environments/deepnoteEnvironmentsActivationService'; import { DeepnoteExtensionSidecarWriter } from '../kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node'; import { DeepnoteNotebookEnvironmentMapper } from '../kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node'; +import { DeepnoteCellExecutionAnalytics } from './deepnote/deepnoteCellExecutionAnalytics'; import { DeepnoteNotebookCommandListener } from './deepnote/deepnoteNotebookCommandListener'; import { DeepnoteInputBlockCellStatusBarItemProvider } from './deepnote/deepnoteInputBlockCellStatusBarProvider'; import { DeepnoteBigNumberCellStatusBarProvider } from './deepnote/deepnoteBigNumberCellStatusBarProvider'; @@ -183,6 +184,10 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea IExtensionSyncActivationService, DeepnoteNotebookCommandListener ); + serviceManager.addSingleton( + IExtensionSyncActivationService, + DeepnoteCellExecutionAnalytics + ); serviceManager.addSingleton(IDeepnoteNotebookManager, DeepnoteNotebookManager); // Bind the platform-layer interface to the same implementation serviceManager.addBinding(IDeepnoteNotebookManager, IPlatformDeepnoteNotebookManager); diff --git a/src/platform/analytics/constants.ts b/src/platform/analytics/constants.ts new file mode 100644 index 0000000000..c2b203af5b --- /dev/null +++ b/src/platform/analytics/constants.ts @@ -0,0 +1,22 @@ +// Substituted at build time from the POSTHOG_API_KEY CI secret (see build/esbuild/build.ts). +// Left undefined in local builds, where telemetry falls back to this inert placeholder. +declare const POSTHOG_API_KEY_BUILD: string | undefined; + +// Substituted at build time from the POSTHOG_CHANNEL env var (see build/esbuild/build.ts): +// 'stable' for main/release builds, 'pr' for pull-request builds, 'development' for CI builds off +// any other ref. Left undefined in local builds, where it also falls back to 'development' so +// dogfood/PR events can be segmented out. +declare const POSTHOG_CHANNEL_BUILD: string | undefined; + +const POSTHOG_API_KEY_PLACEHOLDER = '__POSTHOG_API_KEY__'; + +export const POSTHOG_API_KEY = + typeof POSTHOG_API_KEY_BUILD !== 'undefined' && POSTHOG_API_KEY_BUILD + ? POSTHOG_API_KEY_BUILD + : POSTHOG_API_KEY_PLACEHOLDER; +export const POSTHOG_CHANNEL = + typeof POSTHOG_CHANNEL_BUILD !== 'undefined' && POSTHOG_CHANNEL_BUILD ? POSTHOG_CHANNEL_BUILD : 'development'; +export const POSTHOG_HOST = 'https://us.i.posthog.com'; + +// Guards against initializing PostHog with the inert placeholder key in local/unconfigured builds. +export const IS_POSTHOG_CONFIGURED = POSTHOG_API_KEY !== POSTHOG_API_KEY_PLACEHOLDER && POSTHOG_API_KEY.length > 0; diff --git a/src/platform/analytics/telemetryService.ts b/src/platform/analytics/telemetryService.ts new file mode 100644 index 0000000000..d87a2fc602 --- /dev/null +++ b/src/platform/analytics/telemetryService.ts @@ -0,0 +1,156 @@ +import { inject, injectable } from 'inversify'; +import { PostHog } from 'posthog-node'; +import { env, workspace } from 'vscode'; + +import { IExtensionSyncActivationService } from '../activation/types'; +import { IApplicationEnvironment } from '../common/application/types'; +import { + IAsyncDisposableRegistry, + IDisposableRegistry, + IPersistentState, + IPersistentStateFactory +} from '../common/types'; +import { generateUuid } from '../common/uuid'; +import { logger } from '../logging'; +import { IS_POSTHOG_CONFIGURED, POSTHOG_API_KEY, POSTHOG_CHANNEL, POSTHOG_HOST } from './constants'; +import { ITelemetryService, TelemetryEvent } from './types'; + +const USER_ID_STORAGE_KEY = 'deepnote-telemetry-anonymous-user-id'; +const POSTHOG_FLUSH_AT = 20; +const POSTHOG_FLUSH_INTERVAL = 30000; +const POSTHOG_SHUTDOWN_TIMEOUT = 5000; + +@injectable() +export class TelemetryService implements ITelemetryService, IExtensionSyncActivationService { + private client: PostHog | null; + + // Attached to every event so metrics can be segmented by build channel, extension/VS Code + // version, platform, and session. + private readonly commonProperties: Record; + + private readonly userId: string; + + private userIdState: IPersistentState; + + constructor( + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + @inject(IPersistentStateFactory) private readonly stateFactory: IPersistentStateFactory, + @inject(IAsyncDisposableRegistry) asyncDisposables: IAsyncDisposableRegistry, + @inject(IApplicationEnvironment) appEnvironment: IApplicationEnvironment + ) { + asyncDisposables.push(this); + this.client = null; + this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, ''); + this.userId = this.userIdState.value || generateUuid(); + this.commonProperties = { + channel: POSTHOG_CHANNEL, + extensionVersion: appEnvironment.extensionVersion, + platform: process.platform, + sessionId: env.sessionId + }; + } + + public activate(): void { + try { + // Persistence is floated deliberately: the client and distinctId do not depend on it, and + // awaiting it here would leave `client` null for any event racing activation. + if (this.userIdState.value !== this.userId) { + this.userIdState + .updateValue(this.userId) + .catch((error) => logger.debug(`Failed to persist telemetry user ID: ${error}`)); + } + + this.createClient(); + } catch (error) { + logger.debug(`TelemetryService activation error: ${error}`); + } + + this.disposables.push( + workspace.onDidChangeConfiguration((e) => { + if (e.affectsConfiguration('telemetry') || e.affectsConfiguration('deepnote.telemetry')) { + this.handleConfigChanged(); + } + }), + env.onDidChangeTelemetryEnabled(() => this.handleConfigChanged()) + ); + } + + public async dispose(): Promise { + await this.destroyClient(); + } + + public trackEvent(event: TelemetryEvent): void { + try { + if (!this.client) { + return; + } + + this.client.capture({ + distinctId: this.userId, + event: event.eventName, + properties: { ...event.properties, ...this.commonProperties, $process_person_profile: false } + }); + } catch (ex) { + logger.debug(`PostHog analytics error: ${ex}`); + } + } + + private createClient(): void { + if (this.client || !this.isPostHogConfigured() || !this.isTelemetryEnabled()) { + return; + } + + this.client = new PostHog(POSTHOG_API_KEY, { + flushAt: POSTHOG_FLUSH_AT, + flushInterval: POSTHOG_FLUSH_INTERVAL, + host: POSTHOG_HOST + }); + } + + private async destroyClient(): Promise { + const client = this.client; + this.client = null; + + if (!client) { + return; + } + + try { + await client.shutdown(POSTHOG_SHUTDOWN_TIMEOUT); + } catch (ex) { + logger.debug(`PostHog shutdown error: ${ex}`); + } + } + + private handleConfigChanged(): void { + try { + if (this.isTelemetryEnabled()) { + this.createClient(); + } else { + this.destroyClient().catch((error) => { + logger.debug(`Failed to destroy PostHog client: ${error}`); + }); + } + } catch (error) { + logger.debug(`Failed to handle telemetry configuration change: ${error}`); + } + } + + private isPostHogConfigured(): boolean { + return IS_POSTHOG_CONFIGURED; + } + + private isTelemetryEnabled(): boolean { + if (!env.isTelemetryEnabled) { + return false; + } + + const telemetryLevel = workspace.getConfiguration('telemetry').get('telemetryLevel', 'all'); + + if (telemetryLevel !== 'all') { + return false; + } + + return workspace.getConfiguration('deepnote').get('telemetry.enabled', true); + } +} diff --git a/src/platform/analytics/telemetryService.unit.test.ts b/src/platform/analytics/telemetryService.unit.test.ts new file mode 100644 index 0000000000..882870f2f6 --- /dev/null +++ b/src/platform/analytics/telemetryService.unit.test.ts @@ -0,0 +1,317 @@ +import { assert, use } from 'chai'; +import chaiAsPromised from 'chai-as-promised'; +import * as sinon from 'sinon'; + +import { IApplicationEnvironment } from '../common/application/types'; +import { + IAsyncDisposableRegistry, + IDisposableRegistry, + IPersistentState, + IPersistentStateFactory +} from '../common/types'; +import { TelemetryService } from './telemetryService'; + +use(chaiAsPromised); + +suite('TelemetryService', () => { + let analyticsService: TelemetryService; + let mockDisposables: IDisposableRegistry; + let mockStateFactory: IPersistentStateFactory; + let mockAsyncDisposableRegistry: IAsyncDisposableRegistry; + let mockUserIdState: IPersistentState; + let mockAppEnv: IApplicationEnvironment; + + function createMockPersistentState(initialValue: string): IPersistentState { + let storedValue = initialValue; + + return { + get value() { + return storedValue; + }, + updateValue: sinon.stub().callsFake(async (newValue: string) => { + storedValue = newValue; + }) + }; + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + function getPostHogClient(service: TelemetryService): any { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (service as any).client; + } + + function stubTelemetryEnabled(service: TelemetryService, enabled: boolean): void { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).isTelemetryEnabled = () => enabled; + } + + function stubPostHogConfigured(service: TelemetryService, configured: boolean): void { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (service as any).isPostHogConfigured = () => configured; + } + + // Replaces createClient so no test constructs a real, network-capable PostHog client, + // while preserving the real configured + enabled gating. + function stubClientFactory(service: TelemetryService): { capture: sinon.SinonStub; shutdown: sinon.SinonStub } { + const fakeClient = { capture: sinon.stub(), shutdown: sinon.stub().resolves() }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const internal = service as any; + internal.createClient = () => { + if (internal.client || !internal.isPostHogConfigured() || !internal.isTelemetryEnabled()) { + return; + } + + internal.client = fakeClient; + }; + + return fakeClient; + } + + setup(() => { + mockUserIdState = createMockPersistentState(''); + mockDisposables = []; + mockAppEnv = { extensionVersion: '1.2.3' }; + mockStateFactory = { + createGlobalPersistentState: sinon.stub().returns(mockUserIdState), + createWorkspacePersistentState: sinon.stub().returns(mockUserIdState) + } as unknown as IPersistentStateFactory; + mockAsyncDisposableRegistry = { + push: sinon.stub(), + dispose: sinon.stub().resolves() + }; + }); + + test('should create instance without errors', () => { + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + + assert.isDefined(analyticsService); + }); + + test('activate should not create client when telemetry is disabled', async () => { + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + stubTelemetryEnabled(analyticsService, false); + + await analyticsService.activate(); + + assert.isNull(getPostHogClient(analyticsService), 'PostHog client should not be created'); + assert.isTrue( + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).calledOnce, + 'Should still create persistent state during construction' + ); + }); + + test('activate should create client when telemetry is enabled', async () => { + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + stubTelemetryEnabled(analyticsService, true); + stubPostHogConfigured(analyticsService, true); + stubClientFactory(analyticsService); + + await analyticsService.activate(); + + const client = getPostHogClient(analyticsService); + + assert.isNotNull(client, 'PostHog client should be initialized'); + }); + + test('activate should not create client when PostHog is not configured', async () => { + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + stubTelemetryEnabled(analyticsService, true); + stubPostHogConfigured(analyticsService, false); + + await analyticsService.activate(); + + assert.isNull( + getPostHogClient(analyticsService), + 'PostHog client should not be created with the placeholder key' + ); + }); + + test('should generate and persist a user ID and send it as distinctId with common properties', async () => { + // Default is empty, so the mock state starts empty and activate() must generate + persist a UUID. + const userIdState = createMockPersistentState(''); + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).returns(userIdState); + + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + stubTelemetryEnabled(analyticsService, true); + stubPostHogConfigured(analyticsService, true); + const fakeClient = stubClientFactory(analyticsService); + + await analyticsService.activate(); + + // Catches: distinctId churning because the empty default is never persisted (updateValue never called). + assert.isTrue( + (userIdState.updateValue as sinon.SinonStub).calledOnce, + 'A user ID should be generated and persisted on first activation' + ); + + const persistedId = userIdState.value; + + assert.isNotEmpty(persistedId, 'Persisted user ID should not be empty'); + + analyticsService.trackEvent({ eventName: 'execute_notebook' }); + + assert.isTrue(fakeClient.capture.calledOnce, 'PostHog capture should be called'); + + const captured = fakeClient.capture.firstCall.args[0]; + + assert.strictEqual(captured.distinctId, persistedId, 'distinctId should be the persisted user ID'); + assert.strictEqual(captured.event, 'execute_notebook'); + assert.strictEqual(captured.properties.$process_person_profile, false, 'events must be personless'); + assert.strictEqual(captured.properties.channel, 'development'); + assert.strictEqual(captured.properties.extensionVersion, '1.2.3', 'common properties must be attached'); + }); + + test('should reuse existing user ID', async () => { + mockUserIdState = createMockPersistentState('existing-user-id'); + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).returns(mockUserIdState); + + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + stubTelemetryEnabled(analyticsService, true); + + await analyticsService.activate(); + + assert.isFalse( + (mockUserIdState.updateValue as sinon.SinonStub).called, + 'Should not update value when user ID already exists' + ); + }); + + // A fresh profile whose global-state write never settles or outright rejects. Either way the + // client and distinctId must not wait on it — activate() is registered as a sync service. + function buildServiceWithUnsettledPersist(updateValue: sinon.SinonStub) { + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).returns({ + get value() { + return ''; + }, + updateValue + } as IPersistentState); + + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + stubTelemetryEnabled(analyticsService, true); + stubPostHogConfigured(analyticsService, true); + + return stubClientFactory(analyticsService); + } + + test('should track events without waiting for user ID persistence to settle', () => { + const fakeClient = buildServiceWithUnsettledPersist(sinon.stub().returns(new Promise(() => {}))); + + analyticsService.activate(); + analyticsService.trackEvent({ eventName: 'execute_notebook' }); + + assert.isTrue( + fakeClient.capture.calledOnce, + 'The first event must not be dropped while persistence is pending' + ); + assert.isNotEmpty( + fakeClient.capture.firstCall.args[0].distinctId, + 'distinctId should come from the in-memory user ID' + ); + }); + + test('should still create the client when persisting the user ID fails', () => { + buildServiceWithUnsettledPersist(sinon.stub().rejects(new Error('global state unavailable'))); + + analyticsService.activate(); + + assert.isNotNull( + getPostHogClient(analyticsService), + 'A failed user ID write must not disable telemetry for the whole session' + ); + }); + + test('settings change should destroy client when telemetry is disabled', async () => { + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + stubTelemetryEnabled(analyticsService, true); + stubPostHogConfigured(analyticsService, true); + stubClientFactory(analyticsService); + + await analyticsService.activate(); + + const client = getPostHogClient(analyticsService); + + assert.isNotNull(client, 'Client should be created initially'); + + const shutdownStub = sinon.stub().resolves(); + client.shutdown = shutdownStub; + + stubTelemetryEnabled(analyticsService, false); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (analyticsService as any).handleConfigChanged(); + + assert.isNull(getPostHogClient(analyticsService), 'Client should be destroyed when telemetry is disabled'); + }); + + test('settings change should create client when telemetry is enabled', async () => { + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + stubTelemetryEnabled(analyticsService, false); + stubPostHogConfigured(analyticsService, true); + stubClientFactory(analyticsService); + + await analyticsService.activate(); + + assert.isNull(getPostHogClient(analyticsService), 'Client should not be created initially'); + + stubTelemetryEnabled(analyticsService, true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (analyticsService as any).handleConfigChanged(); + + assert.isNotNull(getPostHogClient(analyticsService), 'Client should be created when telemetry is enabled'); + }); + + test('dispose should not throw even when client is not initialized', async () => { + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); + + await assert.isFulfilled(analyticsService.dispose()); + }); +}); diff --git a/src/platform/analytics/telemetryWebService.ts b/src/platform/analytics/telemetryWebService.ts new file mode 100644 index 0000000000..ddabe87f1b --- /dev/null +++ b/src/platform/analytics/telemetryWebService.ts @@ -0,0 +1,14 @@ +import { injectable } from 'inversify'; + +import { ITelemetryService, TelemetryEvent } from './types'; + +@injectable() +export class TelemetryWebService implements ITelemetryService { + public async dispose(): Promise { + // No-op for web + } + + public trackEvent(_event: TelemetryEvent): void { + // No-op for web + } +} diff --git a/src/platform/analytics/types.ts b/src/platform/analytics/types.ts new file mode 100644 index 0000000000..65788e8e09 --- /dev/null +++ b/src/platform/analytics/types.ts @@ -0,0 +1,82 @@ +import { IAsyncDisposable } from '../common/types'; + +export type TelemetryEventName = + | 'add_block' + | 'authenticate_integration' + | 'configure_integration' + | 'copy_notebook_details' + | 'create_environment' + | 'create_notebook' + | 'create_project' + | 'delete_environment' + | 'delete_integration' + | 'delete_notebook' + | 'duplicate_notebook' + | 'execute_cell' + | 'execute_notebook' + | 'export_notebook' + | 'import_notebook' + | 'open_in_deepnote' + | 'open_notebook' + | 'rename_notebook' + | 'rename_project' + | 'reset_integration' + | 'save_integration' + | 'select_environment' + | 'split_notebook' + | 'switch_sql_integration' + | 'toggle_snapshots' + | 'update_environment'; + +/** Result of a tracked command, so telemetry can separate user drop-off from real failures. */ +export type CommandOutcome = 'completed' | 'cancelled' | 'failed'; + +/** + * Caller-supplied property shape per event. `undefined` means the event carries no caller + * properties (the service still attaches common properties such as version/platform/channel). + */ +export interface TelemetryEventProperties { + add_block: { blockType: string }; + authenticate_integration: { integrationType: string; outcome: CommandOutcome }; + configure_integration: { integrationType: string }; + copy_notebook_details: undefined; + create_environment: { hasDescription: boolean; packageCount: number }; + create_notebook: { outcome: CommandOutcome; source: 'toolbar' | 'project_menu' }; + create_project: { outcome: CommandOutcome }; + delete_environment: undefined; + delete_integration: { integrationType: string }; + delete_notebook: { outcome: CommandOutcome }; + duplicate_notebook: { outcome: CommandOutcome }; + execute_cell: { cellType: 'sql' | 'markdown' | 'code'; integrationType?: string }; + execute_notebook: undefined; + export_notebook: { outcome: CommandOutcome; format?: string }; + import_notebook: { outcome: CommandOutcome; source: 'deepnote' | 'jupyter' }; + open_in_deepnote: { outcome: CommandOutcome }; + open_notebook: { outcome: CommandOutcome }; + rename_notebook: { outcome: CommandOutcome }; + rename_project: { outcome: CommandOutcome }; + reset_integration: { integrationType: string }; + save_integration: { integrationType: string; authMethod?: 'service-account' | 'google-oauth' }; + select_environment: undefined; + split_notebook: { notebookCount: number; outcome: CommandOutcome }; + switch_sql_integration: { integrationType: string }; + toggle_snapshots: { enabled: boolean }; + update_environment: { field: 'name' | 'packages'; packageCount?: number }; +} + +/** + * An event name paired with its event-specific properties. Events whose property type is + * `undefined` may omit `properties`. Distributes over `E` so a union of event names yields the + * corresponding union of `{ eventName, properties }` shapes. + */ +export type TelemetryEvent = E extends TelemetryEventName + ? TelemetryEventProperties[E] extends undefined + ? { eventName: E; properties?: never } + : { eventName: E; properties: TelemetryEventProperties[E] } + : never; + +export const ITelemetryService = Symbol('ITelemetryService'); + +export interface ITelemetryService extends IAsyncDisposable { + trackEvent(event: TelemetryEvent): void; +} diff --git a/src/platform/common/constants.ts b/src/platform/common/constants.ts index 39f3381a0c..f78efcb475 100644 --- a/src/platform/common/constants.ts +++ b/src/platform/common/constants.ts @@ -186,7 +186,6 @@ export namespace Commands { export const RestartKernelAndRunAllCells = 'deepnote.restartkernelandrunallcells'; export const RestartKernelAndRunUpToSelectedCell = 'deepnote.restartkernelandrunuptoselectedcell'; export const NotebookEditorRemoveAllCells = 'deepnote.notebookeditor.removeallcells'; - export const NotebookEditorRunAllCells = 'deepnote.notebookeditor.runallcells'; export const NotebookEditorRunSelectedCell = 'deepnote.notebookeditor.runselectedcell'; export const NotebookEditorRunFocusedCell = 'deepnote.notebookeditor.runfocusedcell'; export const NotebookEditorAddCellBelow = 'deepnote.notebookeditor.addcellbelow'; diff --git a/src/platform/notebooks/deepnote/integrationTypes.ts b/src/platform/notebooks/deepnote/integrationTypes.ts index 8eb24affc5..5f8c602ff0 100644 --- a/src/platform/notebooks/deepnote/integrationTypes.ts +++ b/src/platform/notebooks/deepnote/integrationTypes.ts @@ -74,7 +74,11 @@ export interface LegacyDuckDBIntegrationConfig extends BaseLegacyIntegrationConf type: LegacyIntegrationType.DuckDB; } -import { DatabaseIntegrationConfig, DatabaseIntegrationType } from '@deepnote/database-integrations'; +import { + DatabaseIntegrationConfig, + DatabaseIntegrationType, + isDatabaseIntegrationType +} from '@deepnote/database-integrations'; // Import and re-export Snowflake auth constants from shared module import { type SnowflakeAuthMethod, @@ -143,6 +147,15 @@ export type ConfigurableDatabaseIntegrationConfig = Extract< export type ConfigurableDatabaseIntegrationType = Exclude; +/** + * Narrows a project-file integration type to a value safe to report as telemetry. The `.deepnote` + * schema types `integrations[].type` as a free-form string, so unrecognized values collapse to + * `'unknown'` instead of reaching analytics as unbounded property cardinality. + */ +export function toTelemetryIntegrationType(type: string | undefined): DatabaseIntegrationType | 'unknown' { + return type && isDatabaseIntegrationType(type) ? type : 'unknown'; +} + /** * Integration connection status */ diff --git a/src/platform/serviceRegistry.node.ts b/src/platform/serviceRegistry.node.ts index 73599e069a..d9a3346276 100644 --- a/src/platform/serviceRegistry.node.ts +++ b/src/platform/serviceRegistry.node.ts @@ -6,6 +6,8 @@ import { registerTypes as registerApiTypes } from './api/serviceRegistry.node'; import { registerTypes as registerCommonTypes } from './common/serviceRegistry.node'; import { registerTypes as registerTerminalTypes } from './terminals/serviceRegistry.node'; import { registerTypes as registerInterpreterTypes } from './interpreter/serviceRegistry.node'; +import { ITelemetryService } from './analytics/types'; +import { TelemetryService } from './analytics/telemetryService'; import { DataScienceStartupTime } from './common/constants'; import { IExtensionSyncActivationService } from './activation/types'; import { IConfigurationService, IDataScienceCommandListener } from './common/types'; @@ -26,6 +28,8 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addBinding(FileSystem, IFileSystemNode); serviceManager.addBinding(FileSystem, IFileSystem); serviceManager.addSingleton(IWorkspaceService, WorkspaceService); + serviceManager.addSingleton(ITelemetryService, TelemetryService); + serviceManager.addBinding(ITelemetryService, IExtensionSyncActivationService); serviceManager.addSingleton(IConfigurationService, ConfigurationService); registerApiTypes(serviceManager); diff --git a/src/platform/serviceRegistry.web.ts b/src/platform/serviceRegistry.web.ts index 453d73c0e7..9bc3c26dca 100644 --- a/src/platform/serviceRegistry.web.ts +++ b/src/platform/serviceRegistry.web.ts @@ -24,9 +24,12 @@ import { KernelProgressReporter } from './progress/kernelProgressReporter'; import { WebviewPanelProvider } from './webviews/webviewPanelProvider'; import { WebviewViewProvider } from './webviews/webviewViewProvider'; import { WorkspaceInterpreterTracker } from './interpreter/workspaceInterpreterTracker'; +import { ITelemetryService } from './analytics/types'; +import { TelemetryWebService } from './analytics/telemetryWebService'; import { ApplicationEnvironment } from './common/application/applicationEnvironment'; export function registerTypes(serviceManager: IServiceManager) { + serviceManager.addSingleton(ITelemetryService, TelemetryWebService); serviceManager.addSingleton(IFileSystem, FileSystem); serviceManager.addSingleton(IWorkspaceService, WorkspaceService); serviceManager.addSingleton(IApplicationEnvironment, ApplicationEnvironment); diff --git a/test/e2e/suite/projectRename.e2e.test.ts b/test/e2e/suite/projectRename.e2e.test.ts index 4d123285cf..0aba4869ad 100644 --- a/test/e2e/suite/projectRename.e2e.test.ts +++ b/test/e2e/suite/projectRename.e2e.test.ts @@ -40,14 +40,27 @@ async function leaveUnsavedCellEdit(): Promise { await openWorkspaceFile(DIRTIED_FILE); - // Focus the first CODE cell's Monaco editor by clicking its visible source line (markdown cells - // render without an editor, so scope to code rows), then type a marker into the focused input. - const line = await driver.wait( - async () => (await driver.findElements(By.css('.notebookOverlay .code-cell-row .view-line')))[0], + // Click the first CODE cell's source line to focus its editor (markdown cells have none); locate + // and click in one retried step, since the cell re-renders on open and can stale a prior reference. + await driver.wait( + async () => { + const line = (await driver.findElements(By.css('.notebookOverlay .code-cell-row .view-line')))[0]; + + if (!line) { + return false; + } + + try { + await line.click(); + + return true; + } catch { + return false; + } + }, WORKBENCH_TIMEOUT, - 'the notebook code cell did not render' + 'the notebook code cell did not render or settle enough to focus' ); - await line.click(); await driver.sleep(400); await driver.switchTo().activeElement().sendKeys(DIRTY_MARKER);