diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 35fb1d8d1436..c3bda957409b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -993,7 +993,7 @@ jobs: [ 'angular-17', 'angular-18', - 'aws-lambda-layer', + 'aws-lambda-layer-cjs', 'cloudflare-astro', 'node-express', 'create-react-app', diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/.npmrc b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/.npmrc similarity index 100% rename from dev-packages/e2e-tests/test-applications/aws-lambda-layer/.npmrc rename to dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/.npmrc diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/package.json b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json similarity index 100% rename from dev-packages/e2e-tests/test-applications/aws-lambda-layer/package.json rename to dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/playwright.config.ts b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/playwright.config.ts similarity index 100% rename from dev-packages/e2e-tests/test-applications/aws-lambda-layer/playwright.config.ts rename to dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/playwright.config.ts diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/lambda-function.js b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/lambda-function.js new file mode 100644 index 000000000000..c688ed35a0c4 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/lambda-function.js @@ -0,0 +1,21 @@ +const Sentry = require('@sentry/aws-serverless'); + +const http = require('http'); + +async function handle() { + await Sentry.startSpan({ name: 'manual-span', op: 'test' }, async () => { + await new Promise(resolve => { + http.get('http://example.com', res => { + res.on('data', d => { + process.stdout.write(d); + }); + + res.on('end', () => { + resolve(); + }); + }); + }); + }); +} + +module.exports = { handle }; diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/run-lambda.js b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/run-lambda.js new file mode 100644 index 000000000000..1d6e059e78f3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/run-lambda.js @@ -0,0 +1,7 @@ +const { handle } = require('./lambda-function'); +const event = {}; +const context = { + invokedFunctionArn: 'arn:aws:lambda:us-east-1:123453789012:function:my-lambda', + functionName: 'my-lambda', +}; +handle(event, context); diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/run.js b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/run.js similarity index 64% rename from dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/run.js rename to dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/run.js index 2a99cff2d48e..2605f624ca9a 100644 --- a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/run.js +++ b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/src/run.js @@ -4,8 +4,9 @@ child_process.execSync('node ./src/run-lambda.js', { stdio: 'inherit', env: { ...process.env, - LAMBDA_TASK_ROOT: '.', - _HANDLER: 'handle', + // On AWS, LAMBDA_TASK_ROOT is usually /var/task but for testing, we set it to the CWD to correctly apply our handler + LAMBDA_TASK_ROOT: process.cwd(), + _HANDLER: 'src/lambda-function.handle', NODE_OPTIONS: '--require @sentry/aws-serverless/dist/awslambda-auto', SENTRY_DSN: 'http://public@localhost:3031/1337', diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/start-event-proxy.mjs similarity index 71% rename from dev-packages/e2e-tests/test-applications/aws-lambda-layer/start-event-proxy.mjs rename to dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/start-event-proxy.mjs index e64e99cda75b..abc7ea7b0ab2 100644 --- a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/start-event-proxy.mjs +++ b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/start-event-proxy.mjs @@ -2,6 +2,6 @@ import { startEventProxyServer } from '@sentry-internal/test-utils'; startEventProxyServer({ port: 3031, - proxyServerName: 'aws-serverless-lambda-layer', + proxyServerName: 'aws-serverless-lambda-layer-cjs', forwardToSentry: false, }); diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/tests/basic.test.ts b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/tests/basic.test.ts new file mode 100644 index 000000000000..a1211c0f11bc --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/tests/basic.test.ts @@ -0,0 +1,69 @@ +import * as child_process from 'child_process'; +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('Lambda layer SDK bundle sends events', async ({ request }) => { + const transactionEventPromise = waitForTransaction('aws-serverless-lambda-layer-cjs', transactionEvent => { + return transactionEvent?.transaction === 'my-lambda'; + }); + + // Waiting for 1s here because attaching the listener for events in `waitForTransaction` is not synchronous + // Since in this test, we don't start a browser via playwright, we don't have the usual delays (page.goto, etc) + // which are usually enough for us to never have noticed this race condition before. + // This is a workaround but probably sufficient as long as we only experience it in this test. + await new Promise(resolve => + setTimeout(() => { + resolve(); + }, 1000), + ); + + child_process.execSync('pnpm start', { + stdio: 'ignore', + }); + + const transactionEvent = await transactionEventPromise; + + // shows the SDK sent a transaction + expect(transactionEvent.transaction).toEqual('my-lambda'); // name should be the function name + expect(transactionEvent.contexts?.trace).toEqual({ + data: { + 'sentry.sample_rate': 1, + 'sentry.source': 'custom', + 'sentry.origin': 'auto.otel.aws-lambda', + 'cloud.account.id': '123453789012', + 'faas.id': 'arn:aws:lambda:us-east-1:123453789012:function:my-lambda', + 'otel.kind': 'SERVER', + }, + origin: 'auto.otel.aws-lambda', + span_id: expect.any(String), + status: 'ok', + trace_id: expect.any(String), + }); + + expect(transactionEvent.spans).toHaveLength(2); + + // shows that the Otel Http instrumentation is working + expect(transactionEvent.spans).toContainEqual( + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.otel.http', + url: 'http://example.com/', + }), + description: 'GET http://example.com/', + op: 'http.client', + }), + ); + + // shows that the manual span creation is working + expect(transactionEvent.spans).toContainEqual( + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.op': 'test', + 'sentry.origin': 'manual', + }), + description: 'manual-span', + op: 'test', + }), + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/lambda-function.js b/dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/lambda-function.js deleted file mode 100644 index aa8f236b742d..000000000000 --- a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/lambda-function.js +++ /dev/null @@ -1,19 +0,0 @@ -const Sentry = require('@sentry/aws-serverless'); - -const http = require('http'); - -function handle() { - Sentry.startSpanManual({ name: 'aws-lambda-layer-test-txn', op: 'test' }, span => { - http.get('http://example.com', res => { - res.on('data', d => { - process.stdout.write(d); - }); - - res.on('end', () => { - span.end(); - }); - }); - }); -} - -module.exports = { handle }; diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/run-lambda.js b/dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/run-lambda.js deleted file mode 100644 index 5e573c484637..000000000000 --- a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/run-lambda.js +++ /dev/null @@ -1,2 +0,0 @@ -const { handle } = require('./lambda-function'); -handle(); diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/tests/basic.test.ts b/dev-packages/e2e-tests/test-applications/aws-lambda-layer/tests/basic.test.ts deleted file mode 100644 index 7f7f5ec1854c..000000000000 --- a/dev-packages/e2e-tests/test-applications/aws-lambda-layer/tests/basic.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import * as child_process from 'child_process'; -import { expect, test } from '@playwright/test'; -import { waitForTransaction } from '@sentry-internal/test-utils'; - -test('Lambda layer SDK bundle sends events', async ({ request }) => { - const transactionEventPromise = waitForTransaction('aws-serverless-lambda-layer', transactionEvent => { - return transactionEvent?.transaction === 'aws-lambda-layer-test-txn'; - }); - - await new Promise(resolve => - setTimeout(() => { - resolve(); - }, 1000), - ); - - child_process.execSync('pnpm start', { - stdio: 'ignore', - }); - - const transactionEvent = await transactionEventPromise; - - // shows the SDK sent a transaction - expect(transactionEvent.transaction).toEqual('aws-lambda-layer-test-txn'); - - // shows that the Otel Http instrumentation is working - expect(transactionEvent.spans).toHaveLength(1); - expect(transactionEvent.spans![0]).toMatchObject({ - data: expect.objectContaining({ - 'sentry.op': 'http.client', - 'sentry.origin': 'auto.http.otel.http', - url: 'http://example.com/', - }), - description: 'GET http://example.com/', - op: 'http.client', - }); -}); diff --git a/packages/aws-serverless/src/sdk.ts b/packages/aws-serverless/src/sdk.ts index 7c491fefc008..905672241ffb 100644 --- a/packages/aws-serverless/src/sdk.ts +++ b/packages/aws-serverless/src/sdk.ts @@ -320,7 +320,9 @@ export function wrapHandler( throw e; } finally { clearTimeout(timeoutWarningTimer); - span?.end(); + if (span && span.isRecording()) { + span.end(); + } await flush(options.flushTimeout).catch(e => { DEBUG_BUILD && logger.error(e); }); @@ -328,7 +330,10 @@ export function wrapHandler( return rv; } - if (options.startTrace) { + // 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. + if (options.startTrace && !isWrappedByOtel(handler)) { const eventWithHeaders = event as { headers?: { [key: string]: string } }; const sentryTrace = @@ -361,3 +366,19 @@ export function wrapHandler( }); }; } + +/** + * 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 + ); +}