diff --git a/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts index 8e7fb9fce515..03262edcf6b5 100644 --- a/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts @@ -12,7 +12,6 @@ test('Sends an API route transaction', async ({ baseURL }) => { await fetch(`${baseURL}/test-transaction`); const transactionEvent = await pageloadTransactionEventPromise; - const transactionEventId = transactionEvent.event_id; expect(transactionEvent.contexts?.trace).toEqual({ data: { @@ -119,3 +118,84 @@ test('Sends an API route transaction', async ({ baseURL }) => { trace_id: expect.any(String), }); }); + +test('Sends an API route transaction for an errored route', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('node-express', transactionEvent => { + return ( + transactionEvent.contexts?.trace?.op === 'http.server' && + transactionEvent.transaction === 'GET /test-exception/:id' && + transactionEvent.request?.url === 'http://localhost:3030/test-exception/777' + ); + }); + + await fetch(`${baseURL}/test-exception/777`); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent.contexts?.trace?.op).toEqual('http.server'); + expect(transactionEvent.transaction).toEqual('GET /test-exception/:id'); + expect(transactionEvent.contexts?.trace?.status).toEqual('internal_error'); + expect(transactionEvent.contexts?.trace?.data?.['http.status_code']).toEqual(500); + + const spans = transactionEvent.spans || []; + + expect(spans).toContainEqual({ + data: { + 'sentry.origin': 'auto.http.otel.express', + 'sentry.op': 'middleware.express', + 'http.route': '/', + 'express.name': 'query', + 'express.type': 'middleware', + 'otel.kind': 'INTERNAL', + }, + description: 'query', + op: 'middleware.express', + origin: 'auto.http.otel.express', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + }); + + expect(spans).toContainEqual({ + data: { + 'sentry.origin': 'auto.http.otel.express', + 'sentry.op': 'middleware.express', + 'http.route': '/', + 'express.name': 'expressInit', + 'express.type': 'middleware', + 'otel.kind': 'INTERNAL', + }, + description: 'expressInit', + op: 'middleware.express', + origin: 'auto.http.otel.express', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + }); + + expect(spans).toContainEqual({ + data: { + 'sentry.origin': 'auto.http.otel.express', + 'sentry.op': 'request_handler.express', + 'http.route': '/test-exception/:id', + 'express.name': '/test-exception/:id', + 'express.type': 'request_handler', + 'otel.kind': 'INTERNAL', + }, + description: '/test-exception/:id', + op: 'request_handler.express', + origin: 'auto.http.otel.express', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + }); +}); diff --git a/packages/opentelemetry/src/utils/mapStatus.ts b/packages/opentelemetry/src/utils/mapStatus.ts index 1f65fcb985bb..8df486437d6a 100644 --- a/packages/opentelemetry/src/utils/mapStatus.ts +++ b/packages/opentelemetry/src/utils/mapStatus.ts @@ -1,7 +1,7 @@ import { SpanStatusCode } from '@opentelemetry/api'; import { SEMATTRS_HTTP_STATUS_CODE, SEMATTRS_RPC_GRPC_STATUS_CODE } from '@opentelemetry/semantic-conventions'; import { SPAN_STATUS_ERROR, SPAN_STATUS_OK, getSpanStatusFromHttpCode } from '@sentry/core'; -import type { SpanStatus } from '@sentry/types'; +import type { SpanAttributes, SpanStatus } from '@sentry/types'; import type { AbstractSpan } from '../types'; import { spanHasAttributes, spanHasStatus } from './spanTypes'; @@ -43,7 +43,14 @@ export function mapStatus(span: AbstractSpan): SpanStatus { return { code: SPAN_STATUS_OK }; // If the span is already marked as erroneous we return that exact status } else if (status.code === SpanStatusCode.ERROR) { - if (typeof status.message === 'undefined' || isStatusErrorMessageValid(status.message)) { + if (typeof status.message === 'undefined') { + const inferredStatus = inferStatusFromAttributes(attributes); + if (inferredStatus) { + return inferredStatus; + } + } + + if (status.message && isStatusErrorMessageValid(status.message)) { return { code: SPAN_STATUS_ERROR, message: status.message }; } else { return { code: SPAN_STATUS_ERROR, message: 'unknown_error' }; @@ -51,6 +58,22 @@ export function mapStatus(span: AbstractSpan): SpanStatus { } } + // If the span status is UNSET, we try to infer it from HTTP or GRPC status codes. + const inferredStatus = inferStatusFromAttributes(attributes); + + if (inferredStatus) { + return inferredStatus; + } + + // We default to setting the spans status to ok. + if (status && status.code === SpanStatusCode.UNSET) { + return { code: SPAN_STATUS_OK }; + } else { + return { code: SPAN_STATUS_ERROR, message: 'unknown_error' }; + } +} + +function inferStatusFromAttributes(attributes: SpanAttributes): SpanStatus | undefined { // If the span status is UNSET, we try to infer it from HTTP or GRPC status codes. const httpCodeAttribute = attributes[SEMATTRS_HTTP_STATUS_CODE]; @@ -63,7 +86,7 @@ export function mapStatus(span: AbstractSpan): SpanStatus { ? parseInt(httpCodeAttribute) : undefined; - if (numberHttpCode) { + if (typeof numberHttpCode === 'number') { return getSpanStatusFromHttpCode(numberHttpCode); } @@ -71,10 +94,5 @@ export function mapStatus(span: AbstractSpan): SpanStatus { return { code: SPAN_STATUS_ERROR, message: canonicalGrpcErrorCodesMap[grpcCodeAttribute] || 'unknown_error' }; } - // We default to setting the spans status to ok. - if (status && status.code === SpanStatusCode.UNSET) { - return { code: SPAN_STATUS_OK }; - } else { - return { code: SPAN_STATUS_ERROR, message: 'unknown_error' }; - } + return undefined; } diff --git a/packages/opentelemetry/test/utils/mapStatus.test.ts b/packages/opentelemetry/test/utils/mapStatus.test.ts index 8dcf55a9267b..1734586ee7a2 100644 --- a/packages/opentelemetry/test/utils/mapStatus.test.ts +++ b/packages/opentelemetry/test/utils/mapStatus.test.ts @@ -91,6 +91,19 @@ describe('mapStatus', () => { expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_ERROR, message: 'invalid_argument' }); }); + it('returns error status when span already has error status without message', () => { + const span = createSpan(); + span.setStatus({ code: 2 }); // ERROR + expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_ERROR, message: 'unknown_error' }); + }); + + it('infers error status form attributes when span already has error status without message', () => { + const span = createSpan(); + span.setAttribute(SEMATTRS_HTTP_STATUS_CODE, 500); + span.setStatus({ code: 2 }); // ERROR + expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + }); + it('returns unknown error status when code is unknown', () => { const span = createSpan(); span.setStatus({ code: -1 as 0 });