fix(aws-serverless): Only start root span in Sentry wrapper if Otel didn't wrap handler

.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',

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 };

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);

dev-packages/e2e-tests/test-applications/aws-lambda-layer/src/run.js → 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',

dev-packages/e2e-tests/test-applications/aws-lambda-layer/start-event-proxy.mjs → 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,
 });

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<void>(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',
+    }),
+  );
+});

packages/aws-serverless/src/sdk.ts

@@ -320,15 +320,20 @@ export function wrapHandler<TEvent, TResult>(
         throw e;
       } finally {
         clearTimeout(timeoutWarningTimer);
-        span?.end();
+        if (span && span.isRecording()) {
+          span.end();
+        }
         await flush(options.flushTimeout).catch(e => {
           DEBUG_BUILD && logger.error(e);
         });
       }
       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<TEvent, TResult>(
     });
   };
 }
+
+/**
+ * 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
+  );
+}