Skip to content

Commit f372d84

Browse files
Lms24Lms24
authored
ref(tracing): Remove transaction name and user_id from DSC (getsentry#5363)
This patch temporarily removes the `user_id` and `transaction` (name) fields from the dynamic sampling context, meaning they will no longer propagated with outgoing requests via the baggage Http header or sent to sentry via the `trace` envelope header. We're taking this temporary measure to ensure that for the moment PII is not sent to third parties.
1 parent 815801a commit f372d84

File tree

15 files changed

+66
-44
lines changed

15 files changed

+66
-44
lines changed

‎packages/core/test/lib/envelope.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ describe('createEventEnvelope', () => {
4444
{
4545
environment: 'prod',
4646
release: '1.0.0',
47-
transaction: 'TX',
48-
user_id: 'bob',
47+
// transaction: 'TX',
48+
// user_id: 'bob',
4949
user_segment: 'segmentA',
5050
sample_rate: '0.95',
5151
public_key: 'pubKey123',
@@ -59,8 +59,8 @@ describe('createEventEnvelope', () => {
5959
{
6060
environment: 'prod',
6161
release: '1.0.0',
62-
transaction: 'TX',
63-
user_id: 'bob',
62+
// transaction: 'TX',
63+
// user_id: 'bob',
6464
user_segment: 'segmentA',
6565
sample_rate: '0.95',
6666
public_key: 'pubKey123',

‎packages/integration-tests/suites/tracing/envelope-header-no-pii/test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { sentryTest } from '../../../utils/fixtures';
55
import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers';
66

77
sentryTest(
8-
'should not send user_id in DSC data in trace envelope header if sendDefaultPii option is not set',
8+
'should not send user_id and transaction in DSC data in trace envelope header (for now)',
99
async ({ getLocalTestPath, page }) => {
1010
const url = await getLocalTestPath({ testDir: __dirname });
1111

@@ -14,7 +14,6 @@ sentryTest(
1414
expect(envHeader.trace).toBeDefined();
1515
expect(envHeader.trace).toEqual({
1616
environment: 'production',
17-
transaction: expect.stringContaining('index.html'),
1817
user_segment: 'segmentB',
1918
sample_rate: '1',
2019
trace_id: expect.any(String),

‎packages/integration-tests/suites/tracing/envelope-header/init.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ Sentry.init({
88
integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] })],
99
environment: 'production',
1010
tracesSampleRate: 1,
11-
sendDefaultPii: true,
11+
// TODO: We're rethinking the mechanism for including Pii data in DSC, hence commenting out sendDefaultPii for now
12+
// sendDefaultPii: true,
1213
debug: true,
1314
});
1415

‎packages/integration-tests/suites/tracing/envelope-header/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ sentryTest(
1414
expect(envHeader.trace).toBeDefined();
1515
expect(envHeader.trace).toEqual({
1616
environment: 'production',
17-
transaction: expect.stringContaining('index.html'),
18-
user_id: 'user123',
17+
// transaction: expect.stringContaining('index.html'),
18+
// user_id: 'user123',
1919
user_segment: 'segmentB',
2020
sample_rate: '1',
2121
trace_id: expect.any(String),

‎packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,10 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n
7979
test_data: {
8080
host: 'somewhere.not.sentry',
8181
baggage: expect.stringContaining(
82-
'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' +
83-
'sentry-public_key=public,sentry-trace_id=',
82+
// Commented out as long as transaction and user_id are not part of DSC
83+
// 'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' +
84+
// 'sentry-public_key=public,sentry-trace_id=',
85+
'sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=',
8486
),
8587
},
8688
});
@@ -99,8 +101,10 @@ test('Should populate Sentry and ignore 3rd party content if sentry-trace header
99101
host: 'somewhere.not.sentry',
100102
// TraceId changes, hence we only expect that the string contains the traceid key
101103
baggage: expect.stringContaining(
102-
'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' +
103-
'sentry-public_key=public,sentry-trace_id=',
104+
// Commented out as long as transaction and user_id are not part of DSC
105+
// 'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' +
106+
// 'sentry-public_key=public,sentry-trace_id=',
107+
'sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=',
104108
),
105109
},
106110
});

‎packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,26 @@ test('should attach a `baggage` header to an outgoing request.', async () => {
1313
test_data: {
1414
host: 'somewhere.not.sentry',
1515
baggage:
16-
'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-user_segment=SegmentA' +
16+
// Commented out as long as transaction and user_id are not part of DSC
17+
// 'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-user_segment=SegmentA' +
18+
'sentry-environment=prod,sentry-release=1.0,sentry-user_segment=SegmentA' +
1719
',sentry-public_key=public,sentry-trace_id=86f39e84263a4de99c326acab3bfe3bd,sentry-sample_rate=1',
1820
},
1921
});
2022
});
2123

22-
test('Does not include user_id in baggage if sendDefaultPii is not set', async () => {
24+
test('Does not include user_id and transaction name (for now)', async () => {
2325
const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
2426

2527
const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
28+
const baggageString = response.test_data.baggage;
2629

2730
expect(response).toBeDefined();
2831
expect(response).toMatchObject({
2932
test_data: {
3033
host: 'somewhere.not.sentry',
31-
baggage: expect.not.stringContaining('sentry-user_id'),
3234
},
3335
});
36+
expect(baggageString).not.toContain('sentry-user_id=');
37+
expect(baggageString).not.toContain('sentry-transaction=');
3438
});

‎packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an
3030
test_data: {
3131
host: 'somewhere.not.sentry',
3232
baggage: expect.stringContaining(
33-
'other=vendor,foo=bar,third=party,last=item,sentry-environment=prod,sentry-release=1.0,' +
34-
'sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-public_key=public',
33+
// Commented out as long as transaction and user_id are not part of DSC
34+
// 'other=vendor,foo=bar,third=party,last=item,sentry-environment=prod,sentry-release=1.0,' +
35+
// 'sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-public_key=public',
36+
'other=vendor,foo=bar,third=party,last=item,sentry-environment=prod,sentry-release=1.0,sentry-public_key=public',
3537
),
3638
},
3739
});

‎packages/node-integration-tests/suites/express/sentry-trace/baggage-user-id/server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ Sentry.init({
1414
environment: 'prod',
1515
integrations: [new Sentry.Integrations.Http({ tracing: true }), new Tracing.Integrations.Express({ app })],
1616
tracesSampleRate: 1.0,
17-
sendDefaultPii: true,
17+
// TODO: We're rethinking the mechanism for including Pii data in DSC, hence commenting out sendDefaultPii for now
18+
// sendDefaultPii: true,
1819
});
1920

2021
Sentry.setUser({ id: 'user123', segment: 'SegmentA' });

‎packages/node-integration-tests/suites/express/sentry-trace/baggage-user-id/test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import * as path from 'path';
33
import { getAPIResponse, runServer } from '../../../../utils/index';
44
import { TestAPIResponse } from '../server';
55

6-
test('Includes user_id in baggage if sendDefaultPii is set to true', async () => {
6+
// TODO: Skipping this test because right now we're rethinking the mechanism for including such data
7+
test.skip('Includes user_id in baggage if <optionTBA> is set to true', async () => {
78
const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
89

910
const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;

‎packages/node/test/integrations/http.test.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ describe('tracing', () => {
2424
integrations: [new HttpIntegration({ tracing: true })],
2525
release: '1.0.0',
2626
environment: 'production',
27-
sendDefaultPii: true,
2827
...customOptions,
2928
});
3029
const hub = new Hub(new NodeClient(options));
@@ -113,7 +112,9 @@ describe('tracing', () => {
113112
expect(baggageHeader).toBeDefined();
114113
expect(typeof baggageHeader).toEqual('string');
115114
expect(baggageHeader).toEqual(
116-
'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-user_id=uid123,' +
115+
// Commented out as long as transaction and user_id are not part of DSC
116+
// 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-user_id=uid123,' +
117+
'sentry-environment=production,sentry-release=1.0.0,' +
117118
'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
118119
'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
119120
);
@@ -130,24 +131,31 @@ describe('tracing', () => {
130131
expect(baggageHeader).toBeDefined();
131132
expect(typeof baggageHeader).toEqual('string');
132133
expect(baggageHeader).toEqual(
133-
'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' +
134-
'sentry-user_id=uid123,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
134+
// Commented out as long as transaction and user_id are not part of DSC
135+
// 'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' +
136+
// 'sentry-user_id=uid123,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
137+
// 'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
138+
'dog=great,sentry-environment=production,sentry-release=1.0.0,' +
139+
'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
135140
'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
136141
);
137142
});
138143

139-
it('does not add the user_id to the baggage header if sendDefaultPii is set to false', async () => {
144+
// TODO: Skipping this test because right now we're rethinking the mechanism for including such data
145+
it.skip('does not add the user_id to the baggage header if <optionTBA> is set to false', async () => {
140146
nock('http://dogs.are.great').get('/').reply(200);
141147

142-
createTransactionOnScope({ sendDefaultPii: false });
148+
createTransactionOnScope();
143149

144150
const request = http.get({ host: 'http://dogs.are.great/', headers: { baggage: 'dog=great' } });
145151
const baggageHeader = request.getHeader('baggage') as string;
146152

147153
expect(baggageHeader).toBeDefined();
148154
expect(typeof baggageHeader).toEqual('string');
149155
expect(baggageHeader).toEqual(
150-
'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' +
156+
// Commented out as long as transaction and user_id are not part of DSC
157+
// 'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' +
158+
'dog=great,sentry-environment=production,sentry-release=1.0.0,' +
151159
'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
152160
'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
153161
);

‎packages/tracing/src/transaction.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,15 +229,19 @@ export class Transaction extends SpanClass implements TransactionInterface {
229229
? rate.toLocaleString('fullwide', { useGrouping: false, maximumFractionDigits: 16 })
230230
: undefined;
231231

232+
// For now we're not sending the transaction name and user_id due to PII concerns
233+
// commenting it out for now because we'll probably need it in the future
234+
232235
const scope = hub.getScope();
233-
const { id: user_id, segment: user_segment } = (scope && scope.getUser()) || {};
236+
const { /* id: user_id, */ segment: user_segment } = (scope && scope.getUser()) || {};
234237

235238
return createBaggage(
236239
dropUndefinedKeys({
237240
environment,
238241
release,
239-
transaction: this.name,
240-
...(hub.shouldSendDefaultPii() && { user_id }),
242+
// transaction: this.name,
243+
// replace `someContidion` with whatever decision we come up with to guard PII in DSC
244+
// ...(someCondition && { user_id }),
241245
user_segment,
242246
public_key,
243247
trace_id: this.traceId,

‎packages/tracing/test/browser/browsertracing.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ describe('BrowserTracing', () => {
509509
expect(baggage[0]).toEqual({
510510
release: '1.0.0',
511511
environment: 'production',
512-
transaction: 'blank',
512+
// transaction: 'blank',
513513
public_key: 'pubKey',
514514
trace_id: expect.not.stringMatching('12312012123120121231201212312012'),
515515
});

‎packages/tracing/test/span.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -438,13 +438,12 @@ describe('Span', () => {
438438

439439
const baggage = transaction.getBaggage();
440440

441-
// this is called twice because hub.shouldSendDefaultPii also calls getOptions()
442-
expect(getOptionsSpy).toHaveBeenCalledTimes(2);
441+
expect(getOptionsSpy).toHaveBeenCalledTimes(1);
443442
expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false);
444443
expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({
445444
release: '1.0.1',
446445
environment: 'production',
447-
transaction: 'tx',
446+
// transaction: 'tx',
448447
sample_rate: '0.56',
449448
trace_id: expect.any(String),
450449
});
@@ -468,7 +467,7 @@ describe('Span', () => {
468467
expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({
469468
release: '1.0.1',
470469
environment: 'production',
471-
transaction: 'tx',
470+
// transaction: 'tx',
472471
sample_rate: '0.0000000000000145',
473472
trace_id: expect.any(String),
474473
});

‎packages/types/src/envelope.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ export type DynamicSamplingContext = {
1515
sample_rate?: string;
1616
release?: string;
1717
environment?: string;
18-
transaction?: string;
19-
user_id?: string;
18+
// transaction?: string; // omitted for now
19+
// user_id?: string; // omitted for now
2020
user_segment?: string;
2121
};
2222

‎packages/types/src/options.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,17 +153,16 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
153153
tunnel?: string;
154154

155155
/**
156-
* Currently controls if the user id (e.g. set by `Sentry.setUser`) should
157-
* be used for dynamic sampling. Only if this flag is set to `true`, the user id
158-
* will be propagated to outgoing requests in the `baggage` Http header.
156+
* Important: This option is currently unused and will only work in the next major version of the SDK
159157
*
160-
* Note that in the next major version of this SDK, this option will not only
161-
* control dynamic sampling data: As long as this flag is not set to `true`,
162-
* the SDK will not send sensitive data by default.
158+
* Controls if potentially sensitive data should be sent to Sentry by default.
159+
* Note that this only applies to data that the SDK is sending by default
160+
* but not data that was explicitly set (e.g. by calling `Sentry.setUser()`).
161+
* Details about the implementation are TBD.
163162
*
164163
* Defaults to `false`.
165164
*
166-
* @experimental (will be fully introduced in the next major version)
165+
* @ignore
167166
*/
168167
sendDefaultPii?: boolean;
169168

0 commit comments

Comments
 (0)