diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index aea35b7835ce..9ae0a994ed48 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -1,20 +1,24 @@ /* eslint-disable max-lines */ import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + continueTrace, getActiveSpan, getActiveTransaction, getClient, - getCurrentScope, getDynamicSamplingContextFromSpan, + getRootSpan, + handleCallbackErrors, hasTracingEnabled, setHttpStatus, spanToJSON, spanToTraceHeader, + startSpan, withIsolationScope, } from '@sentry/core'; -import { captureException, getCurrentHub } from '@sentry/node-experimental'; -import type { Hub, Transaction, TransactionSource, WrappedFunction } from '@sentry/types'; +import { captureException } from '@sentry/node-experimental'; +import type { Span, TransactionSource, WrappedFunction } from '@sentry/types'; import { addExceptionMechanism, dynamicSamplingContextToSentryBaggageHeader, @@ -24,7 +28,6 @@ import { loadModule, logger, objectify, - tracingContextFromHeaders, } from '@sentry/utils'; import { DEBUG_BUILD } from './debug-build'; @@ -186,46 +189,50 @@ function makeWrappedDocumentRequestFunction(remixVersion?: number) { context: EntryContext, loadContext?: Record, ): Promise { - let res: Response; - // eslint-disable-next-line deprecation/deprecation - const activeTransaction = getActiveTransaction(); + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); - try { - // eslint-disable-next-line deprecation/deprecation - const span = activeTransaction?.startChild({ - op: 'function.remix.document_request', - origin: 'auto.function.remix', - name: spanToJSON(activeTransaction).description, + const name = rootSpan ? spanToJSON(rootSpan).description : undefined; + + return startSpan( + { + // If we don't have a root span, `onlyIfParent` will lead to the span not being created anyhow + // So we don't need to care too much about the fallback name, it's just for typing purposes.... + name: name || '', + onlyIfParent: true, attributes: { method: request.method, url: request.url, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.remix', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.remix.document_request', }, - }); - - res = await origDocumentRequestFunction.call( - this, - request, - responseStatusCode, - responseHeaders, - context, - loadContext, - ); - - span?.end(); - } catch (err) { - const isRemixV1 = !FUTURE_FLAGS?.v2_errorBoundary && remixVersion !== 2; - - // This exists to capture the server-side rendering errors on Remix v1 - // On Remix v2, we capture SSR errors at `handleError` - // We also skip primitives here, as we can't dedupe them, and also we don't expect any primitive SSR errors. - if (isRemixV1 && !isPrimitive(err)) { - await captureRemixServerException(err, 'documentRequest', request); - } - - throw err; - } - - return res; + }, + () => { + return handleCallbackErrors( + () => { + return origDocumentRequestFunction.call( + this, + request, + responseStatusCode, + responseHeaders, + context, + loadContext, + ); + }, + err => { + const isRemixV1 = !FUTURE_FLAGS?.v2_errorBoundary && remixVersion !== 2; + + // This exists to capture the server-side rendering errors on Remix v1 + // On Remix v2, we capture SSR errors at `handleError` + // We also skip primitives here, as we can't dedupe them, and also we don't expect any primitive SSR errors. + if (isRemixV1 && !isPrimitive(err)) { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + captureRemixServerException(err, 'documentRequest', request); + } + }, + ); + }, + ); }; }; } @@ -242,47 +249,32 @@ function makeWrappedDataFunction( return origFn.call(this, args); } - let res: Response | AppData; - // eslint-disable-next-line deprecation/deprecation - const activeTransaction = getActiveTransaction(); - const currentScope = getCurrentScope(); - - try { - // eslint-disable-next-line deprecation/deprecation - const span = activeTransaction?.startChild({ + return startSpan( + { op: `function.remix.${name}`, origin: 'auto.ui.remix', name: id, attributes: { name, }, - }); - - if (span) { - // Assign data function to hub to be able to see `db` transactions (if any) as children. - // eslint-disable-next-line deprecation/deprecation - currentScope.setSpan(span); - } - - res = await origFn.call(this, args); - - // eslint-disable-next-line deprecation/deprecation - currentScope.setSpan(activeTransaction); - span?.end(); - } catch (err) { - const isRemixV2 = FUTURE_FLAGS?.v2_errorBoundary || remixVersion === 2; - - // On Remix v2, we capture all unexpected errors (except the `Route Error Response`s / Thrown Responses) in `handleError` function. - // This is both for consistency and also avoid duplicates such as primitives like `string` or `number` being captured twice. - // Remix v1 does not have a `handleError` function, so we capture all errors here. - if (isRemixV2 ? isResponse(err) : true) { - await captureRemixServerException(err, name, args.request); - } - - throw err; - } - - return res; + }, + () => { + return handleCallbackErrors( + () => origFn.call(this, args), + err => { + const isRemixV2 = FUTURE_FLAGS?.v2_errorBoundary || remixVersion === 2; + + // On Remix v2, we capture all unexpected errors (except the `Route Error Response`s / Thrown Responses) in `handleError` function. + // This is both for consistency and also avoid duplicates such as primitives like `string` or `number` being captured twice. + // Remix v1 does not have a `handleError` function, so we capture all errors here. + if (isRemixV2 ? isResponse(err) : true) { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + captureRemixServerException(err, name, args.request); + } + }, + ); + }, + ); }; } @@ -382,47 +374,44 @@ export function createRoutes(manifest: ServerRouteManifest, parentId?: string): } /** - * Starts a new transaction for the given request to be used by different `RequestHandler` wrappers. + * Starts a new active span for the given request to be used by different `RequestHandler` wrappers. */ -export function startRequestHandlerTransaction( - hub: Hub, - name: string, - source: TransactionSource, - request: { - headers: { - 'sentry-trace': string; - baggage: string; - }; +export function startRequestHandlerSpan( + { + name, + source, + sentryTrace, + baggage, + method, + }: { + name: string; + source: TransactionSource; + sentryTrace: string; + baggage: string; method: string; }, -): Transaction { - // eslint-disable-next-line deprecation/deprecation - const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( - request.headers['sentry-trace'], - request.headers.baggage, - ); - // eslint-disable-next-line deprecation/deprecation - hub.getScope().setPropagationContext(propagationContext); - - // TODO: Refactor this to `startSpan()` - // eslint-disable-next-line deprecation/deprecation - const transaction = hub.startTransaction({ - name, - op: 'http.server', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.remix', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, - method: request.method, + callback: (span: Span) => T, +): T { + return continueTrace( + { + sentryTrace, + baggage, }, - ...traceparentData, - metadata: { - dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + () => { + return startSpan( + { + name, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.remix', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + method, + }, + }, + callback, + ); }, - }); - - // eslint-disable-next-line deprecation/deprecation - hub.getScope().setSpan(transaction); - return transaction; + ); } /** @@ -445,8 +434,6 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui } return withIsolationScope(async isolationScope => { - // eslint-disable-next-line deprecation/deprecation - const hub = getCurrentHub(); const options = getClient()?.getOptions(); let normalizedRequest: Record = request; @@ -473,23 +460,24 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui return origRequestHandler.call(this, request, loadContext); } - const transaction = startRequestHandlerTransaction(hub, name, source, { - headers: { - 'sentry-trace': request.headers.get('sentry-trace') || '', + return startRequestHandlerSpan( + { + name, + source, + sentryTrace: request.headers.get('sentry-trace') || '', baggage: request.headers.get('baggage') || '', + method: request.method, }, - method: request.method, - }); - - const res = (await origRequestHandler.call(this, request, loadContext)) as Response; + async span => { + const res = (await origRequestHandler.call(this, request, loadContext)) as Response; - if (isResponse(res)) { - setHttpStatus(transaction, res.status); - } - - transaction.end(); + if (isResponse(res)) { + setHttpStatus(span, res.status); + } - return res; + return res; + }, + ); }); }; } diff --git a/packages/remix/src/utils/serverAdapters/express.ts b/packages/remix/src/utils/serverAdapters/express.ts index de1219df5082..27e599e677e7 100644 --- a/packages/remix/src/utils/serverAdapters/express.ts +++ b/packages/remix/src/utils/serverAdapters/express.ts @@ -1,10 +1,10 @@ import { getClient, getCurrentHub, hasTracingEnabled, setHttpStatus, withIsolationScope } from '@sentry/core'; import { flush } from '@sentry/node-experimental'; -import type { Hub, Transaction } from '@sentry/types'; +import type { Hub, Span } from '@sentry/types'; import { extractRequestData, fill, isString, logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; -import { createRoutes, getTransactionName, instrumentBuild, startRequestHandlerTransaction } from '../instrumentServer'; +import { createRoutes, getTransactionName, instrumentBuild, startRequestHandlerSpan } from '../instrumentServer'; import type { AppLoadContext, ExpressCreateRequestHandler, @@ -89,19 +89,24 @@ function startRequestHandlerTransactionWithRoutes( next: ExpressNextFunction, hub: Hub, url: URL, -): Transaction | undefined { +): unknown { const [name, source] = getTransactionName(routes, url); - const transaction = startRequestHandlerTransaction(hub, name, source, { - headers: { - 'sentry-trace': (req.headers && isString(req.headers['sentry-trace']) && req.headers['sentry-trace']) || '', + + return startRequestHandlerSpan( + { + name, + source, + sentryTrace: (req.headers && isString(req.headers['sentry-trace']) && req.headers['sentry-trace']) || '', baggage: (req.headers && isString(req.headers.baggage) && req.headers.baggage) || '', + method: req.method, + }, + span => { + // save a link to the transaction on the response, so that even if there's an error (landing us outside of + // the domain), we can still finish it (albeit possibly missing some scope data) + (res as AugmentedExpressResponse).__sentrySpan = span; + return origRequestHandler.call(this, req, res, next); }, - method: req.method, - }); - // save a link to the transaction on the response, so that even if there's an error (landing us outside of - // the domain), we can still finish it (albeit possibly missing some scope data) - (res as AugmentedExpressResponse).__sentryTransaction = transaction; - return origRequestHandler.call(this, req, res, next); + ); } function wrapGetLoadContext(origGetLoadContext: () => AppLoadContext): GetLoadContextFunction { @@ -168,7 +173,7 @@ export function wrapExpressCreateRequestHandler( } export type AugmentedExpressResponse = ExpressResponse & { - __sentryTransaction?: Transaction; + __sentrySpan?: Span; }; type ResponseEndMethod = AugmentedExpressResponse['end']; @@ -201,16 +206,16 @@ function wrapEndMethod(origEnd: ResponseEndMethod): WrappedResponseEndMethod { * @param res The outgoing response for this request, on which the transaction is stored */ async function finishSentryProcessing(res: AugmentedExpressResponse): Promise { - const { __sentryTransaction: transaction } = res; + const { __sentrySpan: span } = res; - if (transaction) { - setHttpStatus(transaction, res.statusCode); + if (span) { + setHttpStatus(span, res.statusCode); // Push `transaction.finish` to the next event loop so open spans have a better chance of finishing before the // transaction closes, and make sure to wait until that's done before flushing events await new Promise(resolve => { setImmediate(() => { - transaction.end(); + span.end(); resolve(); }); }); diff --git a/packages/remix/test/integration/common/routes/manual-tracing.$id.tsx b/packages/remix/test/integration/common/routes/manual-tracing.$id.tsx index a7383f1b8f5a..1a3cc359c7c1 100644 --- a/packages/remix/test/integration/common/routes/manual-tracing.$id.tsx +++ b/packages/remix/test/integration/common/routes/manual-tracing.$id.tsx @@ -1,7 +1,10 @@ import * as Sentry from '@sentry/remix'; export default function ManualTracing() { - const transaction = Sentry.startTransaction({ name: 'test_transaction_1' }); - transaction.end(); + const span = Sentry.startInactiveSpan({ + name: 'test_transaction_1', + forceTransaction: true, + }); + span.end(); return
; } diff --git a/packages/remix/test/integration/test/client/manualtracing.test.ts b/packages/remix/test/integration/test/client/manualtracing.test.ts index c9985ce75cb7..4c1b01507617 100644 --- a/packages/remix/test/integration/test/client/manualtracing.test.ts +++ b/packages/remix/test/integration/test/client/manualtracing.test.ts @@ -7,6 +7,7 @@ const useV2 = process.env.REMIX_VERSION === '2'; test('should report a manually created / finished transaction.', async ({ page }) => { const envelopes = await getMultipleSentryEnvelopeRequests(page, 2, { url: '/manual-tracing/0', + envelopeType: 'transaction', }); const [manualTransactionEnvelope, pageloadEnvelope] = envelopes;