extension/src/goTelemetry: do our JSON enc/dec for telemetry start time
We noticed the vscode Memento API doesn't behave as we expected. That
made our recent attempt to increase the telemetry prompt rate
stopped by this bug.
Instead of relying on the Memento API's JSON stringify for Date object
encoding, do the enc/dec work on ourside and let Memento work with
string types.
The existing changes cover the json encoding, decoding cases.
Now goMain.ts activate() returns ExtensionTestAPI in testing mode.
In telemetry testing, we want to check if the value recorded with
vscode's real Memento API can be still usable. The real implementation
is accessible only by capturing the ExtensionContext passed to the
activate() invocation. Allow our test to access the extension's
globalState using the new ExtensionTestAPI.
Fixes golang/vscode-go#3312
Change-Id: I4540f83201f315624b077d113d6bfe2b3d608719
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/576775
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Hyang-Ah Hana Kim <hyangah@gmail.com>
kokoro-CI: kokoro <noreply+kokoro@google.com>
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
diff --git a/extension/src/goMain.ts b/extension/src/goMain.ts
index 73265bd..4f213f0 100644
--- a/extension/src/goMain.ts
+++ b/extension/src/goMain.ts
@@ -76,12 +76,24 @@
const goCtx: GoExtensionContext = {};
-export async function activate(ctx: vscode.ExtensionContext): Promise<ExtensionAPI | undefined> {
- if (process.env['VSCODE_GO_IN_TEST'] === '1') {
- // Make sure this does not run when running in test.
- return;
- }
+// Allow tests to access the extension context utilities.
+interface ExtensionTestAPI {
+ globalState: vscode.Memento;
+}
+export async function activate(ctx: vscode.ExtensionContext): Promise<ExtensionAPI | ExtensionTestAPI | undefined> {
+ if (process.env['VSCODE_GO_IN_TEST'] === '1') {
+ // TODO: VSCODE_GO_IN_TEST was introduced long before we learned about
+ // ctx.extensionMode, and used in multiple places.
+ // Investigate if use of VSCODE_GO_IN_TEST can be removed
+ // in favor of ctx.extensionMode and clean up.
+ if (ctx.extensionMode === vscode.ExtensionMode.Test) {
+ return { globalState: ctx.globalState };
+ }
+ // We shouldn't expose the memento in production mode even when VSCODE_GO_IN_TEST
+ // environment variable is set.
+ return; // Skip the remaining activation work.
+ }
const start = Date.now();
setGlobalState(ctx.globalState);
setWorkspaceState(ctx.workspaceState);
diff --git a/extension/src/goTelemetry.ts b/extension/src/goTelemetry.ts
index 081a5aa..f37bd39 100644
--- a/extension/src/goTelemetry.ts
+++ b/extension/src/goTelemetry.ts
@@ -21,6 +21,25 @@
// Exported for testing.
export const TELEMETRY_START_TIME_KEY = 'telemetryStartTime';
+// Run our encode/decode function for the Date object, to be defensive
+// from vscode Memento API behavior change.
+// Exported for testing.
+export function recordTelemetryStartTime(storage: vscode.Memento, date: Date) {
+ storage.update(TELEMETRY_START_TIME_KEY, date.toJSON());
+}
+
+function readTelemetryStartTime(storage: vscode.Memento): Date | null {
+ const value = storage.get<string | number | Date>(TELEMETRY_START_TIME_KEY);
+ if (!value) {
+ return null;
+ }
+ const telemetryStartTime = new Date(value);
+ if (telemetryStartTime.toString() === 'Invalid Date') {
+ return null;
+ }
+ return telemetryStartTime;
+}
+
enum ReporterState {
NOT_INITIALIZED,
IDLE,
@@ -153,9 +172,9 @@
this.active = true;
// record the first time we see the gopls with telemetry support.
// The timestamp will be used to avoid prompting too early.
- const telemetryStartTime = globalState.get<Date>(TELEMETRY_START_TIME_KEY);
+ const telemetryStartTime = readTelemetryStartTime(globalState);
if (!telemetryStartTime) {
- globalState.update(TELEMETRY_START_TIME_KEY, new Date());
+ recordTelemetryStartTime(globalState, new Date());
}
}
@@ -172,9 +191,11 @@
if (!isVSCodeTelemetryEnabled) return;
// Allow at least 7days for gopls to collect some data.
- const now = new Date();
- const telemetryStartTime = this.globalState.get<Date>(TELEMETRY_START_TIME_KEY, now);
- if (daysBetween(telemetryStartTime, now) < 7) {
+ const telemetryStartTime = readTelemetryStartTime(this.globalState);
+ if (!telemetryStartTime) {
+ return;
+ }
+ if (daysBetween(telemetryStartTime, new Date()) < 7) {
return;
}
diff --git a/extension/test/gopls/extension.test.ts b/extension/test/gopls/extension.test.ts
index ee0facb..69d1eb5 100644
--- a/extension/test/gopls/extension.test.ts
+++ b/extension/test/gopls/extension.test.ts
@@ -10,7 +10,7 @@
import { getGoConfig } from '../../src/config';
import sinon = require('sinon');
import { getGoVersion, GoVersion } from '../../src/util';
-import { GOPLS_MAYBE_PROMPT_FOR_TELEMETRY, TELEMETRY_START_TIME_KEY, TelemetryService } from '../../src/goTelemetry';
+import { GOPLS_MAYBE_PROMPT_FOR_TELEMETRY, recordTelemetryStartTime, TelemetryService } from '../../src/goTelemetry';
import { MockMemento } from '../mocks/MockMemento';
import { Env } from './goplsTestEnv.utils';
@@ -205,8 +205,7 @@
const workspaceDir = path.resolve(testdataDir, 'gogetdocTestData');
await env.startGopls(path.join(workspaceDir, 'test.go'), undefined, workspaceDir);
const memento = new MockMemento();
- memento.update(TELEMETRY_START_TIME_KEY, new Date('2000-01-01'));
-
+ recordTelemetryStartTime(memento, new Date('2000-01-01'));
const sut = new TelemetryService(env.languageClient, memento, [GOPLS_MAYBE_PROMPT_FOR_TELEMETRY]);
try {
await Promise.all([
diff --git a/extension/test/gopls/telemetry.test.ts b/extension/test/gopls/telemetry.test.ts
index 32dc1fc..5d09301 100644
--- a/extension/test/gopls/telemetry.test.ts
+++ b/extension/test/gopls/telemetry.test.ts
@@ -10,7 +10,8 @@
GOPLS_MAYBE_PROMPT_FOR_TELEMETRY,
TELEMETRY_START_TIME_KEY,
TelemetryReporter,
- TelemetryService
+ TelemetryService,
+ recordTelemetryStartTime
} from '../../src/goTelemetry';
import { MockMemento } from '../mocks/MockMemento';
import { maybeInstallVSCGO } from '../../src/goInstallTools';
@@ -21,9 +22,12 @@
import { rmdirRecursive } from '../../src/util';
import { extensionId } from '../../src/const';
import { executableFileExists, fileExists } from '../../src/utils/pathUtils';
-import { ExtensionMode } from 'vscode';
+import { ExtensionMode, Memento, extensions } from 'vscode';
-describe('# prompt for telemetry', () => {
+describe('# prompt for telemetry', async () => {
+ const extension = extensions.getExtension(extensionId);
+ assert(extension);
+
it(
'do not prompt if language client is not used',
testTelemetryPrompt(
@@ -131,6 +135,22 @@
false
)
);
+ // testExtensionAPI.globalState is a real memento instance passed by ExtensionHost.
+ // This instance is active throughout the integration test.
+ // When you add more test cases that interact with the globalState,
+ // be aware that multiple test cases may access and mutate it asynchronously.
+ const testExtensionAPI = await extension.activate();
+ it('check we can salvage the value in the real memento', async () => {
+ // write Date with Memento.update - old way. Now we always use string for TELEMETRY_START_TIME_KEY value.
+ testExtensionAPI.globalState.update(TELEMETRY_START_TIME_KEY, new Date(Date.now() - 7 * 24 * 60 * 60 * 1000));
+ await testTelemetryPrompt(
+ {
+ samplingInterval: 1000,
+ mementoInstance: testExtensionAPI.globalState
+ },
+ true
+ )();
+ });
});
interface testCase {
@@ -141,6 +161,7 @@
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.
}
function testTelemetryPrompt(tc: testCase, wantPrompt: boolean) {
@@ -153,9 +174,9 @@
const spy = sinon.spy(languageClient, 'sendRequest');
const lc = tc.noLangClient ? undefined : languageClient;
- const memento = new MockMemento();
+ const memento = tc.mementoInstance ?? new MockMemento();
if (tc.firstDate) {
- memento.update(TELEMETRY_START_TIME_KEY, tc.firstDate);
+ recordTelemetryStartTime(memento, tc.firstDate);
}
const commands = tc.goplsWithoutTelemetry ? [] : [GOPLS_MAYBE_PROMPT_FOR_TELEMETRY];
