diff --git a/packages/core/test/lib/request.test.ts b/packages/core/test/lib/request.test.ts index 7f21ffe6b818..445cbb9abb81 100644 --- a/packages/core/test/lib/request.test.ts +++ b/packages/core/test/lib/request.test.ts @@ -28,7 +28,7 @@ describe('eventToSentryRequest', () => { environment: 'dogpark', event_id: '0908201304152013', release: 'off.leash.park', - user: { id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12' }, + user: { id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12', segment: 'bigs' }, }; describe('error/message events', () => { @@ -65,13 +65,15 @@ describe('eventToSentryRequest', () => { // computeTracestateValue({ // trace_id: '1231201211212012', // environment: 'dogpark', - // release: 'off.leash.park', // public_key: 'dogsarebadatkeepingsecrets', + // release: 'off.leash.park', + // user: { id: '1121', segment: 'bigs' }, // }), tracestate: { sentry: - 'sentry=eyJ0cmFjZV9pZCI6IjEyMzEyMDEyMTEyMTIwMTIiLCJlbnZpcm9ubWVudCI6ImRvZ3BhcmsiLCJyZWxlYXNlIjoib2ZmLmxlYXNo' + - 'LnBhcmsiLCJwdWJsaWNfa2V5IjoiZG9nc2FyZWJhZGF0a2VlcGluZ3NlY3JldHMifQ', + 'sentry=eyJ0cmFjZV9pZCI6IjEyMzEyMDEyMTEyMTIwMTIiLCJlbnZpcm9ubWVudCI6ImRvZ3BhcmsiLCJwdWJsaWNfa2V5Ijo' + + 'iZG9nc2FyZWJhZGF0a2VlcGluZ3NlY3JldHMiLCJyZWxlYXNlIjoib2ZmLmxlYXNoLnBhcmsiLCJ1c2VyIjp7ImlkIjoiMTEyM' + + 'SIsInNlZ21lbnQiOiJiaWdzIn19', }, }, spans: [], @@ -160,10 +162,11 @@ describe('eventToSentryRequest', () => { expect(envelope.envelopeHeader.trace).toBeDefined(); expect(envelope.envelopeHeader.trace).toEqual({ + trace_id: '1231201211212012', environment: 'dogpark', public_key: 'dogsarebadatkeepingsecrets', release: 'off.leash.park', - trace_id: '1231201211212012', + user: { id: '1121', segment: 'bigs' }, }); }); }); diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts index 60ec02a2b278..273d33ff08e0 100644 --- a/packages/tracing/src/span.ts +++ b/packages/tracing/src/span.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ -import { getCurrentHub } from '@sentry/hub'; -import { Hub, Primitive, Span as SpanInterface, SpanContext, TraceHeaders, Transaction } from '@sentry/types'; +import { getCurrentHub, Hub } from '@sentry/hub'; +import { Primitive, Span as SpanInterface, SpanContext, TraceHeaders, Transaction } from '@sentry/types'; import { dropUndefinedKeys, logger, timestampWithMs, uuid4 } from '@sentry/utils'; import { SpanStatus } from './spanstatus'; @@ -288,6 +288,14 @@ export class Span implements SpanInterface { * @inheritDoc */ public getTraceHeaders(): TraceHeaders { + // if this span is part of a transaction, but that transaction doesn't yet have a tracestate value, create one + if (this.transaction && !this.transaction?.metadata.tracestate?.sentry) { + this.transaction.metadata.tracestate = { + ...this.transaction.metadata.tracestate, + sentry: this._getNewTracestate(), + }; + } + const tracestate = this._toTracestate(); return { @@ -357,8 +365,11 @@ export class Span implements SpanInterface { * * @returns The new Sentry tracestate entry, or undefined if there's no client or no dsn */ - protected _getNewTracestate(hub: Hub = getCurrentHub()): string | undefined { + protected _getNewTracestate(): string | undefined { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any + const hub = ((this.transaction as any)?._hub as Hub) || getCurrentHub(); const client = hub.getClient(); + const { id: userId, segment: userSegment } = hub.getScope()?.getUser() || {}; const dsn = client?.getDsn(); if (!client || !dsn) { @@ -372,11 +383,12 @@ export class Span implements SpanInterface { // `dsn.publicKey` required and remove the `!`. return `sentry=${computeTracestateValue({ - traceId: this.traceId, + trace_id: this.traceId, environment, release, // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - publicKey: dsn.publicKey!, + public_key: dsn.publicKey!, + user: { id: userId, segment: userSegment }, })}`; } @@ -392,9 +404,11 @@ export class Span implements SpanInterface { } /** - * Return a tracestate-compatible header string. Returns undefined if there is no client or no DSN. + * Return a tracestate-compatible header string, including both sentry and third-party data (if any). Returns + * undefined if there is no client or no DSN. */ private _toTracestate(): string | undefined { + // if this is an orphan span, create a new tracestate value const sentryTracestate = this.transaction?.metadata?.tracestate?.sentry || this._getNewTracestate(); let thirdpartyTracestate = this.transaction?.metadata?.tracestate?.thirdparty; diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index 5af70cc3e52f..a198e8daeb3c 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -40,11 +40,6 @@ export class Transaction extends SpanClass implements TransactionInterface { this._trimEnd = transactionContext.trimEnd; this._hub = hub || getCurrentHub(); - // create a new sentry tracestate value if we didn't inherit one - if (!this.metadata.tracestate?.sentry) { - this.metadata.tracestate = { ...this.metadata.tracestate, sentry: this._getNewTracestate(this._hub) }; - } - // this is because transactions are also spans, and spans have a transaction pointer this.transaction = this; } @@ -117,6 +112,11 @@ export class Transaction extends SpanClass implements TransactionInterface { }).endTimestamp; } + // ensure that we have a tracestate to attach to the envelope header + if (!this.metadata.tracestate?.sentry) { + this.metadata.tracestate = { ...this.metadata.tracestate, sentry: this._getNewTracestate() }; + } + const transaction: Event = { contexts: { trace: this.getTraceContext(), diff --git a/packages/tracing/src/utils.ts b/packages/tracing/src/utils.ts index 2a6ca3bf4423..946d0a75a038 100644 --- a/packages/tracing/src/utils.ts +++ b/packages/tracing/src/utils.ts @@ -145,10 +145,11 @@ export function secToMs(time: number): number { export { stripUrlQueryAndFragment } from '@sentry/utils'; type SentryTracestateData = { - traceId: string; + trace_id: string; environment: string | undefined | null; release: string | undefined | null; - publicKey: string; + public_key: string; + user: { id: string | undefined | null; segment: string | undefined | null }; }; /** @@ -159,9 +160,11 @@ type SentryTracestateData = { */ export function computeTracestateValue(data: SentryTracestateData): string { // `JSON.stringify` will drop keys with undefined values, but not ones with null values, so this prevents - // `environment` and `release` from being dropped if they haven't been set by `Sentry.init` + // these values from being dropped if they haven't been set by `Sentry.init` data.environment = data.environment || null; data.release = data.release || null; + data.user.id = data.user.id || null; + data.user.segment = data.user.segment || null; // See https://www.w3.org/TR/trace-context/#tracestate-header-field-values // The spec for tracestate header values calls for a string of the form diff --git a/packages/tracing/test/httpheaders.test.ts b/packages/tracing/test/httpheaders.test.ts new file mode 100644 index 000000000000..0211d6ffc5d8 --- /dev/null +++ b/packages/tracing/test/httpheaders.test.ts @@ -0,0 +1,251 @@ +import * as sentryCore from '@sentry/core'; +import { API } from '@sentry/core'; +import { Hub } from '@sentry/hub'; +import { SentryRequest } from '@sentry/types'; +import * as utilsPackage from '@sentry/utils'; +import { base64ToUnicode } from '@sentry/utils'; + +import { Span } from '../src/span'; +import { Transaction } from '../src/transaction'; +import { computeTracestateValue } from '../src/utils'; + +// TODO gather sentry-trace and tracestate tests here + +function parseEnvelopeRequest(request: SentryRequest): any { + const [envelopeHeaderString, itemHeaderString, eventString] = request.body.split('\n'); + + return { + envelopeHeader: JSON.parse(envelopeHeaderString), + itemHeader: JSON.parse(itemHeaderString), + event: JSON.parse(eventString), + }; +} + +describe('sentry-trace', () => { + // TODO gather relevant tests here +}); + +describe('tracestate', () => { + // grab these this way rather than importing them individually to get around TS's guards against instantiating + // abstract classes (using the real classes would create a circulr dependency) + const { BaseClient, BaseBackend } = sentryCore as any; + + const dsn = 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012'; + const environment = 'dogpark'; + const release = 'off.leash.trail'; + const hub = new Hub( + new BaseClient(BaseBackend, { + dsn, + environment, + release, + }), + ); + + describe('sentry tracestate', () => { + describe('lazy creation', () => { + const getNewTracestate = jest + .spyOn(Span.prototype as any, '_getNewTracestate') + .mockReturnValue('sentry=doGsaREgReaT'); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + describe('when creating a transaction', () => { + it('uses sentry tracestate passed to the transaction constructor rather than creating a new one', () => { + const transaction = new Transaction({ + name: 'FETCH /ball', + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT' } }, + }); + + expect(getNewTracestate).not.toHaveBeenCalled(); + expect(transaction.metadata.tracestate?.sentry).toEqual('sentry=doGsaREgReaT'); + }); + + it("doesn't create new sentry tracestate on transaction creation if none provided", () => { + const transaction = new Transaction({ + name: 'FETCH /ball', + }); + + expect(transaction.metadata.tracestate?.sentry).toBeUndefined(); + expect(getNewTracestate).not.toHaveBeenCalled(); + }); + }); + + describe('when getting outgoing request headers', () => { + it('uses existing sentry tracestate when getting tracing headers rather than creating new one', () => { + const transaction = new Transaction({ + name: 'FETCH /ball', + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT' } }, + }); + + expect(transaction.getTraceHeaders().tracestate).toEqual('sentry=doGsaREgReaT'); + expect(getNewTracestate).not.toHaveBeenCalled(); + }); + + it('creates and stores new sentry tracestate when getting tracing headers if none exists', () => { + const transaction = new Transaction({ + name: 'FETCH /ball', + }); + + expect(transaction.metadata.tracestate?.sentry).toBeUndefined(); + + transaction.getTraceHeaders(); + + expect(getNewTracestate).toHaveBeenCalled(); + expect(transaction.metadata.tracestate?.sentry).toEqual('sentry=doGsaREgReaT'); + expect(transaction.getTraceHeaders().tracestate).toEqual('sentry=doGsaREgReaT'); + }); + }); + + describe('when getting envelope headers', () => { + // In real life, `transaction.finish()` calls `captureEvent()`, which eventually calls `eventToSentryRequest()`, + // which in turn calls `base64ToUnicode`. Here we're short circuiting that process a little, to avoid having to + // mock out more of the intermediate pieces. + jest + .spyOn(utilsPackage, 'base64ToUnicode') + .mockImplementation(base64 => + base64 === 'doGsaREgReaT' ? '{"all the":"right stuff here"}' : '{"nope nope nope":"wrong"}', + ); + jest.spyOn(hub, 'captureEvent').mockImplementation(event => { + expect(event).toEqual( + expect.objectContaining({ debug_meta: { tracestate: { sentry: 'sentry=doGsaREgReaT' } } }), + ); + + const envelope = parseEnvelopeRequest(sentryCore.eventToSentryRequest(event, new API(dsn))); + expect(envelope.envelopeHeader).toEqual( + expect.objectContaining({ trace: { 'all the': 'right stuff here' } }), + ); + + // `captureEvent` normally returns the event id + return '11212012041520131231201209082013'; // + }); + + it('uses existing sentry tracestate in envelope headers rather than creating a new one', () => { + // one here, and two inside the `captureEvent` implementation above + expect.assertions(3); + + const transaction = new Transaction( + { + name: 'FETCH /ball', + metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT' } }, + sampled: true, + }, + hub, + ); + + transaction.finish(); + + expect(getNewTracestate).not.toHaveBeenCalled(); + }); + + it('creates new sentry tracestate for envelope header if none exists', () => { + // two here, and two inside the `captureEvent` implementation above + expect.assertions(4); + + const transaction = new Transaction( + { + name: 'FETCH /ball', + sampled: true, + }, + hub, + ); + + expect(transaction.metadata.tracestate?.sentry).toBeUndefined(); + + transaction.finish(); + + expect(getNewTracestate).toHaveBeenCalled(); + }); + }); + }); + + describe('mutibility', () => { + it("won't include data set after transaction is created if there's an inherited value", () => { + expect.assertions(2); + + const inheritedTracestate = `sentry=${computeTracestateValue({ + trace_id: '12312012090820131231201209082013', + environment: 'dogpark', + release: 'off.leash.trail', + public_key: 'dogsarebadatkeepingsecrets', + user: { id: undefined, segment: undefined }, + })}`; + + const transaction = new Transaction( + { + name: 'FETCH /ball', + metadata: { + tracestate: { + sentry: inheritedTracestate, + }, + }, + }, + hub, + ); + + hub.withScope(scope => { + scope.setUser({ id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12', segment: 'bigs' }); + + const tracestateValue = (transaction as any)._toTracestate().replace('sentry=', ''); + const reinflatedTracestate = JSON.parse(base64ToUnicode(tracestateValue)); + + expect(reinflatedTracestate.user.id).toBeNull(); + expect(reinflatedTracestate.user.segment).toBeNull(); + }); + }); + + it("will include data set after transaction is created if there's no inherited value and `getTraceHeaders` hasn't been called", () => { + expect.assertions(2); + + const transaction = new Transaction( + { + name: 'FETCH /ball', + }, + hub, + ); + + hub.withScope(scope => { + scope.setUser({ id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12', segment: 'bigs' }); + + const tracestateValue = (transaction as any)._toTracestate().replace('sentry=', ''); + const reinflatedTracestate = JSON.parse(base64ToUnicode(tracestateValue)); + + expect(reinflatedTracestate.user.id).toEqual('1121'); + expect(reinflatedTracestate.user.segment).toEqual('bigs'); + }); + }); + + it("won't include data set after first call to `getTraceHeaders`", () => { + expect.assertions(2); + + const transaction = new Transaction( + { + name: 'FETCH /ball', + }, + hub, + ); + + transaction.getTraceHeaders(); + + hub.withScope(scope => { + scope.setUser({ id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12', segment: 'bigs' }); + + const tracestateValue = (transaction as any)._toTracestate().replace('sentry=', ''); + const reinflatedTracestate = JSON.parse(base64ToUnicode(tracestateValue)); + + expect(reinflatedTracestate.user.id).toBeNull(); + expect(reinflatedTracestate.user.segment).toBeNull(); + }); + }); + }); + }); + + describe('third-party tracestate', () => { + // TODO gather relevant tests here + }); +}); diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index 4dc977a4539a..ae7ee9b0c1ff 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/unbound-method */ -import { BrowserClient, init as initSDK } from '@sentry/browser'; -import { getCurrentHub, Hub } from '@sentry/hub'; +import { BrowserClient } from '@sentry/browser'; +import { Hub } from '@sentry/hub'; import * as hubModule from '@sentry/hub'; import { TransactionSamplingMethod } from '@sentry/types'; import * as utilsModule from '@sentry/utils'; // for mocking @@ -9,7 +9,7 @@ import { logger } from '@sentry/utils'; import { BrowserTracing } from '../src/browser/browsertracing'; import { addExtensionMethods } from '../src/hubextensions'; import { Transaction } from '../src/transaction'; -import { computeTracestateValue, extractSentrytraceData, SENTRY_TRACE_REGEX } from '../src/utils'; +import { extractSentrytraceData, SENTRY_TRACE_REGEX } from '../src/utils'; import { addDOMPropertiesToGlobal, getSymbolObjectKeyByName, testOnlyIfNodeVersionAtLeast } from './testutils'; addExtensionMethods(); @@ -50,68 +50,6 @@ describe('Hub', () => { expect(transaction).toEqual(expect.objectContaining(transactionContext)); }); - - it('creates a new tracestate value if no tracestate data in transaction context', () => { - const transaction = hub.startTransaction({ name: 'FETCH /ball' }); - - const b64Value = computeTracestateValue({ - traceId: transaction.traceId, - environment: 'dogpark', - release: 'off.leash.trail', - publicKey: 'dogsarebadatkeepingsecrets', - }); - - expect(transaction.metadata?.tracestate?.sentry).toEqual(`sentry=${b64Value}`); - }); - - it('creates a new tracestate value if tracestate data in transaction context only contains third party data', () => { - const transactionContext = { - name: 'FETCH /ball', - traceId: '12312012123120121231201212312012', - parentSpanId: '1121201211212012', - metadata: { tracestate: { thirdparty: 'maisey=silly;charlie=goofy' } }, - }; - - const transaction = hub.startTransaction(transactionContext); - - const b64Value = computeTracestateValue({ - traceId: transaction.traceId, - environment: 'dogpark', - release: 'off.leash.trail', - publicKey: 'dogsarebadatkeepingsecrets', - }); - - expect(transaction).toEqual( - expect.objectContaining({ - metadata: { - tracestate: { - // a new value for `sentry` is created - sentry: `sentry=${b64Value}`, - // the third-party value isn't lost - thirdparty: 'maisey=silly;charlie=goofy', - }, - }, - }), - ); - }); - - it('uses default environment if none given', () => { - const release = 'off.leash.park'; - initSDK({ - dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', - release, - }); - const transaction = getCurrentHub().startTransaction({ name: 'FETCH /ball' }); - - const b64Value = computeTracestateValue({ - traceId: transaction.traceId, - environment: 'production', - release, - publicKey: 'dogsarebadatkeepingsecrets', - }); - - expect(transaction.metadata?.tracestate?.sentry).toEqual(`sentry=${b64Value}`); - }); }); describe('getTransaction()', () => { diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 431c7e4ee718..fc19e1de429f 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -108,8 +108,15 @@ describe('Span', () => { const release = 'off.leash.trail'; const environment = 'dogpark'; const traceId = '12312012123120121231201212312012'; - - const computedTracestate = `sentry=${computeTracestateValue({ traceId, environment, release, publicKey })}`; + const user = { id: '1121', segment: 'bigs' }; + + const computedTracestate = `sentry=${computeTracestateValue({ + trace_id: traceId, + environment, + release, + public_key: publicKey, + user, + })}`; const thirdpartyData = 'maisey=silly,charlie=goofy'; const hub = new Hub( @@ -121,6 +128,10 @@ describe('Span', () => { }), ); + hub.configureScope(scope => { + scope.setUser(user); + }); + test('no third-party data', () => { const transaction = new Transaction({ name: 'FETCH /ball', traceId }, hub); const span = transaction.startChild({ op: 'dig.hole' }); diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index f355acd9644e..f937c28bedc7 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -63,6 +63,11 @@ export interface Transaction extends TransactionContext, Span { */ data: { [key: string]: any }; + /** + * Metadata about the transaction + */ + metadata: TransactionMetadata; + /** * Set the name of the transaction */ diff --git a/packages/types/src/user.ts b/packages/types/src/user.ts index eae46b008b3e..3964096aec77 100644 --- a/packages/types/src/user.ts +++ b/packages/types/src/user.ts @@ -5,4 +5,5 @@ export interface User { ip_address?: string; email?: string; username?: string; + segment?: string; }