golang / vscode-go

Go extension for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=golang.Go
Other
3.78k stars 728 forks source link

goTelemetry: make start time recording robust against unexpected vscode memento API behavior #3312

Closed hyangah closed 2 months ago

hyangah commented 3 months ago

The current implementation intended to allow a one week grace period after gopls with telemetry capability is installed. The intention was to have some time to collect sample telemetry data on disk, so users can inspect the data when they are eventually prompted.

The implementation used VS Code's Memento API to record the Date when the gopls v0.14+ was first installed. https://github.com/golang/vscode-go/blob/19afd64b3242c364d267ee93c6aa2ae27a1d73b4/extension/src/goTelemetry.ts#L176

Recently, however, we noticed the API changed the behavior (https://github.com/microsoft/vscode/issues/209479), and that causespromptForTelemetry abort (https://github.com/golang/vscode-go/blob/19afd64b3242c364d267ee93c6aa2ae27a1d73b4/extension/src/goTelemetry.ts#L177). This was also observed during https://github.com/golang/vscode-go/issues/3256 (the reported issue itself doesn't seem related this a.getTime is not a function error message from Extension Host).

Looks like the encoding is currently working as the API stated (Date.toJSON) but decoding doesn't seem to be working. Roll our own encoding (Date.toJSON) and decoding, to protect us in the future. We will be able to salvage the value currently stored in the memento since the current value is Date.toJSON.

~Gopls v0.14 was released a while ago, and most users already have some data to inspect already. Let's remove this check for simplicity.~

cc @golang/tools-team

gopherbot commented 3 months ago

Change https://go.dev/cl/576136 mentions this issue: extension/src/goTelemetry: remove 1wk grace period

gopherbot commented 2 months ago

Change https://go.dev/cl/576775 mentions this issue: extension/src/goTelemetry: do our JSON enc/dec for telemetry start time

gopherbot commented 2 months ago

Change https://go.dev/cl/580421 mentions this issue: extension/src/goTelemetry: do our JSON enc/dec for telemetry start time