Skip to content

Commit fcf3f0f

Browse files
committed
Preserve NODE_VERSION gating for OTEL data-loader span creation
1 parent 11e22ca commit fcf3f0f

5 files changed

Lines changed: 126 additions & 10 deletions

File tree

packages/react-router/src/server/instrumentation/reactRouter.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
import type * as reactRouter from 'react-router';
1717
import { DEBUG_BUILD } from '../../common/debug-build';
1818
import { isServerBuildLike, setServerBuild } from '../serverBuild';
19-
import { isInstrumentationApiUsed } from '../serverGlobals';
19+
import { isInstrumentationApiUsed, isOtelDataLoaderSpanCreationEnabled } from '../serverGlobals';
2020
import { getOpName, getSpanName, isDataRequest } from './util';
2121

2222
type ReactRouterModuleExports = typeof reactRouter;
@@ -81,12 +81,13 @@ export class ReactRouterInstrumentation extends InstrumentationBase<Instrumentat
8181

8282
const originalRequestHandler = original.apply(this, args);
8383

84-
// Skip per-request wrapping when instrumentation API is active
85-
if (isInstrumentationApiUsed()) {
86-
return originalRequestHandler;
87-
}
88-
8984
return async function sentryWrappedRequestHandler(request: Request, initialContext?: unknown) {
85+
// Skip OTEL span creation when instrumentation API is active or when span creation is not enabled.
86+
// Checked per-request (not at handler-creation time) because in dev, createRequestHandler runs before entry.server.tsx.
87+
if (isInstrumentationApiUsed() || !isOtelDataLoaderSpanCreationEnabled()) {
88+
return originalRequestHandler(request, initialContext);
89+
}
90+
9091
let url: URL;
9192
try {
9293
url = new URL(request.url);

packages/react-router/src/server/integration/reactRouterServer.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
22
import { defineIntegration } from '@sentry/core';
3-
import { generateInstrumentOnce } from '@sentry/node';
3+
import { generateInstrumentOnce, NODE_VERSION } from '@sentry/node';
44
import { ReactRouterInstrumentation } from '../instrumentation/reactRouter';
55
import { registerServerBuildGlobal } from '../serverBuild';
6+
import { enableOtelDataLoaderSpanCreation } from '../serverGlobals';
67

78
const INTEGRATION_NAME = 'ReactRouterServer';
89

@@ -26,8 +27,16 @@ export const reactRouterServerIntegration = defineIntegration(() => {
2627
return {
2728
name: INTEGRATION_NAME,
2829
setupOnce() {
30+
// Enable OTEL data-loader spans only on Node versions without the diagnostics_channel-based instrumentation API.
31+
if (
32+
(NODE_VERSION.major === 20 && NODE_VERSION.minor < 19) ||
33+
(NODE_VERSION.major === 22 && NODE_VERSION.minor < 12)
34+
) {
35+
enableOtelDataLoaderSpanCreation();
36+
}
37+
2938
// Always install to capture ServerBuild for middleware names.
30-
// Skips per-request wrapping when instrumentation API is active.
39+
// Skips per-request wrapping when instrumentation API is active or OTEL span creation is disabled.
3140
instrumentReactRouterServer();
3241
},
3342
processEvent(event) {

packages/react-router/src/server/serverGlobals.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { GLOBAL_OBJ } from '@sentry/core';
22

33
const SENTRY_SERVER_INSTRUMENTATION_FLAG = '__sentryReactRouterServerInstrumentationUsed';
4+
const SENTRY_OTEL_SPAN_CREATION_FLAG = '__sentryReactRouterOtelSpanCreationEnabled';
45

56
type GlobalObjWithFlag = typeof GLOBAL_OBJ & {
67
[SENTRY_SERVER_INSTRUMENTATION_FLAG]?: boolean;
8+
[SENTRY_OTEL_SPAN_CREATION_FLAG]?: boolean;
79
};
810

911
/**
@@ -21,3 +23,19 @@ export function markInstrumentationApiUsed(): void {
2123
export function isInstrumentationApiUsed(): boolean {
2224
return !!(GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_SERVER_INSTRUMENTATION_FLAG];
2325
}
26+
27+
/**
28+
* Enable OTEL data-loader span creation for React Router server.
29+
* @internal
30+
*/
31+
export function enableOtelDataLoaderSpanCreation(): void {
32+
(GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_OTEL_SPAN_CREATION_FLAG] = true;
33+
}
34+
35+
/**
36+
* Check if OTEL data-loader span creation is enabled for React Router server.
37+
* @internal
38+
*/
39+
export function isOtelDataLoaderSpanCreationEnabled(): boolean {
40+
return !!(GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_OTEL_SPAN_CREATION_FLAG];
41+
}

packages/react-router/test/server/instrumentation/reactRouterServer.test.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ describe('ReactRouterInstrumentation', () => {
6565
});
6666

6767
it('should call original handler for non-data requests', async () => {
68+
vi.spyOn(ServerGlobals, 'isOtelDataLoaderSpanCreationEnabled').mockReturnValue(true);
6869
vi.spyOn(Util, 'isDataRequest').mockReturnValue(false);
6970

7071
const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule);
@@ -77,6 +78,7 @@ describe('ReactRouterInstrumentation', () => {
7778
});
7879

7980
it('should call original handler if no active root span', async () => {
81+
vi.spyOn(ServerGlobals, 'isOtelDataLoaderSpanCreationEnabled').mockReturnValue(true);
8082
vi.spyOn(Util, 'isDataRequest').mockReturnValue(true);
8183
vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue(undefined);
8284

@@ -90,6 +92,7 @@ describe('ReactRouterInstrumentation', () => {
9092
});
9193

9294
it('should start a span for data requests with active root span', async () => {
95+
vi.spyOn(ServerGlobals, 'isOtelDataLoaderSpanCreationEnabled').mockReturnValue(true);
9396
vi.spyOn(Util, 'isDataRequest').mockReturnValue(true);
9497
// @ts-expect-error MockSpan just for testing
9598
vi.spyOn(SentryCore, 'getActiveSpan').mockReturnValue(mockSpan as Span);
@@ -112,6 +115,7 @@ describe('ReactRouterInstrumentation', () => {
112115
});
113116

114117
it('should handle invalid URLs gracefully', async () => {
118+
vi.spyOn(ServerGlobals, 'isOtelDataLoaderSpanCreationEnabled').mockReturnValue(true);
115119
const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule);
116120
const wrappedHandler = proxy.createRequestHandler();
117121
const req = { url: 'not a url', method: 'GET' } as any;
@@ -147,12 +151,40 @@ describe('ReactRouterInstrumentation', () => {
147151
expect(spy).toHaveBeenCalledWith(resolvedBuild);
148152
});
149153

150-
it('should return original handler without wrapping when instrumentation API is active', () => {
154+
it('should bypass instrumentation when instrumentation API is active', async () => {
151155
vi.spyOn(ServerGlobals, 'isInstrumentationApiUsed').mockReturnValue(true);
156+
vi.spyOn(ServerGlobals, 'isOtelDataLoaderSpanCreationEnabled').mockReturnValue(true);
157+
vi.spyOn(Util, 'isDataRequest').mockReturnValue(true);
158+
const startSpanSpy = vi.spyOn(SentryCore, 'startSpan');
152159

153160
const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule);
154161
const handler = proxy.createRequestHandler();
155162

156-
expect(handler).toBe(originalHandler);
163+
// Handler is always wrapped; the instrumentation API check happens per-request
164+
expect(handler).not.toBe(originalHandler);
165+
166+
const req = createRequest('https://test.com/data', 'GET');
167+
await handler(req);
168+
169+
// Should delegate to original handler without creating spans
170+
expect(originalHandler).toHaveBeenCalledWith(req, undefined);
171+
expect(startSpanSpy).not.toHaveBeenCalled();
172+
});
173+
174+
it('should skip span creation when OTEL data-loader span creation is disabled', async () => {
175+
vi.spyOn(ServerGlobals, 'isInstrumentationApiUsed').mockReturnValue(false);
176+
vi.spyOn(ServerGlobals, 'isOtelDataLoaderSpanCreationEnabled').mockReturnValue(false);
177+
vi.spyOn(Util, 'isDataRequest').mockReturnValue(true);
178+
const startSpanSpy = vi.spyOn(SentryCore, 'startSpan');
179+
180+
const proxy = (instrumentation as any)._createPatchedModuleProxy(mockModule);
181+
const handler = proxy.createRequestHandler();
182+
183+
const req = createRequest('https://test.com/data', 'GET');
184+
await handler(req);
185+
186+
// Should delegate to original handler without creating spans
187+
expect(originalHandler).toHaveBeenCalledWith(req, undefined);
188+
expect(startSpanSpy).not.toHaveBeenCalled();
157189
});
158190
});

packages/react-router/test/server/integration/reactRouterServer.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,35 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
22
import { ReactRouterInstrumentation } from '../../../src/server/instrumentation/reactRouter';
33
import { reactRouterServerIntegration } from '../../../src/server/integration/reactRouterServer';
44
import * as serverBuild from '../../../src/server/serverBuild';
5+
import * as serverGlobals from '../../../src/server/serverGlobals';
56

67
vi.mock('../../../src/server/instrumentation/reactRouter', () => {
78
return {
89
ReactRouterInstrumentation: vi.fn(),
910
};
1011
});
1112

13+
const mockNodeVersion = { major: 20, minor: 18, patch: 0 };
14+
1215
vi.mock('@sentry/node', () => {
1316
return {
1417
generateInstrumentOnce: vi.fn((_name: string, callback: () => any) => {
1518
return Object.assign(callback, { id: 'test' });
1619
}),
20+
get NODE_VERSION() {
21+
return mockNodeVersion;
22+
},
1723
};
1824
});
1925

2026
describe('reactRouterServerIntegration', () => {
2127
let registerServerBuildGlobalSpy: ReturnType<typeof vi.spyOn>;
28+
let enableOtelDataLoaderSpanCreationSpy: ReturnType<typeof vi.spyOn>;
2229

2330
beforeEach(() => {
2431
vi.clearAllMocks();
2532
registerServerBuildGlobalSpy = vi.spyOn(serverBuild, 'registerServerBuildGlobal');
33+
enableOtelDataLoaderSpanCreationSpy = vi.spyOn(serverGlobals, 'enableOtelDataLoaderSpanCreation');
2634
});
2735

2836
afterEach(() => {
@@ -42,4 +50,52 @@ describe('reactRouterServerIntegration', () => {
4250

4351
expect(registerServerBuildGlobalSpy).toHaveBeenCalledTimes(1);
4452
});
53+
54+
it('enables OTEL data-loader span creation on Node 20.18', () => {
55+
mockNodeVersion.major = 20;
56+
mockNodeVersion.minor = 18;
57+
58+
const integration = reactRouterServerIntegration();
59+
integration.setupOnce!();
60+
61+
expect(enableOtelDataLoaderSpanCreationSpy).toHaveBeenCalledTimes(1);
62+
expect(ReactRouterInstrumentation).toHaveBeenCalledTimes(1);
63+
expect(registerServerBuildGlobalSpy).toHaveBeenCalledTimes(1);
64+
});
65+
66+
it('enables OTEL data-loader span creation on Node 22.11', () => {
67+
mockNodeVersion.major = 22;
68+
mockNodeVersion.minor = 11;
69+
70+
const integration = reactRouterServerIntegration();
71+
integration.setupOnce!();
72+
73+
expect(enableOtelDataLoaderSpanCreationSpy).toHaveBeenCalledTimes(1);
74+
expect(ReactRouterInstrumentation).toHaveBeenCalledTimes(1);
75+
expect(registerServerBuildGlobalSpy).toHaveBeenCalledTimes(1);
76+
});
77+
78+
it('does not enable OTEL data-loader span creation on Node 20.19', () => {
79+
mockNodeVersion.major = 20;
80+
mockNodeVersion.minor = 19;
81+
82+
const integration = reactRouterServerIntegration();
83+
integration.setupOnce!();
84+
85+
expect(enableOtelDataLoaderSpanCreationSpy).not.toHaveBeenCalled();
86+
expect(ReactRouterInstrumentation).toHaveBeenCalledTimes(1);
87+
expect(registerServerBuildGlobalSpy).toHaveBeenCalledTimes(1);
88+
});
89+
90+
it('does not enable OTEL data-loader span creation on Node 22.12', () => {
91+
mockNodeVersion.major = 22;
92+
mockNodeVersion.minor = 12;
93+
94+
const integration = reactRouterServerIntegration();
95+
integration.setupOnce!();
96+
97+
expect(enableOtelDataLoaderSpanCreationSpy).not.toHaveBeenCalled();
98+
expect(ReactRouterInstrumentation).toHaveBeenCalledTimes(1);
99+
expect(registerServerBuildGlobalSpy).toHaveBeenCalledTimes(1);
100+
});
45101
});

0 commit comments

Comments
 (0)