Remove superfluous Glean metrics (#14690)

* Remove superfluous Glean metrics * Update tests/unit/spec/glean/utils.js Co-authored-by: Jan Brasna <1784648+janbrasna@users.noreply.github.com> * Update tests/unit/spec/glean/utils.js Co-authored-by: Jan Brasna <1784648+janbrasna@users.noreply.github.com> --------- Co-authored-by: Jan Brasna <1784648+janbrasna@users.noreply.github.com>
2024-06-19 04:22:38 +01:00 · 2024-06-19 04:22:38 +01:00 · ea5249275f
--- a/bedrock/base/templates/404.html
+++ b/bedrock/base/templates/404.html
@ -8,8 +8,6 @@

 {% block gtm_page_id %}data-gtm-page-id="404"{% endblock %}

-{% block html_attrs %}data-http-status="404"{% endblock %}
-
 {% block page_title %}{{ ftl('not-found-page-not-found-page-page-not-found') }}{% endblock %}

 {% block page_css %}
--- a/glean/metrics.yaml
+++ b/glean/metrics.yaml
@ -2,112 +2,6 @@
 $schema: moz://mozilla.org/schemas/glean/metrics/2-0-0

 page:
-  viewed:
-    type: datetime
-    lifetime: application
-    send_in_pings:
-      - events
-    description: |
-      The time a page was viewed.
-    data_sensitivity:
-      - web_activity
-    bugs:
-      - https://github.com/mozilla/bedrock/issues/10746
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767442
-    notification_emails:
-      - marketing-websites-team@mozilla.com
-    expires: never
-  path:
-    type: string
-    lifetime: application
-    send_in_pings:
-      - events
-    description: |
-      The URL path of the page that was viewed, excluding locale.
-    data_sensitivity:
-      - web_activity
-    bugs:
-      - https://github.com/mozilla/bedrock/issues/10746
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767442
-    notification_emails:
-      - marketing-websites-team@mozilla.com
-    expires: never
-  locale:
-    type: string
-    lifetime: application
-    send_in_pings:
-      - events
-    description: |
-      The locale of the page that was viewed.
-    data_sensitivity:
-      - web_activity
-    bugs:
-      - https://github.com/mozilla/bedrock/issues/10746
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767442
-    notification_emails:
-      - marketing-websites-team@mozilla.com
-    expires: never
-  query_params:
-    type: labeled_string
-    lifetime: application
-    send_in_pings:
-      - events
-    description: |
-      Query parameters associated with the URL of
-      the page that was viewed.
-    bugs:
-      - https://github.com/mozilla/bedrock/issues/10746
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767442
-    notification_emails:
-      - marketing-websites-team@mozilla.com
-    expires: never
-    labels:
-      - utm_source
-      - utm_campaign
-      - utm_medium
-      - utm_content
-      - entrypoint_experiment
-      - entrypoint_variation
-      - experiment
-      - variation
-      - v
-      - xv
-  referrer:
-    type: string
-    lifetime: application
-    send_in_pings:
-      - events
-    description: |
-      The referring URL that linked to the page that was viewed.
-    data_sensitivity:
-      - web_activity
-    bugs:
-      - https://github.com/mozilla/bedrock/issues/10746
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1767442
-    notification_emails:
-      - marketing-websites-team@mozilla.com
-    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:
-      - marketing-websites-team@mozilla.com
-    expires: never
  interaction:
    type: event
    description: |
--- a/media/js/glean/page.es6.js
+++ b/media/js/glean/page.es6.js
@ -5,49 +5,6 @@
 */

 import * as page from '../libs/glean/page.js';
-import Utils from './utils.es6';
-
-const defaultParams = {
-    utm_source: '',
-    utm_campaign: '',
-    utm_medium: '',
-    utm_content: '',
-    entrypoint_experiment: '',
-    entrypoint_variation: '',
-    experiment: '',
-    variation: '',
-    v: '', // short param for 'variation'
-    xv: '' // short param for 'experience version'.
-};
-
-function recordCustomPageMetrics() {
-    page.viewed.set();
-    page.path.set(Utils.getPathFromUrl());
-    page.locale.set(Utils.getLocaleFromUrl());
-    page.referrer.set(Utils.getReferrer());
-    page.httpStatus.set(Utils.getHttpStatus());
-
-    const params = Utils.getQueryParamsFromUrl();
-    const finalParams = {};
-
-    // validate only known & trusted query params
-    // for inclusion in Glean metrics.
-    for (const param in defaultParams) {
-        if (Object.prototype.hasOwnProperty.call(defaultParams, param)) {
-            const allowedChars = /^[\w/.%-]+$/;
-            let v = params.get(param);
-
-            if (v) {
-                v = decodeURIComponent(v);
-                finalParams[param] = allowedChars.test(v) ? v : '';
-            } else {
-                finalParams[param] = '';
-            }
-
-            page.queryParams[param].set(finalParams[param]);
-        }
-    }
-}

 function pageEvent(obj) {
    if (typeof obj !== 'object' && typeof obj.label !== 'string') {
@ -77,4 +34,4 @@ function pageEvent(obj) {
    }
 }

-export { recordCustomPageMetrics, pageEvent };
+export { pageEvent };
--- a/media/js/glean/utils.es6.js
+++ b/media/js/glean/utils.es6.js
@ -6,7 +6,7 @@

 import Glean from '@mozilla/glean/web';
 import GleanMetrics from '@mozilla/glean/metrics';
-import { pageEvent, recordCustomPageMetrics } from './page.es6';
+import { pageEvent } from './page.es6';
 import {
    consentRequired,
    getConsentCookie,
@ -14,12 +14,16 @@ import {
 } from '../base/consent/utils.es6';

 const Utils = {
-    filterNewsletterURL: (str) => {
+    /**
+     * Takes a URL string and filters out any sensitive information,
+     * such as newsletter tokens, before returning the URL.
+     * See issue https://github.com/mozilla/bedrock/issues/13583
+     * @param {String} URL
+     * @returns {String} filtered URL
+     */
+    filterURL: (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/'
@ -45,63 +49,6 @@ const Utils = {
        }
    },

-    getUrl: (str) => {
-        const url = typeof str === 'string' ? str : window.location.href;
-        return Utils.filterNewsletterURL(url);
-    },
-
-    getPathFromUrl: (str) => {
-        let pathName =
-            typeof str === 'string' ? str : document.location.pathname;
-        pathName = pathName.replace(/^(\/\w{2}-\w{2}\/|\/\w{2,3}\/)/, '/');
-        const newsletterPaths = [
-            '/newsletter/existing/',
-            '/newsletter/country/'
-        ];
-
-        // Ensure we don't include tokens in newsletter page pings
-        // Issue https://github.com/mozilla/bedrock/issues/13583
-        newsletterPaths.forEach((path) => {
-            if (pathName.includes(path)) {
-                pathName = path;
-            }
-        });
-
-        return pathName;
-    },
-
-    getLocaleFromUrl: (str) => {
-        const pathName =
-            typeof str === 'string' ? str : document.location.pathname;
-        const locale = pathName.match(/^\/(\w{2}-\w{2}|\w{2,3})\//);
-        // If there's no locale in the path then assume language is `en-US`;
-        return locale && locale.length > 0 ? locale[1] : 'en-US';
-    },
-
-    getQueryParamsFromUrl: (str) => {
-        const query = typeof str === 'string' ? str : window.location.search;
-
-        if (typeof window._SearchParams !== 'undefined') {
-            return new window._SearchParams(query);
-        }
-
-        return false;
-    },
-
-    getReferrer: (str) => {
-        const referrer = typeof str === 'string' ? str : document.referrer;
-        const url = Utils.filterNewsletterURL(referrer);
-
-        return url;
-    },
-
-    getHttpStatus: () => {
-        const pageId = document
-            .getElementsByTagName('html')[0]
-            .getAttribute('data-http-status');
-        return pageId && pageId === '404' ? '404' : '200';
-    },
-
    /**
     * Determine if page URL is /firefox/download/thanks/.
     */
@ -137,8 +84,6 @@ const Utils = {
     * Record page load event and add custom metrics.
     */
    initPageLoadEvent: () => {
-        recordCustomPageMetrics();
-
        /**
         * Manually call Glean's default page_load event. Here
         * we override `url` and `referrer` since we need to
@ -146,8 +91,8 @@ const Utils = {
         * are sent.
         */
        GleanMetrics.pageLoad({
-            url: Utils.getUrl(),
-            referrer: Utils.getReferrer()
+            url: Utils.filterURL(window.location.href),
+            referrer: Utils.filterURL(document.referrer)
        });
    },

--- a/tests/unit/spec/glean/page.js
+++ b/tests/unit/spec/glean/page.js
@ -10,141 +10,18 @@
 */

 import * as page from '../../../../media/js/libs/glean/page.js';
-import Utils from '../../../../media/js/glean/utils.es6';
-import {
-    recordCustomPageMetrics,
-    pageEvent
-} from '../../../../media/js/glean/page.es6';
+import { pageEvent } from '../../../../media/js/glean/page.es6';
 import { testResetGlean } from '@mozilla/glean/testing';

 describe('page.js', function () {
-    beforeEach(async function () {
+    beforeEach(function () {
        /**
         * note: maxEvents is set to a number greater than 1 here,
         * to circumvent a bug in Glean's test helper where `snapshot`
         * will be undefined. This can hopefully be removed in the
         * next release.
         */
-        await testResetGlean('moz-bedrock-test', true, { maxEvents: 100 });
-
-        spyOn(Utils, 'getPathFromUrl').and.returnValue('/firefox/new/');
-        spyOn(Utils, 'getLocaleFromUrl').and.returnValue('en-US');
-        spyOn(Utils, 'getReferrer').and.returnValue('https://google.com/');
-    });
-
-    it('should register a page view correctly', async function () {
-        recordCustomPageMetrics();
-
-        const path = await page.path.testGetValue();
-        expect(path).toEqual('/firefox/new/');
-
-        const locale = await page.locale.testGetValue();
-        expect(locale).toEqual('en-US');
-
-        const referrer = await page.referrer.testGetValue();
-        expect(referrer).toEqual('https://google.com/');
-
-        const httpStatus = await page.httpStatus.testGetValue();
-        expect(httpStatus).toEqual('200');
-    });
-
-    it('should record specific query parameters in the page view', async function () {
-        const query =
-            'utm_source=test-source&utm_campaign=test-campaign&utm_medium=test-medium&utm_content=test-content&entrypoint_experiment=test_entrypoint_experiment&entrypoint_variation=1&experiment=test-experiment&variation=1&v=1&xv=test-xv';
-        spyOn(Utils, 'getQueryParamsFromUrl').and.returnValue(
-            new window._SearchParams(query)
-        );
-
-        recordCustomPageMetrics();
-
-        const source = await page.queryParams['utm_source'].testGetValue();
-        expect(source).toEqual('test-source');
-
-        const campaign = await page.queryParams['utm_campaign'].testGetValue();
-        expect(campaign).toEqual('test-campaign');
-
-        const medium = await page.queryParams['utm_medium'].testGetValue();
-        expect(medium).toEqual('test-medium');
-
-        const content = await page.queryParams['utm_content'].testGetValue();
-        expect(content).toEqual('test-content');
-
-        const entrypointExperiment =
-            await page.queryParams['entrypoint_experiment'].testGetValue();
-        expect(entrypointExperiment).toEqual('test_entrypoint_experiment');
-
-        const entrypointVariation =
-            await page.queryParams['entrypoint_variation'].testGetValue();
-        expect(entrypointVariation).toEqual('1');
-
-        const experiment = await page.queryParams['experiment'].testGetValue();
-        expect(experiment).toEqual('test-experiment');
-
-        const variation = await page.queryParams['variation'].testGetValue();
-        expect(variation).toEqual('1');
-
-        const v = await page.queryParams['v'].testGetValue();
-        expect(v).toEqual('1');
-
-        const xv = await page.queryParams['xv'].testGetValue();
-        expect(xv).toEqual('test-xv');
-    });
-
-    it('should not record unspecified query params in the page view', async function () {
-        const query =
-            'unspecified_param=test-unspecified-param&utm_content=test-content';
-        spyOn(Utils, 'getQueryParamsFromUrl').and.returnValue(
-            new window._SearchParams(query)
-        );
-
-        recordCustomPageMetrics();
-
-        const unspecifiedParam =
-            await page.queryParams['unspecified_param'].testGetValue();
-        expect(unspecifiedParam).toBeUndefined();
-
-        const content = await page.queryParams['utm_content'].testGetValue();
-        expect(content).toEqual('test-content');
-    });
-
-    it('should decode known params', async function () {
-        const query = 'utm_source=%25&utm_campaign=%2F';
-        spyOn(Utils, 'getQueryParamsFromUrl').and.returnValue(
-            new window._SearchParams(query)
-        );
-
-        recordCustomPageMetrics();
-
-        const source = await page.queryParams['utm_source'].testGetValue();
-        expect(source).toEqual('%');
-
-        const campaign = await page.queryParams['utm_campaign'].testGetValue();
-        expect(campaign).toEqual('/');
-    });
-
-    it('should not record known params that contain bad values', async function () {
-        const query =
-            'utm_source=<script>yikes</script>&utm_campaign=%5Ctest&utm_medium=%3Ctest&utm_content=test-content&experiment';
-        spyOn(Utils, 'getQueryParamsFromUrl').and.returnValue(
-            new window._SearchParams(query)
-        );
-
-        recordCustomPageMetrics();
-
-        const source = await page.queryParams['utm_source'].testGetValue();
-        expect(source).toEqual('');
-
-        const campaign = await page.queryParams['utm_campaign'].testGetValue();
-        expect(campaign).toEqual('');
-
-        const medium = await page.queryParams['utm_medium'].testGetValue();
-        expect(medium).toEqual('');
-
-        const content = await page.queryParams['utm_content'].testGetValue();
-        expect(content).toEqual('test-content');
-
-        const experiment = await page.queryParams['experiment'].testGetValue();
-        expect(experiment).toEqual('');
+        testResetGlean('moz-bedrock-test', true, { maxEvents: 100 });
    });

    it('should send an interaction event as expected', async function () {
--- a/tests/unit/spec/glean/utils.js
+++ b/tests/unit/spec/glean/utils.js
@ -12,134 +12,32 @@
 import Utils from '../../../../media/js/glean/utils.es6';

 describe('utils.js', function () {
-    describe('getUrl', function () {
-        it('should return the a complete URL including query parameters', function () {
+    describe('filterURL', function () {
+        it('should retain original URLs including query parameters', function () {
            const url1 = 'https://www.mozilla.org/en-US/';
-            expect(Utils.getUrl(url1)).toEqual(url1);
+            expect(Utils.filterURL(url1)).toEqual(url1);

            const url2 =
                'https://www.mozilla.org/en-US/firefox/new/?utm_source=test&utm_campaign=test';
-            expect(Utils.getUrl(url2)).toEqual(url2);
+            expect(Utils.filterURL(url2)).toEqual(url2);
        });

-        it('should remove newsletter tokens from know URLs', function () {
+        it('should remove newsletter tokens from known URLs', function () {
            const url1 =
                'https://www.mozilla.org/en-US/newsletter/existing/a1a2a3a4-abc1-12ab-a123-12345a12345b/?utm_source=test&utm_campaign=test';
-            expect(Utils.getUrl(url1)).toEqual(
+            expect(Utils.filterURL(url1)).toEqual(
                'https://www.mozilla.org/en-US/newsletter/existing/?utm_source=test&utm_campaign=test'
            );

            const url2 =
                'https://www.mozilla.org/en-US/newsletter/country/a1a2a3a4-abc1-12ab-a123-12345a12345b/?utm_source=test&utm_campaign=test';
-            expect(Utils.getUrl(url2)).toEqual(
+            expect(Utils.filterURL(url2)).toEqual(
                'https://www.mozilla.org/en-US/newsletter/country/?utm_source=test&utm_campaign=test'
            );

            const url3 =
                'https://www.mozilla.org/en-US/newsletter/existing/?utm_source=test&utm_campaign=test';
-            expect(Utils.getUrl(url3)).toEqual(url3);
-        });
-    });
-
-    describe('getPathFromUrl', function () {
-        it('should return the path from a page URL excluding locale', function () {
-            expect(Utils.getPathFromUrl('/en-US/firefox/new/')).toEqual(
-                '/firefox/new/'
-            );
-            expect(Utils.getPathFromUrl('/de/firefox/new/')).toEqual(
-                '/firefox/new/'
-            );
-            expect(Utils.getPathFromUrl('/kab/firefox/new/')).toEqual(
-                '/firefox/new/'
-            );
-            expect(Utils.getPathFromUrl('/fr/')).toEqual('/');
-        });
-
-        it('should return original path when there is no locale', function () {
-            expect(Utils.getPathFromUrl('/locales/')).toEqual('/locales/');
-        });
-
-        it('should exclude tokens from newsletter URLS', function () {
-            expect(
-                Utils.getPathFromUrl(
-                    '/en-US/newsletter/existing/a1a2a3a4-abc1-12ab-a123-12345a12345b/'
-                )
-            ).toEqual('/newsletter/existing/');
-
-            expect(
-                Utils.getPathFromUrl(
-                    '/en-US/newsletter/country/a1a2a3a4-abc1-12ab-a123-12345a12345b/'
-                )
-            ).toEqual('/newsletter/country/');
-        });
-    });
-
-    describe('getLocaleFromUrl', function () {
-        it('should return the locale from a page URL excluding path', function () {
-            expect(Utils.getLocaleFromUrl('/en-US/firefox/new/')).toEqual(
-                'en-US'
-            );
-            expect(Utils.getLocaleFromUrl('/de/firefox/new/')).toEqual('de');
-            expect(Utils.getLocaleFromUrl('/kab/firefox/new/')).toEqual('kab');
-            expect(Utils.getLocaleFromUrl('/fr/')).toEqual('fr');
-        });
-
-        it('should return `en-US` for language when there is no locale', function () {
-            expect(Utils.getLocaleFromUrl('/locales/')).toEqual('en-US');
-        });
-    });
-
-    describe('getQueryParamsFromUrl', function () {
-        it('should return an object made up of params from a query string', function () {
-            const query =
-                'utm_source=test-source&utm_campaign=test-campaign&utm_medium=test-medium&utm_content=test-content&entrypoint_experiment=test_entrypoint_experiment&entrypoint_variation=1&experiment=test-experiment&variation=1&v=1&xv=test-xv';
-
-            expect(Utils.getQueryParamsFromUrl(query).params).toEqual({
-                utm_source: 'test-source',
-                utm_campaign: 'test-campaign',
-                utm_medium: 'test-medium',
-                utm_content: 'test-content',
-                entrypoint_experiment: 'test_entrypoint_experiment',
-                entrypoint_variation: 1,
-                experiment: 'test-experiment',
-                variation: 1,
-                v: 1,
-                xv: 'test-xv'
-            });
-        });
-    });
-
-    describe('getReferrer', function () {
-        it('should return regular referrer', function () {
-            const expected = 'http://www.bing.com/';
-            expect(Utils.getReferrer(expected)).toEqual(expected);
-        });
-
-        it('should strip newsletter tokens from referrer URLs', function () {
-            const expected =
-                'https://www.mozilla.org/en-US/newsletter/country/a1a2a3a4-abc1-12ab-a123-12345a12345b/?utm_source=test&utm_campaign=test';
-            expect(Utils.getReferrer(expected)).toEqual(
-                'https://www.mozilla.org/en-US/newsletter/country/?utm_source=test&utm_campaign=test'
-            );
-        });
-    });
-
-    describe('getHttpStatus', function () {
-        afterEach(function () {
-            document
-                .getElementsByTagName('html')[0]
-                .removeAttribute('data-http-status');
-        });
-
-        it('should return 200 by default', function () {
-            expect(Utils.getHttpStatus()).toEqual('200');
-        });
-
-        it('should return 404 if matching data-http-status attribute is present in the page', function () {
-            document
-                .getElementsByTagName('html')[0]
-                .setAttribute('data-http-status', '404');
-            expect(Utils.getHttpStatus()).toEqual('404');
+            expect(Utils.filterURL(url3)).toEqual(url3);
        });
    });