Track page views in Google Analytics #694

vyorkin · 2025-01-16T10:27:09Z

No description provided.

github-actions · 2025-01-16T10:28:26Z

📦 build.zip [updated at Feb 18, 3:21:12 PM UTC]

zerts · 2025-01-16T17:19:31Z

src/shared/analytics/analytics.background.ts

@@ -348,7 +356,13 @@ export function initialize({ account }: { account: Account }) {
    willSendRequest: createAddProviderHook({ getWalletProvider }),
  });
  const handleUserId = () => mixpanelIdentify(account);
-  account.on('authenticated', () => handleUserId());
+
+  account.on('authenticated', () =>


I think, we can create a separate .on call for GA.
No need to combine them and probably no need to use Promise.allSettled()

Agree, separate handler is better ✅

zerts · 2025-01-16T17:20:48Z

.env.example

@@ -11,3 +11,5 @@ FEATURE_FOOTER_BUG_BUTTON=on
 FEATURE_SEND_FORM=
 MIXPANEL_TOKEN_PUBLIC=
 FEATURE_LOYALTY_FLOW=off
+GOOGLE_ANALYTICS_MEASUREMENT_ID=
+GOOGLE_ANALYTICS_API_SECRET=


This api key will be exposed. Probably it is ok, but everything that has secret in its name raises a concern

👍 Agree, it shouldn't cause much harm, but I'd prefer not to expose it

@vyorkin Not sure what you mean, do you plan to remove it?

Network traffic inspection could indeed reveal these GA API tokens... Could be solved by creating our custom endpoint, just like we did for access to firebase config. I'm just not sure it's worth it at this point

zerts · 2025-01-16T17:25:04Z

src/shared/analytics/google-analytics.ts

+const ENDPOINT = 'https://www.google-analytics.com/mp/collect';
+const DEFAULT_ENGAGEMENT_TIME_IN_MSEC = 100;
+
+function readClientid() {


this should be an async function.

updated ✅

zerts · 2025-01-16T17:25:46Z

src/shared/analytics/google-analytics.ts

+  if (!clientId) {
+    // Generate a unique client ID for Google Analytics V4.
+    // The actual value is not relevant.
+    clientId = crypto.randomUUID();


can't we reuse userId that was generated for mixpanel here?

Ah, I see. Ok then

we have a device id, why not reuse it?

wow, I missed it, thanks, this is exactly what I needed

zerts · 2025-01-16T17:26:26Z

src/shared/analytics/google-analytics.ts

+
+export async function resetGoogleAnalyticsSessionId() {
+  const sessionId = crypto.randomUUID() as string;
+  await chrome.storage.session.set({


Isn't this also await browserStorage.set(CLIENT_ID_KEY, clientId);?

local and session storage are different (according to docs). Probably we could use browserStorage (local) here as well, but the google analytics v4 docs suggests using session storage. I think that is because the session storage is cleared if the extension is disabled, reloaded or updated and when the browser restarts

zerts · 2025-01-16T17:27:00Z

src/shared/analytics/google-analytics.ts

+async function getSessionId() {
+  // A sessionId represents a period of continuous user interaction with the extension.
+  // Tracking it is necessary for Google Analytics V4.
+  const result = await browser.storage.session.get(SESSION_ID_KEY);


Isn't this readClientid()?

Ok, I see the difference, shouldn't we use browserStorage.get<string>(SESSION_ID_KEY) here?

#694 (comment)

If session storage is not written to disk, why use it? Why not just store this in a map?

I changed the implementation, so it is not relevant anymore

zerts · 2025-01-16T17:28:42Z

src/shared/analytics/google-analytics.ts

+  // A sessionId represents a period of continuous user interaction with the extension.
+  // Tracking it is necessary for Google Analytics V4.
+  const result = await browser.storage.session.get(SESSION_ID_KEY);
+  let sessionId = result?.[SESSION_ID_KEY] as string | undefined;


Is it correct that we get result by SESSION_ID_KEY and then reading [SESSION_ID_KEY] in result?

yes, we do the same thing in the webapis/storage.ts

not relevant anymore

zerts · 2025-01-16T17:29:36Z

src/shared/analytics/google-analytics.ts

+  if (event === 'page_view')
+    return {
+      page_title: document.title,
+      page_location: document.location.href,


do we need .href or .hash here?

google analytics docs suggests using .href

zerts · 2025-01-16T17:33:29Z

src/shared/analytics/google-analytics.ts

+  if (process.env.NODE_ENV !== 'development') {
+    onIdle(() => {
+      fetch(
+        `${ENDPOINT}?measurement_id=${GOOGLE_ANALYTICS_MEASUREMENT_ID}&api_secret=${GOOGLE_ANALYTICS_API_SECRET}`,


May be it will be a bit more readable to use URL() constructor here?

not relevant anymore

zerts · 2025-01-16T17:34:06Z

src/shared/analytics/google-analytics.ts

+
+export async function sendToGoogleAnalytics(
+  event: GoogleAnalyticsEvent,
+  extraParams: Record<string, unknown>


Do we need this extra params?

Not yet, but we may need them if we decide to send additional events to GA

zerts · 2025-01-16T17:34:38Z

src/shared/analytics/google-analytics.ts

+
+  if (process.env.NODE_ENV !== 'development') {
+    onIdle(() => {
+      fetch(


What about using ky here?

updated ✅

everdimension · 2025-02-12T09:32:09Z

src/shared/analytics/analytics.background.ts

+    // Google Analytics does not have a clear definition of a user session.
+    // Hence, we need to define what a user session means within the extension.
+    // For simplicity, we initiate a new session each time the user authenticates.
+    Promise.allSettled([handleUserId(), resetGoogleAnalyticsSessionId()])


I don't get it, what does .allSettled achieve here? It's not being awaited nor caught

Thats odd, doesn't make sense to me either... Let me update this part

zerts · 2025-02-18T12:34:15Z

src/shared/analytics/google-analytics.ts

+  if (!userId) {
+    return;
+  }
+  googleAnalyticsApi.setSessionId(userId);


Do we need to store this sessionId if it is a userId and we have it on every event anyway?

I agree, while in mixpanel client it makes sense to store the user_id as a field within the API client class, GA api client is a bit simpler: we only have a single collect method that requires session_id, so we could just pass user_id as a parameter each time it’s called

updated ✅

zerts · 2025-02-18T12:37:35Z

src/shared/analytics/analytics.background.ts

@@ -101,6 +102,13 @@ function trackAppEvents({ account }: { account: Account }) {
    mixpanelTrack(account, 'General: Screen Viewed', mixpanelParams);
  });

+  emitter.on('screenView', async (data) => {
+    gaCollect(account, 'page_view', {
+      page_title: data.title,


Why do we need page title here?

If I remember correctly, this is from GA docs, let me check

Yes, https://developer.chrome.com/docs/extensions/how-to/integrate/google-analytics-4

The page_view event also requires the page_title and page_location parameters

zerts · 2025-02-23T16:55:42Z

src/shared/analytics/analytics.background.ts

@@ -177,6 +185,7 @@ function trackAppEvents({ account }: { account: Account }) {
        ...addressActionAnalytics,
      });
      sendToMetabase('signed_transaction', params);
+      gaCollect(account, 'signed_transaction', params);


may be we can introduce a helper prepareGaParams(account, params) that will get a userId from account and add it to params. To make google-analytics service independent from Account class implementation. Not a big deal though.

zerts · 2025-02-23T16:56:24Z

src/shared/analytics/analytics.background.ts

@@ -348,12 +358,11 @@ export function initialize({ account }: { account: Account }) {
    willSendRequest: createAddProviderHook({ getWalletProvider }),
  });
  const handleUserId = () => mixpanelIdentify(account);
+


I think, we can revert changes here and in the account.on('reset') listener :)
Not a big deal, though

zerts · 2025-02-23T16:57:47Z

src/shared/analytics/google-analytics.ts

+  sendRequestsOverTheNetwork: process.env.NODE_ENV === 'production',
+});
+
+Object.assign(globalThis, { googleAnalyticsApi });


Do we need this?

vyorkin requested review from everdimension and zerts January 16, 2025 10:27

vyorkin self-assigned this Jan 16, 2025

zerts reviewed Jan 16, 2025

View reviewed changes

vyorkin marked this pull request as draft January 20, 2025 16:30

vyorkin removed the request for review from everdimension January 20, 2025 16:30

vyorkin added 3 commits January 26, 2025 14:53

Track page views in Google Analytics

0bc70ed

Refactoring

bdf0a26

Refactoring

b85827b

vyorkin force-pushed the feature/add-conversion-tracking-WLT-6076 branch from 96636f8 to b85827b Compare January 26, 2025 07:54

everdimension reviewed Feb 12, 2025

View reviewed changes

Rewrite event tracking for GA, refactor

da8d93a

vyorkin marked this pull request as ready for review February 15, 2025 12:17

vyorkin requested review from everdimension and zerts February 15, 2025 12:17

zerts reviewed Feb 18, 2025

View reviewed changes

Refactoring: remove sessionId field from GoogleAnalyticsApi

11c321b

vyorkin requested a review from zerts February 20, 2025 07:35

zerts reviewed Feb 23, 2025

View reviewed changes

zerts approved these changes Feb 23, 2025

View reviewed changes

Track page views in Google Analytics #694

Are you sure you want to change the base?

Track page views in Google Analytics #694

Conversation

vyorkin commented Jan 16, 2025

github-actions bot commented Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vyorkin Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zerts Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 16, 2025 •

edited

Loading

vyorkin Jan 17, 2025 •

edited

Loading

zerts Jan 16, 2025 •

edited

Loading