From e89b6b63886b831ce41f98cdfc4e57bcf4693d4f Mon Sep 17 00:00:00 2001 From: tomas Date: Sun, 29 Mar 2026 21:07:03 +0000 Subject: [PATCH 01/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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();