From e5dcf413bd38f36904c4bcfa384b5efa6b98d7ea Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 15 Jan 2024 20:20:03 +0530 Subject: [PATCH] report user and workspace modified configurations (#202489) --- .../standalone/browser/standaloneServices.ts | 1 - .../configuration/common/configuration.ts | 3 - .../common/configurationModels.ts | 1 - .../common/configurationService.ts | 11 --- .../telemetry/common/telemetryUtils.ts | 31 +----- .../browser/parts/sidebar/sidebarPart.ts | 12 --- .../browser/extensionsWorkbenchService.ts | 8 -- .../notebook/browser/notebookOptions.ts | 1 - .../browser/telemetry.contribution.ts | 5 +- .../browser/configurationService.ts | 95 +++++++++++++++---- 10 files changed, 81 insertions(+), 87 deletions(-) diff --git a/src/vs/editor/standalone/browser/standaloneServices.ts b/src/vs/editor/standalone/browser/standaloneServices.ts index 1e915d95a25..ec7e0075f74 100644 --- a/src/vs/editor/standalone/browser/standaloneServices.ts +++ b/src/vs/editor/standalone/browser/standaloneServices.ts @@ -655,7 +655,6 @@ export class StandaloneConfigurationService implements IConfigurationService { if (changedKeys.length > 0) { const configurationChangeEvent = new ConfigurationChangeEvent({ keys: changedKeys, overrides: [] }, previous, this._configuration); configurationChangeEvent.source = ConfigurationTarget.MEMORY; - configurationChangeEvent.sourceConfig = null; this._onDidChangeConfiguration.fire(configurationChangeEvent); } diff --git a/src/vs/platform/configuration/common/configuration.ts b/src/vs/platform/configuration/common/configuration.ts index eb419a6929a..ca25bf1cad3 100644 --- a/src/vs/platform/configuration/common/configuration.ts +++ b/src/vs/platform/configuration/common/configuration.ts @@ -68,9 +68,6 @@ export interface IConfigurationChangeEvent { readonly change: IConfigurationChange; affectsConfiguration(configuration: string, overrides?: IConfigurationOverrides): boolean; - - // Following data is used for telemetry - readonly sourceConfig: any; } export interface IConfigurationValue { diff --git a/src/vs/platform/configuration/common/configurationModels.ts b/src/vs/platform/configuration/common/configurationModels.ts index 0e609c9ba1d..09d12675520 100644 --- a/src/vs/platform/configuration/common/configurationModels.ts +++ b/src/vs/platform/configuration/common/configurationModels.ts @@ -1087,7 +1087,6 @@ export class ConfigurationChangeEvent implements IConfigurationChangeEvent { readonly affectedKeys = new Set(); source!: ConfigurationTarget; - sourceConfig: any; constructor(readonly change: IConfigurationChange, private readonly previous: { workspace?: Workspace; data: IConfigurationData } | undefined, private readonly currentConfiguraiton: Configuration, private readonly currentWorkspace?: Workspace) { for (const key of change.keys) { diff --git a/src/vs/platform/configuration/common/configurationService.ts b/src/vs/platform/configuration/common/configurationService.ts index a985ac1bf14..c040a98c145 100644 --- a/src/vs/platform/configuration/common/configurationService.ts +++ b/src/vs/platform/configuration/common/configurationService.ts @@ -159,19 +159,8 @@ export class ConfigurationService extends Disposable implements IConfigurationSe private trigger(configurationChange: IConfigurationChange, previous: IConfigurationData, source: ConfigurationTarget): void { const event = new ConfigurationChangeEvent(configurationChange, { data: previous }, this.configuration); event.source = source; - event.sourceConfig = this.getTargetConfiguration(source); this._onDidChangeConfiguration.fire(event); } - - private getTargetConfiguration(target: ConfigurationTarget): any { - switch (target) { - case ConfigurationTarget.DEFAULT: - return this.configuration.defaults.contents; - case ConfigurationTarget.USER: - return this.configuration.localUserConfiguration.contents; - } - return {}; - } } class ConfigurationEditing { diff --git a/src/vs/platform/telemetry/common/telemetryUtils.ts b/src/vs/platform/telemetry/common/telemetryUtils.ts index fa443c0fd69..5deb494c3b4 100644 --- a/src/vs/platform/telemetry/common/telemetryUtils.ts +++ b/src/vs/platform/telemetry/common/telemetryUtils.ts @@ -3,12 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { IDisposable } from 'vs/base/common/lifecycle'; import { cloneAndChange, safeStringify } from 'vs/base/common/objects'; import { isObject } from 'vs/base/common/types'; -import { Event } from 'vs/base/common/event'; import { URI } from 'vs/base/common/uri'; -import { ConfigurationTarget, ConfigurationTargetToString, IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IEnvironmentService } from 'vs/platform/environment/common/environment'; import { IProductService } from 'vs/platform/product/common/productService'; import { getRemoteName } from 'vs/platform/remote/common/remoteHosts'; @@ -81,33 +79,6 @@ export interface URIDescriptor { path?: string; } -export function configurationTelemetry(telemetryService: ITelemetryService, configurationService: IConfigurationService): IDisposable { - // Debounce the event by 1000 ms and merge all affected keys into one event - const debouncedConfigService = Event.debounce(configurationService.onDidChangeConfiguration, (last, cur) => { - const newAffectedKeys: ReadonlySet = last ? new Set([...last.affectedKeys, ...cur.affectedKeys]) : cur.affectedKeys; - return { ...cur, affectedKeys: newAffectedKeys }; - }, 1000, true); - - return debouncedConfigService(event => { - if (event.source !== ConfigurationTarget.DEFAULT) { - type UpdateConfigurationClassification = { - owner: 'lramos15, sbatten'; - comment: 'Event which fires when user updates settings'; - configurationSource: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'What configuration file was updated i.e user or workspace' }; - configurationKeys: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'What configuration keys were updated' }; - }; - type UpdateConfigurationEvent = { - configurationSource: string; - configurationKeys: string[]; - }; - telemetryService.publicLog2('updateConfiguration', { - configurationSource: ConfigurationTargetToString(event.source), - configurationKeys: Array.from(event.affectedKeys) - }); - } - }); -} - /** * Determines whether or not we support logging telemetry. * This checks if the product is capable of collecting telemetry but not whether or not it can send it diff --git a/src/vs/workbench/browser/parts/sidebar/sidebarPart.ts b/src/vs/workbench/browser/parts/sidebar/sidebarPart.ts index 2b70ce85cee..e0b21f32517 100644 --- a/src/vs/workbench/browser/parts/sidebar/sidebarPart.ts +++ b/src/vs/workbench/browser/parts/sidebar/sidebarPart.ts @@ -28,8 +28,6 @@ import { HoverPosition } from 'vs/base/browser/ui/hover/hoverWidget'; import { IPaneCompositeBarOptions } from 'vs/workbench/browser/parts/paneCompositeBar'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { Action2, IMenuService, registerAction2 } from 'vs/platform/actions/common/actions'; -import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; -import { ILifecycleService, LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; import { Separator } from 'vs/base/common/actions'; import { ToggleActivityBarVisibilityActionId } from 'vs/workbench/browser/actions/layoutActions'; import { localize } from 'vs/nls'; @@ -79,8 +77,6 @@ export class SidebarPart extends AbstractPaneCompositePart { @IContextKeyService contextKeyService: IContextKeyService, @IExtensionService extensionService: IExtensionService, @IConfigurationService private readonly configurationService: IConfigurationService, - @ITelemetryService telemetryService: ITelemetryService, - @ILifecycleService lifecycleService: ILifecycleService, @IMenuService menuService: IMenuService, ) { super( @@ -114,14 +110,6 @@ export class SidebarPart extends AbstractPaneCompositePart { })); this.registerActions(); - - lifecycleService.when(LifecyclePhase.Eventually).then(() => { - telemetryService.publicLog2<{ location: string }, { - owner: 'sandy081'; - location: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Locaiton where the activity bar is shown' }; - comment: 'This is used to know where activity bar is shown in the workbench.'; - }>('activityBar:location', { location: configurationService.getValue(LayoutSettings.ACTIVITY_BAR_LOCATION) }); - }); } private onDidChangeActivityBarLocation(): void { diff --git a/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts b/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts index 889b216554b..65b2e09affa 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts @@ -804,14 +804,6 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension urlService.registerHandler(this); this.whenInitialized = this.initialize(); - - lifecycleService.when(LifecyclePhase.Eventually).then(() => { - telemetryService.publicLog2<{ mode: string }, { - owner: 'sandy081'; - mode: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Auto Update Mode' }; - comment: 'This is used to know if extensions are getting auto updated or not'; - }>('extensions:autoupdate', { mode: `${this.getAutoUpdateValue()}` }); - }); } private async initialize(): Promise { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookOptions.ts b/src/vs/workbench/contrib/notebook/browser/notebookOptions.ts index 9e4c743e3ef..eb4bbe768ac 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookOptions.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookOptions.ts @@ -269,7 +269,6 @@ export class NotebookOptions extends Disposable { source: ConfigurationTarget.DEFAULT, affectedKeys: new Set([NotebookSetting.insertToolbarLocation]), change: { keys: [NotebookSetting.insertToolbarLocation], overrides: [] }, - sourceConfig: undefined }); } } diff --git a/src/vs/workbench/contrib/telemetry/browser/telemetry.contribution.ts b/src/vs/workbench/contrib/telemetry/browser/telemetry.contribution.ts index 91a902f4b20..5293305cc56 100644 --- a/src/vs/workbench/contrib/telemetry/browser/telemetry.contribution.ts +++ b/src/vs/workbench/contrib/telemetry/browser/telemetry.contribution.ts @@ -15,7 +15,7 @@ import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/ import { language } from 'vs/base/common/platform'; import { Disposable } from 'vs/base/common/lifecycle'; import ErrorTelemetry from 'vs/platform/telemetry/browser/errorTelemetry'; -import { configurationTelemetry, TelemetryTrustedValue } from 'vs/platform/telemetry/common/telemetryUtils'; +import { TelemetryTrustedValue } from 'vs/platform/telemetry/common/telemetryUtils'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { ITextFileService, ITextFileSaveEvent, ITextFileResolveEvent } from 'vs/workbench/services/textfile/common/textfiles'; import { extname, basename, isEqual, isEqualOrParent } from 'vs/base/common/resources'; @@ -126,9 +126,6 @@ export class TelemetryContribution extends Disposable implements IWorkbenchContr // Error Telemetry this._register(new ErrorTelemetry(telemetryService)); - // Configuration Telemetry - this._register(configurationTelemetry(telemetryService, configurationService)); - // Files Telemetry this._register(textFileService.files.onDidResolve(e => this.onTextFileModelResolved(e))); this._register(textFileService.files.onDidSave(e => this.onTextFileModelSaved(e))); diff --git a/src/vs/workbench/services/configuration/browser/configurationService.ts b/src/vs/workbench/services/configuration/browser/configurationService.ts index 7784cde557f..58761151288 100644 --- a/src/vs/workbench/services/configuration/browser/configurationService.ts +++ b/src/vs/workbench/services/configuration/browser/configurationService.ts @@ -37,7 +37,7 @@ import { delta, distinct, equals as arrayEquals } from 'vs/base/common/arrays'; import { IStringDictionary } from 'vs/base/common/collections'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { IWorkbenchAssignmentService } from 'vs/workbench/services/assignment/common/assignmentService'; -import { isUndefined } from 'vs/base/common/types'; +import { isBoolean, isNumber, isString, isUndefined } from 'vs/base/common/types'; import { localize } from 'vs/nls'; import { DidChangeUserDataProfileEvent, IUserDataProfileService } from 'vs/workbench/services/userDataProfile/common/userDataProfile'; import { IPolicyService, NullPolicyService } from 'vs/platform/policy/common/policy'; @@ -47,6 +47,7 @@ import { IBrowserWorkbenchEnvironmentService } from 'vs/workbench/services/envir import { workbenchConfigurationNodeBase } from 'vs/workbench/common/configuration'; import { mainWindow } from 'vs/base/browser/window'; import { runWhenWindowIdle } from 'vs/base/browser/dom'; +import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; function getLocalUserConfigurationScopes(userDataProfile: IUserDataProfile, hasRemote: boolean): ConfigurationScope[] | undefined { return (userDataProfile.isDefault || userDataProfile.useDefaultFlags?.settings) @@ -1018,7 +1019,7 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat } if (overrides?.overrideIdentifiers?.length && overrides.overrideIdentifiers.length > 1) { - const configurationModel = this.getConfigurationModel(editableConfigurationTarget, overrides.resource); + const configurationModel = this.getConfigurationModelForEditableConfigurationTarget(editableConfigurationTarget, overrides.resource); if (configurationModel) { const overrideIdentifiers = overrides.overrideIdentifiers.sort(); const existingOverrides = configurationModel.overrides.find(override => arrayEquals([...override.identifiers].sort(), overrideIdentifiers)); @@ -1052,7 +1053,7 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat } } - private getConfigurationModel(target: EditableConfigurationTarget, resource?: URI | null): ConfigurationModel | undefined { + private getConfigurationModelForEditableConfigurationTarget(target: EditableConfigurationTarget, resource?: URI | null): ConfigurationModel | undefined { switch (target) { case EditableConfigurationTarget.USER_LOCAL: return this._configuration.localUserConfiguration; case EditableConfigurationTarget.USER_REMOTE: return this._configuration.remoteUserConfiguration; @@ -1061,6 +1062,16 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat } } + getConfigurationModel(target: ConfigurationTarget, resource?: URI | null): ConfigurationModel | undefined { + switch (target) { + case ConfigurationTarget.USER_LOCAL: return this._configuration.localUserConfiguration; + case ConfigurationTarget.USER_REMOTE: return this._configuration.remoteUserConfiguration; + case ConfigurationTarget.WORKSPACE: return this._configuration.workspaceConfiguration; + case ConfigurationTarget.WORKSPACE_FOLDER: return resource ? this._configuration.folderConfigurations.get(resource) : undefined; + default: return undefined; + } + } + private deriveConfigurationTargets(key: string, value: any, inspect: IConfigurationValue): ConfigurationTarget[] { if (equals(value, inspect.value)) { return []; @@ -1095,23 +1106,10 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat } const configurationChangeEvent = new ConfigurationChangeEvent(change, previous, this._configuration, this.workspace); configurationChangeEvent.source = target; - configurationChangeEvent.sourceConfig = this.getTargetConfiguration(target); this._onDidChangeConfiguration.fire(configurationChangeEvent); } } - private getTargetConfiguration(target: ConfigurationTarget): any { - switch (target) { - case ConfigurationTarget.DEFAULT: - return this._configuration.defaults.contents; - case ConfigurationTarget.USER: - return this._configuration.userConfiguration.contents; - case ConfigurationTarget.WORKSPACE: - return this._configuration.workspaceConfiguration.contents; - } - return {}; - } - private toEditableConfigurationTarget(target: ConfigurationTarget, key: string): EditableConfigurationTarget | null { if (target === ConfigurationTarget.USER) { if (this.remoteUserConfiguration) { @@ -1322,6 +1320,70 @@ class ResetConfigurationDefaultsOverridesCache extends Disposable implements IWo } } +class ConfigurationTelemetryContribution extends Disposable implements IWorkbenchContribution { + + private readonly configurationRegistry = Registry.as(Extensions.Configuration); + + constructor( + @IConfigurationService private readonly configurationService: WorkspaceService, + @ITelemetryService private readonly telemetryService: ITelemetryService, + ) { + super(); + + const { user, workspace } = configurationService.keys(); + for (const key of user) { + this.reportConfiguration(key, ConfigurationTarget.USER_LOCAL); + } + for (const key of workspace) { + this.reportConfiguration(key, ConfigurationTarget.WORKSPACE); + } + } + + private reportConfiguration(key: string, target: ConfigurationTarget): void { + type UpdateConfigurationClassification = { + owner: 'sandy081'; + comment: 'Event which fires for updated configurations'; + source: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'What configuration file was updated i.e user or workspace' }; + key: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'What configuration key was updated' }; + value?: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Value of the key that was updated' }; + }; + type UpdateConfigurationEvent = { + source: string; + key: string; + value?: any; + }; + this.telemetryService.publicLog2('updateConfiguration', { + source: ConfigurationTargetToString(target), + key, + value: this.getValueToReport(key, target) + }); + } + + private getValueToReport(key: string, target: ConfigurationTarget): any { + const schema = this.configurationRegistry.getConfigurationProperties()[key]; + if (!schema) { + return undefined; + } + const configurationModel = this.configurationService.getConfigurationModel(target); + const value = configurationModel?.getValue(key); + if (isNumber(value) || isBoolean(value)) { + return value; + } + if (isString(value)) { + if (schema.enum?.includes(value)) { + return value; + } + return undefined; + } + if (Array.isArray(value)) { + if (value.every(v => isNumber(v) || isBoolean(v) || (isString(v) && schema.enum?.includes(v)))) { + return value; + } + } + return undefined; + } +} + class UpdateExperimentalSettingsDefaults extends Disposable implements IWorkbenchContribution { private readonly processedExperimentalSettings = new Set(); @@ -1364,6 +1426,7 @@ const workbenchContributionsRegistry = Registry.as(Extensions.Configuration); configurationRegistry.registerConfiguration({