bug(settings): Backend events for login_reg and login_complete have g…

…one missing - Some back end events seemed to disappear - This is due to the entry point not being defined on the event - Fixes missing entrypoint glean data - Ensures entrypoint is sent in graphql requests if it is present in URL - Fixes typo in utm parameter. `utmContext` should have been `utmContent`. - Puts the `MetricContext` model in a shared place to reduce the chance this happens again.
mozilla · Nov 27, 2024 · 360baf8 · 360baf8
1 parent 39fc891
commit 360baf8
Show file tree

Hide file tree

Showing 13 changed files with 118 additions and 40 deletions.
diff --git a/libs/shared/metrics/glean/src/index.ts b/libs/shared/metrics/glean/src/index.ts
@@ -1,3 +1,5 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+export * from './lib/metrics.context';
diff --git a/libs/shared/metrics/glean/src/lib/metrics.context.spec.ts b/libs/shared/metrics/glean/src/lib/metrics.context.spec.ts
@@ -0,0 +1,38 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+import { MetricsContext } from './metrics.context';
+
+describe('metrics context', () => {
+  it('creates', () => {
+    // Note! The auth-server expects quite a few query params to be propagated during
+    // graphql calls. These are commonly referred to as the metrics context.
+    const queryParams = {
+      flowId: 'test1',
+      utmCampaign: 'test2',
+      utmContent: 'test3',
+      utmMedium: 'test4',
+      utmSource: 'test5',
+      utmTerm: 'test6',
+      flowBeginTime: '100',
+      foo: 'test7',
+    };
+    const result = new MetricsContext(queryParams);
+    expect(result.flowId).toEqual(queryParams.flowId);
+    expect(result.utmCampaign).toEqual(queryParams.utmCampaign);
+    expect(result.utmContent).toEqual(queryParams.utmContent);
+    expect(result.utmMedium).toEqual(queryParams.utmMedium);
+    expect(result.utmSource).toEqual(queryParams.utmSource);
+    expect(result.utmTerm).toEqual(queryParams.utmTerm);
+    expect(result.flowBeginTime).toEqual(Number(queryParams.flowBeginTime));
+
+    // Make sure type is legit
+    expect((result as any).foo).toBeUndefined();
+  });
+
+  it('prunes empty fields', () => {
+    const result = new MetricsContext({});
+    const pruned = MetricsContext.prune(result);
+    expect(pruned).toEqual({});
+  });
+});
diff --git a/libs/shared/metrics/glean/src/lib/metrics.context.ts b/libs/shared/metrics/glean/src/lib/metrics.context.ts
@@ -0,0 +1,46 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * Represents common metrics context for auth server glean events. This
+ * is context that typically exists in query params on web pages, and
+ * drives important metric dashboards
+ */
+export class MetricsContext {
+  /** Returns a partial object with empty fields removed. */
+  static prune(context: Partial<MetricsContext>): Partial<MetricsContext> {
+    const pruned: any = Object.assign({}, context);
+    Object.keys(pruned).forEach((x) => {
+      if (pruned[x] === undefined) {
+        delete pruned[x];
+      }
+    });
+    return pruned;
+  }
+
+  deviceId?: string;
+  entrypoint?: string;
+  flowId?: string;
+  flowBeginTime?: number;
+  utmCampaign?: string;
+  utmContent?: string;
+  utmMedium?: string;
+  utmSource?: string;
+  utmTerm?: string;
+
+  constructor(queryParams?: Record<string, string>) {
+    queryParams = queryParams || {};
+    this.deviceId = queryParams['deviceId'];
+    this.entrypoint = queryParams['entrypoint'];
+    this.flowId = queryParams['flowId'];
+    this.flowBeginTime = queryParams['flowBeginTime']
+      ? Number(queryParams['flowBeginTime'])
+      : undefined;
+    this.utmCampaign = queryParams['utmCampaign'];
+    this.utmContent = queryParams['utmContent'];
+    this.utmMedium = queryParams['utmMedium'];
+    this.utmSource = queryParams['utmSource'];
+    this.utmTerm = queryParams['utmTerm'];
+  }
+}
diff --git a/packages/fxa-auth-client/lib/client.ts b/packages/fxa-auth-client/lib/client.ts
@@ -7,6 +7,7 @@ import { Credentials } from './crypto';
 import * as hawk from './hawk';
 import { SaltVersion, createSaltV2 } from './salt';
 import * as Sentry from '@sentry/browser';
+import { MetricsContext } from '@fxa/shared/metrics/glean';
 
 enum ERRORS {
   INVALID_TIMESTAMP = 111,
@@ -118,23 +119,6 @@ export type AuthServerError = Error & {
   retryAfterLocalized?: string;
 };
 
-export const MetricsContextKeys = [
-  'deviceId',
-  'flowId',
-  'flowBeginTime',
-  'utmCampaign',
-  'utmContext',
-  'utmMedium',
-  'utmSource',
-  'utmTerm',
-] as const;
-export type MetricsContext = Omit<
-  {
-    [k in (typeof MetricsContextKeys)[number]]?: string;
-  },
-  'flowBeginTime'
-> & { flowBeginTime?: number };
-
 export type VerificationMethod =
   | 'email'
   | 'email-otp'

diff --git a/packages/fxa-auth-server/lib/metrics/glean/index.ts b/packages/fxa-auth-server/lib/metrics/glean/index.ts
@@ -12,6 +12,7 @@ import { createHash } from 'crypto';
 import { AuthRequest } from '../../types';
 import * as AppError from '../../error';
 import { clientId as clientIdValidator } from '../../oauth/validators';
+import { MetricsContext } from '@fxa/shared/metrics/glean';
 
 // According to @types/hapi, request.auth.credentials.user is of type
 // UserCredentials, which is just {}. That's not actually the case and it
@@ -137,7 +138,7 @@ const createEventFn =
         return;
       }
 
-      const metricsContext = await request.app.metricsContext;
+      const metricsContext: MetricsContext = await request.app.metricsContext;
 
       // metrics sent with every event
       const commonMetrics = {

diff --git a/packages/fxa-settings/package.json b/packages/fxa-settings/package.json
@@ -78,6 +78,7 @@
       "^react-native$": "react-native-web",
       "^.+\\.module\\.(css|sass|scss)$": "identity-obj-proxy",
       "@fxa/shared/l10n": "<rootDir>/../../libs/shared/l10n/src/index.ts",
+      "@fxa/shared/metrics/glean": "<rootDir>/../../libs/shared/metrics/glean/src/index.ts",
       "^@fxa/shared/assets(.*)$": "<rootDir>/../../libs/shared/assets/src$1"
     },
     "moduleFileExtensions": [

diff --git a/packages/fxa-settings/src/lib/metrics.test.ts b/packages/fxa-settings/src/lib/metrics.test.ts
@@ -18,6 +18,7 @@ import {
   usePageViewEvent,
   logErrorEvent,
   initUserPreferences,
+  queryParamsToMetricsContext,
 } from './metrics';
 
 import { window } from './window';
@@ -460,3 +461,19 @@ describe('logError', () => {
     expectPayloadEvents(['error.unknown context.unknown namespace.-1']);
   });
 });
+
+describe('queryParamsToMetricsContext', () => {
+  it('creates metrics context from query params', () => {
+    // Note! The auth-server expects quite a few query params to be propagated during
+    // graphql calls. These are commonly referred to as the metrics context.
+    const queryParams = {
+      flowId: 'test1',
+      foo: 'test2',
+    };
+    const result = queryParamsToMetricsContext(queryParams);
+    expect(result.flowId).toEqual(queryParams.flowId);
+
+    // Make sure type is legit
+    expect((result as any).foo).toBeUndefined();
+  });
+});
diff --git a/packages/fxa-settings/src/lib/metrics.ts b/packages/fxa-settings/src/lib/metrics.ts
@@ -8,7 +8,7 @@ import { v4 as uuid } from 'uuid';
 import { QueryParams } from '..';
 import { once } from './utilities';
 import { useEffect } from 'react';
-import { MetricsContext, MetricsContextKeys } from 'fxa-auth-client/browser';
+import { MetricsContext } from '@fxa/shared/metrics/glean';
 
 export const settingsViewName = 'settings';
 
@@ -512,21 +512,11 @@ export function addExperiment(choice: string, group: string) {
 /**
  * Take a record and pick out key-values for a MetricsContext.  Note that the
  * value passed in could've been asserted to be of QueryParam type with
- * specific keys but the value is actually a record of all the URL query params
+ * specific keys but the value is actually a record of all the URL query params.
  */
 export function queryParamsToMetricsContext(
   queryParams: Record<string, string>
-): MetricsContext {
-  const metricsContext: MetricsContext = {};
-  return MetricsContextKeys.reduce((acc, k) => {
-    if (queryParams[k]) {
-      // Special case for flowBeginTime, which is a number
-      if (k === 'flowBeginTime') {
-        acc[k] = Number(queryParams[k]);
-      } else {
-        acc[k] = queryParams[k] as any;
-      }
-    }
-    return acc;
-  }, metricsContext);
+): Partial<MetricsContext> {
+  const context = new MetricsContext(queryParams);
+  return MetricsContext.prune(context);
 }
diff --git a/packages/fxa-settings/src/models/Account.ts b/packages/fxa-settings/src/models/Account.ts
@@ -10,10 +10,10 @@ import AuthClient, {
   generateRecoveryKey,
   getRecoveryKeyIdByUid,
   getCredentials,
-  MetricsContext,
   getCredentialsV2,
   getKeysV2,
 } from 'fxa-auth-client/browser';
+import { MetricsContext } from '@fxa/shared/glean';
 import {
   currentAccount,
   getStoredAccountData,

diff --git a/packages/fxa-settings/src/pages/ResetPassword/ConfirmResetPassword/interfaces.ts b/packages/fxa-settings/src/pages/ResetPassword/ConfirmResetPassword/interfaces.ts
@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import { MetricsContext } from 'fxa-auth-client/browser';
+import { MetricsContext } from '@fxa/shared/glean';
 import { ResendStatus } from '../../../lib/types';
 
 export interface ConfirmResetPasswordLocationState {

diff --git a/packages/fxa-settings/src/pages/Signin/container.tsx b/packages/fxa-settings/src/pages/Signin/container.tsx
@@ -63,7 +63,7 @@ import { searchParams } from '../../lib/utilities';
 import { QueryParams } from '../..';
 import { queryParamsToMetricsContext } from '../../lib/metrics';
 import OAuthDataError from '../../components/OAuthDataError';
-import { MetricsContext } from 'fxa-auth-client/browser';
+import { MetricsContext } from '@fxa/shared/glean';
 import { isEmailValid } from 'fxa-shared/email/helpers';
 import {
   getHandledError,

diff --git a/packages/fxa-settings/src/pages/Signup/interfaces.ts b/packages/fxa-settings/src/pages/Signup/interfaces.ts
@@ -5,7 +5,7 @@
 import { HandledError } from '../../lib/error-utils';
 import { BaseIntegration, OAuthIntegration } from '../../models';
 import { SignupQueryParams } from '../../models/pages/signup';
-import { MetricsContext } from 'fxa-auth-client/browser';
+import { MetricsContext } from '@fxa/shared/glean';
 
 export interface BeginSignupResponse {
   signUp: {

diff --git a/tsconfig.base.json b/tsconfig.base.json
@@ -27,9 +27,7 @@
       "@fxa/accounts/recovery-phone": [
         "libs/accounts/recovery-phone/src/index.ts"
       ],
-      "@fxa/accounts/two-factor": [
-        "libs/accounts/two-factor/src/index.ts"
-      ],
+      "@fxa/accounts/two-factor": ["libs/accounts/two-factor/src/index.ts"],
       "@fxa/payments/capability": ["libs/payments/capability/src/index.ts"],
       "@fxa/payments/cart": ["libs/payments/cart/src/index.ts"],
       "@fxa/payments/currency": ["libs/payments/currency/src/index.ts"],
@@ -71,6 +69,7 @@
       "@fxa/shared/l10n/server": ["libs/shared/l10n/src/server.ts"],
       "@fxa/shared/log": ["libs/shared/log/src/index.ts"],
       "@fxa/shared/metrics/statsd": ["libs/shared/metrics/statsd/src/index.ts"],
+      "@fxa/shared/metrics/glean": ["libs/shared/metrics/glean/src/index.ts"],
       "@fxa/shared/mozlog": ["libs/shared/mozlog/src/index.ts"],
       "@fxa/shared/notifier": ["libs/shared/notifier/src/index.ts"],
       "@fxa/shared/otel": ["libs/shared/otel/src/index.ts"],