bug(shared): Amplitude crashes on startup with unhandled error

Because: - Content server was restarting due to unknown event group This Commit: - Wraps the mapping function with try catch to ensure it cannot result in an unhandled error - Adds optional statsd and logger params so that we can track potential problems outside of sentry. - Updates initializations to provide statsd and logger params to amplitude's initialization routine.
mozilla · Sep 21, 2023 · dba18b1 · dba18b1
1 parent cc10cba
commit dba18b1
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 81 deletions.
diff --git a/packages/fxa-auth-server/lib/metrics/amplitude.js b/packages/fxa-auth-server/lib/metrics/amplitude.js
@@ -14,6 +14,8 @@
 
 const { Container } = require('typedi');
 const { StatsD } = require('hot-shots');
+const config = require('../../config').default.getProperties();
+const logger = require('../log')(config.log.level, 'amplitude');
 
 const { GROUPS, initialize } =
   require('fxa-shared/metrics/amplitude').amplitude;
@@ -183,7 +185,9 @@ module.exports = (log, config) => {
   const transformEvent = initialize(
     config.oauth.clientIds,
     EVENTS,
-    FUZZY_EVENTS
+    FUZZY_EVENTS,
+    logger,
+    Container.has(StatsD) ? Container.get(StatsD) : undefined
   );
 
   return receiveEvent;

diff --git a/packages/fxa-content-server/server/lib/amplitude.js b/packages/fxa-content-server/server/lib/amplitude.js
@@ -916,7 +916,7 @@ const FUZZY_EVENTS = new Map([
   ],
 ]);
 
-const transform = initialize(SERVICES, EVENTS, FUZZY_EVENTS);
+const transform = initialize(SERVICES, EVENTS, FUZZY_EVENTS, logger, statsd);
 
 module.exports = receiveEvent;
 

diff --git a/packages/fxa-payments-server/server/lib/amplitude.js b/packages/fxa-payments-server/server/lib/amplitude.js
@@ -47,7 +47,9 @@ const FUZZY_EVENTS = new Map([
 const transform = initialize(
   config.get('oauth_client_id_map'),
   {},
-  FUZZY_EVENTS
+  FUZZY_EVENTS,
+  log,
+  Container.has(StatsD) ? Container.get(StatsD) : undefined
 );
 
 // TODO: remove eslint ignore in FXA-6950

diff --git a/packages/fxa-shared/metrics/amplitude.ts b/packages/fxa-shared/metrics/amplitude.ts
@@ -6,6 +6,8 @@ import Ajv from 'ajv';
 import pick from 'lodash.pick';
 import { ParsedUserAgentProperties, ParsedUa, ParsedOs } from './user-agent';
 import { Location } from '../connected-services/models/Location';
+import { ILogger } from '../log';
+import { StatsD } from 'hot-shots';
 
 type AmplitudeEventGroup = typeof GROUPS;
 type AmplitudeEventGroupKey = keyof AmplitudeEventGroup;
@@ -306,6 +308,8 @@ export const amplitude = {
    *                          but here `group` can be a string or a function. If it's
    *                          a function, it will be passed the matched `eventCategory`
    *                          as its argument and should return the group string.
+   * @param {StatsD} statsd   An optional statsd client.
+   * @param {}
    *
    * @returns {Function}      The mapper function.
    */
@@ -324,7 +328,9 @@ export const amplitude = {
         group: AmplitudeEventGroupKey | AmplitudeEventFuzzyEventGroupMapFn;
         event: string | AmplitudeEventFuzzyEventNameMapFn;
       }
-    >
+    >,
+    log?: ILogger,
+    statsd?: StatsD
   ) {
     /**
      * Map from a source event and it's associated data to an amplitude event.
@@ -341,93 +347,98 @@ export const amplitude = {
      *                            ease by perusing the code.
      */
     return (event: { [key: string]: any }, data: EventData) => {
-      if (!event || !data) {
-        return;
-      }
-
-      let eventType = event.type;
-      let mapping = events[eventType];
-      let eventCategory, eventTarget;
-
-      if (!mapping) {
-        for (const [key, value] of fuzzyEvents.entries()) {
-          const match = key.exec(eventType);
-          if (match) {
-            mapping = value;
+      try {
+        if (!event || !data) {
+          return;
+        }
 
-            if (match.length >= 2) {
-              eventCategory = match[1];
-              if (match.length === 3) {
-                eventTarget = match[2];
+        let eventType = event.type;
+        let mapping = events[eventType];
+        let eventCategory, eventTarget;
+
+        if (!mapping) {
+          for (const [key, value] of fuzzyEvents.entries()) {
+            const match = key.exec(eventType);
+            if (match) {
+              mapping = value;
+
+              if (match.length >= 2) {
+                eventCategory = match[1];
+                if (match.length === 3) {
+                  eventTarget = match[2];
+                }
               }
-            }
 
-            break;
+              break;
+            }
           }
         }
-      }
 
-      if (mapping) {
-        eventType = mapping.event;
-        if (typeof eventType === 'function') {
-          eventType = eventType(eventCategory, eventTarget);
-          if (!eventType) {
-            return;
+        if (mapping) {
+          eventType = mapping.event;
+          if (typeof eventType === 'function') {
+            eventType = eventType(eventCategory, eventTarget);
+            if (!eventType) {
+              return;
+            }
           }
-        }
 
-        let eventGroup = mapping.group;
-        if (typeof eventGroup === 'function') {
-          eventGroup = eventGroup(eventCategory);
-          if (!eventGroup) {
-            return;
+          let eventGroup = mapping.group;
+          if (typeof eventGroup === 'function') {
+            eventGroup = eventGroup(eventCategory);
+            if (!eventGroup) {
+              return;
+            }
           }
-        }
 
-        let version;
-        try {
-          // @ts-ignore
-          version = /([0-9]+)\.([0-9]+)$/.exec(data.version)[0];
-        } catch (err) {}
-
-        // minimal data should be enabled for routes used by internal
-        // services like profile-server and token-server
-        if (mapping.minimal) {
-          data = {
-            uid: data.uid,
-            service: data.service,
-            version: data.version,
-          };
-        }
+          let version;
+          try {
+            // @ts-ignore
+            version = /([0-9]+)\.([0-9]+)$/.exec(data.version)[0];
+          } catch (err) {}
+
+          // minimal data should be enabled for routes used by internal
+          // services like profile-server and token-server
+          if (mapping.minimal) {
+            data = {
+              uid: data.uid,
+              service: data.service,
+              version: data.version,
+            };
+          }
 
-        return pruneUnsetValues({
-          op: 'amplitudeEvent',
-          event_type: `${eventGroup} - ${eventType}`,
-          time: event.time,
-          user_id: data.uid,
-          device_id: data.deviceId,
-          session_id: data.flowBeginTime,
-          app_version: version,
-          language: data.lang,
-          country_code: data.countryCode,
-          country: data.country,
-          region: data.region,
-          os_name: data.os,
-          os_version: data.osVersion,
-          device_model: data.formFactor,
-          event_properties: mapEventProperties(
-            eventType,
-            eventGroup as string,
-            eventCategory as string,
-            eventTarget as string,
-            data
-          ),
-          user_properties: mapUserProperties(
-            eventGroup as string,
-            eventCategory as string,
-            data
-          ),
-        });
+          return pruneUnsetValues({
+            op: 'amplitudeEvent',
+            event_type: `${eventGroup} - ${eventType}`,
+            time: event.time,
+            user_id: data.uid,
+            device_id: data.deviceId,
+            session_id: data.flowBeginTime,
+            app_version: version,
+            language: data.lang,
+            country_code: data.countryCode,
+            country: data.country,
+            region: data.region,
+            os_name: data.os,
+            os_version: data.osVersion,
+            device_model: data.formFactor,
+            event_properties: mapEventProperties(
+              eventType,
+              eventGroup as string,
+              eventCategory as string,
+              eventTarget as string,
+              data
+            ),
+            user_properties: mapUserProperties(
+              eventGroup as string,
+              eventCategory as string,
+              data
+            ),
+          });
+        }
+      } catch (err) {
+        statsd?.increment('fxa.amplitude.transform.error');
+        log?.error(err);
       }
 
       return;
@@ -442,6 +453,10 @@ export const amplitude = {
     ) {
       const { serviceName, clientId } = getServiceNameAndClientId(data);
 
+      if (typeof EVENT_PROPERTIES[eventGroup] !== 'function') {
+        throw new Error(`Unknown event group: ${eventGroup}`);
+      }
+
       return Object.assign(
         pruneUnsetValues({
           service: serviceName,

diff --git a/packages/fxa-shared/test/metrics/amplitude.js b/packages/fxa-shared/test/metrics/amplitude.js
@@ -40,7 +40,7 @@ describe('metrics/amplitude:', () => {
 
   it('exports an initialize method', () => {
     assert.isFunction(amplitude.initialize);
-    assert.lengthOf(amplitude.initialize, 3);
+    assert.lengthOf(amplitude.initialize, 5);
   });
 
   describe('initialize:', () => {