feat: geo-enrich analytics events via GeolocationController - #9691
Conversation
Implements ADR-0008 (extension/mobile metric geo-enrichment). GeolocationController now resolves country, region, and timezone from the v2 geolocation endpoint, exposed via new GeolocationController:getGeolocationData and GeolocationApiService:fetchGeolocationData actions (returning a GeolocationData object) plus matching controller state fields. AnalyticsController optionally enriches non-anonymous track, identify, and view payloads with country_code, region, and timezone under context.location. Enrichment is gated behind a new isGeolocationEnabled option (default false); when enabled, geolocation is resolved during the now-async init before queued events are replayed. Anonymous payloads carry no location, per the ADR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
…nit ordering When geolocation enrichment is enabled, AnalyticsController resolves the location during init via GeolocationController:getGeolocationData. If the GeolocationController is not registered/initialized first, that call fails and enrichment is skipped for the session. - Log an actionable message on resolution failure naming the likely cause (init ordering) instead of a generic one, so the otherwise-silent failure is diagnosable. - Document the ordering requirement in the init() JSDoc and changelog. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The v2 geolocation endpoint returns country and region separately and never concatenates them. The legacy v1 endpoint appended the region only for the US and Canada (e.g. US-NY, CA-ON) and returned the country alone elsewhere. Update toLocationCode so the location code derived for getGeolocation/fetchGeolocation preserves that v1 behavior: append the region only for US and CA, return the country code alone for all other countries even when a region is known. The separate country/region/timezone fields are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
340b9ba to
02d7c15
Compare
|
@metamaskbot publish-preview |
## Explanation analytics-controller is enrolled in `lint:tsc` (it has a `tsconfig.lint.json` and is listed in the root `tsconfig.lint.json`). Once it starts importing geolocation-controller and controller-utils (the geo enrichment work in MetaMask#9691), and neither of those is a `lint:tsc` project yet, tsc pulls their source straight into the analytics-controller project and fails with rootDir errors: ``` error TS6059: File '.../controller-utils/src/types.ts' is not under 'rootDir' '.../analytics-controller' error TS6307: File '.../controller-utils/src/types.ts' is not listed within the file list of project '.../analytics-controller/tsconfig.lint.json' ``` This turns the `lint:tsc` job red on MetaMask#9691 and blocks the work. This PR removes `packages/analytics-controller/tsconfig.lint.json` and its entry in the root `tsconfig.lint.json`, so analytics-controller is no longer type checked by `lint:tsc`. `yarn lint:tsc` is green with this change. This is a stopgap to unblock the current work, not the end state. The proper fix is to make controller-utils and geolocation-controller `lint:tsc` projects (give them their own `tsconfig.lint.json`) and enroll analytics-controller again. That is a bigger change because it also turns on type checking for those packages' test files, which surfaces separate pre existing type errors that need their own cleanup. ## References - Unblocks MetaMask#9691 ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2296abe. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR implements client-side geolocation enrichment for analytics events by upgrading @metamask/geolocation-controller to consume the /v2/geolocation API (returning { country, region, timezone }) and adding an opt-in enrichment path in @metamask/analytics-controller that attaches those fields under context.location for non-anonymous payloads.
Changes:
- Add
GeolocationDatasupport (v2 endpoint), new service/controller actions, and controller state fields forcountry,region, andtimezonewhile preserving the legacy location-code API viatoLocationCode. - Add optional geolocation enrichment to analytics (gated by
isGeolocationEnabled, defaultfalse) and makeAnalyticsController.initasynchronous to resolve location before replaying queued/pre-consent events. - Wire up dependency/reference graph updates (dependency, TS project references, README graph) and update package changelogs.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds @metamask/geolocation-controller dependency to the lockfile. |
| README.md | Updates monorepo dependency graph to include analytics-controller → geolocation-controller. |
| packages/geolocation-controller/src/index.ts | Exports new geolocation data types/helpers and new action types. |
| packages/geolocation-controller/src/GeolocationController.ts | Adds getGeolocationData and non-persisted state fields for country/region/timezone; switches controller to call the new service action. |
| packages/geolocation-controller/src/GeolocationController.test.ts | Updates controller tests for v2 data shape and adds coverage for new method/state behavior. |
| packages/geolocation-controller/src/GeolocationController-method-action-types.ts | Adds the GeolocationController:getGeolocationData messenger action type. |
| packages/geolocation-controller/src/geolocation-api-service/geolocation-api-service.ts | Switches to /v2/geolocation, adds parsing/validation for { country, region, timezone }, and keeps legacy location-code behavior via toLocationCode. |
| packages/geolocation-controller/src/geolocation-api-service/geolocation-api-service.test.ts | Updates service tests for v2 JSON responses and adds validation/caching/dedup coverage for new API. |
| packages/geolocation-controller/src/geolocation-api-service/geolocation-api-service-method-action-types.ts | Adds the GeolocationApiService:fetchGeolocationData action type and union update. |
| packages/geolocation-controller/CHANGELOG.md | Documents the breaking v2 geolocation data/actions/state changes. |
| packages/analytics-controller/tsconfig.json | Adds TS project reference to geolocation-controller. |
| packages/analytics-controller/tsconfig.build.json | Adds build-time TS project reference to geolocation-controller. |
| packages/analytics-controller/src/index.ts | Exports new location-context type aliases. |
| packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts | Adds AnalyticsLocationField and AnalyticsLocationContext types for context.location. |
| packages/analytics-controller/src/AnalyticsController.ts | Adds opt-in geolocation enrichment, stores resolved location once per session, and makes init() async to resolve before queue replay. |
| packages/analytics-controller/src/AnalyticsController.test.ts | Updates tests for async init and adds geolocation-enrichment behavior tests. |
| packages/analytics-controller/package.json | Adds @metamask/geolocation-controller dependency. |
| packages/analytics-controller/CHANGELOG.md | Documents breaking async init and optional enrichment behavior/dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously GeolocationApiService only cached a fetched response when the country was non-null. With the v2 response shape a response can be partially known (e.g. a valid timezone without a valid country), which was left uncached and re-fetched on every call within the TTL window. Cache whenever at least one of country/region/timezone resolved; only a fully-unknown response stays uncached so it can be retried. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Previously `init()` set an `#initialized` flag before awaiting geolocation resolution, onSetupCompleted, and queue replay, so an overlapping caller could observe a completed init while that work was still pending, and early events could miss enrichment. Cache the initialization promise instead: `init()` runs the work once (in a private `#performInit`) and returns the same in-flight (or settled) promise on subsequent/overlapping calls. `init` is now a non-async method that returns the cached `Promise<void>` directly. onSetupCompleted still runs at most once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tead of returning stale data Previously a failed lookup was swallowed and the last-known country/region/ timezone was returned, so callers could not distinguish a fresh result from a failure and might enrich with a previous session's location. getGeolocationData now rejects when the service fails (after recording the error in state), letting callers omit location on failure. The legacy getGeolocation/refreshGeolocation string APIs keep their best-effort behavior: they catch and return the last-known location code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The getGeolocationData failure-behavior JSDoc changes were not mirrored into the generated method-action-types file, failing messenger-action-types:check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
NicolasMassart
left a comment
There was a problem hiding this comment.
Looks good to me.
…taMask#9728) ## Explanation Follow-up to MetaMask#9691, which added geo-enrichment of analytics events. As originally merged, the `AnalyticsController` resolved the user's geolocation during `init` regardless of consent state — so a user's location could be requested before they had opted in to analytics (for example, during onboarding). This PR gates geolocation resolution behind user consent: - Resolution now runs **only once the user is opted in**: during `init` for an already-opted-in user, otherwise deferred to `optIn`. A user's location is never requested before they consent to analytics. - `AnalyticsController.optIn` becomes **async** (returns `Promise<void>`). It resolves and awaits geolocation before replaying queued pre-consent events, so those events are enriched with the resolved location as they replay. Anonymous payloads are excluded, per ADR-0008. - Resolution is started at most once per session (`#maybeResolveLocation` is idempotent). `init` was already async (from MetaMask#9691); `optIn` becoming async is the one new breaking change for consumers, who must now `await` it. ## References - Follow-up to MetaMask#9691 - Implements the consent-gating requirement of ADR-0008 (geo-enrichment of Extension/Mobile metric events) ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [x] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Breaking API change (`optIn` async) plus consent-sensitive geolocation and pre-consent replay timing; behavior is well-tested but integrators must await `optIn` and register GeolocationController before consent. > > **Overview** > Follow-up to geo-enrichment in **AnalyticsController**: **geolocation is no longer fetched during `init` for undecided or opted-out users**. With `isGeolocationEnabled`, resolution runs only after the user has opted in—on `init` if already opted in, otherwise on **`optIn`**—via a one-shot `#maybeResolveLocation` path. > > **`optIn` is now async** (`Promise<void>`): it awaits geolocation, then calls `#reconcilePreConsentEvents` instead of replaying the pre-consent queue immediately, so events are not sent if consent was reset while location was resolving. Queued pre-consent events get **`context.location` on replay** through `#enrichPreConsentEvent`; anonymous track payloads stay without location. > > The changelog documents the breaking **`await optIn()`** requirement for consumers. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a8c6462. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
## **Description** Enables geolocation enrichment of analytics events in the extension, wiring it up to the core changes in [MetaMask/core#9691](MetaMask/core#9691) and [MetaMask/core#9728](MetaMask/core#9728) (implements [ADR-0008](MetaMask/decisions#217)). **Why:** Extension metric events currently reach Segment without geolocation, unlike Mobile. This attaches the user's country, region, and timezone so downstream destinations (Segment, BigQuery, Mixpanel) receive the same fields on both clients. **What changed:** - Enable `isGeolocationEnabled` on `AnalyticsController`, so it resolves the user's location and attaches `country_code` / `region` / `timezone` to `context.location` on non-anonymous track, identify, and view payloads. - Delegate `GeolocationController:getGeolocationData` to the analytics controller messenger. - Delegate `GeolocationApiService:fetchGeolocationData` (the v2 action, renamed from `fetchGeolocation`) on the geolocation controller messenger. - Initialize `GeolocationApiService` + `GeolocationController` **before** `AnalyticsController` in the controller init list, since `AnalyticsController.init()` resolves geolocation during its own init via the messenger. - Update the `GeolocationController` init test fixture for the new `country` / `region` / `timezone` state fields. **Dependency / status:** This is tested against **core preview builds** (`previewBuilds` manifest in `package.json`), since the core PR is not yet released. Keep as **draft** until [MetaMask/core#9691](MetaMask/core#9691) merges and publishes, then swap the preview versions for the released package versions. ## **Changelog** CHANGELOG entry: null <!-- Analytics enrichment plumbing; no user-facing UI change. Recommend the `no-changelog` label. --> ## **Related issues** Related to: [MetaMask/core#9691](MetaMask/core#9691), [ADR-0008](MetaMask/decisions#217) Fixes: MetaMask/MetaMask-planning#7001 ## **Manual testing steps** 1. Build and load the extension with analytics enabled (opted in). 2. Trigger some events (e.g. open the wallet, view a screen). 3. Inspect the outgoing Segment payloads and confirm non-anonymous events carry `context.location` with `country_code`, `region`, and `timezone`; anonymous payloads carry no location. ## **Screenshots/Recordings** N/A — no UI changes. ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes analytics payload shape and startup ordering for opted-in users, and adds network calls to geolocation hosts; behavior is gated on analytics consent in core but touches metrics and privacy-sensitive location fields. > > **Overview** > Turns on geolocation enrichment for extension analytics by upgrading `@metamask/analytics-controller` to v2 and `@metamask/geolocation-controller` to v1, then wiring them together at init time. > > `AnalyticsController` is constructed with **`isGeolocationEnabled: true`**, and its messenger is allowed to call **`GeolocationController:getGeolocationData`**. The geolocation stack delegates **`GeolocationApiService:fetchGeolocationData`** (v2 API action rename). **`GeolocationApiService` and `GeolocationController` now initialize immediately before `AnalyticsController`** so location can be resolved during analytics `init()`. > > E2E and privacy fixtures add mocks/allowlist entries for `geolocation.api.cx.metamask.io` (and dev host), plus state snapshots for `country` / `region` / `timezone`. Lavamoat policies drop a nested geolocation `controller-utils` entry in favor of the shared package. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 071aa2b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
## **Description** Enables geolocation enrichment of analytics events in the extension, wiring it up to the core changes in [MetaMask/core#9691](MetaMask/core#9691) and [MetaMask/core#9728](MetaMask/core#9728) (implements [ADR-0008](MetaMask/decisions#217)). **Why:** Extension metric events currently reach Segment without geolocation, unlike Mobile. This attaches the user's country, region, and timezone so downstream destinations (Segment, BigQuery, Mixpanel) receive the same fields on both clients. **What changed:** - Enable `isGeolocationEnabled` on `AnalyticsController`, so it resolves the user's location and attaches `country_code` / `region` / `timezone` to `context.location` on non-anonymous track, identify, and view payloads. - Delegate `GeolocationController:getGeolocationData` to the analytics controller messenger. - Delegate `GeolocationApiService:fetchGeolocationData` (the v2 action, renamed from `fetchGeolocation`) on the geolocation controller messenger. - Initialize `GeolocationApiService` + `GeolocationController` **before** `AnalyticsController` in the controller init list, since `AnalyticsController.init()` resolves geolocation during its own init via the messenger. - Update the `GeolocationController` init test fixture for the new `country` / `region` / `timezone` state fields. **Dependency / status:** This is tested against **core preview builds** (`previewBuilds` manifest in `package.json`), since the core PR is not yet released. Keep as **draft** until [MetaMask/core#9691](MetaMask/core#9691) merges and publishes, then swap the preview versions for the released package versions. ## **Changelog** CHANGELOG entry: null <!-- Analytics enrichment plumbing; no user-facing UI change. Recommend the `no-changelog` label. --> ## **Related issues** Related to: [MetaMask/core#9691](MetaMask/core#9691), [ADR-0008](MetaMask/decisions#217) Fixes: MetaMask/MetaMask-planning#7001 ## **Manual testing steps** 1. Build and load the extension with analytics enabled (opted in). 2. Trigger some events (e.g. open the wallet, view a screen). 3. Inspect the outgoing Segment payloads and confirm non-anonymous events carry `context.location` with `country_code`, `region`, and `timezone`; anonymous payloads carry no location. ## **Screenshots/Recordings** N/A — no UI changes. ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes analytics payload shape and startup ordering for opted-in users, and adds network calls to geolocation hosts; behavior is gated on analytics consent in core but touches metrics and privacy-sensitive location fields. > > **Overview** > Turns on geolocation enrichment for extension analytics by upgrading `@metamask/analytics-controller` to v2 and `@metamask/geolocation-controller` to v1, then wiring them together at init time. > > `AnalyticsController` is constructed with **`isGeolocationEnabled: true`**, and its messenger is allowed to call **`GeolocationController:getGeolocationData`**. The geolocation stack delegates **`GeolocationApiService:fetchGeolocationData`** (v2 API action rename). **`GeolocationApiService` and `GeolocationController` now initialize immediately before `AnalyticsController`** so location can be resolved during analytics `init()`. > > E2E and privacy fixtures add mocks/allowlist entries for `geolocation.api.cx.metamask.io` (and dev host), plus state snapshots for `country` / `region` / `timezone`. Lavamoat policies drop a nested geolocation `controller-utils` entry in favor of the shared package. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 071aa2b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>

Explanation
Metric events need to carry the user's geolocation (country, region, timezone) before reaching downstream destinations, as decided in ADR-0008. Today Extension events reach Segment unenriched, and the underlying geolocation API only returned a country-level location code. This PR implements the MetaMask-native, client-side enrichment path the ADR selected.
geolocation-controllerGeolocationApiServicenow targets thev2geolocation endpoint and exposesfetchGeolocationData(), returning aGeolocationDataobject ({ country, region, timezone }) where each field is independently validated andnullwhen missing/invalid. The existingfetchGeolocation()location-code API is preserved, now derived from the data via a sharedtoLocationCode()helper. TTL caching and in-flight request deduplication are retained.GeolocationControllergainsgetGeolocationData()and three new (non-persisted) state fields:country,region,timezone.GeolocationController:getGeolocationDataandGeolocationApiService:fetchGeolocationData.analytics-controllercountry_code,region, andtimezonetocontext.locationon non-anonymous track, identify, and view payloads. Only fields the API could determine are included, and caller-providedlocationfields are preserved (resolved fields take precedence).isGeolocationEnabledconstructor option (defaultfalse), matching the existingisAnonymousEventsFeatureEnabled/isEventQueuePersistenceEnabled/isPreConsentQueueEnabledpattern. When disabled, the controller never calls the geolocation action, so compositions that don't opt in don't need to register it.init— now asynchronous (Promise<void>) — before any queued/pre-consent events are replayed, so replayed events carry the same location as new ones. Resolution is best-effort: failures are logged and events still deliver.Both packages' changelogs are updated, and the monorepo dependency graph in the README reflects the new
analytics-controller → geolocation-controlleredge.References
Checklist
🤖 Generated with Claude Code
Note
Medium Risk
Breaking async
init()and new geolocation API semantics affect all analytics consumers; opt-in enrichment touches PII-adjacent context on identified events, with documented wiring order when enabled.Overview
Geolocation moves to the
v2JSON API: newfetchGeolocationData/getGeolocationDatareturn validatedcountry,region, andtimezone(each nullable). LegacyfetchGeolocation/getGeolocationstay best-effort and derive ISO location codes viatoLocationCode(US/CA still get region suffix).getGeolocationDatarejects on failure so callers avoid stale enrichment. Controller state gainscountry,region,timezone.Analytics optionally adds
country_code,region, andtimezoneundercontext.locationon non-anonymous track, identify, and view payloads whenisGeolocationEnabledis true (default false). Location is resolved once in asyncinit()(before queue replay) viaGeolocationController:getGeolocationData; failures are logged and events still send without location. Anonymous split payloads still omit location;init()is nowPromise<void>and deduplicates concurrent calls.Adds
@metamask/geolocation-controllerto analytics-controller and documents the new dependency edge in the README.Reviewed by Cursor Bugbot for commit 06cd92e. Bugbot is set up for automated code reviews on this repo. Configure here.