From e89b6b63886b831ce41f98cdfc4e57bcf4693d4f Mon Sep 17 00:00:00 2001 From: tomas Date: Sun, 29 Mar 2026 21:07:03 +0000 Subject: [PATCH 1/9] 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 2/9] 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 3/9] 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 4/9] 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 5/9] 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 6/9] 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 7/9] 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 8/9] 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 9/9] 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);