diff --git a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts index 803484881837..38c6e82043cf 100644 --- a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts +++ b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts @@ -43,7 +43,7 @@ test('AWS Serverless SDK sends events in ESM mode', async ({ request }) => { trace_id: expect.stringMatching(/[a-f0-9]{32}/), }); - expect(transactionEvent.spans).toHaveLength(3); + expect(transactionEvent.spans).toHaveLength(2); // shows that the Otel Http instrumentation is working expect(transactionEvent.spans).toContainEqual( @@ -58,19 +58,6 @@ test('AWS Serverless SDK sends events in ESM mode', async ({ request }) => { }), ); - expect(transactionEvent.spans).toContainEqual( - expect.objectContaining({ - data: { - 'sentry.op': 'function.aws.lambda', - 'sentry.origin': 'auto.function.serverless', - 'sentry.source': 'component', - }, - description: 'my-lambda', - op: 'function.aws.lambda', - origin: 'auto.function.serverless', - }), - ); - // shows that the manual span creation is working expect(transactionEvent.spans).toContainEqual( expect.objectContaining({ diff --git a/packages/aws-serverless/src/sdk.ts b/packages/aws-serverless/src/sdk.ts index 95a23ba514d8..9bad62f3a848 100644 --- a/packages/aws-serverless/src/sdk.ts +++ b/packages/aws-serverless/src/sdk.ts @@ -1,33 +1,24 @@ -import type { Integration, Options, Scope, Span } from '@sentry/core'; -import { - applySdkMetadata, - debug, - getSDKSource, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, -} from '@sentry/core'; +import type { Integration, Options, Scope } from '@sentry/core'; +import { applySdkMetadata, consoleSandbox, debug, getSDKSource } from '@sentry/core'; import type { NodeClient, NodeOptions } from '@sentry/node'; import { captureException, captureMessage, - continueTrace, flush, getCurrentScope, getDefaultIntegrationsWithoutPerformance, initWithoutDefaultIntegrations, - startSpanManual, withScope, } from '@sentry/node'; import type { Context, Handler } from 'aws-lambda'; import { existsSync } from 'fs'; -import { hostname } from 'os'; import { basename, resolve } from 'path'; import { performance } from 'perf_hooks'; import { types } from 'util'; import { DEBUG_BUILD } from './debug-build'; import { awsIntegration } from './integration/aws'; import { awsLambdaIntegration } from './integration/awslambda'; -import { getAwsTraceData, markEventUnhandled } from './utils'; +import { markEventUnhandled } from './utils'; const { isPromise } = types; @@ -54,10 +45,10 @@ export interface WrapperOptions { * @default false */ captureAllSettledReasons: boolean; + // TODO(v11): Remove this option since its no longer used. /** - * Automatically trace all handler invocations. - * You may want to disable this if you use express within Lambda. - * @default true + * @deprecated This option has no effect and will be removed in a future major version. + * If you want to disable tracing, set `SENTRY_TRACES_SAMPLE_RATE` to `0.0`, otherwise OpenTelemetry will automatically trace the handler. */ startTrace: boolean; } @@ -207,18 +198,6 @@ 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. * @@ -231,15 +210,27 @@ export function wrapHandler( wrapOptions: Partial = {}, ): Handler { const START_TIME = performance.now(); + + // eslint-disable-next-line deprecation/deprecation + if (typeof wrapOptions.startTrace !== 'undefined') { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + 'The `startTrace` option is deprecated and will be removed in a future major version. If you want to disable tracing, set `SENTRY_TRACES_SAMPLE_RATE` to `0.0`.', + ); + }); + } + const options: WrapperOptions = { flushTimeout: 2000, callbackWaitsForEmptyEventLoop: false, captureTimeoutWarning: true, timeoutWarningLimit: 500, captureAllSettledReasons: false, - startTrace: true, + startTrace: true, // TODO(v11): Remove this option. Set to true here to satisfy the type, but has no effect. ...wrapOptions, }; + let timeoutWarningTimer: NodeJS.Timeout; // AWSLambda is like Express. It makes a distinction about handlers based on its last argument @@ -293,7 +284,7 @@ export function wrapHandler( }, timeoutWarningDelay) as unknown as NodeJS.Timeout; } - async function processResult(span?: Span): Promise { + async function processResult(): Promise { const scope = getCurrentScope(); let rv: TResult; @@ -314,9 +305,6 @@ export function wrapHandler( throw e; } finally { clearTimeout(timeoutWarningTimer); - if (span?.isRecording()) { - span.end(); - } await flush(options.flushTimeout).catch(e => { DEBUG_BUILD && debug.error(e); }); @@ -324,50 +312,8 @@ export function wrapHandler( return rv; } - // Only start a trace and root span if the handler is not already wrapped by Otel instrumentation - // Otherwise, we create two root spans (one from otel, one from our wrapper). - // If Otel instrumentation didn't work or was filtered by users, we still want to trace the handler. - // TODO: Since bumping the OTEL Instrumentation, this is likely not needed anymore, we can possibly remove this (can be done whenever since it would be non-breaking) - if (options.startTrace && !isWrappedByOtel(handler)) { - const traceData = getAwsTraceData(event as { headers?: Record }, context); - - return continueTrace({ sentryTrace: traceData['sentry-trace'], baggage: traceData.baggage }, () => { - return startSpanManual( - { - name: context.functionName, - op: 'function.aws.lambda', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - }, - span => { - enhanceScopeWithTransactionData(getCurrentScope(), context); - - return processResult(span); - }, - ); - }); - } - return withScope(async () => { - return processResult(undefined); + return processResult(); }); }; } - -/** - * Checks if Otel's AWSLambda instrumentation successfully wrapped the handler. - * Check taken from @opentelemetry/core - */ -function isWrappedByOtel( - // eslint-disable-next-line @typescript-eslint/ban-types - handler: Function & { __original?: unknown; __unwrap?: unknown; __wrapped?: boolean }, -): boolean { - return ( - typeof handler === 'function' && - typeof handler.__original === 'function' && - typeof handler.__unwrap === 'function' && - handler.__wrapped === true - ); -} diff --git a/packages/aws-serverless/test/sdk.test.ts b/packages/aws-serverless/test/sdk.test.ts index ed25b69f49ef..648ef4caeaec 100644 --- a/packages/aws-serverless/test/sdk.test.ts +++ b/packages/aws-serverless/test/sdk.test.ts @@ -1,12 +1,8 @@ import type { Event } from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; import type { Callback, Handler } from 'aws-lambda'; import { beforeEach, describe, expect, test, vi } from 'vitest'; import { init, wrapHandler } from '../src/sdk'; -const mockSpanEnd = vi.fn(); -const mockStartInactiveSpan = vi.fn((...spanArgs) => ({ ...spanArgs })); -const mockStartSpanManual = vi.fn((...spanArgs) => ({ ...spanArgs })); const mockFlush = vi.fn((...args) => Promise.resolve(args)); const mockWithScope = vi.fn(); const mockCaptureMessage = vi.fn(); @@ -28,24 +24,15 @@ vi.mock('@sentry/node', async () => { initWithoutDefaultIntegrations: (options: unknown) => { mockInit(options); }, - startInactiveSpan: (...args: unknown[]) => { - mockStartInactiveSpan(...args); - return { end: mockSpanEnd }; - }, - startSpanManual: (...args: unknown[]) => { - mockStartSpanManual(...args); - mockSpanEnd(); - return original.startSpanManual(...args); - }, getCurrentScope: () => { return mockScope; }, flush: (...args: unknown[]) => { return mockFlush(...args); }, - withScope: (fn: (scope: unknown) => void) => { + withScope: (fn: (scope: unknown) => unknown) => { mockWithScope(fn); - fn(mockScope); + return fn(mockScope); }, captureMessage: (...args: unknown[]) => { mockCaptureMessage(...args); @@ -82,13 +69,6 @@ const fakeCallback: Callback = (err, result) => { }; function expectScopeSettings() { - expect(mockScope.setTransactionName).toBeCalledTimes(1); - expect(mockScope.setTransactionName).toBeCalledWith('functionName'); - - expect(mockScope.setTag).toBeCalledWith('server_name', expect.anything()); - - expect(mockScope.setTag).toBeCalledWith('url', 'awslambda:///functionName'); - expect(mockScope.setContext).toBeCalledWith( 'aws.lambda', expect.objectContaining({ @@ -138,7 +118,7 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(mockWithScope).toBeCalledTimes(1); + expect(mockWithScope).toBeCalledTimes(2); expect(mockCaptureMessage).toBeCalled(); expect(mockScope.setTag).toBeCalledWith('timeout', '1s'); }); @@ -154,7 +134,7 @@ describe('AWSLambda', () => { }); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(mockWithScope).toBeCalledTimes(0); + expect(mockWithScope).toBeCalledTimes(1); expect(mockCaptureMessage).not.toBeCalled(); expect(mockScope.setTag).not.toBeCalledWith('timeout', '1s'); }); @@ -211,25 +191,11 @@ describe('AWSLambda', () => { expect(mockCaptureException).toHaveBeenNthCalledWith(2, error2, expect.any(Function)); expect(mockCaptureException).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); - - expect(mockScope.addEventProcessor).toBeCalledTimes(0); - - expect(mockScope.setTag).toBeCalledTimes(0); - expect(mockStartSpanManual).toBeCalledTimes(0); - }); }); describe('wrapHandler() on sync handler', () => { test('successful execution', async () => { - expect.assertions(10); + expect.assertions(4); const handler: Handler = (_event, _context, callback) => { callback(null, 42); @@ -237,25 +203,14 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler); const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - const fakeTransactionContext = { - name: 'functionName', - op: 'function.aws.lambda', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - }; - expect(rv).toStrictEqual(42); - expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - expect(mockSpanEnd).toBeCalled(); expect(mockFlush).toBeCalledWith(2000); }); test('unsuccessful execution', async () => { - expect.assertions(10); + expect.assertions(4); const error = new Error('sorry'); const handler: Handler = (_event, _context, callback) => { @@ -266,20 +221,9 @@ describe('AWSLambda', () => { try { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); } catch { - const fakeTransactionContext = { - name: 'functionName', - op: 'function.aws.lambda', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - }; - - expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); - expect(mockSpanEnd).toBeCalled(); expect(mockFlush).toBeCalledWith(2000); } }); @@ -297,7 +241,7 @@ describe('AWSLambda', () => { }); test('capture error', async () => { - expect.assertions(10); + expect.assertions(4); const error = new Error('wat'); const handler: Handler = (_event, _context, _callback) => { @@ -308,20 +252,9 @@ describe('AWSLambda', () => { try { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); } catch (e) { - const fakeTransactionContext = { - name: 'functionName', - op: 'function.aws.lambda', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - }; - - expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); expect(mockCaptureException).toBeCalledWith(e, expect.any(Function)); - expect(mockSpanEnd).toBeCalled(); expect(mockFlush).toBeCalled(); } }); @@ -329,7 +262,7 @@ describe('AWSLambda', () => { describe('wrapHandler() on async handler', () => { test('successful execution', async () => { - expect.assertions(10); + expect.assertions(4); const handler: Handler = async (_event, _context) => { return 42; @@ -337,20 +270,9 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler); const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - const fakeTransactionContext = { - name: 'functionName', - op: 'function.aws.lambda', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - }; - expect(rv).toStrictEqual(42); - expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - expect(mockSpanEnd).toBeCalled(); expect(mockFlush).toBeCalled(); }); @@ -366,7 +288,7 @@ describe('AWSLambda', () => { }); test('capture error', async () => { - expect.assertions(10); + expect.assertions(4); const error = new Error('wat'); const handler: Handler = async (_event, _context) => { @@ -377,20 +299,9 @@ describe('AWSLambda', () => { try { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); } catch { - const fakeTransactionContext = { - name: 'functionName', - op: 'function.aws.lambda', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - }; - - expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); - expect(mockSpanEnd).toBeCalled(); expect(mockFlush).toBeCalled(); } }); @@ -411,7 +322,7 @@ describe('AWSLambda', () => { describe('wrapHandler() on async handler with a callback method (aka incorrect usage)', () => { test('successful execution', async () => { - expect.assertions(10); + expect.assertions(4); const handler: Handler = async (_event, _context, _callback) => { return 42; @@ -419,20 +330,9 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler); const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - const fakeTransactionContext = { - name: 'functionName', - op: 'function.aws.lambda', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - }; - expect(rv).toStrictEqual(42); - expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - expect(mockSpanEnd).toBeCalled(); expect(mockFlush).toBeCalled(); }); @@ -448,7 +348,7 @@ describe('AWSLambda', () => { }); test('capture error', async () => { - expect.assertions(10); + expect.assertions(4); const error = new Error('wat'); const handler: Handler = async (_event, _context, _callback) => { @@ -459,20 +359,9 @@ describe('AWSLambda', () => { try { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); } catch { - const fakeTransactionContext = { - name: 'functionName', - op: 'function.aws.lambda', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.serverless', - }, - }; - - expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); - expect(mockSpanEnd).toBeCalled(); expect(mockFlush).toBeCalled(); } }); @@ -492,7 +381,7 @@ describe('AWSLambda', () => { } catch { expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); - const scopeFunction = mockCaptureException.mock.calls[0][1]; + const scopeFunction = mockCaptureException.mock.calls[0]?.[1]; const event: Event = { exception: { values: [{}] } }; let evtProcessor: ((e: Event) => Event) | undefined = undefined; scopeFunction({ addEventProcessor: vi.fn().mockImplementation(proc => (evtProcessor = proc)) });