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 2cde2a4

Browse files
authored
feat(node): Do not drop 300 and 304 status codes by default (#17686)
Looking at https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status#redirection_messages, it seems that we should not filter out 300, 302 and 304 status codes by default: > [300 Multiple Choices](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/300) In [agent-driven content negotiation](https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/Content_negotiation#agent-driven_negotiation), the request has more than one possible response and the user agent or user should choose one of them. There is no standardized way for clients to automatically choose one of the responses, so this is rarely used. > [304 Not Modified](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/304) This is used for caching purposes. It tells the client that the response has not been modified, so the client can continue to use the same [cached](https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/Caching) version of the response. In contrast, the others seem safe to continue to filter: > [301 Moved Permanently](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/301) The URL of the requested resource has been changed permanently. The new URL is given in the response. > [302 Found](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/302) This response code means that the URI of requested resource has been changed temporarily. Further changes in the URI might be made in the future, so the same URI should be used by the client in future requests. > [303 See Other](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/303) The server sent this response to direct the client to get the requested resource at another URI with a [GET](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Methods/GET) request. > [305 Use Proxy Deprecated](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status#305_use_proxy) Defined in a previous version of the HTTP specification to indicate that a requested response must be accessed by a proxy. It has been deprecated due to security concerns regarding in-band configuration of a proxy. > [306 unused](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status#306_unused) This response code is no longer used; but is reserved. It was used in a previous version of the HTTP/1.1 specification. > [307 Temporary Redirect](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/307) The server sends this response to direct the client to get the requested resource at another URI with the same method that was used in the prior request. This has the same semantics as the 302 Found response code, with the exception that the user agent must not change the HTTP method used: if a [POST](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Methods/POST) was used in the first request, a POST must be used in the redirected request. > [308 Permanent Redirect](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/308) This means that the resource is now permanently located at another URI, specified by the [Location](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Location) response header. This has the same semantics as the 301 Moved Permanently HTTP response code, with the exception that the user agent must not change the HTTP method used: if a [POST](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Methods/POST) was used in the first request, a POST must be used in the second request. eventually it makes sense to unify this between node/node-core, but this should happen when we merge the httpIntegration split PR.
1 parent c022972 commit 2cde2a4

File tree

2 files changed

+54
-32
lines changed
  • packages

2 files changed

+54
-32
lines changed

‎packages/node-core/src/integrations/http/index.ts‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { IncomingMessage, RequestOptions } from 'node:http';
2-
import { defineIntegration } from '@sentry/core';
2+
import { debug, defineIntegration } from '@sentry/core';
3+
import { DEBUG_BUILD } from '../../debug-build';
34
import { generateInstrumentOnce } from '../../otel/instrument';
45
import type { SentryHttpInstrumentationOptions } from './SentryHttpInstrumentation';
56
import { SentryHttpInstrumentation } from './SentryHttpInstrumentation';
@@ -62,10 +63,10 @@ interface HttpOptions {
6263

6364
/**
6465
* Do not capture spans for incoming HTTP requests with the given status codes.
65-
* By default, spans with 404 status code are ignored.
66+
* By default, spans with some 3xx and 4xx status codes are ignored (see @default).
6667
* Expects an array of status codes or a range of status codes, e.g. [[300,399], 404] would ignore 3xx and 404 status codes.
6768
*
68-
* @default `[[401, 404], [300, 399]]`
69+
* @default `[[401, 404], [301, 303], [305, 399]]`
6970
*/
7071
dropSpansForIncomingRequestStatusCodes?: (number | [number, number])[];
7172

@@ -115,7 +116,9 @@ export const instrumentSentryHttp = generateInstrumentOnce<SentryHttpInstrumenta
115116
export const httpIntegration = defineIntegration((options: HttpOptions = {}) => {
116117
const dropSpansForIncomingRequestStatusCodes = options.dropSpansForIncomingRequestStatusCodes ?? [
117118
[401, 404],
118-
[300, 399],
119+
// 300 and 304 are possibly valid status codes we do not want to filter
120+
[301, 303],
121+
[305, 399],
119122
];
120123

121124
return {
@@ -133,22 +136,30 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
133136
// Drop transaction if it has a status code that should be ignored
134137
if (event.type === 'transaction') {
135138
const statusCode = event.contexts?.trace?.data?.['http.response.status_code'];
136-
if (
137-
typeof statusCode === 'number' &&
138-
dropSpansForIncomingRequestStatusCodes.some(code => {
139-
if (typeof code === 'number') {
140-
return code === statusCode;
141-
}
142-
143-
const [min, max] = code;
144-
return statusCode >= min && statusCode <= max;
145-
})
146-
) {
147-
return null;
139+
if (typeof statusCode === 'number') {
140+
const shouldDrop = shouldFilterStatusCode(statusCode, dropSpansForIncomingRequestStatusCodes);
141+
if (shouldDrop) {
142+
DEBUG_BUILD && debug.log('Dropping transaction due to status code', statusCode);
143+
return null;
144+
}
148145
}
149146
}
150147

151148
return event;
152149
},
153150
};
154151
});
152+
153+
/**
154+
* If the given status code should be filtered for the given list of status codes/ranges.
155+
*/
156+
function shouldFilterStatusCode(statusCode: number, dropForStatusCodes: (number | [number, number])[]): boolean {
157+
return dropForStatusCodes.some(code => {
158+
if (typeof code === 'number') {
159+
return code === statusCode;
160+
}
161+
162+
const [min, max] = code;
163+
return statusCode >= min && statusCode <= max;
164+
});
165+
}

‎packages/node/src/integrations/http.ts‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { diag } from '@opentelemetry/api';
33
import type { HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http';
44
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
55
import type { Span } from '@sentry/core';
6-
import { defineIntegration, getClient, hasSpansEnabled } from '@sentry/core';
6+
import { debug,defineIntegration, getClient, hasSpansEnabled } from '@sentry/core';
77
import type { HTTPModuleRequestIncomingMessage, NodeClient } from '@sentry/node-core';
88
import {
99
type SentryHttpInstrumentationOptions,
@@ -13,6 +13,7 @@ import {
1313
NODE_VERSION,
1414
SentryHttpInstrumentation,
1515
} from '@sentry/node-core';
16+
import { DEBUG_BUILD } from '../debug-build';
1617
import type { NodeClientOptions } from '../types';
1718

1819
const INTEGRATION_NAME = 'Http';
@@ -84,10 +85,10 @@ interface HttpOptions {
8485

8586
/**
8687
* Do not capture spans for incoming HTTP requests with the given status codes.
87-
* By default, spans with 404 status code are ignored.
88+
* By default, spans with some 3xx and 4xx status codes are ignored (see @default).
8889
* Expects an array of status codes or a range of status codes, e.g. [[300,399], 404] would ignore 3xx and 404 status codes.
8990
*
90-
* @default `[[401, 404], [300, 399]]`
91+
* @default `[[401, 404], [301, 303], [305, 399]]`
9192
*/
9293
dropSpansForIncomingRequestStatusCodes?: (number | [number, number])[];
9394

@@ -195,7 +196,9 @@ export function _shouldUseOtelHttpInstrumentation(
195196
export const httpIntegration = defineIntegration((options: HttpOptions = {}) => {
196197
const dropSpansForIncomingRequestStatusCodes = options.dropSpansForIncomingRequestStatusCodes ?? [
197198
[401, 404],
198-
[300, 399],
199+
// 300 and 304 are possibly valid status codes we do not want to filter
200+
[301, 303],
201+
[305, 399],
199202
];
200203

201204
return {
@@ -225,18 +228,12 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
225228
// Drop transaction if it has a status code that should be ignored
226229
if (event.type === 'transaction') {
227230
const statusCode = event.contexts?.trace?.data?.['http.response.status_code'];
228-
if (
229-
typeof statusCode === 'number' &&
230-
dropSpansForIncomingRequestStatusCodes.some(code => {
231-
if (typeof code === 'number') {
232-
return code === statusCode;
233-
}
234-
235-
const [min, max] = code;
236-
return statusCode >= min && statusCode <= max;
237-
})
238-
) {
239-
return null;
231+
if (typeof statusCode === 'number') {
232+
const shouldDrop = shouldFilterStatusCode(statusCode, dropSpansForIncomingRequestStatusCodes);
233+
if (shouldDrop) {
234+
DEBUG_BUILD && debug.log('Dropping transaction due to status code', statusCode);
235+
return null;
236+
}
240237
}
241238
}
242239

@@ -282,3 +279,17 @@ function getConfigWithDefaults(options: Partial<HttpOptions> = {}): HttpInstrume
282279

283280
return instrumentationConfig;
284281
}
282+
283+
/**
284+
* If the given status code should be filtered for the given list of status codes/ranges.
285+
*/
286+
function shouldFilterStatusCode(statusCode: number, dropForStatusCodes: (number | [number, number])[]): boolean {
287+
return dropForStatusCodes.some(code => {
288+
if (typeof code === 'number') {
289+
return code === statusCode;
290+
}
291+
292+
const [min, max] = code;
293+
return statusCode >= min && statusCode <= max;
294+
});
295+
}

0 commit comments

Comments
(0)

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