diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index ef5635f98541..e3222e2bc4ac 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -504,8 +504,7 @@ export abstract class BaseClient implements Client { // For now the decision is to skip normalization of Transactions and Spans, // so this block overwrites the normalized event to add back the original // Transaction information prior to normalization. - if (event.contexts && event.contexts.trace) { - normalized.contexts = {}; + if (event.contexts && event.contexts.trace && normalized.contexts) { normalized.contexts.trace = event.contexts.trace; // event.contexts.trace.data may contain circular/dangerous data so we need to normalize it diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 0fb974d66bc7..82df1837db22 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,9 +1,11 @@ import { + BaggageObj, DsnComponents, Event, EventEnvelope, EventEnvelopeHeaders, EventItem, + EventTraceContext, SdkInfo, SdkMetadata, Session, @@ -120,28 +122,34 @@ function createEventEnvelopeHeaders( tunnel: string | undefined, dsn: DsnComponents, ): EventEnvelopeHeaders { + const baggage = event.contexts && (event.contexts.baggage as BaggageObj); + const { environment, release, transaction, userid, usersegment } = baggage || {}; + return { event_id: event.event_id as string, sent_at: new Date().toISOString(), ...(sdkInfo && { sdk: sdkInfo }), ...(!!tunnel && { dsn: dsnToString(dsn) }), ...(event.type === 'transaction' && + // If we don't already have a trace context in the event, we can't get the trace id, which makes adding any other + // trace data pointless event.contexts && event.contexts.trace && { - // TODO: Grab this from baggage trace: dropUndefinedKeys({ // Trace context must be defined for transactions // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - trace_id: event.contexts!.trace.trace_id as string, - environment: event.environment, - release: event.release, - transaction: event.transaction, - user: event.user && { - id: event.user.id, - segment: event.user.segment, - }, + trace_id: (event.contexts!.trace as Record).trace_id as string, public_key: dsn.publicKey, - }), + environment: environment, + release: release, + transaction: transaction, + ...((userid || usersegment) && { + user: { + id: userid, + segment: usersegment, + }, + }), + }) as EventTraceContext, }), }; } diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index 13dda0e276d4..8577805330b7 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -31,14 +31,16 @@ describe('createEventEnvelope', () => { const testTable: Array<[string, Event, EventTraceContext]> = [ [ - 'adds only baggage item', + 'adds one baggage item', { type: 'transaction', - release: '1.0.0', contexts: { trace: { trace_id: '1234', }, + baggage: { + release: '1.0.0', + }, }, }, { release: '1.0.0', trace_id: '1234', public_key: 'pubKey123' }, @@ -47,28 +49,33 @@ describe('createEventEnvelope', () => { 'adds two baggage items', { type: 'transaction', - release: '1.0.0', - environment: 'prod', contexts: { trace: { trace_id: '1234', }, + baggage: { + environment: 'prod', + release: '1.0.0', + }, }, }, { release: '1.0.0', environment: 'prod', trace_id: '1234', public_key: 'pubKey123' }, ], [ - 'adds all baggageitems', + 'adds all baggage items', { type: 'transaction', - release: '1.0.0', - environment: 'prod', - user: { id: 'bob', segment: 'segmentA' }, - transaction: 'TX', contexts: { trace: { trace_id: '1234', }, + baggage: { + environment: 'prod', + release: '1.0.0', + userid: 'bob', + usersegment: 'segmentA', + transaction: 'TX', + }, }, }, { diff --git a/packages/integration-tests/suites/tracing/baggage/test.ts b/packages/integration-tests/suites/tracing/baggage/test.ts index 5dcb8e82bcf1..4a8bbf497806 100644 --- a/packages/integration-tests/suites/tracing/baggage/test.ts +++ b/packages/integration-tests/suites/tracing/baggage/test.ts @@ -12,11 +12,12 @@ sentryTest('should send trace context data in transaction envelope header', asyn expect(envHeader.trace).toBeDefined(); expect(envHeader.trace).toMatchObject({ environment: 'production', - transaction: 'testTransactionBaggage', - user: { - id: 'user123', - segment: 'segmentB', - }, + // TODO comment back in once we properly support transaction and user data in Baggage + // transaction: 'testTransactionBaggage', + // user: { + // id: 'user123', + // segment: 'segmentB', + // }, public_key: 'public', trace_id: expect.any(String), }); diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/init.js b/packages/integration-tests/suites/tracing/browsertracing/meta/init.js new file mode 100644 index 000000000000..50f4a898e251 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/meta/init.js @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/browser'; +import { Integrations } from '@sentry/tracing'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [new Integrations.BrowserTracing()], + tracesSampleRate: 1, + environment: 'staging', +}); diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/template.html b/packages/integration-tests/suites/tracing/browsertracing/meta/template.html index 60c6c062c7f3..bff098c0f4f2 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/meta/template.html +++ b/packages/integration-tests/suites/tracing/browsertracing/meta/template.html @@ -2,6 +2,6 @@ - + diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts index d2d44ae74050..a1f18646a9af 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts +++ b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts @@ -21,17 +21,19 @@ sentryTest( }, ); -// TODO this we can't really test until we actually propagate sentry- entries in baggage -// skipping for now but this must be adjusted later on -sentryTest.skip( - 'should pick up `baggage` tag and propagate the content in transaction', +sentryTest( + 'should pick up `baggage` tag, propagate the content in transaction and not add own data', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); expect(envHeader.trace).toBeDefined(); - expect(envHeader.trace).toEqual('{version:2.1.12}'); + expect(envHeader.trace).toEqual({ + public_key: 'public', + trace_id: expect.any(String), + release: '2.1.12', + }); }, ); diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts index c10299996e0f..800608c62488 100644 --- a/packages/tracing/src/span.ts +++ b/packages/tracing/src/span.ts @@ -4,7 +4,6 @@ import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext, Transactio import { createBaggage, dropUndefinedKeys, - isBaggageEmpty, isBaggageMutable, isSentryBaggageEmpty, setBaggageImmutable, @@ -312,7 +311,7 @@ export class Span implements SpanInterface { /** * @inheritdoc */ - public getBaggage(): Baggage | undefined { + public getBaggage(): Baggage { const existingBaggage = this.transaction && this.transaction.metadata.baggage; // Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still @@ -322,7 +321,7 @@ export class Span implements SpanInterface { ? this._getBaggageWithSentryValues(existingBaggage) : existingBaggage; - return isBaggageEmpty(finalBaggage) ? undefined : finalBaggage; + return finalBaggage; } /** @@ -366,7 +365,7 @@ export class Span implements SpanInterface { * * @param baggage * - * @returns modified and immutable maggage object + * @returns modified and immutable baggage object */ private _getBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index d51f4578717a..0e31ae583a41 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -6,7 +6,7 @@ import { TransactionContext, TransactionMetadata, } from '@sentry/types'; -import { dropUndefinedKeys, logger } from '@sentry/utils'; +import { dropUndefinedKeys, getSentryBaggageItems, logger } from '@sentry/utils'; import { Span as SpanClass, SpanRecorder } from './span'; @@ -122,6 +122,7 @@ export class Transaction extends SpanClass implements TransactionInterface { const transaction: Event = { contexts: { trace: this.getTraceContext(), + baggage: getSentryBaggageItems(this.getBaggage()), }, spans: finishedSpans, start_timestamp: this.startTimestamp, diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index f6739ff9f736..a3ad316a8026 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -151,14 +151,14 @@ export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggag * Extracted this logic to a function because it's duplicated in a lot of places. * * @param rawBaggageString - * @param sentryTraceData + * @param sentryTraceHeader */ export function parseBaggageSetMutability( rawBaggageString: string | false | undefined | null, - sentryTraceData: TraceparentData | string | false | undefined | null, + sentryTraceHeader: TraceparentData | string | false | undefined | null, ): Baggage { const baggage = parseBaggageString(rawBaggageString || ''); - if (!isSentryBaggageEmpty(baggage) || (sentryTraceData && isSentryBaggageEmpty(baggage))) { + if (!isSentryBaggageEmpty(baggage) || (sentryTraceHeader && isSentryBaggageEmpty(baggage))) { setBaggageImmutable(baggage); } return baggage;