From e89b6b63886b831ce41f98cdfc4e57bcf4693d4f Mon Sep 17 00:00:00 2001 From: tomas Date: Sun, 29 Mar 2026 21:07:03 +0000 Subject: [PATCH 01/17] feat(telemetry): add PostHog analytics for tracking notebook usage events Introduces an opt-out PostHog telemetry service that tracks user interactions with Deepnote notebooks, including block additions, cell executions, environment operations, integration management, and project/notebook lifecycle events. Co-Authored-By: Claude Opus 4.6 (1M context) --- package-lock.json | 98 ++++++++++-- package.json | 7 + src/extension.node.ts | 8 + .../deepnoteEnvironmentsView.node.ts | 13 +- .../deepnoteEnvironmentsView.unit.test.ts | 3 +- .../deepnote/deepnoteActivationService.ts | 11 +- .../deepnoteCellExecutionAnalytics.ts | 63 ++++++++ .../deepnote/deepnoteExplorerView.ts | 66 +++++--- .../deepnoteNotebookCommandListener.ts | 16 +- ...epnoteNotebookCommandListener.unit.test.ts | 8 +- .../integrations/integrationWebview.ts | 12 +- .../deepnote/openInDeepnoteHandler.node.ts | 13 +- .../openInDeepnoteHandler.node.unit.test.ts | 2 +- src/notebooks/notebookCommandListener.ts | 8 +- src/notebooks/serviceRegistry.node.ts | 5 + src/platform/analytics/constants.ts | 2 + .../analytics/posthogAnalyticsService.ts | 80 ++++++++++ .../posthogAnalyticsService.unit.test.ts | 148 ++++++++++++++++++ src/platform/analytics/types.ts | 6 + src/platform/serviceRegistry.node.ts | 3 + 20 files changed, 524 insertions(+), 48 deletions(-) create mode 100644 src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts create mode 100644 src/platform/analytics/constants.ts create mode 100644 src/platform/analytics/posthogAnalyticsService.ts create mode 100644 src/platform/analytics/posthogAnalyticsService.unit.test.ts create mode 100644 src/platform/analytics/types.ts diff --git a/package-lock.json b/package-lock.json index 187dc191cd..66c2ae9687 100644 --- a/package-lock.json +++ b/package-lock.json @@ -63,6 +63,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", @@ -9683,6 +9684,17 @@ "node": ">=4" } }, + "node_modules/axios": { + "version": "1.14.0", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.14.0.tgz", + "integrity": "sha512-3Y8yrqLSwjuzpXuZ0oIYZ/XGgLwUIBU3uLvbcpb0pidD9ctpShJd43KSlEEkVQg6DS0G9NKyzOvBfUtDKEyHvQ==", + "license": "MIT", + "dependencies": { + "follow-redirects": "^1.15.11", + "form-data": "^4.0.5", + "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", @@ -15590,6 +15602,26 @@ "dev": true, "license": "ISC" }, + "node_modules/follow-redirects": { + "version": "1.15.11", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.11.tgz", + "integrity": "sha512-deG2P0JfjrTxl50XGCDyfI97ZGVCxIpfKYmfyrQ54n5FO/0gfIES8C/Psl6kWVDolizcaaxZJnTS0QSMxvnsBQ==", + "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", @@ -15668,9 +15700,9 @@ } }, "node_modules/form-data": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.4.tgz", - "integrity": "sha512-KrGhL9Q4zjj0kiUt5OO4Mr/A/jlI2jDYs5eHBpYHPcBEVSiipAvn2Ko2HnPe20rmcuuvMHNdZFp+4IlGTMF0Ow==", + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.5.tgz", + "integrity": "sha512-8RipRLol37bNs2bhoV67fiTEvdTrbMUYcFTiy3+wuuOnUog2QBHCZWXDRijWQfAkhBj2Uf5UnVaiWwA5vdd82w==", "license": "MIT", "dependencies": { "asynckit": "^0.4.0", @@ -25016,6 +25048,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", @@ -25305,6 +25349,15 @@ "node": ">= 8" } }, + "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", @@ -38348,6 +38401,16 @@ "integrity": "sha512-/dlp0fxyM3R8YW7MFzaHWXrf4zzbr0vaYb23VBFCl83R7nWNPg/yaQw2Dc8jzCMmDVLhSdzH8MjrsuIUuvX+6g==", "dev": true }, + "axios": { + "version": "1.14.0", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.14.0.tgz", + "integrity": "sha512-3Y8yrqLSwjuzpXuZ0oIYZ/XGgLwUIBU3uLvbcpb0pidD9ctpShJd43KSlEEkVQg6DS0G9NKyzOvBfUtDKEyHvQ==", + "requires": { + "follow-redirects": "^1.15.11", + "form-data": "^4.0.5", + "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", @@ -38740,8 +38803,7 @@ } }, "brace-expansion": { - "version": "1.1.12", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "version": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", "requires": { "balanced-match": "^1.0.0", @@ -42674,6 +42736,11 @@ "integrity": "sha512-PjDse7RzhcPkIJwy5t7KPWQSZ9cAbzQXcafsetQoD7sOJRQlGikNbx7yZp2OotDnJyrDcbyRq3Ttb18iYOqkxA==", "dev": true }, + "follow-redirects": { + "version": "1.15.11", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.11.tgz", + "integrity": "sha512-deG2P0JfjrTxl50XGCDyfI97ZGVCxIpfKYmfyrQ54n5FO/0gfIES8C/Psl6kWVDolizcaaxZJnTS0QSMxvnsBQ==" + }, "font-awesome": { "version": "4.7.0", "resolved": "https://registry.npmjs.org/font-awesome/-/font-awesome-4.7.0.tgz", @@ -42732,9 +42799,9 @@ } }, "form-data": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.4.tgz", - "integrity": "sha512-KrGhL9Q4zjj0kiUt5OO4Mr/A/jlI2jDYs5eHBpYHPcBEVSiipAvn2Ko2HnPe20rmcuuvMHNdZFp+4IlGTMF0Ow==", + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.5.tgz", + "integrity": "sha512-8RipRLol37bNs2bhoV67fiTEvdTrbMUYcFTiy3+wuuOnUog2QBHCZWXDRijWQfAkhBj2Uf5UnVaiWwA5vdd82w==", "requires": { "asynckit": "^0.4.0", "combined-stream": "^1.0.8", @@ -49237,6 +49304,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", @@ -49448,6 +49523,11 @@ "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", "dev": true }, + "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", @@ -50867,7 +50947,7 @@ "integrity": "sha512-DF7ePE5bwitJrRdJSNrV+qAnQsfds0GbRA02ywy6TQrQewkm9DSHGDUxJaoJk2WUMlyQ7Odrf2o1PCZM50BcSg==", "requires": { "jquery": ">=1.8.0", - "jquery-ui": ">=1.8.0" + "jquery-ui": "1.13.2" } }, "smart-buffer": { diff --git a/package.json b/package.json index 90a4c6df6e..c9cdb45a55 100644 --- a/package.json +++ b/package.json @@ -1650,6 +1650,12 @@ "description": "Disable SSL certificate verification (for development only)", "scope": "application" }, + "deepnote.telemetry.enabled": { + "type": "boolean", + "default": true, + "description": "Enable anonymous usage telemetry to help improve Deepnote for VS Code.", + "scope": "application" + }, "deepnote.snapshots.enabled": { "type": "boolean", "default": true, @@ -2725,6 +2731,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/src/extension.node.ts b/src/extension.node.ts index 13460ca9ad..d7a26f424e 100644 --- a/src/extension.node.ts +++ b/src/extension.node.ts @@ -38,6 +38,7 @@ import './platform/logging'; import { commands, env, ExtensionMode, UIKind, workspace, type OutputChannel } from 'vscode'; import { buildApi, IExtensionApi } from './standalone/api'; import { logger, setHomeDirectory } from './platform/logging'; +import { IPostHogAnalyticsService } from './platform/analytics/types'; import { IAsyncDisposableRegistry, IExtensionContext, IsDevMode } from './platform/common/types'; import { IServiceContainer, IServiceManager } from './platform/ioc/types'; import { sendStartupTelemetry } from './platform/telemetry/startupTelemetry'; @@ -133,7 +134,14 @@ export function deactivate(): Thenable { Exiting.isExiting = true; // Make sure to shutdown anybody who needs it. if (activatedServiceContainer) { + const analytics = activatedServiceContainer.tryGet(IPostHogAnalyticsService); + + if (analytics) { + void analytics.shutdown(); + } + const registry = activatedServiceContainer.get(IAsyncDisposableRegistry); + if (registry) { return registry.dispose(); } diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts index 4d6c3ec7fa..fe3934342b 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts @@ -1,4 +1,4 @@ -import { inject, injectable, named } from 'inversify'; +import { inject, injectable, named, optional } from 'inversify'; import { commands, Disposable, @@ -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 { IPostHogAnalyticsService } from '../../../platform/analytics/types'; import { IDisposableRegistry, IOutputChannel } from '../../../platform/common/types'; import { createDeepnoteServerConfigHandle } from '../../../platform/deepnote/deepnoteServerUtils.node'; import { DeepnoteToolkitMissingError } from '../../../platform/errors/deepnoteKernelErrors'; @@ -52,7 +53,8 @@ export class DeepnoteEnvironmentsView implements Disposable { @inject(IDeepnoteNotebookEnvironmentMapper) private readonly notebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper, @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider, - @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel + @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, + @inject(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined ) { // Create tree data provider @@ -193,6 +195,11 @@ export class DeepnoteEnvironmentsView implements Disposable { const config = await this.environmentManager.createEnvironment(options, token); logger.info(`Created environment: ${config.id} (${config.name})`); + this.analytics?.trackEvent('create_environment', { + hasDescription: !!options.description, + hasPackages: !!options.packages?.length + }); + void window.showInformationMessage( l10n.t('Environment "{0}" created successfully!', config.name) ); @@ -314,6 +321,7 @@ export class DeepnoteEnvironmentsView implements Disposable { } ); + this.analytics?.trackEvent('delete_environment'); void window.showInformationMessage(l10n.t('Environment "{0}" deleted', config.name)); } catch (error) { logger.error('Failed to delete environment', error); @@ -483,6 +491,7 @@ export class DeepnoteEnvironmentsView implements Disposable { } ); + this.analytics?.trackEvent('select_environment'); void window.showInformationMessage(l10n.t('Environment switched successfully')); } catch (error) { if (error instanceof DeepnoteToolkitMissingError) { diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts index 0407420162..d7d761a6d4 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts @@ -61,7 +61,8 @@ suite('DeepnoteEnvironmentsView', () => { instance(mockKernelAutoSelector), instance(mockNotebookEnvironmentMapper), instance(mockKernelProvider), - instance(mockOutputChannel) + instance(mockOutputChannel), + undefined ); }); diff --git a/src/notebooks/deepnote/deepnoteActivationService.ts b/src/notebooks/deepnote/deepnoteActivationService.ts index 96c35288cd..4162182f13 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 { IPostHogAnalyticsService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { ILogger } from '../../platform/logging/types'; import { IDeepnoteNotebookManager } from '../types'; @@ -34,7 +35,8 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, @inject(IIntegrationManager) integrationManager: IIntegrationManager, @inject(ILogger) private readonly logger: ILogger, - @inject(SnapshotService) @optional() private readonly snapshotService?: SnapshotService + @inject(SnapshotService) @optional() private readonly snapshotService?: SnapshotService, + @inject(IPostHogAnalyticsService) @optional() private readonly analytics?: IPostHogAnalyticsService ) { this.integrationManager = integrationManager; } @@ -45,7 +47,12 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic */ public activate() { this.serializer = new DeepnoteNotebookSerializer(this.notebookManager, this.snapshotService); - this.explorerView = new DeepnoteExplorerView(this.extensionContext, this.notebookManager, this.logger); + this.explorerView = new DeepnoteExplorerView( + this.extensionContext, + this.notebookManager, + this.logger, + this.analytics + ); this.editProtection = new DeepnoteInputBlockEditProtection(this.logger); this.snapshotsEnabled = this.isSnapshotsEnabled(); diff --git a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts new file mode 100644 index 0000000000..f9bb28793c --- /dev/null +++ b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts @@ -0,0 +1,63 @@ +import { inject, injectable, optional } from 'inversify'; +import { Disposable } from 'vscode'; + +import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { IPostHogAnalyticsService } from '../../platform/analytics/types'; +import { IDisposableRegistry } from '../../platform/common/types'; +import { NotebookCellExecutionState, notebookCellExecutions } from '../../platform/notebooks/cellExecutionStateService'; +import { IDeepnoteNotebookManager } from '../types'; + +/** + * Tracks cell execution events for PostHog analytics. + */ +@injectable() +export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationService { + constructor( + @inject(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined, + @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, + @inject(IDisposableRegistry) private readonly disposables: Disposable[] + ) {} + + public activate(): void { + if (!this.analytics) { + return; + } + + this.disposables.push( + notebookCellExecutions.onDidChangeNotebookCellExecutionState((e) => { + if (e.state !== NotebookCellExecutionState.Executing) { + return; + } + + if (e.cell.notebook.notebookType !== 'deepnote') { + return; + } + + const languageId = e.cell.document.languageId; + const cellType = languageId === 'sql' ? 'sql' : languageId === 'markdown' ? 'markdown' : 'code'; + + const properties: Record = { cellType }; + + if (cellType === 'sql') { + const integrationId = + e.cell.metadata?.__deepnotePocket?.sql_integration_id ?? e.cell.metadata?.sql_integration_id; + + if (integrationId) { + const projectId = e.cell.notebook.metadata?.deepnoteProjectId; + + if (projectId) { + const project = this.notebookManager.getOriginalProject(projectId); + const integration = project?.project.integrations?.find((i) => i.id === integrationId); + + if (integration?.type) { + properties.integrationType = integration.type; + } + } + } + } + + this.analytics?.trackEvent('execute_cell', properties); + }) + ); + } +} diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index 33599da2f0..bac596ca7f 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -3,6 +3,7 @@ import { commands, window, workspace, type TreeView, Uri, l10n } from 'vscode'; import { serializeDeepnoteFile, type DeepnoteBlock, type DeepnoteFile } from '@deepnote/blocks'; import { convertDeepnoteToJupyterNotebooks, convertIpynbFilesToDeepnoteFile } from '@deepnote/convert'; +import { IPostHogAnalyticsService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { IDeepnoteNotebookManager } from '../types'; import { DeepnoteTreeDataProvider } from './deepnoteTreeDataProvider'; @@ -25,7 +26,8 @@ export class DeepnoteExplorerView { constructor( @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, @inject(IDeepnoteNotebookManager) private readonly manager: IDeepnoteNotebookManager, - @inject(ILogger) logger: ILogger + @inject(ILogger) logger: ILogger, + private readonly analytics?: IPostHogAnalyticsService ) { this.treeDataProvider = new DeepnoteTreeDataProvider(logger); } @@ -332,9 +334,10 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.OpenDeepnoteNotebook, (context: DeepnoteTreeItemContext) => - this.openNotebook(context) - ) + commands.registerCommand(Commands.OpenDeepnoteNotebook, (context: DeepnoteTreeItemContext) => { + this.analytics?.trackEvent('open_notebook'); + return this.openNotebook(context); + }) ); this.extensionContext.subscriptions.push( @@ -346,19 +349,31 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.NewProject, () => this.newProject()) + commands.registerCommand(Commands.NewProject, () => { + this.analytics?.trackEvent('create_project'); + return this.newProject(); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ImportNotebook, () => this.importNotebook()) + commands.registerCommand(Commands.ImportNotebook, () => { + this.analytics?.trackEvent('import_notebook'); + return this.importNotebook(); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ImportJupyterNotebook, () => this.importJupyterNotebook()) + commands.registerCommand(Commands.ImportJupyterNotebook, () => { + this.analytics?.trackEvent('import_notebook'); + return this.importJupyterNotebook(); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.NewNotebook, () => this.newNotebook()) + commands.registerCommand(Commands.NewNotebook, () => { + this.analytics?.trackEvent('create_notebook'); + return this.newNotebook(); + }) ); // Context menu commands for tree items @@ -369,9 +384,10 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.DeleteProject, (treeItem: DeepnoteTreeItem) => - this.deleteProject(treeItem) - ) + commands.registerCommand(Commands.DeleteProject, (treeItem: DeepnoteTreeItem) => { + this.analytics?.trackEvent('delete_project'); + return this.deleteProject(treeItem); + }) ); this.extensionContext.subscriptions.push( @@ -381,15 +397,17 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.DeleteNotebook, (treeItem: DeepnoteTreeItem) => - this.deleteNotebook(treeItem) - ) + commands.registerCommand(Commands.DeleteNotebook, (treeItem: DeepnoteTreeItem) => { + this.analytics?.trackEvent('delete_notebook'); + return this.deleteNotebook(treeItem); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.DuplicateNotebook, (treeItem: DeepnoteTreeItem) => - this.duplicateNotebook(treeItem) - ) + commands.registerCommand(Commands.DuplicateNotebook, (treeItem: DeepnoteTreeItem) => { + this.analytics?.trackEvent('duplicate_notebook'); + return this.duplicateNotebook(treeItem); + }) ); this.extensionContext.subscriptions.push( @@ -399,15 +417,17 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ExportProject, (treeItem: DeepnoteTreeItem) => - this.exportProject(treeItem) - ) + commands.registerCommand(Commands.ExportProject, (treeItem: DeepnoteTreeItem) => { + this.analytics?.trackEvent('export_notebook', { format: 'project' }); + return this.exportProject(treeItem); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ExportNotebook, (treeItem: DeepnoteTreeItem) => - this.exportNotebook(treeItem) - ) + commands.registerCommand(Commands.ExportNotebook, (treeItem: DeepnoteTreeItem) => { + this.analytics?.trackEvent('export_notebook', { format: 'notebook' }); + return this.exportNotebook(treeItem); + }) ); } diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts index d91608c15a..1a2150072b 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts @@ -1,4 +1,4 @@ -import { injectable, inject } from 'inversify'; +import { injectable, inject, optional } from 'inversify'; import { commands, ConfigurationTarget, @@ -17,6 +17,7 @@ import z from 'zod'; import { logger } from '../../platform/logging'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { IPostHogAnalyticsService } 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(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined, @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.analytics?.trackEvent('add_block', { blockType: '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.analytics?.trackEvent('add_block', { blockType: '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.analytics?.trackEvent('add_block', { blockType: '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.analytics?.trackEvent('add_block', { 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.analytics?.trackEvent('add_block', { blockType: 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('toggle_snapshots', { enabled: false }); void window.showInformationMessage(l10n.t('Snapshots disabled for this workspace.')); } catch (error) { logger.error('Failed to disable snapshots', error); @@ -569,6 +582,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation undefined, ConfigurationTarget.Workspace ); + this.analytics?.trackEvent('toggle_snapshots', { enabled: true }); } catch (error) { logger.error('Failed to enable snapshots', error); void window.showErrorMessage(l10n.t('Failed to enable snapshots.')); diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts index ff600a2bf9..1683cfffba 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts @@ -44,7 +44,7 @@ suite('DeepnoteNotebookCommandListener', () => { sandbox = sinon.createSandbox(); disposables = []; mockConfigService = createMockConfigService(); - commandListener = new DeepnoteNotebookCommandListener(mockConfigService, disposables); + commandListener = new DeepnoteNotebookCommandListener(undefined, mockConfigService, disposables); }); teardown(() => { @@ -89,7 +89,11 @@ suite('DeepnoteNotebookCommandListener', () => { // Create new instance and activate again const disposables2: IDisposable[] = []; - const commandListener2 = new DeepnoteNotebookCommandListener(createMockConfigService(), disposables2); + const commandListener2 = new DeepnoteNotebookCommandListener( + undefined, + createMockConfigService(), + disposables2 + ); commandListener2.activate(); // Both should register the same number of commands diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index 3daf4dc479..33f7c817d4 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -1,6 +1,7 @@ -import { inject, injectable } from 'inversify'; +import { inject, injectable, optional } from 'inversify'; import { Disposable, l10n, Uri, ViewColumn, WebviewPanel, window } from 'vscode'; +import { IPostHogAnalyticsService } from '../../../platform/analytics/types'; import { IExtensionContext } from '../../../platform/common/types'; import * as localize from '../../../platform/common/utils/localize'; import { logger } from '../../../platform/logging'; @@ -29,7 +30,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { constructor( @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, @inject(IIntegrationStorage) private readonly integrationStorage: IIntegrationStorage, - @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager + @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, + @inject(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined ) {} /** @@ -434,21 +436,27 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { switch (message.type) { case 'configure': if (message.integrationId) { + this.analytics?.trackEvent('configure_integration'); await this.showConfigurationForm(message.integrationId); } break; case 'save': if (message.integrationId && message.config) { + this.analytics?.trackEvent('save_integration', { + integrationType: message.config.type ?? 'unknown' + }); await this.saveConfiguration(message.integrationId, message.config); } break; case 'reset': if (message.integrationId) { + this.analytics?.trackEvent('reset_integration'); await this.resetConfiguration(message.integrationId); } break; case 'delete': if (message.integrationId) { + this.analytics?.trackEvent('delete_integration'); await this.deleteConfiguration(message.integrationId); } break; diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts index 4a62c634d7..d5301413cd 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts @@ -1,9 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { injectable, inject } from 'inversify'; +import { injectable, inject, optional } from 'inversify'; import { commands, window, Uri, env, l10n } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { IPostHogAnalyticsService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { Commands } from '../../platform/common/constants'; import { logger } from '../../platform/logging'; @@ -13,11 +14,17 @@ 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(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined + ) {} public activate(): void { this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.OpenInDeepnote, () => this.handleOpenInDeepnote()) + commands.registerCommand(Commands.OpenInDeepnote, () => { + this.analytics?.trackEvent('open_in_deepnote'); + return this.handleOpenInDeepnote(); + }) ); } diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts index 8b7d0240d5..bed3c1ff56 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts @@ -49,7 +49,7 @@ suite('OpenInDeepnoteHandler', () => { subscriptions: [] } as any; - handler = new OpenInDeepnoteHandlerClass(mockExtensionContext); + handler = new OpenInDeepnoteHandlerClass(mockExtensionContext, undefined); }); teardown(() => { diff --git a/src/notebooks/notebookCommandListener.ts b/src/notebooks/notebookCommandListener.ts index 04907cb1ca..5ed7c3c522 100644 --- a/src/notebooks/notebookCommandListener.ts +++ b/src/notebooks/notebookCommandListener.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { inject, injectable, optional } from 'inversify'; import { ConfigurationTarget, @@ -33,6 +33,7 @@ import { getNotebookMetadata } from '../platform/common/utils'; import { KernelConnector } from './controllers/kernelConnector'; import { IControllerRegistration } from './controllers/types'; import { IExtensionSyncActivationService } from '../platform/activation/types'; +import { IPostHogAnalyticsService } from '../platform/analytics/types'; import { IKernelStatusProvider } from '../kernels/kernelStatusProvider'; export const INotebookCommandHandler = Symbol('INotebookCommandHandler'); @@ -54,7 +55,8 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens @inject(IDataScienceErrorHandler) private errorHandler: IDataScienceErrorHandler, @inject(INotebookEditorProvider) private notebookEditorProvider: INotebookEditorProvider, @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IKernelStatusProvider) private kernelStatusProvider: IKernelStatusProvider + @inject(IKernelStatusProvider) private kernelStatusProvider: IKernelStatusProvider, + @inject(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined ) {} activate(): void { @@ -114,6 +116,7 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens private runAllCells() { if (window.activeNotebookEditor) { + this.analytics?.trackEvent('execute_notebook'); commands.executeCommand('notebook.execute').then(noop, noop); } } @@ -141,6 +144,7 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens private addCellBelow() { if (window.activeNotebookEditor) { + this.analytics?.trackEvent('add_block', { blockType: 'code' }); commands.executeCommand('notebook.cell.insertCodeCellBelow').then(noop, noop); } } diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index cbd8b860fe..0a893faffa 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -83,6 +83,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'; @@ -173,6 +174,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..acbefc5b66 --- /dev/null +++ b/src/platform/analytics/constants.ts @@ -0,0 +1,2 @@ +export const POSTHOG_API_KEY = '__POSTHOG_API_KEY__'; +export const POSTHOG_HOST = 'https://us.i.posthog.com'; diff --git a/src/platform/analytics/posthogAnalyticsService.ts b/src/platform/analytics/posthogAnalyticsService.ts new file mode 100644 index 0000000000..90d1f6118e --- /dev/null +++ b/src/platform/analytics/posthogAnalyticsService.ts @@ -0,0 +1,80 @@ +import { inject, injectable } from 'inversify'; +import { PostHog } from 'posthog-node'; +import { workspace } from 'vscode'; + +import { IPersistentState, IPersistentStateFactory } from '../common/types'; +import { generateUuid } from '../common/uuid'; +import { logger } from '../logging'; +import { POSTHOG_API_KEY, POSTHOG_HOST } from './constants'; +import { IPostHogAnalyticsService } from './types'; + +const USER_ID_STORAGE_KEY = 'posthog-anonymous-user-id'; + +@injectable() +export class PostHogAnalyticsService implements IPostHogAnalyticsService { + private client: PostHog | undefined; + + private initialized = false; + + private userIdState: IPersistentState | undefined; + + constructor(@inject(IPersistentStateFactory) private readonly stateFactory: IPersistentStateFactory) {} + + public trackEvent(eventName: string, properties?: Record): void { + try { + if (!this.isTelemetryEnabled()) { + return; + } + + if (!this.initialized) { + this.initialize(); + } + + if (!this.client || !this.userIdState) { + return; + } + + this.client.capture({ + distinctId: this.userIdState.value, + event: eventName, + properties + }); + } catch (ex) { + logger.debug(`PostHog analytics error: ${ex}`); + } + } + + public async shutdown(): Promise { + try { + await this.client?.shutdown(); + } catch (ex) { + logger.debug(`PostHog shutdown error: ${ex}`); + } + } + + private initialize(): void { + this.initialized = true; + + this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, ''); + + if (!this.userIdState.value) { + void this.userIdState.updateValue(generateUuid()); + } + + this.client = new PostHog(POSTHOG_API_KEY, { + flushAt: 20, + flushInterval: 30000, + host: POSTHOG_HOST + }); + } + + private isTelemetryEnabled(): boolean { + const telemetryLevel = workspace.getConfiguration('telemetry').get('telemetryLevel', 'all'); + + if (telemetryLevel === 'off') { + return false; + } + + return workspace.getConfiguration('deepnote').get('telemetry.enabled', true); + } +} diff --git a/src/platform/analytics/posthogAnalyticsService.unit.test.ts b/src/platform/analytics/posthogAnalyticsService.unit.test.ts new file mode 100644 index 0000000000..1bfc344782 --- /dev/null +++ b/src/platform/analytics/posthogAnalyticsService.unit.test.ts @@ -0,0 +1,148 @@ +import { assert } from 'chai'; +import * as sinon from 'sinon'; + +import { IPersistentState, IPersistentStateFactory } from '../common/types'; +import { PostHogAnalyticsService } from './posthogAnalyticsService'; + +suite('PostHogAnalyticsService', () => { + let analyticsService: PostHogAnalyticsService; + let mockStateFactory: IPersistentStateFactory; + let mockUserIdState: IPersistentState; + let sandbox: sinon.SinonSandbox; + + function createMockPersistentState(initialValue: string): IPersistentState { + let storedValue = initialValue; + + return { + get value() { + return storedValue; + }, + updateValue: sinon.stub().callsFake(async (newValue: string) => { + storedValue = newValue; + }) + }; + } + + setup(() => { + sandbox = sinon.createSandbox(); + mockUserIdState = createMockPersistentState(''); + mockStateFactory = { + createGlobalPersistentState: sinon.stub().returns(mockUserIdState), + createWorkspacePersistentState: sinon.stub().returns(mockUserIdState) + } as unknown as IPersistentStateFactory; + }); + + teardown(() => { + sandbox.restore(); + }); + + test('should create instance without errors', () => { + analyticsService = new PostHogAnalyticsService(mockStateFactory); + + assert.isDefined(analyticsService); + }); + + test('trackEvent should not throw when telemetry is disabled', () => { + // Stub workspace.getConfiguration to return telemetry disabled + const vscode = require('vscode'); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + sandbox.stub(vscode.workspace, 'getConfiguration').callsFake((section: any) => ({ + get: (_key: string, defaultValue: unknown) => { + if (section === 'deepnote' && _key === 'telemetry.enabled') { + return false; + } + + return defaultValue; + } + })); + + analyticsService = new PostHogAnalyticsService(mockStateFactory); + + assert.doesNotThrow(() => { + analyticsService.trackEvent('test_event', { prop: 'value' }); + }); + + // Should not have initialized (no state access) + assert.isFalse( + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).called, + 'Should not create persistent state when telemetry is disabled' + ); + }); + + test('trackEvent should not throw when VSCode telemetry level is off', () => { + const vscode = require('vscode'); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + sandbox.stub(vscode.workspace, 'getConfiguration').callsFake((section: any) => ({ + get: (_key: string, defaultValue: unknown) => { + if (section === 'telemetry' && _key === 'telemetryLevel') { + return 'off'; + } + + return defaultValue; + } + })); + + analyticsService = new PostHogAnalyticsService(mockStateFactory); + + assert.doesNotThrow(() => { + analyticsService.trackEvent('test_event'); + }); + + assert.isFalse( + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).called, + 'Should not create persistent state when VSCode telemetry is off' + ); + }); + + test('should generate user ID on first trackEvent when telemetry enabled', () => { + const vscode = require('vscode'); + + sandbox.stub(vscode.workspace, 'getConfiguration').callsFake(() => ({ + get: (_key: string, defaultValue: unknown) => defaultValue + })); + + analyticsService = new PostHogAnalyticsService(mockStateFactory); + analyticsService.trackEvent('test_event'); + + assert.isTrue( + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).calledOnce, + 'Should create persistent state' + ); + assert.isTrue( + (mockUserIdState.updateValue as sinon.SinonStub).calledOnce, + 'Should generate and persist user ID' + ); + + const generatedId = (mockUserIdState.updateValue as sinon.SinonStub).firstCall.args[0]; + + assert.isString(generatedId); + assert.isNotEmpty(generatedId, 'Generated user ID should not be empty'); + }); + + test('should reuse existing user ID', () => { + const vscode = require('vscode'); + + sandbox.stub(vscode.workspace, 'getConfiguration').callsFake(() => ({ + get: (_key: string, defaultValue: unknown) => defaultValue + })); + + mockUserIdState = createMockPersistentState('existing-user-id'); + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).returns(mockUserIdState); + + analyticsService = new PostHogAnalyticsService(mockStateFactory); + analyticsService.trackEvent('test_event'); + + assert.isFalse( + (mockUserIdState.updateValue as sinon.SinonStub).called, + 'Should not update value when user ID already exists' + ); + }); + + test('shutdown should not throw even when client is not initialized', async () => { + analyticsService = new PostHogAnalyticsService(mockStateFactory); + + await assert.isFulfilled(analyticsService.shutdown()); + }); +}); diff --git a/src/platform/analytics/types.ts b/src/platform/analytics/types.ts new file mode 100644 index 0000000000..2a48fcf60f --- /dev/null +++ b/src/platform/analytics/types.ts @@ -0,0 +1,6 @@ +export const IPostHogAnalyticsService = Symbol('IPostHogAnalyticsService'); + +export interface IPostHogAnalyticsService { + trackEvent(eventName: string, properties?: Record): void; + shutdown(): Promise; +} diff --git a/src/platform/serviceRegistry.node.ts b/src/platform/serviceRegistry.node.ts index 73599e069a..934f98223c 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 { IPostHogAnalyticsService } from './analytics/types'; +import { PostHogAnalyticsService } from './analytics/posthogAnalyticsService'; import { DataScienceStartupTime } from './common/constants'; import { IExtensionSyncActivationService } from './activation/types'; import { IConfigurationService, IDataScienceCommandListener } from './common/types'; @@ -26,6 +28,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addBinding(FileSystem, IFileSystemNode); serviceManager.addBinding(FileSystem, IFileSystem); serviceManager.addSingleton(IWorkspaceService, WorkspaceService); + serviceManager.addSingleton(IPostHogAnalyticsService, PostHogAnalyticsService); serviceManager.addSingleton(IConfigurationService, ConfigurationService); registerApiTypes(serviceManager); From 2cc5a5b8dce53a91d1d9629bd83a41036516d341 Mon Sep 17 00:00:00 2001 From: tomas Date: Sun, 29 Mar 2026 21:17:57 +0000 Subject: [PATCH 02/17] fix(deps): regenerate lockfile with Node 22 npm to fix drift check Co-Authored-By: Claude Opus 4.6 (1M context) --- package-lock.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 66c2ae9687..13a397db57 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38803,7 +38803,8 @@ } }, "brace-expansion": { - "version": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", "requires": { "balanced-match": "^1.0.0", @@ -50947,7 +50948,7 @@ "integrity": "sha512-DF7ePE5bwitJrRdJSNrV+qAnQsfds0GbRA02ywy6TQrQewkm9DSHGDUxJaoJk2WUMlyQ7Odrf2o1PCZM50BcSg==", "requires": { "jquery": ">=1.8.0", - "jquery-ui": "1.13.2" + "jquery-ui": ">=1.8.0" } }, "smart-buffer": { From 01b41d03c4734d501725158aa8b1b1472d62af5c Mon Sep 17 00:00:00 2001 From: tomas Date: Mon, 30 Mar 2026 07:13:16 +0000 Subject: [PATCH 03/17] refactor(telemetry): rename PostHogAnalyticsService to TelemetryService Removes PostHog-specific naming from the interface and implementation to keep the telemetry abstraction provider-agnostic. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/extension.node.ts | 7 --- .../deepnoteEnvironmentsView.node.ts | 4 +- .../deepnote/deepnoteActivationService.ts | 4 +- .../deepnoteCellExecutionAnalytics.ts | 4 +- .../deepnote/deepnoteExplorerView.ts | 44 +++++++++---------- .../deepnoteNotebookCommandListener.ts | 4 +- .../integrations/integrationWebview.ts | 4 +- .../deepnote/openInDeepnoteHandler.node.ts | 8 ++-- src/notebooks/notebookCommandListener.ts | 4 +- ...nalyticsService.ts => telemetryService.ts} | 15 ++++--- ....test.ts => telemetryService.unit.test.ts} | 29 +++++++----- src/platform/analytics/types.ts | 7 +-- src/platform/serviceRegistry.node.ts | 6 +-- 13 files changed, 72 insertions(+), 68 deletions(-) rename src/platform/analytics/{posthogAnalyticsService.ts => telemetryService.ts} (80%) rename src/platform/analytics/{posthogAnalyticsService.unit.test.ts => telemetryService.unit.test.ts} (79%) diff --git a/src/extension.node.ts b/src/extension.node.ts index d7a26f424e..bc22ac6484 100644 --- a/src/extension.node.ts +++ b/src/extension.node.ts @@ -38,7 +38,6 @@ import './platform/logging'; import { commands, env, ExtensionMode, UIKind, workspace, type OutputChannel } from 'vscode'; import { buildApi, IExtensionApi } from './standalone/api'; import { logger, setHomeDirectory } from './platform/logging'; -import { IPostHogAnalyticsService } from './platform/analytics/types'; import { IAsyncDisposableRegistry, IExtensionContext, IsDevMode } from './platform/common/types'; import { IServiceContainer, IServiceManager } from './platform/ioc/types'; import { sendStartupTelemetry } from './platform/telemetry/startupTelemetry'; @@ -134,12 +133,6 @@ export function deactivate(): Thenable { Exiting.isExiting = true; // Make sure to shutdown anybody who needs it. if (activatedServiceContainer) { - const analytics = activatedServiceContainer.tryGet(IPostHogAnalyticsService); - - if (analytics) { - void analytics.shutdown(); - } - const registry = activatedServiceContainer.get(IAsyncDisposableRegistry); if (registry) { diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts index fe3934342b..cf56dbac32 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts @@ -13,7 +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 { IPostHogAnalyticsService } from '../../../platform/analytics/types'; +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 +54,7 @@ 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(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined + @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined ) { // Create tree data provider diff --git a/src/notebooks/deepnote/deepnoteActivationService.ts b/src/notebooks/deepnote/deepnoteActivationService.ts index 4162182f13..0f3696a8df 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.ts @@ -2,7 +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 { IPostHogAnalyticsService } from '../../platform/analytics/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { ILogger } from '../../platform/logging/types'; import { IDeepnoteNotebookManager } from '../types'; @@ -36,7 +36,7 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic @inject(IIntegrationManager) integrationManager: IIntegrationManager, @inject(ILogger) private readonly logger: ILogger, @inject(SnapshotService) @optional() private readonly snapshotService?: SnapshotService, - @inject(IPostHogAnalyticsService) @optional() private readonly analytics?: IPostHogAnalyticsService + @inject(ITelemetryService) @optional() private readonly analytics?: ITelemetryService ) { this.integrationManager = integrationManager; } diff --git a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts index f9bb28793c..25f029b514 100644 --- a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts +++ b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts @@ -2,7 +2,7 @@ import { inject, injectable, optional } from 'inversify'; import { Disposable } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; -import { IPostHogAnalyticsService } from '../../platform/analytics/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IDisposableRegistry } from '../../platform/common/types'; import { NotebookCellExecutionState, notebookCellExecutions } from '../../platform/notebooks/cellExecutionStateService'; import { IDeepnoteNotebookManager } from '../types'; @@ -13,7 +13,7 @@ import { IDeepnoteNotebookManager } from '../types'; @injectable() export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationService { constructor( - @inject(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined, + @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined, @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, @inject(IDisposableRegistry) private readonly disposables: Disposable[] ) {} diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index bac596ca7f..495f37ab0b 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -3,7 +3,7 @@ import { commands, window, workspace, type TreeView, Uri, l10n } from 'vscode'; import { serializeDeepnoteFile, type DeepnoteBlock, type DeepnoteFile } from '@deepnote/blocks'; import { convertDeepnoteToJupyterNotebooks, convertIpynbFilesToDeepnoteFile } from '@deepnote/convert'; -import { IPostHogAnalyticsService } from '../../platform/analytics/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { IDeepnoteNotebookManager } from '../types'; import { DeepnoteTreeDataProvider } from './deepnoteTreeDataProvider'; @@ -27,7 +27,7 @@ export class DeepnoteExplorerView { @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, @inject(IDeepnoteNotebookManager) private readonly manager: IDeepnoteNotebookManager, @inject(ILogger) logger: ILogger, - private readonly analytics?: IPostHogAnalyticsService + private readonly analytics?: ITelemetryService ) { this.treeDataProvider = new DeepnoteTreeDataProvider(logger); } @@ -334,9 +334,9 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.OpenDeepnoteNotebook, (context: DeepnoteTreeItemContext) => { + commands.registerCommand(Commands.OpenDeepnoteNotebook, async (context: DeepnoteTreeItemContext) => { + await this.openNotebook(context); this.analytics?.trackEvent('open_notebook'); - return this.openNotebook(context); }) ); @@ -349,30 +349,30 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.NewProject, () => { + commands.registerCommand(Commands.NewProject, async () => { + await this.newProject(); this.analytics?.trackEvent('create_project'); - return this.newProject(); }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ImportNotebook, () => { + commands.registerCommand(Commands.ImportNotebook, async () => { + await this.importNotebook(); this.analytics?.trackEvent('import_notebook'); - return this.importNotebook(); }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ImportJupyterNotebook, () => { + commands.registerCommand(Commands.ImportJupyterNotebook, async () => { + await this.importJupyterNotebook(); this.analytics?.trackEvent('import_notebook'); - return this.importJupyterNotebook(); }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.NewNotebook, () => { + commands.registerCommand(Commands.NewNotebook, async () => { + await this.newNotebook(); this.analytics?.trackEvent('create_notebook'); - return this.newNotebook(); }) ); @@ -384,9 +384,9 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.DeleteProject, (treeItem: DeepnoteTreeItem) => { + commands.registerCommand(Commands.DeleteProject, async (treeItem: DeepnoteTreeItem) => { + await this.deleteProject(treeItem); this.analytics?.trackEvent('delete_project'); - return this.deleteProject(treeItem); }) ); @@ -397,16 +397,16 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.DeleteNotebook, (treeItem: DeepnoteTreeItem) => { + commands.registerCommand(Commands.DeleteNotebook, async (treeItem: DeepnoteTreeItem) => { + await this.deleteNotebook(treeItem); this.analytics?.trackEvent('delete_notebook'); - return this.deleteNotebook(treeItem); }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.DuplicateNotebook, (treeItem: DeepnoteTreeItem) => { + commands.registerCommand(Commands.DuplicateNotebook, async (treeItem: DeepnoteTreeItem) => { + await this.duplicateNotebook(treeItem); this.analytics?.trackEvent('duplicate_notebook'); - return this.duplicateNotebook(treeItem); }) ); @@ -417,16 +417,16 @@ export class DeepnoteExplorerView { ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ExportProject, (treeItem: DeepnoteTreeItem) => { + commands.registerCommand(Commands.ExportProject, async (treeItem: DeepnoteTreeItem) => { + await this.exportProject(treeItem); this.analytics?.trackEvent('export_notebook', { format: 'project' }); - return this.exportProject(treeItem); }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.ExportNotebook, (treeItem: DeepnoteTreeItem) => { + commands.registerCommand(Commands.ExportNotebook, async (treeItem: DeepnoteTreeItem) => { + await this.exportNotebook(treeItem); this.analytics?.trackEvent('export_notebook', { format: 'notebook' }); - return this.exportNotebook(treeItem); }) ); } diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts index 1a2150072b..7a2e3a5f39 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts @@ -17,7 +17,7 @@ import z from 'zod'; import { logger } from '../../platform/logging'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; -import { IPostHogAnalyticsService } from '../../platform/analytics/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'; @@ -152,7 +152,7 @@ export function getNextDeepnoteVariableName(cells: NotebookCell[], prefix: 'df' @injectable() export class DeepnoteNotebookCommandListener implements IExtensionSyncActivationService { constructor( - @inject(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined, + @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry ) {} diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index 33f7c817d4..30f823f4ce 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -1,7 +1,7 @@ import { inject, injectable, optional } from 'inversify'; import { Disposable, l10n, Uri, ViewColumn, WebviewPanel, window } from 'vscode'; -import { IPostHogAnalyticsService } from '../../../platform/analytics/types'; +import { ITelemetryService } from '../../../platform/analytics/types'; import { IExtensionContext } from '../../../platform/common/types'; import * as localize from '../../../platform/common/utils/localize'; import { logger } from '../../../platform/logging'; @@ -31,7 +31,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(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined + @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined ) {} /** diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts index d5301413cd..988266073d 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts @@ -4,7 +4,7 @@ import { injectable, inject, optional } from 'inversify'; import { commands, window, Uri, env, l10n } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; -import { IPostHogAnalyticsService } from '../../platform/analytics/types'; +import { ITelemetryService } from '../../platform/analytics/types'; import { IExtensionContext } from '../../platform/common/types'; import { Commands } from '../../platform/common/constants'; import { logger } from '../../platform/logging'; @@ -16,14 +16,14 @@ import { initImport, uploadFile, getErrorMessage, MAX_FILE_SIZE, getDeepnoteDoma export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { constructor( @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, - @inject(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined + @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined ) {} public activate(): void { this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.OpenInDeepnote, () => { + commands.registerCommand(Commands.OpenInDeepnote, async () => { + await this.handleOpenInDeepnote(); this.analytics?.trackEvent('open_in_deepnote'); - return this.handleOpenInDeepnote(); }) ); } diff --git a/src/notebooks/notebookCommandListener.ts b/src/notebooks/notebookCommandListener.ts index 5ed7c3c522..febcea905e 100644 --- a/src/notebooks/notebookCommandListener.ts +++ b/src/notebooks/notebookCommandListener.ts @@ -33,7 +33,7 @@ import { getNotebookMetadata } from '../platform/common/utils'; import { KernelConnector } from './controllers/kernelConnector'; import { IControllerRegistration } from './controllers/types'; import { IExtensionSyncActivationService } from '../platform/activation/types'; -import { IPostHogAnalyticsService } from '../platform/analytics/types'; +import { ITelemetryService } from '../platform/analytics/types'; import { IKernelStatusProvider } from '../kernels/kernelStatusProvider'; export const INotebookCommandHandler = Symbol('INotebookCommandHandler'); @@ -56,7 +56,7 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens @inject(INotebookEditorProvider) private notebookEditorProvider: INotebookEditorProvider, @inject(IServiceContainer) private serviceContainer: IServiceContainer, @inject(IKernelStatusProvider) private kernelStatusProvider: IKernelStatusProvider, - @inject(IPostHogAnalyticsService) @optional() private readonly analytics: IPostHogAnalyticsService | undefined + @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined ) {} activate(): void { diff --git a/src/platform/analytics/posthogAnalyticsService.ts b/src/platform/analytics/telemetryService.ts similarity index 80% rename from src/platform/analytics/posthogAnalyticsService.ts rename to src/platform/analytics/telemetryService.ts index 90d1f6118e..f3fccf9029 100644 --- a/src/platform/analytics/posthogAnalyticsService.ts +++ b/src/platform/analytics/telemetryService.ts @@ -2,23 +2,28 @@ import { inject, injectable } from 'inversify'; import { PostHog } from 'posthog-node'; import { workspace } from 'vscode'; -import { IPersistentState, IPersistentStateFactory } from '../common/types'; +import { IAsyncDisposableRegistry, IPersistentState, IPersistentStateFactory } from '../common/types'; import { generateUuid } from '../common/uuid'; import { logger } from '../logging'; import { POSTHOG_API_KEY, POSTHOG_HOST } from './constants'; -import { IPostHogAnalyticsService } from './types'; +import { ITelemetryService } from './types'; const USER_ID_STORAGE_KEY = 'posthog-anonymous-user-id'; @injectable() -export class PostHogAnalyticsService implements IPostHogAnalyticsService { +export class TelemetryService implements ITelemetryService { private client: PostHog | undefined; private initialized = false; private userIdState: IPersistentState | undefined; - constructor(@inject(IPersistentStateFactory) private readonly stateFactory: IPersistentStateFactory) {} + constructor( + @inject(IPersistentStateFactory) private readonly stateFactory: IPersistentStateFactory, + @inject(IAsyncDisposableRegistry) asyncDisposables: IAsyncDisposableRegistry + ) { + asyncDisposables.push(this); + } public trackEvent(eventName: string, properties?: Record): void { try { @@ -44,7 +49,7 @@ export class PostHogAnalyticsService implements IPostHogAnalyticsService { } } - public async shutdown(): Promise { + public async dispose(): Promise { try { await this.client?.shutdown(); } catch (ex) { diff --git a/src/platform/analytics/posthogAnalyticsService.unit.test.ts b/src/platform/analytics/telemetryService.unit.test.ts similarity index 79% rename from src/platform/analytics/posthogAnalyticsService.unit.test.ts rename to src/platform/analytics/telemetryService.unit.test.ts index 1bfc344782..fc63bbaf30 100644 --- a/src/platform/analytics/posthogAnalyticsService.unit.test.ts +++ b/src/platform/analytics/telemetryService.unit.test.ts @@ -1,12 +1,13 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; -import { IPersistentState, IPersistentStateFactory } from '../common/types'; -import { PostHogAnalyticsService } from './posthogAnalyticsService'; +import { IAsyncDisposableRegistry, IPersistentState, IPersistentStateFactory } from '../common/types'; +import { TelemetryService } from './telemetryService'; -suite('PostHogAnalyticsService', () => { - let analyticsService: PostHogAnalyticsService; +suite('TelemetryService', () => { + let analyticsService: TelemetryService; let mockStateFactory: IPersistentStateFactory; + let mockAsyncDisposableRegistry: IAsyncDisposableRegistry; let mockUserIdState: IPersistentState; let sandbox: sinon.SinonSandbox; @@ -30,6 +31,10 @@ suite('PostHogAnalyticsService', () => { createGlobalPersistentState: sinon.stub().returns(mockUserIdState), createWorkspacePersistentState: sinon.stub().returns(mockUserIdState) } as unknown as IPersistentStateFactory; + mockAsyncDisposableRegistry = { + push: sinon.stub(), + dispose: sinon.stub().resolves() + }; }); teardown(() => { @@ -37,7 +42,7 @@ suite('PostHogAnalyticsService', () => { }); test('should create instance without errors', () => { - analyticsService = new PostHogAnalyticsService(mockStateFactory); + analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); assert.isDefined(analyticsService); }); @@ -57,7 +62,7 @@ suite('PostHogAnalyticsService', () => { } })); - analyticsService = new PostHogAnalyticsService(mockStateFactory); + analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); assert.doesNotThrow(() => { analyticsService.trackEvent('test_event', { prop: 'value' }); @@ -84,7 +89,7 @@ suite('PostHogAnalyticsService', () => { } })); - analyticsService = new PostHogAnalyticsService(mockStateFactory); + analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); assert.doesNotThrow(() => { analyticsService.trackEvent('test_event'); @@ -103,7 +108,7 @@ suite('PostHogAnalyticsService', () => { get: (_key: string, defaultValue: unknown) => defaultValue })); - analyticsService = new PostHogAnalyticsService(mockStateFactory); + analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); analyticsService.trackEvent('test_event'); assert.isTrue( @@ -131,7 +136,7 @@ suite('PostHogAnalyticsService', () => { mockUserIdState = createMockPersistentState('existing-user-id'); (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).returns(mockUserIdState); - analyticsService = new PostHogAnalyticsService(mockStateFactory); + analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); analyticsService.trackEvent('test_event'); assert.isFalse( @@ -140,9 +145,9 @@ suite('PostHogAnalyticsService', () => { ); }); - test('shutdown should not throw even when client is not initialized', async () => { - analyticsService = new PostHogAnalyticsService(mockStateFactory); + test('dispose should not throw even when client is not initialized', async () => { + analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); - await assert.isFulfilled(analyticsService.shutdown()); + await assert.isFulfilled(analyticsService.dispose()); }); }); diff --git a/src/platform/analytics/types.ts b/src/platform/analytics/types.ts index 2a48fcf60f..c0276475b8 100644 --- a/src/platform/analytics/types.ts +++ b/src/platform/analytics/types.ts @@ -1,6 +1,7 @@ -export const IPostHogAnalyticsService = Symbol('IPostHogAnalyticsService'); +import { IAsyncDisposable } from '../common/types'; -export interface IPostHogAnalyticsService { +export const ITelemetryService = Symbol('ITelemetryService'); + +export interface ITelemetryService extends IAsyncDisposable { trackEvent(eventName: string, properties?: Record): void; - shutdown(): Promise; } diff --git a/src/platform/serviceRegistry.node.ts b/src/platform/serviceRegistry.node.ts index 934f98223c..3017f187c4 100644 --- a/src/platform/serviceRegistry.node.ts +++ b/src/platform/serviceRegistry.node.ts @@ -6,8 +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 { IPostHogAnalyticsService } from './analytics/types'; -import { PostHogAnalyticsService } from './analytics/posthogAnalyticsService'; +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'; @@ -28,7 +28,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addBinding(FileSystem, IFileSystemNode); serviceManager.addBinding(FileSystem, IFileSystem); serviceManager.addSingleton(IWorkspaceService, WorkspaceService); - serviceManager.addSingleton(IPostHogAnalyticsService, PostHogAnalyticsService); + serviceManager.addSingleton(ITelemetryService, TelemetryService); serviceManager.addSingleton(IConfigurationService, ConfigurationService); registerApiTypes(serviceManager); From c1181bd56a0b52f2b39c0376b3d20b8c4b0185ea Mon Sep 17 00:00:00 2001 From: tomas Date: Mon, 30 Mar 2026 08:01:47 +0000 Subject: [PATCH 04/17] refactor(telemetry): use typed event names and single-argument trackEvent Replace untyped string event names with a TelemetryEventName literal union type for static checking. Change trackEvent signature to accept a single TelemetryEvent object ({ eventName, properties? }) instead of positional args. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../deepnoteEnvironmentsView.node.ts | 13 +++++---- .../deepnoteCellExecutionAnalytics.ts | 4 +-- .../deepnote/deepnoteExplorerView.ts | 20 ++++++------- .../deepnoteNotebookCommandListener.ts | 14 ++++----- .../integrations/integrationWebview.ts | 11 +++---- .../deepnote/openInDeepnoteHandler.node.ts | 2 +- src/notebooks/notebookCommandListener.ts | 4 +-- src/platform/analytics/telemetryService.ts | 6 ++-- .../analytics/telemetryService.unit.test.ts | 8 ++--- src/platform/analytics/types.ts | 29 ++++++++++++++++++- 10 files changed, 71 insertions(+), 40 deletions(-) diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts index cf56dbac32..bf6abef5d9 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts @@ -195,9 +195,12 @@ export class DeepnoteEnvironmentsView implements Disposable { const config = await this.environmentManager.createEnvironment(options, token); logger.info(`Created environment: ${config.id} (${config.name})`); - this.analytics?.trackEvent('create_environment', { - hasDescription: !!options.description, - hasPackages: !!options.packages?.length + this.analytics?.trackEvent({ + eventName: 'create_environment', + properties: { + hasDescription: !!options.description, + hasPackages: !!options.packages?.length + } }); void window.showInformationMessage( @@ -321,7 +324,7 @@ export class DeepnoteEnvironmentsView implements Disposable { } ); - this.analytics?.trackEvent('delete_environment'); + 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); @@ -491,7 +494,7 @@ export class DeepnoteEnvironmentsView implements Disposable { } ); - this.analytics?.trackEvent('select_environment'); + this.analytics?.trackEvent({ eventName: 'select_environment' }); void window.showInformationMessage(l10n.t('Environment switched successfully')); } catch (error) { if (error instanceof DeepnoteToolkitMissingError) { diff --git a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts index 25f029b514..14f66b87a5 100644 --- a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts +++ b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts @@ -8,7 +8,7 @@ import { NotebookCellExecutionState, notebookCellExecutions } from '../../platfo import { IDeepnoteNotebookManager } from '../types'; /** - * Tracks cell execution events for PostHog analytics. + * Tracks cell execution events for telemetry. */ @injectable() export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationService { @@ -56,7 +56,7 @@ export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationS } } - this.analytics?.trackEvent('execute_cell', properties); + this.analytics?.trackEvent({ eventName: 'execute_cell', properties }); }) ); } diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index 495f37ab0b..7e0435d112 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -336,7 +336,7 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.OpenDeepnoteNotebook, async (context: DeepnoteTreeItemContext) => { await this.openNotebook(context); - this.analytics?.trackEvent('open_notebook'); + this.analytics?.trackEvent({ eventName: 'open_notebook' }); }) ); @@ -351,28 +351,28 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.NewProject, async () => { await this.newProject(); - this.analytics?.trackEvent('create_project'); + this.analytics?.trackEvent({ eventName: 'create_project' }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ImportNotebook, async () => { await this.importNotebook(); - this.analytics?.trackEvent('import_notebook'); + this.analytics?.trackEvent({ eventName: 'import_notebook' }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ImportJupyterNotebook, async () => { await this.importJupyterNotebook(); - this.analytics?.trackEvent('import_notebook'); + this.analytics?.trackEvent({ eventName: 'import_notebook' }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.NewNotebook, async () => { await this.newNotebook(); - this.analytics?.trackEvent('create_notebook'); + this.analytics?.trackEvent({ eventName: 'create_notebook' }); }) ); @@ -386,7 +386,7 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DeleteProject, async (treeItem: DeepnoteTreeItem) => { await this.deleteProject(treeItem); - this.analytics?.trackEvent('delete_project'); + this.analytics?.trackEvent({ eventName: 'delete_project' }); }) ); @@ -399,14 +399,14 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DeleteNotebook, async (treeItem: DeepnoteTreeItem) => { await this.deleteNotebook(treeItem); - this.analytics?.trackEvent('delete_notebook'); + this.analytics?.trackEvent({ eventName: 'delete_notebook' }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DuplicateNotebook, async (treeItem: DeepnoteTreeItem) => { await this.duplicateNotebook(treeItem); - this.analytics?.trackEvent('duplicate_notebook'); + this.analytics?.trackEvent({ eventName: 'duplicate_notebook' }); }) ); @@ -419,14 +419,14 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ExportProject, async (treeItem: DeepnoteTreeItem) => { await this.exportProject(treeItem); - this.analytics?.trackEvent('export_notebook', { format: 'project' }); + this.analytics?.trackEvent({ eventName: 'export_notebook', properties: { format: 'project' } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ExportNotebook, async (treeItem: DeepnoteTreeItem) => { await this.exportNotebook(treeItem); - this.analytics?.trackEvent('export_notebook', { format: 'notebook' }); + this.analytics?.trackEvent({ eventName: 'export_notebook', properties: { format: 'notebook' } }); }) ); } diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts index 7a2e3a5f39..acec3062d3 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts @@ -266,7 +266,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert SQL block')); } - this.analytics?.trackEvent('add_block', { blockType: 'sql' }); + this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: 'sql' } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -309,7 +309,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert big number chart block')); } - this.analytics?.trackEvent('add_block', { blockType: 'big-number' }); + this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: 'big-number' } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -365,7 +365,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new WrappedError(l10n.t('Failed to insert chart block')); } - this.analytics?.trackEvent('add_block', { blockType: 'visualization' }); + this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: 'visualization' } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); @@ -414,7 +414,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert input block')); } - this.analytics?.trackEvent('add_block', { blockType }); + this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -549,7 +549,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert text block')); } - this.analytics?.trackEvent('add_block', { blockType: textBlockType }); + this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: textBlockType } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -566,7 +566,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation undefined, ConfigurationTarget.Workspace ); - this.analytics?.trackEvent('toggle_snapshots', { enabled: false }); + 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); @@ -582,7 +582,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation undefined, ConfigurationTarget.Workspace ); - this.analytics?.trackEvent('toggle_snapshots', { enabled: true }); + 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.')); diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index 30f823f4ce..3b2e433ae2 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -436,27 +436,28 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { switch (message.type) { case 'configure': if (message.integrationId) { - this.analytics?.trackEvent('configure_integration'); + this.analytics?.trackEvent({ eventName: 'configure_integration' }); await this.showConfigurationForm(message.integrationId); } break; case 'save': if (message.integrationId && message.config) { - this.analytics?.trackEvent('save_integration', { - integrationType: message.config.type ?? 'unknown' + this.analytics?.trackEvent({ + eventName: 'save_integration', + properties: { integrationType: message.config.type ?? 'unknown' } }); await this.saveConfiguration(message.integrationId, message.config); } break; case 'reset': if (message.integrationId) { - this.analytics?.trackEvent('reset_integration'); + this.analytics?.trackEvent({ eventName: 'reset_integration' }); await this.resetConfiguration(message.integrationId); } break; case 'delete': if (message.integrationId) { - this.analytics?.trackEvent('delete_integration'); + this.analytics?.trackEvent({ eventName: 'delete_integration' }); await this.deleteConfiguration(message.integrationId); } break; diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts index 988266073d..e28e223443 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts @@ -23,7 +23,7 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.OpenInDeepnote, async () => { await this.handleOpenInDeepnote(); - this.analytics?.trackEvent('open_in_deepnote'); + this.analytics?.trackEvent({ eventName: 'open_in_deepnote' }); }) ); } diff --git a/src/notebooks/notebookCommandListener.ts b/src/notebooks/notebookCommandListener.ts index febcea905e..d9bffffad8 100644 --- a/src/notebooks/notebookCommandListener.ts +++ b/src/notebooks/notebookCommandListener.ts @@ -116,7 +116,7 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens private runAllCells() { if (window.activeNotebookEditor) { - this.analytics?.trackEvent('execute_notebook'); + this.analytics?.trackEvent({ eventName: 'execute_notebook' }); commands.executeCommand('notebook.execute').then(noop, noop); } } @@ -144,7 +144,7 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens private addCellBelow() { if (window.activeNotebookEditor) { - this.analytics?.trackEvent('add_block', { blockType: 'code' }); + this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: 'code' } }); commands.executeCommand('notebook.cell.insertCodeCellBelow').then(noop, noop); } } diff --git a/src/platform/analytics/telemetryService.ts b/src/platform/analytics/telemetryService.ts index f3fccf9029..0dbcefdcc2 100644 --- a/src/platform/analytics/telemetryService.ts +++ b/src/platform/analytics/telemetryService.ts @@ -6,9 +6,9 @@ import { IAsyncDisposableRegistry, IPersistentState, IPersistentStateFactory } f import { generateUuid } from '../common/uuid'; import { logger } from '../logging'; import { POSTHOG_API_KEY, POSTHOG_HOST } from './constants'; -import { ITelemetryService } from './types'; +import { ITelemetryService, TelemetryEvent } from './types'; -const USER_ID_STORAGE_KEY = 'posthog-anonymous-user-id'; +const USER_ID_STORAGE_KEY = 'deepnote-telemetry-anonymous-user-id'; @injectable() export class TelemetryService implements ITelemetryService { @@ -25,7 +25,7 @@ export class TelemetryService implements ITelemetryService { asyncDisposables.push(this); } - public trackEvent(eventName: string, properties?: Record): void { + public trackEvent({ eventName, properties }: TelemetryEvent): void { try { if (!this.isTelemetryEnabled()) { return; diff --git a/src/platform/analytics/telemetryService.unit.test.ts b/src/platform/analytics/telemetryService.unit.test.ts index fc63bbaf30..6826759b99 100644 --- a/src/platform/analytics/telemetryService.unit.test.ts +++ b/src/platform/analytics/telemetryService.unit.test.ts @@ -65,7 +65,7 @@ suite('TelemetryService', () => { analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); assert.doesNotThrow(() => { - analyticsService.trackEvent('test_event', { prop: 'value' }); + analyticsService.trackEvent({ eventName: 'open_notebook', properties: { prop: 'value' } }); }); // Should not have initialized (no state access) @@ -92,7 +92,7 @@ suite('TelemetryService', () => { analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); assert.doesNotThrow(() => { - analyticsService.trackEvent('test_event'); + analyticsService.trackEvent({ eventName: 'open_notebook' }); }); assert.isFalse( @@ -109,7 +109,7 @@ suite('TelemetryService', () => { })); analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); - analyticsService.trackEvent('test_event'); + analyticsService.trackEvent({ eventName: 'open_notebook' }); assert.isTrue( (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).calledOnce, @@ -137,7 +137,7 @@ suite('TelemetryService', () => { (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).returns(mockUserIdState); analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); - analyticsService.trackEvent('test_event'); + analyticsService.trackEvent({ eventName: 'open_notebook' }); assert.isFalse( (mockUserIdState.updateValue as sinon.SinonStub).called, diff --git a/src/platform/analytics/types.ts b/src/platform/analytics/types.ts index c0276475b8..97b8b8b04f 100644 --- a/src/platform/analytics/types.ts +++ b/src/platform/analytics/types.ts @@ -1,7 +1,34 @@ import { IAsyncDisposable } from '../common/types'; +export type TelemetryEventName = + | 'add_block' + | 'configure_integration' + | 'create_environment' + | 'create_notebook' + | 'create_project' + | 'delete_environment' + | 'delete_integration' + | 'delete_notebook' + | 'delete_project' + | 'duplicate_notebook' + | 'execute_cell' + | 'execute_notebook' + | 'export_notebook' + | 'import_notebook' + | 'open_in_deepnote' + | 'open_notebook' + | 'reset_integration' + | 'save_integration' + | 'select_environment' + | 'toggle_snapshots'; + +export interface TelemetryEvent { + eventName: TelemetryEventName; + properties?: Record; +} + export const ITelemetryService = Symbol('ITelemetryService'); export interface ITelemetryService extends IAsyncDisposable { - trackEvent(eventName: string, properties?: Record): void; + trackEvent(event: TelemetryEvent): void; } From 2cd604dfd87cee60e06bc961a9af57a906431db1 Mon Sep 17 00:00:00 2001 From: tomas Date: Mon, 30 Mar 2026 08:15:21 +0000 Subject: [PATCH 05/17] refactor(telemetry): make ITelemetryService non-optional with web no-op Add TelemetryWebService as a no-op implementation registered in the web service registry, allowing ITelemetryService to be injected as required everywhere instead of optional. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../deepnoteEnvironmentsView.node.ts | 10 +++---- .../deepnoteEnvironmentsView.unit.test.ts | 3 +- .../deepnote/deepnoteActivationService.ts | 4 +-- .../deepnoteActivationService.unit.test.ts | 30 +++++++++++++++---- .../deepnoteCellExecutionAnalytics.ts | 10 ++----- .../deepnote/deepnoteExplorerView.ts | 22 +++++++------- .../deepnoteExplorerView.unit.test.ts | 11 ++++--- .../deepnoteNotebookCommandListener.ts | 18 +++++------ ...epnoteNotebookCommandListener.unit.test.ts | 7 +++-- .../integrations/integrationWebview.ts | 12 ++++---- .../deepnote/openInDeepnoteHandler.node.ts | 6 ++-- .../openInDeepnoteHandler.node.unit.test.ts | 6 +++- src/notebooks/notebookCommandListener.ts | 8 ++--- src/platform/analytics/telemetryWebService.ts | 14 +++++++++ src/platform/serviceRegistry.web.ts | 3 ++ 15 files changed, 104 insertions(+), 60 deletions(-) create mode 100644 src/platform/analytics/telemetryWebService.ts diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts index bf6abef5d9..1e30d67465 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts @@ -1,4 +1,4 @@ -import { inject, injectable, named, optional } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import { commands, Disposable, @@ -54,7 +54,7 @@ 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(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined + @inject(ITelemetryService) private readonly analytics: ITelemetryService ) { // Create tree data provider @@ -195,7 +195,7 @@ export class DeepnoteEnvironmentsView implements Disposable { const config = await this.environmentManager.createEnvironment(options, token); logger.info(`Created environment: ${config.id} (${config.name})`); - this.analytics?.trackEvent({ + this.analytics.trackEvent({ eventName: 'create_environment', properties: { hasDescription: !!options.description, @@ -324,7 +324,7 @@ export class DeepnoteEnvironmentsView implements Disposable { } ); - this.analytics?.trackEvent({ eventName: 'delete_environment' }); + 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,7 +494,7 @@ export class DeepnoteEnvironmentsView implements Disposable { } ); - this.analytics?.trackEvent({ eventName: 'select_environment' }); + this.analytics.trackEvent({ eventName: 'select_environment' }); void window.showInformationMessage(l10n.t('Environment switched successfully')); } catch (error) { if (error instanceof DeepnoteToolkitMissingError) { diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts index d7d761a6d4..75e44f5c49 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts @@ -5,6 +5,7 @@ import { CancellationToken, Disposable, NotebookDocument, ProgressOptions, Uri } import { DeepnoteEnvironmentsView } from './deepnoteEnvironmentsView.node'; import { IDeepnoteEnvironmentManager, IDeepnoteKernelAutoSelector, IDeepnoteNotebookEnvironmentMapper } 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'; @@ -62,7 +63,7 @@ suite('DeepnoteEnvironmentsView', () => { instance(mockNotebookEnvironmentMapper), instance(mockKernelProvider), instance(mockOutputChannel), - undefined + { trackEvent: sinon.stub(), dispose: sinon.stub().resolves() } as unknown as ITelemetryService ); }); diff --git a/src/notebooks/deepnote/deepnoteActivationService.ts b/src/notebooks/deepnote/deepnoteActivationService.ts index 0f3696a8df..cd9e571a21 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.ts @@ -35,8 +35,8 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, @inject(IIntegrationManager) integrationManager: IIntegrationManager, @inject(ILogger) private readonly logger: ILogger, - @inject(SnapshotService) @optional() private readonly snapshotService?: SnapshotService, - @inject(ITelemetryService) @optional() private readonly analytics?: ITelemetryService + @inject(ITelemetryService) private readonly analytics: ITelemetryService, + @inject(SnapshotService) @optional() private readonly snapshotService?: SnapshotService ) { this.integrationManager = integrationManager; } diff --git a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts index a8068d5f64..5354719676 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 { ITelemetryService } from '../../platform/analytics/types'; import { DeepnoteActivationService } from './deepnoteActivationService'; import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; import { IExtensionContext } from '../../platform/common/types'; @@ -25,6 +26,10 @@ suite('DeepnoteActivationService', () => { let manager: DeepnoteNotebookManager; let mockIntegrationManager: IIntegrationManager; let mockLogger: ILogger; + const mockAnalytics = { + trackEvent: () => undefined, + dispose: async () => undefined + } as unknown as ITelemetryService; setup(() => { mockExtensionContext = { @@ -42,7 +47,8 @@ suite('DeepnoteActivationService', () => { mockExtensionContext, manager, mockIntegrationManager, - mockLogger + mockLogger, + mockAnalytics ); }); @@ -103,6 +109,7 @@ suite('DeepnoteActivationService', () => { manager, mockIntegrationManager, mockLogger, + mockAnalytics, mockSnapshotService ); @@ -150,6 +157,7 @@ suite('DeepnoteActivationService', () => { manager, mockIntegrationManager, mockLogger, + mockAnalytics, mockSnapshotService ); @@ -206,8 +214,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 +264,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 index 14f66b87a5..c2246b6369 100644 --- a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts +++ b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts @@ -1,4 +1,4 @@ -import { inject, injectable, optional } from 'inversify'; +import { inject, injectable } from 'inversify'; import { Disposable } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; @@ -13,16 +13,12 @@ import { IDeepnoteNotebookManager } from '../types'; @injectable() export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationService { constructor( - @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined, + @inject(ITelemetryService) private readonly analytics: ITelemetryService, @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, @inject(IDisposableRegistry) private readonly disposables: Disposable[] ) {} public activate(): void { - if (!this.analytics) { - return; - } - this.disposables.push( notebookCellExecutions.onDidChangeNotebookCellExecutionState((e) => { if (e.state !== NotebookCellExecutionState.Executing) { @@ -56,7 +52,7 @@ export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationS } } - this.analytics?.trackEvent({ eventName: 'execute_cell', properties }); + this.analytics.trackEvent({ eventName: 'execute_cell', properties }); }) ); } diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index 7e0435d112..cd67c42799 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -27,7 +27,7 @@ export class DeepnoteExplorerView { @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, @inject(IDeepnoteNotebookManager) private readonly manager: IDeepnoteNotebookManager, @inject(ILogger) logger: ILogger, - private readonly analytics?: ITelemetryService + private readonly analytics: ITelemetryService ) { this.treeDataProvider = new DeepnoteTreeDataProvider(logger); } @@ -336,7 +336,7 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.OpenDeepnoteNotebook, async (context: DeepnoteTreeItemContext) => { await this.openNotebook(context); - this.analytics?.trackEvent({ eventName: 'open_notebook' }); + this.analytics.trackEvent({ eventName: 'open_notebook' }); }) ); @@ -351,28 +351,28 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.NewProject, async () => { await this.newProject(); - this.analytics?.trackEvent({ eventName: 'create_project' }); + this.analytics.trackEvent({ eventName: 'create_project' }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ImportNotebook, async () => { await this.importNotebook(); - this.analytics?.trackEvent({ eventName: 'import_notebook' }); + this.analytics.trackEvent({ eventName: 'import_notebook' }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ImportJupyterNotebook, async () => { await this.importJupyterNotebook(); - this.analytics?.trackEvent({ eventName: 'import_notebook' }); + this.analytics.trackEvent({ eventName: 'import_notebook' }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.NewNotebook, async () => { await this.newNotebook(); - this.analytics?.trackEvent({ eventName: 'create_notebook' }); + this.analytics.trackEvent({ eventName: 'create_notebook' }); }) ); @@ -386,7 +386,7 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DeleteProject, async (treeItem: DeepnoteTreeItem) => { await this.deleteProject(treeItem); - this.analytics?.trackEvent({ eventName: 'delete_project' }); + this.analytics.trackEvent({ eventName: 'delete_project' }); }) ); @@ -399,14 +399,14 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DeleteNotebook, async (treeItem: DeepnoteTreeItem) => { await this.deleteNotebook(treeItem); - this.analytics?.trackEvent({ eventName: 'delete_notebook' }); + this.analytics.trackEvent({ eventName: 'delete_notebook' }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DuplicateNotebook, async (treeItem: DeepnoteTreeItem) => { await this.duplicateNotebook(treeItem); - this.analytics?.trackEvent({ eventName: 'duplicate_notebook' }); + this.analytics.trackEvent({ eventName: 'duplicate_notebook' }); }) ); @@ -419,14 +419,14 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ExportProject, async (treeItem: DeepnoteTreeItem) => { await this.exportProject(treeItem); - this.analytics?.trackEvent({ eventName: 'export_notebook', properties: { format: 'project' } }); + this.analytics.trackEvent({ eventName: 'export_notebook', properties: { format: 'project' } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ExportNotebook, async (treeItem: DeepnoteTreeItem) => { await this.exportNotebook(treeItem); - this.analytics?.trackEvent({ eventName: 'export_notebook', properties: { format: 'notebook' } }); + this.analytics.trackEvent({ eventName: 'export_notebook', properties: { format: 'notebook' } }); }) ); } diff --git a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts index 5e5e2de826..c1bdc638b7 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts @@ -8,12 +8,15 @@ import { stringify as yamlStringify } from 'yaml'; import { DeepnoteExplorerView } from './deepnoteExplorerView'; import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; import { DeepnoteTreeItem, DeepnoteTreeItemType, type DeepnoteTreeItemContext } from './deepnoteTreeItem'; +import { ITelemetryService } from '../../platform/analytics/types'; import type { IExtensionContext } from '../../platform/common/types'; import type { DeepnoteNotebook } from '../../platform/deepnote/deepnoteTypes'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; import { ILogger } from '../../platform/logging/types'; import * as uuidModule from '../../platform/common/uuid'; +const mockAnalytics = { trackEvent: () => undefined, dispose: async () => undefined } as unknown as ITelemetryService; + function createMockLogger(): ILogger { return { error: () => undefined, @@ -52,7 +55,7 @@ suite('DeepnoteExplorerView', () => { manager = new DeepnoteNotebookManager(); mockLogger = createMockLogger(); - explorerView = new DeepnoteExplorerView(mockExtensionContext, manager, mockLogger); + explorerView = new DeepnoteExplorerView(mockExtensionContext, manager, mockLogger, mockAnalytics); }); suite('constructor', () => { @@ -190,8 +193,8 @@ suite('DeepnoteExplorerView', () => { const manager2 = new DeepnoteNotebookManager(); const logger1 = createMockLogger(); const logger2 = createMockLogger(); - const view1 = new DeepnoteExplorerView(context1, manager1, logger1); - const view2 = new DeepnoteExplorerView(context2, manager2, logger2); + const view1 = new DeepnoteExplorerView(context1, manager1, logger1, mockAnalytics); + const view2 = new DeepnoteExplorerView(context2, manager2, logger2, mockAnalytics); // Verify each view has its own context assert.strictEqual((view1 as any).extensionContext, context1); @@ -234,7 +237,7 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { mockManager = new DeepnoteNotebookManager(); const mockLogger = createMockLogger(); - explorerView = new DeepnoteExplorerView(mockContext, mockManager, mockLogger); + explorerView = new DeepnoteExplorerView(mockContext, mockManager, mockLogger, mockAnalytics); }); teardown(() => { diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts index acec3062d3..c410bd9a83 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts @@ -1,4 +1,4 @@ -import { injectable, inject, optional } from 'inversify'; +import { injectable, inject } from 'inversify'; import { commands, ConfigurationTarget, @@ -152,7 +152,7 @@ export function getNextDeepnoteVariableName(cells: NotebookCell[], prefix: 'df' @injectable() export class DeepnoteNotebookCommandListener implements IExtensionSyncActivationService { constructor( - @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined, + @inject(ITelemetryService) private readonly analytics: ITelemetryService, @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry ) {} @@ -266,7 +266,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert SQL block')); } - this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: 'sql' } }); + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'sql' } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -309,7 +309,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert big number chart block')); } - this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: 'big-number' } }); + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'big-number' } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -365,7 +365,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new WrappedError(l10n.t('Failed to insert chart block')); } - this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: 'visualization' } }); + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'visualization' } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); @@ -414,7 +414,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert input block')); } - this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType } }); + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -549,7 +549,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert text block')); } - this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: textBlockType } }); + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: textBlockType } }); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -566,7 +566,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation undefined, ConfigurationTarget.Workspace ); - this.analytics?.trackEvent({ eventName: 'toggle_snapshots', properties: { enabled: false } }); + 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); @@ -582,7 +582,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation undefined, ConfigurationTarget.Workspace ); - this.analytics?.trackEvent({ eventName: 'toggle_snapshots', properties: { enabled: true } }); + 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.')); diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts index 1683cfffba..f48a19be14 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts @@ -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 mockAnalytics: ITelemetryService; function createMockConfigService(): IConfigurationService { return { @@ -44,7 +46,8 @@ suite('DeepnoteNotebookCommandListener', () => { sandbox = sinon.createSandbox(); disposables = []; mockConfigService = createMockConfigService(); - commandListener = new DeepnoteNotebookCommandListener(undefined, mockConfigService, disposables); + mockAnalytics = { trackEvent: sinon.stub(), dispose: sinon.stub().resolves() } as unknown as ITelemetryService; + commandListener = new DeepnoteNotebookCommandListener(mockAnalytics, mockConfigService, disposables); }); teardown(() => { @@ -90,7 +93,7 @@ suite('DeepnoteNotebookCommandListener', () => { // Create new instance and activate again const disposables2: IDisposable[] = []; const commandListener2 = new DeepnoteNotebookCommandListener( - undefined, + mockAnalytics, createMockConfigService(), disposables2 ); diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index 3b2e433ae2..1fc5c9c184 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -1,4 +1,4 @@ -import { inject, injectable, optional } from 'inversify'; +import { inject, injectable } from 'inversify'; import { Disposable, l10n, Uri, ViewColumn, WebviewPanel, window } from 'vscode'; import { ITelemetryService } from '../../../platform/analytics/types'; @@ -31,7 +31,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) @optional() private readonly analytics: ITelemetryService | undefined + @inject(ITelemetryService) private readonly analytics: ITelemetryService ) {} /** @@ -436,13 +436,13 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { switch (message.type) { case 'configure': if (message.integrationId) { - this.analytics?.trackEvent({ eventName: 'configure_integration' }); + this.analytics.trackEvent({ eventName: 'configure_integration' }); await this.showConfigurationForm(message.integrationId); } break; case 'save': if (message.integrationId && message.config) { - this.analytics?.trackEvent({ + this.analytics.trackEvent({ eventName: 'save_integration', properties: { integrationType: message.config.type ?? 'unknown' } }); @@ -451,13 +451,13 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { break; case 'reset': if (message.integrationId) { - this.analytics?.trackEvent({ eventName: 'reset_integration' }); + this.analytics.trackEvent({ eventName: 'reset_integration' }); await this.resetConfiguration(message.integrationId); } break; case 'delete': if (message.integrationId) { - this.analytics?.trackEvent({ eventName: 'delete_integration' }); + this.analytics.trackEvent({ eventName: 'delete_integration' }); await this.deleteConfiguration(message.integrationId); } break; diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts index e28e223443..4251bb865c 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { injectable, inject, optional } from 'inversify'; +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'; @@ -16,14 +16,14 @@ import { initImport, uploadFile, getErrorMessage, MAX_FILE_SIZE, getDeepnoteDoma export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { constructor( @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, - @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined + @inject(ITelemetryService) private readonly analytics: ITelemetryService ) {} public activate(): void { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.OpenInDeepnote, async () => { await this.handleOpenInDeepnote(); - this.analytics?.trackEvent({ eventName: 'open_in_deepnote' }); + this.analytics.trackEvent({ eventName: 'open_in_deepnote' }); }) ); } diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts index bed3c1ff56..0ef99453a1 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts @@ -6,6 +6,7 @@ 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'; @@ -49,7 +50,10 @@ suite('OpenInDeepnoteHandler', () => { subscriptions: [] } as any; - handler = new OpenInDeepnoteHandlerClass(mockExtensionContext, undefined); + handler = new OpenInDeepnoteHandlerClass(mockExtensionContext, { + trackEvent: sinon.stub(), + dispose: sinon.stub().resolves() + } as unknown as ITelemetryService); }); teardown(() => { diff --git a/src/notebooks/notebookCommandListener.ts b/src/notebooks/notebookCommandListener.ts index d9bffffad8..d9bb97e37e 100644 --- a/src/notebooks/notebookCommandListener.ts +++ b/src/notebooks/notebookCommandListener.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { inject, injectable, optional } from 'inversify'; +import { inject, injectable } from 'inversify'; import { ConfigurationTarget, @@ -56,7 +56,7 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens @inject(INotebookEditorProvider) private notebookEditorProvider: INotebookEditorProvider, @inject(IServiceContainer) private serviceContainer: IServiceContainer, @inject(IKernelStatusProvider) private kernelStatusProvider: IKernelStatusProvider, - @inject(ITelemetryService) @optional() private readonly analytics: ITelemetryService | undefined + @inject(ITelemetryService) private readonly analytics: ITelemetryService ) {} activate(): void { @@ -116,7 +116,7 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens private runAllCells() { if (window.activeNotebookEditor) { - this.analytics?.trackEvent({ eventName: 'execute_notebook' }); + this.analytics.trackEvent({ eventName: 'execute_notebook' }); commands.executeCommand('notebook.execute').then(noop, noop); } } @@ -144,7 +144,7 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens private addCellBelow() { if (window.activeNotebookEditor) { - this.analytics?.trackEvent({ eventName: 'add_block', properties: { blockType: 'code' } }); + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'code' } }); commands.executeCommand('notebook.cell.insertCodeCellBelow').then(noop, noop); } } diff --git a/src/platform/analytics/telemetryWebService.ts b/src/platform/analytics/telemetryWebService.ts new file mode 100644 index 0000000000..9dd7b611d9 --- /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 trackEvent(_event: TelemetryEvent): void { + // No-op for web + } + + public async dispose(): Promise { + // No-op for web + } +} 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); From 8ca48b3657353b403f7b87a075d5db08dc0690bc Mon Sep 17 00:00:00 2001 From: tomas Date: Mon, 30 Mar 2026 15:25:19 +0000 Subject: [PATCH 06/17] refactor(telemetry): replace ITelemetryService with NoOpTelemetryService in tests --- .../deepnoteEnvironmentsView.unit.test.ts | 4 +- .../deepnoteActivationService.unit.test.ts | 7 +- .../deepnote/deepnoteExplorerView.ts | 128 +++++++++++------- .../deepnoteExplorerView.unit.test.ts | 4 +- ...epnoteNotebookCommandListener.unit.test.ts | 12 +- .../openInDeepnoteHandler.node.unit.test.ts | 7 +- .../analytics/noOpTelemetryService.ts | 14 ++ 7 files changed, 110 insertions(+), 66 deletions(-) create mode 100644 src/platform/analytics/noOpTelemetryService.ts diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts index 75e44f5c49..cdb608e336 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts @@ -5,7 +5,7 @@ import { CancellationToken, Disposable, NotebookDocument, ProgressOptions, Uri } import { DeepnoteEnvironmentsView } from './deepnoteEnvironmentsView.node'; import { IDeepnoteEnvironmentManager, IDeepnoteKernelAutoSelector, IDeepnoteNotebookEnvironmentMapper } from '../types'; import { IPythonApiProvider } from '../../../platform/api/types'; -import { ITelemetryService } from '../../../platform/analytics/types'; +import { NoOpTelemetryService } from '../../../platform/analytics/noOpTelemetryService'; import { IDisposableRegistry, IOutputChannel } from '../../../platform/common/types'; import { IKernelProvider } from '../../../kernels/types'; import { DeepnoteEnvironment } from './deepnoteEnvironment'; @@ -63,7 +63,7 @@ suite('DeepnoteEnvironmentsView', () => { instance(mockNotebookEnvironmentMapper), instance(mockKernelProvider), instance(mockOutputChannel), - { trackEvent: sinon.stub(), dispose: sinon.stub().resolves() } as unknown as ITelemetryService + new NoOpTelemetryService() ); }); diff --git a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts index 5354719676..cdfe5d52b2 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts @@ -1,7 +1,7 @@ import { assert } from 'chai'; import { anything, verify, when } from 'ts-mockito'; -import { ITelemetryService } from '../../platform/analytics/types'; +import { NoOpTelemetryService } from '../../platform/analytics/noOpTelemetryService'; import { DeepnoteActivationService } from './deepnoteActivationService'; import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; import { IExtensionContext } from '../../platform/common/types'; @@ -26,10 +26,7 @@ suite('DeepnoteActivationService', () => { let manager: DeepnoteNotebookManager; let mockIntegrationManager: IIntegrationManager; let mockLogger: ILogger; - const mockAnalytics = { - trackEvent: () => undefined, - dispose: async () => undefined - } as unknown as ITelemetryService; + const mockAnalytics = new NoOpTelemetryService(); setup(() => { mockExtensionContext = { diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index cd67c42799..c34801ac70 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -144,9 +144,9 @@ export class DeepnoteExplorerView { } } - public async deleteNotebook(treeItem: DeepnoteTreeItem): Promise { + public async deleteNotebook(treeItem: DeepnoteTreeItem): Promise { if (treeItem.type !== DeepnoteTreeItemType.Notebook) { - return; + return false; } const notebook = treeItem.data as DeepnoteNotebook; @@ -159,7 +159,7 @@ export class DeepnoteExplorerView { ); if (confirmation !== l10n.t('Delete')) { - return; + return false; } try { @@ -168,7 +168,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return false; } projectData.project.notebooks = projectData.project.notebooks.filter( @@ -186,9 +186,13 @@ export class DeepnoteExplorerView { await this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook deleted: {0}', notebookName)); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to delete notebook: {0}', errorMessage)); + + return false; } } @@ -350,22 +354,22 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.NewProject, async () => { - await this.newProject(); - this.analytics.trackEvent({ eventName: 'create_project' }); + const completed = await this.newProject(); + this.analytics.trackEvent({ eventName: 'create_project', properties: { completed } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ImportNotebook, async () => { - await this.importNotebook(); - this.analytics.trackEvent({ eventName: 'import_notebook' }); + const completed = await this.importNotebook(); + this.analytics.trackEvent({ eventName: 'import_notebook', properties: { completed } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ImportJupyterNotebook, async () => { - await this.importJupyterNotebook(); - this.analytics.trackEvent({ eventName: 'import_notebook' }); + const completed = await this.importJupyterNotebook(); + this.analytics.trackEvent({ eventName: 'import_notebook', properties: { completed } }); }) ); @@ -385,8 +389,8 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DeleteProject, async (treeItem: DeepnoteTreeItem) => { - await this.deleteProject(treeItem); - this.analytics.trackEvent({ eventName: 'delete_project' }); + const completed = await this.deleteProject(treeItem); + this.analytics.trackEvent({ eventName: 'delete_project', properties: { completed } }); }) ); @@ -398,8 +402,8 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DeleteNotebook, async (treeItem: DeepnoteTreeItem) => { - await this.deleteNotebook(treeItem); - this.analytics.trackEvent({ eventName: 'delete_notebook' }); + const completed = await this.deleteNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'delete_notebook', properties: { completed } }); }) ); @@ -418,15 +422,21 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ExportProject, async (treeItem: DeepnoteTreeItem) => { - await this.exportProject(treeItem); - this.analytics.trackEvent({ eventName: 'export_notebook', properties: { format: 'project' } }); + const completed = await this.exportProject(treeItem); + this.analytics.trackEvent({ + eventName: 'export_notebook', + properties: { completed, format: 'project' } + }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ExportNotebook, async (treeItem: DeepnoteTreeItem) => { - await this.exportNotebook(treeItem); - this.analytics.trackEvent({ eventName: 'export_notebook', properties: { format: 'notebook' } }); + const completed = await this.exportNotebook(treeItem); + this.analytics.trackEvent({ + eventName: 'export_notebook', + properties: { completed, format: 'notebook' } + }); }) ); } @@ -633,7 +643,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?'), @@ -645,7 +655,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return; + return false; } const projectName = await window.showInputBox({ @@ -661,7 +671,7 @@ export class DeepnoteExplorerView { }); if (!projectName) { - return; + return false; } try { @@ -673,7 +683,7 @@ export class DeepnoteExplorerView { try { await workspace.fs.stat(fileUri); await window.showErrorMessage(l10n.t('A file named "{0}" already exists in this workspace.', fileName)); - return; + return false; } catch { // File doesn't exist, continue } @@ -730,10 +740,14 @@ export class DeepnoteExplorerView { preserveFocus: false, preview: false }); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t(`Failed to create project: {0}`, errorMessage)); + + return false; } } @@ -765,7 +779,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?'), @@ -777,7 +791,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return; + return false; } const fileUris = await window.showOpenDialog({ @@ -791,7 +805,7 @@ export class DeepnoteExplorerView { }); if (!fileUris || fileUris.length === 0) { - return; + return false; } try { @@ -810,7 +824,7 @@ export class DeepnoteExplorerView { await window.showErrorMessage( l10n.t('A file named "{0}" already exists in this workspace.', fileName) ); - return; + return false; } catch { // File doesn't exist, continue } @@ -828,7 +842,7 @@ export class DeepnoteExplorerView { await window.showErrorMessage( l10n.t('A file named "{0}" already exists in this workspace.', outputFileName) ); - return; + return false; } catch { // File doesn't exist, continue } @@ -869,14 +883,18 @@ export class DeepnoteExplorerView { } this.treeDataProvider.refresh(); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(`Failed to import notebook: ${errorMessage}`); + + return false; } } - 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?'), @@ -888,7 +906,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return; + return false; } const fileUris = await window.showOpenDialog({ @@ -902,7 +920,7 @@ export class DeepnoteExplorerView { }); if (!fileUris || fileUris.length === 0) { - return; + return false; } try { @@ -921,7 +939,7 @@ export class DeepnoteExplorerView { await window.showErrorMessage( l10n.t('A file named "{0}" already exists in this workspace.', outputFileName) ); - return; + return false; } catch { // File doesn't exist, continue } @@ -942,16 +960,20 @@ export class DeepnoteExplorerView { } this.treeDataProvider.refresh(); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t(`Failed to import Jupyter notebook: {0}`, errorMessage)); + + return false; } } - private async deleteProject(treeItem: DeepnoteTreeItem): Promise { + private async deleteProject(treeItem: DeepnoteTreeItem): Promise { if (treeItem.type !== DeepnoteTreeItemType.ProjectFile) { - return; + return false; } const project = treeItem.data as DeepnoteFile; @@ -964,7 +986,7 @@ export class DeepnoteExplorerView { ); if (confirmation !== l10n.t('Delete')) { - return; + return false; } try { @@ -972,9 +994,13 @@ export class DeepnoteExplorerView { await workspace.fs.delete(fileUri); this.treeDataProvider.refresh(); await window.showInformationMessage(l10n.t('Project deleted: {0}', projectName)); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to delete project: {0}', errorMessage)); + + return false; } } @@ -1002,9 +1028,9 @@ export class DeepnoteExplorerView { * Exports all notebooks from a Deepnote project to Jupyter format * @param treeItem The tree item representing a project */ - private async exportProject(treeItem: DeepnoteTreeItem): Promise { + private async exportProject(treeItem: DeepnoteTreeItem): Promise { if (treeItem.type !== DeepnoteTreeItemType.ProjectFile) { - return; + return false; } try { @@ -1013,7 +1039,7 @@ export class DeepnoteExplorerView { }); if (!format) { - return; + return false; } const fileUri = Uri.file(treeItem.context.filePath); @@ -1022,7 +1048,7 @@ export class DeepnoteExplorerView { if (!projectData?.project) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return false; } const outputFolder = await window.showOpenDialog({ @@ -1034,7 +1060,7 @@ export class DeepnoteExplorerView { }); if (!outputFolder?.length) { - return; + return false; } const jupyterNotebooks = convertDeepnoteToJupyterNotebooks(projectData); @@ -1061,7 +1087,7 @@ export class DeepnoteExplorerView { ); if (result !== overwrite) { - return; + return false; } } @@ -1078,9 +1104,13 @@ export class DeepnoteExplorerView { : l10n.t('Exported {0} notebooks successfully', count); await window.showInformationMessage(message); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to export: {0}', errorMessage)); + + return false; } } @@ -1088,9 +1118,9 @@ export class DeepnoteExplorerView { * Exports a single notebook from a Deepnote project to Jupyter format * @param treeItem The tree item representing a notebook */ - private async exportNotebook(treeItem: DeepnoteTreeItem): Promise { + private async exportNotebook(treeItem: DeepnoteTreeItem): Promise { if (treeItem.type !== DeepnoteTreeItemType.Notebook) { - return; + return false; } try { @@ -1099,7 +1129,7 @@ export class DeepnoteExplorerView { }); if (!format) { - return; + return false; } const fileUri = Uri.file(treeItem.context.filePath); @@ -1108,7 +1138,7 @@ export class DeepnoteExplorerView { if (!projectData?.project) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return false; } const outputFolder = await window.showOpenDialog({ @@ -1120,7 +1150,7 @@ export class DeepnoteExplorerView { }); if (!outputFolder?.length) { - return; + return false; } const targetNotebook = projectData.project.notebooks.find((nb) => nb.id === treeItem.context.notebookId); @@ -1128,7 +1158,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return false; } const filteredProject = { @@ -1162,7 +1192,7 @@ export class DeepnoteExplorerView { ); if (result !== overwrite) { - return; + return false; } } @@ -1172,9 +1202,13 @@ export class DeepnoteExplorerView { ); await window.showInformationMessage(l10n.t('Exported 1 notebook successfully')); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to export: {0}', errorMessage)); + + return false; } } } diff --git a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts index c1bdc638b7..e3fd349333 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts @@ -8,14 +8,14 @@ import { stringify as yamlStringify } from 'yaml'; import { DeepnoteExplorerView } from './deepnoteExplorerView'; import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; import { DeepnoteTreeItem, DeepnoteTreeItemType, type DeepnoteTreeItemContext } from './deepnoteTreeItem'; -import { ITelemetryService } from '../../platform/analytics/types'; +import { NoOpTelemetryService } from '../../platform/analytics/noOpTelemetryService'; import type { IExtensionContext } from '../../platform/common/types'; import type { DeepnoteNotebook } from '../../platform/deepnote/deepnoteTypes'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; import { ILogger } from '../../platform/logging/types'; import * as uuidModule from '../../platform/common/uuid'; -const mockAnalytics = { trackEvent: () => undefined, dispose: async () => undefined } as unknown as ITelemetryService; +const mockAnalytics = new NoOpTelemetryService(); function createMockLogger(): ILogger { return { diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts index f48a19be14..2bb310277c 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts @@ -18,7 +18,7 @@ import { InputBlockType } from './deepnoteNotebookCommandListener'; import { formatInputBlockCellContent, getInputBlockLanguage } from './inputBlockContentFormatter'; -import { ITelemetryService } from '../../platform/analytics/types'; +import { NoOpTelemetryService } from '../../platform/analytics/noOpTelemetryService'; import { IConfigurationService, IDisposable } from '../../platform/common/types'; import * as notebookUpdater from '../../kernels/execution/notebookUpdater'; import { createMockedNotebookDocument } from '../../test/datascience/editor-integration/helpers'; @@ -32,7 +32,6 @@ suite('DeepnoteNotebookCommandListener', () => { let disposables: IDisposable[]; let sandbox: sinon.SinonSandbox; let mockConfigService: IConfigurationService; - let mockAnalytics: ITelemetryService; function createMockConfigService(): IConfigurationService { return { @@ -46,8 +45,11 @@ suite('DeepnoteNotebookCommandListener', () => { sandbox = sinon.createSandbox(); disposables = []; mockConfigService = createMockConfigService(); - mockAnalytics = { trackEvent: sinon.stub(), dispose: sinon.stub().resolves() } as unknown as ITelemetryService; - commandListener = new DeepnoteNotebookCommandListener(mockAnalytics, mockConfigService, disposables); + commandListener = new DeepnoteNotebookCommandListener( + new NoOpTelemetryService(), + mockConfigService, + disposables + ); }); teardown(() => { @@ -93,7 +95,7 @@ suite('DeepnoteNotebookCommandListener', () => { // Create new instance and activate again const disposables2: IDisposable[] = []; const commandListener2 = new DeepnoteNotebookCommandListener( - mockAnalytics, + new NoOpTelemetryService(), createMockConfigService(), disposables2 ); diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts index 0ef99453a1..bd71fac431 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts @@ -6,7 +6,7 @@ import * as fs from 'fs'; import esmock from 'esmock'; import type { OpenInDeepnoteHandler } from './openInDeepnoteHandler.node'; -import { ITelemetryService } from '../../platform/analytics/types'; +import { NoOpTelemetryService } from '../../platform/analytics/noOpTelemetryService'; import { IExtensionContext } from '../../platform/common/types'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; import { MAX_FILE_SIZE } from './importClient.node'; @@ -50,10 +50,7 @@ suite('OpenInDeepnoteHandler', () => { subscriptions: [] } as any; - handler = new OpenInDeepnoteHandlerClass(mockExtensionContext, { - trackEvent: sinon.stub(), - dispose: sinon.stub().resolves() - } as unknown as ITelemetryService); + handler = new OpenInDeepnoteHandlerClass(mockExtensionContext, new NoOpTelemetryService()); }); teardown(() => { diff --git a/src/platform/analytics/noOpTelemetryService.ts b/src/platform/analytics/noOpTelemetryService.ts new file mode 100644 index 0000000000..091994fb07 --- /dev/null +++ b/src/platform/analytics/noOpTelemetryService.ts @@ -0,0 +1,14 @@ +import { ITelemetryService, TelemetryEvent } from './types'; + +/** + * No-op telemetry service for use in tests. + */ +export class NoOpTelemetryService implements ITelemetryService { + public trackEvent(_event: TelemetryEvent): void { + // No-op + } + + public async dispose(): Promise { + // No-op + } +} From 282cad6c4624012de2d67f0df5f9160c4bc45178 Mon Sep 17 00:00:00 2001 From: tomas Date: Mon, 30 Mar 2026 20:37:05 +0000 Subject: [PATCH 07/17] Update tests --- .../environments/deepnoteEnvironmentsView.unit.test.ts | 6 ++++-- .../deepnote/deepnoteActivationService.unit.test.ts | 6 +++--- .../deepnote/deepnoteExplorerView.unit.test.ts | 4 ++-- .../deepnoteNotebookCommandListener.unit.test.ts | 10 ++++++---- .../deepnote/openInDeepnoteHandler.node.unit.test.ts | 4 ++-- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts index cdb608e336..5b76a0dc2c 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts @@ -5,7 +5,7 @@ import { CancellationToken, Disposable, NotebookDocument, ProgressOptions, Uri } import { DeepnoteEnvironmentsView } from './deepnoteEnvironmentsView.node'; import { IDeepnoteEnvironmentManager, IDeepnoteKernelAutoSelector, IDeepnoteNotebookEnvironmentMapper } from '../types'; import { IPythonApiProvider } from '../../../platform/api/types'; -import { NoOpTelemetryService } from '../../../platform/analytics/noOpTelemetryService'; +import { ITelemetryService } from '../../../platform/analytics/types'; import { IDisposableRegistry, IOutputChannel } from '../../../platform/common/types'; import { IKernelProvider } from '../../../kernels/types'; import { DeepnoteEnvironment } from './deepnoteEnvironment'; @@ -26,6 +26,7 @@ suite('DeepnoteEnvironmentsView', () => { let mockNotebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper; let mockKernelProvider: IKernelProvider; let mockOutputChannel: IOutputChannel; + let mockTelemetryService: ITelemetryService; let disposables: Disposable[] = []; let pythonEnvironments: PythonExtension['environments']; @@ -44,6 +45,7 @@ suite('DeepnoteEnvironmentsView', () => { mockNotebookEnvironmentMapper = mock(); mockKernelProvider = mock(); mockOutputChannel = mock(); + mockTelemetryService = mock(); // Mock onDidChangeEnvironments to return a disposable event when(mockConfigManager.onDidChangeEnvironments).thenReturn((_listener: () => void) => { @@ -63,7 +65,7 @@ suite('DeepnoteEnvironmentsView', () => { instance(mockNotebookEnvironmentMapper), instance(mockKernelProvider), instance(mockOutputChannel), - new NoOpTelemetryService() + instance(mockTelemetryService) ); }); diff --git a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts index cdfe5d52b2..d3e16a88a8 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts @@ -1,7 +1,7 @@ import { assert } from 'chai'; -import { anything, verify, when } from 'ts-mockito'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; -import { NoOpTelemetryService } from '../../platform/analytics/noOpTelemetryService'; +import { ITelemetryService } from '../../platform/analytics/types'; import { DeepnoteActivationService } from './deepnoteActivationService'; import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; import { IExtensionContext } from '../../platform/common/types'; @@ -26,7 +26,7 @@ suite('DeepnoteActivationService', () => { let manager: DeepnoteNotebookManager; let mockIntegrationManager: IIntegrationManager; let mockLogger: ILogger; - const mockAnalytics = new NoOpTelemetryService(); + const mockAnalytics = instance(mock()); setup(() => { mockExtensionContext = { diff --git a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts index e3fd349333..8316c46c88 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts @@ -8,14 +8,14 @@ import { stringify as yamlStringify } from 'yaml'; import { DeepnoteExplorerView } from './deepnoteExplorerView'; import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; import { DeepnoteTreeItem, DeepnoteTreeItemType, type DeepnoteTreeItemContext } from './deepnoteTreeItem'; -import { NoOpTelemetryService } from '../../platform/analytics/noOpTelemetryService'; +import { ITelemetryService } from '../../platform/analytics/types'; import type { IExtensionContext } from '../../platform/common/types'; import type { DeepnoteNotebook } from '../../platform/deepnote/deepnoteTypes'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; import { ILogger } from '../../platform/logging/types'; import * as uuidModule from '../../platform/common/uuid'; -const mockAnalytics = new NoOpTelemetryService(); +const mockAnalytics = instance(mock()); function createMockLogger(): ILogger { return { diff --git a/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts index 2bb310277c..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,7 +18,7 @@ import { InputBlockType } from './deepnoteNotebookCommandListener'; import { formatInputBlockCellContent, getInputBlockLanguage } from './inputBlockContentFormatter'; -import { NoOpTelemetryService } from '../../platform/analytics/noOpTelemetryService'; +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'; @@ -32,6 +32,7 @@ suite('DeepnoteNotebookCommandListener', () => { let disposables: IDisposable[]; let sandbox: sinon.SinonSandbox; let mockConfigService: IConfigurationService; + let mockTelemetryService: ITelemetryService; function createMockConfigService(): IConfigurationService { return { @@ -45,8 +46,9 @@ suite('DeepnoteNotebookCommandListener', () => { sandbox = sinon.createSandbox(); disposables = []; mockConfigService = createMockConfigService(); + mockTelemetryService = mock(); commandListener = new DeepnoteNotebookCommandListener( - new NoOpTelemetryService(), + instance(mockTelemetryService), mockConfigService, disposables ); @@ -95,7 +97,7 @@ suite('DeepnoteNotebookCommandListener', () => { // Create new instance and activate again const disposables2: IDisposable[] = []; const commandListener2 = new DeepnoteNotebookCommandListener( - new NoOpTelemetryService(), + instance(mockTelemetryService), createMockConfigService(), disposables2 ); diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts index bd71fac431..c501940389 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts @@ -6,7 +6,7 @@ import * as fs from 'fs'; import esmock from 'esmock'; import type { OpenInDeepnoteHandler } from './openInDeepnoteHandler.node'; -import { NoOpTelemetryService } from '../../platform/analytics/noOpTelemetryService'; +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'; @@ -50,7 +50,7 @@ suite('OpenInDeepnoteHandler', () => { subscriptions: [] } as any; - handler = new OpenInDeepnoteHandlerClass(mockExtensionContext, new NoOpTelemetryService()); + handler = new OpenInDeepnoteHandlerClass(mockExtensionContext, instance(mock())); }); teardown(() => { From bd7935682270663099b231b6390d5aa40f793957 Mon Sep 17 00:00:00 2001 From: tomas Date: Wed, 1 Apr 2026 21:30:41 +0000 Subject: [PATCH 08/17] refactor(tests): update telemetry service usage in Deepnote tests - Replaced instances of ITelemetryService with a properly initialized mock in DeepnoteActivationService and DeepnoteExplorerView tests. - Adjusted NoOpTelemetryService methods to align with the new telemetry service structure. - Enhanced TelemetryService to ensure proper initialization and event tracking based on telemetry settings. --- .../deepnoteActivationService.unit.test.ts | 3 +- .../deepnoteExplorerView.unit.test.ts | 6 +- .../analytics/noOpTelemetryService.ts | 4 +- src/platform/analytics/telemetryService.ts | 54 +++++----- .../analytics/telemetryService.unit.test.ts | 102 ++++++++---------- 5 files changed, 80 insertions(+), 89 deletions(-) diff --git a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts index d3e16a88a8..e4409af9ad 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.unit.test.ts @@ -26,7 +26,7 @@ suite('DeepnoteActivationService', () => { let manager: DeepnoteNotebookManager; let mockIntegrationManager: IIntegrationManager; let mockLogger: ILogger; - const mockAnalytics = instance(mock()); + let mockAnalytics: ITelemetryService; setup(() => { mockExtensionContext = { @@ -40,6 +40,7 @@ suite('DeepnoteActivationService', () => { } }; mockLogger = createMockLogger(); + mockAnalytics = instance(mock()); activationService = new DeepnoteActivationService( mockExtensionContext, manager, diff --git a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts index 8316c46c88..512a53567d 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts @@ -15,8 +15,6 @@ import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock import { ILogger } from '../../platform/logging/types'; import * as uuidModule from '../../platform/common/uuid'; -const mockAnalytics = instance(mock()); - function createMockLogger(): ILogger { return { error: () => undefined, @@ -47,6 +45,7 @@ suite('DeepnoteExplorerView', () => { let mockExtensionContext: IExtensionContext; let manager: DeepnoteNotebookManager; let mockLogger: ILogger; + let mockAnalytics: ITelemetryService; setup(() => { mockExtensionContext = { @@ -55,6 +54,7 @@ suite('DeepnoteExplorerView', () => { manager = new DeepnoteNotebookManager(); mockLogger = createMockLogger(); + mockAnalytics = instance(mock()); explorerView = new DeepnoteExplorerView(mockExtensionContext, manager, mockLogger, mockAnalytics); }); @@ -225,6 +225,7 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { let mockManager: DeepnoteNotebookManager; let sandbox: sinon.SinonSandbox; let uuidStubs: sinon.SinonStub[] = []; + let mockAnalytics: ITelemetryService; setup(() => { sandbox = sinon.createSandbox(); @@ -237,6 +238,7 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { mockManager = new DeepnoteNotebookManager(); const mockLogger = createMockLogger(); + mockAnalytics = instance(mock()); explorerView = new DeepnoteExplorerView(mockContext, mockManager, mockLogger, mockAnalytics); }); diff --git a/src/platform/analytics/noOpTelemetryService.ts b/src/platform/analytics/noOpTelemetryService.ts index 091994fb07..db39a2d7de 100644 --- a/src/platform/analytics/noOpTelemetryService.ts +++ b/src/platform/analytics/noOpTelemetryService.ts @@ -4,11 +4,11 @@ import { ITelemetryService, TelemetryEvent } from './types'; * No-op telemetry service for use in tests. */ export class NoOpTelemetryService implements ITelemetryService { - public trackEvent(_event: TelemetryEvent): void { + public async dispose(): Promise { // No-op } - public async dispose(): Promise { + public trackEvent(_event: TelemetryEvent): void { // No-op } } diff --git a/src/platform/analytics/telemetryService.ts b/src/platform/analytics/telemetryService.ts index 0dbcefdcc2..a3502ffb68 100644 --- a/src/platform/analytics/telemetryService.ts +++ b/src/platform/analytics/telemetryService.ts @@ -25,6 +25,34 @@ export class TelemetryService implements ITelemetryService { asyncDisposables.push(this); } + private initialize(): void { + try { + this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, ''); + + if (!this.userIdState.value) { + void this.userIdState.updateValue(generateUuid()); + } + + this.client = new PostHog(POSTHOG_API_KEY, { + flushAt: 20, + flushInterval: 30000, + host: POSTHOG_HOST + }); + } catch (error) { + this.initialized = false; + throw error; + } + this.initialized = true; + } + + public async dispose(): Promise { + try { + await this.client?.shutdown(); + } catch (ex) { + logger.debug(`PostHog shutdown error: ${ex}`); + } + } + public trackEvent({ eventName, properties }: TelemetryEvent): void { try { if (!this.isTelemetryEnabled()) { @@ -49,34 +77,10 @@ export class TelemetryService implements ITelemetryService { } } - public async dispose(): Promise { - try { - await this.client?.shutdown(); - } catch (ex) { - logger.debug(`PostHog shutdown error: ${ex}`); - } - } - - private initialize(): void { - this.initialized = true; - - this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, ''); - - if (!this.userIdState.value) { - void this.userIdState.updateValue(generateUuid()); - } - - this.client = new PostHog(POSTHOG_API_KEY, { - flushAt: 20, - flushInterval: 30000, - host: POSTHOG_HOST - }); - } - private isTelemetryEnabled(): boolean { const telemetryLevel = workspace.getConfiguration('telemetry').get('telemetryLevel', 'all'); - if (telemetryLevel === 'off') { + if (telemetryLevel !== 'off') { return false; } diff --git a/src/platform/analytics/telemetryService.unit.test.ts b/src/platform/analytics/telemetryService.unit.test.ts index 6826759b99..082cbd4a04 100644 --- a/src/platform/analytics/telemetryService.unit.test.ts +++ b/src/platform/analytics/telemetryService.unit.test.ts @@ -9,7 +9,6 @@ suite('TelemetryService', () => { let mockStateFactory: IPersistentStateFactory; let mockAsyncDisposableRegistry: IAsyncDisposableRegistry; let mockUserIdState: IPersistentState; - let sandbox: sinon.SinonSandbox; function createMockPersistentState(initialValue: string): IPersistentState { let storedValue = initialValue; @@ -24,8 +23,18 @@ suite('TelemetryService', () => { }; } + // 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; + } + setup(() => { - sandbox = sinon.createSandbox(); mockUserIdState = createMockPersistentState(''); mockStateFactory = { createGlobalPersistentState: sinon.stub().returns(mockUserIdState), @@ -37,78 +46,40 @@ suite('TelemetryService', () => { }; }); - teardown(() => { - sandbox.restore(); - }); - test('should create instance without errors', () => { analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); assert.isDefined(analyticsService); }); - test('trackEvent should not throw when telemetry is disabled', () => { - // Stub workspace.getConfiguration to return telemetry disabled - const vscode = require('vscode'); - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sandbox.stub(vscode.workspace, 'getConfiguration').callsFake((section: any) => ({ - get: (_key: string, defaultValue: unknown) => { - if (section === 'deepnote' && _key === 'telemetry.enabled') { - return false; - } - - return defaultValue; - } - })); - + test('trackEvent should not initialize when telemetry is disabled', () => { analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + stubTelemetryEnabled(analyticsService, false); - assert.doesNotThrow(() => { - analyticsService.trackEvent({ eventName: 'open_notebook', properties: { prop: 'value' } }); - }); + analyticsService.trackEvent({ eventName: 'open_notebook', properties: { prop: 'value' } }); - // Should not have initialized (no state access) + assert.isUndefined(getPostHogClient(analyticsService), 'PostHog client should not be created'); assert.isFalse( (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).called, 'Should not create persistent state when telemetry is disabled' ); }); - test('trackEvent should not throw when VSCode telemetry level is off', () => { - const vscode = require('vscode'); - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sandbox.stub(vscode.workspace, 'getConfiguration').callsFake((section: any) => ({ - get: (_key: string, defaultValue: unknown) => { - if (section === 'telemetry' && _key === 'telemetryLevel') { - return 'off'; - } - - return defaultValue; - } - })); - + test('trackEvent should initialize and call capture when telemetry is enabled', () => { analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + stubTelemetryEnabled(analyticsService, true); - assert.doesNotThrow(() => { - analyticsService.trackEvent({ eventName: 'open_notebook' }); - }); - - assert.isFalse( - (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).called, - 'Should not create persistent state when VSCode telemetry is off' - ); - }); + analyticsService.trackEvent({ eventName: 'open_notebook' }); - test('should generate user ID on first trackEvent when telemetry enabled', () => { - const vscode = require('vscode'); + const client = getPostHogClient(analyticsService); - sandbox.stub(vscode.workspace, 'getConfiguration').callsFake(() => ({ - get: (_key: string, defaultValue: unknown) => defaultValue - })); + assert.isDefined(client, 'PostHog client should be initialized'); + }); + test('should generate user ID and call PostHog capture on first trackEvent', () => { analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + stubTelemetryEnabled(analyticsService, true); + analyticsService.trackEvent({ eventName: 'open_notebook' }); assert.isTrue( @@ -124,19 +95,32 @@ suite('TelemetryService', () => { assert.isString(generatedId); assert.isNotEmpty(generatedId, 'Generated user ID should not be empty'); - }); - test('should reuse existing user ID', () => { - const vscode = require('vscode'); + // Stub capture on the initialized client and verify next event + const client = getPostHogClient(analyticsService); + + assert.isDefined(client, 'PostHog client should be initialized'); + + const captureStub = sinon.stub(); + client.capture = captureStub; + + analyticsService.trackEvent({ eventName: 'execute_notebook' }); - sandbox.stub(vscode.workspace, 'getConfiguration').callsFake(() => ({ - get: (_key: string, defaultValue: unknown) => defaultValue - })); + assert.isTrue(captureStub.calledOnce, 'PostHog capture should be called'); + assert.deepStrictEqual(captureStub.firstCall.args[0], { + distinctId: generatedId, + event: 'execute_notebook', + properties: undefined + }); + }); + test('should reuse existing user ID', () => { mockUserIdState = createMockPersistentState('existing-user-id'); (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).returns(mockUserIdState); analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + stubTelemetryEnabled(analyticsService, true); + analyticsService.trackEvent({ eventName: 'open_notebook' }); assert.isFalse( From 21aab9aba735e86bfe8bdd329e09f839be6294c4 Mon Sep 17 00:00:00 2001 From: tomas Date: Thu, 2 Apr 2026 09:45:16 +0000 Subject: [PATCH 09/17] refactor(telemetry): enhance TelemetryService with activation and client management - Updated TelemetryService to implement IExtensionSyncActivationService, allowing for better integration with extension lifecycle events. - Introduced methods for client creation and destruction based on telemetry settings, improving resource management. - Adjusted tests to validate the new activation logic and client handling, ensuring proper behavior when telemetry settings change. --- src/platform/analytics/telemetryService.ts | 100 ++++++++++++------ .../analytics/telemetryService.unit.test.ts | 98 ++++++++++++----- src/platform/serviceRegistry.node.ts | 1 + 3 files changed, 135 insertions(+), 64 deletions(-) diff --git a/src/platform/analytics/telemetryService.ts b/src/platform/analytics/telemetryService.ts index a3502ffb68..eff82300a3 100644 --- a/src/platform/analytics/telemetryService.ts +++ b/src/platform/analytics/telemetryService.ts @@ -2,67 +2,59 @@ import { inject, injectable } from 'inversify'; import { PostHog } from 'posthog-node'; import { workspace } from 'vscode'; -import { IAsyncDisposableRegistry, IPersistentState, IPersistentStateFactory } from '../common/types'; +import { IExtensionSyncActivationService } from '../activation/types'; +import { + IAsyncDisposableRegistry, + IDisposableRegistry, + IPersistentState, + IPersistentStateFactory +} from '../common/types'; import { generateUuid } from '../common/uuid'; import { logger } from '../logging'; import { POSTHOG_API_KEY, POSTHOG_HOST } from './constants'; import { ITelemetryService, TelemetryEvent } from './types'; const USER_ID_STORAGE_KEY = 'deepnote-telemetry-anonymous-user-id'; +const POSTHOG_SHUTDOWN_TIMEOUT = 5000; @injectable() -export class TelemetryService implements ITelemetryService { - private client: PostHog | undefined; +export class TelemetryService implements ITelemetryService, IExtensionSyncActivationService { + private client: PostHog | null; - private initialized = false; - - private userIdState: IPersistentState | undefined; + private userIdState: IPersistentState; constructor( + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IPersistentStateFactory) private readonly stateFactory: IPersistentStateFactory, @inject(IAsyncDisposableRegistry) asyncDisposables: IAsyncDisposableRegistry ) { asyncDisposables.push(this); + this.client = null; + this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, generateUuid()); } - private initialize(): void { + public async activate(): Promise { try { - this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, ''); - - if (!this.userIdState.value) { - void this.userIdState.updateValue(generateUuid()); - } - - this.client = new PostHog(POSTHOG_API_KEY, { - flushAt: 20, - flushInterval: 30000, - host: POSTHOG_HOST - }); + this.createClient(); } catch (error) { - this.initialized = false; - throw error; + logger.debug(`TelemetryService activation error: ${error}`); } - this.initialized = true; + + this.disposables.push( + workspace.onDidChangeConfiguration((e) => { + if (e.affectsConfiguration('telemetry') || e.affectsConfiguration('deepnote.telemetry')) { + this.handleConfigChanged(); + } + }) + ); } public async dispose(): Promise { - try { - await this.client?.shutdown(); - } catch (ex) { - logger.debug(`PostHog shutdown error: ${ex}`); - } + await this.destroyClient(); } public trackEvent({ eventName, properties }: TelemetryEvent): void { try { - if (!this.isTelemetryEnabled()) { - return; - } - - if (!this.initialized) { - this.initialize(); - } - if (!this.client || !this.userIdState) { return; } @@ -77,13 +69,51 @@ export class TelemetryService implements ITelemetryService { } } + private createClient(): void { + if (this.client || !this.isTelemetryEnabled()) { + return; + } + + this.client = new PostHog(POSTHOG_API_KEY, { + flushAt: 20, + flushInterval: 30000, + 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 isTelemetryEnabled(): boolean { const telemetryLevel = workspace.getConfiguration('telemetry').get('telemetryLevel', 'all'); - if (telemetryLevel !== 'off') { + if (telemetryLevel !== 'all') { return false; } return workspace.getConfiguration('deepnote').get('telemetry.enabled', true); } + + private handleConfigChanged(): void { + if (this.isTelemetryEnabled()) { + this.createClient(); + } else { + this.destroyClient().catch((error) => { + logger.error(`Failed to destroy PostHog client: ${error}`); + this.client = null; + }); + } + } } diff --git a/src/platform/analytics/telemetryService.unit.test.ts b/src/platform/analytics/telemetryService.unit.test.ts index 082cbd4a04..bddcef0294 100644 --- a/src/platform/analytics/telemetryService.unit.test.ts +++ b/src/platform/analytics/telemetryService.unit.test.ts @@ -1,11 +1,17 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; -import { IAsyncDisposableRegistry, IPersistentState, IPersistentStateFactory } from '../common/types'; +import { + IAsyncDisposableRegistry, + IDisposableRegistry, + IPersistentState, + IPersistentStateFactory +} from '../common/types'; import { TelemetryService } from './telemetryService'; suite('TelemetryService', () => { let analyticsService: TelemetryService; + let mockDisposables: IDisposableRegistry; let mockStateFactory: IPersistentStateFactory; let mockAsyncDisposableRegistry: IAsyncDisposableRegistry; let mockUserIdState: IPersistentState; @@ -36,6 +42,7 @@ suite('TelemetryService', () => { setup(() => { mockUserIdState = createMockPersistentState(''); + mockDisposables = []; mockStateFactory = { createGlobalPersistentState: sinon.stub().returns(mockUserIdState), createWorkspacePersistentState: sinon.stub().returns(mockUserIdState) @@ -47,56 +54,54 @@ suite('TelemetryService', () => { }); test('should create instance without errors', () => { - analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); assert.isDefined(analyticsService); }); - test('trackEvent should not initialize when telemetry is disabled', () => { - analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + test('activate should not create client when telemetry is disabled', async () => { + analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, false); - analyticsService.trackEvent({ eventName: 'open_notebook', properties: { prop: 'value' } }); + await analyticsService.activate(); - assert.isUndefined(getPostHogClient(analyticsService), 'PostHog client should not be created'); - assert.isFalse( - (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).called, - 'Should not create persistent state when telemetry is disabled' + 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('trackEvent should initialize and call capture when telemetry is enabled', () => { - analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + test('activate should create client when telemetry is enabled', async () => { + analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); - analyticsService.trackEvent({ eventName: 'open_notebook' }); + await analyticsService.activate(); const client = getPostHogClient(analyticsService); assert.isDefined(client, 'PostHog client should be initialized'); }); - test('should generate user ID and call PostHog capture on first trackEvent', () => { - analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + test('should generate user ID and call PostHog capture on first trackEvent', async () => { + (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).callsFake( + (_key: string, defaultValue: string) => createMockPersistentState(defaultValue) + ); + + analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); - analyticsService.trackEvent({ eventName: 'open_notebook' }); + await analyticsService.activate(); - assert.isTrue( - (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).calledOnce, - 'Should create persistent state' - ); - assert.isTrue( - (mockUserIdState.updateValue as sinon.SinonStub).calledOnce, - 'Should generate and persist user ID' - ); + const createStateSpy = mockStateFactory.createGlobalPersistentState as sinon.SinonStub; - const generatedId = (mockUserIdState.updateValue as sinon.SinonStub).firstCall.args[0]; + assert.isTrue(createStateSpy.calledOnce, 'Should create persistent state'); + + const generatedId = createStateSpy.firstCall.args[1]; assert.isString(generatedId); assert.isNotEmpty(generatedId, 'Generated user ID should not be empty'); - // Stub capture on the initialized client and verify next event const client = getPostHogClient(analyticsService); assert.isDefined(client, 'PostHog client should be initialized'); @@ -114,14 +119,14 @@ suite('TelemetryService', () => { }); }); - test('should reuse existing user ID', () => { + test('should reuse existing user ID', async () => { mockUserIdState = createMockPersistentState('existing-user-id'); (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).returns(mockUserIdState); - analyticsService = new TelemetryService(mockStateFactory, mockAsyncDisposableRegistry); + analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); - analyticsService.trackEvent({ eventName: 'open_notebook' }); + await analyticsService.activate(); assert.isFalse( (mockUserIdState.updateValue as sinon.SinonStub).called, @@ -129,8 +134,43 @@ suite('TelemetryService', () => { ); }); + test('settings change should destroy client when telemetry is disabled', async () => { + analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); + stubTelemetryEnabled(analyticsService, true); + + await analyticsService.activate(); + + const client = getPostHogClient(analyticsService); + + assert.isDefined(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); + stubTelemetryEnabled(analyticsService, false); + + 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.isDefined(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(mockStateFactory, mockAsyncDisposableRegistry); + analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); await assert.isFulfilled(analyticsService.dispose()); }); diff --git a/src/platform/serviceRegistry.node.ts b/src/platform/serviceRegistry.node.ts index 3017f187c4..d9a3346276 100644 --- a/src/platform/serviceRegistry.node.ts +++ b/src/platform/serviceRegistry.node.ts @@ -29,6 +29,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addBinding(FileSystem, IFileSystem); serviceManager.addSingleton(IWorkspaceService, WorkspaceService); serviceManager.addSingleton(ITelemetryService, TelemetryService); + serviceManager.addBinding(ITelemetryService, IExtensionSyncActivationService); serviceManager.addSingleton(IConfigurationService, ConfigurationService); registerApiTypes(serviceManager); From f4cc7ac4df1472fe56ec248ef1d05bb45e0120ef Mon Sep 17 00:00:00 2001 From: tomas Date: Fri, 17 Jul 2026 12:08:47 +0000 Subject: [PATCH 10/17] feat(telemetry): cover new user actions and fix completion accuracy Addresses the coverage gaps and correctness issues found by auditing the telemetry PR against the merged single-notebook refactor. Coverage (new events): - rename_notebook, rename_project (context-menu renames) - create_notebook now also fires for Add-notebook-to-project, previously an untracked path of the same metric - split_notebook {completed, notebookCount} for the legacy multi-notebook split - authenticate_integration for the federated-auth (BigQuery OAuth) flow - switch_sql_integration {integrationType} from the SQL cell status bar - update_environment {field} for environment rename / package edits - select_environment now also fires on the first-run kernel picker Correctness: - Gate open_notebook / create_notebook / duplicate_notebook / open_in_deepnote on success ({completed}) instead of firing on cancel/guard/error paths. - import_notebook completed now reflects whether anything imported (numberOfNotebooks > 0) rather than an unconditional true. - save/reset/delete_integration fire only after the persist succeeds. - execute_notebook / add_block(code) fire only for deepnote notebooks. - export_notebook format label 'notebook' -> 'jupyter'. Adds the new names to the TelemetryEventName union and injects ITelemetryService into the splitter, SQL status bar, and kernel auto-selector. typecheck 0, lint 0, compile-tsc 0, unit tests 2502 passing / 0 failing. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ --- .../deepnoteEnvironmentsView.node.ts | 5 + .../deepnote/deepnoteActivationService.ts | 3 +- .../deepnote/deepnoteExplorerView.ts | 107 +++++++++++------- .../deepnoteKernelAutoSelector.node.ts | 5 +- ...epnoteKernelAutoSelector.node.unit.test.ts | 4 +- .../deepnote/deepnoteMultiNotebookSplitter.ts | 21 +++- ...deepnoteMultiNotebookSplitter.unit.test.ts | 10 +- .../integrations/integrationWebview.ts | 47 ++++++-- .../deepnote/openInDeepnoteHandler.node.ts | 22 ++-- .../deepnote/sqlCellStatusBarProvider.ts | 13 ++- .../sqlCellStatusBarProvider.unit.test.ts | 20 +++- src/notebooks/notebookCommandListener.ts | 14 ++- src/platform/analytics/types.ts | 8 +- 13 files changed, 198 insertions(+), 81 deletions(-) diff --git a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts index 81b0323b32..0684a334ba 100644 --- a/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts +++ b/src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts @@ -556,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.')); @@ -614,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/notebooks/deepnote/deepnoteActivationService.ts b/src/notebooks/deepnote/deepnoteActivationService.ts index 8e10abe94b..cb2144b9fd 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.ts @@ -89,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/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index 6e4836eaeb..e26e2803d0 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -146,9 +146,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 false; } try { @@ -158,7 +158,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return false; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -166,7 +166,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return false; } const currentName = targetNotebook.name; @@ -175,7 +175,7 @@ export class DeepnoteExplorerView { const newName = await this.promptForNotebookName(currentName, existingNames); if (!newName || newName === currentName) { - return; + return false; } // Flush the open document and re-read before rewriting, so we serialize the user's live cell @@ -185,7 +185,7 @@ export class DeepnoteExplorerView { l10n.t('Could not save "{0}" before renaming. The notebook was left unchanged.', currentName) ); - return; + return false; } const freshData = await readDeepnoteProjectFile(fileUri); @@ -194,7 +194,7 @@ export class DeepnoteExplorerView { if (!freshTarget) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return false; } freshTarget.name = newName; @@ -203,9 +203,13 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook renamed to: {0}', newName)); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to rename notebook: {0}', errorMessage)); + + return false; } } @@ -282,9 +286,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 false; } try { @@ -294,7 +298,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return false; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -302,7 +306,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return; + return false; } const existingNames = await this.collectNotebookNamesForProject(treeItem.context.projectId); @@ -323,7 +327,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook duplicated: {0}', newName)); - return; + return true; } // Legacy multi-notebook file: append the duplicate in place (existing behavior). @@ -340,15 +344,19 @@ export class DeepnoteExplorerView { }); await window.showInformationMessage(l10n.t('Notebook duplicated: {0}', newName)); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to duplicate notebook: {0}', errorMessage)); + + return false; } } - public async renameProject(treeItem: DeepnoteTreeItem): Promise { + public async renameProject(treeItem: DeepnoteTreeItem): Promise { if (treeItem.extra.type !== DeepnoteTreeItemType.ProjectGroup) { - return; + return false; } const group = treeItem.extra.data; @@ -367,7 +375,7 @@ export class DeepnoteExplorerView { }); if (!newName || newName === currentName) { - return; + return false; } try { @@ -384,7 +392,7 @@ export class DeepnoteExplorerView { ) ); - return; + return false; } } @@ -417,9 +425,13 @@ export class DeepnoteExplorerView { } else { await window.showInformationMessage(l10n.t('Project renamed to: {0}', newName)); } + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to rename project: {0}', errorMessage)); + + return false; } } @@ -430,8 +442,8 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.OpenDeepnoteNotebook, async (context: DeepnoteTreeItemContext) => { - await this.openNotebook(context); - this.analytics.trackEvent({ eventName: 'open_notebook' }); + const completed = await this.openNotebook(context); + this.analytics.trackEvent({ eventName: 'open_notebook', properties: { completed } }); }) ); @@ -466,22 +478,24 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.NewNotebook, async () => { - await this.newNotebook(); - this.analytics.trackEvent({ eventName: 'create_notebook' }); + const completed = await this.newNotebook(); + this.analytics.trackEvent({ eventName: 'create_notebook', properties: { completed } }); }) ); // 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 completed = await this.renameProject(treeItem); + this.analytics.trackEvent({ eventName: 'rename_project', properties: { completed } }); + }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.RenameNotebook, (treeItem: DeepnoteTreeItem) => - this.renameNotebook(treeItem) - ) + commands.registerCommand(Commands.RenameNotebook, async (treeItem: DeepnoteTreeItem) => { + const completed = await this.renameNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'rename_notebook', properties: { completed } }); + }) ); this.extensionContext.subscriptions.push( @@ -493,15 +507,16 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DuplicateNotebook, async (treeItem: DeepnoteTreeItem) => { - await this.duplicateNotebook(treeItem); - this.analytics.trackEvent({ eventName: 'duplicate_notebook' }); + const completed = await this.duplicateNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'duplicate_notebook', properties: { completed } }); }) ); this.extensionContext.subscriptions.push( - commands.registerCommand(Commands.AddNotebookToProject, (treeItem: DeepnoteTreeItem) => - this.addNotebookToProject(treeItem) - ) + commands.registerCommand(Commands.AddNotebookToProject, async (treeItem: DeepnoteTreeItem) => { + const completed = await this.addNotebookToProject(treeItem); + this.analytics.trackEvent({ eventName: 'create_notebook', properties: { completed } }); + }) ); this.extensionContext.subscriptions.push( @@ -509,7 +524,7 @@ export class DeepnoteExplorerView { const completed = await this.exportNotebook(treeItem); this.analytics.trackEvent({ eventName: 'export_notebook', - properties: { completed, format: 'notebook' } + properties: { completed, format: 'jupyter' } }); }) ); @@ -689,7 +704,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); @@ -698,10 +713,14 @@ export class DeepnoteExplorerView { preview: false, preserveFocus: false }); + + return true; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(`Failed to open notebook: ${errorMessage}`); + + return false; } } @@ -871,13 +890,13 @@ export class DeepnoteExplorerView { } } - 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 false; } const document = activeEditor.notebook; @@ -899,9 +918,13 @@ export class DeepnoteExplorerView { await window.showInformationMessage(l10n.t('Created new notebook: {0}', result.name)); } + + return result !== null; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to add notebook: {0}', errorMessage)); + + return false; } } @@ -1059,7 +1082,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); - return true; + return numberOfNotebooks > 0; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; @@ -1119,7 +1142,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); - return true; + return numberOfNotebooks > 0; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; @@ -1129,9 +1152,9 @@ export class DeepnoteExplorerView { } } - private async addNotebookToProject(treeItem: DeepnoteTreeItem): Promise { + private async addNotebookToProject(treeItem: DeepnoteTreeItem): Promise { if (treeItem.extra.type !== DeepnoteTreeItemType.ProjectGroup) { - return; + return false; } const group = treeItem.extra.data; @@ -1140,7 +1163,7 @@ export class DeepnoteExplorerView { if (!sourceFile) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return; + return false; } try { @@ -1153,9 +1176,13 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Created new notebook: {0}', result.name)); } + + return result !== null; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to add notebook: {0}', errorMessage)); + + return false; } } diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index 8c0b93cc4a..fd8fe6449d 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() { @@ -766,6 +768,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, Cancellation.throwIfCanceled(token); await this.notebookEnvironmentMapper.setEnvironmentForNotebook(notebook.uri, selectedEnvironment.id); + this.analytics.trackEvent({ eventName: 'select_environment' }); const result = await this.setupKernelForEnvironment(notebook, selectedEnvironment, notebookKey, token); diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts index d5f71bc77c..7607d20a6c 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts @@ -2,6 +2,7 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import { DeepnoteKernelAutoSelector } from './deepnoteKernelAutoSelector.node'; +import { ITelemetryService } from '../../platform/analytics/types'; import { createMockChildProcess } from '../../kernels/deepnote/deepnoteTestHelpers.node'; import { ServerHandleRegistry } from '../../kernels/deepnote/deepnoteServerHandleRegistry.node'; import { @@ -141,7 +142,8 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { instance(mockNotebookEnvironmentMapper), instance(mockOutputChannel), instance(mockToolkitInstaller), - registry + registry, + instance(mock()) ); }); diff --git a/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts b/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts index 49da7ee7c6..0e9cbda899 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'; @@ -35,16 +36,20 @@ export class DeepnoteMultiNotebookSplitter { private readonly refreshTree: () => void; + private readonly analytics: ITelemetryService; + constructor( 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,18 @@ export class DeepnoteMultiNotebookSplitter { ); if (selection === SPLIT_ACTION) { - await this.splitFile(fileUri); + const notebookCount = await this.splitFile(fileUri); + this.analytics.trackEvent({ + eventName: 'split_notebook', + properties: { completed: notebookCount > 0, notebookCount } + }); } } 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 +143,7 @@ export class DeepnoteMultiNotebookSplitter { l10n.t('Could not save the file before splitting. The file was left unchanged.') ); - return; + return 0; } } @@ -193,6 +202,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 +212,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..cc04d13a91 100644 --- a/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.unit.test.ts @@ -3,6 +3,7 @@ import { assert } from 'chai'; import { anything, instance, mock, 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'; @@ -193,7 +194,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(mock()) ); splitter.activate(); }); @@ -403,7 +405,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 +585,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(); diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index d30bc68c97..bbc0dc77d4 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -579,27 +579,38 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { break; case 'save': if (message.integrationId && message.config) { - this.analytics.trackEvent({ - eventName: 'save_integration', - properties: { integrationType: message.config.type ?? 'unknown' } - }); - await this.saveConfiguration(message.integrationId, message.config); + const saved = await this.saveConfiguration(message.integrationId, message.config); + + if (saved) { + this.analytics.trackEvent({ + eventName: 'save_integration', + properties: { integrationType: message.config.type ?? 'unknown' } + }); + } } break; case 'reset': if (message.integrationId) { - this.analytics.trackEvent({ eventName: 'reset_integration' }); - await this.resetConfiguration(message.integrationId); + const reset = await this.resetConfiguration(message.integrationId); + + if (reset) { + this.analytics.trackEvent({ eventName: 'reset_integration' }); + } } break; case 'delete': if (message.integrationId) { - this.analytics.trackEvent({ eventName: 'delete_integration' }); - await this.deleteConfiguration(message.integrationId); + const deleted = await this.deleteConfiguration(message.integrationId); + + if (deleted) { + this.analytics.trackEvent({ eventName: 'delete_integration' }); + } } break; case 'authenticate': if (message.integrationId) { + this.analytics.trackEvent({ eventName: 'authenticate_integration' }); + try { await commands.executeCommand(Commands.AuthenticateIntegration, message.integrationId); } catch (error) { @@ -638,7 +649,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); @@ -674,6 +685,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { type: 'success' }); } + + return persisted; } catch (error) { logger.error('Failed to save integration configuration', error); await this.currentPanel?.webview.postMessage({ @@ -683,13 +696,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) => { @@ -714,6 +729,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { type: 'success' }); } + + return persisted; } catch (error) { logger.error('Failed to reset integration configuration', error); await this.currentPanel?.webview.postMessage({ @@ -723,13 +740,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) => { @@ -749,6 +768,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { type: 'success' }); } + + return persisted; } catch (error) { logger.error('Failed to delete integration', error); await this.currentPanel?.webview.postMessage({ @@ -758,6 +779,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { ), type: 'error' }); + + return false; } } diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts index 8af533ac56..355209a0ec 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts @@ -22,13 +22,13 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { public activate(): void { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.OpenInDeepnote, async () => { - await this.handleOpenInDeepnote(); - this.analytics.trackEvent({ eventName: 'open_in_deepnote' }); + const completed = await this.handleOpenInDeepnote(); + this.analytics.trackEvent({ eventName: 'open_in_deepnote', properties: { completed } }); }) ); } - private async handleOpenInDeepnote(): Promise { + private async handleOpenInDeepnote(): Promise { try { let fileUri: Uri | undefined; let isNotebook = false; @@ -46,7 +46,7 @@ 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; @@ -54,7 +54,7 @@ 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) { @@ -65,7 +65,7 @@ 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; } } } @@ -78,12 +78,12 @@ 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'), @@ -112,10 +112,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; } } ); @@ -123,6 +127,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/sqlCellStatusBarProvider.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts index 5bf312977c..2546ded250 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts @@ -19,6 +19,7 @@ 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'; @@ -69,7 +70,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 +461,14 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid // Trigger status bar update this._onDidChangeCellStatusBarItems.fire(); + + const selectedIntegration = projectIntegrations.find((i) => i.id === selectedId); + this.analytics.trackEvent({ + eventName: 'switch_sql_integration', + properties: { + integrationType: + selectedId === DATAFRAME_SQL_INTEGRATION_ID ? 'duckdb' : selectedIntegration?.type ?? 'unknown' + } + }); } } diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts index f729453582..b52592940b 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts @@ -4,6 +4,7 @@ import { CancellationToken, CancellationTokenSource, EventEmitter, NotebookCell 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 @@ -809,7 +818,8 @@ suite('SqlCellStatusBarProvider', () => { commandProvider = new SqlCellStatusBarProvider( commandDisposables, instance(commandIntegrationStorage), - instance(commandNotebookManager) + instance(commandNotebookManager), + instance(mock()) ); // Capture the command handler diff --git a/src/notebooks/notebookCommandListener.ts b/src/notebooks/notebookCommandListener.ts index d9bb97e37e..1a1b8da0ea 100644 --- a/src/notebooks/notebookCommandListener.ts +++ b/src/notebooks/notebookCommandListener.ts @@ -115,8 +115,11 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens } private runAllCells() { - if (window.activeNotebookEditor) { - this.analytics.trackEvent({ eventName: 'execute_notebook' }); + const editor = window.activeNotebookEditor; + if (editor) { + if (editor.notebook.notebookType === 'deepnote') { + this.analytics.trackEvent({ eventName: 'execute_notebook' }); + } commands.executeCommand('notebook.execute').then(noop, noop); } } @@ -143,8 +146,11 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens } private addCellBelow() { - if (window.activeNotebookEditor) { - this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'code' } }); + const editor = window.activeNotebookEditor; + if (editor) { + if (editor.notebook.notebookType === 'deepnote') { + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'code' } }); + } commands.executeCommand('notebook.cell.insertCodeCellBelow').then(noop, noop); } } diff --git a/src/platform/analytics/types.ts b/src/platform/analytics/types.ts index 9cb347dd4e..1695978eed 100644 --- a/src/platform/analytics/types.ts +++ b/src/platform/analytics/types.ts @@ -2,6 +2,7 @@ import { IAsyncDisposable } from '../common/types'; export type TelemetryEventName = | 'add_block' + | 'authenticate_integration' | 'configure_integration' | 'create_environment' | 'create_notebook' @@ -16,10 +17,15 @@ export type TelemetryEventName = | 'import_notebook' | 'open_in_deepnote' | 'open_notebook' + | 'rename_notebook' + | 'rename_project' | 'reset_integration' | 'save_integration' | 'select_environment' - | 'toggle_snapshots'; + | 'split_notebook' + | 'switch_sql_integration' + | 'toggle_snapshots' + | 'update_environment'; export interface TelemetryEvent { eventName: TelemetryEventName; From 8c04f542e4057fa718b1a080745b16d564ee5f28 Mon Sep 17 00:00:00 2001 From: tomas Date: Fri, 17 Jul 2026 14:35:12 +0000 Subject: [PATCH 11/17] test(e2e): harden project-rename setup against a stale-element race leaveUnsavedCellEdit located the first code cell's view-line, then clicked it in a separate step. A freshly opened Deepnote notebook re-renders its cells for a beat (status-bar items, kernel wiring), which can invalidate that element reference before the click lands, failing the before-all hook with StaleElementReferenceError. Locate and click in one waited step and retry on failure, so a re-render between locating and clicking no longer breaks it. Verified: the projectRename suite passes 3/3 locally against the packaged VSIX built from the current source. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ --- test/e2e/suite/projectRename.e2e.test.ts | 25 ++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/test/e2e/suite/projectRename.e2e.test.ts b/test/e2e/suite/projectRename.e2e.test.ts index 4d123285cf..2a2c455672 100644 --- a/test/e2e/suite/projectRename.e2e.test.ts +++ b/test/e2e/suite/projectRename.e2e.test.ts @@ -42,12 +42,29 @@ async function leaveUnsavedCellEdit(): Promise { // 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], + // Locate AND click in one waited step, retrying on failure: a freshly opened Deepnote notebook + // re-renders its cells for a beat (status-bar items, kernel wiring), which can invalidate a + // separately-located element reference before the click lands (StaleElementReferenceError). + 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 { + // Stale reference (the cell re-rendered) or not yet clickable — re-locate and retry. + 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); From fbd4a6563887b821fd3197b4f22e5700e26c6653 Mon Sep 17 00:00:00 2001 From: tomas Date: Fri, 17 Jul 2026 15:37:04 +0000 Subject: [PATCH 12/17] fix(telemetry): bake PostHog API key into CD builds The __POSTHOG_API_KEY__ placeholder shipped unreplaced, so PostHog initialized with an invalid key and dropped all events. CD now injects the POSTHOG_API_KEY secret at build time via an esbuild define, falling back to the inert placeholder for local builds. Also address review feedback: - extract add_block and integration tracking into helpers; all integration events now send a consistent integrationType - localize the deepnote.telemetry.enabled setting description Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ --- .github/workflows/cd.yml | 2 ++ build/esbuild/build.ts | 6 +++++ package.json | 2 +- package.nls.json | 1 + .../deepnoteNotebookCommandListener.ts | 14 +++++++---- .../integrations/integrationWebview.ts | 23 +++++++++++-------- src/platform/analytics/constants.ts | 9 +++++++- 7 files changed, 41 insertions(+), 16 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 473d733d08..e6fad70eea 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -58,6 +58,8 @@ jobs: - name: Package extension run: npm run package + env: + POSTHOG_API_KEY: ${{ secrets.POSTHOG_API_KEY }} - name: Rename VSIX file run: | diff --git a/build/esbuild/build.ts b/build/esbuild/build.ts index c313ce8cf8..a2b65c699e 100644 --- a/build/esbuild/build.ts +++ b/build/esbuild/build.ts @@ -231,6 +231,12 @@ function createConfig( inject.push(path.join(__dirname, isDevbuild ? 'process.development.js' : 'process.production.js')); } } + if (target === 'desktop') { + // Bake the PostHog key in from the CI secret at build time; falls back to the placeholder locally (see constants.ts). + define = { + POSTHOG_API_KEY_BUILD: JSON.stringify(process.env.POSTHOG_API_KEY ?? '') + }; + } if (source.endsWith(path.join('data-explorer', 'index.tsx'))) { inject.push(path.join(__dirname, 'jquery.js')); } diff --git a/package.json b/package.json index 6337d3bc43..e44e44ecb5 100644 --- a/package.json +++ b/package.json @@ -1646,7 +1646,7 @@ "deepnote.telemetry.enabled": { "type": "boolean", "default": true, - "description": "Enable anonymous usage telemetry to help improve Deepnote for VS Code.", + "description": "%deepnote.configuration.deepnote.telemetry.enabled.description%", "scope": "application" }, "deepnote.snapshots.enabled": { 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/notebooks/deepnote/deepnoteNotebookCommandListener.ts b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts index c410bd9a83..27865247d0 100644 --- a/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts +++ b/src/notebooks/deepnote/deepnoteNotebookCommandListener.ts @@ -266,7 +266,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert SQL block')); } - this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'sql' } }); + this.trackAddBlock('sql'); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -309,7 +309,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert big number chart block')); } - this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'big-number' } }); + this.trackAddBlock('big-number'); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -365,7 +365,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new WrappedError(l10n.t('Failed to insert chart block')); } - this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'visualization' } }); + this.trackAddBlock('visualization'); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); @@ -414,7 +414,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert input block')); } - this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType } }); + this.trackAddBlock(blockType); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -549,7 +549,7 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation throw new Error(l10n.t('Failed to insert text block')); } - this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: textBlockType } }); + this.trackAddBlock(textBlockType); const notebookRange = new NotebookRange(insertIndex, insertIndex + 1); editor.revealRange(notebookRange, NotebookEditorRevealType.Default); @@ -588,4 +588,8 @@ export class DeepnoteNotebookCommandListener implements IExtensionSyncActivation 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/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index bbc0dc77d4..15dae3772f 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -3,7 +3,7 @@ import { commands, Disposable, l10n, Uri, ViewColumn, WebviewPanel, window } fro import { BigQueryAuthMethods } from '@deepnote/database-integrations'; -import { ITelemetryService } from '../../../platform/analytics/types'; +import { ITelemetryService, TelemetryEventName } 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'; @@ -564,6 +564,10 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } } + private trackIntegrationEvent(eventName: TelemetryEventName, integrationType: string | undefined): void { + this.analytics.trackEvent({ eventName, properties: { integrationType: integrationType ?? 'unknown' } }); + } + /** Handle messages from the webview; mirrors the `WebviewOutboundMessage` union in `src/webviews/webview-side/integrations/types.ts`. */ private async handleMessage(message: { type: string; @@ -573,7 +577,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { switch (message.type) { case 'configure': if (message.integrationId) { - this.analytics.trackEvent({ eventName: 'configure_integration' }); + const integrationType = this.integrations.get(message.integrationId)?.integrationType; + this.trackIntegrationEvent('configure_integration', integrationType); await this.showConfigurationForm(message.integrationId); } break; @@ -582,34 +587,34 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { const saved = await this.saveConfiguration(message.integrationId, message.config); if (saved) { - this.analytics.trackEvent({ - eventName: 'save_integration', - properties: { integrationType: message.config.type ?? 'unknown' } - }); + this.trackIntegrationEvent('save_integration', message.config.type); } } break; case 'reset': if (message.integrationId) { + const integrationType = this.integrations.get(message.integrationId)?.integrationType; const reset = await this.resetConfiguration(message.integrationId); if (reset) { - this.analytics.trackEvent({ eventName: 'reset_integration' }); + this.trackIntegrationEvent('reset_integration', integrationType); } } break; case 'delete': if (message.integrationId) { + const integrationType = this.integrations.get(message.integrationId)?.integrationType; const deleted = await this.deleteConfiguration(message.integrationId); if (deleted) { - this.analytics.trackEvent({ eventName: 'delete_integration' }); + this.trackIntegrationEvent('delete_integration', integrationType); } } break; case 'authenticate': if (message.integrationId) { - this.analytics.trackEvent({ eventName: 'authenticate_integration' }); + const integrationType = this.integrations.get(message.integrationId)?.integrationType; + this.trackIntegrationEvent('authenticate_integration', integrationType); try { await commands.executeCommand(Commands.AuthenticateIntegration, message.integrationId); diff --git a/src/platform/analytics/constants.ts b/src/platform/analytics/constants.ts index acbefc5b66..a06a0d860c 100644 --- a/src/platform/analytics/constants.ts +++ b/src/platform/analytics/constants.ts @@ -1,2 +1,9 @@ -export const POSTHOG_API_KEY = '__POSTHOG_API_KEY__'; +// 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; + +export const POSTHOG_API_KEY = + typeof POSTHOG_API_KEY_BUILD !== 'undefined' && POSTHOG_API_KEY_BUILD + ? POSTHOG_API_KEY_BUILD + : '__POSTHOG_API_KEY__'; export const POSTHOG_HOST = 'https://us.i.posthog.com'; From 3f4eef5eae73ee68d07e0d7be3fc070207d0b7d7 Mon Sep 17 00:00:00 2001 From: tomas Date: Fri, 17 Jul 2026 20:56:06 +0000 Subject: [PATCH 13/17] fix(telemetry): report rename_project completion accurately renameProject returned true even when sibling file writes failed, so the rename_project event recorded completed: true on partial failure. Return failedCount === 0 so completion reflects a fully successful rename. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ --- src/notebooks/deepnote/deepnoteExplorerView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index e26e2803d0..463b985769 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -426,7 +426,7 @@ export class DeepnoteExplorerView { await window.showInformationMessage(l10n.t('Project renamed to: {0}', newName)); } - return true; + return failedCount === 0; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(l10n.t('Failed to rename project: {0}', errorMessage)); From d9f88d6457fa9ac58f362054abcdfb0769f8f015 Mon Sep 17 00:00:00 2001 From: tomas Date: Sat, 18 Jul 2026 07:50:31 +0000 Subject: [PATCH 14/17] fix(telemetry): skip PostHog init when the API key is unconfigured Add IS_POSTHOG_CONFIGURED (POSTHOG_API_KEY is not the build-time placeholder and non-empty) and early-return from createClient() when it is false, so local and key-less builds no longer spin up a PostHog client against the inert '__POSTHOG_API_KEY__' placeholder. The guard is read through a private isPostHogConfigured() predicate so it stays mockable in the ESM unit tests (sinon cannot stub module-level constants); tests cover both the configured and unconfigured paths. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ --- src/platform/analytics/constants.ts | 7 +++++- src/platform/analytics/telemetryService.ts | 8 +++++-- .../analytics/telemetryService.unit.test.ts | 22 +++++++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/platform/analytics/constants.ts b/src/platform/analytics/constants.ts index a06a0d860c..9d7afe74fd 100644 --- a/src/platform/analytics/constants.ts +++ b/src/platform/analytics/constants.ts @@ -2,8 +2,13 @@ // Left undefined in local builds, where telemetry falls back to this inert placeholder. declare const POSTHOG_API_KEY_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__'; + : POSTHOG_API_KEY_PLACEHOLDER; 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 index eff82300a3..991764adc6 100644 --- a/src/platform/analytics/telemetryService.ts +++ b/src/platform/analytics/telemetryService.ts @@ -11,7 +11,7 @@ import { } from '../common/types'; import { generateUuid } from '../common/uuid'; import { logger } from '../logging'; -import { POSTHOG_API_KEY, POSTHOG_HOST } from './constants'; +import { IS_POSTHOG_CONFIGURED, POSTHOG_API_KEY, POSTHOG_HOST } from './constants'; import { ITelemetryService, TelemetryEvent } from './types'; const USER_ID_STORAGE_KEY = 'deepnote-telemetry-anonymous-user-id'; @@ -70,7 +70,7 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva } private createClient(): void { - if (this.client || !this.isTelemetryEnabled()) { + if (this.client || !this.isPostHogConfigured() || !this.isTelemetryEnabled()) { return; } @@ -96,6 +96,10 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva } } + private isPostHogConfigured(): boolean { + return IS_POSTHOG_CONFIGURED; + } + private isTelemetryEnabled(): boolean { const telemetryLevel = workspace.getConfiguration('telemetry').get('telemetryLevel', 'all'); diff --git a/src/platform/analytics/telemetryService.unit.test.ts b/src/platform/analytics/telemetryService.unit.test.ts index bddcef0294..65fac3948e 100644 --- a/src/platform/analytics/telemetryService.unit.test.ts +++ b/src/platform/analytics/telemetryService.unit.test.ts @@ -40,6 +40,11 @@ suite('TelemetryService', () => { (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; + } + setup(() => { mockUserIdState = createMockPersistentState(''); mockDisposables = []; @@ -75,6 +80,7 @@ suite('TelemetryService', () => { test('activate should create client when telemetry is enabled', async () => { analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); + stubPostHogConfigured(analyticsService, true); await analyticsService.activate(); @@ -83,6 +89,19 @@ suite('TelemetryService', () => { assert.isDefined(client, 'PostHog client should be initialized'); }); + test('activate should not create client when PostHog is not configured', async () => { + analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); + 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 user ID and call PostHog capture on first trackEvent', async () => { (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).callsFake( (_key: string, defaultValue: string) => createMockPersistentState(defaultValue) @@ -90,6 +109,7 @@ suite('TelemetryService', () => { analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); + stubPostHogConfigured(analyticsService, true); await analyticsService.activate(); @@ -137,6 +157,7 @@ suite('TelemetryService', () => { test('settings change should destroy client when telemetry is disabled', async () => { analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); + stubPostHogConfigured(analyticsService, true); await analyticsService.activate(); @@ -157,6 +178,7 @@ suite('TelemetryService', () => { test('settings change should create client when telemetry is enabled', async () => { analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, false); + stubPostHogConfigured(analyticsService, true); await analyticsService.activate(); From 1799d1cf9cee78074d9a44925111106992bdc912 Mon Sep 17 00:00:00 2001 From: tomas Date: Sun, 19 Jul 2026 14:29:11 +0000 Subject: [PATCH 15/17] fix(telemetry): address PR review findings across telemetry events Reviewer-confirmed fixes from the multi-reviewer audit of the PostHog telemetry work: - Gate telemetry on env.isTelemetryEnabled and its change event, not just the raw telemetry.telemetryLevel setting (M02) - Send personless events with $process_person_profile: false (M15) - Bake a POSTHOG_CHANNEL ('pr' vs 'stable', 'development' locally) into builds and tag every event so PR-build telemetry is segmentable (M16) - execute_cell: read the authoritative top-level sql_integration_id and map the built-in DataFrame SQL id to 'duckdb' (M03, M04) - Track save/reset/delete_integration on storage success rather than the project-YAML sync result (M05) - Fire execute_notebook / add_block('code') after the command resolves (M10) - Report command completion as a tri-state outcome (completed | cancelled | failed) instead of a boolean (M11) - Emit select_environment once, after the kernel switch succeeds (M12) - Distinguish import (deepnote/jupyter) and create (toolbar/project_menu) sources and report the picked export format (M19) - Harden handleConfigChanged and guard palette-invoked tree commands against an undefined tree item (M18, M23) Test and hygiene: register chai-as-promised, use assert.isNotNull, stub the PostHog client factory, replace 'deepnote' literals with isDeepnoteNotebook(), delete the unused NoOpTelemetryService, extract flush constants / fix member ordering, and log the e2e retry error (M06, M07, M08, M17, M21, M22, M24). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ --- .github/workflows/cd.yml | 2 + build/esbuild/build.ts | 5 +- src/extension.node.ts | 1 - .../deepnoteCellExecutionAnalytics.ts | 15 +- .../deepnote/deepnoteExplorerView.ts | 205 +++++++++--------- .../deepnoteKernelAutoSelector.node.ts | 5 +- ...epnoteKernelAutoSelector.node.unit.test.ts | 4 +- .../deepnote/deepnoteMultiNotebookSplitter.ts | 4 +- .../integrations/integrationWebview.ts | 12 +- .../deepnote/openInDeepnoteHandler.node.ts | 4 + src/notebooks/notebookCommandListener.ts | 22 +- src/platform/analytics/constants.ts | 7 + .../analytics/noOpTelemetryService.ts | 14 -- src/platform/analytics/telemetryService.ts | 50 +++-- .../analytics/telemetryService.unit.test.ts | 36 ++- src/platform/analytics/telemetryWebService.ts | 4 +- test/e2e/suite/projectRename.e2e.test.ts | 4 +- 17 files changed, 227 insertions(+), 167 deletions(-) delete mode 100644 src/platform/analytics/noOpTelemetryService.ts diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index e6fad70eea..76aae9a442 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -60,6 +60,8 @@ jobs: run: npm run package env: POSTHOG_API_KEY: ${{ secrets.POSTHOG_API_KEY }} + # Tag PR-build telemetry as 'pr' so dogfood installs are segmentable out of production ('stable'). + POSTHOG_CHANNEL: ${{ github.event_name == 'pull_request' && 'pr' || 'stable' }} - name: Rename VSIX file run: | diff --git a/build/esbuild/build.ts b/build/esbuild/build.ts index a2b65c699e..76fb76ad68 100644 --- a/build/esbuild/build.ts +++ b/build/esbuild/build.ts @@ -232,9 +232,10 @@ function createConfig( } } if (target === 'desktop') { - // Bake the PostHog key in from the CI secret at build time; falls back to the placeholder locally (see constants.ts). + // 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_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'))) { diff --git a/src/extension.node.ts b/src/extension.node.ts index bc22ac6484..13460ca9ad 100644 --- a/src/extension.node.ts +++ b/src/extension.node.ts @@ -134,7 +134,6 @@ export function deactivate(): Thenable { // Make sure to shutdown anybody who needs it. if (activatedServiceContainer) { const registry = activatedServiceContainer.get(IAsyncDisposableRegistry); - if (registry) { return registry.dispose(); } diff --git a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts index bc2c0cdaef..a202720671 100644 --- a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts +++ b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts @@ -4,7 +4,9 @@ import { Disposable } 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 } from '../../platform/notebooks/deepnote/integrationTypes'; import { IDeepnoteNotebookManager } from '../types'; /** @@ -25,7 +27,7 @@ export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationS return; } - if (e.cell.notebook.notebookType !== 'deepnote') { + if (!isDeepnoteNotebook(e.cell.notebook)) { return; } @@ -35,10 +37,15 @@ export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationS const properties: Record = { cellType }; if (cellType === 'sql') { - const integrationId = - e.cell.metadata?.__deepnotePocket?.sql_integration_id ?? e.cell.metadata?.sql_integration_id; + // 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) { + 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; diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index 463b985769..6669d27f24 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -25,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. @@ -146,9 +149,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 false; + return 'cancelled'; } try { @@ -158,7 +161,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return false; + return 'failed'; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -166,7 +169,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return false; + return 'failed'; } const currentName = targetNotebook.name; @@ -175,7 +178,7 @@ export class DeepnoteExplorerView { const newName = await this.promptForNotebookName(currentName, existingNames); if (!newName || newName === currentName) { - return false; + return 'cancelled'; } // Flush the open document and re-read before rewriting, so we serialize the user's live cell @@ -185,7 +188,7 @@ export class DeepnoteExplorerView { l10n.t('Could not save "{0}" before renaming. The notebook was left unchanged.', currentName) ); - return false; + return 'failed'; } const freshData = await readDeepnoteProjectFile(fileUri); @@ -194,7 +197,7 @@ export class DeepnoteExplorerView { if (!freshTarget) { await window.showErrorMessage(l10n.t('Notebook not found')); - return false; + return 'failed'; } freshTarget.name = newName; @@ -204,18 +207,18 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook renamed to: {0}', newName)); - return true; + 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 false; + return 'failed'; } } - public async deleteNotebook(treeItem: DeepnoteTreeItem): Promise { + public async deleteNotebook(treeItem: DeepnoteTreeItem): Promise { if (!this.itemIsNotebookScoped(treeItem)) { - return false; + return 'cancelled'; } try { @@ -225,7 +228,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return false; + return 'failed'; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -233,7 +236,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return false; + return 'failed'; } const notebookName = targetNotebook.name; @@ -245,7 +248,7 @@ export class DeepnoteExplorerView { ); if (confirmation !== l10n.t('Delete')) { - return false; + return 'cancelled'; } // A single-notebook file's only non-init notebook is the file itself: delete the file. @@ -254,7 +257,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); await window.showInformationMessage(l10n.t('Notebook deleted: {0}', notebookName)); - return true; + return 'completed'; } // Legacy multi-notebook file: remove the notebook from the array. @@ -267,12 +270,12 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook deleted: {0}', notebookName)); - return true; + 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 false; + return 'failed'; } } @@ -286,9 +289,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 false; + return 'cancelled'; } try { @@ -298,7 +301,7 @@ export class DeepnoteExplorerView { if (!projectData?.project?.notebooks) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return false; + return 'failed'; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -306,7 +309,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return false; + return 'failed'; } const existingNames = await this.collectNotebookNamesForProject(treeItem.context.projectId); @@ -327,7 +330,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refreshNotebook(treeItem.context.projectId); await window.showInformationMessage(l10n.t('Notebook duplicated: {0}', newName)); - return true; + return 'completed'; } // Legacy multi-notebook file: append the duplicate in place (existing behavior). @@ -345,18 +348,18 @@ export class DeepnoteExplorerView { await window.showInformationMessage(l10n.t('Notebook duplicated: {0}', newName)); - return true; + 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 false; + return 'failed'; } } - public async renameProject(treeItem: DeepnoteTreeItem): Promise { - if (treeItem.extra.type !== DeepnoteTreeItemType.ProjectGroup) { - return false; + public async renameProject(treeItem: DeepnoteTreeItem): Promise { + if (treeItem?.extra?.type !== DeepnoteTreeItemType.ProjectGroup) { + return 'cancelled'; } const group = treeItem.extra.data; @@ -375,7 +378,7 @@ export class DeepnoteExplorerView { }); if (!newName || newName === currentName) { - return false; + return 'cancelled'; } try { @@ -392,7 +395,7 @@ export class DeepnoteExplorerView { ) ); - return false; + return 'failed'; } } @@ -426,12 +429,12 @@ export class DeepnoteExplorerView { await window.showInformationMessage(l10n.t('Project renamed to: {0}', newName)); } - return failedCount === 0; + 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 false; + return 'failed'; } } @@ -442,8 +445,8 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.OpenDeepnoteNotebook, async (context: DeepnoteTreeItemContext) => { - const completed = await this.openNotebook(context); - this.analytics.trackEvent({ eventName: 'open_notebook', properties: { completed } }); + const outcome = await this.openNotebook(context); + this.analytics.trackEvent({ eventName: 'open_notebook', properties: { outcome } }); }) ); @@ -457,74 +460,80 @@ export class DeepnoteExplorerView { this.extensionContext.subscriptions.push( commands.registerCommand(Commands.NewProject, async () => { - const completed = await this.newProject(); - this.analytics.trackEvent({ eventName: 'create_project', properties: { completed } }); + const outcome = await this.newProject(); + this.analytics.trackEvent({ eventName: 'create_project', properties: { outcome } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ImportNotebook, async () => { - const completed = await this.importNotebook(); - this.analytics.trackEvent({ eventName: 'import_notebook', properties: { completed } }); + const outcome = await this.importNotebook(); + this.analytics.trackEvent({ + eventName: 'import_notebook', + properties: { outcome, source: 'deepnote' } + }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.ImportJupyterNotebook, async () => { - const completed = await this.importJupyterNotebook(); - this.analytics.trackEvent({ eventName: 'import_notebook', properties: { completed } }); + const outcome = await this.importJupyterNotebook(); + this.analytics.trackEvent({ eventName: 'import_notebook', properties: { outcome, source: 'jupyter' } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.NewNotebook, async () => { - const completed = await this.newNotebook(); - this.analytics.trackEvent({ eventName: 'create_notebook', properties: { completed } }); + 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, async (treeItem: DeepnoteTreeItem) => { - const completed = await this.renameProject(treeItem); - this.analytics.trackEvent({ eventName: 'rename_project', properties: { completed } }); + const outcome = await this.renameProject(treeItem); + this.analytics.trackEvent({ eventName: 'rename_project', properties: { outcome } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.RenameNotebook, async (treeItem: DeepnoteTreeItem) => { - const completed = await this.renameNotebook(treeItem); - this.analytics.trackEvent({ eventName: 'rename_notebook', properties: { completed } }); + const outcome = await this.renameNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'rename_notebook', properties: { outcome } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DeleteNotebook, async (treeItem: DeepnoteTreeItem) => { - const completed = await this.deleteNotebook(treeItem); - this.analytics.trackEvent({ eventName: 'delete_notebook', properties: { completed } }); + const outcome = await this.deleteNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'delete_notebook', properties: { outcome } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.DuplicateNotebook, async (treeItem: DeepnoteTreeItem) => { - const completed = await this.duplicateNotebook(treeItem); - this.analytics.trackEvent({ eventName: 'duplicate_notebook', properties: { completed } }); + const outcome = await this.duplicateNotebook(treeItem); + this.analytics.trackEvent({ eventName: 'duplicate_notebook', properties: { outcome } }); }) ); this.extensionContext.subscriptions.push( commands.registerCommand(Commands.AddNotebookToProject, async (treeItem: DeepnoteTreeItem) => { - const completed = await this.addNotebookToProject(treeItem); - this.analytics.trackEvent({ eventName: 'create_notebook', properties: { completed } }); + 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, async (treeItem: DeepnoteTreeItem) => { - const completed = await this.exportNotebook(treeItem); + const { outcome, format } = await this.exportNotebook(treeItem); this.analytics.trackEvent({ eventName: 'export_notebook', - properties: { completed, format: 'jupyter' } + properties: { outcome, ...(format ? { format } : {}) } }); }) ); @@ -536,8 +545,8 @@ export class DeepnoteExplorerView { */ private itemIsNotebookScoped(treeItem: DeepnoteTreeItem): boolean { return ( - treeItem.extra.type === DeepnoteTreeItemType.ProjectFile || - treeItem.extra.type === DeepnoteTreeItemType.Notebook + treeItem?.extra?.type === DeepnoteTreeItemType.ProjectFile || + treeItem?.extra?.type === DeepnoteTreeItemType.Notebook ); } @@ -558,7 +567,7 @@ export class DeepnoteExplorerView { * multi-notebook file's in-file child. */ private itemIsSingleNotebookFile(treeItem: DeepnoteTreeItem, projectData: DeepnoteFile): boolean { - if (treeItem.extra.type !== DeepnoteTreeItemType.ProjectFile) { + if (treeItem?.extra?.type !== DeepnoteTreeItemType.ProjectFile) { return false; } @@ -704,7 +713,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); @@ -714,18 +723,18 @@ export class DeepnoteExplorerView { preserveFocus: false }); - return true; + return 'completed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(`Failed to open notebook: ${errorMessage}`); - return false; + return 'failed'; } } private async openFile(treeItem: DeepnoteTreeItem): Promise { - if (treeItem.extra.type !== DeepnoteTreeItemType.ProjectFile) { + if (treeItem?.extra?.type !== DeepnoteTreeItemType.ProjectFile) { return; } @@ -784,7 +793,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?'), @@ -796,7 +805,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return false; + return 'cancelled'; } const projectName = await window.showInputBox({ @@ -812,7 +821,7 @@ export class DeepnoteExplorerView { }); if (!projectName) { - return false; + return 'cancelled'; } try { @@ -825,7 +834,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 false; + return 'failed'; } catch { // File doesn't exist, continue } @@ -880,23 +889,23 @@ export class DeepnoteExplorerView { preview: false }); - return true; + 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 false; + 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 false; + return 'cancelled'; } const document = activeEditor.notebook; @@ -919,12 +928,12 @@ export class DeepnoteExplorerView { await window.showInformationMessage(l10n.t('Created new notebook: {0}', result.name)); } - return result !== null; + 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 false; + return 'failed'; } } @@ -1004,7 +1013,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?'), @@ -1016,7 +1025,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return false; + return 'cancelled'; } const fileUris = await window.showOpenDialog({ @@ -1030,7 +1039,7 @@ export class DeepnoteExplorerView { }); if (!fileUris || fileUris.length === 0) { - return false; + return 'cancelled'; } try { @@ -1050,14 +1059,14 @@ export class DeepnoteExplorerView { l10n.t('A file named "{0}" already exists in this workspace.', fileName) ); - return false; + return 'failed'; } catch { // File doesn't exist, continue } } if (!(await this.checkJupyterImportTargetsAvailable(jupyterUris, workspaceFolder.uri))) { - return false; + return 'failed'; } // Import deepnote files @@ -1082,17 +1091,17 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); - return numberOfNotebooks > 0; + return numberOfNotebooks > 0 ? 'completed' : 'failed'; } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; await window.showErrorMessage(`Failed to import notebook: ${errorMessage}`); - return false; + 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?'), @@ -1104,7 +1113,7 @@ export class DeepnoteExplorerView { await commands.executeCommand('vscode.openFolder'); } - return false; + return 'cancelled'; } const fileUris = await window.showOpenDialog({ @@ -1118,14 +1127,14 @@ export class DeepnoteExplorerView { }); if (!fileUris || fileUris.length === 0) { - return false; + return 'cancelled'; } try { const workspaceFolder = workspace.workspaceFolders[0]; if (!(await this.checkJupyterImportTargetsAvailable(fileUris, workspaceFolder.uri))) { - return false; + return 'failed'; } const failedCount = await this.convertJupyterUrisToDeepnoteFiles(fileUris, workspaceFolder.uri); @@ -1142,19 +1151,19 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); - return numberOfNotebooks > 0; + 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 false; + return 'failed'; } } - private async addNotebookToProject(treeItem: DeepnoteTreeItem): Promise { - if (treeItem.extra.type !== DeepnoteTreeItemType.ProjectGroup) { - return false; + private async addNotebookToProject(treeItem: DeepnoteTreeItem): Promise { + if (treeItem?.extra?.type !== DeepnoteTreeItemType.ProjectGroup) { + return 'cancelled'; } const group = treeItem.extra.data; @@ -1163,7 +1172,7 @@ export class DeepnoteExplorerView { if (!sourceFile) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return false; + return 'failed'; } try { @@ -1177,19 +1186,19 @@ export class DeepnoteExplorerView { await window.showInformationMessage(l10n.t('Created new notebook: {0}', result.name)); } - return result !== null; + 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 false; + 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 false; + return { outcome: 'cancelled' }; } try { @@ -1198,7 +1207,7 @@ export class DeepnoteExplorerView { }); if (!format) { - return false; + return { outcome: 'cancelled' }; } const fileUri = Uri.file(treeItem.context.filePath); @@ -1207,7 +1216,7 @@ export class DeepnoteExplorerView { if (!projectData?.project) { await window.showErrorMessage(l10n.t('Invalid Deepnote file format')); - return false; + return { outcome: 'failed' }; } const outputFolder = await window.showOpenDialog({ @@ -1219,7 +1228,7 @@ export class DeepnoteExplorerView { }); if (!outputFolder?.length) { - return false; + return { outcome: 'cancelled' }; } const targetNotebook = this.resolveTargetNotebook(treeItem, projectData); @@ -1227,7 +1236,7 @@ export class DeepnoteExplorerView { if (!targetNotebook) { await window.showErrorMessage(l10n.t('Notebook not found')); - return false; + return { outcome: 'failed' }; } const filteredProject = { @@ -1262,7 +1271,7 @@ export class DeepnoteExplorerView { ); if (result !== overwrite) { - return false; + return { outcome: 'cancelled' }; } } @@ -1273,12 +1282,12 @@ export class DeepnoteExplorerView { await window.showInformationMessage(l10n.t('Exported 1 notebook successfully')); - return true; + 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 false; + return { outcome: 'failed' }; } } } diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index fd8fe6449d..8c0b93cc4a 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts @@ -44,7 +44,6 @@ 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'; @@ -99,8 +98,7 @@ 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(ITelemetryService) private readonly analytics: ITelemetryService + @inject(IServerHandleRegistry) private readonly serverHandleRegistry: IServerHandleRegistry ) {} public activate() { @@ -768,7 +766,6 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, Cancellation.throwIfCanceled(token); await this.notebookEnvironmentMapper.setEnvironmentForNotebook(notebook.uri, selectedEnvironment.id); - this.analytics.trackEvent({ eventName: 'select_environment' }); const result = await this.setupKernelForEnvironment(notebook, selectedEnvironment, notebookKey, token); diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts index 7607d20a6c..d5f71bc77c 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts @@ -2,7 +2,6 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import { DeepnoteKernelAutoSelector } from './deepnoteKernelAutoSelector.node'; -import { ITelemetryService } from '../../platform/analytics/types'; import { createMockChildProcess } from '../../kernels/deepnote/deepnoteTestHelpers.node'; import { ServerHandleRegistry } from '../../kernels/deepnote/deepnoteServerHandleRegistry.node'; import { @@ -142,8 +141,7 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { instance(mockNotebookEnvironmentMapper), instance(mockOutputChannel), instance(mockToolkitInstaller), - registry, - instance(mock()) + registry ); }); diff --git a/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts b/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts index 0e9cbda899..112a4d2a3b 100644 --- a/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts +++ b/src/notebooks/deepnote/deepnoteMultiNotebookSplitter.ts @@ -24,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; @@ -36,8 +38,6 @@ export class DeepnoteMultiNotebookSplitter { private readonly refreshTree: () => void; - private readonly analytics: ITelemetryService; - constructor( envMapper: IDeepnoteNotebookEnvironmentMapper | undefined, refreshTree: () => void, diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index 15dae3772f..9447591287 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -691,7 +691,9 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { }); } - return persisted; + // 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({ @@ -735,7 +737,9 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { }); } - return persisted; + // 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({ @@ -774,7 +778,9 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { }); } - return persisted; + // 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({ diff --git a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts index 355209a0ec..a224c126f1 100644 --- a/src/notebooks/deepnote/openInDeepnoteHandler.node.ts +++ b/src/notebooks/deepnote/openInDeepnoteHandler.node.ts @@ -46,6 +46,7 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { const activeEditor = window.activeTextEditor; if (!activeEditor) { void window.showErrorMessage('Please open a .deepnote file first'); + return false; } @@ -54,6 +55,7 @@ export class OpenInDeepnoteHandler implements IExtensionSyncActivationService { if (!fileUri.fsPath.endsWith('.deepnote')) { void window.showErrorMessage('This command only works with .deepnote files'); + return false; } @@ -65,6 +67,7 @@ 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 false; } } @@ -78,6 +81,7 @@ 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 false; } diff --git a/src/notebooks/notebookCommandListener.ts b/src/notebooks/notebookCommandListener.ts index 1a1b8da0ea..d0fc13908f 100644 --- a/src/notebooks/notebookCommandListener.ts +++ b/src/notebooks/notebookCommandListener.ts @@ -29,7 +29,7 @@ import { IServiceContainer } from '../platform/ioc/types'; import { endCellAndDisplayErrorsInCell } from '../kernels/execution/helpers'; import { chainWithPendingUpdates } from '../kernels/execution/notebookUpdater'; import { IDataScienceErrorHandler } from '../kernels/errors/types'; -import { getNotebookMetadata } from '../platform/common/utils'; +import { getNotebookMetadata, isDeepnoteNotebook } from '../platform/common/utils'; import { KernelConnector } from './controllers/kernelConnector'; import { IControllerRegistration } from './controllers/types'; import { IExtensionSyncActivationService } from '../platform/activation/types'; @@ -117,10 +117,12 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens private runAllCells() { const editor = window.activeNotebookEditor; if (editor) { - if (editor.notebook.notebookType === 'deepnote') { - this.analytics.trackEvent({ eventName: 'execute_notebook' }); - } - commands.executeCommand('notebook.execute').then(noop, noop); + const isDeepnote = isDeepnoteNotebook(editor.notebook); + commands.executeCommand('notebook.execute').then(() => { + if (isDeepnote) { + this.analytics.trackEvent({ eventName: 'execute_notebook' }); + } + }, noop); } } @@ -148,10 +150,12 @@ export class NotebookCommandListener implements INotebookCommandHandler, IExtens private addCellBelow() { const editor = window.activeNotebookEditor; if (editor) { - if (editor.notebook.notebookType === 'deepnote') { - this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'code' } }); - } - commands.executeCommand('notebook.cell.insertCodeCellBelow').then(noop, noop); + const isDeepnote = isDeepnoteNotebook(editor.notebook); + commands.executeCommand('notebook.cell.insertCodeCellBelow').then(() => { + if (isDeepnote) { + this.analytics.trackEvent({ eventName: 'add_block', properties: { blockType: 'code' } }); + } + }, noop); } } diff --git a/src/platform/analytics/constants.ts b/src/platform/analytics/constants.ts index 9d7afe74fd..c2396f1795 100644 --- a/src/platform/analytics/constants.ts +++ b/src/platform/analytics/constants.ts @@ -2,12 +2,19 @@ // 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. Left undefined in local +// builds, where it 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. diff --git a/src/platform/analytics/noOpTelemetryService.ts b/src/platform/analytics/noOpTelemetryService.ts deleted file mode 100644 index db39a2d7de..0000000000 --- a/src/platform/analytics/noOpTelemetryService.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { ITelemetryService, TelemetryEvent } from './types'; - -/** - * No-op telemetry service for use in tests. - */ -export class NoOpTelemetryService implements ITelemetryService { - public async dispose(): Promise { - // No-op - } - - public trackEvent(_event: TelemetryEvent): void { - // No-op - } -} diff --git a/src/platform/analytics/telemetryService.ts b/src/platform/analytics/telemetryService.ts index 991764adc6..e56b9092c9 100644 --- a/src/platform/analytics/telemetryService.ts +++ b/src/platform/analytics/telemetryService.ts @@ -1,6 +1,6 @@ import { inject, injectable } from 'inversify'; import { PostHog } from 'posthog-node'; -import { workspace } from 'vscode'; +import { env, workspace } from 'vscode'; import { IExtensionSyncActivationService } from '../activation/types'; import { @@ -11,10 +11,12 @@ import { } from '../common/types'; import { generateUuid } from '../common/uuid'; import { logger } from '../logging'; -import { IS_POSTHOG_CONFIGURED, POSTHOG_API_KEY, POSTHOG_HOST } from './constants'; +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() @@ -30,11 +32,15 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva ) { asyncDisposables.push(this); this.client = null; - this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, generateUuid()); + this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, 'anonymous'); } public async activate(): Promise { try { + if (!this.userIdState.value) { + await this.userIdState.updateValue(generateUuid()); + } + this.createClient(); } catch (error) { logger.debug(`TelemetryService activation error: ${error}`); @@ -45,7 +51,8 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva if (e.affectsConfiguration('telemetry') || e.affectsConfiguration('deepnote.telemetry')) { this.handleConfigChanged(); } - }) + }), + env.onDidChangeTelemetryEnabled(() => this.handleConfigChanged()) ); } @@ -62,7 +69,7 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva this.client.capture({ distinctId: this.userIdState.value, event: eventName, - properties + properties: { ...properties, channel: POSTHOG_CHANNEL, $process_person_profile: false } }); } catch (ex) { logger.debug(`PostHog analytics error: ${ex}`); @@ -75,8 +82,8 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva } this.client = new PostHog(POSTHOG_API_KEY, { - flushAt: 20, - flushInterval: 30000, + flushAt: POSTHOG_FLUSH_AT, + flushInterval: POSTHOG_FLUSH_INTERVAL, host: POSTHOG_HOST }); } @@ -96,11 +103,29 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva } } + 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') { @@ -109,15 +134,4 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva return workspace.getConfiguration('deepnote').get('telemetry.enabled', true); } - - private handleConfigChanged(): void { - if (this.isTelemetryEnabled()) { - this.createClient(); - } else { - this.destroyClient().catch((error) => { - logger.error(`Failed to destroy PostHog client: ${error}`); - this.client = null; - }); - } - } } diff --git a/src/platform/analytics/telemetryService.unit.test.ts b/src/platform/analytics/telemetryService.unit.test.ts index 65fac3948e..c83939479d 100644 --- a/src/platform/analytics/telemetryService.unit.test.ts +++ b/src/platform/analytics/telemetryService.unit.test.ts @@ -1,4 +1,5 @@ -import { assert } from 'chai'; +import { assert, use } from 'chai'; +import chaiAsPromised from 'chai-as-promised'; import * as sinon from 'sinon'; import { @@ -9,6 +10,8 @@ import { } from '../common/types'; import { TelemetryService } from './telemetryService'; +use(chaiAsPromised); + suite('TelemetryService', () => { let analyticsService: TelemetryService; let mockDisposables: IDisposableRegistry; @@ -45,6 +48,23 @@ suite('TelemetryService', () => { (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 = []; @@ -81,12 +101,13 @@ suite('TelemetryService', () => { analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); stubPostHogConfigured(analyticsService, true); + stubClientFactory(analyticsService); await analyticsService.activate(); const client = getPostHogClient(analyticsService); - assert.isDefined(client, 'PostHog client should be initialized'); + assert.isNotNull(client, 'PostHog client should be initialized'); }); test('activate should not create client when PostHog is not configured', async () => { @@ -110,6 +131,7 @@ suite('TelemetryService', () => { analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); stubPostHogConfigured(analyticsService, true); + stubClientFactory(analyticsService); await analyticsService.activate(); @@ -124,7 +146,7 @@ suite('TelemetryService', () => { const client = getPostHogClient(analyticsService); - assert.isDefined(client, 'PostHog client should be initialized'); + assert.isNotNull(client, 'PostHog client should be initialized'); const captureStub = sinon.stub(); client.capture = captureStub; @@ -135,7 +157,7 @@ suite('TelemetryService', () => { assert.deepStrictEqual(captureStub.firstCall.args[0], { distinctId: generatedId, event: 'execute_notebook', - properties: undefined + properties: { channel: 'development', $process_person_profile: false } }); }); @@ -158,12 +180,13 @@ suite('TelemetryService', () => { analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, true); stubPostHogConfigured(analyticsService, true); + stubClientFactory(analyticsService); await analyticsService.activate(); const client = getPostHogClient(analyticsService); - assert.isDefined(client, 'Client should be created initially'); + assert.isNotNull(client, 'Client should be created initially'); const shutdownStub = sinon.stub().resolves(); client.shutdown = shutdownStub; @@ -179,6 +202,7 @@ suite('TelemetryService', () => { analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); stubTelemetryEnabled(analyticsService, false); stubPostHogConfigured(analyticsService, true); + stubClientFactory(analyticsService); await analyticsService.activate(); @@ -188,7 +212,7 @@ suite('TelemetryService', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any (analyticsService as any).handleConfigChanged(); - assert.isDefined(getPostHogClient(analyticsService), 'Client should be created when telemetry is enabled'); + assert.isNotNull(getPostHogClient(analyticsService), 'Client should be created when telemetry is enabled'); }); test('dispose should not throw even when client is not initialized', async () => { diff --git a/src/platform/analytics/telemetryWebService.ts b/src/platform/analytics/telemetryWebService.ts index 9dd7b611d9..ddabe87f1b 100644 --- a/src/platform/analytics/telemetryWebService.ts +++ b/src/platform/analytics/telemetryWebService.ts @@ -4,11 +4,11 @@ import { ITelemetryService, TelemetryEvent } from './types'; @injectable() export class TelemetryWebService implements ITelemetryService { - public trackEvent(_event: TelemetryEvent): void { + public async dispose(): Promise { // No-op for web } - public async dispose(): Promise { + public trackEvent(_event: TelemetryEvent): void { // No-op for web } } diff --git a/test/e2e/suite/projectRename.e2e.test.ts b/test/e2e/suite/projectRename.e2e.test.ts index 2a2c455672..529a7cf810 100644 --- a/test/e2e/suite/projectRename.e2e.test.ts +++ b/test/e2e/suite/projectRename.e2e.test.ts @@ -57,8 +57,10 @@ async function leaveUnsavedCellEdit(): Promise { await line.click(); return true; - } catch { + } catch (error) { // Stale reference (the cell re-rendered) or not yet clickable — re-locate and retry. + console.warn('[deepnote-e2e] locate/click notebook code cell (retrying):', error); + return false; } }, From 5c9cc69b1420f20f56ed889f6924e8e4eae8a9c2 Mon Sep 17 00:00:00 2001 From: tomas Date: Sun, 19 Jul 2026 19:46:37 +0000 Subject: [PATCH 16/17] fix(deepnote): localize error message for notebook import failures Updated the error handling in the DeepnoteExplorerView to use localized strings for displaying import failure messages. This change enhances user experience by providing clearer feedback in the user's preferred language. --- src/notebooks/deepnote/deepnoteExplorerView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index 6669d27f24..16570d9d96 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -1095,7 +1095,7 @@ export class DeepnoteExplorerView { } 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'; } From 11cf5c6589308940ce53f84cf06109880540a106 Mon Sep 17 00:00:00 2001 From: tomas Date: Sun, 19 Jul 2026 21:02:04 +0000 Subject: [PATCH 17/17] feat(telemetry): typed events, common properties, and stable anonymous id Follow-ups from comparing the new PostHog analytics service to the inherited @vscode/extension-telemetry framework: - Type every event's properties. TelemetryEventProperties maps each event name to its property shape and trackEvent is generic over it, so outcome/source/format/cellType payloads are compile-checked at every call site (previously a loose Record). - Attach common properties to every event (channel, extensionVersion, platform, sessionId) so metrics can be segmented by build/version/ platform/session. - Persist the anonymous distinctId: default the stored id to '' so activate() generates and persists a per-install UUID once (stable id, no per-session churn) while events stay personless. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AGod1izfU9yA8ZQfxD5yeZ --- .../deepnoteCellExecutionAnalytics.ts | 2 +- .../integrations/integrationWebview.ts | 18 ++- src/platform/analytics/telemetryService.ts | 22 +++- .../analytics/telemetryService.unit.test.ts | 109 +++++++++++++----- src/platform/analytics/types.ts | 49 +++++++- 5 files changed, 155 insertions(+), 45 deletions(-) diff --git a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts index a202720671..372798339f 100644 --- a/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts +++ b/src/notebooks/deepnote/deepnoteCellExecutionAnalytics.ts @@ -34,7 +34,7 @@ export class DeepnoteCellExecutionAnalytics implements IExtensionSyncActivationS const languageId = e.cell.document.languageId; const cellType = languageId === 'sql' ? 'sql' : languageId === 'markdown' ? 'markdown' : 'code'; - const properties: Record = { cellType }; + 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 diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index 9447591287..e3bdf22882 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -3,7 +3,7 @@ import { commands, Disposable, l10n, Uri, ViewColumn, WebviewPanel, window } fro import { BigQueryAuthMethods } from '@deepnote/database-integrations'; -import { ITelemetryService, TelemetryEventName } from '../../../platform/analytics/types'; +import { ITelemetryService, TelemetryEvent } 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'; @@ -564,8 +564,20 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } } - private trackIntegrationEvent(eventName: TelemetryEventName, integrationType: string | undefined): void { - this.analytics.trackEvent({ eventName, properties: { integrationType: integrationType ?? 'unknown' } }); + private trackIntegrationEvent< + E extends + | 'authenticate_integration' + | 'configure_integration' + | 'delete_integration' + | 'reset_integration' + | 'save_integration' + >(eventName: E, integrationType: string | undefined): void { + // All five integration events share the { integrationType } shape; the cast bridges the + // generic event name to the discriminated TelemetryEvent union (unprovable for an abstract E). + this.analytics.trackEvent({ + eventName, + properties: { integrationType: integrationType ?? 'unknown' } + } as TelemetryEvent); } /** Handle messages from the webview; mirrors the `WebviewOutboundMessage` union in `src/webviews/webview-side/integrations/types.ts`. */ diff --git a/src/platform/analytics/telemetryService.ts b/src/platform/analytics/telemetryService.ts index e56b9092c9..f4fa0857c3 100644 --- a/src/platform/analytics/telemetryService.ts +++ b/src/platform/analytics/telemetryService.ts @@ -3,6 +3,7 @@ import { PostHog } from 'posthog-node'; import { env, workspace } from 'vscode'; import { IExtensionSyncActivationService } from '../activation/types'; +import { IApplicationEnvironment } from '../common/application/types'; import { IAsyncDisposableRegistry, IDisposableRegistry, @@ -23,16 +24,27 @@ const POSTHOG_SHUTDOWN_TIMEOUT = 5000; 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 userIdState: IPersistentState; constructor( @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IPersistentStateFactory) private readonly stateFactory: IPersistentStateFactory, - @inject(IAsyncDisposableRegistry) asyncDisposables: IAsyncDisposableRegistry + @inject(IAsyncDisposableRegistry) asyncDisposables: IAsyncDisposableRegistry, + @inject(IApplicationEnvironment) appEnvironment: IApplicationEnvironment ) { asyncDisposables.push(this); this.client = null; - this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, 'anonymous'); + this.userIdState = this.stateFactory.createGlobalPersistentState(USER_ID_STORAGE_KEY, ''); + this.commonProperties = { + channel: POSTHOG_CHANNEL, + extensionVersion: appEnvironment.extensionVersion, + platform: process.platform, + sessionId: env.sessionId + }; } public async activate(): Promise { @@ -60,7 +72,7 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva await this.destroyClient(); } - public trackEvent({ eventName, properties }: TelemetryEvent): void { + public trackEvent(event: TelemetryEvent): void { try { if (!this.client || !this.userIdState) { return; @@ -68,8 +80,8 @@ export class TelemetryService implements ITelemetryService, IExtensionSyncActiva this.client.capture({ distinctId: this.userIdState.value, - event: eventName, - properties: { ...properties, channel: POSTHOG_CHANNEL, $process_person_profile: false } + event: event.eventName, + properties: { ...event.properties, ...this.commonProperties, $process_person_profile: false } }); } catch (ex) { logger.debug(`PostHog analytics error: ${ex}`); diff --git a/src/platform/analytics/telemetryService.unit.test.ts b/src/platform/analytics/telemetryService.unit.test.ts index c83939479d..16cdfcf84d 100644 --- a/src/platform/analytics/telemetryService.unit.test.ts +++ b/src/platform/analytics/telemetryService.unit.test.ts @@ -2,6 +2,7 @@ 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, @@ -18,6 +19,7 @@ suite('TelemetryService', () => { let mockStateFactory: IPersistentStateFactory; let mockAsyncDisposableRegistry: IAsyncDisposableRegistry; let mockUserIdState: IPersistentState; + let mockAppEnv: IApplicationEnvironment; function createMockPersistentState(initialValue: string): IPersistentState { let storedValue = initialValue; @@ -68,6 +70,7 @@ suite('TelemetryService', () => { setup(() => { mockUserIdState = createMockPersistentState(''); mockDisposables = []; + mockAppEnv = { extensionVersion: '1.2.3' }; mockStateFactory = { createGlobalPersistentState: sinon.stub().returns(mockUserIdState), createWorkspacePersistentState: sinon.stub().returns(mockUserIdState) @@ -79,13 +82,23 @@ suite('TelemetryService', () => { }); test('should create instance without errors', () => { - analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); + 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); + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); stubTelemetryEnabled(analyticsService, false); await analyticsService.activate(); @@ -98,7 +111,12 @@ suite('TelemetryService', () => { }); test('activate should create client when telemetry is enabled', async () => { - analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); stubTelemetryEnabled(analyticsService, true); stubPostHogConfigured(analyticsService, true); stubClientFactory(analyticsService); @@ -111,7 +129,12 @@ suite('TelemetryService', () => { }); test('activate should not create client when PostHog is not configured', async () => { - analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); stubTelemetryEnabled(analyticsService, true); stubPostHogConfigured(analyticsService, false); @@ -123,49 +146,56 @@ suite('TelemetryService', () => { ); }); - test('should generate user ID and call PostHog capture on first trackEvent', async () => { - (mockStateFactory.createGlobalPersistentState as sinon.SinonStub).callsFake( - (_key: string, defaultValue: string) => createMockPersistentState(defaultValue) - ); + 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); + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); stubTelemetryEnabled(analyticsService, true); stubPostHogConfigured(analyticsService, true); - stubClientFactory(analyticsService); + const fakeClient = stubClientFactory(analyticsService); await analyticsService.activate(); - const createStateSpy = mockStateFactory.createGlobalPersistentState as sinon.SinonStub; - - assert.isTrue(createStateSpy.calledOnce, 'Should create persistent state'); - - const generatedId = createStateSpy.firstCall.args[1]; + // 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' + ); - assert.isString(generatedId); - assert.isNotEmpty(generatedId, 'Generated user ID should not be empty'); + const persistedId = userIdState.value; - const client = getPostHogClient(analyticsService); + assert.isNotEmpty(persistedId, 'Persisted user ID should not be empty'); - assert.isNotNull(client, 'PostHog client should be initialized'); + analyticsService.trackEvent({ eventName: 'execute_notebook' }); - const captureStub = sinon.stub(); - client.capture = captureStub; + assert.isTrue(fakeClient.capture.calledOnce, 'PostHog capture should be called'); - analyticsService.trackEvent({ eventName: 'execute_notebook' }); + const captured = fakeClient.capture.firstCall.args[0]; - assert.isTrue(captureStub.calledOnce, 'PostHog capture should be called'); - assert.deepStrictEqual(captureStub.firstCall.args[0], { - distinctId: generatedId, - event: 'execute_notebook', - properties: { channel: 'development', $process_person_profile: false } - }); + 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); + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); stubTelemetryEnabled(analyticsService, true); await analyticsService.activate(); @@ -177,7 +207,12 @@ suite('TelemetryService', () => { }); test('settings change should destroy client when telemetry is disabled', async () => { - analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); stubTelemetryEnabled(analyticsService, true); stubPostHogConfigured(analyticsService, true); stubClientFactory(analyticsService); @@ -199,7 +234,12 @@ suite('TelemetryService', () => { }); test('settings change should create client when telemetry is enabled', async () => { - analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); stubTelemetryEnabled(analyticsService, false); stubPostHogConfigured(analyticsService, true); stubClientFactory(analyticsService); @@ -216,7 +256,12 @@ suite('TelemetryService', () => { }); test('dispose should not throw even when client is not initialized', async () => { - analyticsService = new TelemetryService(mockDisposables, mockStateFactory, mockAsyncDisposableRegistry); + analyticsService = new TelemetryService( + mockDisposables, + mockStateFactory, + mockAsyncDisposableRegistry, + mockAppEnv + ); await assert.isFulfilled(analyticsService.dispose()); }); diff --git a/src/platform/analytics/types.ts b/src/platform/analytics/types.ts index 1695978eed..80a375a83f 100644 --- a/src/platform/analytics/types.ts +++ b/src/platform/analytics/types.ts @@ -27,13 +27,54 @@ export type TelemetryEventName = | 'toggle_snapshots' | 'update_environment'; -export interface TelemetryEvent { - eventName: TelemetryEventName; - properties?: Record; +/** 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 }; + configure_integration: { integrationType: string }; + create_environment: { hasDescription: boolean; hasPackages: boolean }; + 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: { completed: boolean }; + open_notebook: { outcome: CommandOutcome }; + rename_notebook: { outcome: CommandOutcome }; + rename_project: { outcome: CommandOutcome }; + reset_integration: { integrationType: string }; + save_integration: { integrationType: string }; + select_environment: undefined; + split_notebook: { completed: boolean; notebookCount: number }; + 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?: undefined } + : { eventName: E; properties: TelemetryEventProperties[E] } + : never; + export const ITelemetryService = Symbol('ITelemetryService'); export interface ITelemetryService extends IAsyncDisposable { - trackEvent(event: TelemetryEvent): void; + trackEvent(event: TelemetryEvent): void; }