Added Notebook telemetry for livesite notebooks (#9948)

* Added Notebook telemetry for livesite

* added back the metadata retension code

* Unified Telemetry services

* fixed names

* Fixed texts to use new NullAdsTelemetryService

* Validate GUID before sending it

* made GUID check a bit more robust

Co-authored-by: Rajesh Kamath <rajkashop@hotmail.com>

src/sql/platform/telemetry/common/telemetryKeys.ts

@@ -60,15 +60,14 @@ export const DeleteAgentAlert = 'DeleteAgentAlert';
 export const DeleteAgentOperator = 'DeleteAgentOperator';
 export const DeleteAgentProxy = 'DeleteAgentProxy';
 
-// Notebook Events:
-export const NotebookMarkdownRendered = 'NotebookMarkdownRendered';
-
 export enum TelemetryView {
 	Shell = 'Shell',
 	ExtensionRecommendationDialog = 'ExtensionRecommendationDialog',
-	ResultsPanel = 'ResultsPanel'
+	ResultsPanel = 'ResultsPanel',
+	Notebook = 'Notebook'
 }
 
 export enum TelemetryAction {
-	Click = 'Click'
+	Click = 'Click',
+	Open = 'Open'
 }

src/sql/workbench/contrib/notebook/browser/notebook.component.ts

@@ -45,7 +45,6 @@ import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { ILogService } from 'vs/platform/log/common/log';
 import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
 import { LabeledMenuItemActionItem, fillInActions } from 'vs/platform/actions/browser/menuEntryActionViewItem';
-import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { Button } from 'sql/base/browser/ui/button/button';
 import { isUndefinedOrNull } from 'vs/base/common/types';
 import { IBootstrapParams } from 'sql/workbench/services/bootstrap/common/bootstrapParams';
@@ -56,6 +55,7 @@ import { TextCellComponent } from 'sql/workbench/contrib/notebook/browser/cellVi
 import { NotebookInput } from 'sql/workbench/contrib/notebook/browser/models/notebookInput';
 import { IColorTheme } from 'vs/platform/theme/common/themeService';
 import { ICommandService } from 'vs/platform/commands/common/commands';
+import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry';
 
 
 export const NOTEBOOK_SELECTOR: string = 'notebook-component';
@@ -104,8 +104,8 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
 		@Inject(ICapabilitiesService) private capabilitiesService: ICapabilitiesService,
 		@Inject(ITextFileService) private textFileService: ITextFileService,
 		@Inject(ILogService) private readonly logService: ILogService,
-		@Inject(ITelemetryService) private telemetryService: ITelemetryService,
+		@Inject(ICommandService) private commandService: ICommandService,
-		@Inject(ICommandService) private commandService: ICommandService
+		@Inject(IAdsTelemetryService) private adstelemetryService: IAdsTelemetryService
 	) {
 		super();
 		this.updateProfile();
@@ -310,7 +310,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
 			layoutChanged: this._notebookParams.input.layoutChanged,
 			capabilitiesService: this.capabilitiesService,
 			editorLoadedTimestamp: this._notebookParams.input.editorOpenedTimestamp
-		}, this.profile, this.logService, this.notificationService, this.telemetryService);
+		}, this.profile, this.logService, this.notificationService, this.adstelemetryService);
 		let trusted = await this.notebookService.isNotebookTrustCached(this._notebookParams.notebookUri, this.isDirty());
 		this._register(model.onError((errInfo: INotification) => this.handleModelError(errInfo)));
 		this._register(model.contentChanged((change) => this.handleContentChanged(change)));

src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts

@@ -39,6 +39,7 @@ import { assign } from 'vs/base/common/objects';
 import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
 import { IStorageService } from 'vs/platform/storage/common/storage';
 import { TestStorageService, TestTextResourcePropertiesService } from 'vs/workbench/test/common/workbenchTestServices';
+import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService';
 
 
 class ServiceAccessor {
@@ -877,7 +878,7 @@ suite('Notebook Editor Model', function (): void {
 		let options: INotebookModelOptions = assign({}, defaultModelOptions, <Partial<INotebookModelOptions>><unknown>{
 			factory: mockModelFactory.object
 		});
-		notebookModel = new NotebookModel(options, undefined, logService, undefined, undefined);
+		notebookModel = new NotebookModel(options, undefined, logService, undefined, new NullAdsTelemetryService());
 		await notebookModel.loadContents();
 	}
 

src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts

@@ -29,6 +29,7 @@ import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServic
 import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/browser/models/notebookInput';
 import { NotebookRange } from 'sql/workbench/services/notebook/browser/notebookService';
 import { NotebookMarkdownRenderer } from 'sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown';
+import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService';
 
 let expectedNotebookContent: nb.INotebookContents = {
 	cells: [{
@@ -364,7 +365,7 @@ suite('Notebook Find Model', function (): void {
 		mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(contents));
 		defaultModelOptions.contentManager = mockContentManager.object;
 		// Initialize the model
-		model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.loadContents();
 		await model.requestModelLoad();
 	}

src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts

@@ -35,6 +35,7 @@ import { SessionManager } from 'sql/workbench/services/notebook/browser/sessionM
 import { mssqlProviderName } from 'sql/platform/connection/common/constants';
 import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
 import { uriPrefixes } from 'sql/platform/connection/common/utils';
+import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService';
 
 let expectedNotebookContent: nb.INotebookContents = {
 	cells: [{
@@ -153,7 +154,7 @@ suite('notebook model', function (): void {
 		mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(emptyNotebook));
 		defaultModelOptions.contentManager = mockContentManager.object;
 		// When I initialize the model
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.loadContents();
 
 		// Then I expect to have 0 code cell as the contents
@@ -169,7 +170,7 @@ suite('notebook model', function (): void {
 		mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
 		defaultModelOptions.contentManager = mockContentManager.object;
 		// When I initialize the model
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.loadContents(true);
 		await model.requestModelLoad();
 
@@ -186,7 +187,7 @@ suite('notebook model', function (): void {
 
 		// When I initalize the model
 		// Then it should throw
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		assert.equal(model.inErrorState, false);
 		await assert.rejects(async () => { await model.loadContents(); });
 		assert.equal(model.inErrorState, true);
@@ -199,7 +200,7 @@ suite('notebook model', function (): void {
 		defaultModelOptions.contentManager = mockContentManager.object;
 
 		// When I initalize the model
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.loadContents();
 
 		// Then I expect all cells to be in the model
@@ -227,7 +228,7 @@ suite('notebook model', function (): void {
 		defaultModelOptions.providerId = 'jupyter';
 
 		// When I initalize the model
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.loadContents();
 
 		// I expect the default provider to be jupyter
@@ -237,7 +238,7 @@ suite('notebook model', function (): void {
 		defaultModelOptions.providerId = 'SQL';
 
 		// When I initalize the model
-		model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.loadContents();
 
 		// I expect the default provider to be SQL
@@ -262,7 +263,7 @@ suite('notebook model', function (): void {
 		defaultModelOptions.contentManager = mockContentManager.object;
 
 		// When I initalize the model
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.loadContents();
 
 		let activeCellChangeCount = 0;
@@ -319,7 +320,7 @@ suite('notebook model', function (): void {
 		defaultModelOptions.contentManager = mockContentManager.object;
 
 		// When I initalize the model
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.loadContents();
 
 		// Count number of times onError event is fired
@@ -376,7 +377,7 @@ suite('notebook model', function (): void {
 		sessionReady.resolve();
 		let sessionFired = false;
 
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		model.onClientSessionReady((session) => sessionFired = true);
 		await model.loadContents();
 		await model.requestModelLoad();
@@ -402,14 +403,14 @@ suite('notebook model', function (): void {
 	});
 
 	test('Should sanitize kernel display name when IP is included', async function (): Promise<void> {
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		let displayName = 'PySpark (1.1.1.1)';
 		let sanitizedDisplayName = model.sanitizeDisplayName(displayName);
 		assert.equal(sanitizedDisplayName, 'PySpark');
 	});
 
 	test('Should sanitize kernel display name properly when IP is not included', async function (): Promise<void> {
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		let displayName = 'PySpark';
 		let sanitizedDisplayName = model.sanitizeDisplayName(displayName);
 		assert.equal(sanitizedDisplayName, 'PySpark');
@@ -420,7 +421,7 @@ suite('notebook model', function (): void {
 		let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
 		mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
 		defaultModelOptions.contentManager = mockContentManager.object;
-		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService());
 		await model.requestModelLoad();
 
 		let actualChanged: NotebookContentChange;
@@ -480,6 +481,24 @@ suite('notebook model', function (): void {
 
 	});
 
+	test('Should not delete custom metadata', async function (): Promise<void> {
+		expectedNotebookContent.metadata['custom-string'] = 'some_string';
+		expectedNotebookContent.metadata['custom-object'] = { prop1: 'value1', prop2: 'value2' };
+
+		// Given a notebook
+		let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
+		mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
+		defaultModelOptions.contentManager = mockContentManager.object;
+		// When I initialize the model
+		let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined);
+		await model.loadContents();
+
+		let output = model.toJSON();
+		assert(output.metadata['custom-string'] === 'some_string', 'Custom metadata was not preserved');
+		assert(output.metadata['custom-object']['prop1'] === 'value1', 'Custom metadata for object was not preserved');
+		assert(output.metadata['custom-object']['prop2'] === 'value2', 'Custom metadata for object was not preserved');
+	});
+
 	async function loadModelAndStartClientSession(): Promise<NotebookModel> {
 		let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager);
 		mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent));
@@ -500,7 +519,7 @@ suite('notebook model', function (): void {
 		let options: INotebookModelOptions = assign({}, defaultModelOptions, <Partial<INotebookModelOptions>>{
 			factory: mockModelFactory.object
 		});
-		let model = new NotebookModel(options, undefined, logService, undefined, undefined);
+		let model = new NotebookModel(options, undefined, logService, undefined, new NullAdsTelemetryService());
 		model.onClientSessionReady((session) => actualSession = session);
 		await model.requestModelLoad();
 

src/sql/workbench/services/notebook/browser/models/notebookModel.ts

@@ -24,11 +24,11 @@ import { ConnectionProfile } from 'sql/platform/connection/common/connectionProf
 import { uriPrefixes } from 'sql/platform/connection/common/utils';
 import { keys } from 'vs/base/common/map';
 import { ILogService } from 'vs/platform/log/common/log';
-import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { getErrorMessage } from 'vs/base/common/errors';
 import { find, firstIndex } from 'vs/base/common/arrays';
 import { startsWith } from 'vs/base/common/strings';
 import { notebookConstants } from 'sql/workbench/services/notebook/browser/interfaces';
+import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry';
 
 /*
 * Used to control whether a message in a dialog/wizard is displayed as an error,
@@ -63,6 +63,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 	private _cells: ICellModel[];
 	private _defaultLanguageInfo: nb.ILanguageInfo;
 	private _tags: string[];
+	private _existingMetadata = {};
 	private _language: string;
 	private _onErrorEmitter = new Emitter<INotification>();
 	private _savedKernelInfo: nb.IKernelInfo;
@@ -87,7 +88,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 		public connectionProfile: IConnectionProfile | undefined,
 		@ILogService private readonly logService: ILogService,
 		@INotificationService private readonly notificationService: INotificationService,
-		@ITelemetryService private readonly telemetryService: ITelemetryService
+		@IAdsTelemetryService private readonly adstelemetryService: IAdsTelemetryService
 	) {
 		super();
 		if (!_notebookOptions || !_notebookOptions.notebookUri || !_notebookOptions.notebookManagers) {
@@ -303,6 +304,25 @@ export class NotebookModel extends Disposable implements INotebookModel {
 			if (contents) {
 				this._defaultLanguageInfo = contents.metadata && contents.metadata.language_info;
 				this._savedKernelInfo = this.getSavedKernelInfo(contents);
+				if (contents.metadata) {
+					//Telemetry of loading notebook
+					if (contents.metadata.azdata_notebook_guid && contents.metadata.azdata_notebook_guid.length === 36) {
+						//Verify if it is actual GUID and then send it to the telemetry
+						let regex = new RegExp('(\{){0,1}[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}(\}){0,1}');
+						if (regex.test(contents.metadata.azdata_notebook_guid)) {
+							this.adstelemetryService.createActionEvent(TelemetryKeys.TelemetryView.Notebook, TelemetryKeys.TelemetryAction.Open)
+								.withAdditionalProperties({ azdata_notebook_guid: contents.metadata.azdata_notebook_guid })
+								.send();
+						}
+					}
+					Object.keys(contents.metadata).forEach(key => {
+						let expectedKeys = ['kernelspec', 'language_info', 'tags'];
+						// If custom metadata is defined, add to the _existingMetadata object
+						if (expectedKeys.indexOf(key) < 0) {
+							this._existingMetadata[key] = contents.metadata[key];
+						}
+					});
+				}
 				if (contents.cells && contents.cells.length > 0) {
 					this._cells = contents.cells.map(c => {
 						let cellModel = factory.createCell(c, { notebook: this, isTrusted: isTrusted });
@@ -937,7 +957,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 				if (this._textCellsLoading <= 0) {
 					if (this._notebookOptions.editorLoadedTimestamp) {
 						let markdownRenderingTime = Date.now() - this._notebookOptions.editorLoadedTimestamp;
-						this.telemetryService.publicLog(TelemetryKeys.NotebookMarkdownRendered, { markdownRenderingElapsedMs: markdownRenderingTime });
+						this.adstelemetryService.sendMetricsEvent({ markdownRenderingElapsedMs: markdownRenderingTime }, TelemetryKeys.TelemetryView.Notebook);
 					}
 				}
 			}
@@ -971,6 +991,9 @@ export class NotebookModel extends Disposable implements INotebookModel {
 		metadata.kernelspec = this._savedKernelInfo;
 		metadata.language_info = this.languageInfo;
 		metadata.tags = this._tags;
+		Object.keys(this._existingMetadata).forEach(key => {
+			metadata[key] = this._existingMetadata[key];
+		});
 		return {
 			metadata,
 			nbformat_minor: this._nbformatMinor,