Skip to content

Clear contents of persisted files on opt out #87

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

Merged
merged 10 commits into from
May 3, 2023

Conversation

eliasyishak
Copy link
Contributor

@eliasyishak eliasyishak commented Apr 28, 2023

When the user opts out of telemetry collection, we will clear the contents of the following files:

  • CLIENT_ID
  • dart-flutter-telemetry.log
  • dart-flutter-session.json

If the user chooses to opt back into telemetry collection, these files will be regenerated:

  • CLIENT_ID – a new client id will be regenerated
  • dart-flutter-telemetry.log – events will begin to get persisted locally again as they are sent
  • dart-flutter-session.json – the json object containing the session_id and last_ping will be regenerated

Reference issue: #84

@eliasyishak eliasyishak requested a review from jcollins-g May 2, 2023 16:11
@eliasyishak
Copy link
Contributor Author

@jcollins-g this is the second PR that is required by the PDD to clear the files on opt out – I have landed the previous one that removed the pddFlag already

@eliasyishak eliasyishak requested a review from bwilkerson May 3, 2023 14:50
Copy link
Member

@bwilkerson bwilkerson left a comment

Choose a reason for hiding this comment

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

Assuming that there's a good reason for leaving empty files on the disk, lgtm.

if (!reportingBool) {
_sessionHandler.sessionFile.writeAsStringSync('');
_logHandler.logFile.writeAsStringSync('');
_clientIdFile.writeAsStringSync('');
Copy link
Member

Choose a reason for hiding this comment

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

I would have assumed that the files would be deleted from disk, not just emptied. Is there a reason for leaving an empty file on disk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We asked to have it be cleared because I didn't want to change the initializer logic, on startup, the Initializer class checks to make sure each of these files exist, if not, it will create them. And this happens before we determine if the user has opted out so it would be a lot more refactoring

@eliasyishak eliasyishak removed the request for review from jcollins-g May 3, 2023 20:01
@eliasyishak eliasyishak merged commit 6c68bca into dart-lang:main May 3, 2023
@eliasyishak eliasyishak deleted the clear-files-on-opt-out branch May 9, 2023 16:44
dcharkes pushed a commit that referenced this pull request May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants