Setting up package:unified_analytics within devtools

[eliasyishak]

Part of tracker issue:

• ☔ Migration to package:unified_analytics for analytics collection #7083

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read the Flutter Style Guide recently, and have followed its advice.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or there is a reason for not adding tests.

If you need help, consider asking for help on Discord.

[eliasyishak]

Swap placeholders out

[eliasyishak]

@kenzieschmoll any recommendations for where I should create the Analytics instance? Ideally it can be somewhere it can get imported from or somewhere near the entrypoint so you can direct inject it into where it is needed

Also what's the best way to resolve these parameters?

[kenzieschmoll]

How do we get these values from other tools?

Since this code is on the devtools_server where we have access to dart:io, one idea is to run flutter --version and parse the output. We have a class that parses the output from Flutter Tools' registered flutterVersion service here: https://github.com/flutter/devtools/blob/master/packages/devtools_app_shared/lib/src/service/flutter_version.dart/#L16. We could move this class to devtools_shared if it was useful for parsing the output from flutter --version.

A couple things we will need to be careful about:

• using the flutter that this DevTools server instance is being run from, since a user could have multiple flutter's on their machine / path.
• ensuring these values can be null when flutter is not on the user's machine (for example if DevTools was being used from a pure Dart user who only writes Dart CLI apps or Dart web apps.

WRT for where to create the Analytics instance, where do we create the Analytics instance for other tools? Near the root of initialization? Some ideas I have:

• create it in the constructor of the DevToolsServer class: https://github.com/dart-lang/sdk/blob/main/pkg/dds/lib/devtools_server.dart#L23
• make it a late final variable that is created and passed into defaultHandler similar to what we do for clientManager: https://github.com/dart-lang/sdk/blob/main/pkg/dds/lib/devtools_server.dart#L265-L271

both of these scenarios would require that the Analytics instance is passed in as a parameter to ServerApi.handle (similar to how extensionsManager and deepLinkManager are passed in): https://github.com/flutter/devtools/blob/master/packages/devtools_shared/lib/src/server/server_api.dart/#L37

It is also worth noting that ServerApi.handle is called from google3 on a separate path, so we'd have to make sure we create the Analytics instance properly there too.

[eliasyishak]

using the flutter that this DevTools server instance is being run from, since a user could have multiple flutter's on their machine / path.

Yeah this was the first thing I was concerned about, since devtools also downloads its own separate flutter sdk in /devtools/tool/flutter-sdk, wouldn't running flutter --version --machine return versioning information for flutter that is on the path?

For example, the flutter sdk that is in the devtools directory is version "frameworkVersion": "3.20.0-0.0.pre", while my flutter in path resolves to "frameworkVersion": "3.20.0-2.0.pre.9",... which of these values would we want to report for analytics?

[kenzieschmoll]

The tool/flutter-sdk Fluter isn't used in production. This is just for all of our repository management scripts as well as for the workflows we run on the CI.

We care about the Dart or Flutter SDK from which DevTools was started by the IDE. I believe the IDEs run the dart devtools command to start a DevTools server instance.

[kenzieschmoll]

I had another thought WRT opt-in logic. Since we are going to continue to send events to the old analytics ID and through the new unified_analytics package, should we keep the opt in states separate for those? So old analytics would check out existing logic and new analytics would use the unified_analytics opt in logic? This would mean we'd need different apiGetDevToolsEnabled and apiSetDevToolsEnabled endpoints for both legacy analytics and new analytics.

[eliasyishak]

hmm so are you suggesting we would have to perform 2 separate actions if we wanted to opt out of analytics completely as a devtools user? Or is this more of a suggestion to have another case clause in the server_api.dart?

Having to perform 2 separate actions to opt out may confuse some of our users

[kenzieschmoll]

"and to allow...". This part is a little confusing to me. What if the user has selected 'no thanks'? We still need to mark the consent message as shown right? But this makes it sound like this method not only marks the consent message as shown, but also opts in to analytics.

[eliasyishak]

So I made the decision to automatically mark the message as shown when we show the text. If the user was previously opted out through other dash tools, then they will remain opted out. If they press the accept button, we will change the their telemetry status to opted in for all dash tools.

But in both cases, marking the message as shown just tells the package to add it to the configuration file and log the date the message was shown

[kenzieschmoll]

cleanup nit: we can use the helper LinkTextSpan for this. If you do use that helper, the ga properties for Link can be made optional.

[eliasyishak]

Sounds good, i made the fields Link.gaScreenName and Link.gaSelectedItemDescription nullable. I also added conditionals for where we call ga.select() with those two fields to not send anything if they are null

[kenzieschmoll]

when these diverge, should we be setting the value of _devtoolsUsage.analyticsEnabled to analytics.telemetryEnabled?

[eliasyishak]

I think once we have fully migrated, we could probably get rid of _devtoolsUsage all together since the package will check itself when an event is attempted to be sent. Example below of how we do it in the flutter-tool by just calling the send method without any wrapping class

https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/runner.dart#L190