Skip to content
Open
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
10 changes: 8 additions & 2 deletions packages/core/src/tracing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,14 @@ export { startIdleSpan, TRACING_DEFAULTS } from './idleSpan';
export { SentrySpan } from './sentrySpan';
export { _INTERNAL_setDeferSegmentSpanCapture } from './deferSegmentSpanCapture';
export { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
export { setHttpStatus, getSpanStatusFromHttpCode } from './spanstatus';
export { SPAN_STATUS_ERROR, SPAN_STATUS_OK, SPAN_STATUS_UNSET } from './spanstatus';
export {
setHttpStatus,
getSpanStatusFromHttpCode,
isStatusErrorMessageValid,
SPAN_STATUS_ERROR,
SPAN_STATUS_OK,
SPAN_STATUS_UNSET,
} from './spanstatus';
export {
startSpan,
startInactiveSpan,
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/tracing/spanstatus.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import type { Span } from '../types/span';
import type { SpanStatus } from '../types/spanStatus';
import type { SpanStatusType } from '../types/spanStatus';
import { SPAN_STATUS_TYPES, type SpanStatus } from '../types/spanStatus';

export const SPAN_STATUS_UNSET = 0;
export const SPAN_STATUS_OK = 1;
export const SPAN_STATUS_ERROR = 2;

export function isStatusErrorMessageValid(message: string): boolean {
return SPAN_STATUS_TYPES.includes(message as SpanStatusType);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ok accepted as error status

Low Severity

isStatusErrorMessageValid now checks membership in SPAN_STATUS_TYPES, which includes 'ok'. For a span with error code and message 'ok', getStatusMessage and mapStatus treat that message as valid, so legacy transaction serialization can emit status: 'ok' for an error span instead of normalizing to 'internal_error'.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7bd130c. Configure here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is legit


/**
* Converts a HTTP status code into a sentry status with a message.
*
Expand Down
39 changes: 21 additions & 18 deletions packages/core/src/types/spanStatus.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,41 @@
export type SpanStatusType =
export const SPAN_STATUS_TYPES = [
/** The operation completed successfully. */
| 'ok'
'ok',
/** Deadline expired before operation could complete. */
| 'deadline_exceeded'
'deadline_exceeded',
/** 401 Unauthorized (actually does mean unauthenticated according to RFC 7235) */
| 'unauthenticated'
'unauthenticated',
/** 403 Forbidden */
| 'permission_denied'
'permission_denied',
/** 404 Not Found. Some requested entity (file or directory) was not found. */
| 'not_found'
'not_found',
/** 429 Too Many Requests */
| 'resource_exhausted'
'resource_exhausted',
/** Client specified an invalid argument. 4xx. */
| 'invalid_argument'
'invalid_argument',
/** 501 Not Implemented */
| 'unimplemented'
'unimplemented',
/** 503 Service Unavailable */
| 'unavailable'
'unavailable',
/** Other/generic 5xx. */
| 'internal_error'
'internal_error',
/** Unknown. Any non-standard HTTP status code. */
| 'unknown_error'
'unknown_error',
/** The operation was cancelled (typically by the user). */
| 'cancelled'
'cancelled',
/** Already exists (409) */
| 'already_exists'
'already_exists',
/** Operation was rejected because the system is not in a state required for the operation's */
| 'failed_precondition'
'failed_precondition',
/** The operation was aborted, typically due to a concurrency issue. */
| 'aborted'
'aborted',
/** Operation was attempted past the valid range. */
| 'out_of_range'
'out_of_range',
/** Unrecoverable data loss or corruption */
| 'data_loss';
'data_loss',
] as const;

export type SpanStatusType = (typeof SPAN_STATUS_TYPES)[number];

// These are aligned with OpenTelemetry span status codes
const SPAN_STATUS_UNSET = 0;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/utils/spanUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
SEMANTIC_ATTRIBUTE_SENTRY_STATUS_MESSAGE,
} from '../semanticAttributes';
import type { SentrySpan } from '../tracing/sentrySpan';
import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
import { isStatusErrorMessageValid, SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
import { getCapturedScopesOnSpan } from '../tracing/utils';
import type { TraceContext } from '../types/context';
import type { SpanLink, SpanLinkJSON } from '../types/link';
Expand Down Expand Up @@ -326,7 +326,7 @@ export function getStatusMessage(status: SpanStatus | undefined): string | undef
return 'ok';
}

return status.message || 'internal_error';
return status.message && isStatusErrorMessageValid(status.message) ? status.message : 'internal_error';
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/lib/tracing/sentrySpan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe('SentrySpan', () => {
describe('tracer-provider span sealing', () => {
it('seals a tracer-provider span against all mutation after it ends', () => {
const span = new SentrySpan({ name: 'original', startTimestamp: 1, attributes: { key: 'before' } });
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'before' });
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'permission_denied' });
span.addEvent('measurement', {
[SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: 'millisecond',
Expand All @@ -156,7 +156,7 @@ describe('SentrySpan', () => {
// Every mutator must no-op on a tracer-provider span once it has ended, mirroring OTel SDK spans.
span.setAttribute('key', 'after');
span.setAttributes({ key2: 'after' });
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'after' });
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'already_exists' });
span.updateName('after');
span.updateStartTime(999);
span.addLink({ context: linked.spanContext() });
Expand All @@ -169,7 +169,7 @@ describe('SentrySpan', () => {
const json = spanToJSON(span);
expect(json.data?.['key']).toBe('before');
expect(json.data?.['key2']).toBeUndefined();
expect(json.status).toBe('before');
expect(json.status).toBe('permission_denied');
expect(json.description).toBe('original');
expect(json.start_timestamp).toBe(1);
expect(json.links).toBeUndefined();
Expand Down
2 changes: 1 addition & 1 deletion packages/deno/test/deno-redis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ Deno.test('denoRedisIntegration: errors on the command channel set span status',
// as `status: 'X'` (the message takes the slot). Both "not ok" and the
// forwarded message confirm the error path fired.
assert(redisSpan!.status && redisSpan!.status !== 'ok', `expected error-shaped status, got ${redisSpan!.status}`);
assertEquals(redisSpan!.status, 'ECONNREFUSED');
assertEquals(redisSpan!.status, 'internal_error');
});

Deno.test('denoRedisIntegration: ioredis:command channel produces a db.redis child span', async () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/opentelemetry/src/applyOtelSpanData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import {
spanToJSON,
SPAN_STATUS_ERROR,
SPAN_STATUS_OK,
isStatusErrorMessageValid,
} from '@sentry/core';
import type { Span, SpanAttributes } from '@sentry/core';
import { inferStatusFromAttributes, isStatusErrorMessageValid } from './utils/mapStatus';
import { inferStatusFromAttributes } from './utils/mapStatus';
import { inferSpanData } from './utils/parseSpanDescription';

type SentrySpanWithOtelKind = Span & { kind?: SpanKind };
Expand Down
6 changes: 1 addition & 5 deletions packages/opentelemetry/src/utils/mapStatus.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { SpanStatusCode } from '@opentelemetry/api';
import { HTTP_RESPONSE_STATUS_CODE, HTTP_STATUS_CODE, RPC_GRPC_STATUS_CODE } from '@sentry/conventions/attributes';
import type { SpanAttributes, SpanStatus } from '@sentry/core';
import { getSpanStatusFromHttpCode, SPAN_STATUS_ERROR, SPAN_STATUS_OK } from '@sentry/core';
import { getSpanStatusFromHttpCode, isStatusErrorMessageValid, SPAN_STATUS_ERROR, SPAN_STATUS_OK } from '@sentry/core';
import type { AbstractSpan } from '../types';
import { spanHasAttributes, spanHasStatus } from './spanTypes';

Expand All @@ -25,10 +25,6 @@ const canonicalGrpcErrorCodesMap: Record<string, SpanStatus['message']> = {
'16': 'unauthenticated',
} as const;

export const isStatusErrorMessageValid = (message: string): boolean => {
return Object.values(canonicalGrpcErrorCodesMap).includes(message as SpanStatus['message']);
};

/**
* Get a Sentry span status from an otel span.
*/
Expand Down
9 changes: 6 additions & 3 deletions packages/opentelemetry/test/tracerProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getCapturedScopesOnSpan,
getRootSpan,
spanToJSON,
spanToStreamedSpanJSON,
SPAN_STATUS_ERROR,
SPAN_STATUS_OK,
startSpanManual,
Expand Down Expand Up @@ -194,15 +195,17 @@ describe('SentryTracerProvider', () => {

it('preserves a non-canonical error status message under span streaming', () => {
// Under streaming the streamed serializer surfaces the raw message as `sentry.status.message`, so
// finalizing must not normalize it to `internal_error` the way it does for the non-streamed
// transaction status field. Without streaming, `finalizes span statuses` covers the `internal_error` case.
// finalizing must not overwrite the live span status. The transaction `status` field is always
// normalized to a valid value (`internal_error`), but the raw message survives on the streamed span.
initTestClient({ tracesSampleRate: 1, traceLifecycle: 'stream' });
const span = trace.getTracer('test').startSpan('db-error');
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'Cannot enqueue Query after fatal error.' });

applyOtelSpanData(span as Span, { finalizeStatus: true });

expect(spanToJSON(span as Span).status).toBe('Cannot enqueue Query after fatal error.');
const streamed = spanToStreamedSpanJSON(span as Span);
expect(streamed.status).toBe('error');
expect(streamed.attributes?.['sentry.status.message']).toBe('Cannot enqueue Query after fatal error.');
});

it('infers route source, op, and name for HTTP server spans', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ describe('subscribeMysql2DiagnosticChannels', () => {
{ error: new Error('table missing') },
);

expect(spanToJSON(span!).status).toBe('table missing');
expect(spanToJSON(span!).status).toBe('internal_error');
expect(spanToJSON(span!).timestamp).toBeDefined();
expect(captureExceptionSpy).not.toHaveBeenCalled();
});
Expand Down
31 changes: 18 additions & 13 deletions packages/server-utils/test/tracing-channel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
resolvedSyncPromise,
setAsyncContextStrategy,
spanToJSON,
spanToStreamedSpanJSON,
startInactiveSpan,
startSpan,
} from '@sentry/core';
Expand Down Expand Up @@ -292,7 +293,7 @@ describe('bindTracingChannelToSpan', () => {
).toThrow(error);

expect(endSpy).toHaveBeenCalledTimes(1);
expect(spanToJSON(span).status).toBe('sync-throw');
expect(spanToJSON(span).status).toBe('internal_error');
expect(captureExceptionSpy).not.toHaveBeenCalled();
});

Expand Down Expand Up @@ -361,7 +362,7 @@ describe('bindTracingChannelToSpan', () => {
await expect(promise).rejects.toThrow(error);

expect(endSpy).toHaveBeenCalledTimes(1);
expect(spanToJSON(span).status).toBe('async-reject');
expect(spanToJSON(span).status).toBe('internal_error');
expect(captureExceptionSpy).not.toHaveBeenCalled();
});

Expand All @@ -379,7 +380,7 @@ describe('bindTracingChannelToSpan', () => {
).toThrow(error);

expect(endSpy).toHaveBeenCalledTimes(1);
expect(spanToJSON(span).status).toBe('promise-sync-throw');
expect(spanToJSON(span).status).toBe('internal_error');
expect(captureExceptionSpy).not.toHaveBeenCalled();
});

Expand Down Expand Up @@ -421,7 +422,7 @@ describe('bindTracingChannelToSpan', () => {
});

expect(endSpy).toHaveBeenCalledTimes(1);
expect(spanToJSON(span).status).toBe('callback-error');
expect(spanToJSON(span).status).toBe('internal_error');
expect(captureExceptionSpy).not.toHaveBeenCalled();
});

Expand All @@ -442,7 +443,7 @@ describe('bindTracingChannelToSpan', () => {
).toThrow(error);

expect(endSpy).toHaveBeenCalledTimes(1);
expect(spanToJSON(span).status).toBe('callback-sync-throw');
expect(spanToJSON(span).status).toBe('internal_error');
expect(captureExceptionSpy).not.toHaveBeenCalled();
});

Expand All @@ -459,8 +460,11 @@ describe('bindTracingChannelToSpan', () => {
),
).toThrow('bad input');

// The transaction status field is normalized to a valid value; the raw message survives on
// the streamed span as `sentry.status.message`.
const { status, data } = spanToJSON(span);
expect(status).toBe('bad input');
expect(status).toBe('internal_error');
expect(spanToStreamedSpanJSON(span).attributes?.['sentry.status.message']).toBe('bad input');
expect(data['error.type']).toBe('TypeError');
});

Expand All @@ -477,7 +481,8 @@ describe('bindTracingChannelToSpan', () => {
).toThrow('plain failure');

const { status, data } = spanToJSON(span);
expect(status).toBe('plain failure');
expect(status).toBe('internal_error');
expect(spanToStreamedSpanJSON(span).attributes?.['sentry.status.message']).toBe('plain failure');
expect(data['error.type']).toBe('unknown');
});

Expand Down Expand Up @@ -546,7 +551,7 @@ describe('bindTracingChannelToSpan', () => {
).rejects.toThrow(error);

expect(captureExceptionSpy).not.toHaveBeenCalled();
expect(spanToJSON(span).status).toBe('db-down');
expect(spanToJSON(span).status).toBe('internal_error');
expect(spanToJSON(span).timestamp).toBeDefined();
});

Expand Down Expand Up @@ -576,7 +581,7 @@ describe('bindTracingChannelToSpan', () => {
expect(captureExceptionSpy).toHaveBeenCalledWith(error, {
mechanism: { type: 'auto.diagnostic_channels.bind_span', handled: false },
});
expect(spanToJSON(span).status).toBe('boom');
expect(spanToJSON(span).status).toBe('internal_error');
});

it('captures the exception on the synchronous error path when `captureError` is true', () => {
Expand Down Expand Up @@ -605,7 +610,7 @@ describe('bindTracingChannelToSpan', () => {
expect(captureExceptionSpy).toHaveBeenCalledWith(error, {
mechanism: { type: 'auto.diagnostic_channels.bind_span', handled: false },
});
expect(spanToJSON(span).status).toBe('sync-boom');
expect(spanToJSON(span).status).toBe('internal_error');
});

it('captures the exception on the callback error path when `captureError` is true', async () => {
Expand Down Expand Up @@ -637,7 +642,7 @@ describe('bindTracingChannelToSpan', () => {
expect(captureExceptionSpy).toHaveBeenCalledWith(error, {
mechanism: { type: 'auto.diagnostic_channels.bind_span', handled: false },
});
expect(spanToJSON(span).status).toBe('callback-boom');
expect(spanToJSON(span).status).toBe('internal_error');
});

it('captures the exception with the hint returned by a `captureError` function, passing it the thrown error', async () => {
Expand Down Expand Up @@ -669,7 +674,7 @@ describe('bindTracingChannelToSpan', () => {
expect(captureExceptionSpy).toHaveBeenCalledWith(error, {
mechanism: { type: 'auto.http.custom', handled: false },
});
expect(spanToJSON(span).status).toBe('boom');
expect(spanToJSON(span).status).toBe('internal_error');
});

it('uses the default mechanism when `captureError` is a function on the synchronous error path', () => {
Expand Down Expand Up @@ -826,7 +831,7 @@ describe('bindTracingChannelToSpan', () => {
it('`end(error)` sets error status and the `error.type` attribute, then ends', () => {
const { span, endSpy, end } = setupDeferred('test:defer:error');
end(new TypeError('stream blew up'));
expect(spanToJSON(span).status).toBe('stream blew up');
expect(spanToJSON(span).status).toBe('internal_error');
expect(spanToJSON(span).data['error.type']).toBe('TypeError');
expect(endSpy).toHaveBeenCalledTimes(1);
});
Expand Down
Loading