Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
hyangah opened this issue Apr 3, 2024 · 3 comments
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Apr 3, 2024

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.

const telemetryStartTime = this.globalState.get<Date>(TELEMETRY_START_TIME_KEY, now);

Recently, however, we noticed the API changed the behavior (microsoft/vscode#209479),
and that causespromptForTelemetry abort (

if (daysBetween(telemetryStartTime, now) < 7) {
). This was also observed during #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 gopherbot added this to the Untriaged milestone Apr 3, 2024
@gopherbot
Copy link
Collaborator

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

@hyangah hyangah modified the milestones: Untriaged, v0.41.3 Apr 4, 2024
@hyangah hyangah changed the title goTelemetry: remove 1 week grace period check before prompting goTelemetry: make start time recording robust against unexpected vscode memento API behavior Apr 4, 2024
@gopherbot
Copy link
Collaborator

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

@gopherbot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants