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

Fix telemetry disable command #9290

Merged
merged 7 commits into from
Aug 15, 2024
Merged

Fix telemetry disable command #9290

merged 7 commits into from
Aug 15, 2024

Conversation

DavidMulder0
Copy link
Contributor

No description provided.

@DavidMulder0
Copy link
Contributor Author

Minor thought, the right thing to do might be to discard any telemetry data collected from the period this command was broken.

This comment was marked as resolved.

@dcousens
Copy link
Member

dcousens commented Aug 14, 2024

The telemetry disable command was broken in e69def2 (by me, sigh) - but this is the first report we have had about this problem, which is interesting.

Affected versions are >=6.1.0, which was published on 30th of April 2024.

$ keystone telemetry disable
telemetry disable is an unknown command

As we want to err on the side of consent being missed, I think we could do the following:

  • Delete project events where @keystone-6/core === 6.1.0 and @keystone-6/core === 6.2.0
  • Delete ALL device events between 2024-04-29 (N-1) and 2024-??-??

The nature of device events being unversioned however results in an ongoing problem that we would not know if device events are coming from a device with @keystone-6/core === 6.1.0 or @keystone-6/core === 6.2.0. In light of that, I think the easiest conclusion is to shutdown the current telemetry.keystonejs.com/v1/ endpoint and activate a new, different endpoint in a future pull request; deleting any data >= 2024-04-29.

I'm open to feedback on this from the community, and thanks @DavidMulder0 for reporting this.
The response may seem reactive, but I think it's important to show that we respect any data collected from the community.

Summary of SQL deletion commands
DELETE FROM "DeviceEvent" WHERE "reportedAt" <= NOW() - INTERVAL '1 year';
DELETE FROM "DeviceEvent" WHERE "reportedAt" >= '2024-04-29';
DELETE FROM "ProjectEvent" WHERE "reportedAt" <= NOW() - INTERVAL '1 year';
DELETE FROM "ProjectEvent" WHERE "reportedAt" >= '2024-04-29';

@dcousens dcousens changed the title Fix telemetry commands (#9289) Fix telemetry disable command Aug 14, 2024
@dcousens dcousens added 🐛 bug Unresolved bug help wanted labels Aug 15, 2024
// every informedAt was a copy of device.informedAt, it was copied everywhere
informedAt: existing.device.informedAt,
const replacement: TelemetryVersion2and3 = {
informedAt: null, // re-inform
Copy link
Member

@dcousens dcousens Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone has unknowingly missed their opt-out failing, this will re-inform them


store.set('telemetry', {
...existing,
informedAt: null, // re-inform
Copy link
Member

@dcousens dcousens Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone has unknowingly missed their opt-out failing, this will re-inform them

@dcousens dcousens merged commit 8745a39 into keystonejs:main Aug 15, 2024
42 of 43 checks passed
@dcousens
Copy link
Member

Merged as 830d46d

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

Successfully merging this pull request may close these issues.

2 participants