From 1f898b6de5cc3eb4fecbcc3248778148c11cfcab Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein <83657429+Zen-cronic@users.noreply.github.com> Date: Mon, 23 Sep 2024 07:31:04 -0400 Subject: [PATCH] feat: Set log level for Fetch/XHR breadcrumbs based on status code (#13711) Fixes #13359 - [x] If you've added code that should be tested, please add tests. - [x] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`). --------- Signed-off-by: Kaung Zin Hein Co-authored-by: Luca Forstner --- .../Breadcrumbs/fetch/statusCode/subject.js | 3 + .../Breadcrumbs/fetch/statusCode/test.ts | 71 +++++++++++++++++++ .../Breadcrumbs/xhr/statusCode/subject.js | 10 +++ .../Breadcrumbs/xhr/statusCode/test.ts | 71 +++++++++++++++++++ .../browser/src/integrations/breadcrumbs.ts | 7 ++ packages/cloudflare/src/integrations/fetch.ts | 10 ++- packages/deno/src/integrations/breadcrumbs.ts | 4 ++ packages/node/src/integrations/http.ts | 13 +++- packages/node/src/integrations/node-fetch.ts | 7 +- packages/types/src/scope.ts | 6 +- packages/utils/src/breadcrumb-log-level.ts | 17 +++++ packages/utils/src/index.ts | 1 + .../utils/test/breadcrumb-log-level.test.ts | 15 ++++ .../src/integrations/wintercg-fetch.ts | 10 ++- 14 files changed, 236 insertions(+), 9 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/test.ts create mode 100644 packages/utils/src/breadcrumb-log-level.ts create mode 100644 packages/utils/test/breadcrumb-log-level.test.ts diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/subject.js b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/subject.js new file mode 100644 index 000000000000..1cbadc6e36e6 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/subject.js @@ -0,0 +1,3 @@ +fetch('http://sentry-test.io/foo').then(() => { + Sentry.captureException('test error'); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/test.ts new file mode 100644 index 000000000000..70cd868ccfe1 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/fetch/statusCode/test.ts @@ -0,0 +1,71 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; + +sentryTest('captures Breadcrumb with log level for 4xx response code', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('**/foo', async route => { + await route.fulfill({ + status: 404, + contentType: 'text/plain', + body: 'Not Found!', + }); + }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + + expect(eventData?.breadcrumbs?.length).toBe(1); + expect(eventData!.breadcrumbs![0]).toEqual({ + timestamp: expect.any(Number), + category: 'fetch', + type: 'http', + data: { + method: 'GET', + status_code: 404, + url: 'http://sentry-test.io/foo', + }, + level: 'warning', + }); + + await page.route('**/foo', async route => { + await route.fulfill({ + status: 500, + contentType: 'text/plain', + body: 'Internal Server Error', + }); + }); +}); + +sentryTest('captures Breadcrumb with log level for 5xx response code', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('**/foo', async route => { + await route.fulfill({ + status: 500, + contentType: 'text/plain', + body: 'Internal Server Error', + }); + }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + + expect(eventData?.breadcrumbs?.length).toBe(1); + expect(eventData!.breadcrumbs![0]).toEqual({ + timestamp: expect.any(Number), + category: 'fetch', + type: 'http', + data: { + method: 'GET', + status_code: 500, + url: 'http://sentry-test.io/foo', + }, + level: 'error', + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/subject.js b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/subject.js new file mode 100644 index 000000000000..8202bb03803b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/subject.js @@ -0,0 +1,10 @@ +const xhr = new XMLHttpRequest(); + +xhr.open('GET', 'http://sentry-test.io/foo'); +xhr.send(); + +xhr.addEventListener('readystatechange', function () { + if (xhr.readyState === 4) { + Sentry.captureException('test error'); + } +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/test.ts b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/test.ts new file mode 100644 index 000000000000..eb7014df5890 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/xhr/statusCode/test.ts @@ -0,0 +1,71 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; + +sentryTest('captures Breadcrumb with log level for 4xx response code', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('**/foo', async route => { + await route.fulfill({ + status: 404, + contentType: 'text/plain', + body: 'Not Found!', + }); + }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + + expect(eventData?.breadcrumbs?.length).toBe(1); + expect(eventData!.breadcrumbs![0]).toEqual({ + timestamp: expect.any(Number), + category: 'xhr', + type: 'http', + data: { + method: 'GET', + status_code: 404, + url: 'http://sentry-test.io/foo', + }, + level: 'warning', + }); + + await page.route('**/foo', async route => { + await route.fulfill({ + status: 500, + contentType: 'text/plain', + body: 'Internal Server Error', + }); + }); +}); + +sentryTest('captures Breadcrumb with log level for 5xx response code', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('**/foo', async route => { + await route.fulfill({ + status: 500, + contentType: 'text/plain', + body: 'Internal Server Error', + }); + }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + + expect(eventData?.breadcrumbs?.length).toBe(1); + expect(eventData!.breadcrumbs![0]).toEqual({ + timestamp: expect.any(Number), + category: 'xhr', + type: 'http', + data: { + method: 'GET', + status_code: 500, + url: 'http://sentry-test.io/foo', + }, + level: 'error', + }); +}); diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index e3c1120fca57..db30a48dda67 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -23,6 +23,7 @@ import type { import { addConsoleInstrumentationHandler, addFetchInstrumentationHandler, + getBreadcrumbLogLevelFromHttpStatusCode, getComponentName, getEventDescription, htmlTreeAsString, @@ -247,11 +248,14 @@ function _getXhrBreadcrumbHandler(client: Client): (handlerData: HandlerDataXhr) endTimestamp, }; + const level = getBreadcrumbLogLevelFromHttpStatusCode(status_code); + addBreadcrumb( { category: 'xhr', data, type: 'http', + level, }, hint, ); @@ -309,11 +313,14 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe startTimestamp, endTimestamp, }; + const level = getBreadcrumbLogLevelFromHttpStatusCode(data.status_code); + addBreadcrumb( { category: 'fetch', data, type: 'http', + level, }, hint, ); diff --git a/packages/cloudflare/src/integrations/fetch.ts b/packages/cloudflare/src/integrations/fetch.ts index 4781a71a896d..4bada212e7d5 100644 --- a/packages/cloudflare/src/integrations/fetch.ts +++ b/packages/cloudflare/src/integrations/fetch.ts @@ -7,7 +7,12 @@ import type { IntegrationFn, Span, } from '@sentry/types'; -import { LRUMap, addFetchInstrumentationHandler, stringMatchesSomePattern } from '@sentry/utils'; +import { + LRUMap, + addFetchInstrumentationHandler, + getBreadcrumbLogLevelFromHttpStatusCode, + stringMatchesSomePattern, +} from '@sentry/utils'; const INTEGRATION_NAME = 'Fetch'; @@ -144,11 +149,14 @@ function createBreadcrumb(handlerData: HandlerDataFetch): void { startTimestamp, endTimestamp, }; + const level = getBreadcrumbLogLevelFromHttpStatusCode(data.status_code); + addBreadcrumb( { category: 'fetch', data, type: 'http', + level, }, hint, ); diff --git a/packages/deno/src/integrations/breadcrumbs.ts b/packages/deno/src/integrations/breadcrumbs.ts index 47953d4d7ce8..6b945ebc37f5 100644 --- a/packages/deno/src/integrations/breadcrumbs.ts +++ b/packages/deno/src/integrations/breadcrumbs.ts @@ -11,6 +11,7 @@ import type { import { addConsoleInstrumentationHandler, addFetchInstrumentationHandler, + getBreadcrumbLogLevelFromHttpStatusCode, getEventDescription, safeJoin, severityLevelFromString, @@ -178,11 +179,14 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe startTimestamp, endTimestamp, }; + const level = getBreadcrumbLogLevelFromHttpStatusCode(data.status_code); + addBreadcrumb( { category: 'fetch', data, type: 'http', + level, }, hint, ); diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 126f22a06063..d6796aa866e5 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -15,7 +15,12 @@ import { import { getClient } from '@sentry/opentelemetry'; import type { IntegrationFn, SanitizedRequestData } from '@sentry/types'; -import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils'; +import { + getBreadcrumbLogLevelFromHttpStatusCode, + getSanitizedUrlString, + parseUrl, + stripUrlQueryAndFragment, +} from '@sentry/utils'; import type { NodeClient } from '../sdk/client'; import { setIsolationScope } from '../sdk/scope'; import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module'; @@ -243,14 +248,18 @@ function _addRequestBreadcrumb( } const data = getBreadcrumbData(request); + const statusCode = response.statusCode; + const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); + addBreadcrumb( { category: 'http', data: { - status_code: response.statusCode, + status_code: statusCode, ...data, }, type: 'http', + level, }, { event: 'response', diff --git a/packages/node/src/integrations/node-fetch.ts b/packages/node/src/integrations/node-fetch.ts index 0726c2c63f9b..ee797b0587d7 100644 --- a/packages/node/src/integrations/node-fetch.ts +++ b/packages/node/src/integrations/node-fetch.ts @@ -3,7 +3,7 @@ import { UndiciInstrumentation } from '@opentelemetry/instrumentation-undici'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, addBreadcrumb, defineIntegration } from '@sentry/core'; import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; import type { IntegrationFn, SanitizedRequestData } from '@sentry/types'; -import { getSanitizedUrlString, parseUrl } from '@sentry/utils'; +import { getBreadcrumbLogLevelFromHttpStatusCode, getSanitizedUrlString, parseUrl } from '@sentry/utils'; interface NodeFetchOptions { /** @@ -56,15 +56,18 @@ export const nativeNodeFetchIntegration = defineIntegration(_nativeNodeFetchInte /** Add a breadcrumb for outgoing requests. */ function addRequestBreadcrumb(request: UndiciRequest, response: UndiciResponse): void { const data = getBreadcrumbData(request); + const statusCode = response.statusCode; + const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); addBreadcrumb( { category: 'http', data: { - status_code: response.statusCode, + status_code: statusCode, ...data, }, type: 'http', + level, }, { event: 'response', diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts index aa51c69035f1..a4b91f4b5d96 100644 --- a/packages/types/src/scope.ts +++ b/packages/types/src/scope.ts @@ -189,8 +189,8 @@ export interface Scope { clear(): this; /** - * Sets the breadcrumbs in the scope - * @param breadcrumbs Breadcrumb + * Adds a breadcrumb to the scope + * @param breadcrumb Breadcrumb * @param maxBreadcrumbs number of max breadcrumbs to merged into event. */ addBreadcrumb(breadcrumb: Breadcrumb, maxBreadcrumbs?: number): this; @@ -201,7 +201,7 @@ export interface Scope { getLastBreadcrumb(): Breadcrumb | undefined; /** - * Clears all currently set Breadcrumbs. + * Clears all breadcrumbs from the scope. */ clearBreadcrumbs(): this; diff --git a/packages/utils/src/breadcrumb-log-level.ts b/packages/utils/src/breadcrumb-log-level.ts new file mode 100644 index 000000000000..a19d70e00412 --- /dev/null +++ b/packages/utils/src/breadcrumb-log-level.ts @@ -0,0 +1,17 @@ +import type { SeverityLevel } from '@sentry/types'; + +/** + * Determine a breadcrumb's log level (only `warning` or `error`) based on an HTTP status code. + */ +export function getBreadcrumbLogLevelFromHttpStatusCode(statusCode: number | undefined): SeverityLevel | undefined { + // NOTE: undefined defaults to 'info' in Sentry + if (statusCode === undefined) { + return undefined; + } else if (statusCode >= 400 && statusCode < 500) { + return 'warning'; + } else if (statusCode >= 500) { + return 'error'; + } else { + return undefined; + } +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 822d150dfde1..245751b3e72c 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -1,5 +1,6 @@ export * from './aggregate-errors'; export * from './array'; +export * from './breadcrumb-log-level'; export * from './browser'; export * from './dsn'; export * from './error'; diff --git a/packages/utils/test/breadcrumb-log-level.test.ts b/packages/utils/test/breadcrumb-log-level.test.ts new file mode 100644 index 000000000000..49792d2726bb --- /dev/null +++ b/packages/utils/test/breadcrumb-log-level.test.ts @@ -0,0 +1,15 @@ +import { getBreadcrumbLogLevelFromHttpStatusCode } from '../src/breadcrumb-log-level'; + +describe('getBreadcrumbLogLevelFromHttpStatusCode()', () => { + it.each([ + ['warning', '4xx', 403], + ['error', '5xx', 500], + [undefined, '3xx', 307], + [undefined, '2xx', 200], + [undefined, '1xx', 103], + [undefined, '0', 0], + [undefined, 'undefined', undefined], + ])('should return `%s` for %s', (output, _codeRange, input) => { + expect(getBreadcrumbLogLevelFromHttpStatusCode(input)).toEqual(output); + }); +}); diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts index 970e10958333..4dd4bf399e4c 100644 --- a/packages/vercel-edge/src/integrations/wintercg-fetch.ts +++ b/packages/vercel-edge/src/integrations/wintercg-fetch.ts @@ -7,7 +7,12 @@ import type { IntegrationFn, Span, } from '@sentry/types'; -import { LRUMap, addFetchInstrumentationHandler, stringMatchesSomePattern } from '@sentry/utils'; +import { + LRUMap, + addFetchInstrumentationHandler, + getBreadcrumbLogLevelFromHttpStatusCode, + stringMatchesSomePattern, +} from '@sentry/utils'; const INTEGRATION_NAME = 'WinterCGFetch'; @@ -150,11 +155,14 @@ function createBreadcrumb(handlerData: HandlerDataFetch): void { startTimestamp, endTimestamp, }; + const level = getBreadcrumbLogLevelFromHttpStatusCode(data.status_code); + addBreadcrumb( { category: 'fetch', data, type: 'http', + level, }, hint, );