Skip to content

Commit 538f41c

Browse files
authored
fix(node): Add conditions to TracingHanlder.startTransaction (#5485)
Currently, our TracingHandler always starts a transaction, whether or not tracing is enabled. It also traces OPTIONS and HEAD requests, which it shouldn't. This fixes both of those problems by checking both the current client's options (for tracing enablement) and the incoming request's method before starting a transaction.
1 parent 1182e71 commit 538f41c

File tree

2 files changed

+49
-2
lines changed

2 files changed

+49
-2
lines changed

packages/node/src/handlers.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,24 @@ export function tracingHandler(): (
3333
res: http.ServerResponse,
3434
next: (error?: any) => void,
3535
): void {
36+
const hub = getCurrentHub();
37+
const options = hub.getClient()?.getOptions();
38+
39+
if (!options || req.method?.toUpperCase() === 'OPTIONS' || req.method?.toUpperCase() === 'HEAD') {
40+
return next();
41+
}
42+
43+
// TODO: This is the `hasTracingEnabled` check, but we're doing it manually since `@sentry/tracing` isn't a
44+
// dependency of `@sentry/node`. Long term, that function should probably move to `@sentry/hub.
45+
if (!('tracesSampleRate' in options) && !('tracesSampler' in options)) {
46+
__DEBUG_BUILD__ &&
47+
logger.warn(
48+
'Sentry `tracingHandler` is being used, but tracing is disabled. Please enable tracing by setting ' +
49+
'either `tracesSampleRate` or `tracesSampler` in your `Sentry.init()` options.',
50+
);
51+
return next();
52+
}
53+
3654
// If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision)
3755
const traceparentData =
3856
req.headers && isString(req.headers['sentry-trace']) && extractTraceparentData(req.headers['sentry-trace']);
@@ -53,7 +71,7 @@ export function tracingHandler(): (
5371
);
5472

5573
// We put the transaction on the scope so users can attach children to it
56-
getCurrentHub().configureScope(scope => {
74+
hub.configureScope(scope => {
5775
scope.setSpan(transaction);
5876
});
5977

packages/node/test/handlers.test.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,16 @@ describe('tracingHandler', () => {
150150

151151
const sentryTracingMiddleware = tracingHandler();
152152

153-
let req: http.IncomingMessage, res: http.ServerResponse, next: () => undefined;
153+
let hub: Hub, req: http.IncomingMessage, res: http.ServerResponse, next: () => undefined;
154154

155155
function createNoOpSpy() {
156156
const noop = { noop: () => undefined }; // this is wrapped in an object so jest can spy on it
157157
return jest.spyOn(noop, 'noop') as any;
158158
}
159159

160160
beforeEach(() => {
161+
hub = new Hub(new NodeClient(getDefaultNodeClientOptions({ tracesSampleRate: 1.0 })));
162+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
161163
req = {
162164
headers,
163165
method,
@@ -181,6 +183,33 @@ describe('tracingHandler', () => {
181183
expect(startTransaction).toHaveBeenCalled();
182184
});
183185

186+
it("doesn't create a transaction when handling a `HEAD` request", () => {
187+
const startTransaction = jest.spyOn(sentryCore, 'startTransaction');
188+
req.method = 'HEAD';
189+
190+
sentryTracingMiddleware(req, res, next);
191+
192+
expect(startTransaction).not.toHaveBeenCalled();
193+
});
194+
195+
it("doesn't create a transaction when handling an `OPTIONS` request", () => {
196+
const startTransaction = jest.spyOn(sentryCore, 'startTransaction');
197+
req.method = 'OPTIONS';
198+
199+
sentryTracingMiddleware(req, res, next);
200+
201+
expect(startTransaction).not.toHaveBeenCalled();
202+
});
203+
204+
it("doesn't create a transaction if tracing is disabled", () => {
205+
delete hub.getClient()?.getOptions().tracesSampleRate;
206+
const startTransaction = jest.spyOn(sentryCore, 'startTransaction');
207+
208+
sentryTracingMiddleware(req, res, next);
209+
210+
expect(startTransaction).not.toHaveBeenCalled();
211+
});
212+
184213
it("pulls parent's data from tracing header on the request", () => {
185214
req.headers = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' };
186215

0 commit comments

Comments
 (0)