feat(analytics-controller): add optional event fragments for journey-scoped properties - #10055
Open
gauthierpetetin wants to merge 11 commits into
Open
feat(analytics-controller): add optional event fragments for journey-scoped properties#10055gauthierpetetin wants to merge 11 commits into
gauthierpetetin wants to merge 11 commits into
Conversation
…scoped properties Clients currently re-derive the same analytics properties for every event in a signature or transaction flow. Fragments let those properties accumulate once, with consent, persistence, and emission going through the existing trackEvent path. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Current version of PR was reviewed by /review-bugbot on Sep 1, 19:46 GMT+2. It flagged 0 findings. Bugbot on commit |
There was a problem hiding this comment.
Pull request overview
Adds an optional “event fragments” feature to @metamask/analytics-controller, allowing clients (when opted-in via isEventFragmentsEnabled) to persist and update journey-scoped analytics property bags and optionally emit initial/success/failure events through the existing trackEvent pipeline and consent gating.
Changes:
- Introduces
AnalyticsEventFragment*types and exports them from the package public API. - Extends
AnalyticsControllerstate, metadata, messenger methods, and implementation to support fragment lifecycle (create/upsert/update/get/delete/finalize) with consent-aware reconciliation oninit. - Adds selectors + comprehensive tests, and documents the feature in the README/changelog.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/analytics-controller/src/selectors.ts | Adds selectors for reading all fragments and a fragment by ID. |
| packages/analytics-controller/src/selectors.test.ts | Adds unit tests for the new selectors. |
| packages/analytics-controller/src/index.ts | Exposes event fragment types and new controller method action types from the public entrypoint. |
| packages/analytics-controller/src/EventFragment.types.ts | Defines the event fragment data model and option/payload types. |
| packages/analytics-controller/src/AnalyticsController.ts | Implements fragment storage, reconciliation on init, consent gating integration, and lifecycle methods. |
| packages/analytics-controller/src/AnalyticsController.test.ts | Adds extensive tests covering fragment behavior, consent gating, persistence, and messenger exposure. |
| packages/analytics-controller/src/AnalyticsController-method-action-types.ts | Adds messenger action type definitions for fragment lifecycle methods. |
| packages/analytics-controller/README.md | Documents the new feature and its usage patterns. |
| packages/analytics-controller/CHANGELOG.md | Adds an Unreleased entry describing the new feature. |
Suppressed comments (3)
packages/analytics-controller/src/AnalyticsController.ts:541
mergeEventFragmentContextreturns{ ...base, ...override }, butbase/overrideareAnalyticsContext | undefined. Coalesce to{}before spreading to avoid TypeScript errors and make the intent explicit.
return { ...base, ...override };
packages/analytics-controller/src/AnalyticsController-method-action-types.ts:101
- The
getEventFragmentByIdaction-type JSDoc omits the consent gate, but the handler returnsundefinedwhen capture is not allowed (opted out, or undecided without pre-consent queue). Align the docs with the implementation to avoid consumer confusion.
* @returns The fragment, or `undefined` when no fragment has that ID or the
* event fragments feature is disabled.
*/
packages/analytics-controller/src/AnalyticsController.ts:1629
finalizeEventFragmentis a logged no-op when the feature is disabled or consent gating disallows capture, so the@throwsdoc should note that a missing fragment will not throw in those ignored-call states.
* @throws Error if no fragment has that ID.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ngelog to PR Regenerate AnalyticsController method action types after JSDoc updates and add the PR link required by the changelog CI check. Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Keep a single event fragments bullet with the PR link and remove nested detail bullets from the Unreleased section. Co-authored-by: Cursor <cursoragent@cursor.com>
Prevent accidental mutation of the shared empty record returned by selectEventFragments when state has no fragments yet. Co-authored-by: Cursor <cursoragent@cursor.com>
…preading Default omitted properties, sensitiveProperties, and context to empty objects so fragment merge and create paths stay type-safe. Co-authored-by: Cursor <cursoragent@cursor.com>
Document that updateEventFragment and finalizeEventFragment only throw for a missing fragment when the call is not ignored by consent or feature flags. Co-authored-by: Cursor <cursoragent@cursor.com>
Snapshot fragment IDs before awaited init work so reconciliation drops only stale non-persistent leftovers, not in-flight journeys started while init runs. Co-authored-by: Cursor <cursoragent@cursor.com>
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 1b12a93. Configure here.
Snapshot fragment createdAt at init start so purge can distinguish stale leftovers from fragments replaced via createEventFragment while init runs. Co-authored-by: Cursor <cursoragent@cursor.com>
Apply Prettier formatting for lint:misc:check and add tests covering fragment context preservation and AnalyticsPlatformAdapterSetupError. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
NicolasMassart
left a comment
NicolasMassart
left a comment
Contributor
There was a problem hiding this comment.
suggestions for some changes on fragments
7 tasks
Drop persisted fragments whose lastUpdated is older than 24 hours on init so abandoned journeys cannot keep properties in storage indefinitely. Co-authored-by: Cursor <cursoragent@cursor.com>
Expose ReadonlyAnalyticsEventFragment from create and get so callers cannot mutate controller state without going through update or upsert. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Clients currently re-derive the same analytics properties for every event in a signature or transaction confirmation. That duplication is easy to get wrong and hard to keep consistent as more UI pieces contribute to the same journey.
This adds optional event fragments to
AnalyticsController, disabled by default viaisEventFragmentsEnabled. A fragment is a persisted bag ofpropertiesandsensitivePropertiesthat any part of a client can write to while a journey is in progress:initialEvent,successEvent, andfailureEvent. Creation can emit the initial event, andfinalizeEventFragmentemits success or failure.getEventFragmentByIdwhen it emits its own event.Emission goes through
trackEvent, so consent gating, anonymous event splitting, the pre-consent queue, and geolocation enrichment all apply. Accumulation uses that same consent gate: fragments are only stored while the user is opted in, or while they are undecided and the pre-consent queue is enabled.eventFragmentsis persisted but excluded from state logs, debug snapshots, and UI, matching the other analytics queues.This is not a breaking change. Clients must opt in with the constructor flag.
Here's a draft PR on Extension repo where this code can be tested: MetaMask/metamask-extension#45961
References
Contributes to: https://github.com/MetaMask/MetaMask-planning/issues/7582
Checklist
Made with Cursor