simplify extra properties logic such that it be moved soon

src/vs/platform/telemetry/common/telemetry.ts

@@ -51,7 +51,7 @@ export const NullTelemetryService: ITelemetryService = {
 };
 
 export interface ITelemetryAppender extends IDisposable {
-	log(eventName: string, data?: any): void;
+	log(eventName: string, data?: any): any;
 }
 
 // --- util

src/vs/platform/telemetry/node/appInsightsAppender.ts

@@ -5,6 +5,8 @@
 'use strict';
 
 import errors = require('vs/base/common/errors');
+import {TPromise} from 'vs/base/common/winjs.base';
+import {mixin} from 'vs/base/common/objects';
 import {IStorageService} from 'vs/platform/storage/common/storage';
 import {ITelemetryAppender} from 'vs/platform/telemetry/common/telemetry';
 import {AIAdapter, IAIAdapter} from 'vs/base/node/aiAdapter';
@@ -23,21 +25,15 @@ export class AppInsightsAppender implements ITelemetryAppender {
 	private static EVENT_NAME_PREFIX: string = 'monacoworkbench';
 	private static SQM_KEY: string = '\\Software\\Microsoft\\SQMClient';
 
-	private storageService: IStorageService;
 	private appInsights: IAIAdapter;
 	private appInsightsVortex: IAIAdapter;
-	private commonProperties: { [key: string]: string };
-	private commonMetrics: { [key: string]: number };
+	private _additionalProperties: TPromise<{ [key: string]: any }>;
 
 	constructor(
 		@IStorageService storageService: IStorageService,
 		config: {key: string; asimovKey: string},
 		_testing_client?: IAIAdapter
 	) {
-		this.commonProperties = {};
-		this.commonMetrics = {};
-		this.storageService = storageService;
-
 		let {key, asimovKey} = config;
 		if (_testing_client) {
 			// for test
@@ -54,130 +50,118 @@ export class AppInsightsAppender implements ITelemetryAppender {
 			}
 		}
 
-		this.loadAddtionaProperties();
+		this._additionalProperties = AppInsightsAppender._loadAddtionalProperties(storageService);
 	}
 
-	private loadAddtionaProperties(): void {
+	private static _loadAddtionalProperties(storageService: IStorageService): TPromise<{ [key: string]: any }> {
+
+		const result: { [key: string]: any } = Object.create(null);
+		let promises: TPromise<any>[] = [];
+
 		// add shell & render version
 		if (process.versions) {
-			this.commonProperties['version.shell'] = (<any>process).versions['electron'];
-			this.commonProperties['version.renderer'] = (<any>process).versions['chrome'];
+			result['common.version.shell'] = (<any>process).versions['electron'];
+			result['common.version.renderer'] = (<any>process).versions['chrome'];
 		}
 
 		// add SQM data for windows machines
 		if (process.platform === 'win32') {
-			var sqmUserId = this.storageService.get(StorageKeys.sqmUserId);
+			var sqmUserId = storageService.get(StorageKeys.sqmUserId);
 			if (sqmUserId) {
-				this.commonProperties['sqm.userid'] = sqmUserId;
+				result['common.sqm.userid'] = sqmUserId;
 			} else {
-				AppInsightsAppender._getWinRegKeyData(AppInsightsAppender.SQM_KEY, 'UserId', winreg.HKCU, (error, result: string) => {
-					if (!error && result) {
-						this.commonProperties['sqm.userid'] = result;
-						this.storageService.store(StorageKeys.sqmUserId, result);
+				promises.push(AppInsightsAppender._getWinRegKeyData(AppInsightsAppender.SQM_KEY, 'UserId', winreg.HKCU).then(result => {
+					if (result) {
+						result['common.sqm.userid'] = result;
+						storageService.store(StorageKeys.sqmUserId, result);
 					}
-				});
+				}));
 			}
 
-			var sqmMachineId = this.storageService.get(StorageKeys.sqmMachineId);
+			var sqmMachineId = storageService.get(StorageKeys.sqmMachineId);
 			if (sqmMachineId) {
-				this.commonProperties['sqm.machineid'] = sqmMachineId;
+				result['common.sqm.machineid'] = sqmMachineId;
 			}
 			else {
-				AppInsightsAppender._getWinRegKeyData(AppInsightsAppender.SQM_KEY, 'MachineId', winreg.HKLM, (error, result) => {
-					if (!error && result) {
-						this.commonProperties['sqm.machineid'] = result;
-						this.storageService.store(StorageKeys.sqmMachineId, result);
+				promises.push(AppInsightsAppender._getWinRegKeyData(AppInsightsAppender.SQM_KEY, 'MachineId', winreg.HKLM).then(result => {
+					if (result) {
+						result['common.sqm.machineid'] = result;
+						storageService.store(StorageKeys.sqmMachineId, result);
 					}
-				});
+				}));
 			}
 		}
 
-		var firstSessionDate = this.storageService.get(StorageKeys.firstSessionDate);
+		var firstSessionDate = storageService.get(StorageKeys.firstSessionDate);
 		if (!firstSessionDate) {
 			firstSessionDate = (new Date()).toUTCString();
-			this.storageService.store(StorageKeys.firstSessionDate, firstSessionDate);
+			storageService.store(StorageKeys.firstSessionDate, firstSessionDate);
 		}
-		this.commonProperties['firstSessionDate'] = firstSessionDate;
+		result['common.firstSessionDate'] = firstSessionDate;
 
 		//report last session date and isNewSession flag
-		var lastSessionDate = this.storageService.get(StorageKeys.lastSessionDate);
+		var lastSessionDate = storageService.get(StorageKeys.lastSessionDate);
 		if (!lastSessionDate) {
-			this.commonMetrics['isNewSession'] = 1;
+			result['common.isNewSession'] = 1;
 		} else {
-			this.commonMetrics['isNewSession'] = 0;
-			this.commonProperties['lastSessionDate'] = lastSessionDate;
+			result['common.isNewSession'] = 0;
+			result['common.lastSessionDate'] = lastSessionDate;
 		}
 
-		this.storageService.store(StorageKeys.lastSessionDate, (new Date()).toUTCString());
+		storageService.store(StorageKeys.lastSessionDate, (new Date()).toUTCString());
 
 		if (os) {
-			this.commonProperties['osVersion'] = os.release();
+			result['common.osVersion'] = os.release();
 		}
+
+		return TPromise.join(promises).then(() => result, () => result);
 	}
 
-	private static _getWinRegKeyData(key: string, name: string, hive: string, callback: (error: Error, userId: string) => void): void {
-		if (process.platform === 'win32') {
-			try {
-				var reg = new winreg({
-					hive: hive,
-					key: key
-				});
-
-				reg.get(name, (e, result) => {
-					if (e || !result) {
-						callback(e, null);
-					} else {
-						callback(null, result.value);
-					}
-				});
-			} catch (err) {
-				errors.onUnexpectedError(err);
-				callback(err, null);
+	private static _getWinRegKeyData(key: string, name: string, hive: string): TPromise<string> {
+		return new TPromise<string>((resolve, reject) => {
+			if (process.platform === 'win32') {
+				try {
+					var reg = new winreg({ hive, key });
+					reg.get(name, (e, result) => {
+						if (e || !result) {
+							reject(null);
+						} else {
+							resolve(result.value);
+						}
+					});
+				} catch (err) {
+					errors.onUnexpectedError(err);
+					reject(err);
+				}
+			} else {
+				resolve(null);
 			}
-		} else {
-			callback(null, null);
-		}
+		}).then(undefined, err => {
+			// we only want success
+			return undefined;
+		});
 	}
 
-	public log(eventName: string, data?: any): void {
-
-		data = data || Object.create(null);
-		data = this.addCommonMetrics(data);
-		data = this.addCommonProperties(data);
-
-		if (this.appInsights) {
-			this.appInsights.log(eventName, data);
-		}
-
-		if (this.appInsightsVortex) {
-			this.appInsightsVortex.log(eventName, data);
-		}
+	public log(eventName: string, data?: any): TPromise<any> {
+		return this._additionalProperties.then(additionalProperties => {
+			data = mixin(data, additionalProperties);
+			if (this.appInsights) {
+				this.appInsights.log(eventName, data);
+			}
+			if (this.appInsightsVortex) {
+				this.appInsightsVortex.log(eventName, data);
+			}
+		});
 	}
 
 	public dispose(): void {
 		if (this.appInsights) {
 			this.appInsights.dispose();
 		}
-
 		if (this.appInsightsVortex) {
 			this.appInsightsVortex.dispose();
 		}
-
 		this.appInsights = null;
 		this.appInsightsVortex = null;
 	}
-
-	protected addCommonProperties(properties: any): any {
-		for (var prop in this.commonProperties) {
-			properties['common.' + prop] = this.commonProperties[prop];
-		}
-		return properties;
-	}
-
-	protected addCommonMetrics(metrics: any): any {
-		for (var prop in this.commonMetrics) {
-			metrics['common.' + prop] = this.commonMetrics[prop];
-		}
-		return metrics;
-	}
 }
\ No newline at end of file

src/vs/platform/telemetry/test/node/appInsightsAppender.test.ts

@@ -52,65 +52,66 @@ suite('Telemetry - AppInsightsTelemetryAppender', () => {
 	});
 
 	test('Simple event', () => {
-		appender.log('testEvent');
-
-		assert.equal(appInsightsMock.events.length, 1);
-		assert.equal(appInsightsMock.events[0].eventName, 'testPrefix/testEvent');
+		return appender.log('testEvent').then(_ => {
+			assert.equal(appInsightsMock.events.length, 1);
+			assert.equal(appInsightsMock.events[0].eventName, 'testPrefix/testEvent');
+		});
 	});
 
 	test('test additional properties', () => {
-		appender.log('testEvent');
-		assert.equal(appInsightsMock.events.length, 1);
-
-		let [first] = appInsightsMock.events;
-		assert.equal(first.eventName, 'testPrefix/testEvent');
-		assert.ok('common.osVersion' in first.data);
-		assert.ok('common.isNewSession' in first.data);
-		assert.ok('common.firstSessionDate' in first.data);
-		assert.ok(!('common.lastSessionDate' in first.data)); // conditional, see below
-		// assert.ok('common.version.shell' in first.data); // only when running on electron
-		// assert.ok('common.version.renderer' in first.data);
-		if (process.platform === 'win32') { // SQM only on windows
-			assert.ok('common.sqm.userid' in first.data);
-			assert.ok('common.sqm.machineid' in first.data);
-		}
+		return appender.log('testEvent').then(_ => {
+			assert.equal(appInsightsMock.events.length, 1);
+
+			let [first] = appInsightsMock.events;
+			assert.equal(first.eventName, 'testPrefix/testEvent');
+			assert.ok('common.osVersion' in first.data);
+			assert.ok('common.isNewSession' in first.data);
+			assert.ok('common.firstSessionDate' in first.data);
+			assert.ok(!('common.lastSessionDate' in first.data)); // conditional, see below
+			// assert.ok('common.version.shell' in first.data); // only when running on electron
+			// assert.ok('common.version.renderer' in first.data);
+			if (process.platform === 'win32') { // SQM only on windows
+				assert.ok('common.sqm.userid' in first.data);
+				assert.ok('common.sqm.machineid' in first.data);
+			}
+		});
 	});
 
 	test('test additional properties with storage data', () => {
 		const storage = new TestStorageService();
 		storage.store(StorageKeys.lastSessionDate, 'somevalue');
 		let appender = new AppInsightsAppender(storage, { key: '123', asimovKey: undefined }, appInsightsMock);
-		appender.log('testEvent');
-		let [first] = appInsightsMock.events;
-		assert.ok('common.lastSessionDate' in first.data); // conditional
-		appender.dispose();
+		return appender.log('testEvent').then(_ => {
+			let [first] = appInsightsMock.events;
+			assert.ok('common.lastSessionDate' in first.data); // conditional
+			appender.dispose();
+		});
 	});
 
 	test('Event with data', () => {
-		appender.log('testEvent', {
+		return appender.log('testEvent', {
 			title: 'some title',
 			width: 100,
 			height: 200
-		});
-
-		assert.equal(appInsightsMock.events.length, 1);
-		assert.equal(appInsightsMock.events[0].eventName, 'testPrefix/testEvent');
-
-		assert.equal(appInsightsMock.events[0].data['title'], 'some title');
-		assert.equal(appInsightsMock.events[0].data['width'], 100);
-		assert.equal(appInsightsMock.events[0].data['height'], 200);
+		}).then(_ => {
+			assert.equal(appInsightsMock.events.length, 1);
+			assert.equal(appInsightsMock.events[0].eventName, 'testPrefix/testEvent');
 
+			assert.equal(appInsightsMock.events[0].data['title'], 'some title');
+			assert.equal(appInsightsMock.events[0].data['width'], 100);
+			assert.equal(appInsightsMock.events[0].data['height'], 200);
+		});
 	});
 
 	test('Test asimov', () => {
 		appender = new AppInsightsAppender(new TestStorageService(), { key: '123', asimovKey: 'AIF-123' }, appInsightsMock);
 
-		appender.log('testEvent');
+		return appender.log('testEvent').then(_ => {
+			assert.equal(appInsightsMock.events.length, 2);
+			assert.equal(appInsightsMock.events[0].eventName, 'testPrefix/testEvent');
 
-		assert.equal(appInsightsMock.events.length, 2);
-		assert.equal(appInsightsMock.events[0].eventName, 'testPrefix/testEvent');
-
-		// test vortex
-		assert.equal(appInsightsMock.events[1].eventName, 'testPrefix/testEvent');
+			// test vortex
+			assert.equal(appInsightsMock.events[1].eventName, 'testPrefix/testEvent');
+		});
 	});
 });
\ No newline at end of file