From 3dcc354df7d5172459474bdc09c92c9cae23ea46 Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Mon, 3 Jul 2017 11:45:05 -0700 Subject: [PATCH] Separate exeriments from telemetry (fixes #29339) --- .../standalone/browser/simpleServices.ts | 6 +- .../platform/telemetry/common/experiments.ts | 74 +++++++++++++++++++ src/vs/platform/telemetry/common/telemetry.ts | 6 -- .../telemetry/common/telemetryService.ts | 11 +-- .../telemetry/common/telemetryUtils.ts | 67 +---------------- src/vs/workbench/api/node/extHostTelemetry.ts | 6 +- src/vs/workbench/electron-browser/shell.ts | 9 +-- .../electron-browser/vs_code_welcome_page.ts | 7 -- .../page/electron-browser/welcomePage.ts | 28 ------- .../quickopen.perf.integrationTest.ts | 7 +- .../textsearch.perf.integrationTest.ts | 7 +- 11 files changed, 85 insertions(+), 143 deletions(-) create mode 100644 src/vs/platform/telemetry/common/experiments.ts diff --git a/src/vs/editor/standalone/browser/simpleServices.ts b/src/vs/editor/standalone/browser/simpleServices.ts index b44d504ae8f..b39ed672ee0 100644 --- a/src/vs/editor/standalone/browser/simpleServices.ts +++ b/src/vs/editor/standalone/browser/simpleServices.ts @@ -33,7 +33,7 @@ import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { KeybindingsRegistry, IKeybindingItem } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { MenuId, IMenu, IMenuService } from 'vs/platform/actions/common/actions'; import { Menu } from 'vs/platform/actions/common/menu'; -import { ITelemetryService, ITelemetryExperiments, ITelemetryInfo } from 'vs/platform/telemetry/common/telemetry'; +import { ITelemetryService, ITelemetryInfo } from 'vs/platform/telemetry/common/telemetry'; import { ResolvedKeybinding, Keybinding, createKeybinding, SimpleKeybinding } from 'vs/base/common/keyCodes'; import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem'; import { OS } from 'vs/base/common/platform'; @@ -513,10 +513,6 @@ export class StandaloneTelemetryService implements ITelemetryService { public getTelemetryInfo(): TPromise { return null; } - - public getExperiments(): ITelemetryExperiments { - return null; - } } export class SimpleWorkspaceContextService implements IWorkspaceContextService { diff --git a/src/vs/platform/telemetry/common/experiments.ts b/src/vs/platform/telemetry/common/experiments.ts new file mode 100644 index 00000000000..b338b4518db --- /dev/null +++ b/src/vs/platform/telemetry/common/experiments.ts @@ -0,0 +1,74 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +'use strict'; + +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { IStorageService } from 'vs/platform/storage/common/storage'; +import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; + +export interface ITelemetryExperiments { +} + +const defaultExperiments: ITelemetryExperiments = { +}; + +export function loadExperiments(accessor?: ServicesAccessor): ITelemetryExperiments { + + // shortcut since there are currently no experiments (should introduce separate service to load only once) + if (!accessor) { + return {}; + } + + const storageService = accessor.get(IStorageService); + const configurationService = accessor.get(IConfigurationService); + + let { + } = splitExperimentsRandomness(storageService); + + return applyOverrides(defaultExperiments, configurationService); +} + +function applyOverrides(experiments: ITelemetryExperiments, configurationService: IConfigurationService): ITelemetryExperiments { + const experimentsConfig = getExperimentsOverrides(configurationService); + Object.keys(experiments).forEach(key => { + if (key in experimentsConfig) { + experiments[key] = experimentsConfig[key]; + } + }); + return experiments; +} + +function splitExperimentsRandomness(storageService: IStorageService): ITelemetryExperiments { + const random1 = getExperimentsRandomness(storageService); + const [random2, /* showTaskDocumentation */] = splitRandom(random1); + const [random3, /* openUntitledFile */] = splitRandom(random2); + const [random4, /* mergeQuickLinks */] = splitRandom(random3); + // tslint:disable-next-line:no-unused-variable (https://github.com/Microsoft/TypeScript/issues/16628) + const [random5, /* enableWelcomePage */] = splitRandom(random4); + return { + }; +} + +function getExperimentsRandomness(storageService: IStorageService) { + const key = 'experiments.randomness'; + let valueString = storageService.get(key); + if (!valueString) { + valueString = Math.random().toString(); + storageService.store(key, valueString); + } + + return parseFloat(valueString); +} + +function splitRandom(random: number): [number, boolean] { + const scaled = random * 2; + const i = Math.floor(scaled); + return [scaled - i, i === 1]; +} + +function getExperimentsOverrides(configurationService: IConfigurationService): ITelemetryExperiments { + const config: any = configurationService.getConfiguration('telemetry'); + return config && config.experiments || {}; +} diff --git a/src/vs/platform/telemetry/common/telemetry.ts b/src/vs/platform/telemetry/common/telemetry.ts index 2d57d289534..ebfc8144e6e 100644 --- a/src/vs/platform/telemetry/common/telemetry.ts +++ b/src/vs/platform/telemetry/common/telemetry.ts @@ -21,10 +21,6 @@ export interface ITelemetryData { [key: string]: any; } -export interface ITelemetryExperiments { - mergeQuickLinks: boolean; -} - export interface ITelemetryService { _serviceBrand: any; @@ -38,6 +34,4 @@ export interface ITelemetryService { getTelemetryInfo(): TPromise; isOptedIn: boolean; - - getExperiments(): ITelemetryExperiments; } diff --git a/src/vs/platform/telemetry/common/telemetryService.ts b/src/vs/platform/telemetry/common/telemetryService.ts index edd480496c6..b452e7f8342 100644 --- a/src/vs/platform/telemetry/common/telemetryService.ts +++ b/src/vs/platform/telemetry/common/telemetryService.ts @@ -7,8 +7,8 @@ import { localize } from 'vs/nls'; import { escapeRegExpCharacters } from 'vs/base/common/strings'; -import { ITelemetryService, ITelemetryInfo, ITelemetryExperiments, ITelemetryData } from 'vs/platform/telemetry/common/telemetry'; -import { ITelemetryAppender, defaultExperiments } from 'vs/platform/telemetry/common/telemetryUtils'; +import { ITelemetryService, ITelemetryInfo, ITelemetryData } from 'vs/platform/telemetry/common/telemetry'; +import { ITelemetryAppender } from 'vs/platform/telemetry/common/telemetryUtils'; import { optional } from 'vs/platform/instantiation/common/instantiation'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IConfigurationRegistry, Extensions } from 'vs/platform/configuration/common/configurationRegistry'; @@ -22,7 +22,6 @@ export interface ITelemetryServiceConfig { commonProperties?: TPromise<{ [name: string]: any }>; piiPaths?: string[]; userOptIn?: boolean; - experiments?: ITelemetryExperiments; } export class TelemetryService implements ITelemetryService { @@ -36,7 +35,6 @@ export class TelemetryService implements ITelemetryService { private _commonProperties: TPromise<{ [name: string]: any; }>; private _piiPaths: string[]; private _userOptIn: boolean; - private _experiments: ITelemetryExperiments; private _disposables: IDisposable[] = []; private _cleanupPatterns: [RegExp, string][] = []; @@ -49,7 +47,6 @@ export class TelemetryService implements ITelemetryService { this._commonProperties = config.commonProperties || TPromise.as({}); this._piiPaths = config.piiPaths || []; this._userOptIn = typeof config.userOptIn === 'undefined' ? true : config.userOptIn; - this._experiments = config.experiments || defaultExperiments; // static cleanup patterns for: // #1 `file:///DANGEROUS/PATH/resources/app/Useful/Information` @@ -81,10 +78,6 @@ export class TelemetryService implements ITelemetryService { return this._userOptIn; } - getExperiments(): ITelemetryExperiments { - return this._experiments; - } - getTelemetryInfo(): TPromise { return this._commonProperties.then(values => { // well known properties diff --git a/src/vs/platform/telemetry/common/telemetryUtils.ts b/src/vs/platform/telemetry/common/telemetryUtils.ts index 6ac90a73e41..8813aaedf4e 100644 --- a/src/vs/platform/telemetry/common/telemetryUtils.ts +++ b/src/vs/platform/telemetry/common/telemetryUtils.ts @@ -12,17 +12,10 @@ import URI from 'vs/base/common/uri'; import { ConfigurationSource, IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IKeybindingService, KeybindingSource } from 'vs/platform/keybinding/common/keybinding'; import { ILifecycleService, ShutdownReason } from 'vs/platform/lifecycle/common/lifecycle'; -import { IStorageService } from 'vs/platform/storage/common/storage'; -import { ITelemetryService, ITelemetryExperiments, ITelemetryInfo, ITelemetryData } from 'vs/platform/telemetry/common/telemetry'; -import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; - -export const defaultExperiments: ITelemetryExperiments = { - mergeQuickLinks: false, -}; +import { ITelemetryService, ITelemetryInfo, ITelemetryData } from 'vs/platform/telemetry/common/telemetry'; export const NullTelemetryService = { _serviceBrand: undefined, - _experiments: defaultExperiments, publicLog(eventName: string, data?: ITelemetryData) { return TPromise.as(null); }, @@ -33,67 +26,9 @@ export const NullTelemetryService = { sessionId: 'someValue.sessionId', machineId: 'someValue.machineId' }); - }, - getExperiments(): ITelemetryExperiments { - return this._experiments; } }; -export function loadExperiments(accessor: ServicesAccessor): ITelemetryExperiments { - // const storageService = accessor.get(IStorageService); - const configurationService = accessor.get(IConfigurationService); - - // let { - // } = splitExperimentsRandomness(storageService); - - return applyOverrides(defaultExperiments, configurationService); -} - -function applyOverrides(experiments: ITelemetryExperiments, configurationService: IConfigurationService): ITelemetryExperiments { - const experimentsConfig = getExperimentsOverrides(configurationService); - Object.keys(experiments).forEach(key => { - if (key in experimentsConfig) { - experiments[key] = experimentsConfig[key]; - } - }); - return experiments; -} - -// tslint:disable-next-line:no-unused-variable -function splitExperimentsRandomness(storageService: IStorageService): ITelemetryExperiments { - const random1 = getExperimentsRandomness(storageService); - const [random2, /* showTaskDocumentation */] = splitRandom(random1); - const [random3, /* openUntitledFile */] = splitRandom(random2); - const [random4, mergeQuickLinks] = splitRandom(random3); - // tslint:disable-next-line:no-unused-variable (https://github.com/Microsoft/TypeScript/issues/16628) - const [random5, /* enableWelcomePage */] = splitRandom(random4); - return { - mergeQuickLinks, - }; -} - -function getExperimentsRandomness(storageService: IStorageService) { - const key = 'experiments.randomness'; - let valueString = storageService.get(key); - if (!valueString) { - valueString = Math.random().toString(); - storageService.store(key, valueString); - } - - return parseFloat(valueString); -} - -function splitRandom(random: number): [number, boolean] { - const scaled = random * 2; - const i = Math.floor(scaled); - return [scaled - i, i === 1]; -} - -function getExperimentsOverrides(configurationService: IConfigurationService): ITelemetryExperiments { - const config: any = configurationService.getConfiguration('telemetry'); - return config && config.experiments || {}; -} - export interface ITelemetryAppender { log(eventName: string, data: any): void; } diff --git a/src/vs/workbench/api/node/extHostTelemetry.ts b/src/vs/workbench/api/node/extHostTelemetry.ts index 3c787653ce3..35849674f38 100644 --- a/src/vs/workbench/api/node/extHostTelemetry.ts +++ b/src/vs/workbench/api/node/extHostTelemetry.ts @@ -6,7 +6,7 @@ import { notImplemented } from 'vs/base/common/errors'; import { TPromise } from 'vs/base/common/winjs.base'; -import { ITelemetryService, ITelemetryInfo, ITelemetryExperiments } from 'vs/platform/telemetry/common/telemetry'; +import { ITelemetryService, ITelemetryInfo } from 'vs/platform/telemetry/common/telemetry'; import { IThreadService } from 'vs/workbench/services/thread/common/threadService'; import { MainContext, MainThreadTelemetryShape } from './extHost.protocol'; @@ -26,10 +26,6 @@ export class RemoteTelemetryService implements ITelemetryService { throw notImplemented(); } - getExperiments(): ITelemetryExperiments { - throw notImplemented(); - } - getTelemetryInfo(): TPromise { return this._proxy.$getTelemetryInfo(); } diff --git a/src/vs/workbench/electron-browser/shell.ts b/src/vs/workbench/electron-browser/shell.ts index 276ea16e5a4..e7d4031090e 100644 --- a/src/vs/workbench/electron-browser/shell.ts +++ b/src/vs/workbench/electron-browser/shell.ts @@ -22,7 +22,8 @@ import pkg from 'vs/platform/node/package'; import { ContextViewService } from 'vs/platform/contextview/browser/contextViewService'; import { Workbench, IWorkbenchStartedInfo } from 'vs/workbench/electron-browser/workbench'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; -import { NullTelemetryService, configurationTelemetry, loadExperiments, lifecycleTelemetry } from 'vs/platform/telemetry/common/telemetryUtils'; +import { NullTelemetryService, configurationTelemetry, lifecycleTelemetry } from 'vs/platform/telemetry/common/telemetryUtils'; +import { loadExperiments } from 'vs/platform/telemetry/common/experiments'; import { ITelemetryAppenderChannel, TelemetryAppenderClient } from 'vs/platform/telemetry/common/telemetryIpc'; import { TelemetryService, ITelemetryServiceConfig } from 'vs/platform/telemetry/common/telemetryService'; import { IdleMonitor, UserStatus } from 'vs/platform/telemetry/browser/idleMonitor'; @@ -218,7 +219,7 @@ export class WorkbenchShell { customKeybindingsCount: info.customKeybindingsCount, theme: this.themeService.getColorTheme().id, language: platform.language, - experiments: this.telemetryService.getExperiments(), + experiments: loadExperiments(), pinnedViewlets: info.pinnedViewlets, restoredViewlet: info.restoredViewlet, restoredEditors: info.restoredEditors.length, @@ -287,8 +288,7 @@ export class WorkbenchShell { const config: ITelemetryServiceConfig = { appender: new TelemetryAppenderClient(channel), commonProperties: resolveWorkbenchCommonProperties(this.storageService, commit, version), - piiPaths: [this.environmentService.appRoot, this.environmentService.extensionsPath], - experiments: instantiationService.invokeFunction(loadExperiments) + piiPaths: [this.environmentService.appRoot, this.environmentService.extensionsPath] }; const telemetryService = instantiationService.createInstance(TelemetryService, config); @@ -305,7 +305,6 @@ export class WorkbenchShell { disposables.push(telemetryService, errorTelemetry, listener, idleMonitor); } else { - NullTelemetryService._experiments = instantiationService.invokeFunction(loadExperiments); this.telemetryService = NullTelemetryService; } diff --git a/src/vs/workbench/parts/welcome/page/electron-browser/vs_code_welcome_page.ts b/src/vs/workbench/parts/welcome/page/electron-browser/vs_code_welcome_page.ts index 8897c2302ce..926d92f29b3 100644 --- a/src/vs/workbench/parts/welcome/page/electron-browser/vs_code_welcome_page.ts +++ b/src/vs/workbench/parts/welcome/page/electron-browser/vs_code_welcome_page.ts @@ -72,13 +72,6 @@ export default () => `
    • -
    -

    ${escape(localize('welcomePage.quickLinks', "Quick links"))}

    -
      -
      • -
      • -
    -
    diff --git a/src/vs/workbench/parts/welcome/page/electron-browser/welcomePage.ts b/src/vs/workbench/parts/welcome/page/electron-browser/welcomePage.ts index eb12b56e3ec..d8f457f5577 100644 --- a/src/vs/workbench/parts/welcome/page/electron-browser/welcomePage.ts +++ b/src/vs/workbench/parts/welcome/page/electron-browser/welcomePage.ts @@ -106,16 +106,6 @@ export class WelcomePageAction extends Action { } } -const reorderedQuickLinks = [ - 'showInterfaceOverview', - 'selectTheme', - 'showRecommendedKeymapExtensions', - 'showCommands', - 'keybindingsReference', - 'openGlobalSettings', - 'showInteractivePlayground', -]; - interface ExtensionSuggestion { name: string; id: string; @@ -271,24 +261,6 @@ class WelcomePage { }); }).then(null, onUnexpectedError); - const customize = container.querySelector('.commands .section.customize'); - const learn = container.querySelector('.commands .section.learn'); - const quickLinks = container.querySelector('.commands .section.quickLinks'); - if (this.telemetryService.getExperiments().mergeQuickLinks) { - const ul = quickLinks.querySelector('ul'); - reorderedQuickLinks.forEach(clazz => { - const link = container.querySelector(`.commands .${clazz}`); - if (link) { - ul.appendChild(link); - } - }); - customize.remove(); - learn.remove(); - container.querySelector('.keybindingsReferenceLink').remove(); - } else { - quickLinks.remove(); - } - this.addExtensionList(container, '.extensionPackList', extensionPacks, extensionPackStrings); this.addExtensionList(container, '.keymapList', keymapExtensions, keymapStrings); diff --git a/src/vs/workbench/test/electron-browser/quickopen.perf.integrationTest.ts b/src/vs/workbench/test/electron-browser/quickopen.perf.integrationTest.ts index cbaf130bec5..e9348ede436 100644 --- a/src/vs/workbench/test/electron-browser/quickopen.perf.integrationTest.ts +++ b/src/vs/workbench/test/electron-browser/quickopen.perf.integrationTest.ts @@ -11,8 +11,7 @@ import { IWorkspaceContextService, LegacyWorkspace } from 'vs/platform/workspace import { createSyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; import { IEditorGroupService } from 'vs/workbench/services/group/common/groupService'; import { ISearchService } from 'vs/platform/search/common/search'; -import { ITelemetryService, ITelemetryInfo, ITelemetryExperiments } from 'vs/platform/telemetry/common/telemetry'; -import { defaultExperiments } from 'vs/platform/telemetry/common/telemetryUtils'; +import { ITelemetryService, ITelemetryInfo } from 'vs/platform/telemetry/common/telemetry'; import { IUntitledEditorService, UntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService'; import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService'; import * as minimist from 'minimist'; @@ -177,8 +176,4 @@ class TestTelemetryService implements ITelemetryService { machineId: 'someValue.machineId' }); } - - public getExperiments(): ITelemetryExperiments { - return defaultExperiments; - } }; diff --git a/src/vs/workbench/test/electron-browser/textsearch.perf.integrationTest.ts b/src/vs/workbench/test/electron-browser/textsearch.perf.integrationTest.ts index ffe598e89ae..8b789b0d03f 100644 --- a/src/vs/workbench/test/electron-browser/textsearch.perf.integrationTest.ts +++ b/src/vs/workbench/test/electron-browser/textsearch.perf.integrationTest.ts @@ -12,8 +12,7 @@ import { IWorkspaceContextService, LegacyWorkspace } from 'vs/platform/workspace import { createSyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; import { IEditorGroupService } from 'vs/workbench/services/group/common/groupService'; import { ISearchService, IQueryOptions } from 'vs/platform/search/common/search'; -import { ITelemetryService, ITelemetryInfo, ITelemetryExperiments } from 'vs/platform/telemetry/common/telemetry'; -import { defaultExperiments } from 'vs/platform/telemetry/common/telemetryUtils'; +import { ITelemetryService, ITelemetryInfo } from 'vs/platform/telemetry/common/telemetry'; import { IUntitledEditorService, UntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService'; import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService'; import * as minimist from 'minimist'; @@ -166,8 +165,4 @@ class TestTelemetryService implements ITelemetryService { machineId: 'someValue.machineId' }); } - - public getExperiments(): ITelemetryExperiments { - return defaultExperiments; - } };