- move telemetry logging to abstract synchroniser

- improve logging

src/vs/platform/userDataSync/common/abstractSynchronizer.ts

@@ -13,6 +13,11 @@ import { joinPath, dirname } from 'vs/base/common/resources';
 import { toLocalISOString } from 'vs/base/common/date';
 import { ThrottledDelayer } from 'vs/base/common/async';
 import { Emitter, Event } from 'vs/base/common/event';
+import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
+
+type SyncConflictsClassification = {
+	source?: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
+};
 
 export abstract class AbstractSynchroniser extends Disposable {
 
@@ -34,6 +39,7 @@ export abstract class AbstractSynchroniser extends Disposable {
 		@IFileService protected readonly fileService: IFileService,
 		@IEnvironmentService environmentService: IEnvironmentService,
 		@IUserDataSyncStoreService protected readonly userDataSyncStoreService: IUserDataSyncStoreService,
+		@ITelemetryService private readonly telemetryService: ITelemetryService,
 	) {
 		super();
 		this.syncFolder = joinPath(environmentService.userDataSyncHome, source);
@@ -43,8 +49,17 @@ export abstract class AbstractSynchroniser extends Disposable {
 
 	protected setStatus(status: SyncStatus): void {
 		if (this._status !== status) {
+			const oldStatus = this._status;
 			this._status = status;
 			this._onDidChangStatus.fire(status);
+			if (status === SyncStatus.HasConflicts) {
+				// Log to telemetry when there is a sync conflict
+				this.telemetryService.publicLog2<{ source: string }, SyncConflictsClassification>('sync/conflictsDetected', { source: this.source });
+			}
+			if (oldStatus === SyncStatus.HasConflicts && status === SyncStatus.Idle) {
+				// Log to telemetry when conflicts are resolved
+				this.telemetryService.publicLog2<{ source: string }, SyncConflictsClassification>('sync/conflictsResolved', { source: this.source });
+			}
 		}
 	}
 
@@ -112,8 +127,9 @@ export abstract class AbstractFileSynchroniser extends AbstractSynchroniser {
 		@IFileService fileService: IFileService,
 		@IEnvironmentService environmentService: IEnvironmentService,
 		@IUserDataSyncStoreService userDataSyncStoreService: IUserDataSyncStoreService,
+		@ITelemetryService telemetryService: ITelemetryService,
 	) {
-		super(source, fileService, environmentService, userDataSyncStoreService);
+		super(source, fileService, environmentService, userDataSyncStoreService, telemetryService);
 		this._register(this.fileService.watch(dirname(file)));
 		this._register(Event.filter(this.fileService.onFileChanges, e => e.contains(file))(() => this._onDidChangeLocal.fire()));
 	}

src/vs/platform/userDataSync/common/extensionsSync.ts

@@ -15,6 +15,7 @@ import { localize } from 'vs/nls';
 import { merge } from 'vs/platform/userDataSync/common/extensionsMerge';
 import { isNonEmptyArray } from 'vs/base/common/arrays';
 import { AbstractSynchroniser } from 'vs/platform/userDataSync/common/abstractSynchronizer';
+import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 
 interface ISyncPreviewResult {
 	readonly added: ISyncExtension[];
@@ -41,8 +42,9 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
 		@IUserDataSyncLogService private readonly logService: IUserDataSyncLogService,
 		@IExtensionGalleryService private readonly extensionGalleryService: IExtensionGalleryService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
+		@ITelemetryService telemetryService: ITelemetryService,
 	) {
-		super(SyncSource.Extensions, fileService, environmentService, userDataSyncStoreService);
+		super(SyncSource.Extensions, fileService, environmentService, userDataSyncStoreService, telemetryService);
 		this._register(
 			Event.debounce(
 				Event.any<any>(
@@ -114,15 +116,15 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
 
 	async sync(): Promise<void> {
 		if (!this.configurationService.getValue<boolean>('sync.enableExtensions')) {
-			this.logService.trace('Extensions: Skipping synchronizing extensions as it is disabled.');
+			this.logService.info('Extensions: Skipping synchronizing extensions as it is disabled.');
 			return;
 		}
 		if (!this.extensionGalleryService.isEnabled()) {
-			this.logService.trace('Extensions: Skipping synchronizing extensions as gallery is disabled.');
+			this.logService.info('Extensions: Skipping synchronizing extensions as gallery is disabled.');
 			return;
 		}
 		if (this.status !== SyncStatus.Idle) {
-			this.logService.trace('Extensions: Skipping synchronizing extensions as it is running already.');
+			this.logService.info('Extensions: Skipping synchronizing extensions as it is running already.');
 			return;
 		}
 

src/vs/platform/userDataSync/common/globalStateSync.ts

@@ -15,6 +15,7 @@ import { merge } from 'vs/platform/userDataSync/common/globalStateMerge';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { parse } from 'vs/base/common/json';
 import { AbstractSynchroniser } from 'vs/platform/userDataSync/common/abstractSynchronizer';
+import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 
 const argvProperties: string[] = ['locale'];
 
@@ -33,8 +34,9 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
 		@IUserDataSyncLogService private readonly logService: IUserDataSyncLogService,
 		@IEnvironmentService private readonly environmentService: IEnvironmentService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
+		@ITelemetryService telemetryService: ITelemetryService,
 	) {
-		super(SyncSource.GlobalState, fileService, environmentService, userDataSyncStoreService);
+		super(SyncSource.GlobalState, fileService, environmentService, userDataSyncStoreService, telemetryService);
 		this._register(this.fileService.watch(dirname(this.environmentService.argvResource)));
 		this._register(Event.filter(this.fileService.onFileChanges, e => e.contains(this.environmentService.argvResource))(() => this._onDidChangeLocal.fire()));
 	}
@@ -98,12 +100,11 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
 
 	async sync(): Promise<void> {
 		if (!this.configurationService.getValue<boolean>('sync.enableUIState')) {
-			this.logService.trace('UI State: Skipping synchronizing UI state as it is disabled.');
+			this.logService.info('UI State: Skipping synchronizing UI state as it is disabled.');
 			return;
 		}
-
 		if (this.status !== SyncStatus.Idle) {
-			this.logService.trace('UI State: Skipping synchronizing ui state as it is running already.');
+			this.logService.info('UI State: Skipping synchronizing ui state as it is running already.');
 			return;
 		}
 

src/vs/platform/userDataSync/common/keybindingsSync.ts

@@ -18,6 +18,7 @@ import { isUndefined } from 'vs/base/common/types';
 import { FormattingOptions } from 'vs/base/common/jsonFormatter';
 import { isNonEmptyArray } from 'vs/base/common/arrays';
 import { AbstractFileSynchroniser } from 'vs/platform/userDataSync/common/abstractSynchronizer';
+import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 
 interface ISyncContent {
 	mac?: string;
@@ -46,8 +47,9 @@ export class KeybindingsSynchroniser extends AbstractFileSynchroniser implements
 		@IFileService fileService: IFileService,
 		@IEnvironmentService private readonly environmentService: IEnvironmentService,
 		@IUserDataSyncUtilService private readonly userDataSyncUtilService: IUserDataSyncUtilService,
+		@ITelemetryService telemetryService: ITelemetryService,
 	) {
-		super(environmentService.keybindingsResource, SyncSource.Keybindings, fileService, environmentService, userDataSyncStoreService);
+		super(environmentService.keybindingsResource, SyncSource.Keybindings, fileService, environmentService, userDataSyncStoreService, telemetryService);
 	}
 
 	protected getRemoteDataResourceKey(): string { return 'keybindings'; }
@@ -137,18 +139,16 @@ export class KeybindingsSynchroniser extends AbstractFileSynchroniser implements
 
 	async sync(): Promise<void> {
 		if (!this.configurationService.getValue<boolean>('sync.enableKeybindings')) {
-			this.logService.trace('Keybindings: Skipping synchronizing keybindings as it is disabled.');
+			this.logService.info('Keybindings: Skipping synchronizing keybindings as it is disabled.');
 			return;
 		}
-
 		if (this.status !== SyncStatus.Idle) {
-			this.logService.trace('Keybindings: Skipping synchronizing keybindings as it is running already.');
+			this.logService.info('Keybindings: Skipping synchronizing keybindings as it is running already.');
 			return;
 		}
 
 		this.logService.trace('Keybindings: Started synchronizing keybindings...');
 		this.setStatus(SyncStatus.Syncing);
-
 		return this.doSync();
 	}
 

src/vs/platform/userDataSync/common/settingsSync.ts

@@ -20,6 +20,7 @@ import * as objects from 'vs/base/common/objects';
 import { isEmptyObject, isUndefinedOrNull } from 'vs/base/common/types';
 import { edit } from 'vs/platform/userDataSync/common/content';
 import { AbstractFileSynchroniser } from 'vs/platform/userDataSync/common/abstractSynchronizer';
+import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 
 interface ISyncPreviewResult {
 	readonly fileContent: IFileContent | null;
@@ -50,8 +51,9 @@ export class SettingsSynchroniser extends AbstractFileSynchroniser implements IS
 		@IUserDataSyncLogService private readonly logService: IUserDataSyncLogService,
 		@IUserDataSyncUtilService private readonly userDataSyncUtilService: IUserDataSyncUtilService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
+		@ITelemetryService telemetryService: ITelemetryService,
 	) {
-		super(environmentService.settingsResource, SyncSource.Settings, fileService, environmentService, userDataSyncStoreService);
+		super(environmentService.settingsResource, SyncSource.Settings, fileService, environmentService, userDataSyncStoreService, telemetryService);
 	}
 
 	protected getRemoteDataResourceKey(): string { return 'settings'; }
@@ -165,12 +167,11 @@ export class SettingsSynchroniser extends AbstractFileSynchroniser implements IS
 
 	async sync(): Promise<void> {
 		if (!this.configurationService.getValue<boolean>('sync.enableSettings')) {
-			this.logService.trace('Settings: Skipping synchronizing settings as it is disabled.');
+			this.logService.info('Settings: Skipping synchronizing settings as it is disabled.');
 			return;
 		}
-
 		if (this.status !== SyncStatus.Idle) {
-			this.logService.trace('Settings: Skipping synchronizing settings as it is running already.');
+			this.logService.info('Settings: Skipping synchronizing settings as it is running already.');
 			return;
 		}
 

src/vs/platform/userDataSync/common/userDataSyncService.ts

@@ -14,10 +14,6 @@ import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { localize } from 'vs/nls';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 
-type SyncConflictsClassification = {
-	source?: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
-};
-
 type SyncErrorClassification = {
 	source: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
 };
@@ -259,18 +255,8 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
 	private updateStatus(): void {
 		const status = this.computeStatus();
 		if (this._status !== status) {
-			const oldStatus = this._status;
-			const oldConflictsSource = this._conflictsSource;
 			this._conflictsSource = this.computeConflictsSource();
 			this._status = status;
-			if (status === SyncStatus.HasConflicts) {
-				// Log to telemetry when there is a sync conflict
-				this.telemetryService.publicLog2<{ source: string }, SyncConflictsClassification>('sync/conflictsDetected', { source: this._conflictsSource! });
-			}
-			if (oldStatus === SyncStatus.HasConflicts && status === SyncStatus.Idle) {
-				// Log to telemetry when conflicts are resolved
-				this.telemetryService.publicLog2<{ source: string }, SyncConflictsClassification>('sync/conflictsResolved', { source: oldConflictsSource! });
-			}
 			this._onDidChangeStatus.fire(status);
 		}
 	}