Skip to content

Commit

Permalink
Better telemetry setting UI + clearer wording surrounding new setting (…
Browse files Browse the repository at this point in the history 
…#135030)

* Address #134660

* Further address #134660

* 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 <[email protected]>

* Update telemetry text

* fix strings

Co-authored-by: SteVen Batten <[email protected]>
  • Loading branch information
@lramos15 @sbatten
lramos15 and sbatten authored Oct 14, 2021
1 parent fd6f80b commit c13f1ab
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 35 deletions.
5 changes: 2 additions & 3 deletions src/vs/code/electron-main/app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand Down
4 changes: 3 additions & 1 deletion src/vs/platform/telemetry/common/telemetry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
72 changes: 45 additions & 27 deletions src/vs/platform/telemetry/common/telemetryService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,29 +76,12 @@ 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 _updateTelemetryLevel(): void {
this._telemetryLevel = getTelemetryLevel(this._configurationService);
}
Expand Down Expand Up @@ -256,7 +239,44 @@ 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 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");
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} ${externalLinksStatement} ${restartString}
&nbsp;
${telemetryTableDescription}
${telemetryTable}
&nbsp;
${deprecatedSettingNote}
`;

return telemetryDescription;
}

Registry.as<IConfigurationRegistry>(Extensions.Configuration).registerConfiguration({
'id': TELEMETRY_SECTION_ID,
'order': 110,
Expand All @@ -265,16 +285,14 @@ Registry.as<IConfigurationRegistry>(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.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', "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.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,
'markdownDescription': getTelemetryLevelSettingDescription(),
'default': TelemetryConfiguration.ON,
'restricted': true,
'scope': ConfigurationScope.APPLICATION,
Expand All @@ -298,7 +316,7 @@ Registry.as<IConfigurationRegistry>(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', "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']
}
Expand Down
10 changes: 7 additions & 3 deletions src/vs/platform/telemetry/common/telemetryUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,22 @@ export function supportsTelemetry(productService: IProductService, environmentSe
*/
export function getTelemetryLevel(configurationService: IConfigurationService): TelemetryLevel {
const newConfig = configurationService.getValue<TelemetryConfiguration>(TELEMETRY_SETTING_ID);
const oldConfig = configurationService.getValue(TELEMETRY_OLD_SETTING_ID);
const crashReporterConfig = configurationService.getValue<boolean | undefined>('telemetry.enableCrashReporter');
const oldConfig = configurationService.getValue<boolean | undefined>(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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/electron-sandbox/desktop.contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', "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}#\``),
}
}
});
Expand Down

0 comments on commit c13f1ab

Please sign in to comment.