Skip to content

Conversation

@david0xd
Copy link
Contributor

@david0xd david0xd commented Mar 31, 2025

This PR moves MetaMetrics tracking of Snap export usage to Snap Controller.

Related issue: #3259

Extension integration PR: MetaMask/metamask-extension#31553
Mobile integration PR: MetaMask/metamask-mobile#14428

@david0xd david0xd self-assigned this Mar 31, 2025
@codecov
Copy link

codecov bot commented Apr 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.06%. Comparing base (37e79af) to head (6848f81).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3281   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files         397      397           
  Lines       10898    10929   +31     
  Branches     1714     1720    +6     
=======================================
+ Hits        10687    10718   +31     
  Misses        211      211           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@david0xd david0xd marked this pull request as ready for review April 8, 2025 10:17
@david0xd david0xd requested a review from a team as a code owner April 8, 2025 10:17
@david0xd david0xd changed the title Add Snap export handler usage tracking feat: Add Snap export handler usage tracking Apr 16, 2025
FrederikBolding
FrederikBolding previously approved these changes May 2, 2025
@david0xd david0xd force-pushed the dd/add-handler-tracking branch from 035e985 to 943ff00 Compare May 5, 2025 10:32
david0xd added 2 commits May 5, 2025 12:49
Use registry for metadata

Replace controller with hook approach

Fix broken unit tests

Add unit tests

Fix coverage after rebase

Remove category

Remove async from registry metadata and tracking

Update throttling logic and tests

Rearrange unit tests

Update call to get registry metadata

Remove unused getRegistryMetadata

Refactor tracking call

Add logic optimization for throttling function

Skip preinstalled Snaps tracking

Add error handling for MetaMetrics hook

Add unit test for preinstalled snap
@david0xd david0xd added this pull request to the merge queue May 5, 2025
Merged via the queue into main with commit 87528de May 5, 2025
115 checks passed
@david0xd david0xd deleted the dd/add-handler-tracking branch May 5, 2025 11:09
Mrtenz pushed a commit to MetaMask/metamask-mobile that referenced this pull request May 13, 2025
…14428)

## **Description**
This PR adds MetaMetrics tracking hook to Snap controller init. This way
we delegate Snap export tracking to Snap controller.

## **Related issues**
Related PR:
MetaMask/snaps#3281
MetaMask/metamask-extension#31553 

## **Notes**
- Requires Snaps release.
- Snaps PR: MetaMask/snaps#3281
- Targets Snaps release when ready.

## **Manual testing steps**
1. TBD

## **Screenshots/Recordings**
### **Before**
n/a

### **After**
n/a

## **Pre-merge author checklist**
- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/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.
Mrtenz pushed a commit to MetaMask/metamask-mobile that referenced this pull request May 13, 2025
…14428)

## **Description**
This PR adds MetaMetrics tracking hook to Snap controller init. This way
we delegate Snap export tracking to Snap controller.

## **Related issues**
Related PR:
MetaMask/snaps#3281
MetaMask/metamask-extension#31553 

## **Notes**
- Requires Snaps release.
- Snaps PR: MetaMask/snaps#3281
- Targets Snaps release when ready.

## **Manual testing steps**
1. TBD

## **Screenshots/Recordings**
### **Before**
n/a

### **After**
n/a

## **Pre-merge author checklist**
- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/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.
Mrtenz pushed a commit to MetaMask/metamask-extension that referenced this pull request May 13, 2025
## **Description**
This PR is the next step in moving Snap export tracking away from the
extension and transferring its functionality and responsibility to Snap
Controller.

## **Notes**
- Requires Snaps release.
- Snaps PR: MetaMask/snaps#3281
- Targets Snaps release when ready.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31553?quickstart=1)

## **Related issues**
Fixes: MetaMask/snaps#3259

## **Manual testing steps**
1. Find a way to verify that events are tracked (test locally, add
console log, etc.)

## **Screenshots/Recordings**
### **Before**
n/a

### **After**
n/a

## **Pre-merge author checklist**
- [ ] 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).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants