Skip to content

feat(core): Deprecate Span.setHttpStatus in favor of setHttpStatus #10268

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 4 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,7 @@ finished. This is useful for event emitters or similar.
function middleware(_req, res, next) {
return Sentry.startSpanManual({ name: 'middleware' }, (span, finish) => {
res.once('finish', () => {
span?.setHttpStatus(res.status);
setHttpStatus(span, res.status);
finish();
});
return next();
Expand Down
1 change: 1 addition & 0 deletions biome.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"dev-packages/browser-integration-tests/suites/**/*.json",
"dev-packages/browser-integration-tests/loader-suites/**/*.js",
"dev-packages/browser-integration-tests/suites/stacktraces/**/*.js",
".next/**/*",
"**/fixtures/*/*.json",
"**/*.min.js",
".next/**",
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/server/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, setHttpStatus } from '@sentry/core';
import {
captureException,
continueTrace,
Expand Down Expand Up @@ -142,7 +142,7 @@ async function instrumentRequest(
const originalResponse = await next();

if (span && originalResponse.status) {
span.setHttpStatus(originalResponse.status);
setHttpStatus(span, originalResponse.status);
}

const scope = getCurrentScope();
Expand Down
6 changes: 4 additions & 2 deletions packages/bun/src/integrations/bunserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
convertIntegrationFnToClass,
getCurrentScope,
runWithAsyncContext,
setHttpStatus,
startSpan,
} from '@sentry/core';
import type { IntegrationFn } from '@sentry/types';
Expand Down Expand Up @@ -93,8 +94,9 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
typeof serveOptions.fetch
>);
if (response && response.status) {
span?.setHttpStatus(response.status);
span?.setAttribute('http.response.status_code', response.status);
if (span) {
setHttpStatus(span, response.status);
}
if (span instanceof Transaction) {
const scope = getCurrentScope();
scope.setContext('response', {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tracing/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
} from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import type { SpanStatusType } from './span';
import type { SpanStatusType } from './spanstatus';
import { getActiveTransaction } from './utils';

let errorsInstrumented = false;
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/tracing/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
export { startIdleTransaction, addTracingExtensions } from './hubextensions';
export { IdleTransaction, TRACING_DEFAULTS } from './idletransaction';
export type { BeforeFinishCallback } from './idletransaction';
export { Span, spanStatusfromHttpCode } from './span';
export { Span } from './span';
export { Transaction } from './transaction';
// eslint-disable-next-line deprecation/deprecation
export { extractTraceparentData, getActiveTransaction } from './utils';
// eslint-disable-next-line deprecation/deprecation
export { SpanStatus } from './spanstatus';
export type { SpanStatusType } from './span';
export { setHttpStatus, spanStatusfromHttpCode } from './spanstatus';
export type { SpanStatusType } from './spanstatus';
export {
// eslint-disable-next-line deprecation/deprecation
trace,
Expand Down
94 changes: 4 additions & 90 deletions packages/core/src/tracing/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import {
spanToTraceContext,
spanToTraceHeader,
} from '../utils/spanUtils';
import type { SpanStatusType } from './spanstatus';
import { setHttpStatus } from './spanstatus';

/**
* Keeps track of finished spans for a given transaction
Expand Down Expand Up @@ -470,16 +472,10 @@ export class Span implements SpanInterface {

/**
* @inheritDoc
* @deprecated Use top-level `setHttpStatus()` instead.
*/
public setHttpStatus(httpStatus: number): this {
// eslint-disable-next-line deprecation/deprecation
this.setTag('http.status_code', String(httpStatus));
// eslint-disable-next-line deprecation/deprecation
this.setData('http.response.status_code', httpStatus);
const spanStatus = spanStatusfromHttpCode(httpStatus);
if (spanStatus !== 'unknown_error') {
this.setStatus(spanStatus);
}
setHttpStatus(this, httpStatus);
return this;
}

Expand Down Expand Up @@ -675,85 +671,3 @@ export class Span implements SpanInterface {
return hasData ? data : attributes;
}
}

export type SpanStatusType =
/** The operation completed successfully. */
| 'ok'
/** Deadline expired before operation could complete. */
| 'deadline_exceeded'
/** 401 Unauthorized (actually does mean unauthenticated according to RFC 7235) */
| 'unauthenticated'
/** 403 Forbidden */
| 'permission_denied'
/** 404 Not Found. Some requested entity (file or directory) was not found. */
| 'not_found'
/** 429 Too Many Requests */
| 'resource_exhausted'
/** Client specified an invalid argument. 4xx. */
| 'invalid_argument'
/** 501 Not Implemented */
| 'unimplemented'
/** 503 Service Unavailable */
| 'unavailable'
/** Other/generic 5xx. */
| 'internal_error'
/** Unknown. Any non-standard HTTP status code. */
| 'unknown_error'
/** The operation was cancelled (typically by the user). */
| 'cancelled'
/** Already exists (409) */
| 'already_exists'
/** Operation was rejected because the system is not in a state required for the operation's */
| 'failed_precondition'
/** The operation was aborted, typically due to a concurrency issue. */
| 'aborted'
/** Operation was attempted past the valid range. */
| 'out_of_range'
/** Unrecoverable data loss or corruption */
| 'data_loss';

/**
* Converts a HTTP status code into a {@link SpanStatusType}.
*
* @param httpStatus The HTTP response status code.
* @returns The span status or unknown_error.
*/
export function spanStatusfromHttpCode(httpStatus: number): SpanStatusType {
if (httpStatus < 400 && httpStatus >= 100) {
return 'ok';
}

if (httpStatus >= 400 && httpStatus < 500) {
switch (httpStatus) {
case 401:
return 'unauthenticated';
case 403:
return 'permission_denied';
case 404:
return 'not_found';
case 409:
return 'already_exists';
case 413:
return 'failed_precondition';
case 429:
return 'resource_exhausted';
default:
return 'invalid_argument';
}
}

if (httpStatus >= 500 && httpStatus < 600) {
switch (httpStatus) {
case 501:
return 'unimplemented';
case 503:
return 'unavailable';
case 504:
return 'deadline_exceeded';
default:
return 'internal_error';
}
}

return 'unknown_error';
}
107 changes: 107 additions & 0 deletions packages/core/src/tracing/spanstatus.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { Span } from '@sentry/types';

/** The status of an Span.
*
* @deprecated Use string literals - if you require type casting, cast to SpanStatusType type
Expand Down Expand Up @@ -38,3 +40,108 @@ export enum SpanStatus {
/** Unrecoverable data loss or corruption */
DataLoss = 'data_loss',
}

export type SpanStatusType =
/** The operation completed successfully. */
| 'ok'
/** Deadline expired before operation could complete. */
| 'deadline_exceeded'
/** 401 Unauthorized (actually does mean unauthenticated according to RFC 7235) */
| 'unauthenticated'
/** 403 Forbidden */
| 'permission_denied'
/** 404 Not Found. Some requested entity (file or directory) was not found. */
| 'not_found'
/** 429 Too Many Requests */
| 'resource_exhausted'
/** Client specified an invalid argument. 4xx. */
| 'invalid_argument'
/** 501 Not Implemented */
| 'unimplemented'
/** 503 Service Unavailable */
| 'unavailable'
/** Other/generic 5xx. */
| 'internal_error'
/** Unknown. Any non-standard HTTP status code. */
| 'unknown_error'
/** The operation was cancelled (typically by the user). */
| 'cancelled'
/** Already exists (409) */
| 'already_exists'
/** Operation was rejected because the system is not in a state required for the operation's */
| 'failed_precondition'
/** The operation was aborted, typically due to a concurrency issue. */
| 'aborted'
/** Operation was attempted past the valid range. */
| 'out_of_range'
/** Unrecoverable data loss or corruption */
| 'data_loss';

/**
* Converts a HTTP status code into a {@link SpanStatusType}.
*
* @param httpStatus The HTTP response status code.
* @returns The span status or unknown_error.
*/
export function spanStatusfromHttpCode(httpStatus: number): SpanStatusType {
if (httpStatus < 400 && httpStatus >= 100) {
return 'ok';
}

if (httpStatus >= 400 && httpStatus < 500) {
switch (httpStatus) {
case 401:
return 'unauthenticated';
case 403:
return 'permission_denied';
case 404:
return 'not_found';
case 409:
return 'already_exists';
case 413:
return 'failed_precondition';
case 429:
return 'resource_exhausted';
default:
return 'invalid_argument';
}
}

if (httpStatus >= 500 && httpStatus < 600) {
switch (httpStatus) {
case 501:
return 'unimplemented';
case 503:
return 'unavailable';
case 504:
return 'deadline_exceeded';
default:
return 'internal_error';
}
}

return 'unknown_error';
}

/**
* Sets the Http status attributes on the current span based on the http code.
* Additionally, the span's status is updated, depending on the http code.
*/
export function setHttpStatus(span: Span, httpStatus: number): void {
// TODO (v8): Remove these calls
// Relay does not require us to send the status code as a tag
// For now, just because users might expect it to land as a tag we keep sending it.
// Same with data.
// In v8, we replace both, simply with
// span.setAttribute('http.response.status_code', httpStatus);

// eslint-disable-next-line deprecation/deprecation
span.setTag('http.status_code', String(httpStatus));
// eslint-disable-next-line deprecation/deprecation
span.setData('http.response.status_code', httpStatus);

const spanStatus = spanStatusfromHttpCode(httpStatus);
if (spanStatus !== 'unknown_error') {
span.setStatus(spanStatus);
}
}
41 changes: 41 additions & 0 deletions packages/core/test/lib/tracing/spanstatus.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Span, setHttpStatus, spanToJSON } from '../../../src/index';

describe('setHttpStatus', () => {
it.each([
[200, 'ok'],
[300, 'ok'],
[401, 'unauthenticated'],
[403, 'permission_denied'],
[404, 'not_found'],
[409, 'already_exists'],
[413, 'failed_precondition'],
[429, 'resource_exhausted'],
[455, 'invalid_argument'],
[501, 'unimplemented'],
[503, 'unavailable'],
[504, 'deadline_exceeded'],
[520, 'internal_error'],
])('applies the correct span status and http status code to the span (%s - $%s)', (code, status) => {
const span = new Span({ name: 'test' });

setHttpStatus(span!, code);

const { status: spanStatus, data, tags } = spanToJSON(span!);

expect(spanStatus).toBe(status);
expect(data).toMatchObject({ 'http.response.status_code': code });
expect(tags).toMatchObject({ 'http.status_code': String(code) });
});

it("doesn't set the status for an unknown http status code", () => {
const span = new Span({ name: 'test' });

setHttpStatus(span!, 600);

const { status: spanStatus, data, tags } = spanToJSON(span!);

expect(spanStatus).toBeUndefined();
expect(data).toMatchObject({ 'http.response.status_code': 600 });
expect(tags).toMatchObject({ 'http.status_code': '600' });
});
});
11 changes: 7 additions & 4 deletions packages/nextjs/src/common/utils/edgeWrapperUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
captureException,
continueTrace,
handleCallbackErrors,
setHttpStatus,
startSpan,
} from '@sentry/core';
import { winterCGRequestToRequestData } from '@sentry/utils';
Expand Down Expand Up @@ -67,10 +68,12 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
},
);

if (handlerResult instanceof Response) {
span?.setHttpStatus(handlerResult.status);
} else {
span?.setStatus('ok');
if (span) {
if (handlerResult instanceof Response) {
setHttpStatus(span, handlerResult.status);
} else {
span.setStatus('ok');
}
}

return handlerResult;
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/common/utils/responseEnd.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ServerResponse } from 'http';
import { flush } from '@sentry/core';
import { flush, setHttpStatus } from '@sentry/core';
import type { Transaction } from '@sentry/types';
import { fill, logger } from '@sentry/utils';

Expand Down Expand Up @@ -41,7 +41,7 @@ export function autoEndTransactionOnResponseEnd(transaction: Transaction, res: S
/** Finish the given response's transaction and set HTTP status data */
export function finishTransaction(transaction: Transaction | undefined, res: ServerResponse): void {
if (transaction) {
transaction.setHttpStatus(res.statusCode);
setHttpStatus(transaction, res.statusCode);
transaction.end();
}
}
Expand Down
Loading