Add Telemetry for VSC Notebooks (#12542)

DonJayamanne · web-flow · commit 6afc0839101f · 2020-06-24T17:14:10.000-07:00
* Added some missing telemetry for VS Code Notebooks  
* Track the type of notebook used in various operations (running cell, interrupting kernels, etc, determine whether user is performing such operations while in Native/Custom/Old editor)  
* Added a type for INobteookEditor (temporary, until VSC Notebook is stable)  
* Ensure users cannot opt into VSC Notebook Editor experiment when in VSC Stable.
diff --git a/package.nls.json b/package.nls.json
@@ -536,5 +536,6 @@
     "DataScience.reloadCustomEditor": "Please reload VS Code to use the custom editor API",
     "DataScience.reloadVSCodeNotebookEditor": "Please reload VS Code to use the Notebook Editor",
     "DataScience.step": "Run next line (F10)",
-    "DataScience.usingPreviewNotebookWithOtherNotebookWarning": "Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks."
+    "DataScience.usingPreviewNotebookWithOtherNotebookWarning": "Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks.",
+    "DataScience.previewNotebookOnlySupportedInVSCInsiders": "The Preview Notebook Editor is supported only in the Insiders version of Visual Studio Code."
 }
diff --git a/src/client/common/experiments/telemetry.ts b/src/client/common/experiments/telemetry.ts
@@ -9,7 +9,9 @@ import { sendTelemetryEvent, setSharedProperty } from '../../telemetry';
 export class ExperimentationTelemetry implements IExperimentationTelemetry {
     public setSharedProperty(name: string, value: string): void {
         // Add the shared property to all telemetry being sent, not just events being sent by the experimentation package.
-        setSharedProperty(name, value);
+        // We are not in control of these props, just cast to `any`, i.e. we cannot strongly type these external props.
+        // tslint:disable-next-line: no-any
+        setSharedProperty(name as any, value as any);
     }
 
     public postEvent(eventName: string, properties: Map<string, string>): void {
diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -985,6 +985,10 @@ export namespace DataScience {
         'DataScience.usingPreviewNotebookWithOtherNotebookWarning',
         'Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks.'
     );
+    export const previewNotebookOnlySupportedInVSCInsiders = localize(
+        'DataScience.previewNotebookOnlySupportedInVSCInsiders',
+        'The Preview Notebook Editor is supported only in the Insiders version of Visual Studio Code.'
+    );
 }
 
 export namespace StartPage {
diff --git a/src/client/datascience/activation.ts b/src/client/datascience/activation.ts
@@ -9,7 +9,7 @@ import '../common/extensions';
 import { IPythonDaemonExecutionService, IPythonExecutionFactory } from '../common/process/types';
 import { IDisposableRegistry } from '../common/types';
 import { debounceAsync, swallowExceptions } from '../common/utils/decorators';
-import { sendTelemetryEvent } from '../telemetry';
+import { sendTelemetryEvent, setSharedProperty } from '../telemetry';
 import { JupyterDaemonModule, Telemetry } from './constants';
 import { ActiveEditorContextService } from './context/activeEditorContext';
 import { JupyterInterpreterService } from './jupyter/interpreter/jupyterInterpreterService';
@@ -37,9 +37,10 @@ export class Activation implements IExtensionSingleActivationService {
         this.tracker.startTracking();
     }
 
-    private onDidOpenNotebookEditor(_: INotebookEditor) {
+    private onDidOpenNotebookEditor(e: INotebookEditor) {
         this.notebookOpened = true;
         this.PreWarmDaemonPool().ignoreErrors();
+        setSharedProperty('ds_notebookeditor', e.type);
         sendTelemetryEvent(Telemetry.OpenNotebookAll);
         // Warm up our selected interpreter for the extension
         this.jupyterInterpreterService.setInitialInterpreter().ignoreErrors();
diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts
@@ -395,6 +395,21 @@ export enum NativeMouseCommandTelemetry {
     ToggleVariableExplorer = 'DATASCIENCE.NATIVE.MOUSE.TOGGLE_VARIABLE_EXPLORER'
 }
 
+/**
+ * Notebook editing in VS Code Notebooks is handled by VSC.
+ * There's no way for us to know whether user added a cell using keyboard or not.
+ * Similarly a cell could have been added as part of an undo operation.
+ * All we know is previously user had n # of cells and now they have m # of cells.
+ */
+export enum VSCodeNativeTelemetry {
+    AddCell = 'DATASCIENCE.VSCODE_NATIVE.INSERT_CELL',
+    RunAllCells = 'DATASCIENCE.VSCODE_NATIVE.RUN_ALL',
+    DeleteCell = 'DATASCIENCE.VSCODE_NATIVE.DELETE_CELL',
+    MoveCell = 'DATASCIENCE.VSCODE_NATIVE.MOVE_CELL',
+    ChangeToCode = 'DATASCIENCE.VSCODE_NATIVE.CHANGE_TO_CODE', // Not guaranteed to work see, https://github.com/microsoft/vscode/issues/100042
+    ChangeToMarkdown = 'DATASCIENCE.VSCODE_NATIVE.CHANGE_TO_MARKDOWN' // Not guaranteed to work see, https://github.com/microsoft/vscode/issues/100042
+}
+
 export namespace HelpLinks {
     export const PythonInteractiveHelpLink = 'https://aka.ms/pyaiinstall';
     export const JupyterDataRateHelpLink = 'https://aka.ms/AA5ggm0'; // This redirects here: https://jupyter-notebook.readthedocs.io/en/stable/config.html
diff --git a/src/client/datascience/context/activeEditorContext.ts b/src/client/datascience/context/activeEditorContext.ts
@@ -11,6 +11,7 @@ import { PYTHON_LANGUAGE } from '../../common/constants';
 import { ContextKey } from '../../common/contextKey';
 import { NotebookEditorSupport } from '../../common/experiments/groups';
 import { IDisposable, IDisposableRegistry, IExperimentsManager } from '../../common/types';
+import { setSharedProperty } from '../../telemetry';
 import { EditorContexts } from '../constants';
 import { IInteractiveWindow, IInteractiveWindowProvider, INotebookEditor, INotebookEditorProvider } from '../types';
 
@@ -86,6 +87,9 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
         this.updateMergedContexts();
     }
     private onDidChangeActiveNotebookEditor(e?: INotebookEditor) {
+        // This will ensure all subsequent telemetry will get the context of whether it is a custom/native/old notebook editor.
+        // This is temporary, and once we ship native editor this needs to be removed.
+        setSharedProperty('ds_notebookeditor', e?.type);
         this.nativeContext.set(!!e).ignoreErrors();
         this.updateMergedContexts();
     }
diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts
@@ -96,6 +96,7 @@ import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher';
 const nativeEditorDir = path.join(EXTENSION_ROOT_DIR, 'out', 'datascience-ui', 'notebook');
 @injectable()
 export class NativeEditor extends InteractiveBase implements INotebookEditor {
+    public readonly type: 'old' | 'custom' = 'custom';
     public get onDidChangeViewState(): Event<void> {
         return this._onDidChangeViewState.event;
     }
diff --git a/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts b/src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts
@@ -62,6 +62,7 @@ enum AskForSaveResult {
 
 @injectable()
 export class NativeEditorOldWebView extends NativeEditor {
+    public readonly type = 'old';
     public get visible(): boolean {
         return this.viewState.visible;
     }
diff --git a/src/client/datascience/notebook/contentProvider.ts b/src/client/datascience/notebook/contentProvider.ts
@@ -16,7 +16,7 @@ import type {
 import { ICommandManager } from '../../common/application/types';
 import { MARKDOWN_LANGUAGE } from '../../common/constants';
 import { DataScience } from '../../common/utils/localize';
-import { captureTelemetry } from '../../telemetry';
+import { captureTelemetry, sendTelemetryEvent, setSharedProperty } from '../../telemetry';
 import { Telemetry } from '../constants';
 import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider';
 import { notebookModelToVSCNotebookData } from './helpers/helpers';
@@ -70,6 +70,9 @@ export class NotebookContentProvider implements INotebookContentProvider {
         const model = await (openContext.backupId
             ? this.notebookStorage.load(uri, undefined, openContext.backupId)
             : this.notebookStorage.load(uri, undefined, true));
+
+        setSharedProperty('ds_notebookeditor', 'native');
+        sendTelemetryEvent(Telemetry.CellCount, undefined, { count: model.cells.length });
         return notebookModelToVSCNotebookData(model);
     }
     @captureTelemetry(Telemetry.Save, undefined, true)
diff --git a/src/client/datascience/notebook/executionService.ts b/src/client/datascience/notebook/executionService.ts
@@ -17,7 +17,7 @@ import { noop } from '../../common/utils/misc';
 import { StopWatch } from '../../common/utils/stopWatch';
 import { IServiceContainer } from '../../ioc/types';
 import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
-import { Commands, Telemetry } from '../constants';
+import { Commands, Telemetry, VSCodeNativeTelemetry } from '../constants';
 import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider';
 import { IDataScienceErrorHandler, INotebook, INotebookModel, INotebookProvider } from '../types';
 import { findMappedNotebookCellModel } from './helpers/cellMappers';
@@ -81,6 +81,7 @@ export class NotebookExecutionService implements INotebookExecutionService {
         await this.executeIndividualCell(notebookAndModel, document, cell, token, stopWatch);
     }
     @captureTelemetry(Telemetry.ExecuteNativeCell, undefined, true)
+    @captureTelemetry(VSCodeNativeTelemetry.RunAllCells, undefined, true)
     public async executeAllCells(document: NotebookDocument, token: CancellationToken): Promise<void> {
         const stopWatch = new StopWatch();
         const notebookAndModel = this.getNotebookAndModel(document);
diff --git a/src/client/datascience/notebook/helpers/cellUpdateHelpers.ts b/src/client/datascience/notebook/helpers/cellUpdateHelpers.ts
@@ -18,6 +18,8 @@ import {
 } from '../../../common/application/types';
 import { MARKDOWN_LANGUAGE } from '../../../common/constants';
 import { traceError } from '../../../common/logger';
+import { sendTelemetryEvent } from '../../../telemetry';
+import { VSCodeNativeTelemetry } from '../../constants';
 import { INotebookModel } from '../../types';
 import { findMappedNotebookCellModel } from './cellMappers';
 import {
@@ -94,8 +96,21 @@ function clearCellOutput(change: NotebookCellOutputsChangeEvent, model: INoteboo
     return true;
 }
 
+/**
+ * VS Code doesn't seem to fire this when changing between markdown & code.
+ * Its only fired when changing the language from python to csharp.
+ * https://github.com/microsoft/vscode/issues/100042
+ */
 function changeCellLanguage(change: NotebookCellLanguageChangeEvent, model: INotebookModel) {
     const cellModel = findMappedNotebookCellModel(change.cell, model.cells);
+    if (
+        (change.cell.cellKind === vscodeNotebookEnums.CellKind.Markdown && cellModel.data.cell_type === 'markdown') ||
+        (change.cell.cellKind === vscodeNotebookEnums.CellKind.Code && cellModel.data.cell_type === 'code')
+    ) {
+        // This is when user changes from python to csharp or similar.
+        return;
+    }
+    // Here we have changed from a code cell to markdown or vice versa.
     const cellData = createCellFrom(cellModel.data, change.language === MARKDOWN_LANGUAGE ? 'markdown' : 'code');
     // tslint:disable-next-line: no-any
     change.cell.outputs = createVSCCellOutputsFromOutputs(cellData.outputs as any);
@@ -106,15 +121,23 @@ function changeCellLanguage(change: NotebookCellLanguageChangeEvent, model: INot
     // Create a new cell & replace old one.
     const oldCellIndex = model.cells.indexOf(cellModel);
     model.cells[oldCellIndex] = createCellFromVSCNotebookCell(change.cell, model);
+    sendTelemetryEvent(
+        change.cell.cellKind === vscodeNotebookEnums.CellKind.Markdown
+            ? VSCodeNativeTelemetry.ChangeToMarkdown
+            : VSCodeNativeTelemetry.ChangeToCode
+    );
 }
 
 function handleChangesToCells(change: NotebookCellsChangeEvent, model: INotebookModel) {
     if (isCellMoveChange(change)) {
         handleCellMove(change, model);
+        sendTelemetryEvent(VSCodeNativeTelemetry.MoveCell);
     } else if (isCellDelete(change)) {
         handleCellDelete(change, model);
+        sendTelemetryEvent(VSCodeNativeTelemetry.DeleteCell);
     } else if (isCellInsertion(change)) {
         handleCellInsertion(change, model);
+        sendTelemetryEvent(VSCodeNativeTelemetry.AddCell);
     } else {
         traceError('Unsupported cell change', change);
         throw new Error('Unsupported cell change');
diff --git a/src/client/datascience/notebook/integration.ts b/src/client/datascience/notebook/integration.ts
@@ -4,7 +4,12 @@
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { IExtensionSingleActivationService } from '../../activation/types';
-import { ICommandManager, IVSCodeNotebook } from '../../common/application/types';
+import {
+    IApplicationEnvironment,
+    IApplicationShell,
+    ICommandManager,
+    IVSCodeNotebook
+} from '../../common/application/types';
 import { NotebookEditorSupport } from '../../common/experiments/groups';
 import { IFileSystem } from '../../common/platform/types';
 import { IDisposableRegistry, IExperimentsManager, IExtensionContext } from '../../common/types';
@@ -31,7 +36,9 @@ export class NotebookIntegration implements IExtensionSingleActivationService {
         @inject(IFileSystem) private readonly fs: IFileSystem,
         @inject(ICommandManager) private readonly commandManager: ICommandManager,
         @inject(NotebookKernel) private readonly notebookKernel: NotebookKernel,
-        @inject(NotebookOutputRenderer) private readonly renderer: NotebookOutputRenderer
+        @inject(NotebookOutputRenderer) private readonly renderer: NotebookOutputRenderer,
+        @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment,
+        @inject(IApplicationShell) private readonly shell: IApplicationShell
     ) {}
     public get isEnabled() {
         const packageJsonFile = path.join(this.context.extensionPath, 'package.json');
@@ -91,6 +98,10 @@ export class NotebookIntegration implements IExtensionSingleActivationService {
         );
     }
     private async enableNotebooks(useVSCodeNotebookAsDefaultEditor: boolean) {
+        if (this.env.channel === 'stable') {
+            this.shell.showErrorMessage(DataScience.previewNotebookOnlySupportedInVSCInsiders()).then(noop, noop);
+            return;
+        }
         const packageJsonFile = path.join(this.context.extensionPath, 'package.json');
         const content = JSON.parse(this.fs.readFileSync(packageJsonFile));
 
diff --git a/src/client/datascience/notebook/notebookEditor.ts b/src/client/datascience/notebook/notebookEditor.ts
@@ -25,6 +25,7 @@ import { getDefaultCodeLanguage } from './helpers/helpers';
 import { INotebookExecutionService } from './types';
 
 export class NotebookEditor implements INotebookEditor {
+    public readonly type = 'native';
     public get onDidChangeViewState(): Event<void> {
         return this.changedViewState.event;
     }
diff --git a/src/client/datascience/notebook/notebookEditorProvider.ts b/src/client/datascience/notebook/notebookEditorProvider.ts
@@ -19,7 +19,8 @@ import '../../common/extensions';
 import { IConfigurationService, IDisposableRegistry } from '../../common/types';
 import { createDeferred, Deferred } from '../../common/utils/async';
 import { IServiceContainer } from '../../ioc/types';
-import { Commands } from '../constants';
+import { captureTelemetry, setSharedProperty } from '../../telemetry';
+import { Commands, Telemetry } from '../constants';
 import { updateModelForUseWithVSCodeNotebook } from '../interactive-ipynb/nativeEditorStorage';
 import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider';
 import { INotebookEditor, INotebookEditorProvider, INotebookProvider, IStatusProvider } from '../types';
@@ -83,6 +84,8 @@ export class NotebookEditorProvider implements INotebookEditorProvider {
         this.disposables.push(
             this.commandManager.registerCommand(Commands.OpenNotebookInPreviewEditor, async (uri?: Uri) => {
                 if (uri) {
+                    setSharedProperty('ds_notebookeditor', 'native');
+                    captureTelemetry(Telemetry.OpenNotebook, { scope: 'command' }, false);
                     const integration = serviceContainer.get<NotebookIntegration>(NotebookIntegration);
                     // If user is not meant to be using VSC Notebooks, and it is not enabled,
                     // then enable it for side by side usage.
@@ -112,6 +115,7 @@ export class NotebookEditorProvider implements INotebookEditorProvider {
     }
 
     public async open(file: Uri): Promise<INotebookEditor> {
+        setSharedProperty('ds_notebookeditor', 'native');
         if (this.notebooksWaitingToBeOpenedByUri.get(file.toString())) {
             return this.notebooksWaitingToBeOpenedByUri.get(file.toString())!.promise;
         }
@@ -132,7 +136,9 @@ export class NotebookEditorProvider implements INotebookEditorProvider {
         // We do not need this.
         return;
     }
+    @captureTelemetry(Telemetry.CreateNewNotebook, undefined, false)
     public async createNew(contents?: string): Promise<INotebookEditor> {
+        setSharedProperty('ds_notebookeditor', 'native');
         const model = await this.storage.createNew(contents, true);
         return this.open(model.file);
     }
diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts
@@ -536,6 +536,11 @@ export interface INotebookEditorProvider {
 // For native editing, the INotebookEditor acts like a TextEditor and a TextDocument together
 export const INotebookEditor = Symbol('INotebookEditor');
 export interface INotebookEditor extends IInteractiveBase {
+    /**
+     * Type of editor, whether it is the old, custom or native notebook editor.
+     * Once VSC Notebook is stable, this property can be removed.
+     */
+    readonly type: 'old' | 'custom' | 'native';
     readonly onDidChangeViewState: Event<void>;
     readonly closed: Event<INotebookEditor>;
     readonly executed: Event<INotebookEditor>;
diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts
diff --git a/src/test/telemetry/index.unit.test.ts b/src/test/telemetry/index.unit.test.ts

Original file line number	Diff line number	Diff line change
`@@ -536,5 +536,6 @@`
`536`	`536`	`"DataScience.reloadCustomEditor": "Please reload VS Code to use the custom editor API",`
`537`	`537`	`"DataScience.reloadVSCodeNotebookEditor": "Please reload VS Code to use the Notebook Editor",`
`538`	`538`	`"DataScience.step": "Run next line (F10)",`
`539`		`- "DataScience.usingPreviewNotebookWithOtherNotebookWarning": "Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks."`
	`539`	`+ "DataScience.usingPreviewNotebookWithOtherNotebookWarning": "Using the Preview Notebook Editor along with the stable Notebook Editor is not recommended. Doing so could result in data loss or corruption of notebooks.",`
	`540`	`+ "DataScience.previewNotebookOnlySupportedInVSCInsiders": "The Preview Notebook Editor is supported only in the Insiders version of Visual Studio Code."`
`540`	`541`	`}`
Original file line number	Diff line number	Diff line change
`@@ -96,6 +96,7 @@ import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher';`
`96`	`96`	`const nativeEditorDir = path.join(EXTENSION_ROOT_DIR, 'out', 'datascience-ui', 'notebook');`
`97`	`97`	`@injectable()`
`98`	`98`	`export class NativeEditor extends InteractiveBase implements INotebookEditor {`
	`99`	`+ public readonly type: 'old' \| 'custom' = 'custom';`
`99`	`100`	`public get onDidChangeViewState(): Event<void> {`
`100`	`101`	`return this._onDidChangeViewState.event;`
`101`	`102`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ enum AskForSaveResult {`
`62`	`62`
`63`	`63`	`@injectable()`
`64`	`64`	`export class NativeEditorOldWebView extends NativeEditor {`
	`65`	`+ public readonly type = 'old';`
`65`	`66`	`public get visible(): boolean {`
`66`	`67`	`return this.viewState.visible;`
`67`	`68`	`}`