Remove old Glean page hit event (Issue #13431) (#13988)

mozilla · Jan 3, 2024 · 3f83865 · 3f83865
1 parent a9aaba1
commit 3f83865
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 130 deletions.
diff --git a/glean/metrics.yaml b/glean/metrics.yaml
@@ -93,32 +93,18 @@ page:
       - [email protected]
     expires: never
   http_status:
-      type: string
-      description: |
-        The HTTP status code of the page.
-      lifetime: application
-      send_in_pings:
-        - events
-      data_sensitivity:
-        - technical
-      bugs:
-        - https://github.com/mozilla/bedrock/issues/13581
-      data_reviews:
-        - https://bugzilla.mozilla.org/show_bug.cgi?id=1848981
-      notification_emails:
-        - [email protected]
-      expires: never
-  hit:
-    type: event
+    type: string
     description: |
-      An event which is sent every time a page is loaded.
+      The HTTP status code of the page.
+    lifetime: application
+    send_in_pings:
+      - events
     data_sensitivity:
-      - web_activity
       - technical
     bugs:
-      - https://github.com/mozilla/bedrock/issues/10746
+      - https://github.com/mozilla/bedrock/issues/13581
     data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767442
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1848981
     notification_emails:
       - [email protected]
     expires: never

diff --git a/media/js/glean/init.es6.js b/media/js/glean/init.es6.js
@@ -5,22 +5,21 @@
  */
 
 import Glean from '@mozilla/glean/web';
+import GleanMetrics from '@mozilla/glean/metrics';
 import { BrowserSendBeaconUploader } from '@mozilla/glean/web';
-import { initPageView, pageEvent } from './page.es6';
+import { recordCustomPageMetrics, pageEvent } from './page.es6';
 import { clickEvent } from './elements.es6';
 import Utils from './utils.es6';
 
-const shouldInitialize = Utils.isValidHttpUrl(window.location.href);
-
 function initGlean() {
     const pageUrl = window.location.href;
     const endpoint = 'https://www.mozilla.org';
-    const channel = pageUrl.startsWith('https://www.mozilla.org/')
-        ? 'prod'
-        : 'non-prod';
+    const channel = pageUrl.startsWith(endpoint) ? 'prod' : 'non-prod';
 
-    // Ensure telemetry coming from automated testing is tagged
-    // https://mozilla.github.io/glean/book/reference/debug/sourceTags.html
+    /**
+     * Ensure telemetry coming from automated testing is tagged
+     * https://mozilla.github.io/glean/book/reference/debug/sourceTags.html
+     */
     if (pageUrl.includes('automation=true')) {
         Glean.setSourceTags(['automation']);
     }
@@ -30,31 +29,44 @@ function initGlean() {
         serverEndpoint: endpoint,
         httpClient: BrowserSendBeaconUploader // use sendBeacon since Firefox does not yet support keepalive using fetch()
     });
+
+    /**
+     * Record some additional page metrics that
+     * aren't in the default page_load event.
+     */
+    recordCustomPageMetrics();
+
+    /**
+     * Manually call Glean's default page_load event. Here
+     * we override `url` and `referrer` since we need to
+     * apply some custom logic to these values before they
+     * are sent.
+     */
+    GleanMetrics.pageLoad({
+        url: Utils.getUrl(),
+        referrer: Utils.getReferrer()
+    });
 }
 
+/**
+ * Creates global helpers on the window.Mozilla.Glean
+ * namespace, so that external JS bundles can trigger
+ * custom interaction events.
+ */
 function initHelpers() {
     if (typeof window.Mozilla === 'undefined') {
         window.Mozilla = {};
     }
 
-    // Create a global for external bundles to fire interaction pings.
     window.Mozilla.Glean = {
         pageEvent: (obj) => {
-            if (shouldInitialize) {
-                pageEvent(obj);
-            }
+            pageEvent(obj);
         },
         clickEvent: (obj) => {
-            if (shouldInitialize) {
-                clickEvent(obj);
-            }
+            clickEvent(obj);
         }
     };
 }
 
-if (shouldInitialize) {
-    initGlean();
-    initPageView();
-}
-
+initGlean();
 initHelpers();
diff --git a/media/js/glean/page.es6.js b/media/js/glean/page.es6.js
@@ -4,7 +4,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-import GleanMetrics from '@mozilla/glean/metrics';
 import * as page from '../libs/glean/page.js';
 import Utils from './utils.es6';
 
@@ -21,7 +20,7 @@ const defaultParams = {
     xv: '' // short param for 'experience version'.
 };
 
-function initPageView() {
+function recordCustomPageMetrics() {
     page.viewed.set();
     page.path.set(Utils.getPathFromUrl());
     page.locale.set(Utils.getLocaleFromUrl());
@@ -48,24 +47,6 @@ function initPageView() {
             page.queryParams[param].set(finalParams[param]);
         }
     }
-
-    /**
-     * Manually call Glean's default page_load event. Here
-     * we override `url` and `referrer` since we need to
-     * apply some custom logic to these values before they
-     * are sent.
-     */
-    GleanMetrics.pageLoad({
-        url: Utils.getUrl(),
-        referrer: Utils.getReferrer()
-    });
-
-    /**
-     * This old page hit event can be removed once we're
-     * confident that the new page_load event is working
-     * as expected.
-     */
-    page.hit.record();
 }
 
 function pageEvent(obj) {
@@ -96,4 +77,4 @@ function pageEvent(obj) {
     }
 }
 
-export { initPageView, pageEvent };
+export { recordCustomPageMetrics, pageEvent };
diff --git a/media/js/glean/utils.es6.js b/media/js/glean/utils.es6.js
@@ -5,32 +5,40 @@
  */
 
 const Utils = {
-    getUrl: (str) => {
-        const urlString = typeof str === 'string' ? str : window.location.href;
-        const url = new URL(urlString);
-
-        // Ensure we don't include tokens in newsletter page load event pings
-        // Issue https://github.com/mozilla/bedrock/issues/13583
-        const newsletterPaths = [
-            '/newsletter/existing/',
-            '/newsletter/country/'
-        ];
-
-        newsletterPaths.forEach((path) => {
-            // Find the index of the newsletter pathname
-            const index = url.pathname.indexOf(path);
-
-            // Remove everything after the pathname (which is the token)
-            if (index !== -1) {
-                const newPathname = url.pathname.substring(
-                    0,
-                    index + path.length
-                );
-                url.pathname = newPathname;
-            }
-        });
+    filterNewsletterURL: (str) => {
+        try {
+            const url = new URL(str);
+
+            // Ensure we don't include tokens in newsletter page load event pings
+            // Issue https://github.com/mozilla/bedrock/issues/13583
+            const newsletterPaths = [
+                '/newsletter/existing/',
+                '/newsletter/country/'
+            ];
+
+            newsletterPaths.forEach((path) => {
+                // Find the index of the newsletter pathname
+                const index = url.pathname.indexOf(path);
+
+                // Remove everything after the pathname (which is the token)
+                if (index !== -1) {
+                    const newPathname = url.pathname.substring(
+                        0,
+                        index + path.length
+                    );
+                    url.pathname = newPathname;
+                }
+            });
+
+            return url.toString();
+        } catch (e) {
+            return str;
+        }
+    },
 
-        return url.toString();
+    getUrl: (str) => {
+        const url = typeof str === 'string' ? str : window.location.href;
+        return Utils.filterNewsletterURL(url);
     },
 
     getPathFromUrl: (str) => {
@@ -73,12 +81,13 @@ const Utils = {
 
     getReferrer: (str) => {
         const referrer = typeof str === 'string' ? str : document.referrer;
+        const url = Utils.filterNewsletterURL(referrer);
 
         if (typeof window.Mozilla.Analytics !== 'undefined') {
-            return Mozilla.Analytics.getReferrer(referrer);
+            return Mozilla.Analytics.getReferrer(url);
         }
 
-        return referrer;
+        return url;
     },
 
     getHttpStatus: () => {
@@ -88,18 +97,6 @@ const Utils = {
         return pageId && pageId === '404' ? '404' : '200';
     },
 
-    isValidHttpUrl: (str) => {
-        let url;
-
-        try {
-            url = new URL(str);
-        } catch (e) {
-            return false;
-        }
-
-        return url.protocol === 'http:' || url.protocol === 'https:';
-    },
-
     isTelemetryEnabled: () => {
         if (
             typeof Mozilla.Cookies !== 'undefined' &&

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -7,7 +7,7 @@
     "@babel/core": "^7.23.6",
     "@babel/preset-env": "^7.23.6",
     "@mozilla-protocol/core": "^18.0.0",
-    "@mozilla/glean": "^4.0.0-pre.2",
+    "@mozilla/glean": "^4.0.0-pre.3",
     "@mozmeao/cookie-helper": "^1.1.0",
     "@mozmeao/dnt-helper": "^1.0.0",
     "@mozmeao/trafficcop": "^2.0.1",

diff --git a/tests/unit/spec/glean/page.js b/tests/unit/spec/glean/page.js
@@ -11,7 +11,10 @@
 
 import * as page from '../../../../media/js/libs/glean/page.js';
 import Utils from '../../../../media/js/glean/utils.es6';
-import { initPageView, pageEvent } from '../../../../media/js/glean/page.es6';
+import {
+    recordCustomPageMetrics,
+    pageEvent
+} from '../../../../media/js/glean/page.es6';
 import { testResetGlean } from '@mozilla/glean/testing';
 
 describe('page.js', function () {
@@ -30,7 +33,7 @@ describe('page.js', function () {
     });
 
     it('should register a page view correctly', async function () {
-        initPageView();
+        recordCustomPageMetrics();
 
         const path = await page.path.testGetValue();
         expect(path).toEqual('/firefox/new/');
@@ -52,7 +55,7 @@ describe('page.js', function () {
             new window._SearchParams(query)
         );
 
-        initPageView();
+        recordCustomPageMetrics();
 
         const source = await page.queryParams['utm_source'].testGetValue();
         expect(source).toEqual('test-source');
@@ -94,7 +97,7 @@ describe('page.js', function () {
             new window._SearchParams(query)
         );
 
-        initPageView();
+        recordCustomPageMetrics();
 
         const unspecifiedParam =
             await page.queryParams['unspecified_param'].testGetValue();
@@ -110,7 +113,7 @@ describe('page.js', function () {
             new window._SearchParams(query)
         );
 
-        initPageView();
+        recordCustomPageMetrics();
 
         const source = await page.queryParams['utm_source'].testGetValue();
         expect(source).toEqual('%');
@@ -126,7 +129,7 @@ describe('page.js', function () {
             new window._SearchParams(query)
         );
 
-        initPageView();
+        recordCustomPageMetrics();
 
         const source = await page.queryParams['utm_source'].testGetValue();
         expect(source).toEqual('');