Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// `node:sqlite`, which `flue` needs, and the Vercel keep-alive, which needs `http.server.response.finish`.
const NODE_ONLY = [
'suites/anr/test.ts',
'suites/aws-lambda-web/**',
'suites/aws-serverless/**',
'suites/breadcrumbs/**',
'suites/child-process/test.ts',
Expand Down
9 changes: 7 additions & 2 deletions dev-packages/deno-integration-tests/node-suites/excludes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@
// fails on Deno runs only on the other runtimes, with `test.runIf(supports({ runtimes }))` in the
// Node suite, and is not listed here.

// Node-only features: ANR and native thread watchdogs, child process and worker thread breadcrumbs.
const NODE_ONLY = ['suites/anr/test.ts', 'suites/breadcrumbs/**', 'suites/thread-blocked-native/test.ts'];
// Node-only features: ANR and native thread watchdogs, child process and worker thread breadcrumbs, and the AWS Lambda Node runtime.
const NODE_ONLY = [
'suites/anr/test.ts',
'suites/aws-lambda-web/**',
'suites/breadcrumbs/**',
'suites/thread-blocked-native/test.ts',
];

// `@sentry/node` instruments `fetch` through undici's diagnostics channels, which Deno's `fetch`
// does not publish. `@sentry/deno` has its own `fetchIntegration` for this. These suites check
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const DEPENDENTS: Dependent[] = [
exports: Object.keys(SentryBun),
ignoreExports: [
// not supported in bun:
'awsLambdaWebIntegration',
'NodeClient',
'NODE_VERSION',
'childProcessIntegration',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

Sentry.init({
dsn: 'https://public@example.com/1',
tracesSampleRate: process.env.NO_TRACING ? undefined : 1,
transport: loggingTransport,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import * as Sentry from '@sentry/node';
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
import express from 'express';

const app = express();
let releaseFirst;
const secondRequestStarted = new Promise(resolve => {
releaseFirst = resolve;
});

app.get('/:request', async (req, res) => {
if (req.params.request === 'first') {
process.stdout.write('FIRST_STARTED\n');
await secondRequestStarted;
} else {
releaseFirst();
}

Sentry.captureException(new Error(req.params.request));
Sentry.logger.info('Request handled');
Sentry.metrics.count('request_count', 1);
res.send({ ok: true });
setImmediate(() => {
void Sentry.flush();
});
});

startExpressServerAndSendPortToRunner(app);
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import {
AWS_LAMBDA_WEB_ENDPOINT_NAME,
CLOUD_PLATFORM,
CLOUD_PROVIDER,
CLOUD_REGION,
FAAS_COLDSTART,
FAAS_INVOCATION_ID,
FAAS_MAX_MEMORY,
SENTRY_OP,
} from '@sentry/conventions/attributes';
import { expect, vi } from 'vitest';
import { createEsmAndCjsTests } from '../../../utils/runner';

createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: I rarely critique style, so sorry in advance 😅 I think this test could benefit from a few line breaks to make it a bit easier to understand what belongs directly together.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's very fair, i added a few line breaks 😄

test.each([
{ name: 'default tracing', tracing: true },
{ name: 'tracing disabled', tracing: false },
])('$name: isolates concurrent requests and leaves logs and metrics unenriched', async ({ tracing }) => {
const runner = createRunner()
.withEnv({
AWS_LAMBDA_HTTP_ENDPOINT: '0.0.0.0:3000',
AWS_REGION: 'us-east-1',
AWS_LAMBDA_FUNCTION_MEMORY_SIZE: '2048',
AWS_LAMBDA_INITIALIZATION_TYPE: 'native-http',
AWS_EXECUTION_ENV: 'AWS_Lambda_nodejs24.x-lite',
NO_TRACING: tracing ? '' : '1',
})
.unordered();

const spans = [];

for (const request of ['first', 'second']) {
runner.expect({
event: {
exception: { values: [expect.objectContaining({ value: request })] },
contexts: {
cloud_resource: {
[CLOUD_PROVIDER]: 'aws',
[CLOUD_PLATFORM]: 'aws_lambda',
[CLOUD_REGION]: 'us-east-1',
},
'aws.lambda': {
memory_size: 2_147_483_648,
initialization_type: 'native-http',
cold_start: request === 'first',
aws_request_id: request,
endpoint_name: `${request}-endpoint`,
},
},
},
});

if (tracing) {
spans.push({
request,
collected: runner.collectStreamedSpansUntilSegment(
span => span.attributes[FAAS_INVOCATION_ID]?.value === request,
),
});
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved
}
}

const assertUnenriched = (items: { attributes?: Record<string, unknown> }[]): void => {
expect(items).toHaveLength(2);
for (const item of items) {
expect(Object.keys(item.attributes || {}).filter(key => /^(aws\.lambda\.|faas\.|cloud\.)/.test(key))).toEqual(
[],
);
}
};

runner.expect({ log: container => assertUnenriched(container.items) });
runner.expect({ trace_metric: container => assertUnenriched(container.items) });

const running = runner.start();
const first = running.makeRequest('get', '/first', {
headers: { 'lambda-web-request-id': 'first', 'lambda-web-endpoint-name': 'first-endpoint' },
});
await vi.waitFor(() => expect(running.getLogs().join('\n')).toContain('FIRST_STARTED'), { timeout: 10_000 });

await Promise.all([
first,
running.makeRequest('get', '/second', {
headers: { 'lambda-web-request-id': 'second', 'lambda-web-endpoint-name': 'second-endpoint' },
}),
]);
await running.completed();

const segmentSpans = [];

for (const { request, collected } of spans) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: should we assert on the number of segment spans / traces? I believe it should be 2, correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes should be 2, added the assertion

const items = await collected;
segmentSpans.push(...items.filter(item => item.is_segment));

expect(items.find(item => item.is_segment)).toMatchObject({
name: 'GET /:request',
attributes: {
[SENTRY_OP]: { type: 'string', value: 'http.server' },
[CLOUD_PLATFORM]: { type: 'string', value: 'aws_lambda' },
[CLOUD_REGION]: { type: 'string', value: 'us-east-1' },
[FAAS_MAX_MEMORY]: { type: 'integer', value: 2_147_483_648 },
[FAAS_COLDSTART]: { type: 'boolean', value: request === 'first' },
[FAAS_INVOCATION_ID]: { type: 'string', value: request },
[AWS_LAMBDA_WEB_ENDPOINT_NAME]: { type: 'string', value: `${request}-endpoint` },
},
});
}

expect(segmentSpans).toHaveLength(tracing ? 2 : 0);
});
});
1 change: 1 addition & 0 deletions packages/astro/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export {
nativeNodeFetchIntegration,
NodeClient,
nodeContextIntegration,
awsLambdaWebIntegration,
onUncaughtExceptionIntegration,
onUnhandledRejectionIntegration,
mistralAIIntegration,
Expand Down
1 change: 1 addition & 0 deletions packages/aws-serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export {
type NodeRuntimeMetricsOptions,
contextLinesIntegration,
nodeContextIntegration,
awsLambdaWebIntegration,
localVariablesIntegration,
requestDataIntegration,
functionToStringIntegration,
Expand Down
1 change: 1 addition & 0 deletions packages/google-cloud-serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export {
type NodeRuntimeMetricsOptions,
contextLinesIntegration,
nodeContextIntegration,
awsLambdaWebIntegration,
localVariablesIntegration,
requestDataIntegration,
fsIntegration,
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { httpIntegration } from './integrations/http';
export { awsLambdaWebIntegration } from './integrations/awsLambdaWeb';
export { nativeNodeFetchIntegration } from './integrations/node-fetch';
export { fsIntegration } from './integrations/fs';
// oxlint-disable-next-line typescript/no-deprecated
Expand Down
76 changes: 76 additions & 0 deletions packages/node/src/integrations/awsLambdaWeb.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import {
AWS_LAMBDA_INITIALIZATION_TYPE,
AWS_LAMBDA_WEB_ENDPOINT_NAME,
FAAS_COLDSTART,
FAAS_INVOCATION_ID,
FAAS_MAX_MEMORY,
} from '@sentry/conventions/attributes';
import type { IntegrationFn, SpanAttributes } from '@sentry/core';
import { defineIntegration, getIsolationScope, safeSetSpanJSONAttributes } from '@sentry/core';

const INTEGRATION_NAME = 'AwsLambdaWeb';

let requestHandled = false;

const _awsLambdaWebIntegration = (() => {
// Lambda Web Functions run ordinary HTTP servers without a Lambda handler to wrap.
// We read their request metadata through the `httpServerRequest` hook. Since this hook
// runs for all HTTP servers, we bail out early when we are not in a Lambda Web Function.
// https://docs.aws.amazon.com/lambda/latest/dg/web-functions-revisions-env-vars.html
if (!process.env.AWS_LAMBDA_HTTP_ENDPOINT) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add a comment here about the logic here. which is, if I understand correctly, that we cannot hook into this any better, so this has to be handled via client.on('httpServerRequest'), which would run for each server. so in order to avoid the overhead of having this run everywhere, we opt out here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes exactly, it's just a regular HTTP server with the request metadata living in request headers. added a comment to explain.

return { name: INTEGRATION_NAME };
}

const memory = Number(process.env.AWS_LAMBDA_FUNCTION_MEMORY_SIZE) * 1024 * 1024;
const lambdaContext = {
memory_size: Number.isFinite(memory) && memory > 0 ? memory : undefined,
initialization_type: process.env.AWS_LAMBDA_INITIALIZATION_TYPE,
};
const environmentAttributes = {
[FAAS_MAX_MEMORY]: lambdaContext.memory_size,
[AWS_LAMBDA_INITIALIZATION_TYPE]: lambdaContext.initialization_type,
};

return {
name: INTEGRATION_NAME,
setup(client) {
client.on('httpServerRequest', (_request, _response, request) => {
const coldStart = !requestHandled;
requestHandled = true;
const requestId = request.headers?.['lambda-web-request-id'];
const endpointName = request.headers?.['lambda-web-endpoint-name'];
const scope = getIsolationScope();
scope.setContext('aws.lambda', {
...scope.getScopeData().contexts['aws.lambda'],
cold_start: coldStart,
aws_request_id: typeof requestId === 'string' ? requestId : undefined,
endpoint_name: typeof endpointName === 'string' ? endpointName : undefined,
});
});
},
processEvent(event) {
event.contexts = {
...event.contexts,
'aws.lambda': { ...lambdaContext, ...event.contexts?.['aws.lambda'] },
};
return event;
},
processSegmentSpan(span) {
const requestContext = getIsolationScope().getScopeData().contexts['aws.lambda'];
const attributes: SpanAttributes = { ...environmentAttributes };
if (typeof requestContext?.cold_start === 'boolean') {
attributes[FAAS_COLDSTART] = requestContext.cold_start;
}
if (typeof requestContext?.aws_request_id === 'string') {
attributes[FAAS_INVOCATION_ID] = requestContext.aws_request_id;
}
if (typeof requestContext?.endpoint_name === 'string') {
attributes[AWS_LAMBDA_WEB_ENDPOINT_NAME] = requestContext.endpoint_name;
}
safeSetSpanJSONAttributes(span, attributes);
},
};
}) satisfies IntegrationFn;

/** Enrich AWS Lambda Web Function spans and error contexts with platform metadata. */
export const awsLambdaWebIntegration = defineIntegration(_awsLambdaWebIntegration);
3 changes: 2 additions & 1 deletion packages/node/src/integrations/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,11 @@ function getCloudResourceContext(): CloudResourceContext | undefined {
};
} else if (process.env.AWS_REGION) {
// https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html
const executionEnv = process.env.AWS_EXECUTION_ENV;
return {
[CLOUD_PROVIDER]: 'aws',
[CLOUD_REGION]: process.env.AWS_REGION,
[CLOUD_PLATFORM]: process.env.AWS_EXECUTION_ENV,
[CLOUD_PLATFORM]: executionEnv?.startsWith('AWS_Lambda') ? 'aws_lambda' : executionEnv,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silent cloud.platform value change

Medium Severity

nodeContextIntegration now writes cloud.platform as aws_lambda whenever AWS_EXECUTION_ENV starts with AWS_Lambda, instead of the previous raw value such as AWS_Lambda_nodejs24.x. This applies to every Node Lambda app, not only Web Functions, so existing queries and alerts keyed on the old value can stop matching, and the diff has no migration note. Flagged because the review rules call out silent payload changes without a migration callout.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit bf3b43e. Configure here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like a reasonable comment to look into I'd say!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's fair. I laid out here why I think just aws_lambda is the correct value here in all cases, not just for web functions. Do you think we can get away by labeling this as a fix (to align with conventions/otel)? Otherwise we can do it for web functions only now and fix it for other Node apps in v12.

@msonnb msonnb Oct 29, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

briefly discussed with @Lms24 offline and we agreed that it's okay to fix this here. OTel conventions explicitly say that

cloud.platform has the following list of well-known values [list includes aws_lambda]. If one of them applies, then the respective value MUST be used; otherwise, a custom value MAY be used.

};
} else if (process.env.GCP_PROJECT) {
// https://cloud.google.com/composer/docs/how-to/managing/environment-variables#reserved_variables
Expand Down
2 changes: 2 additions & 0 deletions packages/node/src/sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import { isMainThread, parentPort } from 'node:worker_threads';
import { detectOrchestrionSetup, getErrorIntegrations, getTracingIntegrations } from '@sentry/server-utils';
import { DEBUG_BUILD } from '../debug-build';
import { awsLambdaWebIntegration } from '../integrations/awsLambdaWeb';
import { childProcessIntegration } from '../integrations/childProcess';
import { consoleIntegration } from '../integrations/console';
import { nodeContextIntegration } from '../integrations/context';
Expand Down Expand Up @@ -66,6 +67,7 @@ function getBaseDefaultIntegrations(): Integration[] {
// Event Info
contextLinesIntegration(),
localVariablesIntegration(),
awsLambdaWebIntegration(),
nodeContextIntegration(),
childProcessIntegration(),
workerThreadsIntegration(),
Expand Down
Loading
Loading