Polish sentry traceid on logs

libs/accounts/errors/src/app-error.ts

@@ -9,7 +9,6 @@ import {
   TOO_LARGE,
   DEFAULT_ERRROR,
   IGNORED_ERROR_NUMBERS,
-  DEBUGGABLE_PAYLOAD_KEYS,
 } from './constants';
 import { OauthError } from './oauth-error';
 import type { Request as HapiRequest } from 'hapi';
@@ -1787,8 +1786,6 @@ export class AppError extends Error {
         method: request.method,
         path: request.path,
         query: request.query,
-        payload: scrubPii(request.payload),
-        headers: scrubHeaders(request.headers),
       };
     }
   }
@@ -1841,42 +1838,6 @@ export class AppError extends Error {
   }
 }
 
-/**
- * Tries to remove PII from payload data.
- * @param payload
- * @returns
- */
-function scrubPii(payload: any) {
-  if (!payload) {
-    return;
-  }
-
-  return Object.entries(payload).reduce((scrubbed: any, [key, value]) => {
-    if (DEBUGGABLE_PAYLOAD_KEYS.has(key)) {
-      scrubbed[key] = value;
-    }
-
-    return scrubbed;
-  }, {});
-}
-
-/**
- * Deletes feilds with senstive data from headers.
- * @param headers
- * @returns
- */
-function scrubHeaders(
-  headers?: Record<string, string>
-): Record<string, string> {
-  if (headers == null) {
-    return {};
-  }
-
-  const scrubbed = { ...headers };
-  delete scrubbed['x-forwarded-for'];
-  return scrubbed;
-}
-
 /**
  * Prevents errors from being captured by Sentry.
  *

libs/accounts/errors/src/constants.ts

@@ -248,37 +248,3 @@ export const BAD_SIGNATURE_ERRORS = [
   'Missing required payload hash',
   'Payload is invalid',
 ];
-
-/**
- * Payload properties that might help us debug unexpected errors
- * when they show up in production. Obviously we don't want to
- * accidentally send any sensitive data or PII to a 3rd-party,
- * so the set is opt-in rather than opt-out.
- */
-export const DEBUGGABLE_PAYLOAD_KEYS = new Set([
-  'availableCommands',
-  'capabilities',
-  'client_id',
-  'code',
-  'command',
-  'duration',
-  'excluded',
-  'features',
-  'messageId',
-  'metricsContext',
-  'name',
-  'preVerified',
-  'publicKey',
-  'reason',
-  'redirectTo',
-  'reminder',
-  'scope',
-  'service',
-  'target',
-  'to',
-  'TTL',
-  'ttl',
-  'type',
-  'unblockCode',
-  'verificationMethod',
-]);

libs/shared/otel/src/lib/config.ts

@@ -25,6 +25,9 @@ export type TracingOpts = {
     url: string;
     concurrencyLimit: number;
   };
+  sentry?: {
+    enabled: boolean;
+  };
 };
 
 /** Default convict config for node tracing */

libs/shared/otel/src/lib/node-tracing.ts

@@ -11,6 +11,7 @@ import {
   checkServiceName,
   logType,
 } from './config';
+import { SentryContextManager } from '@sentry/node';
 import {
   NodeTracerProvider,
   TraceIdRatioBasedSampler,
@@ -109,10 +110,26 @@ export class NodeTracingInitializer {
           '@opentelemetry/instrumentation-aws-lambda': {
             enabled: false,
           },
+          '@opentelemetry/instrumentation-http': {
+            // Important! If Sentry is enabled, we want to rely on Sentry's http instrumentation!
+            enabled: this.opts.sentry?.enabled ? false : true,
+          },
         }),
       ],
     });
-    this.provider.register();
+    // Use SentryContextManager so that Sentry's per-request isolation scopes
+    // are properly forked for every api.context.with() call. Without this,
+    // NodeTracerProvider.register() would install a plain
+    // AsyncLocalStorageContextManager that doesn't understand Sentry's scope
+    // forking, and Sentry.init()'s own setupOtel() would overwrite our
+    // NodeTracerProvider (discarding the custom exporters).
+    if (this.opts.sentry?.enabled) {
+      this.provider.register({
+        contextManager: new SentryContextManager(),
+      });
+    } else {
+      this.provider.register();
+    }
   }
 
   public startSpan(name: string, action: () => void) {

libs/shared/sentry/src/lib/models/SentryConfigOpts.ts

@@ -34,6 +34,9 @@ export type SentryConfigOpts = {
      * When using strings, partial matches will be filtered out. If you need to filter by exact match, use regex patterns instead */
     ignoreErrors?: (string | RegExp)[];
 
+    /** Let Sentry know that otel setup should be handled by the app. */
+    skipOpenTelemetrySetup?: boolean;
+
     /** When set to true, building a configuration will throw an error critical fields are missing. */
     strict?: boolean;
 

packages/fxa-auth-server/lib/log.js

@@ -64,16 +64,30 @@ util.inherits(Lug, EventEmitter);
 Lug.prototype.close = function () {};
 
 Lug.prototype.getTraceId = function (data) {
-  let result = {};
+  const result = {};
 
-  if (this.nodeTracer) {
-    try {
-      result = { traceId: this.nodeTracer.getTraceId() };
-    } catch {
-      // don't let a tracing issue break logging
-      this.debug('log', { msg: 'could not get trace id' });
+  // Try decorating otel traceId
+  try {
+    const otelTraceId = this.nodeTracer?.getTraceId();
+    if (otelTraceId) {
+      result.otelTraceId = otelTraceId;
     }
+  } catch (err) {
+    // Don't let an otel hiccup break logging
+    result.otelTraceIdErr = err;
   }
+
+  // Try decorating currenty sentry trace id
+  try {
+    const spanContext = Sentry.getActiveSpan()?.spanContext();
+    if (spanContext) {
+      result.sentryTraceId = spanContext.traceId;
+    }
+  } catch (err) {
+    // Don't let a sentry hicup break logging
+    result.sentryTraceIdError = err;
+  }
+
   return result;
 };
 

packages/fxa-auth-server/lib/monitoring.js

@@ -21,7 +21,11 @@ initMonitoring({
     ...config.getProperties(),
     release: version,
     eventFilters: [filterSentryEvent],
-    integrations: [Sentry.linkedErrorsIntegration({ key: 'jse_cause' })],
+    integrations: [
+      // Important! This fixes a ton of problems with our previous integration.
+      Sentry.hapiIntegration(),
+      Sentry.linkedErrorsIntegration({ key: 'jse_cause' }),
+    ],
   },
 });
 

packages/fxa-auth-server/lib/routes/util.js

@@ -4,10 +4,12 @@
 
 'use strict';
 
+const Sentry = require('@sentry/node');
 const isA = require('joi');
 const random = require('../crypto/random');
 const validators = require('./validators');
 const UTIL_DOCS = require('../../docs/swagger/util-api').default;
+const { AppError } = require('../../../../libs/accounts/errors/src');
 
 const HEX_STRING = validators.HEX_STRING;
 
@@ -67,5 +69,39 @@ module.exports = (log, config, redirectDomain) => {
         return h.redirect(config.contentServer.url + request.raw.req.url);
       },
     },
+    {
+      method: 'GET',
+      path: '/boom',
+      options: {},
+      handler: async function (request, h) {
+        if (config.sentry.env === 'local') {
+          const traceId = Sentry.getActiveSpan().spanContext().traceId;
+          console.log(`${traceId}: crumb ${new Date().toString()}`);
+          await (async () => {
+            await new Promise((r) => {
+              setTimeout(() => {
+                console.log(`${traceId}: crumb ${new Date().toString()}`);
+                r();
+              }, 100);
+            });
+
+            // Throw fabricated error
+            console.log(`${traceId}: crumb ${new Date().toString()}`);
+            throw new AppError(
+              {
+                code: 500,
+                error: 'BOOM',
+                errno: AppError.ERRNO.UNEXPECTED_ERROR,
+                message: 'Testing error capture',
+              },
+              {
+                op: { foo: 'bar' },
+                data: { foo: 'baz' },
+              }
+            );
+          })();
+        }
+      },
+    },
   ];
 };