ref(tracing): Only add user_id to DSC if sendDefaultPii is true (#5344)

Add the check if `sendDefaultPii` is set to `true` before adding the `user_id` when populating DSC.
Additionally, add some tests to check for the changed behaviour.
Conforms with Sentry Dev Spec for handling sensitive data in DSC: https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#handling-sensitive-data-and-pii

Author: Lms24

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

@@ -0,0 +1,17 @@
+import * as Sentry from '@sentry/browser';
+import { Integrations } from '@sentry/tracing';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] })],
+  environment: 'production',
+  tracesSampleRate: 1,
+  debug: true,
+});
+
+Sentry.configureScope(scope => {
+  scope.setUser({ id: 'user123', segment: 'segmentB' });
+  scope.setTransactionName('testTransactionDSC');
+});

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

@@ -0,0 +1,24 @@
+import { expect } from '@playwright/test';
+import { EventEnvelopeHeaders } from '@sentry/types';
+
+import { sentryTest } from '../../../utils/fixtures';
+import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers';
+
+sentryTest(
+  'should not send user_id in DSC data in trace envelope header if sendDefaultPii option is not set',
+  async ({ getLocalTestPath, page }) => {
+    const url = await getLocalTestPath({ testDir: __dirname });
+
+    const envHeader = await getFirstSentryEnvelopeRequest<EventEnvelopeHeaders>(page, url, envelopeHeaderRequestParser);
+
+    expect(envHeader.trace).toBeDefined();
+    expect(envHeader.trace).toEqual({
+      environment: 'production',
+      transaction: expect.stringContaining('index.html'),
+      user_segment: 'segmentB',
+      sample_rate: '1',
+      trace_id: expect.any(String),
+      public_key: 'public',
+    });
+  },
+);

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

@@ -8,6 +8,7 @@ Sentry.init({
   integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] })],
   environment: 'production',
   tracesSampleRate: 1,
+  sendDefaultPii: true,
   debug: true,
 });
 

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

@@ -0,0 +1,39 @@
+import * as Sentry from '@sentry/node';
+import * as Tracing from '@sentry/tracing';
+import cors from 'cors';
+import express from 'express';
+import http from 'http';
+
+const app = express();
+
+export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } };
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  environment: 'prod',
+  integrations: [new Sentry.Integrations.Http({ tracing: true }), new Tracing.Integrations.Express({ app })],
+  tracesSampleRate: 1.0,
+});
+
+Sentry.setUser({ id: 'user123', segment: 'SegmentA' });
+
+app.use(Sentry.Handlers.requestHandler());
+app.use(Sentry.Handlers.tracingHandler());
+
+app.use(cors());
+
+app.get('/test/express', (_req, res) => {
+  const transaction = Sentry.getCurrentHub().getScope()?.getTransaction();
+  if (transaction) {
+    transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
+  }
+  const headers = http.get('http://somewhere.not.sentry/').getHeaders();
+
+  // Responding with the headers outgoing request headers back to the assertions.
+  res.send({ test_data: headers });
+});
+
+app.use(Sentry.Handlers.errorHandler());
+
+export default app;

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

@@ -4,15 +4,31 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('should attach a `baggage` header to an outgoing request.', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
 
   const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: expect.stringMatching('sentry-environment=prod,sentry-release=1.0'),
+      baggage:
+        'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-user_segment=SegmentA' +
+        ',sentry-public_key=public,sentry-trace_id=86f39e84263a4de99c326acab3bfe3bd,sentry-sample_rate=1',
+    },
+  });
+});
+
+test('Does not include user_id in baggage if sendDefaultPii is not set', async () => {
+  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+
+  const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: expect.not.stringContaining('sentry-user_id'),
     },
   });
 });

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

@@ -0,0 +1,40 @@
+import * as Sentry from '@sentry/node';
+import * as Tracing from '@sentry/tracing';
+import cors from 'cors';
+import express from 'express';
+import http from 'http';
+
+const app = express();
+
+export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } };
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  environment: 'prod',
+  integrations: [new Sentry.Integrations.Http({ tracing: true }), new Tracing.Integrations.Express({ app })],
+  tracesSampleRate: 1.0,
+  sendDefaultPii: true,
+});
+
+Sentry.setUser({ id: 'user123', segment: 'SegmentA' });
+
+app.use(Sentry.Handlers.requestHandler());
+app.use(Sentry.Handlers.tracingHandler());
+
+app.use(cors());
+
+app.get('/test/express', (_req, res) => {
+  const transaction = Sentry.getCurrentHub().getScope()?.getTransaction();
+  if (transaction) {
+    transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
+  }
+  const headers = http.get('http://somewhere.not.sentry/').getHeaders();
+
+  // Responding with the headers outgoing request headers back to the assertions.
+  res.send({ test_data: headers });
+});
+
+app.use(Sentry.Handlers.errorHandler());
+
+export default app;

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

@@ -0,0 +1,18 @@
+import * as path from 'path';
+
+import { getAPIResponse, runServer } from '../../../../utils/index';
+import { TestAPIResponse } from '../server';
+
+test('Includes user_id in baggage if sendDefaultPii is set to true', async () => {
+  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+
+  const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: expect.stringContaining('sentry-user_id=user123'),
+    },
+  });
+});

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

@@ -11,18 +11,21 @@ import * as nock from 'nock';
 import { Breadcrumb } from '../../src';
 import { NodeClient } from '../../src/client';
 import { Http as HttpIntegration } from '../../src/integrations/http';
+import { NodeClientOptions } from '../../src/types';
 import { getDefaultNodeClientOptions } from '../helper/node-client-options';
 
 const NODE_VERSION = parseSemver(process.versions.node);
 
 describe('tracing', () => {
-  function createTransactionOnScope() {
+  function createTransactionOnScope(customOptions: Partial<NodeClientOptions> = {}) {
     const options = getDefaultNodeClientOptions({
       dsn: 'https://[email protected]/12312012',
       tracesSampleRate: 1.0,
       integrations: [new HttpIntegration({ tracing: true })],
       release: '1.0.0',
       environment: 'production',
+      sendDefaultPii: true,
+      ...customOptions,
     });
     const hub = new Hub(new NodeClient(options));
     addExtensionMethods();
@@ -133,6 +136,23 @@ describe('tracing', () => {
     );
   });
 
+  it('does not add the user_id to the baggage header if sendDefaultPii is set to false', async () => {
+    nock('http://dogs.are.great').get('/').reply(200);
+
+    createTransactionOnScope({ sendDefaultPii: false });
+
+    const request = http.get({ host: 'http://dogs.are.great/', headers: { baggage: 'dog=great' } });
+    const baggageHeader = request.getHeader('baggage') as string;
+
+    expect(baggageHeader).toBeDefined();
+    expect(typeof baggageHeader).toEqual('string');
+    expect(baggageHeader).toEqual(
+      'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' +
+        'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
+        'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
+    );
+  });
+
   it("doesn't attach the sentry-trace header to outgoing sentry requests", () => {
     nock('http://squirrelchasers.ingest.sentry.io').get('/api/12312012/store/').reply(200);
 

packages/tracing/src/transaction.ts

@@ -237,7 +237,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
         environment,
         release,
         transaction: this.name,
-        user_id,
+        ...(hub.shouldSendDefaultPii() && { user_id }),
         user_segment,
         public_key,
         trace_id: this.traceId,

packages/tracing/test/span.test.ts

@@ -434,11 +434,12 @@ describe('Span', () => {
         hub,
       );
 
-      const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions');
+      const getOptionsSpy = jest.spyOn(hub.getClient()!, 'getOptions');
 
       const baggage = transaction.getBaggage();
 
-      expect(hubSpy).toHaveBeenCalledTimes(1);
+      // this is called twice because hub.shouldSendDefaultPii also calls getOptions()
+      expect(getOptionsSpy).toHaveBeenCalledTimes(2);
       expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false);
       expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({
         release: '1.0.1',