From 84de86102ab9c1917f814499d6f51b1390aef6a3 Mon Sep 17 00:00:00 2001 From: Logan Ramos Date: Mon, 11 Oct 2021 10:07:58 -0400 Subject: [PATCH 1/5] Address #134660 --- .../telemetry/common/telemetryService.ts | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/vs/platform/telemetry/common/telemetryService.ts b/src/vs/platform/telemetry/common/telemetryService.ts index dd93c6426b717..79dcfa754a005 100644 --- a/src/vs/platform/telemetry/common/telemetryService.ts +++ b/src/vs/platform/telemetry/common/telemetryService.ts @@ -76,30 +76,24 @@ export class TelemetryService implements ITelemetryService { }; this.publicLog2<{ usingFallbackGuid: boolean }, MachineIdFallbackClassification>('machineIdFallback', { usingFallbackGuid: !isHashedId }); }); - - // TODO @sbatten @lramos15 bring this code in after one iteration - // Once the service initializes we update the telemetry value to the new format - // this._convertOldTelemetrySettingToNew(); - // this._configurationService.onDidChangeConfiguration(e => { - // if (e.affectsConfiguration(TELEMETRY_OLD_SETTING_ID)) { - // this._convertOldTelemetrySettingToNew(); - // } - // }, this); } setExperimentProperty(name: string, value: string): void { this._experimentProperties[name] = value; } - // TODO: @sbatten @lramos15 bring this code in after one iteration - // private _convertOldTelemetrySettingToNew(): void { - // const telemetryValue = this._configurationService.getValue(TELEMETRY_OLD_SETTING_ID); - // if (typeof telemetryValue === 'boolean') { - // this._configurationService.updateValue(TELEMETRY_SETTING_ID, telemetryValue ? 'true' : 'false'); - // } - // } + private _convertOldTelemetrySettingToNew(): void { + const oldTelemetryValue = this._configurationService.getValue(TELEMETRY_OLD_SETTING_ID); + const crashReporterSetting = this._configurationService.getValue('telemetry.enableCrashReporter'); + if (oldTelemetryValue === false || crashReporterSetting === false) { + this._configurationService.updateValue(TELEMETRY_SETTING_ID, TelemetryConfiguration.OFF); + this._configurationService.updateValue(TELEMETRY_OLD_SETTING_ID, undefined); + this._configurationService.updateValue('telemetry.enableCrashReporter', undefined); + } + } private _updateTelemetryLevel(): void { + this._convertOldTelemetrySettingToNew(); this._telemetryLevel = getTelemetryLevel(this._configurationService); } From 1f916516d8bcaa7b2667130aebe904a3aaf5a255 Mon Sep 17 00:00:00 2001 From: Logan Ramos Date: Mon, 11 Oct 2021 17:31:54 -0400 Subject: [PATCH 2/5] Further address #134660 --- .../platform/telemetry/common/telemetryService.ts | 14 ++++++-------- .../electron-sandbox/desktop.contribution.ts | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/vs/platform/telemetry/common/telemetryService.ts b/src/vs/platform/telemetry/common/telemetryService.ts index 79dcfa754a005..64e2a08eefad5 100644 --- a/src/vs/platform/telemetry/common/telemetryService.ts +++ b/src/vs/platform/telemetry/common/telemetryService.ts @@ -87,8 +87,6 @@ export class TelemetryService implements ITelemetryService { const crashReporterSetting = this._configurationService.getValue('telemetry.enableCrashReporter'); if (oldTelemetryValue === false || crashReporterSetting === false) { this._configurationService.updateValue(TELEMETRY_SETTING_ID, TelemetryConfiguration.OFF); - this._configurationService.updateValue(TELEMETRY_OLD_SETTING_ID, undefined); - this._configurationService.updateValue('telemetry.enableCrashReporter', undefined); } } @@ -261,14 +259,14 @@ Registry.as(Extensions.Configuration).registerConfigurat 'type': 'string', 'enum': [TelemetryConfiguration.ON, TelemetryConfiguration.ERROR, TelemetryConfiguration.OFF], 'enumDescriptions': [ - localize('telemetry.enableTelemetry.default', "Enables all telemetry data to be collected."), - localize('telemetry.enableTelemetry.error', "Enables only error telemetry data and not general usage data."), - localize('telemetry.enableTelemetry.off', "Disables all product telemetry.") + localize('telemetry.telemetryLevel.default', "Enables all telemetry data to be collected."), + localize('telemetry.telemetryLevel.error', "Enables only error telemetry data and not general usage data."), + localize('telemetry.telemetryLevel.off', "Disables all product telemetry.") ], 'markdownDescription': !product.privacyStatementUrl ? - localize('telemetry.enableTelemetry', "Enable diagnostic data to be collected. This helps us to better understand how {0} is performing and where improvements need to be made.", product.nameLong) + restartString : - localize('telemetry.enableTelemetryMd', "Enable diagnostic data to be collected. This helps us to better understand how {0} is performing and where improvements need to be made. [Read more]({1}) about what we collect and our privacy statement.", product.nameLong, product.privacyStatementUrl) + restartString, + localize('telemetry.telemetryLevel', "Enable diagnostic data to be collected. This helps us to better understand how {0} is performing and where improvements need to be made. If this setting is set to 'off' no telemetry will be sent regardless of other settings.", product.nameLong) + restartString : + localize('telemetry.telemetryLevelMd', "Enable diagnostic data to be collected. This helps us to better understand how {0} is performing and where improvements need to be made. If this setting is set to 'off' no telemetry will be sent regardless of other settings. [Read more]({1}) about what we collect and our privacy statement.", product.nameLong, product.privacyStatementUrl) + restartString, 'default': TelemetryConfiguration.ON, 'restricted': true, 'scope': ConfigurationScope.APPLICATION, @@ -292,7 +290,7 @@ Registry.as(Extensions.Configuration).registerConfigurat localize('telemetry.enableTelemetryMd', "Enable diagnostic data to be collected. This helps us to better understand how {0} is performing and where improvements need to be made. [Read more]({1}) about what we collect and our privacy statement.", product.nameLong, product.privacyStatementUrl), 'default': true, 'restricted': true, - 'markdownDeprecationMessage': localize('enableTelemetryDeprecated', "Deprecated in favor of the {0} setting.", `\`#${TELEMETRY_SETTING_ID}#\``), + 'markdownDeprecationMessage': localize('enableTelemetryDeprecated', "Deprecated in favor of the {0} setting. If this setting is false, no telemetry will be sent regardless of the new setting's value.", `\`#${TELEMETRY_SETTING_ID}#\``), 'scope': ConfigurationScope.APPLICATION, 'tags': ['usesOnlineServices', 'telemetry'] } diff --git a/src/vs/workbench/electron-sandbox/desktop.contribution.ts b/src/vs/workbench/electron-sandbox/desktop.contribution.ts index 29031833f45cc..b102282c6e8e6 100644 --- a/src/vs/workbench/electron-sandbox/desktop.contribution.ts +++ b/src/vs/workbench/electron-sandbox/desktop.contribution.ts @@ -235,7 +235,7 @@ import { TELEMETRY_SETTING_ID } from 'vs/platform/telemetry/common/telemetry'; 'description': localize('telemetry.enableCrashReporting', "Enable crash reports to be collected. This helps us improve stability. \nThis option requires restart to take effect."), 'default': true, 'tags': ['usesOnlineServices', 'telemetry'], - 'markdownDeprecationMessage': localize('enableCrashReporterDeprecated', "Deprecated due to being combined into the {0} setting.", `\`#${TELEMETRY_SETTING_ID}#\``), + 'markdownDeprecationMessage': localize('enableCrashReporterDeprecated', "Deprecated due to being combined into the {0} setting. If this setting is false, no telemetry will be sent regardless of the new setting's value.", `\`#${TELEMETRY_SETTING_ID}#\``), } } }); From e37c10f1187a3feed896cade6372fdfe6e292dfe Mon Sep 17 00:00:00 2001 From: Logan Ramos Date: Wed, 13 Oct 2021 15:03:12 -0400 Subject: [PATCH 3/5] Clearer Settings UI for new telemetry setting (#135028) * Work on improving telemetry setting description * two options * third option * Add old crash reporter to getTelemetryLevel * More verbose wording + comments Co-authored-by: SteVen Batten --- src/vs/code/electron-main/app.ts | 5 +- src/vs/platform/telemetry/common/telemetry.ts | 4 +- .../telemetry/common/telemetryService.ts | 60 +++++++++++++------ .../telemetry/common/telemetryUtils.ts | 10 +++- .../electron-sandbox/desktop.contribution.ts | 2 +- 5 files changed, 55 insertions(+), 26 deletions(-) diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index 1d71361b94274..d2341a7aee3bc 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -990,9 +990,8 @@ export class CodeApplication extends Disposable { const argvString = argvContent.value.toString(); const argvJSON = JSON.parse(stripComments(argvString)); if (argvJSON['enable-crash-reporter'] === undefined) { - const telemetryConfig = getTelemetryLevel(this.configurationService); - const enableCrashReporterSetting = telemetryConfig >= TelemetryLevel.ERROR; - const enableCrashReporter = typeof enableCrashReporterSetting === 'boolean' ? enableCrashReporterSetting : true; + const telemetryLevel = getTelemetryLevel(this.configurationService); + const enableCrashReporter = telemetryLevel >= TelemetryLevel.CRASH; const additionalArgvContent = [ '', ' // Allows to disable crash reporting.', diff --git a/src/vs/platform/telemetry/common/telemetry.ts b/src/vs/platform/telemetry/common/telemetry.ts index 2746301674ca2..4f0dfa36049cc 100644 --- a/src/vs/platform/telemetry/common/telemetry.ts +++ b/src/vs/platform/telemetry/common/telemetry.ts @@ -79,12 +79,14 @@ export const TELEMETRY_OLD_SETTING_ID = 'telemetry.enableTelemetry'; export const enum TelemetryLevel { NONE = 0, + CRASH = 1, ERROR = 2, USAGE = 3 } export const enum TelemetryConfiguration { OFF = 'off', + CRASH = 'crash', ERROR = 'error', - ON = 'on' + ON = 'all' } diff --git a/src/vs/platform/telemetry/common/telemetryService.ts b/src/vs/platform/telemetry/common/telemetryService.ts index 64e2a08eefad5..445622c350ea0 100644 --- a/src/vs/platform/telemetry/common/telemetryService.ts +++ b/src/vs/platform/telemetry/common/telemetryService.ts @@ -82,16 +82,7 @@ export class TelemetryService implements ITelemetryService { this._experimentProperties[name] = value; } - private _convertOldTelemetrySettingToNew(): void { - const oldTelemetryValue = this._configurationService.getValue(TELEMETRY_OLD_SETTING_ID); - const crashReporterSetting = this._configurationService.getValue('telemetry.enableCrashReporter'); - if (oldTelemetryValue === false || crashReporterSetting === false) { - this._configurationService.updateValue(TELEMETRY_SETTING_ID, TelemetryConfiguration.OFF); - } - } - private _updateTelemetryLevel(): void { - this._convertOldTelemetrySettingToNew(); this._telemetryLevel = getTelemetryLevel(this._configurationService); } @@ -248,7 +239,42 @@ export class TelemetryService implements ITelemetryService { } } -const restartString = !isWeb ? ' ' + localize('telemetry.restart', 'Some features may require a restart to take effect.') : ''; +function getTelemetryLevelSettingDescription(): string { + const telemetryText = localize('telemetry.telemetryLevelMd', "Controls all core and first party extension telemetry. This helps us to better understand how {0} is performing, where improvements need to be made, and how features are being used.", product.nameLong); + const privacyStatement = product.privacyStatementUrl ? ' ' + localize("telemetry.privacyStatement", "[Read more]({1}) about what we collect and our privacy statement.", product.privacyStatementUrl) : ''; + const restartString = !isWeb ? ' ' + localize('telemetry.restart', 'A full restart of the application is necessary for crash reporting changes to take effect.') : ''; + + const crashReportsHeader = localize('telemetry.crashReports', "Crash Reports"); + const errorsHeader = localize('telemetry.errors', "Error Telemetry"); + const usageHeader = localize('telemetry.usage', "Usage Data"); + + const telemetryTableDescription = localize('telemetry.telemetryLevel.tableDescription', "The following table outlines the data sent with each setting:"); + const telemetryTable = ` +| | ${crashReportsHeader} | ${errorsHeader} | ${usageHeader} | +|:------|:---------------------:|:---------------:|:--------------:| +| all | ✓ | ✓ | ✓ | +| error | ✓ | ✓ | - | +| crash | ✓ | - | - | +| off | - | - | - | +`; + + const deprecatedSettingNote = localize('telemetry.telemetryLevel.deprecated', "****Note:*** If this setting is 'off', no telemetry will be sent regardless of other telemetry settings. If this setting is set to anything except 'off' and telemetry is disabled with deprecated settings, no telemetry will be sent.*"); + const telemetryDescription = ` +${telemetryText}${privacyStatement}${restartString} + +  + +${telemetryTableDescription} +${telemetryTable} + +  + +${deprecatedSettingNote} +`; + + return telemetryDescription; +} + Registry.as(Extensions.Configuration).registerConfiguration({ 'id': TELEMETRY_SECTION_ID, 'order': 110, @@ -257,16 +283,14 @@ Registry.as(Extensions.Configuration).registerConfigurat 'properties': { [TELEMETRY_SETTING_ID]: { 'type': 'string', - 'enum': [TelemetryConfiguration.ON, TelemetryConfiguration.ERROR, TelemetryConfiguration.OFF], + 'enum': [TelemetryConfiguration.ON, TelemetryConfiguration.ERROR, TelemetryConfiguration.CRASH, TelemetryConfiguration.OFF], 'enumDescriptions': [ - localize('telemetry.telemetryLevel.default', "Enables all telemetry data to be collected."), - localize('telemetry.telemetryLevel.error', "Enables only error telemetry data and not general usage data."), + localize('telemetry.telemetryLevel.default', "Sends usage data, errors, and crash reports."), + localize('telemetry.telemetryLevel.error', "Sends general error telemetry and crash reports."), + localize('telemetry.telemetryLevel.crash', "Sends OS level crash reports."), localize('telemetry.telemetryLevel.off', "Disables all product telemetry.") ], - 'markdownDescription': - !product.privacyStatementUrl ? - localize('telemetry.telemetryLevel', "Enable diagnostic data to be collected. This helps us to better understand how {0} is performing and where improvements need to be made. If this setting is set to 'off' no telemetry will be sent regardless of other settings.", product.nameLong) + restartString : - localize('telemetry.telemetryLevelMd', "Enable diagnostic data to be collected. This helps us to better understand how {0} is performing and where improvements need to be made. If this setting is set to 'off' no telemetry will be sent regardless of other settings. [Read more]({1}) about what we collect and our privacy statement.", product.nameLong, product.privacyStatementUrl) + restartString, + 'markdownDescription': getTelemetryLevelSettingDescription(), 'default': TelemetryConfiguration.ON, 'restricted': true, 'scope': ConfigurationScope.APPLICATION, @@ -290,7 +314,7 @@ Registry.as(Extensions.Configuration).registerConfigurat localize('telemetry.enableTelemetryMd', "Enable diagnostic data to be collected. This helps us to better understand how {0} is performing and where improvements need to be made. [Read more]({1}) about what we collect and our privacy statement.", product.nameLong, product.privacyStatementUrl), 'default': true, 'restricted': true, - 'markdownDeprecationMessage': localize('enableTelemetryDeprecated', "Deprecated in favor of the {0} setting. If this setting is false, no telemetry will be sent regardless of the new setting's value.", `\`#${TELEMETRY_SETTING_ID}#\``), + 'markdownDeprecationMessage': localize('enableTelemetryDeprecated', "If this setting is false, no telemetry will be sent regardless of the new setting's value. Deprecated in favor of the {0} setting.", `\`#${TELEMETRY_SETTING_ID}#\``), 'scope': ConfigurationScope.APPLICATION, 'tags': ['usesOnlineServices', 'telemetry'] } diff --git a/src/vs/platform/telemetry/common/telemetryUtils.ts b/src/vs/platform/telemetry/common/telemetryUtils.ts index a558f880a1dc6..6ab2158d01ef0 100644 --- a/src/vs/platform/telemetry/common/telemetryUtils.ts +++ b/src/vs/platform/telemetry/common/telemetryUtils.ts @@ -119,18 +119,22 @@ export function supportsTelemetry(productService: IProductService, environmentSe */ export function getTelemetryLevel(configurationService: IConfigurationService): TelemetryLevel { const newConfig = configurationService.getValue(TELEMETRY_SETTING_ID); - const oldConfig = configurationService.getValue(TELEMETRY_OLD_SETTING_ID); + const crashReporterConfig = configurationService.getValue('telemetry.enableCrashReporter'); + const oldConfig = configurationService.getValue(TELEMETRY_OLD_SETTING_ID); - // Check old config for disablement - if (oldConfig !== undefined && oldConfig === false) { + // If `telemetry.enableCrashReporter` is false or `telemetry.enableTelemetry' is false, disable telemetry + if (oldConfig === false || crashReporterConfig === false) { return TelemetryLevel.NONE; } + // Maps new telemetry setting to a telemetry level switch (newConfig ?? TelemetryConfiguration.ON) { case TelemetryConfiguration.ON: return TelemetryLevel.USAGE; case TelemetryConfiguration.ERROR: return TelemetryLevel.ERROR; + case TelemetryConfiguration.CRASH: + return TelemetryLevel.CRASH; case TelemetryConfiguration.OFF: return TelemetryLevel.NONE; } diff --git a/src/vs/workbench/electron-sandbox/desktop.contribution.ts b/src/vs/workbench/electron-sandbox/desktop.contribution.ts index b102282c6e8e6..d0e1b87402cfb 100644 --- a/src/vs/workbench/electron-sandbox/desktop.contribution.ts +++ b/src/vs/workbench/electron-sandbox/desktop.contribution.ts @@ -235,7 +235,7 @@ import { TELEMETRY_SETTING_ID } from 'vs/platform/telemetry/common/telemetry'; 'description': localize('telemetry.enableCrashReporting', "Enable crash reports to be collected. This helps us improve stability. \nThis option requires restart to take effect."), 'default': true, 'tags': ['usesOnlineServices', 'telemetry'], - 'markdownDeprecationMessage': localize('enableCrashReporterDeprecated', "Deprecated due to being combined into the {0} setting. If this setting is false, no telemetry will be sent regardless of the new setting's value.", `\`#${TELEMETRY_SETTING_ID}#\``), + 'markdownDeprecationMessage': localize('enableCrashReporterDeprecated', "If this setting is false, no telemetry will be sent regardless of the new setting's value. Deprecated due to being combined into the {0} setting.", `\`#${TELEMETRY_SETTING_ID}#\``), } } }); From d5654c6006273634fb6689ad5f59edb61a87006e Mon Sep 17 00:00:00 2001 From: Logan Ramos Date: Wed, 13 Oct 2021 19:22:12 -0400 Subject: [PATCH 4/5] Update telemetry text --- src/vs/platform/telemetry/common/telemetryService.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/telemetry/common/telemetryService.ts b/src/vs/platform/telemetry/common/telemetryService.ts index 445622c350ea0..12e746ccb0fb8 100644 --- a/src/vs/platform/telemetry/common/telemetryService.ts +++ b/src/vs/platform/telemetry/common/telemetryService.ts @@ -241,7 +241,8 @@ export class TelemetryService implements ITelemetryService { function getTelemetryLevelSettingDescription(): string { const telemetryText = localize('telemetry.telemetryLevelMd', "Controls all core and first party extension telemetry. This helps us to better understand how {0} is performing, where improvements need to be made, and how features are being used.", product.nameLong); - const privacyStatement = product.privacyStatementUrl ? ' ' + localize("telemetry.privacyStatement", "[Read more]({1}) about what we collect and our privacy statement.", product.privacyStatementUrl) : ''; + const docsStatement = ' ' + localize("telemetry.docsStatement", "Read more about the [data we collect]({0})", 'https://aka.ms/vscode-telemetry'); + const privacyStatement = product.privacyStatementUrl || true ? docsStatement + ' ' + localize("telemetry.privacyStatement", "and our [privacy statement]({0}).", product.privacyStatementUrl) : docsStatement + '.'; const restartString = !isWeb ? ' ' + localize('telemetry.restart', 'A full restart of the application is necessary for crash reporting changes to take effect.') : ''; const crashReportsHeader = localize('telemetry.crashReports', "Crash Reports"); From 3c133b78d60375d05b3c6858a8276e9cc5d246c5 Mon Sep 17 00:00:00 2001 From: SteVen Batten Date: Wed, 13 Oct 2021 17:26:56 -0700 Subject: [PATCH 5/5] fix strings --- src/vs/platform/telemetry/common/telemetryService.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/vs/platform/telemetry/common/telemetryService.ts b/src/vs/platform/telemetry/common/telemetryService.ts index 12e746ccb0fb8..a3b17f5a82120 100644 --- a/src/vs/platform/telemetry/common/telemetryService.ts +++ b/src/vs/platform/telemetry/common/telemetryService.ts @@ -241,9 +241,10 @@ export class TelemetryService implements ITelemetryService { function getTelemetryLevelSettingDescription(): string { const telemetryText = localize('telemetry.telemetryLevelMd', "Controls all core and first party extension telemetry. This helps us to better understand how {0} is performing, where improvements need to be made, and how features are being used.", product.nameLong); - const docsStatement = ' ' + localize("telemetry.docsStatement", "Read more about the [data we collect]({0})", 'https://aka.ms/vscode-telemetry'); - const privacyStatement = product.privacyStatementUrl || true ? docsStatement + ' ' + localize("telemetry.privacyStatement", "and our [privacy statement]({0}).", product.privacyStatementUrl) : docsStatement + '.'; - const restartString = !isWeb ? ' ' + localize('telemetry.restart', 'A full restart of the application is necessary for crash reporting changes to take effect.') : ''; + const externalLinksStatement = !product.privacyStatementUrl ? + localize("telemetry.docsStatement", "Read more about the [data we collect]({0}).", 'https://aka.ms/vscode-telemetry') : + localize("telemetry.docsAndPrivacyStatement", "Read more about the [data we collect]({0}) and our [privacy statement]({1}).", 'https://aka.ms/vscode-telemetry', product.privacyStatementUrl); + const restartString = !isWeb ? localize('telemetry.restart', 'A full restart of the application is necessary for crash reporting changes to take effect.') : ''; const crashReportsHeader = localize('telemetry.crashReports', "Crash Reports"); const errorsHeader = localize('telemetry.errors', "Error Telemetry"); @@ -261,7 +262,7 @@ function getTelemetryLevelSettingDescription(): string { const deprecatedSettingNote = localize('telemetry.telemetryLevel.deprecated', "****Note:*** If this setting is 'off', no telemetry will be sent regardless of other telemetry settings. If this setting is set to anything except 'off' and telemetry is disabled with deprecated settings, no telemetry will be sent.*"); const telemetryDescription = ` -${telemetryText}${privacyStatement}${restartString} +${telemetryText} ${externalLinksStatement} ${restartString}