From 25f51f3c8e03969842f5a58e190d43c3fefd1634 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Tue, 16 Feb 2021 08:49:27 +0100 Subject: [PATCH 1/6] Applying sentry-trace in AWSLambdaIntegration transaction --- packages/serverless/src/awslambda.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 0c915cd19e25..5908c317c3b6 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -10,7 +10,8 @@ import { } from '@sentry/node'; import * as Sentry from '@sentry/node'; import { Integration } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { logger, isString } from '@sentry/utils'; +import { extractTraceparentData } from '@sentry/tracing'; // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil // eslint-disable-next-line import/no-unresolved import { Context, Handler } from 'aws-lambda'; @@ -112,7 +113,7 @@ export function tryPatchHandler(taskRoot: string, handlerPath: string): void { let mod: HandlerBag; let functionName: string | undefined; - handlerName.split('.').forEach(name => { + handlerName.split('.').forEach((name) => { mod = obj; obj = obj && (obj as HandlerModule)[name]; functionName = name; @@ -235,16 +236,22 @@ export function wrapHandler( const timeoutWarningDelay = context.getRemainingTimeInMillis() - options.timeoutWarningLimit; timeoutWarningTimer = setTimeout(() => { - withScope(scope => { + withScope((scope) => { scope.setTag('timeout', humanReadableTimeout); captureMessage(`Possible function timeout: ${context.functionName}`, Severity.Warning); }); }, timeoutWarningDelay); } + // Applying `sentry-trace` to context + let traceparentData; + if (event.headers && isString(event.headers['sentry-trace'])) { + traceparentData = extractTraceparentData(event.headers['sentry-trace'] as string); + } const transaction = startTransaction({ name: context.functionName, op: 'awslambda.handler', + ...traceparentData, }); const hub = getCurrentHub(); From c80ad0e50898116bcdb5799f5c330d2d22dff490 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Tue, 16 Feb 2021 11:37:02 +0100 Subject: [PATCH 2/6] Propagate trace in AWSLambda functions --- packages/serverless/src/awslambda.ts | 5 +++-- packages/serverless/test/awslambda.test.ts | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 5908c317c3b6..00de188c80eb 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -245,8 +245,9 @@ export function wrapHandler( // Applying `sentry-trace` to context let traceparentData; - if (event.headers && isString(event.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(event.headers['sentry-trace'] as string); + const eventWithHeaders = event as { headers?: { [key: string]: string } }; + if (eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])) { + traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace'] as string); } const transaction = startTransaction({ name: context.functionName, diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts index 003e357fca59..23bbd60a0fc6 100644 --- a/packages/serverless/test/awslambda.test.ts +++ b/packages/serverless/test/awslambda.test.ts @@ -16,9 +16,7 @@ const { wrapHandler } = Sentry.AWSLambda; // Default `timeoutWarningLimit` is 500ms so leaving some space for it to trigger when necessary const DEFAULT_EXECUTION_TIME = 100; -const fakeEvent = { - fortySix: 'o_O', -}; +let fakeEvent: { [key: string]: unknown }; const fakeContext = { callbackWaitsForEmptyEventLoop: false, functionName: 'functionName', @@ -72,6 +70,12 @@ function expectScopeSettings() { } describe('AWSLambda', () => { + beforeEach(() => { + fakeEvent = { + fortySix: 'o_O', + }; + }); + afterEach(() => { // @ts-ignore see "Why @ts-ignore" note Sentry.resetMocks(); @@ -228,9 +232,16 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler); try { + fakeEvent.headers = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' }; await wrappedHandler(fakeEvent, fakeContext, fakeCallback); } catch (e) { - expect(Sentry.startTransaction).toBeCalledWith({ name: 'functionName', op: 'awslambda.handler' }); + expect(Sentry.startTransaction).toBeCalledWith({ + name: 'functionName', + op: 'awslambda.handler', + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + }); expectScopeSettings(); expect(Sentry.captureException).toBeCalledWith(e); // @ts-ignore see "Why @ts-ignore" note From 1b0b13769502022ebb9fc0af94f2402f52224d4d Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Tue, 16 Feb 2021 12:38:46 +0100 Subject: [PATCH 3/6] Propagate trace handlers in GCP http functions --- packages/serverless/src/gcpfunction/http.ts | 10 ++++++++- packages/serverless/test/gcpfunction.test.ts | 22 ++++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 229295f8d77d..64fd29f4e2ab 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -1,5 +1,6 @@ import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; -import { logger, stripUrlQueryAndFragment } from '@sentry/utils'; +import { logger, stripUrlQueryAndFragment, isString } from '@sentry/utils'; +import { extractTraceparentData } from '@sentry/tracing'; import { domainify, getActiveDomain, proxyFunction } from './../utils'; import { HttpFunction, WrapperOptions } from './general'; @@ -48,9 +49,16 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial { afterEach(() => { @@ -26,13 +27,17 @@ describe('GCPFunction', () => { Sentry.resetMocks(); }); - async function handleHttp(fn: HttpFunction): Promise { + async function handleHttp(fn: HttpFunction, trace_headers: objectOfStrings | null = null): Promise { + let headers: objectOfStrings = { host: 'hostname', 'content-type': 'application/json' }; + if (trace_headers) { + headers = { ...headers, ...trace_headers }; + } return new Promise((resolve, _reject) => { const d = domain.create(); const req = { method: 'POST', url: '/path?q=query', - headers: { host: 'hostname', 'content-type': 'application/json' }, + headers: headers, body: { foo: 'bar' }, } as Request; const res = { end: resolve } as Response; @@ -124,8 +129,17 @@ describe('GCPFunction', () => { throw error; }; const wrappedHandler = wrapHttpFunction(handler); - await handleHttp(wrappedHandler); - expect(Sentry.startTransaction).toBeCalledWith({ name: 'POST /path', op: 'gcp.function.http' }); + + const trace_headers: objectOfStrings = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' }; + + await handleHttp(wrappedHandler, trace_headers); + expect(Sentry.startTransaction).toBeCalledWith({ + name: 'POST /path', + op: 'gcp.function.http', + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + }); // @ts-ignore see "Why @ts-ignore" note expect(Sentry.fakeScope.setSpan).toBeCalledWith(Sentry.fakeTransaction); expect(Sentry.captureException).toBeCalledWith(error); From df65ced661ca6eb10b9ffb3cfffd194ba4ae32d5 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Tue, 16 Feb 2021 13:21:34 +0100 Subject: [PATCH 4/6] Linting fix --- packages/serverless/src/awslambda.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 00de188c80eb..99912137d19a 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -113,7 +113,7 @@ export function tryPatchHandler(taskRoot: string, handlerPath: string): void { let mod: HandlerBag; let functionName: string | undefined; - handlerName.split('.').forEach((name) => { + handlerName.split('.').forEach(name => { mod = obj; obj = obj && (obj as HandlerModule)[name]; functionName = name; @@ -236,7 +236,7 @@ export function wrapHandler( const timeoutWarningDelay = context.getRemainingTimeInMillis() - options.timeoutWarningLimit; timeoutWarningTimer = setTimeout(() => { - withScope((scope) => { + withScope(scope => { scope.setTag('timeout', humanReadableTimeout); captureMessage(`Possible function timeout: ${context.functionName}`, Severity.Warning); }); From 2390ac6b2866baf3d43f2b52f711d517d4167b91 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Tue, 16 Feb 2021 13:39:04 +0100 Subject: [PATCH 5/6] Linting fixes --- packages/serverless/package.json | 1 + packages/serverless/src/awslambda.ts | 4 ++-- packages/serverless/src/gcpfunction/http.ts | 2 +- packages/serverless/test/awslambda.test.ts | 1 - 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/serverless/package.json b/packages/serverless/package.json index 1146f296f64d..6f02aa8a57f1 100644 --- a/packages/serverless/package.json +++ b/packages/serverless/package.json @@ -18,6 +18,7 @@ "dependencies": { "@sentry/minimal": "6.1.0", "@sentry/node": "6.1.0", + "@sentry/tracing": "6.1.0", "@sentry/types": "6.1.0", "@sentry/utils": "6.1.0", "@types/aws-lambda": "^8.10.62", diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 99912137d19a..dda99c704ae4 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -9,9 +9,9 @@ import { withScope, } from '@sentry/node'; import * as Sentry from '@sentry/node'; -import { Integration } from '@sentry/types'; -import { logger, isString } from '@sentry/utils'; import { extractTraceparentData } from '@sentry/tracing'; +import { Integration } from '@sentry/types'; +import { isString, logger } from '@sentry/utils'; // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil // eslint-disable-next-line import/no-unresolved import { Context, Handler } from 'aws-lambda'; diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 64fd29f4e2ab..f1a7fdadc4ed 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -1,6 +1,6 @@ import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node'; -import { logger, stripUrlQueryAndFragment, isString } from '@sentry/utils'; import { extractTraceparentData } from '@sentry/tracing'; +import { isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; import { domainify, getActiveDomain, proxyFunction } from './../utils'; import { HttpFunction, WrapperOptions } from './general'; diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts index 23bbd60a0fc6..f825924371c9 100644 --- a/packages/serverless/test/awslambda.test.ts +++ b/packages/serverless/test/awslambda.test.ts @@ -1,4 +1,3 @@ -import { Event } from '@sentry/types'; // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil // eslint-disable-next-line import/no-unresolved import { Callback, Handler } from 'aws-lambda'; From 1cfa5439bf3e1d4460f50826411210c51589b91a Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Wed, 17 Feb 2021 19:15:48 +0100 Subject: [PATCH 6/6] Removed redundant type --- packages/serverless/test/gcpfunction.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 7f5ae0cffb7e..e5f6094b8172 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -19,7 +19,6 @@ import { * A hack-ish way to contain everything related to mocks in the same __mocks__ file. * Thanks to this, we don't have to do more magic than necessary. Just add and export desired method and assert on it. */ -type objectOfStrings = { [key: string]: string }; describe('GCPFunction', () => { afterEach(() => { @@ -27,8 +26,8 @@ describe('GCPFunction', () => { Sentry.resetMocks(); }); - async function handleHttp(fn: HttpFunction, trace_headers: objectOfStrings | null = null): Promise { - let headers: objectOfStrings = { host: 'hostname', 'content-type': 'application/json' }; + async function handleHttp(fn: HttpFunction, trace_headers: { [key: string]: string } | null = null): Promise { + let headers: { [key: string]: string } = { host: 'hostname', 'content-type': 'application/json' }; if (trace_headers) { headers = { ...headers, ...trace_headers }; } @@ -130,7 +129,9 @@ describe('GCPFunction', () => { }; const wrappedHandler = wrapHttpFunction(handler); - const trace_headers: objectOfStrings = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' }; + const trace_headers: { [key: string]: string } = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + }; await handleHttp(wrappedHandler, trace_headers); expect(Sentry.startTransaction).toBeCalledWith({