extension/src: remove sampling and machine ID from telemetry service

- CL 625495 increased the telemetry prompting to 100%, sampling
threshold and machine ID is no longer needed. (Previuosly, it's
determined by hash<machineID> % 1000 > threshold). Clean up test
that relevant to sampling threshold and machine ID.
- Remove TODO to prompt telemetry regardless of vscode env
isTelemetryEnabled.
See https://code.visualstudio.com/api/extension-guides/telemetry#without-the-telemetry-module
- VSCode-Go delegates most of telemetry logic to gopls even
including the telemetry opt in prompting. VSCode-Go send a
ExecuteCommandRequest to gopls and gopls returns a
ShowMessageRequest back to client for prompting.
It's the language server (gopls)'s responsibility to memorize
the user's selection after prompting and avoid re-sending the
same prompt next time.

For #3697

Change-Id: I89cd0324354ed00192dc8d696996d23ba0946861
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/660396
Reviewed-by: Madeline Kalil <[email protected]>
kokoro-CI: kokoro <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: h9jiang

extension/src/goTelemetry.ts

@@ -79,6 +79,13 @@ export class TelemetryReporter implements vscode.Disposable {
 	private _counters: { [key: string]: number } = {};
 	private _flushTimer: NodeJS.Timeout | undefined;
 	private _tool = '';
+
+	/**
+	 * @param flushIntervalMs is the interval (in milliseconds) between periodic
+	 * `flush()` calls.
+	 * @param counterFile is the file path for writing telemetry data (used for
+	 * testing).
+	 */
 	constructor(flushIntervalMs = 60_000, private counterFile: string = '') {
 		if (flushIntervalMs > 0) {
 			// Periodically call flush.
@@ -203,32 +210,27 @@ export class TelemetryService {
 	constructor(
 		private languageClient: Pick<LanguageClient, 'sendRequest'> | undefined,
 		private globalState: vscode.Memento,
-		commands: string[] = []
+		serverCommands: string[] = []
 	) {
-		if (!languageClient || !commands.includes(GOPLS_MAYBE_PROMPT_FOR_TELEMETRY)) {
-			// we are not backed by the gopls version that supports telemetry.
+		if (!languageClient || !serverCommands.includes(GOPLS_MAYBE_PROMPT_FOR_TELEMETRY)) {
+			// We are not backed by the gopls version that supports telemetry.
 			return;
 		}
 
 		this.active = true;
-		// record the first time we see the gopls with telemetry support.
+		// Record the first time we see the gopls with telemetry support.
 		// The timestamp will be used to avoid prompting too early.
 		const telemetryStartTime = readTelemetryStartTime(globalState);
 		if (!telemetryStartTime) {
 			recordTelemetryStartTime(globalState, new Date());
 		}
 	}
 
-	async promptForTelemetry(
-		isPreviewExtension: boolean,
-		isVSCodeTelemetryEnabled: boolean = vscode.env.isTelemetryEnabled,
-		samplingInterval = 1000 /* prompt N out of 1000. 1000 = 100% */
-	) {
+	async promptForTelemetry(isVSCodeTelemetryEnabled: boolean = vscode.env.isTelemetryEnabled) {
 		if (!this.active) return;
 
-		// Do not prompt yet if the user disabled vscode's telemetry.
-		// TODO(hyangah): remove this condition after we roll out to 100%. It's possible
-		// users who don't want vscode's telemetry are still willing to opt in.
+		// Do not prompt if the user disabled vscode's telemetry.
+		// See https://code.visualstudio.com/api/extension-guides/telemetry#without-the-telemetry-module
 		if (!isVSCodeTelemetryEnabled) return;
 
 		// Allow at least 7days for gopls to collect some data.
@@ -240,11 +242,6 @@ export class TelemetryService {
 			return;
 		}
 
-		// For official extension users, prompt only N out of 1000.
-		if (!isPreviewExtension && this.hashMachineID() % 1000 >= samplingInterval) {
-			return;
-		}
-
 		try {
 			await this.languageClient?.sendRequest(ExecuteCommandRequest.type, {
 				command: GOPLS_MAYBE_PROMPT_FOR_TELEMETRY
@@ -253,11 +250,6 @@ export class TelemetryService {
 			console.log(`failed to send telemetry request: ${e}`);
 		}
 	}
-
-	// exported for testing.
-	public hashMachineID(salt?: string): number {
-		return hashMachineID(salt);
-	}
 }
 
 /**

extension/src/language/goLanguageServer.ts

@@ -1702,7 +1702,7 @@ export function maybePromptForTelemetry(goCtx: GoExtensionContext) {
 			setTimeout(callback, 5 * timeMinute - Math.max(idleTime, 0));
 			return;
 		}
-		goCtx.telemetryService?.promptForTelemetry(extensionInfo.isPreview);
+		goCtx.telemetryService?.promptForTelemetry();
 	};
 	callback();
 }

extension/test/gopls/extension.test.ts

@@ -211,11 +211,7 @@ suite('Go Extension Tests With Gopls', function () {
 			await Promise.all([
 				// we want to see the prompt command flowing.
 				env.onMessageInTrace(GOPLS_MAYBE_PROMPT_FOR_TELEMETRY, 60_000),
-				sut.promptForTelemetry(
-					false /* not a preview */,
-					true /* vscode telemetry not disabled */,
-					1000 /* 1000 out of 1000 users */
-				)
+				sut.promptForTelemetry(true /* vscode telemetry not disabled */)
 			]);
 		} catch (e) {
 			assert(false, `unexpected failure: ${e}`);

extension/test/gopls/telemetry.test.ts

@@ -33,8 +33,7 @@ describe('# prompt for telemetry', async () => {
 		testTelemetryPrompt(
 			{
 				noLangClient: true,
-				previewExtension: true,
-				samplingInterval: 1000
+				previewExtension: true
 			},
 			false
 		)
@@ -44,8 +43,7 @@ describe('# prompt for telemetry', async () => {
 		testTelemetryPrompt(
 			{
 				goplsWithoutTelemetry: true,
-				previewExtension: true,
-				samplingInterval: 1000
+				previewExtension: true
 			},
 			false
 		)
@@ -54,8 +52,7 @@ describe('# prompt for telemetry', async () => {
 		'prompt when telemetry started a while ago',
 		testTelemetryPrompt(
 			{
-				firstDate: new Date('2022-01-01'),
-				samplingInterval: 1000
+				firstDate: new Date('2022-01-01')
 			},
 			true
 		)
@@ -64,8 +61,7 @@ describe('# prompt for telemetry', async () => {
 		'do not prompt if telemetry started two days ago',
 		testTelemetryPrompt(
 			{
-				firstDate: new Date(Date.now() - 2 * 24 * 60 * 60 * 1000), // two days ago!
-				samplingInterval: 1000
+				firstDate: new Date(Date.now() - 2 * 24 * 60 * 60 * 1000) // two days ago!
 			},
 			false
 		)
@@ -74,40 +70,7 @@ describe('# prompt for telemetry', async () => {
 		'do not prompt if gopls with telemetry never ran',
 		testTelemetryPrompt(
 			{
-				firstDate: undefined, // gopls with telemetry not seen before.
-				samplingInterval: 1000
-			},
-			false
-		)
-	);
-	it(
-		'do not prompt if not sampled',
-		testTelemetryPrompt(
-			{
-				firstDate: new Date('2022-01-01'),
-				samplingInterval: 0
-			},
-			false
-		)
-	);
-	it(
-		'prompt only if sampled (machineID = 0, samplingInterval = 1)',
-		testTelemetryPrompt(
-			{
-				firstDate: new Date('2022-01-01'),
-				samplingInterval: 1,
-				hashMachineID: 0
-			},
-			true
-		)
-	);
-	it(
-		'prompt only if sampled (machineID = 1, samplingInterval = 1)',
-		testTelemetryPrompt(
-			{
-				firstDate: new Date('2022-01-01'),
-				samplingInterval: 1,
-				hashMachineID: 1
+				firstDate: undefined // gopls with telemetry not seen before.
 			},
 			false
 		)
@@ -117,8 +80,7 @@ describe('# prompt for telemetry', async () => {
 		testTelemetryPrompt(
 			{
 				firstDate: new Date('2022-01-01'),
-				previewExtension: true,
-				samplingInterval: 0
+				previewExtension: true
 			},
 			true
 		)
@@ -129,8 +91,7 @@ describe('# prompt for telemetry', async () => {
 			{
 				firstDate: new Date('2022-01-01'),
 				vsTelemetryDisabled: true,
-				previewExtension: true,
-				samplingInterval: 1000
+				previewExtension: true
 			},
 			false
 		)
@@ -145,7 +106,6 @@ describe('# prompt for telemetry', async () => {
 		testExtensionAPI.globalState.update(TELEMETRY_START_TIME_KEY, new Date(Date.now() - 7 * 24 * 60 * 60 * 1000));
 		await testTelemetryPrompt(
 			{
-				samplingInterval: 1000,
 				mementoInstance: testExtensionAPI.globalState
 			},
 			true
@@ -159,8 +119,6 @@ interface testCase {
 	firstDate?: Date; // first date the extension observed gopls with telemetry feature.
 	previewExtension?: boolean; // assume we are in dev/nightly extension.
 	vsTelemetryDisabled?: boolean; // assume the user disabled vscode general telemetry.
-	samplingInterval: number; // N where N out of 1000 are sampled.
-	hashMachineID?: number; // stub the machine id hash computation function.
 	mementoInstance?: Memento; // if set, use this instead of mock memento.
 }
 
@@ -181,10 +139,7 @@ function testTelemetryPrompt(tc: testCase, wantPrompt: boolean) {
 		const commands = tc.goplsWithoutTelemetry ? [] : [GOPLS_MAYBE_PROMPT_FOR_TELEMETRY];
 
 		const sut = new TelemetryService(lc, memento, commands);
-		if (tc.hashMachineID !== undefined) {
-			sinon.stub(sut, 'hashMachineID').returns(tc.hashMachineID);
-		}
-		await sut.promptForTelemetry(!!tc.previewExtension, !tc.vsTelemetryDisabled, tc.samplingInterval);
+		await sut.promptForTelemetry(!tc.vsTelemetryDisabled);
 		if (wantPrompt) {
 			sinon.assert.calledOnce(spy);
 		} else {

internal/vscgo/main.go

@@ -164,33 +164,32 @@ func help(name string) {
 }
 
 // runIncCounters increments telemetry counters read from stdin.
-func runIncCounters(_ []string) error {
+// Write the counters to file provided by env var TELEMETRY_COUNTER_FILE.
+func runIncCounters(_ []string) (rerr error) {
 	scanner := bufio.NewScanner(os.Stdin)
-	if counterFile := os.Getenv("TELEMETRY_COUNTER_FILE"); counterFile != "" {
-		return printCounter(counterFile, scanner)
-	}
-	return runIncCountersImpl(scanner, counter.Add)
-}
 
-func printCounter(fname string, scanner *bufio.Scanner) (rerr error) {
-	f, err := os.OpenFile(fname, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
-	if err != nil {
-		return err
-	}
-	defer func() {
-		if err := f.Close(); rerr == nil {
-			rerr = err
+	if counterFile := os.Getenv("TELEMETRY_COUNTER_FILE"); counterFile != "" {
+		f, err := os.OpenFile(counterFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
+		if err != nil {
+			return err
 		}
-	}()
-	return runIncCountersImpl(scanner, func(name string, count int64) {
-		fmt.Fprintln(f, name, count)
-	})
+		defer func() {
+			if err := f.Close(); err == nil {
+				rerr = err
+			}
+		}()
+		return runIncCountersImpl(scanner, func(name string, count int64) { fmt.Fprintln(f, name, count) })
+	} else {
+		return runIncCountersImpl(scanner, counter.Add)
+	}
 }
 
 const (
 	incCountersBadInput = "inc_counters_bad_input"
 )
 
+// incCountersInputLength returns the counter name based on input counters
+// length.
 func incCountersInputLength(n int) string {
 	const name = "inc_counters_num_input"
 	for i := 1; i < 8; i *= 2 {
@@ -201,6 +200,7 @@ func incCountersInputLength(n int) string {
 	return name + ":>=8"
 }
 
+// incCountersDuration returns the counter name based on input duration.
 func incCountersDuration(duration time.Duration) string {
 	const name = "inc_counters_duration"
 	switch {
@@ -233,7 +233,9 @@ func runIncCountersImpl(scanner *bufio.Scanner, incCounter func(name string, cou
 		linenum++
 		incCounter(name, int64(count))
 	}
+	// Keep track of counter line number.
 	incCounter(incCountersInputLength(linenum), 1)
+	// Keep track of time consumed for each round of counter increment.
 	incCounter(incCountersDuration(time.Since(start)), 1)
 	return nil
 }