From 467c9990926539155a6c444187bf3a0f0485699d Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Sun, 26 Jan 2020 17:47:38 +0100 Subject: [PATCH] Fix #86297 --- .../common/userDataSyncService.ts | 23 +++++-- .../userDataSync/browser/userDataSync.ts | 61 ++++++++++--------- 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/src/vs/platform/userDataSync/common/userDataSyncService.ts b/src/vs/platform/userDataSync/common/userDataSyncService.ts index 270bab9d5a2..770f57e941f 100644 --- a/src/vs/platform/userDataSync/common/userDataSyncService.ts +++ b/src/vs/platform/userDataSync/common/userDataSyncService.ts @@ -14,6 +14,11 @@ import { KeybindingsSynchroniser } from 'vs/platform/userDataSync/common/keybind import { GlobalStateSynchroniser } from 'vs/platform/userDataSync/common/globalStateSync'; 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 }; +}; export class UserDataSyncService extends Disposable implements IUserDataSyncService { @@ -41,6 +46,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ @ISettingsSyncService private readonly settingsSynchroniser: ISettingsSyncService, @IUserDataSyncLogService private readonly logService: IUserDataSyncLogService, @IUserDataAuthTokenService private readonly userDataAuthTokenService: IUserDataAuthTokenService, + @ITelemetryService private readonly telemetryService: ITelemetryService, ) { super(); this.keybindingsSynchroniser = this._register(this.instantiationService.createInstance(KeybindingsSynchroniser)); @@ -252,13 +258,20 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ } private updateStatus(): void { - this._conflictsSource = this.computeConflictsSource(); - this.setStatus(this.computeStatus()); - } - - private setStatus(status: SyncStatus): 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); } } diff --git a/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts b/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts index c6f4d0e9352..34eaab54565 100644 --- a/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts +++ b/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts @@ -41,11 +41,10 @@ import type { IEditorContribution } from 'vs/editor/common/editorCommon'; import type { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { FloatingClickWidget } from 'vs/workbench/browser/parts/editor/editorWidgets'; import { registerEditorContribution } from 'vs/editor/browser/editorExtensions'; -import { areSame } from 'vs/platform/userDataSync/common/settingsMerge'; -import { getIgnoredSettings } from 'vs/platform/userDataSync/common/settingsSync'; import type { IEditorInput } from 'vs/workbench/common/editor'; import { Action } from 'vs/base/common/actions'; import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences'; +import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; const enum AuthStatus { Initializing = 'Initializing', @@ -65,6 +64,14 @@ function getSyncAreaLabel(source: SyncSource): string { } } +type FirstTimeSyncClassification = { + action: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true }; +}; + +type SyncErrorClassification = { + source: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true }; +}; + export class UserDataSyncWorkbenchContribution extends Disposable implements IWorkbenchContribution { private static readonly ENABLEMENT_SETTING = 'sync.enable'; @@ -92,8 +99,9 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo @IOutputService private readonly outputService: IOutputService, @IUserDataAuthTokenService private readonly userDataAuthTokenService: IUserDataAuthTokenService, @IUserDataAutoSyncService userDataAutoSyncService: IUserDataAutoSyncService, - @ITextModelService private readonly textModelResolverService: ITextModelService, + @ITextModelService textModelResolverService: ITextModelService, @IPreferencesService private readonly preferencesService: IPreferencesService, + @ITelemetryService private readonly telemetryService: ITelemetryService, ) { super(); this.userDataSyncStore = getUserDataSyncStore(configurationService); @@ -253,6 +261,7 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo private onAutoSyncError(code: UserDataSyncErrorCode, source?: SyncSource): void { switch (code) { case UserDataSyncErrorCode.TooManyFailures: + this.telemetryService.publicLog2('sync/errorTooMany'); this.disableSync(); this.notificationService.notify({ severity: Severity.Error, @@ -263,6 +272,7 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo }); return; case UserDataSyncErrorCode.TooLarge: + this.telemetryService.publicLog2<{ source: string }, SyncErrorClassification>('sync/errorTooLarge', { source: source! }); if (source === SyncSource.Keybindings || source === SyncSource.Settings) { const sourceArea = getSyncAreaLabel(source); this.disableSync(); @@ -418,9 +428,17 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo } ); switch (result.choice) { - case 0: await this.userDataSyncService.sync(); break; - case 1: throw canceled(); - case 2: await this.userDataSyncService.pull(); break; + case 0: + this.telemetryService.publicLog2<{ action: string }, FirstTimeSyncClassification>('sync/firstTimeSync', { action: 'merge' }); + await this.userDataSyncService.sync(); + break; + case 1: + this.telemetryService.publicLog2<{ action: string }, FirstTimeSyncClassification>('sync/firstTimeSync', { action: 'cancelled' }); + throw canceled(); + case 2: + this.telemetryService.publicLog2<{ action: string }, FirstTimeSyncClassification>('sync/firstTimeSync', { action: 'replace-local' }); + await this.userDataSyncService.pull(); + break; } } @@ -441,6 +459,7 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo if (result.confirmed) { await this.disableSync(); if (result.checkboxChecked) { + this.telemetryService.publicLog2('sync/turnOffEveryWhere'); await this.userDataSyncService.reset(); } } @@ -492,7 +511,7 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo } if (previewResource) { const remoteContentResource = toRemoteContentResource(this.userDataSyncService.conflictsSource!); - const editor = await this.editorService.openEditor({ + await this.editorService.openEditor({ leftResource: remoteContentResource, rightResource: previewResource, label, @@ -502,27 +521,6 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo revealIfVisible: true, }, }); - if (editor?.input) { - const disposable = editor.input.onDispose(async () => { - disposable.dispose(); - const source = getSyncSourceFromRemoteContentResource(remoteContentResource); - if (source === undefined || this.userDataSyncService.conflictsSource !== source) { - return; - } - - const remoteModelRef = await this.textModelResolverService.createModelReference(remoteContentResource); - const previewModelRef = await this.textModelResolverService.createModelReference(previewResource!); - const remoteModelContent = remoteModelRef.object.textEditorModel.getValue(); - const preivewContent = previewModelRef.object.textEditorModel.getValue(); - remoteModelRef.dispose(); - previewModelRef.dispose(); - if (remoteModelContent !== preivewContent - || (source === SyncSource.Settings && !areSame(remoteModelContent, preivewContent, getIgnoredSettings(this.configurationService)))) { - return; - } - await this.userDataSyncService.resolveConflictsAndContinueSync(preivewContent); - }); - } } } @@ -682,6 +680,11 @@ class UserDataRemoteContentProvider implements ITextModelContentProvider { } } +type SyncConflictsClassification = { + source: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true }; + action: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true }; +}; + class AcceptChangesContribution extends Disposable implements IEditorContribution { static get(editor: ICodeEditor): AcceptChangesContribution { @@ -700,6 +703,7 @@ class AcceptChangesContribution extends Disposable implements IEditorContributio @INotificationService private readonly notificationService: INotificationService, @IDialogService private readonly dialogService: IDialogService, @IConfigurationService private readonly configurationService: IConfigurationService, + @ITelemetryService private readonly telemetryService: ITelemetryService ) { super(); @@ -760,6 +764,7 @@ class AcceptChangesContribution extends Disposable implements IEditorContributio if (model) { const conflictsSource = this.userDataSyncService.conflictsSource; const syncSource = getSyncSourceFromRemoteContentResource(model.uri); + this.telemetryService.publicLog2<{ source: string, action: string }, SyncConflictsClassification>('sync/handleConflicts', { source: conflictsSource!, action: syncSource !== undefined ? 'replaceLocal' : 'apply' }); if (syncSource !== undefined) { const syncAreaLabel = getSyncAreaLabel(syncSource); const result = await this.dialogService.confirm({ -- GitLab