Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(analytics): improve dapp details tracking and SDK RPC request analytics #1179

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

abretonc7s
Copy link
Collaborator

@abretonc7s abretonc7s commented Dec 17, 2024

Description

This PR addresses two critical analytics-related issues that were causing missing dapp details and incomplete event tracking:

Issue 1: Missing SDK RPC Request Analytics

When using the deeplink protocol, the SDK would directly open the deeplink without going through the network path, causing analytics events to be missed. This PR ensures analytics events are properly tracked regardless of the communication method used.

Changes:

  • Added analytics tracking for SDK RPC requests when not using network communication
  • Implemented SendAnalytics function to log SDK_RPC_REQUEST events with method and source information
  • Maintained existing network path analytics while adding coverage for direct deeplink interactions

Issue 2: Invalid Cached Dapp Details

The system was not properly validating cached dapp information in Redis, leading to events being sent without proper dapp details.

Changes:

  • Enhanced validation of cached channel information
  • Added explicit checks for URL and dappId presence in cached data
  • Improved logging for cases with empty or invalid cached channel info
  • Refactored cache operation tracking for better monitoring
  • Removed deprecated SDK version ID constant

Technical Implementation:

  • Added validation logic to verify cached channel info contains valid URL
  • Improved error logging for debugging cache-related issues
  • Refactored Redis cache operation tracking for better clarity
  • Enhanced type safety with proper null checks and boolean operations

Breaking Changes

None. This PR maintains backward compatibility while improving analytics tracking reliability.

Testing Instructions

  1. Test SDK RPC Request Analytics:

    • Connect to a dapp using deeplink protocol
    • Verify analytics events are sent for RPC requests
    • Check that method and source information are correctly captured
  2. Test Dapp Details Caching:

    • Connect to a dapp and verify proper caching of dapp details
    • Refresh the dapp and confirm cached details are properly validated
    • Verify analytics events contain complete dapp information

Related Issues

  • Fixes SDK-199: Missing dapp details in analytics

- Implemented analytics tracking for SDK RPC requests when not sending via network.
- Integrated SendAnalytics function to log SDK_RPC_REQUEST events with relevant parameters.
- Removed deprecated SDK version ID and updated channelId declaration to use const.
- Enhanced caching logic for channel information, ensuring validation of cached data.
- Added logging for cases with empty cached channel info to improve debugging.
- Refactored incrementRedisCacheOperation calls for better readability.
@abretonc7s abretonc7s changed the title feat: add analytics tracking for SDK RPC requests - Implemented analytics tracking for SDK RPC requests when not sending via network. - Integrated SendAnalytics function to log SDK_RPC_REQUEST events with relevant parameters. fix(analytics): improve dapp details tracking and SDK RPC request analytics Dec 17, 2024
@abretonc7s abretonc7s marked this pull request as ready for review December 17, 2024 13:57
@abretonc7s abretonc7s requested a review from a team as a code owner December 17, 2024 13:57
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

Attention: Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.

Project coverage is 74.03%. Comparing base (86052b0) to head (4babf9f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ices/RemoteCommunicationPostMessageStream/write.ts 14.28% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1179      +/-   ##
==========================================
- Coverage   74.12%   74.03%   -0.10%     
==========================================
  Files         181      181              
  Lines        4298     4305       +7     
  Branches     1053     1056       +3     
==========================================
+ Hits         3186     3187       +1     
- Misses       1112     1118       +6     

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

Copy link

@abretonc7s abretonc7s merged commit ba1bee1 into main Dec 17, 2024
34 of 36 checks passed
@abretonc7s abretonc7s deleted the fixanalytics branch December 17, 2024 14:30
@christopherferreira9 christopherferreira9 mentioned this pull request Dec 17, 2024
3 tasks
@GIgako19929
Copy link

This pull request includes several changes to the analytics-api.ts and write.ts files in the SDK. The changes primarily focus on code cleanup, type consistency, and adding analytics tracking. Below are the most important changes:

Code Cleanup and Type Consistency:

  • Removed the unused constant SDK_EXTENSION_DEFAULT_ID from analytics-api.ts.
  • Changed the channelId variable from let to const for better immutability in analytics-api.ts.
  • Added explicit initialization of channelInfo to null for clarity in analytics-api.ts.

Analytics Tracking:

  • Added import statements for TrackingEvents, SendAnalytics, and DEFAULT_SERVER_URL from @metamask/sdk-communication-layer in write.ts.
  • Implemented a new block to send analytics data when not sending via network in write.ts. This includes tracking SDK RPC requests and handling potential errors.

Code Refactoring:

  • Refactored the incrementRedisCacheOperation call to use Boolean(userIdHash) for better readability in analytics-api.ts.
  • Added additional logging and validation for cached channel information to handle potential issues with empty URLs and dApp IDs in analytics-api.ts.

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.

3 participants