Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit cfd2e10

Browse files
committed
PR cleanup
1 parent d4c0f70 commit cfd2e10

File tree

1 file changed

+54
-42
lines changed

1 file changed

+54
-42
lines changed

‎packages/node-core/src/integrations/http/incoming-requests.ts

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,7 @@ import { DEBUG_BUILD } from '../../debug-build';
3535
import type { NodeClient } from '../../sdk/client';
3636
import { INSTRUMENTATION_NAME, MAX_BODY_BYTE_LENGTH } from './constants';
3737

38-
type Emit = typeof Server.prototype.emit & {
39-
__sentry_patched__?: boolean;
40-
__sentryOriginalFn__?: typeof Server.prototype.emit;
41-
};
38+
type ServerEmit = typeof Server.prototype.emit;
4239

4340
const HTTP_SERVER_INSTRUMENTED_KEY = createContextKey('sentry_http_server_instrumented');
4441

@@ -50,7 +47,8 @@ const clientToRequestSessionAggregatesMap = new Map<
5047
// We keep track of emit functions we wrapped, to avoid double wrapping
5148
// We do this instead of putting a non-enumerable property on the function, because
5249
// sometimes the property seems to be migrated to forks of the emit function, which we do not want to happen
53-
const wrappedEmitFns = new WeakSet<Emit>();
50+
// This was the case in the nestjs-distributed-tracing E2E test
51+
const wrappedEmitFns = new WeakSet<ServerEmit>();
5452

5553
/**
5654
* Instrument a server to capture incoming requests.
@@ -90,10 +88,8 @@ export function instrumentServer(
9088
};
9189
},
9290
): void {
93-
type Emit = typeof server.emit & { __sentryOriginalFn__?: typeof server.emit };
94-
9591
// eslint-disable-next-line @typescript-eslint/unbound-method
96-
const originalEmit: Emit = server.emit;
92+
const originalEmit: ServerEmit = server.emit;
9793

9894
if (wrappedEmitFns.has(originalEmit)) {
9995
DEBUG_BUILD &&
@@ -103,33 +99,6 @@ export function instrumentServer(
10399

104100
const { requestHook, responseHook, applyCustomAttributesOnSpan } = instrumentation ?? {};
105101

106-
function shouldIgnoreSpansForIncomingRequest(request: IncomingMessage): boolean {
107-
if (isTracingSuppressed(context.active())) {
108-
return true;
109-
}
110-
111-
// request.url is the only property that holds any information about the url
112-
// it only consists of the URL path and query string (if any)
113-
const urlPath = request.url;
114-
115-
const method = request.method?.toUpperCase();
116-
// We do not capture OPTIONS/HEAD requests as spans
117-
if (method === 'OPTIONS' || method === 'HEAD' || !urlPath) {
118-
return true;
119-
}
120-
121-
// Default static asset filtering
122-
if (ignoreStaticAssets && method === 'GET' && isStaticAssetRequest(urlPath)) {
123-
return true;
124-
}
125-
126-
if (ignoreSpansForIncomingRequests?.(urlPath, request)) {
127-
return true;
128-
}
129-
130-
return false;
131-
}
132-
133102
const newEmit = new Proxy(originalEmit, {
134103
apply(target, thisArg, args: [event: string, ...args: unknown[]]) {
135104
// Only traces request events
@@ -138,7 +107,7 @@ export function instrumentServer(
138107
}
139108

140109
// Make sure we do not double execute our wrapper code, for edge cases...
141-
// Without this check, if we double-wrap emit, for whatever reason, you'd get to http.server spans (one the children of the other)
110+
// Without this check, if we double-wrap emit, for whatever reason, you'd get two http.server spans (one the children of the other)
142111
if (context.active().getValue(HTTP_SERVER_INSTRUMENTED_KEY)) {
143112
return target.apply(thisArg, args);
144113
}
@@ -193,7 +162,14 @@ export function instrumentServer(
193162

194163
return context.with(ctx, () => {
195164
// if opting out of span creation, we can end here
196-
if (!spans || shouldIgnoreSpansForIncomingRequest(request) || !client) {
165+
if (
166+
!spans ||
167+
!client ||
168+
shouldIgnoreSpansForIncomingRequest(request, {
169+
ignoreStaticAssets,
170+
ignoreSpansForIncomingRequests,
171+
})
172+
) {
197173
DEBUG_BUILD && debug.log(INSTRUMENTATION_NAME, 'Skipping span creation for incoming request');
198174
return target.apply(thisArg, args);
199175
}
@@ -249,7 +225,8 @@ export function instrumentServer(
249225
context.bind(context.active(), request);
250226
context.bind(context.active(), response);
251227

252-
// After 'error', no further events other than 'close' should be emitted.
228+
// Ensure we only end the span once
229+
// E.g. error can be emitted before close is emitted
253230
let isEnded = false;
254231
function endSpan(status: SpanStatus): void {
255232
if (isEnded) {
@@ -264,10 +241,9 @@ export function instrumentServer(
264241
span.end();
265242

266243
// Update the transaction name if the route has changed
267-
if (newAttributes['http.route']) {
268-
getIsolationScope().setTransactionName(
269-
`${request.method?.toUpperCase() || 'GET'} ${newAttributes['http.route']}`,
270-
);
244+
const route = newAttributes['http.route'];
245+
if (route) {
246+
getIsolationScope().setTransactionName(`${request.method?.toUpperCase() || 'GET'} ${route}`);
271247
}
272248
}
273249

@@ -577,3 +553,39 @@ export function isStaticAssetRequest(urlPath: string): boolean {
577553

578554
return false;
579555
}
556+
557+
function shouldIgnoreSpansForIncomingRequest(
558+
request: IncomingMessage,
559+
{
560+
ignoreStaticAssets,
561+
ignoreSpansForIncomingRequests,
562+
}: {
563+
ignoreStaticAssets?: boolean;
564+
ignoreSpansForIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean;
565+
},
566+
): boolean {
567+
if (isTracingSuppressed(context.active())) {
568+
return true;
569+
}
570+
571+
// request.url is the only property that holds any information about the url
572+
// it only consists of the URL path and query string (if any)
573+
const urlPath = request.url;
574+
575+
const method = request.method?.toUpperCase();
576+
// We do not capture OPTIONS/HEAD requests as spans
577+
if (method === 'OPTIONS' || method === 'HEAD' || !urlPath) {
578+
return true;
579+
}
580+
581+
// Default static asset filtering
582+
if (ignoreStaticAssets && method === 'GET' && isStaticAssetRequest(urlPath)) {
583+
return true;
584+
}
585+
586+
if (ignoreSpansForIncomingRequests?.(urlPath, request)) {
587+
return true;
588+
}
589+
590+
return false;
591+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /