Skip to content

Commit

Permalink
feat(serverlesss): allow disabling transaction traces (#9154)
Browse files Browse the repository at this point in the history
## Background
- My team runs Express inside Lambda - we have a monolithic Lambda
function with express handling routing, middleware, etc
- We are using `@sentry/node` today, and tried adding
`@sentry/serverless` for it's timeout warning, auto-handling of `flush`,
and links to cloudwatch logs
- But this caused all of our performance transaction traces to have an
incorrect name (and various other tag + context issues)

## Proposal
- A new `startTrace` boolean flag that turns on/off the
transaction tracing feature of `@sentry/serverless`
- This will allow our team to use `@sentry/serverless`, but still rely
on express `tracingHandler` for performance traces
- This also follows the general approach of the serverless package,
where each feature is gated by a wrapOptions flag
  • Loading branch information
aldenquimby authored Oct 12, 2023
1 parent e507110 commit 28b6d75
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 37 deletions.
80 changes: 50 additions & 30 deletions packages/serverless/src/awslambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ export interface WrapperOptions {
* @default false
*/
captureAllSettledReasons: boolean;
/**
* Automatically trace all handler invocations.
* You may want to disable this if you use express within Lambda (use tracingHandler instead).
* @default true
*/
startTrace: boolean;
}

export const defaultIntegrations: Integration[] = [...Sentry.defaultIntegrations, new AWSServices({ optional: true })];
Expand Down Expand Up @@ -178,11 +184,6 @@ function tryGetRemainingTimeInMillis(context: Context): number {
* @param startTime performance.now() when wrapHandler was invoked
*/
function enhanceScopeWithEnvironmentData(scope: Scope, context: Context, startTime: number): void {
scope.setTransactionName(context.functionName);

scope.setTag('server_name', process.env._AWS_XRAY_DAEMON_ADDRESS || process.env.SENTRY_NAME || hostname());
scope.setTag('url', `awslambda:///${context.functionName}`);

scope.setContext('aws.lambda', {
aws_request_id: context.awsRequestId,
function_name: context.functionName,
Expand All @@ -204,6 +205,18 @@ function enhanceScopeWithEnvironmentData(scope: Scope, context: Context, startTi
});
}

/**
* Adds additional transaction-related information from the environment and AWS Context to the Sentry Scope.
*
* @param scope Scope that should be enhanced
* @param context AWS Lambda context that will be used to extract some part of the data
*/
function enhanceScopeWithTransactionData(scope: Scope, context: Context): void {
scope.setTransactionName(context.functionName);
scope.setTag('server_name', process.env._AWS_XRAY_DAEMON_ADDRESS || process.env.SENTRY_NAME || hostname());
scope.setTag('url', `awslambda:///${context.functionName}`);
}

/**
* Wraps a lambda handler adding it error capture and tracing capabilities.
*
Expand All @@ -222,6 +235,7 @@ export function wrapHandler<TEvent, TResult>(
captureTimeoutWarning: true,
timeoutWarningLimit: 500,
captureAllSettledReasons: false,
startTrace: true,
...wrapOptions,
};
let timeoutWarningTimer: NodeJS.Timeout;
Expand Down Expand Up @@ -279,36 +293,42 @@ export function wrapHandler<TEvent, TResult>(

const hub = getCurrentHub();

const eventWithHeaders = event as { headers?: { [key: string]: string } };

const sentryTrace =
eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])
? eventWithHeaders.headers['sentry-trace']
: undefined;
const baggage = eventWithHeaders.headers?.baggage;
const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
sentryTrace,
baggage,
);
hub.getScope().setPropagationContext(propagationContext);

const transaction = hub.startTransaction({
name: context.functionName,
op: 'function.aws.lambda',
origin: 'auto.function.serverless',
...traceparentData,
metadata: {
dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
source: 'component',
},
}) as Sentry.Transaction | undefined;
let transaction: Sentry.Transaction | undefined;
if (options.startTrace) {
const eventWithHeaders = event as { headers?: { [key: string]: string } };

const sentryTrace =
eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])
? eventWithHeaders.headers['sentry-trace']
: undefined;
const baggage = eventWithHeaders.headers?.baggage;
const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
sentryTrace,
baggage,
);
hub.getScope().setPropagationContext(propagationContext);

transaction = hub.startTransaction({
name: context.functionName,
op: 'function.aws.lambda',
origin: 'auto.function.serverless',
...traceparentData,
metadata: {
dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
source: 'component',
},
});
}

const scope = hub.pushScope();
let rv: TResult;
try {
enhanceScopeWithEnvironmentData(scope, context, START_TIME);
// We put the transaction on the scope so users can attach children to it
scope.setSpan(transaction);
if (options.startTrace) {
enhanceScopeWithTransactionData(scope, context);
// We put the transaction on the scope so users can attach children to it
scope.setSpan(transaction);
}
rv = await asyncHandler(event, context);

// We manage lambdas that use Promise.allSettled by capturing the errors of failed promises
Expand Down
1 change: 1 addition & 0 deletions packages/serverless/test/__mocks__/@sentry/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const resetMocks = (): void => {
fakeHub.pushScope.mockClear();
fakeHub.popScope.mockClear();
fakeHub.getScope.mockClear();
fakeHub.startTransaction.mockClear();

fakeScope.addEventProcessor.mockClear();
fakeScope.setTransactionName.mockClear();
Expand Down
32 changes: 25 additions & 7 deletions packages/serverless/test/awslambda.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ function expectScopeSettings(fakeTransactionContext: any) {
// @ts-expect-error see "Why @ts-expect-error" note
const fakeTransaction = { ...SentryNode.fakeTransaction, ...fakeTransactionContext };
// @ts-expect-error see "Why @ts-expect-error" note
expect(SentryNode.fakeScope.setTransactionName).toBeCalledWith('functionName');
// @ts-expect-error see "Why @ts-expect-error" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
// @ts-expect-error see "Why @ts-expect-error" note
expect(SentryNode.fakeScope.setTag).toBeCalledWith('server_name', expect.anything());
Expand Down Expand Up @@ -180,11 +182,27 @@ describe('AWSLambda', () => {
expect(SentryNode.captureException).toHaveBeenNthCalledWith(2, error2, expect.any(Function));
expect(SentryNode.captureException).toBeCalledTimes(2);
});

// "wrapHandler() ... successful execution" tests the default of startTrace enabled
test('startTrace disabled', async () => {
expect.assertions(3);

const handler: Handler = async (_event, _context) => 42;
const wrappedHandler = wrapHandler(handler, { startTrace: false });
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);

// @ts-expect-error see "Why @ts-expect-error" note
expect(SentryNode.fakeScope.setTransactionName).toBeCalledTimes(0);
// @ts-expect-error see "Why @ts-expect-error" note
expect(SentryNode.fakeScope.setTag).toBeCalledTimes(0);
// @ts-expect-error see "Why @ts-expect-error" note
expect(SentryNode.fakeHub.startTransaction).toBeCalledTimes(0);
});
});

describe('wrapHandler() on sync handler', () => {
test('successful execution', async () => {
expect.assertions(9);
expect.assertions(10);

const handler: Handler = (_event, _context, callback) => {
callback(null, 42);
Expand All @@ -209,7 +227,7 @@ describe('AWSLambda', () => {
});

test('unsuccessful execution', async () => {
expect.assertions(9);
expect.assertions(10);

const error = new Error('sorry');
const handler: Handler = (_event, _context, callback) => {
Expand Down Expand Up @@ -284,7 +302,7 @@ describe('AWSLambda', () => {
});

test('capture error', async () => {
expect.assertions(9);
expect.assertions(10);

const error = new Error('wat');
const handler: Handler = (_event, _context, _callback) => {
Expand Down Expand Up @@ -319,7 +337,7 @@ describe('AWSLambda', () => {

describe('wrapHandler() on async handler', () => {
test('successful execution', async () => {
expect.assertions(9);
expect.assertions(10);

const handler: Handler = async (_event, _context) => {
return 42;
Expand Down Expand Up @@ -355,7 +373,7 @@ describe('AWSLambda', () => {
});

test('capture error', async () => {
expect.assertions(9);
expect.assertions(10);

const error = new Error('wat');
const handler: Handler = async (_event, _context) => {
Expand Down Expand Up @@ -401,7 +419,7 @@ describe('AWSLambda', () => {

describe('wrapHandler() on async handler with a callback method (aka incorrect usage)', () => {
test('successful execution', async () => {
expect.assertions(9);
expect.assertions(10);

const handler: Handler = async (_event, _context, _callback) => {
return 42;
Expand Down Expand Up @@ -437,7 +455,7 @@ describe('AWSLambda', () => {
});

test('capture error', async () => {
expect.assertions(9);
expect.assertions(10);

const error = new Error('wat');
const handler: Handler = async (_event, _context, _callback) => {
Expand Down

0 comments on commit 28b6d75

Please sign in to comment.