Skip to content

Commit 493f178

Browse files
committed
ref: Set normalizedRequest instead of request various places
The request data integration prefers this over `request`, we want to get rid of `request` in v9. Part of #14298 url should not be set on server-components
1 parent bac6c93 commit 493f178

File tree

21 files changed

+314
-83
lines changed

21 files changed

+314
-83
lines changed

dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,12 @@ test('Sends a transaction for a request to app router', async ({ page }) => {
3333
trace_id: expect.any(String),
3434
});
3535

36-
expect(transactionEvent).toEqual(
37-
expect.objectContaining({
38-
request: {
39-
cookies: {},
40-
headers: expect.any(Object),
41-
url: expect.any(String),
42-
},
36+
expect(transactionEvent.request).toEqual({
37+
cookies: {},
38+
headers: expect.objectContaining({
39+
'user-agent': expect.any(String),
4340
}),
44-
);
45-
46-
expect(Object.keys(transactionEvent.request?.headers!).length).toBeGreaterThan(0);
41+
});
4742

4843
// The transaction should not contain any spans with the same name as the transaction
4944
// e.g. "GET /server-component/parameter/[...parameters]"

packages/astro/src/server/middleware.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ import {
1212
startSpan,
1313
withIsolationScope,
1414
} from '@sentry/node';
15-
import type { Scope, SpanAttributes } from '@sentry/types';
15+
import type { Request, Scope, SpanAttributes } from '@sentry/types';
1616
import {
1717
addNonEnumerableProperty,
18+
extractQueryParamsFromUrl,
1819
logger,
1920
objectify,
2021
stripUrlQueryAndFragment,
@@ -111,7 +112,13 @@ async function instrumentRequest(
111112
getCurrentScope().setSDKProcessingMetadata({
112113
// We store the request on the current scope, not isolation scope,
113114
// because we may have multiple requests nested inside each other
114-
request: isDynamicPageRequest ? winterCGRequestToRequestData(request) : { method, url: request.url },
115+
normalizedRequest: (isDynamicPageRequest
116+
? winterCGRequestToRequestData(request)
117+
: {
118+
method,
119+
url: request.url,
120+
query_string: extractQueryParamsFromUrl(request.url),
121+
}) satisfies Request,
115122
});
116123

117124
if (options.trackClientIp && isDynamicPageRequest) {

packages/astro/test/server/middleware.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ describe('sentryMiddleware', () => {
221221
await middleware(ctx, next);
222222

223223
expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({
224-
request: {
224+
normalizedRequest: {
225225
method: 'GET',
226226
url: '/users',
227227
headers: {
@@ -254,7 +254,7 @@ describe('sentryMiddleware', () => {
254254
await middleware(ctx, next);
255255

256256
expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({
257-
request: {
257+
normalizedRequest: {
258258
method: 'GET',
259259
url: '/users',
260260
},

packages/bun/src/integrations/bunserver.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import {
99
startSpan,
1010
withIsolationScope,
1111
} from '@sentry/core';
12-
import type { IntegrationFn, SpanAttributes } from '@sentry/types';
13-
import { getSanitizedUrlString, parseUrl } from '@sentry/utils';
12+
import type { IntegrationFn, Request, SpanAttributes } from '@sentry/types';
13+
import { extractQueryParamsFromUrl, getSanitizedUrlString, parseUrl } from '@sentry/utils';
1414

1515
const INTEGRATION_NAME = 'BunServer';
1616

@@ -76,11 +76,12 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
7676
const url = getSanitizedUrlString(parsedUrl);
7777

7878
isolationScope.setSDKProcessingMetadata({
79-
request: {
79+
normalizedRequest: {
8080
url,
8181
method: request.method,
8282
headers: request.headers.toJSON(),
83-
},
83+
query_string: extractQueryParamsFromUrl(url),
84+
} satisfies Request,
8485
});
8586

8687
return continueTrace(

packages/cloudflare/src/scope-utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,5 @@ export function addCultureContext(scope: Scope, cf: IncomingRequestCfProperties)
2525
* Set request data on scope
2626
*/
2727
export function addRequest(scope: Scope, request: Request): void {
28-
scope.setSDKProcessingMetadata({ request: winterCGRequestToRequestData(request) });
28+
scope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(request) });
2929
}

packages/cloudflare/test/request.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ describe('withSentry', () => {
109109
},
110110
);
111111

112-
expect(sentryEvent.sdkProcessingMetadata?.request).toEqual({
112+
expect(sentryEvent.sdkProcessingMetadata?.normalizedRequest).toEqual({
113113
headers: {},
114114
url: 'https://example.com/',
115115
method: 'GET',

packages/core/src/scope.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type {
2424
import { dateTimestampInSeconds, generatePropagationContext, isPlainObject, logger, uuid4 } from '@sentry/utils';
2525

2626
import { updateSession } from './session';
27+
import { mergeSdkProcessingMetadata } from './utils/applyScopeDataToEvent';
2728
import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';
2829

2930
/**
@@ -479,8 +480,7 @@ class ScopeClass implements ScopeInterface {
479480
* @inheritDoc
480481
*/
481482
public setSDKProcessingMetadata(newData: { [key: string]: unknown }): this {
482-
this._sdkProcessingMetadata = { ...this._sdkProcessingMetadata, ...newData };
483-
483+
this._sdkProcessingMetadata = mergeSdkProcessingMetadata(this._sdkProcessingMetadata, newData);
484484
return this;
485485
}
486486

packages/core/src/utils/applyScopeDataToEvent.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ export function mergeScopeData(data: ScopeData, mergeData: ScopeData): void {
4646
mergeAndOverwriteScopeData(data, 'tags', tags);
4747
mergeAndOverwriteScopeData(data, 'user', user);
4848
mergeAndOverwriteScopeData(data, 'contexts', contexts);
49-
mergeAndOverwriteScopeData(data, 'sdkProcessingMetadata', sdkProcessingMetadata);
49+
50+
data.sdkProcessingMetadata = mergeSdkProcessingMetadata(data.sdkProcessingMetadata, sdkProcessingMetadata);
5051

5152
if (level) {
5253
data.level = level;
@@ -115,6 +116,35 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
115116
event[prop] = merged.length ? merged : undefined;
116117
}
117118

119+
/**
120+
* Merge new SDK processing metadata into existing data.
121+
* New data will overwrite existing data.
122+
* `normalizedRequest` is special handled and will also be merged.
123+
*/
124+
export function mergeSdkProcessingMetadata(
125+
sdkProcessingMetadata: ScopeData['sdkProcessingMetadata'],
126+
newSdkProcessingMetadata: ScopeData['sdkProcessingMetadata'],
127+
): ScopeData['sdkProcessingMetadata'] {
128+
// We want to merge `normalizedRequest` to avoid some partial entry on the scope
129+
// overwriting potentially more complete data on the isolation scope
130+
const normalizedRequestBefore = sdkProcessingMetadata['normalizedRequest'];
131+
const normalizedRequest = newSdkProcessingMetadata['normalizedRequest'];
132+
133+
const newData = {
134+
...sdkProcessingMetadata,
135+
...newSdkProcessingMetadata,
136+
};
137+
138+
if (normalizedRequestBefore || normalizedRequest) {
139+
newData['normalizedRequest'] = {
140+
...(normalizedRequestBefore || {}),
141+
...(normalizedRequest || {}),
142+
};
143+
}
144+
145+
return newData;
146+
}
147+
118148
function applyDataToEvent(event: Event, data: ScopeData): void {
119149
const { extra, tags, user, contexts, level, transactionName } = data;
120150

packages/core/test/lib/scope.test.ts

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,10 +204,47 @@ describe('Scope', () => {
204204
expect(scope['_user']).toEqual({});
205205
});
206206

207-
test('setProcessingMetadata', () => {
208-
const scope = new Scope();
209-
scope.setSDKProcessingMetadata({ dogs: 'are great!' });
210-
expect(scope['_sdkProcessingMetadata'].dogs).toEqual('are great!');
207+
describe('setProcessingMetadata', () => {
208+
test('it works with no initial data', () => {
209+
const scope = new Scope();
210+
scope.setSDKProcessingMetadata({ dogs: 'are great!' });
211+
expect(scope['_sdkProcessingMetadata'].dogs).toEqual('are great!');
212+
});
213+
214+
test('it overwrites arbitrary data', () => {
215+
const scope = new Scope();
216+
scope.setSDKProcessingMetadata({ dogs: 'are great!' });
217+
scope.setSDKProcessingMetadata({ dogs: 'are really great!' });
218+
scope.setSDKProcessingMetadata({ cats: 'are also great!' });
219+
scope.setSDKProcessingMetadata({ obj: { nested: 'value1' } });
220+
scope.setSDKProcessingMetadata({ obj: { nested2: 'value2' } });
221+
222+
expect(scope['_sdkProcessingMetadata']).toEqual({
223+
dogs: 'are really great!',
224+
cats: 'are also great!',
225+
obj: { nested2: 'value2' },
226+
});
227+
});
228+
229+
test('it merges normalizedRequest data', () => {
230+
const scope = new Scope();
231+
scope.setSDKProcessingMetadata({
232+
normalizedRequest: {
233+
url: 'value1',
234+
method: 'value1',
235+
},
236+
});
237+
scope.setSDKProcessingMetadata({
238+
normalizedRequest: {
239+
url: 'value2',
240+
headers: {},
241+
},
242+
});
243+
244+
expect(scope['_sdkProcessingMetadata']).toEqual({
245+
normalizedRequest: { url: 'value2', method: 'value1', headers: {} },
246+
});
247+
});
211248
});
212249

213250
test('set and get propagation context', () => {

0 commit comments

Comments
 (0)