diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index e682a69594e3..8398b4831f53 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -44,8 +44,7 @@ describe('createEventEnvelope', () => { { environment: 'prod', release: '1.0.0', - // transaction: 'TX', - // user_id: 'bob', + transaction: 'TX', user_segment: 'segmentA', sample_rate: '0.95', public_key: 'pubKey123', @@ -59,8 +58,7 @@ describe('createEventEnvelope', () => { { environment: 'prod', release: '1.0.0', - // transaction: 'TX', - // user_id: 'bob', + transaction: 'TX', user_segment: 'segmentA', sample_rate: '0.95', public_key: 'pubKey123', diff --git a/packages/integration-tests/suites/tracing/envelope-header-no-pii/init.js b/packages/integration-tests/suites/tracing/envelope-header-transaction-name/init.js similarity index 88% rename from packages/integration-tests/suites/tracing/envelope-header-no-pii/init.js rename to packages/integration-tests/suites/tracing/envelope-header-transaction-name/init.js index fbce5a16116a..efb7b577f75b 100644 --- a/packages/integration-tests/suites/tracing/envelope-header-no-pii/init.js +++ b/packages/integration-tests/suites/tracing/envelope-header-transaction-name/init.js @@ -14,4 +14,5 @@ Sentry.init({ Sentry.configureScope(scope => { scope.setUser({ id: 'user123', segment: 'segmentB' }); scope.setTransactionName('testTransactionDSC'); + scope.getTransaction().setMetadata({ source: 'custom' }); }); diff --git a/packages/integration-tests/suites/tracing/envelope-header-no-pii/test.ts b/packages/integration-tests/suites/tracing/envelope-header-transaction-name/test.ts similarity index 83% rename from packages/integration-tests/suites/tracing/envelope-header-no-pii/test.ts rename to packages/integration-tests/suites/tracing/envelope-header-transaction-name/test.ts index 1e14efaaec08..7ef6bd4304e1 100644 --- a/packages/integration-tests/suites/tracing/envelope-header-no-pii/test.ts +++ b/packages/integration-tests/suites/tracing/envelope-header-transaction-name/test.ts @@ -5,7 +5,7 @@ import { sentryTest } from '../../../utils/fixtures'; import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; sentryTest( - 'should not send user_id and transaction in DSC data in trace envelope header (for now)', + 'should only include transaction name if source is better than an unparameterized URL', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); @@ -16,6 +16,7 @@ sentryTest( environment: 'production', user_segment: 'segmentB', sample_rate: '1', + transaction: expect.stringContaining('/index.html'), trace_id: expect.any(String), public_key: 'public', }); diff --git a/packages/integration-tests/suites/tracing/envelope-header/init.js b/packages/integration-tests/suites/tracing/envelope-header/init.js index 57cad0867442..fbce5a16116a 100644 --- a/packages/integration-tests/suites/tracing/envelope-header/init.js +++ b/packages/integration-tests/suites/tracing/envelope-header/init.js @@ -8,8 +8,6 @@ Sentry.init({ integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] })], environment: 'production', tracesSampleRate: 1, - // TODO: We're rethinking the mechanism for including Pii data in DSC, hence commenting out sendDefaultPii for now - // sendDefaultPii: true, debug: true, }); diff --git a/packages/integration-tests/suites/tracing/envelope-header/test.ts b/packages/integration-tests/suites/tracing/envelope-header/test.ts index e4db6e34332d..fef9ae14a8fb 100644 --- a/packages/integration-tests/suites/tracing/envelope-header/test.ts +++ b/packages/integration-tests/suites/tracing/envelope-header/test.ts @@ -11,11 +11,13 @@ sentryTest( const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); + // In this test, we don't expect trace.transaction to be present because without a custom routing instrumentation + // we for now don't have parameterization. This might change in the future but for now the only way of having + // transaction in DSC with the default BrowserTracing integration is when the transaction name is set manually. + // This scenario is covered in another integration test (envelope-header-transaction-name). expect(envHeader.trace).toBeDefined(); expect(envHeader.trace).toEqual({ environment: 'production', - // transaction: expect.stringContaining('index.html'), - // user_id: 'user123', user_segment: 'segmentB', sample_rate: '1', trace_id: expect.any(String), diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts index ab3be6009adc..f50c782fdb4a 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts @@ -78,10 +78,8 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', + // TraceId changes, hence we only expect that the string contains the traceid key baggage: expect.stringContaining( - // Commented out as long as transaction and user_id are not part of DSC - // 'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' + - // 'sentry-public_key=public,sentry-trace_id=', 'sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=', ), }, @@ -101,9 +99,6 @@ test('Should populate Sentry and ignore 3rd party content if sentry-trace header host: 'somewhere.not.sentry', // TraceId changes, hence we only expect that the string contains the traceid key baggage: expect.stringContaining( - // Commented out as long as transaction and user_id are not part of DSC - // 'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' + - // 'sentry-public_key=public,sentry-trace_id=', 'sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=', ), }, diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts index accfae88bc84..a2eaebbf8c42 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts @@ -13,15 +13,13 @@ test('should attach a `baggage` header to an outgoing request.', async () => { test_data: { host: 'somewhere.not.sentry', baggage: - // Commented out as long as transaction and user_id are not part of DSC - // 'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-user_segment=SegmentA' + 'sentry-environment=prod,sentry-release=1.0,sentry-user_segment=SegmentA' + ',sentry-public_key=public,sentry-trace_id=86f39e84263a4de99c326acab3bfe3bd,sentry-sample_rate=1', }, }); }); -test('Does not include user_id and transaction name (for now)', async () => { +test('Does not include transaction name if transaction source is not set', async () => { const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`); const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse; diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts index fe85d0c4b954..30cfc024016b 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts @@ -30,9 +30,6 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an test_data: { host: 'somewhere.not.sentry', baggage: expect.stringContaining( - // Commented out as long as transaction and user_id are not part of DSC - // 'other=vendor,foo=bar,third=party,last=item,sentry-environment=prod,sentry-release=1.0,' + - // 'sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-public_key=public', 'other=vendor,foo=bar,third=party,last=item,sentry-environment=prod,sentry-release=1.0,sentry-public_key=public', ), }, diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-user-id/server.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts similarity index 96% rename from packages/node-integration-tests/suites/express/sentry-trace/baggage-user-id/server.ts rename to packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts index 199fa1fe455c..6e814bc5e333 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/baggage-user-id/server.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts @@ -29,6 +29,7 @@ app.get('/test/express', (_req, res) => { const transaction = Sentry.getCurrentHub().getScope()?.getTransaction(); if (transaction) { transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd'; + transaction.setMetadata({ source: 'route' }); } const headers = http.get('http://somewhere.not.sentry/').getHeaders(); diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-user-id/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/test.ts similarity index 64% rename from packages/node-integration-tests/suites/express/sentry-trace/baggage-user-id/test.ts rename to packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/test.ts index 0a0ecf072874..34ff4a64bf94 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/baggage-user-id/test.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/test.ts @@ -3,8 +3,7 @@ import * as path from 'path'; import { getAPIResponse, runServer } from '../../../../utils/index'; import { TestAPIResponse } from '../server'; -// TODO: Skipping this test because right now we're rethinking the mechanism for including such data -test.skip('Includes user_id in baggage if is set to true', async () => { +test('Includes transaction in baggage if the transaction name is parameterized', async () => { const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`); const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse; @@ -13,7 +12,7 @@ test.skip('Includes user_id in baggage if is set to true', async () expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: expect.stringContaining('sentry-user_id=user123'), + baggage: expect.stringContaining('sentry-transaction=GET%20%2Ftest%2Fexpress'), }, }); }); diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 1ff8ce824eee..0c72efdced6c 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -2,6 +2,7 @@ import * as sentryCore from '@sentry/core'; import * as hubModule from '@sentry/hub'; import { Hub } from '@sentry/hub'; import { addExtensionMethods, Span, TRACEPARENT_REGEXP, Transaction } from '@sentry/tracing'; +import { TransactionContext } from '@sentry/types'; import { parseSemver } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; @@ -17,7 +18,10 @@ import { getDefaultNodeClientOptions } from '../helper/node-client-options'; const NODE_VERSION = parseSemver(process.versions.node); describe('tracing', () => { - function createTransactionOnScope(customOptions: Partial = {}) { + function createTransactionOnScope( + customOptions: Partial = {}, + customContext?: Partial, + ) { const options = getDefaultNodeClientOptions({ dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', tracesSampleRate: 1.0, @@ -44,7 +48,9 @@ describe('tracing', () => { const transaction = hub.startTransaction({ name: 'dogpark', traceId: '12312012123120121231201212312012', + ...customContext, }); + hub.getScope()?.setSpan(transaction); return transaction; @@ -112,8 +118,6 @@ describe('tracing', () => { expect(baggageHeader).toBeDefined(); expect(typeof baggageHeader).toEqual('string'); expect(baggageHeader).toEqual( - // Commented out as long as transaction and user_id are not part of DSC - // 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-user_id=uid123,' + 'sentry-environment=production,sentry-release=1.0.0,' + 'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + 'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1', @@ -131,21 +135,16 @@ describe('tracing', () => { expect(baggageHeader).toBeDefined(); expect(typeof baggageHeader).toEqual('string'); expect(baggageHeader).toEqual( - // Commented out as long as transaction and user_id are not part of DSC - // 'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' + - // 'sentry-user_id=uid123,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + - // 'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1', 'dog=great,sentry-environment=production,sentry-release=1.0.0,' + 'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + 'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1', ); }); - // TODO: Skipping this test because right now we're rethinking the mechanism for including such data - it.skip('does not add the user_id to the baggage header if is set to false', async () => { + it('adds the transaction name to the the baggage header if a valid transaction source is set', async () => { nock('http://dogs.are.great').get('/').reply(200); - createTransactionOnScope(); + createTransactionOnScope({}, { metadata: { source: 'custom' } }); const request = http.get({ host: 'http://dogs.are.great/', headers: { baggage: 'dog=great' } }); const baggageHeader = request.getHeader('baggage') as string; @@ -153,9 +152,7 @@ describe('tracing', () => { expect(baggageHeader).toBeDefined(); expect(typeof baggageHeader).toEqual('string'); expect(baggageHeader).toEqual( - // Commented out as long as transaction and user_id are not part of DSC - // 'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' + - 'dog=great,sentry-environment=production,sentry-release=1.0.0,' + + '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', ); diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index ea8dced4f47b..eb64f035d57b 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -235,19 +235,17 @@ export class Transaction extends SpanClass implements TransactionInterface { ? rate.toLocaleString('fullwide', { useGrouping: false, maximumFractionDigits: 16 }) : undefined; - // For now we're not sending the transaction name and user_id due to PII concerns - // commenting it out for now because we'll probably need it in the future - const scope = hub.getScope(); - const { /* id: user_id, */ segment: user_segment } = (scope && scope.getUser()) || {}; + const { segment: user_segment } = (scope && scope.getUser()) || {}; + + const source = this.metadata.source; + const transaction = source && source !== 'url' ? this.name : undefined; return createBaggage( dropUndefinedKeys({ environment, release, - // transaction: this.name, - // replace `someContidion` with whatever decision we come up with to guard PII in DSC - // ...(someCondition && { user_id }), + transaction, user_segment, public_key, trace_id: this.traceId, diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index c3f3460b8a58..466f0cfa78ee 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -509,7 +509,6 @@ describe('BrowserTracing', () => { expect(baggage[0]).toEqual({ release: '1.0.0', environment: 'production', - // transaction: 'blank', public_key: 'pubKey', trace_id: expect.not.stringMatching('12312012123120121231201212312012'), }); diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 72b2e6e99d81..16a1a03749d7 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -1,6 +1,6 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, makeMain, Scope } from '@sentry/hub'; -import { BaseTransportOptions, ClientOptions } from '@sentry/types'; +import { BaseTransportOptions, ClientOptions, TransactionSource } from '@sentry/types'; import { createBaggage, getSentryBaggageItems, getThirdPartyBaggage, isSentryBaggageEmpty } from '@sentry/utils'; import { Span, Transaction } from '../src'; @@ -443,7 +443,6 @@ describe('Span', () => { expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({ release: '1.0.1', environment: 'production', - // transaction: 'tx', sample_rate: '0.56', trace_id: expect.any(String), }); @@ -473,6 +472,46 @@ describe('Span', () => { }); expect(baggage && getThirdPartyBaggage(baggage)).toStrictEqual(''); }); + + describe('Including transaction name in DSC', () => { + test.each([ + ['is not included if transaction source is not set', undefined], + ['is not included if transaction source is url', 'url'], + ])('%s', (_: string, source) => { + const transaction = new Transaction( + { + name: 'tx', + metadata: { + ...(source && { source: source as TransactionSource }), + }, + }, + hub, + ); + + const dsc = getSentryBaggageItems(transaction.getBaggage()); + + expect(dsc.transaction).toBeUndefined(); + }); + + test.each([ + ['is included if transaction source is paremeterized route/url', 'route'], + ['is included if transaction source is a custom name', 'custom'], + ])('%s', (_: string, source) => { + const transaction = new Transaction( + { + name: 'tx', + metadata: { + ...(source && { source: source as TransactionSource }), + }, + }, + hub, + ); + + const dsc = getSentryBaggageItems(transaction.getBaggage()); + + expect(dsc.transaction).toEqual('tx'); + }); + }); }); describe('Transaction source', () => { diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 015c4f6cfaef..ef3ddb1c0c4f 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -15,8 +15,7 @@ export type DynamicSamplingContext = { sample_rate?: string; release?: string; environment?: string; - // transaction?: string; // omitted for now - // user_id?: string; // omitted for now + transaction?: string; user_segment?: string; }; diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index beb01ce3e7d6..737253c0aaf2 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -162,8 +162,6 @@ export type TransactionSource = | 'route' /** Name of the view handling the request */ | 'view' - /** This is the default value set by Relay for legacy SDKs. */ - | 'unknown' /** Named after a software component, such as a function or class name. */ | 'component' /** Name of a background task (e.g. a Celery task) */