Skip to content

Commit 8554dca

Browse files
authored
feat(core): Deprecate Span.setHttpStatus in favor of setHttpStatus (#10268)
Deprecate the last remaining otel-incompatible API on the `Span` class and interface - `Span.setHttpStatus`. We replace this functionality with a `setHttpStatus` wrapper because in addition to setting the data/tag value (more on that below), this function also derives the span status from the status code. Added tests to make sure this is covered properly (I think it wasn't covered well before and we need to remove the old tests anyway).
1 parent 0612090 commit 8554dca

File tree

26 files changed

+225
-126
lines changed

26 files changed

+225
-126
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1103,7 +1103,7 @@ finished. This is useful for event emitters or similar.
11031103
function middleware(_req, res, next) {
11041104
return Sentry.startSpanManual({ name: 'middleware' }, (span, finish) => {
11051105
res.once('finish', () => {
1106-
span?.setHttpStatus(res.status);
1106+
setHttpStatus(span, res.status);
11071107
finish();
11081108
});
11091109
return next();

biome.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"dev-packages/browser-integration-tests/suites/**/*.json",
4848
"dev-packages/browser-integration-tests/loader-suites/**/*.js",
4949
"dev-packages/browser-integration-tests/suites/stacktraces/**/*.js",
50+
".next/**/*",
5051
"**/fixtures/*/*.json",
5152
"**/*.min.js",
5253
".next/**",

packages/astro/src/server/middleware.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, setHttpStatus } from '@sentry/core';
22
import {
33
captureException,
44
continueTrace,
@@ -142,7 +142,7 @@ async function instrumentRequest(
142142
const originalResponse = await next();
143143

144144
if (span && originalResponse.status) {
145-
span.setHttpStatus(originalResponse.status);
145+
setHttpStatus(span, originalResponse.status);
146146
}
147147

148148
const scope = getCurrentScope();

packages/bun/src/integrations/bunserver.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
convertIntegrationFnToClass,
88
getCurrentScope,
99
runWithAsyncContext,
10+
setHttpStatus,
1011
startSpan,
1112
} from '@sentry/core';
1213
import type { IntegrationFn } from '@sentry/types';
@@ -93,8 +94,9 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
9394
typeof serveOptions.fetch
9495
>);
9596
if (response && response.status) {
96-
span?.setHttpStatus(response.status);
97-
span?.setAttribute('http.response.status_code', response.status);
97+
if (span) {
98+
setHttpStatus(span, response.status);
99+
}
98100
if (span instanceof Transaction) {
99101
const scope = getCurrentScope();
100102
scope.setContext('response', {

packages/core/src/tracing/errors.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
} from '@sentry/utils';
66

77
import { DEBUG_BUILD } from '../debug-build';
8-
import type { SpanStatusType } from './span';
8+
import type { SpanStatusType } from './spanstatus';
99
import { getActiveTransaction } from './utils';
1010

1111
let errorsInstrumented = false;

packages/core/src/tracing/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
export { startIdleTransaction, addTracingExtensions } from './hubextensions';
22
export { IdleTransaction, TRACING_DEFAULTS } from './idletransaction';
33
export type { BeforeFinishCallback } from './idletransaction';
4-
export { Span, spanStatusfromHttpCode } from './span';
4+
export { Span } from './span';
55
export { Transaction } from './transaction';
66
// eslint-disable-next-line deprecation/deprecation
77
export { extractTraceparentData, getActiveTransaction } from './utils';
88
// eslint-disable-next-line deprecation/deprecation
99
export { SpanStatus } from './spanstatus';
10-
export type { SpanStatusType } from './span';
10+
export { setHttpStatus, spanStatusfromHttpCode } from './spanstatus';
11+
export type { SpanStatusType } from './spanstatus';
1112
export {
1213
// eslint-disable-next-line deprecation/deprecation
1314
trace,

packages/core/src/tracing/span.ts

Lines changed: 4 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import {
2626
spanToTraceContext,
2727
spanToTraceHeader,
2828
} from '../utils/spanUtils';
29+
import type { SpanStatusType } from './spanstatus';
30+
import { setHttpStatus } from './spanstatus';
2931

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

471473
/**
472474
* @inheritDoc
475+
* @deprecated Use top-level `setHttpStatus()` instead.
473476
*/
474477
public setHttpStatus(httpStatus: number): this {
475-
// eslint-disable-next-line deprecation/deprecation
476-
this.setTag('http.status_code', String(httpStatus));
477-
// eslint-disable-next-line deprecation/deprecation
478-
this.setData('http.response.status_code', httpStatus);
479-
const spanStatus = spanStatusfromHttpCode(httpStatus);
480-
if (spanStatus !== 'unknown_error') {
481-
this.setStatus(spanStatus);
482-
}
478+
setHttpStatus(this, httpStatus);
483479
return this;
484480
}
485481

@@ -675,85 +671,3 @@ export class Span implements SpanInterface {
675671
return hasData ? data : attributes;
676672
}
677673
}
678-
679-
export type SpanStatusType =
680-
/** The operation completed successfully. */
681-
| 'ok'
682-
/** Deadline expired before operation could complete. */
683-
| 'deadline_exceeded'
684-
/** 401 Unauthorized (actually does mean unauthenticated according to RFC 7235) */
685-
| 'unauthenticated'
686-
/** 403 Forbidden */
687-
| 'permission_denied'
688-
/** 404 Not Found. Some requested entity (file or directory) was not found. */
689-
| 'not_found'
690-
/** 429 Too Many Requests */
691-
| 'resource_exhausted'
692-
/** Client specified an invalid argument. 4xx. */
693-
| 'invalid_argument'
694-
/** 501 Not Implemented */
695-
| 'unimplemented'
696-
/** 503 Service Unavailable */
697-
| 'unavailable'
698-
/** Other/generic 5xx. */
699-
| 'internal_error'
700-
/** Unknown. Any non-standard HTTP status code. */
701-
| 'unknown_error'
702-
/** The operation was cancelled (typically by the user). */
703-
| 'cancelled'
704-
/** Already exists (409) */
705-
| 'already_exists'
706-
/** Operation was rejected because the system is not in a state required for the operation's */
707-
| 'failed_precondition'
708-
/** The operation was aborted, typically due to a concurrency issue. */
709-
| 'aborted'
710-
/** Operation was attempted past the valid range. */
711-
| 'out_of_range'
712-
/** Unrecoverable data loss or corruption */
713-
| 'data_loss';
714-
715-
/**
716-
* Converts a HTTP status code into a {@link SpanStatusType}.
717-
*
718-
* @param httpStatus The HTTP response status code.
719-
* @returns The span status or unknown_error.
720-
*/
721-
export function spanStatusfromHttpCode(httpStatus: number): SpanStatusType {
722-
if (httpStatus < 400 && httpStatus >= 100) {
723-
return 'ok';
724-
}
725-
726-
if (httpStatus >= 400 && httpStatus < 500) {
727-
switch (httpStatus) {
728-
case 401:
729-
return 'unauthenticated';
730-
case 403:
731-
return 'permission_denied';
732-
case 404:
733-
return 'not_found';
734-
case 409:
735-
return 'already_exists';
736-
case 413:
737-
return 'failed_precondition';
738-
case 429:
739-
return 'resource_exhausted';
740-
default:
741-
return 'invalid_argument';
742-
}
743-
}
744-
745-
if (httpStatus >= 500 && httpStatus < 600) {
746-
switch (httpStatus) {
747-
case 501:
748-
return 'unimplemented';
749-
case 503:
750-
return 'unavailable';
751-
case 504:
752-
return 'deadline_exceeded';
753-
default:
754-
return 'internal_error';
755-
}
756-
}
757-
758-
return 'unknown_error';
759-
}

packages/core/src/tracing/spanstatus.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type { Span } from '@sentry/types';
2+
13
/** The status of an Span.
24
*
35
* @deprecated Use string literals - if you require type casting, cast to SpanStatusType type
@@ -38,3 +40,108 @@ export enum SpanStatus {
3840
/** Unrecoverable data loss or corruption */
3941
DataLoss = 'data_loss',
4042
}
43+
44+
export type SpanStatusType =
45+
/** The operation completed successfully. */
46+
| 'ok'
47+
/** Deadline expired before operation could complete. */
48+
| 'deadline_exceeded'
49+
/** 401 Unauthorized (actually does mean unauthenticated according to RFC 7235) */
50+
| 'unauthenticated'
51+
/** 403 Forbidden */
52+
| 'permission_denied'
53+
/** 404 Not Found. Some requested entity (file or directory) was not found. */
54+
| 'not_found'
55+
/** 429 Too Many Requests */
56+
| 'resource_exhausted'
57+
/** Client specified an invalid argument. 4xx. */
58+
| 'invalid_argument'
59+
/** 501 Not Implemented */
60+
| 'unimplemented'
61+
/** 503 Service Unavailable */
62+
| 'unavailable'
63+
/** Other/generic 5xx. */
64+
| 'internal_error'
65+
/** Unknown. Any non-standard HTTP status code. */
66+
| 'unknown_error'
67+
/** The operation was cancelled (typically by the user). */
68+
| 'cancelled'
69+
/** Already exists (409) */
70+
| 'already_exists'
71+
/** Operation was rejected because the system is not in a state required for the operation's */
72+
| 'failed_precondition'
73+
/** The operation was aborted, typically due to a concurrency issue. */
74+
| 'aborted'
75+
/** Operation was attempted past the valid range. */
76+
| 'out_of_range'
77+
/** Unrecoverable data loss or corruption */
78+
| 'data_loss';
79+
80+
/**
81+
* Converts a HTTP status code into a {@link SpanStatusType}.
82+
*
83+
* @param httpStatus The HTTP response status code.
84+
* @returns The span status or unknown_error.
85+
*/
86+
export function spanStatusfromHttpCode(httpStatus: number): SpanStatusType {
87+
if (httpStatus < 400 && httpStatus >= 100) {
88+
return 'ok';
89+
}
90+
91+
if (httpStatus >= 400 && httpStatus < 500) {
92+
switch (httpStatus) {
93+
case 401:
94+
return 'unauthenticated';
95+
case 403:
96+
return 'permission_denied';
97+
case 404:
98+
return 'not_found';
99+
case 409:
100+
return 'already_exists';
101+
case 413:
102+
return 'failed_precondition';
103+
case 429:
104+
return 'resource_exhausted';
105+
default:
106+
return 'invalid_argument';
107+
}
108+
}
109+
110+
if (httpStatus >= 500 && httpStatus < 600) {
111+
switch (httpStatus) {
112+
case 501:
113+
return 'unimplemented';
114+
case 503:
115+
return 'unavailable';
116+
case 504:
117+
return 'deadline_exceeded';
118+
default:
119+
return 'internal_error';
120+
}
121+
}
122+
123+
return 'unknown_error';
124+
}
125+
126+
/**
127+
* Sets the Http status attributes on the current span based on the http code.
128+
* Additionally, the span's status is updated, depending on the http code.
129+
*/
130+
export function setHttpStatus(span: Span, httpStatus: number): void {
131+
// TODO (v8): Remove these calls
132+
// Relay does not require us to send the status code as a tag
133+
// For now, just because users might expect it to land as a tag we keep sending it.
134+
// Same with data.
135+
// In v8, we replace both, simply with
136+
// span.setAttribute('http.response.status_code', httpStatus);
137+
138+
// eslint-disable-next-line deprecation/deprecation
139+
span.setTag('http.status_code', String(httpStatus));
140+
// eslint-disable-next-line deprecation/deprecation
141+
span.setData('http.response.status_code', httpStatus);
142+
143+
const spanStatus = spanStatusfromHttpCode(httpStatus);
144+
if (spanStatus !== 'unknown_error') {
145+
span.setStatus(spanStatus);
146+
}
147+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { Span, setHttpStatus, spanToJSON } from '../../../src/index';
2+
3+
describe('setHttpStatus', () => {
4+
it.each([
5+
[200, 'ok'],
6+
[300, 'ok'],
7+
[401, 'unauthenticated'],
8+
[403, 'permission_denied'],
9+
[404, 'not_found'],
10+
[409, 'already_exists'],
11+
[413, 'failed_precondition'],
12+
[429, 'resource_exhausted'],
13+
[455, 'invalid_argument'],
14+
[501, 'unimplemented'],
15+
[503, 'unavailable'],
16+
[504, 'deadline_exceeded'],
17+
[520, 'internal_error'],
18+
])('applies the correct span status and http status code to the span (%s - $%s)', (code, status) => {
19+
const span = new Span({ name: 'test' });
20+
21+
setHttpStatus(span!, code);
22+
23+
const { status: spanStatus, data, tags } = spanToJSON(span!);
24+
25+
expect(spanStatus).toBe(status);
26+
expect(data).toMatchObject({ 'http.response.status_code': code });
27+
expect(tags).toMatchObject({ 'http.status_code': String(code) });
28+
});
29+
30+
it("doesn't set the status for an unknown http status code", () => {
31+
const span = new Span({ name: 'test' });
32+
33+
setHttpStatus(span!, 600);
34+
35+
const { status: spanStatus, data, tags } = spanToJSON(span!);
36+
37+
expect(spanStatus).toBeUndefined();
38+
expect(data).toMatchObject({ 'http.response.status_code': 600 });
39+
expect(tags).toMatchObject({ 'http.status_code': '600' });
40+
});
41+
});

packages/nextjs/src/common/utils/edgeWrapperUtils.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
captureException,
66
continueTrace,
77
handleCallbackErrors,
8+
setHttpStatus,
89
startSpan,
910
} from '@sentry/core';
1011
import { winterCGRequestToRequestData } from '@sentry/utils';
@@ -67,10 +68,12 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
6768
},
6869
);
6970

70-
if (handlerResult instanceof Response) {
71-
span?.setHttpStatus(handlerResult.status);
72-
} else {
73-
span?.setStatus('ok');
71+
if (span) {
72+
if (handlerResult instanceof Response) {
73+
setHttpStatus(span, handlerResult.status);
74+
} else {
75+
span.setStatus('ok');
76+
}
7477
}
7578

7679
return handlerResult;

packages/nextjs/src/common/utils/responseEnd.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ServerResponse } from 'http';
2-
import { flush } from '@sentry/core';
2+
import { flush, setHttpStatus } from '@sentry/core';
33
import type { Transaction } from '@sentry/types';
44
import { fill, logger } from '@sentry/utils';
55

@@ -41,7 +41,7 @@ export function autoEndTransactionOnResponseEnd(transaction: Transaction, res: S
4141
/** Finish the given response's transaction and set HTTP status data */
4242
export function finishTransaction(transaction: Transaction | undefined, res: ServerResponse): void {
4343
if (transaction) {
44-
transaction.setHttpStatus(res.statusCode);
44+
setHttpStatus(transaction, res.statusCode);
4545
transaction.end();
4646
}
4747
}

0 commit comments

Comments
 (0)