Skip to content
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

fix(serverless): Handle incoming "sentry-trace" header #3261

Merged
merged 7 commits into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/serverless/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 9 additions & 1 deletion packages/serverless/src/awslambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import {
withScope,
} from '@sentry/node';
import * as Sentry from '@sentry/node';
import { extractTraceparentData } from '@sentry/tracing';
import { Integration } from '@sentry/types';
import { logger } from '@sentry/utils';
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';
Expand Down Expand Up @@ -242,9 +243,16 @@ export function wrapHandler<TEvent, TResult>(
}, timeoutWarningDelay);
}

// Applying `sentry-trace` to context
let traceparentData;
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,
op: 'awslambda.handler',
...traceparentData,
});

const hub = getCurrentHub();
Expand Down
10 changes: 9 additions & 1 deletion packages/serverless/src/gcpfunction/http.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
import { logger, stripUrlQueryAndFragment } 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';
Expand Down Expand Up @@ -48,9 +49,16 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
const reqMethod = (req.method || '').toUpperCase();
const reqUrl = stripUrlQueryAndFragment(req.originalUrl || req.url || '');

// Applying `sentry-trace` to context
let traceparentData;
const reqWithHeaders = req as { headers?: { [key: string]: string } };
if (reqWithHeaders.headers && isString(reqWithHeaders.headers['sentry-trace'])) {
traceparentData = extractTraceparentData(reqWithHeaders.headers['sentry-trace'] as string);
}
const transaction = startTransaction({
name: `${reqMethod} ${reqUrl}`,
op: 'gcp.function.http',
...traceparentData,
});

// getCurrentHub() is expected to use current active domain as a carrier
Expand Down
20 changes: 15 additions & 5 deletions packages/serverless/test/awslambda.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -16,9 +15,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',
Expand Down Expand Up @@ -72,6 +69,12 @@ function expectScopeSettings() {
}

describe('AWSLambda', () => {
beforeEach(() => {
fakeEvent = {
fortySix: 'o_O',
};
});

afterEach(() => {
// @ts-ignore see "Why @ts-ignore" note
Sentry.resetMocks();
Expand Down Expand Up @@ -228,9 +231,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
Expand Down
22 changes: 18 additions & 4 deletions packages/serverless/test/gcpfunction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,25 @@ 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 };
ahmedetefy marked this conversation as resolved.
Show resolved Hide resolved

describe('GCPFunction', () => {
afterEach(() => {
// @ts-ignore see "Why @ts-ignore" note
Sentry.resetMocks();
});

async function handleHttp(fn: HttpFunction): Promise<void> {
async function handleHttp(fn: HttpFunction, trace_headers: objectOfStrings | null = null): Promise<void> {
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;
Expand Down Expand Up @@ -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);
Expand Down