From ba1bee12973681af61c61ae3968e75aafde4e6f9 Mon Sep 17 00:00:00 2001
From: abretonc7s <107169956+abretonc7s@users.noreply.github.com>
Date: Tue, 17 Dec 2024 22:30:48 +0800
Subject: [PATCH] fix(analytics): improve dapp details tracking and SDK RPC
 request analytics (#1179)

* 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.

* refactor: improve analytics event handling and caching logic

- 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.
---
 .../src/analytics-api.ts                      | 33 ++++++++++++++-----
 .../write.ts                                  | 23 +++++++++++++
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/packages/sdk-socket-server-next/src/analytics-api.ts b/packages/sdk-socket-server-next/src/analytics-api.ts
index 95894c716..f53a155cb 100644
--- a/packages/sdk-socket-server-next/src/analytics-api.ts
+++ b/packages/sdk-socket-server-next/src/analytics-api.ts
@@ -28,9 +28,6 @@ import {
 
 const logger = getLogger();
 
-// SDK version prev 0.27.0 uses 'sdk' as the default id, below value is the sha1 hash of 'sdk'
-const SDK_EXTENSION_DEFAULT_ID = '5a374dcd2e5eb762b527af3a5bab6072a4d24493';
-
 // Initialize Redis Cluster client
 let redisNodes: {
   host: string;
@@ -246,7 +243,7 @@ app.post('/evt', evtMetricsMiddleware, async (_req, res) => {
       return res.status(400).json({ error: 'wrong event name' });
     }
 
-    let channelId: string = body.id || 'sdk';
+    const channelId: string = body.id || 'sdk';
     // Prevent caching of events coming from extension since they are not re-using the same id and prevent increasing redis queue size.
     let isExtensionEvent = body.from === 'extension';
 
@@ -272,7 +269,10 @@ app.post('/evt', evtMetricsMiddleware, async (_req, res) => {
       ? crypto.createHash('sha1').update(channelId).digest('hex')
       : await pubClient.get(channelId);
 
-    incrementRedisCacheOperation('analytics-get-channel-id', !!userIdHash);
+    incrementRedisCacheOperation(
+      'analytics-get-channel-id',
+      Boolean(userIdHash),
+    );
 
     if (!userIdHash) {
       userIdHash = crypto.createHash('sha1').update(channelId).digest('hex');
@@ -294,23 +294,40 @@ app.post('/evt', evtMetricsMiddleware, async (_req, res) => {
       await inspectRedis(channelId);
     }
 
-    let channelInfo: ChannelInfo | null;
+    let channelInfo: ChannelInfo | null = null;
     const cachedChannelInfo = isAnonUser
       ? null
       : await pubClient.get(userIdHash);
 
+    // Potential issue because we may have a cached channel info but the url inside may be empty and dappId "N/A".
+    // We then need to actually fully parse the string and extract the channelInfo object to validate the url and dappId.
+    const hasCachedChannelInfo = Boolean(cachedChannelInfo);
+
     incrementRedisCacheOperation(
       'analytics-get-channel-info',
-      !!cachedChannelInfo,
+      hasCachedChannelInfo,
     );
 
+    let hasValidCachedChannelInfo = false;
+
     if (cachedChannelInfo) {
       logger.debug(
         `Found cached channel info for ${userIdHash}`,
         cachedChannelInfo,
       );
       channelInfo = JSON.parse(cachedChannelInfo);
-    } else {
+      hasValidCachedChannelInfo =
+        channelInfo !== null &&
+        Boolean(channelInfo.url && channelInfo.url.length > 0);
+
+      if (!hasValidCachedChannelInfo) {
+        logger.warn(
+          `event: ${body.event} channelId: ${channelId}  - empty cached channel info for ${userIdHash} dAppId=${channelInfo?.dappId}`,
+        );
+      }
+    }
+
+    if (!hasValidCachedChannelInfo) {
       logger.info(
         `event: ${body.event} channelId: ${channelId}  - No cached channel info found for ${userIdHash}`,
       );
diff --git a/packages/sdk/src/services/RemoteCommunicationPostMessageStream/write.ts b/packages/sdk/src/services/RemoteCommunicationPostMessageStream/write.ts
index 2af638151..3ca937f87 100644
--- a/packages/sdk/src/services/RemoteCommunicationPostMessageStream/write.ts
+++ b/packages/sdk/src/services/RemoteCommunicationPostMessageStream/write.ts
@@ -1,3 +1,8 @@
+import {
+  TrackingEvents,
+  SendAnalytics,
+  DEFAULT_SERVER_URL,
+} from '@metamask/sdk-communication-layer';
 import { RemoteCommunicationPostMessageStream } from '../../PostMessageStream/RemoteCommunicationPostMessageStream';
 import { METHODS_TO_REDIRECT, RPC_METHODS } from '../../config';
 import {
@@ -67,6 +72,24 @@ export async function write(
         .catch((err: unknown) => {
           logger(`[RCPMS: _write()] error sending message`, err);
         });
+    } else {
+      try {
+        // Only send analytics if we are not sending via network.
+        await SendAnalytics(
+          {
+            id: channelId,
+            event: TrackingEvents.SDK_RPC_REQUEST,
+            params: {
+              method: targetMethod,
+              from: 'mobile',
+            },
+          },
+          instance.state.remote?.state.communicationServerUrl ??
+            DEFAULT_SERVER_URL,
+        );
+      } catch (error) {
+        logger(`[RCPMS: _write()] error sending analytics`, error);
+      }
     }
 
     if (!isSecure) {