Skip to content
New issue

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

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

Already on GitHub? # to your account

unified_analytics does not understand opt-out from legacy analytics #74

Closed
jcollins-g opened this issue Apr 5, 2023 · 10 comments · Fixed by #80
Closed

unified_analytics does not understand opt-out from legacy analytics #74

jcollins-g opened this issue Apr 5, 2023 · 10 comments · Fixed by #80

Comments

@jcollins-g
Copy link
Contributor

jcollins-g commented Apr 5, 2023

@jacob314

Currently, unified_analytics does not recognize opt-outs from the legacy analytics. Jacob indicated that this functionality has been discussed as being a possible enhancement for 3.0 to reduce unnecessary prompting, so we should add the ability to read legacy analytics and forward an opt-out there to a reporting=0 in the new config file. We should only forward an opt-out.

@eliasyishak
Copy link
Contributor

As a follow up, I think it would probably be best for the each client using this package to make the new package "aware" of the legacy opt out status.

This PR (flutter/flutter#124606) is an example of how the flutter tool will set telemetry to false if the legacy analytics class is reporting that the developer has opted out

Thoughts @jcollins-g ? I feel like this may be easier than trying to find where each tool persists files for opt out status within the package

@jcollins-g
Copy link
Contributor Author

I had thought that legacy analytics was driven entirely through the https://pub.dev/packages/usage package. But if that's not the case and every tool has its own location to pick up analytics, then yes it might make sense to do that.

@eliasyishak
Copy link
Contributor

Perfect, I'll go ahead and close this issue then. Feel free to reopen if something comes up

@jcollins-g
Copy link
Contributor Author

I am a little confused here, again I thought that legacy analytics was driven through one entry point which would mean in my view that unified_analytics should just handle it for all clients.

@eliasyishak
Copy link
Contributor

hm let me know if this example explains it? I could totally be missing what you're trying to say.

So consider that a flutter developer has been using flutter for the past year and has already opted out. Once they upgrade to the new version that includes unified_analytics, the flutter tool will be able to communicate to unified_analytics to let it know that this user has already opted out. So internally within flutter tools, it will invoke the analytics.setTelemetry(false) method.

// Trivial example
if (!legacyAnalytics.enabled && unifiedAnalytics.telemetryEnabled) {
  unifiedAnalytics.setTelemetry(false)
}

We would essentially have any tool that is a client of this package do this check with it's legacy analytics runner.

@bwilkerson
Copy link
Member

... any tool that is a client of this package ...

I think that's the crux of the comment. Why should every tool repeat this code? The unified_analytics package could have the same check so that every tool would do the right thing without adding any code to the tool.

@eliasyishak
Copy link
Contributor

eliasyishak commented Apr 12, 2023

Because each client using this package would know how to access their telemetry opt in status best instead of having this package attempt to check for a persisted file that each tool uses. I think there would be a lot more variability encountered that can be error prone.

ie. in my example above, it's much easier for the client using the package to invoke the pseudo command 'legacyAnalytics.enabled` than it would be for the package to know where this state is being persisted for flutter_tools. Especially considering certain users may have several flutter versions on their machine

@bwilkerson
Copy link
Member

Because each client using this package would know how to access their telemetry opt in status best ...

One premise of the question is that, as far as we know, all of the current and future clients of this package were using the usage package to report analytics, so there is, as far as we know, only one place that would need to be queried.

@eliasyishak
Copy link
Contributor

eliasyishak commented Apr 13, 2023

Following discussion:

Need to check the following files for legacy opt out status:

  • Dart: $HOME/.dart/dartdev.json
  • Flutter: $HOME/.flutter

I believe that's all we need to check right? @jcollins-g

@jcollins-g
Copy link
Contributor Author

@eliasyishak Yes, that's correct to my reading of the code.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Apr 17, 2023
This will customize the consent message based on the `dart` or `flutter`
entry points and allow unified_analytics to honor the legacy analytics
opt-out.

Changes:
```
> git log --format="%C(auto) %h %s" 2308c67..545d7e1
 https://dart.googlesource.com/tools.git/+/545d7e1 Honor legacy opt out status (80)

```

Diff: https://dart.googlesource.com/tools.git/+/2308c672e0d7446f5bfdba96594b41f26fa24a2e..545d7e1c73ce21b8c91f638021f9d487d324a501/
Bug: dart-lang/tools#74
Bug: dart-lang/tools#82
Change-Id: I36bffde69b6d7f798a40f140ca5e838336d81d3f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295742
Reviewed-by: Elias Yishak <eliasyishak@google.com>
Commit-Queue: Elias Yishak <eliasyishak@google.com>
Commit-Queue: Janice Collins <jcollins@google.com>
Auto-Submit: Janice Collins <jcollins@google.com>
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Apr 20, 2023
This will customize the consent message based on the `dart` or `flutter`
entry points and allow unified_analytics to honor the legacy analytics
opt-out.

Changes:
```
> git log --format="%C(auto) %h %s" 2308c67..545d7e1
 https://dart.googlesource.com/tools.git/+/545d7e1 Honor legacy opt out status (80)

```

Diff: https://dart.googlesource.com/tools.git/+/2308c672e0d7446f5bfdba96594b41f26fa24a2e..545d7e1c73ce21b8c91f638021f9d487d324a501/
Bug: dart-lang/tools#74
Bug: dart-lang/tools#82
Change-Id: I72f20185c6de18079fc66b812fb2b51c4e608441
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/295742
Commit-Queue: Janice Collins <jcollins@google.com>
Auto-Submit: Janice Collins <jcollins@google.com>
Fixes: #52082
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295920
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants