Skip to content

Commit f55fc30

Browse files
chargomechargome
authored
feat(core): Migrate request data to dataCollection (#21071)
- `httpHeadersToSpanAttributes()` now accepts `ResolvedDataCollection` and delegates filtering to `filterKeyValueData()` from the data-collection utils instead of reimplementing deny/allow logic. Boolean (`sendDefaultPii`) is still accepted for backward compat with external callers (TODO for v11 removal). - `requestDataIntegration` reads `client.getDataCollectionOptions()` instead of `client.getOptions().sendDefaultPii`. `DEFAULT_INCLUDE` is now derived from `dataCollection` at call time, with integration-level `include` options overriding per spec. - `server-subscription.ts` passes `client.getDataCollectionOptions()` to header attribute creation closes #21048
1 parent b2aa011 commit f55fc30

8 files changed

Lines changed: 483 additions & 147 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/requestData-streamed/instrument-with-datacollection-no-userinfo.mjs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
traceLifecycle: 'stream',
10+
dataCollection: {
11+
userInfo: false,
12+
},
13+
});

‎dev-packages/node-integration-tests/suites/tracing/requestData-streamed/instrument-with-datacollection.mjs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
traceLifecycle: 'stream',
10+
dataCollection: {
11+
userInfo: true,
12+
},
13+
});

‎dev-packages/node-integration-tests/suites/tracing/requestData-streamed/test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,79 @@ describe('requestData-streamed', () => {
4949
});
5050
});
5151

52+
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument-with-datacollection.mjs', (createRunner, test) => {
53+
test('applies request data attributes when using dataCollection config', async () => {
54+
const runner = createRunner()
55+
.expect({
56+
span: container => {
57+
const serverSpan = container.items.find(item => item.is_segment);
58+
59+
expect(serverSpan).toBeDefined();
60+
61+
expect(serverSpan?.attributes?.['url.full']).toEqual({
62+
type: 'string',
63+
value: expect.stringContaining('/test?foo=bar'),
64+
});
65+
66+
expect(serverSpan?.attributes?.['http.request.method']).toEqual({
67+
type: 'string',
68+
value: 'GET',
69+
});
70+
71+
expect(serverSpan?.attributes?.['url.query']).toEqual({
72+
type: 'string',
73+
value: 'foo=bar',
74+
});
75+
76+
expect(serverSpan?.attributes?.['http.request.header.host']).toEqual({
77+
type: 'string',
78+
value: expect.any(String),
79+
});
80+
81+
expect(serverSpan?.attributes?.['user.ip_address']).toEqual({
82+
type: 'string',
83+
value: expect.any(String),
84+
});
85+
},
86+
})
87+
.start();
88+
89+
await runner.makeRequest('get', '/test?foo=bar');
90+
91+
await runner.completed();
92+
});
93+
});
94+
95+
createEsmAndCjsTests(
96+
__dirname,
97+
'server.mjs',
98+
'instrument-with-datacollection-no-userinfo.mjs',
99+
(createRunner, test) => {
100+
test('does not include user.ip_address when dataCollection.userInfo is false', async () => {
101+
const runner = createRunner()
102+
.expect({
103+
span: container => {
104+
const serverSpan = container.items.find(item => item.is_segment);
105+
106+
expect(serverSpan).toBeDefined();
107+
108+
expect(serverSpan?.attributes?.['http.request.header.host']).toEqual({
109+
type: 'string',
110+
value: expect.any(String),
111+
});
112+
113+
expect(serverSpan?.attributes?.['user.ip_address']).toBeUndefined();
114+
},
115+
})
116+
.start();
117+
118+
await runner.makeRequest('get', '/test?foo=bar');
119+
120+
await runner.completed();
121+
});
122+
},
123+
);
124+
52125
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument-without-request-data.mjs', (createRunner, test) => {
53126
test('does not apply request data attributes when requestDataIntegration is removed', async () => {
54127
const runner = createRunner()

‎packages/core/src/integrations/http/server-subscription.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -309,10 +309,7 @@ function buildServerSpanWrap(
309309
'http.flavor': httpVersion,
310310
'net.transport': httpVersion?.toUpperCase() === 'QUIC' ? 'ip_udp' : 'ip_tcp',
311311
...getRequestContentLengthAttribute(request),
312-
...httpHeadersToSpanAttributes(
313-
normalizedRequest.headers || {},
314-
client.getOptions().sendDefaultPii ?? false,
315-
),
312+
...httpHeadersToSpanAttributes(normalizedRequest.headers || {}, client.getDataCollectionOptions()),
316313
},
317314
},
318315
span => {
@@ -334,7 +331,7 @@ function buildServerSpanWrap(
334331
'http.status_code': response.statusCode,
335332
...httpHeadersToSpanAttributes(
336333
headersToDict(response.headers),
337-
client?.getOptions().sendDefaultPii ?? false,
334+
client?.getDataCollectionOptions() ?? false,
338335
'response',
339336
),
340337
});

‎packages/core/src/integrations/requestdata.ts‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
import type { Client } from '../client';
12
import { getIsolationScope } from '../currentScopes';
23
import { defineIntegration } from '../integration';
34
import { SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS } from '../semanticAttributes';
5+
import type { ResolvedDataCollection } from '../types/datacollection';
46
import type { Event } from '../types/event';
57
import type { IntegrationFn } from '../types/integration';
68
import type { QueryParams, RequestEventData } from '../types/request';
@@ -26,36 +28,52 @@ type RequestDataIntegrationOptions = {
2628
include?: RequestDataIncludeOptions;
2729
};
2830

29-
// TODO(v11): Change defaults based on `sendDefaultPii`
30-
const DEFAULT_INCLUDE: RequestDataIncludeOptions = {
31-
cookies: true,
32-
data: true,
33-
headers: true,
34-
query_string: true,
35-
url: true,
36-
};
37-
3831
const INTEGRATION_NAME = 'RequestData';
3932

4033
const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) => {
41-
const include = {
42-
...DEFAULT_INCLUDE,
43-
...options.include,
44-
};
34+
// Per spec, integration-level options override global dataCollection.
35+
// When include overrides a category back on that dataCollection turned off,
36+
// we flip the dataCollection behavior to true (default denylist filtering).
37+
function resolveIncludeAndDataCollection(client: Client): {
38+
include: RequestDataIncludeOptions;
39+
dataCollection: ResolvedDataCollection;
40+
} {
41+
const dc = client.getDataCollectionOptions();
42+
const dataCollection: ResolvedDataCollection = {
43+
...dc,
44+
...(options.include?.cookies === true && dc.cookies === false && { cookies: true as const }),
45+
...(options.include?.headers === true &&
46+
dc.httpHeaders.request === false && {
47+
httpHeaders: { ...dc.httpHeaders, request: true as const },
48+
}),
49+
};
50+
51+
return {
52+
dataCollection,
53+
include: {
54+
cookies: dataCollection.cookies !== false,
55+
// Always attach body data that's already on the scope — dataCollection.httpBodies gates write-time, not read-time
56+
data: true,
57+
headers: dataCollection.httpHeaders.request !== false,
58+
ip: dataCollection.userInfo,
59+
query_string: dataCollection.queryParams !== false,
60+
// No dataCollection equivalent — URL is always included
61+
url: true,
62+
...options.include,
63+
},
64+
};
65+
}
4566

4667
return {
4768
name: INTEGRATION_NAME,
4869
processEvent(event, _hint, client) {
4970
const { sdkProcessingMetadata = {} } = event;
5071
const { normalizedRequest, ipAddress } = sdkProcessingMetadata;
5172

52-
const includeWithDefaultPiiApplied: RequestDataIncludeOptions = {
53-
...include,
54-
ip: include.ip ?? client.getOptions().sendDefaultPii,
55-
};
73+
const { include } = resolveIncludeAndDataCollection(client);
5674

5775
if (normalizedRequest) {
58-
addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, includeWithDefaultPiiApplied);
76+
addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, include);
5977
}
6078

6179
return event;
@@ -68,13 +86,9 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) =
6886
return;
6987
}
7088

71-
const { sendDefaultPii } = client.getOptions();
72-
const includeWithDefaultPiiApplied: RequestDataIncludeOptions = {
73-
...include,
74-
ip: include.ip ?? sendDefaultPii,
75-
};
89+
const { include, dataCollection } = resolveIncludeAndDataCollection(client);
7690

77-
addNormalizedRequestDataToSpan(span, normalizedRequest, ipAddress, includeWithDefaultPiiApplied, sendDefaultPii);
91+
addNormalizedRequestDataToSpan(span, normalizedRequest, ipAddress, include, dataCollection);
7892
},
7993
};
8094
}) satisfies IntegrationFn;
@@ -117,7 +131,7 @@ function addNormalizedRequestDataToSpan(
117131
normalizedRequest: RequestEventData,
118132
ipAddress: string | undefined,
119133
include: RequestDataIncludeOptions,
120-
sendDefaultPii: boolean | undefined,
134+
dataCollection: ResolvedDataCollection,
121135
): void {
122136
const requestData = extractNormalizedRequestData(normalizedRequest, include);
123137
const attributes: Record<string, unknown> = {};
@@ -142,12 +156,12 @@ function addNormalizedRequestDataToSpan(
142156
const cookieString = Object.entries(requestData.cookies)
143157
.map(([name, value]) => `${name}=${value}`)
144158
.join('; ');
145-
const cookieAttributes = httpHeadersToSpanAttributes({ cookie: cookieString }, sendDefaultPii ?? false, 'request');
159+
const cookieAttributes = httpHeadersToSpanAttributes({ cookie: cookieString }, dataCollection, 'request');
146160
safeSetSpanJSONAttributes(span, cookieAttributes);
147161
}
148162

149163
if (requestData.headers) {
150-
const headerAttributes = httpHeadersToSpanAttributes(requestData.headers, sendDefaultPii ?? false, 'request');
164+
const headerAttributes = httpHeadersToSpanAttributes(requestData.headers, dataCollection, 'request');
151165
safeSetSpanJSONAttributes(span, headerAttributes);
152166
}
153167

0 commit comments

Comments
 (0)