Skip to content

feat: Set log level for Fetch/XHR breadcrumbs based on status code #13711

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

Merged
merged 9 commits into from
Sep 23, 2024
8 changes: 7 additions & 1 deletion packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
addHistoryInstrumentationHandler,
addXhrInstrumentationHandler,
} from '@sentry-internal/browser-utils';
import { addBreadcrumb, defineIntegration, getClient } from '@sentry/core';
import { addBreadcrumb, defineIntegration, getBreadcrumbLogLevel, getClient } from '@sentry/core';
import type {
Breadcrumb,
Client,
Expand Down Expand Up @@ -247,11 +247,14 @@ function _getXhrBreadcrumbHandler(client: Client): (handlerData: HandlerDataXhr)
endTimestamp,
};

const level = getBreadcrumbLogLevel(status_code);

addBreadcrumb(
{
category: 'xhr',
data,
type: 'http',
level,
},
hint,
);
Expand Down Expand Up @@ -309,11 +312,14 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
startTimestamp,
endTimestamp,
};
const level = getBreadcrumbLogLevel(data.status_code);

addBreadcrumb(
{
category: 'fetch',
data,
type: 'http',
level,
},
hint,
);
Expand Down
12 changes: 11 additions & 1 deletion packages/cloudflare/src/integrations/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { addBreadcrumb, defineIntegration, getClient, instrumentFetchRequest, isSentryRequestUrl } from '@sentry/core';
import {
addBreadcrumb,
defineIntegration,
getBreadcrumbLogLevel,
getClient,
instrumentFetchRequest,
isSentryRequestUrl,
} from '@sentry/core';
import type {
Client,
FetchBreadcrumbData,
Expand Down Expand Up @@ -144,11 +151,14 @@ function createBreadcrumb(handlerData: HandlerDataFetch): void {
startTimestamp,
endTimestamp,
};
const level = getBreadcrumbLogLevel(data.status_code);

addBreadcrumb(
{
category: 'fetch',
data,
type: 'http',
level,
},
hint,
);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export { parseSampleRate } from './utils/parseSampleRate';
export { applySdkMetadata } from './utils/sdkMetadata';
export { getTraceData } from './utils/traceData';
export { getTraceMetaTags } from './utils/meta';
export { getBreadcrumbLogLevel } from './utils/breadcrumbsUtils';
export { DEFAULT_ENVIRONMENT } from './constants';
export { addBreadcrumb } from './breadcrumbs';
export { functionToStringIntegration } from './integrations/functiontostring';
Expand Down
17 changes: 17 additions & 0 deletions packages/core/src/utils/breadcrumbsUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import type { SeverityLevel } from '@sentry/types';

/**
* Determine a breadcrumb's log level based on the response status code
* @param statusCode
*/
export function getBreadcrumbLogLevel(statusCode: number | undefined): SeverityLevel {
if (statusCode === undefined) {
return 'info';
} else if (statusCode >= 400 && statusCode < 500) {
return 'warning';
} else if (statusCode >= 500) {
return 'error';
} else {
return 'info';
Copy link
Contributor Author

@Zen-cronic Zen-cronic Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the undefined and other cases, the fallback level is info. So, all the test cases for breadcrumbs in browser-integration-tests must be updated, which is quite a lot.

Is this behaviour needed? E.g., non-error request breadcrumb will have an additional property level: 'info'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently many tests are failing b/c of the default property. Maybe we shouldn't set it if the response has a non-error status code. Otherwise, it's a handful to fix those tests (though it's simply just adding the property).

Any thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, i've left out the default value (info) to not break the many other tests. But other sdks use a default value: getsentry/sentry-php@d12482a. So we should consider it.

}
}
15 changes: 15 additions & 0 deletions packages/core/test/lib/utils/breadcrumbsUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import type { SeverityLevel } from '@sentry/types';
import { getBreadcrumbLogLevel } from '../../../src/utils/breadcrumbsUtils';

describe('getBreadcrumbLogLevel()', () => {
it.each([
['warning', '4xx', 403],
['error', '5xx', 500],
['info', '3xx', 307],
['info', '2xx', 200],
['info', '1xx', 103],
['info', 'undefined', undefined],
] as [SeverityLevel, string, number | undefined][])('should return `%s` for %s', (output, _codeRange, input) => {
expect(getBreadcrumbLogLevel(input)).toBe(output);
});
});
5 changes: 4 additions & 1 deletion packages/deno/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { addBreadcrumb, defineIntegration, getClient } from '@sentry/core';
import { addBreadcrumb, defineIntegration, getBreadcrumbLogLevel, getClient } from '@sentry/core';
import type {
Client,
Event as SentryEvent,
Expand Down Expand Up @@ -178,11 +178,14 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
startTimestamp,
endTimestamp,
};
const level = getBreadcrumbLogLevel(data.status_code);

addBreadcrumb(
{
category: 'fetch',
data,
type: 'http',
level,
},
hint,
);
Expand Down
7 changes: 6 additions & 1 deletion packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry';
import {
addBreadcrumb,
defineIntegration,
getBreadcrumbLogLevel,
getCapturedScopesOnSpan,
getCurrentScope,
getIsolationScope,
Expand Down Expand Up @@ -229,14 +230,18 @@ function _addRequestBreadcrumb(
}

const data = getBreadcrumbData(request);
const statusCode = response.statusCode;
const level = getBreadcrumbLogLevel(statusCode);

addBreadcrumb(
{
category: 'http',
data: {
status_code: response.statusCode,
status_code: statusCode,
...data,
},
type: 'http',
level,
},
{
event: 'response',
Expand Down
12 changes: 10 additions & 2 deletions packages/node/src/integrations/node-fetch.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import type { UndiciRequest, UndiciResponse } from '@opentelemetry/instrumentation-undici';
import { UndiciInstrumentation } from '@opentelemetry/instrumentation-undici';
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, addBreadcrumb, defineIntegration } from '@sentry/core';
import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
addBreadcrumb,
defineIntegration,
getBreadcrumbLogLevel,
} from '@sentry/core';
import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry';
import type { IntegrationFn, SanitizedRequestData } from '@sentry/types';
import { getSanitizedUrlString, parseUrl } from '@sentry/utils';
Expand Down Expand Up @@ -56,15 +61,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 = getBreadcrumbLogLevel(statusCode);

addBreadcrumb(
{
category: 'http',
data: {
status_code: response.statusCode,
status_code: statusCode,
...data,
},
type: 'http',
level,
},
{
event: 'response',
Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ export interface Scope {
clear(): this;

/**
* Sets the breadcrumbs in the scope
* @param breadcrumbs Breadcrumb
* Sets the breadcrumb in the scope
* @param breadcrumb Breadcrumb
* @param maxBreadcrumbs number of max breadcrumbs to merged into event.
*/
addBreadcrumb(breadcrumb: Breadcrumb, maxBreadcrumbs?: number): this;
Expand Down
12 changes: 11 additions & 1 deletion packages/vercel-edge/src/integrations/wintercg-fetch.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { addBreadcrumb, defineIntegration, getClient, instrumentFetchRequest, isSentryRequestUrl } from '@sentry/core';
import {
addBreadcrumb,
defineIntegration,
getBreadcrumbLogLevel,
getClient,
instrumentFetchRequest,
isSentryRequestUrl,
} from '@sentry/core';
import type {
Client,
FetchBreadcrumbData,
Expand Down Expand Up @@ -150,11 +157,14 @@ function createBreadcrumb(handlerData: HandlerDataFetch): void {
startTimestamp,
endTimestamp,
};
const level = getBreadcrumbLogLevel(data.status_code);

addBreadcrumb(
{
category: 'fetch',
data,
type: 'http',
level,
},
hint,
);
Expand Down
Loading