Update logic for getting `Analytics` instance from package:unified_analytics #134756

eliasyishak · 2023-09-14T18:36:20Z

Part of:

☂️ package:unified_analytics implementation in flutter tool #128251

Currently, when we want to use the analytics instance from package:unified_analytics, we are just grabbing it from globals. However, with the legacy analytics instance, there are some things we check to return a no-op version of the instance.. for example, if we are running on bots or a non standard branch, we use a no-op instance

This PR uses the same previous checks for the new analytics instance

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 and followed the Flutter Style Guide, including Features we expect every widget to implement.
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 this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

eliasyishak · 2023-09-14T18:55:36Z

packages/flutter_tools/lib/src/reporting/unified_analytics.dart

+
+import '../globals.dart' as globals;
+import '../version.dart';
+


The logic for this function is the same as to what was used for package:usage.

https://github.com/eliasyishak/flutter/blob/use-same-analytics-logic-as-pkg-usage/packages/flutter_tools/lib/src/reporting/usage.dart#L152-L163

packages/flutter_tools/lib/src/context_runner.dart

packages/flutter_tools/lib/src/reporting/unified_analytics.dart

This will be useful when we want to validate that the events sent are in line with GA4's limitations which are checked within `package:unified_analytics`

This reverts commit f2181ae.

Per the instructions in the wiki https://github.com/flutter/flutter/wiki/The-flutter-tool#adding-removing-or-making-changes-to-dart-dependencies

This reverts commit 2cb2300.

…s-logic-as-pkg-usage

dev/a11y_assessments/pubspec.yaml

This reverts commit 86848dd.

christopherfujino · 2023-09-20T18:39:18Z

FYI I'm unsubscribing for now so I don't get notified each time you push. Feel free to ping me again when it's ready for review.

eliasyishak · 2023-09-28T13:24:20Z

@christopherfujino this PR is ready for your review now

packages/flutter_tools/test/general.shard/unified_analytics_test.dart

packages/flutter_tools/lib/src/reporting/unified_analytics.dart

eliasyishak · 2023-09-28T21:15:31Z

packages/flutter_tools/test/src/context.dart

          Pub: () => ThrowingPub(), // prevent accidentally using pub.
          CrashReporter: () => const NoopCrashReporter(),
          TemplateRenderer: () => const MustacheTemplateRenderer(),
+          Analytics: () => NoOpAnalytics(),


Added default here @christopherfujino

christopherfujino

LGTM

…ified_analytics (flutter/flutter#134756)

flutter/flutter@57b5c3c...d3df8f6 2023-09-30 [email protected] Roll Flutter Engine from 68fc1a567195 to 01fc3c6624ce (1 revision) (flutter/flutter#135808) 2023-09-30 [email protected] Roll Flutter Engine from 4ed733d99cd5 to 68fc1a567195 (1 revision) (flutter/flutter#135807) 2023-09-30 [email protected] Roll Flutter Engine from d814c69abe26 to 4ed733d99cd5 (1 revision) (flutter/flutter#135801) 2023-09-30 [email protected] Roll Flutter Engine from f726438d4d65 to d814c69abe26 (1 revision) (flutter/flutter#135799) 2023-09-30 [email protected] Roll Flutter Engine from 0b96a82cc874 to f726438d4d65 (1 revision) (flutter/flutter#135796) 2023-09-30 [email protected] Roll Flutter Engine from f3a4cab69962 to 0b96a82cc874 (1 revision) (flutter/flutter#135793) 2023-09-30 [email protected] Roll Flutter Engine from e719b7957763 to f3a4cab69962 (2 revisions) (flutter/flutter#135792) 2023-09-30 [email protected] Roll Flutter Engine from 8939f9c1f4e4 to e719b7957763 (2 revisions) (flutter/flutter#135789) 2023-09-30 [email protected] Roll Flutter Engine from 6a13607008ed to 8939f9c1f4e4 (1 revision) (flutter/flutter#135788) 2023-09-30 [email protected] Add device ready check (flutter/flutter#135526) 2023-09-30 [email protected] Temporarily disable NexusLowRes, version=29 (flutter/flutter#135785) 2023-09-30 [email protected] Cover more test/widgets tests with leak tracking #12. (flutter/flutter#135385) 2023-09-30 [email protected] DraggableScrollableController should dispatch creation in constructor. (flutter/flutter#135423) 2023-09-30 [email protected] Roll Flutter Engine from d203e349781c to 6a13607008ed (2 revisions) (flutter/flutter#135780) 2023-09-29 [email protected] Roll Flutter Engine from 48973d7f9beb to d203e349781c (5 revisions) (flutter/flutter#135776) 2023-09-29 [email protected] ViewportOffset should dispatch creation in constructor. (flutter/flutter#135420) 2023-09-29 [email protected] Use platform dill from the engine when compiling with dart2wasm. (flutter/flutter#134970) 2023-09-29 [email protected] Roll Flutter Engine from d5fcd78a879b to 48973d7f9beb (5 revisions) (flutter/flutter#135765) 2023-09-29 [email protected] Upgrade leak_tracker to fix flackiness. (flutter/flutter#135760) 2023-09-29 [email protected] Roll Flutter Engine from c52251a8b2d0 to d5fcd78a879b (2 revisions) (flutter/flutter#135755) 2023-09-29 [email protected] Update `Drawer` tests for M2/M3 (flutter/flutter#135752) 2023-09-29 [email protected] Expose onAttach and onDetach in ScrollController subclasses (flutter/flutter#135721) 2023-09-29 [email protected] Update the wasm bootstrap to allow access to the emscripten wrapper to skwasm. (flutter/flutter#135723) 2023-09-29 [email protected] Update logic for getting `Analytics` instance from package:unified_analytics (flutter/flutter#134756) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…alytics (flutter#134756) Part of: - flutter#128251 Currently, when we want to use the analytics instance from `package:unified_analytics`, we are just grabbing it from globals. However, with the legacy analytics instance, there are some things we check to return a no-op version of the instance.. for example, if we are running on bots or a non standard branch, we use a no-op instance This PR uses the same previous checks for the new analytics instance

Move the logic into context runner

2ae2d57

github-actions bot added a: text input Entering text in a text field or keyboard related problems tool Affects the "flutter" command-line tool. See also t: labels. labels Sep 14, 2023

Use function instead of handling logic in context runner

b77f179

eliasyishak changed the title ~~Move the logic into context runner~~ Update logic for getting Analytics instance from package:unified_analytics Sep 14, 2023

eliasyishak removed the a: text input Entering text in a text field or keyboard related problems label Sep 14, 2023

eliasyishak commented Sep 14, 2023

View reviewed changes

DI parameters for getAnalytics function

8f1a03f

github-actions bot added the a: text input Entering text in a text field or keyboard related problems label Sep 14, 2023

christopherfujino reviewed Sep 14, 2023

View reviewed changes

packages/flutter_tools/lib/src/context_runner.dart Show resolved Hide resolved

christopherfujino reviewed Sep 14, 2023

View reviewed changes

packages/flutter_tools/lib/src/context_runner.dart Outdated Show resolved Hide resolved

christopherfujino reviewed Sep 14, 2023

View reviewed changes

packages/flutter_tools/lib/src/reporting/unified_analytics.dart Outdated Show resolved Hide resolved

Pass env map to function + clean up from dartfmt

1418db9

eliasyishak removed the a: text input Entering text in a text field or keyboard related problems label Sep 15, 2023

Include default parameter for enableAsserts

2aa03aa

This will be useful when we want to validate that the events sent are in line with GA4's limitations which are checked within `package:unified_analytics`

github-actions bot added the a: text input Entering text in a text field or keyboard related problems label Sep 15, 2023

eliasyishak added 2 commits September 18, 2023 11:36

Create unified_analytics_test.dart

9e895e2

flutter update-packages to get unified analytics 4.0.0

f2181ae

eliasyishak added 5 commits September 19, 2023 10:34

Revert "flutter update-packages to get unified analytics 4.0.0"

302b652

This reverts commit f2181ae.

Running update pkgs after manually editing pubspec

2cb2300

Per the instructions in the wiki https://github.com/flutter/flutter/wiki/The-flutter-tool#adding-removing-or-making-changes-to-dart-dependencies

Revert "Running update pkgs after manually editing pubspec"

4cebbd7

This reverts commit 2cb2300.

Merge remote-tracking branch 'upstream/master' into use-same-analytic…

9b0db48

…s-logic-as-pkg-usage

Update packages after merge with master

86848dd

christopherfujino reviewed Sep 19, 2023

View reviewed changes

dev/a11y_assessments/pubspec.yaml Outdated Show resolved Hide resolved

eliasyishak added 3 commits September 19, 2023 18:24

Remove analyze error

1b8d0d9

Revert "Update packages after merge with master"

4071722

This reverts commit 86848dd.

Merge branch 'master' into use-same-analytics-logic-as-pkg-usage

49843f8

eliasyishak added 4 commits September 26, 2023 12:14

Merge branch 'master' into use-same-analytics-logic-as-pkg-usage

0cd38fe

testUsingContext -> testWithoutContext

8b71197

For real instance, check it is not noop id

3ca830f

Static getter to check client id as well runtime type

522bba2

eliasyishak marked this pull request as ready for review September 28, 2023 12:22

Merge branch 'master' into use-same-analytics-logic-as-pkg-usage

2db4e08

christopherfujino reviewed Sep 28, 2023

View reviewed changes

eliasyishak added 2 commits September 28, 2023 15:50

Nit fixes

ca47ef7

Update default context for Analytics in tests

41a6bb0

eliasyishak commented Sep 28, 2023

View reviewed changes

christopherfujino approved these changes Sep 28, 2023

View reviewed changes

eliasyishak added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 29, 2023

auto-submit bot merged commit ba14169 into flutter:master Sep 29, 2023

engine-flutter-autoroll mentioned this pull request Sep 30, 2023

Roll Flutter from 57b5c3cda000 to 8e3b7a9cb5d1 (22 revisions) flutter/packages#5041

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 30, 2023

ba14169 Update logic for getting Analytics instance from package:un…

1bddfcf

…ified_analytics (flutter/flutter#134756)

engine-flutter-autoroll mentioned this pull request Sep 30, 2023

Roll Flutter from 57b5c3cda000 to 0a8963095468 (23 revisions) flutter/packages#5042

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 30, 2023

ba14169 Update logic for getting Analytics instance from package:un…

d3d749e

…ified_analytics (flutter/flutter#134756)

engine-flutter-autoroll mentioned this pull request Sep 30, 2023

Roll Flutter from 57b5c3cda000 to d3df8f6bbf74 (24 revisions) flutter/packages#5043

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 30, 2023

ba14169 Update logic for getting Analytics instance from package:un…

9807f29

…ified_analytics (flutter/flutter#134756)

eliasyishak mentioned this pull request Oct 16, 2023

Initialize Analytics class using same conditionals as legacy analytics #130342

Closed

eliasyishak deleted the use-same-analytics-logic-as-pkg-usage branch October 27, 2023 22:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update logic for getting `Analytics` instance from package:unified_analytics #134756

Update logic for getting `Analytics` instance from package:unified_analytics #134756

Uh oh!

eliasyishak commented Sep 14, 2023 •

edited

Loading

Uh oh!

eliasyishak Sep 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christopherfujino commented Sep 20, 2023

Uh oh!

eliasyishak commented Sep 28, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eliasyishak Sep 28, 2023

Uh oh!

christopherfujino Sep 28, 2023

Uh oh!

christopherfujino left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		import '../globals.dart' as globals;
		import '../version.dart';

Update logic for getting Analytics instance from package:unified_analytics #134756

Update logic for getting Analytics instance from package:unified_analytics #134756

Uh oh!

Conversation

eliasyishak commented Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

eliasyishak Sep 14, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christopherfujino commented Sep 20, 2023

Uh oh!

eliasyishak commented Sep 28, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eliasyishak Sep 28, 2023

Choose a reason for hiding this comment

Uh oh!

christopherfujino Sep 28, 2023

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Update logic for getting `Analytics` instance from package:unified_analytics #134756

Update logic for getting `Analytics` instance from package:unified_analytics #134756

eliasyishak commented Sep 14, 2023 •

edited

Loading