simplify appender, telemetry service config

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

@@ -54,6 +54,14 @@ export interface ITelemetryAppender extends IDisposable {
 	log(eventName: string, data?: any): any;
 }
 
+export function combinedAppender(...appenders: ITelemetryAppender[]): ITelemetryAppender {
+	const log = (e, d) => appenders.forEach(a => a.log(e, d));
+	const dispose = () => appenders.forEach(a => a.dispose());
+	return { log, dispose };
+}
+
+export const NullAppender = combinedAppender();
+
 // --- util
 
 export function anonymize(input: string): string {

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

@@ -18,7 +18,7 @@ import {cloneAndChange, mixin} from 'vs/base/common/objects';
 import {Registry} from 'vs/platform/platform';
 
 export interface ITelemetryServiceConfig {
-	appender: ITelemetryAppender[];
+	appender: ITelemetryAppender;
 	commonProperties?: TPromise<{ [name: string]: any }>;
 	piiPaths?: string[];
 	userOptIn?: boolean;
@@ -26,12 +26,16 @@ export interface ITelemetryServiceConfig {
 
 export class TelemetryService implements ITelemetryService {
 
-	public static IDLE_START_EVENT_NAME = 'UserIdleStart';
-	public static IDLE_STOP_EVENT_NAME = 'UserIdleStop';
+	static IDLE_START_EVENT_NAME = 'UserIdleStart';
+	static IDLE_STOP_EVENT_NAME = 'UserIdleStop';
 
-	public serviceId = ITelemetryService;
+	serviceId = ITelemetryService;
+
+	private _appender: ITelemetryAppender;
+	private _commonProperties: TPromise<{ [name: string]: any; }>;
+	private _piiPaths: string[];
+	private _userOptIn: boolean;
 
-	private _configuration: ITelemetryServiceConfig;
 	private _disposables: IDisposable[] = [];
 	private _timeKeeper: TimeKeeper;
 	private _cleanupPatterns: [RegExp, string][] = [];
@@ -40,18 +44,16 @@ export class TelemetryService implements ITelemetryService {
 		config: ITelemetryServiceConfig,
 		@optional(IConfigurationService) private _configurationService: IConfigurationService
 	) {
-		this._configuration = mixin(config, <ITelemetryServiceConfig>{
-			appender: [],
-			commonProperties: TPromise.as({}),
-			piiPaths: [],
-			userOptIn: true
-		}, false);
+		this._appender = config.appender;
+		this._commonProperties = config.commonProperties || TPromise.as({});
+		this._piiPaths = config.piiPaths || [];
+		this._userOptIn = typeof config.userOptIn === 'undefined' ? true : config.userOptIn;
 
 		// static cleanup patterns for:
 		// #1 `file:///DANGEROUS/PATH/resources/app/Useful/Information`
 		// #2 // Any other file path that doesn't match the approved form above should be cleaned.
 		// #3 "Error: ENOENT; no such file or directory" is often followed with PII, clean it
-		for (let piiPath of this._configuration.piiPaths) {
+		for (let piiPath of this._piiPaths) {
 			this._cleanupPatterns.push([new RegExp(escapeRegExpCharacters(piiPath), 'gi'), '']);
 		}
 		this._cleanupPatterns.push(
@@ -67,13 +69,13 @@ export class TelemetryService implements ITelemetryService {
 		if (this._configurationService) {
 			this._updateUserOptIn();
 			this._configurationService.onDidUpdateConfiguration(this._updateUserOptIn, this, this._disposables);
-			this.publicLog('optInStatus', { optIn: this._configuration.userOptIn });
+			this.publicLog('optInStatus', { optIn: this._userOptIn });
 		}
 	}
 
 	private _updateUserOptIn(): void {
 		const config = this._configurationService.getConfiguration<any>(TELEMETRY_SECTION_ID);
-		this._configuration.userOptIn = config ? config.enableTelemetry : this._configuration.userOptIn;
+		this._userOptIn = config ? config.enableTelemetry : this._userOptIn;
 	}
 
 	private _onTelemetryTimerEventStop(events: ITimerEvent[]): void {
@@ -86,11 +88,11 @@ export class TelemetryService implements ITelemetryService {
 	}
 
 	get isOptedIn(): boolean {
-		return this._configuration.userOptIn;
+		return this._userOptIn;
 	}
 
-	public getTelemetryInfo(): TPromise<ITelemetryInfo> {
-		return this._configuration.commonProperties.then(values => {
+	getTelemetryInfo(): TPromise<ITelemetryInfo> {
+		return this._commonProperties.then(values => {
 			// well known properties
 			let sessionId = values['sessionID'];
 			let instanceId = values['common.instanceId'];
@@ -100,11 +102,11 @@ export class TelemetryService implements ITelemetryService {
 		});
 	}
 
-	public dispose(): void {
+	dispose(): void {
 		this._disposables = dispose(this._disposables);
 	}
 
-	public timedPublicLog(name: string, data?: any): ITimerEvent {
+	timedPublicLog(name: string, data?: any): ITimerEvent {
 		let topic = 'public';
 		let event = this._timeKeeper.start(topic, name);
 		if (data) {
@@ -113,13 +115,13 @@ export class TelemetryService implements ITelemetryService {
 		return event;
 	}
 
-	public publicLog(eventName: string, data?: any): TPromise<any> {
+	publicLog(eventName: string, data?: any): TPromise<any> {
 		// don't send events when the user is optout unless the event is the opt{in|out} signal
-		if (!this._configuration.userOptIn && eventName !== 'optInStatus') {
+		if (!this._userOptIn && eventName !== 'optInStatus') {
 			return TPromise.as(undefined);
 		}
 
-		return this._configuration.commonProperties.then(values => {
+		return this._commonProperties.then(values => {
 
 			// (first) add common properties
 			data = mixin(data, values);
@@ -131,9 +133,7 @@ export class TelemetryService implements ITelemetryService {
 				}
 			});
 
-			for (let appender of this._configuration.appender) {
-				appender.log(eventName, data);
-			}
+			this._appender.log(eventName, data);
 
 		}, err => {
 			// unsure what to do now...

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

@@ -5,19 +5,22 @@
 'use strict';
 
 import {IEnvironment} from 'vs/platform/workspace/common/workspace';
-import {ITelemetryAppender} from 'vs/platform/telemetry/common/telemetry';
+import {ITelemetryAppender, combinedAppender} from 'vs/platform/telemetry/common/telemetry';
 import {createAIAdapter} from 'vs/base/parts/ai/node/ai';
 
 const eventPrefix = 'monacoworkbench';
 
-export function createAppender(env: IEnvironment): ITelemetryAppender[]{
+export function createAppender(env: IEnvironment): ITelemetryAppender {
 	const result: ITelemetryAppender[] = [];
-	let {key, asimovKey} = env.aiConfig;
+	const { key, asimovKey } = env.aiConfig;
+
 	if (key) {
 		result.push(createAIAdapter(key, eventPrefix, undefined));
 	}
+
 	if (asimovKey) {
 		result.push(createAIAdapter(asimovKey, eventPrefix, undefined));
 	}
-	return result;
+
+	return combinedAppender(...result);
 }

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

@@ -85,7 +85,7 @@ suite('TelemetryService', () => {
 
 	test('Disposing', sinon.test(function () {
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 
 		return service.publicLog('testPrivateEvent').then(() => {
 			assert.equal(testAppender.getEventsCount(), 1);
@@ -98,7 +98,7 @@ suite('TelemetryService', () => {
 	// event reporting
 	test('Simple event', sinon.test(function () {
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 
 		return service.publicLog('testEvent').then(_ => {
 			assert.equal(testAppender.getEventsCount(), 1);
@@ -111,7 +111,7 @@ suite('TelemetryService', () => {
 
 	test('Event with data', sinon.test(function () {
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 
 		return service.publicLog('testEvent', {
 			'stringProp': 'property',
@@ -137,7 +137,7 @@ suite('TelemetryService', () => {
 	test('common properties added to *all* events, simple event', function () {
 		let testAppender = new TestTelemetryAppender();
 		let service = new TelemetryService({
-			appender: [testAppender],
+			appender: testAppender,
 			commonProperties: TPromise.as({ foo: 'JA!', get bar() { return Math.random(); } })
 		}, undefined);
 
@@ -155,7 +155,7 @@ suite('TelemetryService', () => {
 	test('common properties added to *all* events, event with data', function () {
 		let testAppender = new TestTelemetryAppender();
 		let service = new TelemetryService({
-			appender: [testAppender],
+			appender: testAppender,
 			commonProperties: TPromise.as({ foo: 'JA!', get bar() { return Math.random(); } })
 		}, undefined);
 
@@ -174,7 +174,7 @@ suite('TelemetryService', () => {
 
 	test('TelemetryInfo comes from properties', function () {
 		let service = new TelemetryService({
-			appender: [],
+			appender: Telemetry.NullAppender,
 			commonProperties: TPromise.as({
 				sessionID: 'one',
 				['common.instanceId']: 'two',
@@ -195,7 +195,7 @@ suite('TelemetryService', () => {
 		Timer.ENABLE_TIMER = true;
 
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 
 		let t1 = service.timedPublicLog('editorDance');
 		this.clock.tick(20);
@@ -227,7 +227,7 @@ suite('TelemetryService', () => {
 
 	test('enableTelemetry on by default', sinon.test(function () {
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 
 		return service.publicLog('testEvent').then(() => {
 			assert.equal(testAppender.getEventsCount(), 1);
@@ -244,7 +244,7 @@ suite('TelemetryService', () => {
 
 		try {
 			let testAppender = new TestTelemetryAppender();
-			let service = new TelemetryService({ appender: [testAppender] }, undefined);
+			let service = new TelemetryService({ appender: testAppender }, undefined);
 			const errorTelemetry = new ErrorTelemetry(service);
 
 
@@ -300,7 +300,7 @@ suite('TelemetryService', () => {
 		let errorStub = this.stub(window, 'onerror');
 
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 		const errorTelemetry = new ErrorTelemetry(service);
 
 		let testError = new Error('test');
@@ -326,7 +326,7 @@ suite('TelemetryService', () => {
 		let errorStub = this.stub(window, 'onerror');
 		let settings = new ErrorTestingSettings();
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 		const errorTelemetry = new ErrorTelemetry(service);
 
 		let dangerousFilenameError: any = new Error('dangerousFilename');
@@ -356,7 +356,7 @@ suite('TelemetryService', () => {
 		try {
 			let settings = new ErrorTestingSettings();
 			let testAppender = new TestTelemetryAppender();
-			let service = new TelemetryService({ appender: [testAppender] }, undefined);
+			let service = new TelemetryService({ appender: testAppender }, undefined);
 			const errorTelemetry = new ErrorTelemetry(service);
 
 			let dangerousPathWithoutImportantInfoError: any = new Error(settings.dangerousPathWithoutImportantInfo);
@@ -384,7 +384,7 @@ suite('TelemetryService', () => {
 		let errorStub = this.stub(window, 'onerror');
 		let settings = new ErrorTestingSettings();
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 		const errorTelemetry = new ErrorTelemetry(service);
 
 		let dangerousPathWithoutImportantInfoError: any = new Error('dangerousPathWithoutImportantInfo');
@@ -413,7 +413,7 @@ suite('TelemetryService', () => {
 		try {
 			let settings = new ErrorTestingSettings();
 			let testAppender = new TestTelemetryAppender();
-			let service = new TelemetryService({ appender: [testAppender] }, undefined);
+			let service = new TelemetryService({ appender: testAppender }, undefined);
 			const errorTelemetry = new ErrorTelemetry(service);
 
 			let dangerousPathWithImportantInfoError: any = new Error(settings.dangerousPathWithImportantInfo);
@@ -444,7 +444,7 @@ suite('TelemetryService', () => {
 		let errorStub = this.stub(window, 'onerror');
 		let settings = new ErrorTestingSettings();
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 		const errorTelemetry = new ErrorTelemetry(service);
 
 		let dangerousPathWithImportantInfoError: any = new Error('dangerousPathWithImportantInfo');
@@ -475,7 +475,7 @@ suite('TelemetryService', () => {
 		try {
 			let settings = new ErrorTestingSettings();
 			let testAppender = new TestTelemetryAppender();
-			let service = new TelemetryService({ appender: [testAppender] }, undefined);
+			let service = new TelemetryService({ appender: testAppender }, undefined);
 			const errorTelemetry = new ErrorTelemetry(service);
 
 			let missingModelError: any = new Error(settings.missingModelMessage);
@@ -506,7 +506,7 @@ suite('TelemetryService', () => {
 		let errorStub = this.stub(window, 'onerror');
 		let settings = new ErrorTestingSettings();
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ appender: testAppender }, undefined);
 		const errorTelemetry = new ErrorTelemetry(service);
 
 		let missingModelError: any = new Error('missingModelMessage');
@@ -538,7 +538,7 @@ suite('TelemetryService', () => {
 		try {
 			let settings = new ErrorTestingSettings();
 			let testAppender = new TestTelemetryAppender();
-			let service = new TelemetryService({ appender: [testAppender] }, undefined);
+			let service = new TelemetryService({ appender: testAppender }, undefined);
 			const errorTelemetry = new ErrorTelemetry(service);
 
 			let noSuchFileError: any = new Error(settings.noSuchFileMessage);
@@ -573,7 +573,7 @@ suite('TelemetryService', () => {
 			let errorStub = this.stub(window, 'onerror');
 			let settings = new ErrorTestingSettings();
 			let testAppender = new TestTelemetryAppender();
-			let service = new TelemetryService({ appender: [testAppender] }, undefined);
+			let service = new TelemetryService({ appender: testAppender }, undefined);
 			const errorTelemetry = new ErrorTelemetry(service);
 
 			let noSuchFileError: any = new Error('noSuchFileMessage');
@@ -603,7 +603,7 @@ suite('TelemetryService', () => {
 
 	test('Telemetry Service respects user opt-in settings', sinon.test(function () {
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ userOptIn: false, appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ userOptIn: false, appender: testAppender }, undefined);
 
 		return service.publicLog('testEvent').then(() => {
 			assert.equal(testAppender.getEventsCount(), 0);
@@ -613,7 +613,7 @@ suite('TelemetryService', () => {
 
 	test('Telemetry Service sends events when enableTelemetry is on even user optin is on', sinon.test(function () {
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ userOptIn: true, appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ userOptIn: true, appender: testAppender }, undefined);
 
 		return service.publicLog('testEvent').then(() => {
 			assert.equal(testAppender.getEventsCount(), 1);
@@ -623,7 +623,7 @@ suite('TelemetryService', () => {
 
 	test('Telemetry Service allows optin friendly events', sinon.test(function () {
 		let testAppender = new TestTelemetryAppender();
-		let service = new TelemetryService({ userOptIn: false, appender: [testAppender] }, undefined);
+		let service = new TelemetryService({ userOptIn: false, appender: testAppender }, undefined);
 
 		return service.publicLog('testEvent').then(() => {
 			assert.equal(testAppender.getEventsCount(), 0);
@@ -643,7 +643,7 @@ suite('TelemetryService', () => {
 
 		let testAppender = new TestTelemetryAppender();
 		let service = new TelemetryService({
-			appender: [testAppender]
+			appender: testAppender
 		}, {
 				serviceId: undefined,
 				hasWorkspaceConfiguration() {

src/vs/workbench/electron-browser/shell.ts

@@ -234,7 +234,7 @@ export class WorkbenchShell {
 					: TelemetryService.IDLE_START_EVENT_NAME
 			));
 
-			disposables.add(telemetryService, errorTelemetry, listener, idleMonitor, ...appender);
+			disposables.add(telemetryService, errorTelemetry, listener, idleMonitor, appender);
 		} else {
 			this.telemetryService = NullTelemetryService;
 		}