Repository navigation
feat(node): Add awsLambdaWebIntegration for Lambda Web Functions
#25355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6fb87da
17c9712
9c4fff8
df7d78c
50ac1de
b82dd4b
d10c959
568bc15
2e962af
e9b8247
cdf454e
230b31c
c0111af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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) => { | ||
| 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, | ||
| ), | ||
| }); | ||
|
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) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
| }); | ||
| }); | ||
| 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) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Silent cloud.platform value changeMedium Severity
Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit bf3b43e. Configure here.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like a reasonable comment to look into I'd say!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, that's fair. I laid out here why I think just
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
| }; | ||
| } else if (process.env.GCP_PROJECT) { | ||
| // https://cloud.google.com/composer/docs/how-to/managing/environment-variables#reserved_variables | ||
|
|
||


There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 😄