Pass GCPFunction scope settings for transactions

getsentry · Oct 14, 2020 · a36385b · a36385b
1 parent 750e493
commit a36385b
Show file tree

Hide file tree

Showing 7 changed files with 148 additions and 118 deletions.
diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts
@@ -4,14 +4,12 @@ import {
   flush,
   getCurrentHub,
   Scope,
-  SDK_VERSION,
   Severity,
   startTransaction,
   withScope,
 } from '@sentry/node';
 import * as Sentry from '@sentry/node';
 import { Integration } from '@sentry/types';
-import { addExceptionMechanism } from '@sentry/utils';
 // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil
 // eslint-disable-next-line import/no-unresolved
 import { Context, Handler } from 'aws-lambda';
@@ -20,6 +18,7 @@ import { performance } from 'perf_hooks';
 import { types } from 'util';
 
 import { AWSServices } from './awsservices';
+import { addServerlessEventProcessor as addServerlessEventProcessorGeneric } from './utils';
 
 export * from '@sentry/node';
 
@@ -64,27 +63,7 @@ export function init(options: Sentry.NodeOptions = {}): void {
  * @param scope Scope that processor should be added to
  */
 function addServerlessEventProcessor(scope: Scope): void {
-  scope.addEventProcessor(event => {
-    event.sdk = {
-      ...event.sdk,
-      name: 'sentry.javascript.serverless',
-      integrations: [...((event.sdk && event.sdk.integrations) || []), 'AWSLambda'],
-      packages: [
-        ...((event.sdk && event.sdk.packages) || []),
-        {
-          name: 'npm:@sentry/serverless',
-          version: SDK_VERSION,
-        },
-      ],
-      version: SDK_VERSION,
-    };
-
-    addExceptionMechanism(event, {
-      handled: false,
-    });
-
-    return event;
-  });
+  addServerlessEventProcessorGeneric(scope, 'AWSLambda');
 }
 
 /**

diff --git a/packages/serverless/src/gcpfunction/cloud_events.ts b/packages/serverless/src/gcpfunction/cloud_events.ts
@@ -4,10 +4,10 @@ import {
   CloudEventFunction,
   CloudEventFunctionWithCallback,
 } from '@google-cloud/functions-framework/build/src/functions';
-import { flush, getCurrentHub, startTransaction } from '@sentry/node';
+import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
 import { logger } from '@sentry/utils';
 
-import { captureEventError, getActiveDomain, WrapperOptions } from './general';
+import { configureScopeWithContext, getActiveDomain, WrapperOptions } from './general';
 
 export type CloudEventFunctionWrapperOptions = WrapperOptions;
 
@@ -32,20 +32,22 @@ export function wrapCloudEventFunction(
       op: 'gcp.function.cloud_event',
     });
 
-    // We put the transaction on the scope so users can attach children to it
+    // getCurrentHub() is expected to use current active domain as a carrier
+    // since functions-framework creates a domain for each incoming request.
+    // So adding of event processors every time should not lead to memory bloat.
     getCurrentHub().configureScope(scope => {
+      configureScopeWithContext(scope, context);
+      // We put the transaction on the scope so users can attach children to it
       scope.setSpan(transaction);
     });
 
     const activeDomain = getActiveDomain();
 
-    activeDomain.on('error', err => {
-      captureEventError(err, context);
-    });
+    activeDomain.on('error', captureException);
 
     const newCallback = activeDomain.bind((...args: unknown[]) => {
       if (args[0] !== null && args[0] !== undefined) {
-        captureEventError(args[0], context);
+        captureException(args[0]);
       }
       transaction.finish();
 

diff --git a/packages/serverless/src/gcpfunction/events.ts b/packages/serverless/src/gcpfunction/events.ts
@@ -1,10 +1,10 @@
 // '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
 // When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
 import { EventFunction, EventFunctionWithCallback } from '@google-cloud/functions-framework/build/src/functions';
-import { flush, getCurrentHub, startTransaction } from '@sentry/node';
+import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
 import { logger } from '@sentry/utils';
 
-import { captureEventError, getActiveDomain, WrapperOptions } from './general';
+import { configureScopeWithContext, getActiveDomain, WrapperOptions } from './general';
 
 export type EventFunctionWrapperOptions = WrapperOptions;
 
@@ -29,20 +29,22 @@ export function wrapEventFunction(
       op: 'gcp.function.event',
     });
 
-    // We put the transaction on the scope so users can attach children to it
+    // getCurrentHub() is expected to use current active domain as a carrier
+    // since functions-framework creates a domain for each incoming request.
+    // So adding of event processors every time should not lead to memory bloat.
     getCurrentHub().configureScope(scope => {
+      configureScopeWithContext(scope, context);
+      // We put the transaction on the scope so users can attach children to it
       scope.setSpan(transaction);
     });
 
     const activeDomain = getActiveDomain();
 
-    activeDomain.on('error', err => {
-      captureEventError(err, context);
-    });
+    activeDomain.on('error', captureException);
 
     const newCallback = activeDomain.bind((...args: unknown[]) => {
       if (args[0] !== null && args[0] !== undefined) {
-        captureEventError(args[0], context);
+        captureException(args[0]);
       }
       transaction.finish();
 

diff --git a/packages/serverless/src/gcpfunction/general.ts b/packages/serverless/src/gcpfunction/general.ts
@@ -1,33 +1,31 @@
 // '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
 // When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
 import { Context } from '@google-cloud/functions-framework/build/src/functions';
-import { captureException, Scope, SDK_VERSION, withScope } from '@sentry/node';
+import { Scope } from '@sentry/node';
 import { Context as SentryContext } from '@sentry/types';
-import { addExceptionMechanism } from '@sentry/utils';
 import * as domain from 'domain';
 import { hostname } from 'os';
 
+import { addServerlessEventProcessor as addServerlessEventProcessorGeneric } from '../utils';
+
 export interface WrapperOptions {
   flushTimeout: number;
 }
 
 /**
- * Capture exception with additional event information.
+ * Enhances the scope with additional event information.
  *
- * @param e exception to be captured
+ * @param scope scope
  * @param context event context
  */
-export function captureEventError(e: unknown, context: Context): void {
-  withScope(scope => {
-    addServerlessEventProcessor(scope);
-    scope.setContext('runtime', {
-      name: 'node',
-      version: global.process.version,
-    });
-    scope.setTag('server_name', process.env.SENTRY_NAME || hostname());
-    scope.setContext('gcp.function.context', { ...context } as SentryContext);
-    captureException(e);
+export function configureScopeWithContext(scope: Scope, context: Context): void {
+  addServerlessEventProcessor(scope);
+  scope.setContext('runtime', {
+    name: 'node',
+    version: global.process.version,
   });
+  scope.setTag('server_name', process.env.SENTRY_NAME || hostname());
+  scope.setContext('gcp.function.context', { ...context } as SentryContext);
 }
 
 /**
@@ -37,27 +35,7 @@ export function captureEventError(e: unknown, context: Context): void {
  * @param scope Scope that processor should be added to
  */
 export function addServerlessEventProcessor(scope: Scope): void {
-  scope.addEventProcessor(event => {
-    event.sdk = {
-      ...event.sdk,
-      name: 'sentry.javascript.serverless',
-      integrations: [...((event.sdk && event.sdk.integrations) || []), 'GCPFunction'],
-      packages: [
-        ...((event.sdk && event.sdk.packages) || []),
-        {
-          name: 'npm:@sentry/serverless',
-          version: SDK_VERSION,
-        },
-      ],
-      version: SDK_VERSION,
-    };
-
-    addExceptionMechanism(event, {
-      handled: false,
-    });
-
-    return event;
-  });
+  addServerlessEventProcessorGeneric(scope, 'GCPFunction');
 }
 
 /**

diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts
@@ -1,7 +1,7 @@
 // '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
 // When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
 import { HttpFunction } from '@google-cloud/functions-framework/build/src/functions';
-import { captureException, flush, getCurrentHub, Handlers, startTransaction, withScope } from '@sentry/node';
+import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
 import { logger, stripUrlQueryAndFragment } from '@sentry/utils';
 
 import { addServerlessEventProcessor, getActiveDomain, WrapperOptions } from './general';
@@ -18,21 +18,6 @@ export { Request, Response };
 
 const { parseRequest } = Handlers;
 
-/**
- * Capture exception with additional request information.
- *
- * @param e exception to be captured
- * @param req incoming request
- * @param options request capture options
- */
-function captureRequestError(e: unknown, req: Request, options: ParseRequestOptions): void {
-  withScope(scope => {
-    addServerlessEventProcessor(scope);
-    scope.addEventProcessor(event => parseRequest(event, req, options));
-    captureException(e);
-  });
-}
-
 /**
  * Wraps an HTTP function handler adding it error capture and tracing capabilities.
  *
@@ -58,8 +43,13 @@ export function wrapHttpFunction(
       op: 'gcp.function.http',
     });
 
-    // We put the transaction on the scope so users can attach children to it
+    // getCurrentHub() is expected to use current active domain as a carrier
+    // since functions-framework creates a domain for each incoming request.
+    // So adding of event processors every time should not lead to memory bloat.
     getCurrentHub().configureScope(scope => {
+      addServerlessEventProcessor(scope);
+      scope.addEventProcessor(event => parseRequest(event, req, options.parseRequestOptions));
+      // We put the transaction on the scope so users can attach children to it
       scope.setSpan(transaction);
     });
 
@@ -71,7 +61,7 @@ export function wrapHttpFunction(
     // functions-framework creates a domain for each incoming request so we take advantage of this fact and add an error handler.
     // BTW this is the only way to catch any exception occured during request lifecycle.
     getActiveDomain().on('error', err => {
-      captureRequestError(err, req, options.parseRequestOptions);
+      captureException(err);
     });
 
     // eslint-disable-next-line @typescript-eslint/unbound-method

diff --git a/packages/serverless/src/utils.ts b/packages/serverless/src/utils.ts
@@ -0,0 +1,33 @@
+import { Scope, SDK_VERSION } from '@sentry/node';
+import { addExceptionMechanism } from '@sentry/utils';
+
+/**
+ * Add event processor that will override SDK details to point to the serverless SDK instead of Node,
+ * as well as set correct mechanism type, which should be set to `handled: false`.
+ * We do it like this, so that we don't introduce any side-effects in this module, which makes it tree-shakeable.
+ * @param scope Scope that processor should be added to
+ * @param integration Name of the serverless integration ('AWSLambda', 'GCPFunction', etc)
+ */
+export function addServerlessEventProcessor(scope: Scope, integration: string): void {
+  scope.addEventProcessor(event => {
+    event.sdk = {
+      ...event.sdk,
+      name: 'sentry.javascript.serverless',
+      integrations: [...((event.sdk && event.sdk.integrations) || []), integration],
+      packages: [
+        ...((event.sdk && event.sdk.packages) || []),
+        {
+          name: 'npm:@sentry/serverless',
+          version: SDK_VERSION,
+        },
+      ],
+      version: SDK_VERSION,
+    };
+
+    addExceptionMechanism(event, {
+      handled: false,
+    });
+
+    return event;
+  });
+}