diff --git a/packages/hono/test/shared/patchAppRequest.test.ts b/packages/hono/test/shared/patchAppRequest.test.ts index 245821091143..6019cf50c95e 100644 --- a/packages/hono/test/shared/patchAppRequest.test.ts +++ b/packages/hono/test/shared/patchAppRequest.test.ts @@ -42,7 +42,7 @@ describe('patchAppRequest', () => { ); }); - it('skips span creation when there is no active span', async () => { + it('requires a parent when starting a span', async () => { getActiveSpanMock.mockReturnValue(undefined); const app = new Hono(); @@ -51,7 +51,17 @@ describe('patchAppRequest', () => { const res = await app.request('/hello'); - expect(startSpanMock).not.toHaveBeenCalled(); + expect(startSpanMock).toHaveBeenCalledWith( + { + name: 'GET /hello', + onlyIfParent: true, + attributes: { + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.hono.internal_request', + }, + }, + expect.any(Function), + ); expect(await res.text()).toBe('world'); }); diff --git a/packages/remix/src/server/integrations/tracing-channel.ts b/packages/remix/src/server/integrations/tracing-channel.ts index 8e93813a19dd..d87c8fac4956 100644 --- a/packages/remix/src/server/integrations/tracing-channel.ts +++ b/packages/remix/src/server/integrations/tracing-channel.ts @@ -192,6 +192,7 @@ function subscribeCallRouteLoader(): void { // With span streaming, a `function` span is named after the function it wraps. The route id // stays on `match.route.id`. name: client && hasSpanStreamingEnabled(client) ? 'loader' : description, + onlyIfParent: true, attributes: { [SENTRY_ORIGIN]: ORIGIN, [SENTRY_OP]: FUNCTION, @@ -203,7 +204,6 @@ function subscribeCallRouteLoader(): void { }); }, { - requiresParentSpan: true, beforeSpanEnd: (span, data) => setResponseStatus(span, data.result), }, ); @@ -214,23 +214,13 @@ function subscribeCallRouteAction(formDataCapture: FormDataCapture | undefined): diagnosticsChannel.tracingChannel(remixChannels.REMIX_CALL_ROUTE_ACTION), data => { const params = (data.arguments[0] ?? {}) as RouteCallParams; - // Start reading the form data now (from a clone taken before the action consumes the body), so - // it overlaps the action's execution. Unlike the patched instrumentation, a channel can't - // delay the action promise, so reading only after it settles would race the parent - // `requestHandler` span flushing the transaction. Reading here means the promise is (virtually - // always) already resolved by `asyncEnd`, so ending the span costs a single microtask. - if (formDataCapture && params.request) { - const formData = params.request.clone().formData(); - // Attach a handler so an unconsumed rejection (e.g. the action errored) isn't unhandled. - formData.catch(() => undefined); - data._sentryFormData = formData; - } const client = getClient(); const description = `ACTION ${params.routeId}`; - return startInactiveSpan({ + const span = startInactiveSpan({ // With span streaming, a `function` span is named after the function it wraps. The route id // stays on `match.route.id`. name: client && hasSpanStreamingEnabled(client) ? 'action' : description, + onlyIfParent: true, attributes: { [SENTRY_ORIGIN]: ORIGIN, [SENTRY_OP]: FUNCTION, @@ -240,9 +230,18 @@ function subscribeCallRouteAction(formDataCapture: FormDataCapture | undefined): ...getMatchAttributes(params), }, }); + + // Read before the action consumes the body so enrichment can finish before the request span ends. + if (span.isRecording() && formDataCapture && params.request) { + const formData = params.request.clone().formData(); + // Attach a handler so an unconsumed rejection (e.g. the action errored) isn't unhandled. + formData.catch(() => undefined); + data._sentryFormData = formData; + } + + return span; }, { - requiresParentSpan: true, beforeSpanEnd: (span, data) => setResponseStatus(span, data.result), // Hold the span end until the (already in-flight) form-data read resolves, then apply the // attributes and end (which sets the response status via `beforeSpanEnd`). On error, or when diff --git a/packages/remix/test/server/tracing-channel-test-utils.ts b/packages/remix/test/server/tracing-channel-test-utils.ts index 1efee5a23210..a614707db85f 100644 --- a/packages/remix/test/server/tracing-channel-test-utils.ts +++ b/packages/remix/test/server/tracing-channel-test-utils.ts @@ -72,6 +72,7 @@ export function makeSpan(data: Record = {}): Span { setAttributes: vi.fn(), setAttribute: vi.fn(), updateName: vi.fn(), + isRecording: () => true, getSpanJSON: () => ({ attributes: data }), getStaticSpanJSON: () => ({ data }), } as unknown as Span; diff --git a/packages/remix/test/server/tracing-channel.test.ts b/packages/remix/test/server/tracing-channel.test.ts index c11200f643b1..e4f09e2398de 100644 --- a/packages/remix/test/server/tracing-channel.test.ts +++ b/packages/remix/test/server/tracing-channel.test.ts @@ -27,7 +27,6 @@ describe('remixIntegration (Orchestrion-based)', () => { beforeEach(() => { span = makeSpan(); startInactiveSpanSpy = vi.spyOn(SentryCore, 'startInactiveSpan').mockReturnValue(span); - // A truthy active span by default, so the `requiresParentSpan` gate passes. getActiveSpanSpy = vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue({} as Span); }); @@ -112,6 +111,7 @@ describe('remixIntegration (Orchestrion-based)', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'LOADER routes/users.$userId', + onlyIfParent: true, attributes: expect.objectContaining({ 'sentry.description': 'LOADER routes/users.$userId', 'sentry.origin': 'auto.http.remix', @@ -129,13 +129,27 @@ describe('remixIntegration (Orchestrion-based)', () => { expect(span.end).toHaveBeenCalledTimes(1); }); - it('callRouteLoader: does not create a span without an active parent span', async () => { + it('callRouteLoader: requires a parent when starting a span', async () => { getActiveSpanSpy.mockReturnValue(undefined); const ctx = { arguments: [{ routeId: 'x', request: makeRequest(), params: {} }] }; await tracingChannel(remixChannels.REMIX_CALL_ROUTE_LOADER).tracePromise(async () => ({ status: 200 }), ctx); - expect(startInactiveSpanSpy).not.toHaveBeenCalled(); + expect(startInactiveSpanSpy).toHaveBeenCalledWith({ + name: 'LOADER x', + onlyIfParent: true, + attributes: { + 'sentry.origin': 'auto.http.remix', + 'sentry.op': 'function', + 'sentry.description': 'LOADER x', + 'code.function.name': 'loader', + 'http.request.method': 'GET', + 'url.full': 'http://localhost/test', + 'url.path': '/test', + 'match.route.id': 'x', + 'router.navigation.route.id': 'x', + }, + }); }); it('callRouteAction: builds an ACTION span and captures the configured form-data keys', async () => { @@ -154,6 +168,7 @@ describe('remixIntegration (Orchestrion-based)', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'ACTION routes/submit', + onlyIfParent: true, attributes: expect.objectContaining({ 'sentry.description': 'ACTION routes/submit', 'sentry.op': 'function', diff --git a/packages/server-utils/src/integrations/dataloader.ts b/packages/server-utils/src/integrations/dataloader.ts index 5fb50a577325..63983e9b32b3 100644 --- a/packages/server-utils/src/integrations/dataloader.ts +++ b/packages/server-utils/src/integrations/dataloader.ts @@ -179,9 +179,7 @@ function subscribeConstruct(): void { function subscribeLoad(): void { const channel = diagnosticsChannel.tracingChannel(CHANNELS.DATALOADER_LOAD); - bindTracingChannelToSpan(channel, data => startInactiveSpanFor(data.self, 'load', data.arguments[0]), { - requiresParentSpan: true, - }); + bindTracingChannelToSpan(channel, data => startInactiveSpanFor(data.self, 'load', data.arguments[0])); channel.end.subscribe(message => { const data = message as TracingChannelPayloadWithSpan; @@ -194,10 +192,8 @@ function subscribeLoad(): void { } function subscribeSimpleOperation(channelName: ChannelName, operation: Operation): void { - bindTracingChannelToSpan( - diagnosticsChannel.tracingChannel(channelName), - data => startInactiveSpanFor(data.self, operation, data.arguments[0]), - { requiresParentSpan: true }, + bindTracingChannelToSpan(diagnosticsChannel.tracingChannel(channelName), data => + startInactiveSpanFor(data.self, operation, data.arguments[0]), ); } diff --git a/packages/server-utils/src/integrations/hono/honoIntegration.ts b/packages/server-utils/src/integrations/hono/honoIntegration.ts index 575462d6d018..d5ef285cdf62 100644 --- a/packages/server-utils/src/integrations/hono/honoIntegration.ts +++ b/packages/server-utils/src/integrations/hono/honoIntegration.ts @@ -111,13 +111,13 @@ function instrumentInternalRequests(): void { ).toUpperCase(); return startInactiveSpan({ name: `${method} ${extractPathname(input)}`, + onlyIfParent: true, attributes: { [SENTRY_OP]: HTTP_SERVER, [SENTRY_ORIGIN]: INTERNAL_REQUEST_ORIGIN, }, }); }, - { requiresParentSpan: true }, ); } diff --git a/packages/server-utils/src/integrations/hono/patchAppRequest.ts b/packages/server-utils/src/integrations/hono/patchAppRequest.ts index 15bb20cc21fd..af84a2df6983 100644 --- a/packages/server-utils/src/integrations/hono/patchAppRequest.ts +++ b/packages/server-utils/src/integrations/hono/patchAppRequest.ts @@ -1,6 +1,6 @@ import { SENTRY_OP, SENTRY_ORIGIN } from '@sentry/conventions/attributes'; import { HTTP_SERVER } from '@sentry/conventions/op'; -import { debug, getActiveSpan, getOriginalFunction, startSpan, type WrappedFunction } from '@sentry/core'; +import { debug, getOriginalFunction, startSpan, type WrappedFunction } from '@sentry/core'; import { INTERNAL_REQUEST_ORIGIN } from './constants'; import type { Env, Hono } from './honoTypes'; import { DEBUG_BUILD } from '../../debug-build'; @@ -82,10 +82,6 @@ export function patchAppRequest(app: Hono): void { apply(_target, thisArg, args: [string | Request | URL, RequestInit?, ...unknown[]]) { const [input, requestInit, ...rest] = args; - if (!getActiveSpan()) { - return Reflect.apply(_target, thisArg, args); - } - let method = requestInit?.method ?? (input instanceof Request ? input.method : 'GET'); method = method.toUpperCase();