From 0ef7352a477083aaffd79fd175a214fec5e7a409 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 13 May 2022 12:19:01 +0200 Subject: [PATCH 01/11] feat(core): Send baggage in envelope header WIP, currently a very simple and unsustainable solution that doesn't take immutability or additional data into account --- packages/core/src/baseclient.ts | 2 +- packages/core/src/envelope.ts | 5 ++++- packages/types/src/envelope.ts | 2 +- packages/utils/src/index.ts | 1 + 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index f64f2b11d114..563c8ce599c3 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -618,7 +618,7 @@ export abstract class BaseClient implements Client { throw new SentryError('`beforeSend` returned `null`, will not send event.'); } - const session = scope && scope.getSession && scope.getSession(); + const session = scope && scope.getSession(); if (!isTransaction && session) { this._updateSessionFromEvent(session, processedEvent); } diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 2f69884e8bc3..146fbdcd8f42 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -10,7 +10,7 @@ import { SessionEnvelope, SessionItem, } from '@sentry/types'; -import { createEnvelope, dsnToString } from '@sentry/utils'; +import { createBaggage, createEnvelope, dsnToString, serializeBaggage } from '@sentry/utils'; /** Extract sdk info from from the API metadata */ function getSdkMetadataForEnvelopeHeader(metadata?: SdkMetadata): SdkInfo | undefined { @@ -101,9 +101,12 @@ export function createEventEnvelope( // TODO: This is NOT part of the hack - DO NOT DELETE delete event.sdkProcessingMetadata; + const baggage = createBaggage({ environment: event.environment, release: event.release }); + const envelopeHeaders = { event_id: event.event_id as string, sent_at: new Date().toISOString(), + baggage: serializeBaggage(baggage), ...(sdkInfo && { sdk: sdkInfo }), ...(!!tunnel && { dsn: dsnToString(dsn) }), }; diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 33846f077a79..487212d793a9 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -56,7 +56,7 @@ export type SessionItem = | BaseEnvelopeItem; export type ClientReportItem = BaseEnvelopeItem; -type EventEnvelopeHeaders = { event_id: string; sent_at: string }; +type EventEnvelopeHeaders = { event_id: string; sent_at: string; baggage?: string }; type SessionEnvelopeHeaders = { sent_at: string }; type ClientReportEnvelopeHeaders = BaseEnvelopeHeaders; diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 01b875dc31c3..fd627db6f2e5 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -23,3 +23,4 @@ export * from './env'; export * from './envelope'; export * from './clientreport'; export * from './ratelimit'; +export * from './baggage'; From ceec823acd10432ad704872c17843e58cf88b7bf Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 13 May 2022 17:36:16 +0200 Subject: [PATCH 02/11] add other properties, only attach baggage to transaction events --- packages/core/src/envelope.ts | 25 ++++++++++++++++++++----- packages/types/src/envelope.ts | 2 +- packages/types/src/index.ts | 1 + packages/utils/src/baggage.ts | 2 +- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 146fbdcd8f42..2a6961dbb1af 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -2,6 +2,7 @@ import { DsnComponents, Event, EventEnvelope, + EventEnvelopeHeaders, EventItem, SdkInfo, SdkMetadata, @@ -10,7 +11,7 @@ import { SessionEnvelope, SessionItem, } from '@sentry/types'; -import { createBaggage, createEnvelope, dsnToString, serializeBaggage } from '@sentry/utils'; +import { BaggageObj, createBaggage, createEnvelope, dsnToString, serializeBaggage } from '@sentry/utils'; /** Extract sdk info from from the API metadata */ function getSdkMetadataForEnvelopeHeader(metadata?: SdkMetadata): SdkInfo | undefined { @@ -101,15 +102,29 @@ export function createEventEnvelope( // TODO: This is NOT part of the hack - DO NOT DELETE delete event.sdkProcessingMetadata; - const baggage = createBaggage({ environment: event.environment, release: event.release }); - - const envelopeHeaders = { + let envelopeHeaders: EventEnvelopeHeaders = { event_id: event.event_id as string, sent_at: new Date().toISOString(), - baggage: serializeBaggage(baggage), ...(sdkInfo && { sdk: sdkInfo }), ...(!!tunnel && { dsn: dsnToString(dsn) }), }; + + if (eventType === 'transaction') { + const baggage = createBaggage({ + environment: event.environment, + release: event.release, + transaction: event.transaction, + userid: event.user && event.user.id, + // TODO user.segment currently doesn't exist explicitly in interface User (just as a record key) + usersegment: event.user && event.user.segment, + } as BaggageObj); + + envelopeHeaders = { + ...envelopeHeaders, + baggage: serializeBaggage(baggage), + }; + } + const eventItem: EventItem = [ { type: eventType, diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 487212d793a9..7e350b129b21 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -56,7 +56,7 @@ export type SessionItem = | BaseEnvelopeItem; export type ClientReportItem = BaseEnvelopeItem; -type EventEnvelopeHeaders = { event_id: string; sent_at: string; baggage?: string }; +export type EventEnvelopeHeaders = { event_id: string; sent_at: string; baggage?: string }; type SessionEnvelopeHeaders = { sent_at: string }; type ClientReportEnvelopeHeaders = BaseEnvelopeHeaders; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 8fe14cd3ac16..48c2ef6ff5e4 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -15,6 +15,7 @@ export type { EnvelopeItemType, EnvelopeItem, EventEnvelope, + EventEnvelopeHeaders, EventItem, SessionEnvelope, SessionItem, diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index db402608a22b..8c683a1aec7d 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -1,7 +1,7 @@ import { IS_DEBUG_BUILD } from './flags'; import { logger } from './logger'; -export type AllowedBaggageKeys = 'environment' | 'release'; // TODO: Add remaining allowed baggage keys | 'transaction' | 'userid' | 'usersegment'; +export type AllowedBaggageKeys = 'environment' | 'release' | 'userid' | 'transaction' | 'usersegment'; export type BaggageObj = Partial & Record>; /** From a7e24ae6678c4a430da35233be2659b3de91c5eb Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 May 2022 10:13:30 +0200 Subject: [PATCH 03/11] drop undefined baggage keys, only add baggage header if baggage not empty --- packages/core/src/envelope.ts | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 2a6961dbb1af..86e1896acf48 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -11,7 +11,14 @@ import { SessionEnvelope, SessionItem, } from '@sentry/types'; -import { BaggageObj, createBaggage, createEnvelope, dsnToString, serializeBaggage } from '@sentry/utils'; +import { + BaggageObj, + createBaggage, + createEnvelope, + dsnToString, + dropUndefinedKeys, + serializeBaggage, +} from '@sentry/utils'; /** Extract sdk info from from the API metadata */ function getSdkMetadataForEnvelopeHeader(metadata?: SdkMetadata): SdkInfo | undefined { @@ -110,18 +117,20 @@ export function createEventEnvelope( }; if (eventType === 'transaction') { - const baggage = createBaggage({ - environment: event.environment, - release: event.release, - transaction: event.transaction, - userid: event.user && event.user.id, - // TODO user.segment currently doesn't exist explicitly in interface User (just as a record key) - usersegment: event.user && event.user.segment, - } as BaggageObj); + const baggage = dropUndefinedKeys( + createBaggage({ + environment: event.environment && event.environment, + release: event.release, + transaction: event.transaction, + userid: event.user && event.user.id, + // TODO user.segment currently doesn't exist explicitly in interface User (just as a record key) + usersegment: event.user && event.user.segment, + } as BaggageObj), + ); envelopeHeaders = { ...envelopeHeaders, - baggage: serializeBaggage(baggage), + ...(Object.keys(baggage[0]).length > 0 && { baggage: serializeBaggage(baggage) }), }; } From 24412cb790b77032132e6944e1b068c1bc7f7e07 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 May 2022 10:14:19 +0200 Subject: [PATCH 04/11] add `createEventEnvelope` unit tests for baggage handling --- packages/core/test/lib/envelope.test.ts | 53 +++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 packages/core/test/lib/envelope.test.ts diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts new file mode 100644 index 000000000000..4afed96059b1 --- /dev/null +++ b/packages/core/test/lib/envelope.test.ts @@ -0,0 +1,53 @@ +import { createEventEnvelope } from '../../src/envelope'; +import { DsnComponents, Event } from '@sentry/types'; + +const testDsn: DsnComponents = { protocol: 'https', projectId: 'abc', host: 'testry.io' }; + +describe('createEventEnvelope', () => { + describe('baggage header', () => { + it(`doesn't add baggage header if event is not a transaction`, () => { + const event: Event = {}; + const envelopeHeaders = createEventEnvelope(event, testDsn)[0]; + + expect(envelopeHeaders).toBeDefined(); + expect(envelopeHeaders.baggage).toBeUndefined(); + }); + + it(`doesn't add baggage header if no baggage data is available`, () => { + const event: Event = { + type: 'transaction', + }; + const envelopeHeaders = createEventEnvelope(event, testDsn)[0]; + + expect(envelopeHeaders).toBeDefined(); + expect(envelopeHeaders.baggage).toBeUndefined(); + }); + + const testTable: Array<[string, Event, string]> = [ + ['adds only baggage item', { type: 'transaction', release: '1.0.0' }, 'sentry-release=1.0.0'], + [ + 'adds two baggage items', + { type: 'transaction', release: '1.0.0', environment: 'prod' }, + 'sentry-environment=prod,sentry-release=1.0.0', + ], + [ + 'adds all baggageitems', + { + type: 'transaction', + release: '1.0.0', + environment: 'prod', + user: { id: 'bob', segment: 'segmentA' }, + transaction: 'TX', + }, + 'sentry-environment=prod,sentry-release=1.0.0,sentry-transaction=TX,sentry-userid=bob,sentry-usersegment=segmentA', + ], + ]; + it.each(testTable)('%s', (_: string, event, serializedBaggage) => { + const envelopeHeaders = createEventEnvelope(event, testDsn)[0]; + + expect(envelopeHeaders).toBeDefined(); + expect(envelopeHeaders.baggage).toBeDefined(); + expect(envelopeHeaders.baggage).toEqual(serializedBaggage); + }); + }); +}); From 97790a04d648cf9bf5c346347fb974d15f9d4a1e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 May 2022 11:03:32 +0200 Subject: [PATCH 05/11] add baggage in envelope header integration test --- packages/core/src/envelope.ts | 4 ++-- .../suites/tracing/baggage/init.js | 14 +++++++++++++ .../suites/tracing/baggage/subject.js | 4 ++++ .../suites/tracing/baggage/template.html | 8 +++++++ .../suites/tracing/baggage/test.ts | 21 +++++++++++++++++++ packages/integration-tests/utils/helpers.ts | 21 +++++++++++++++---- 6 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 packages/integration-tests/suites/tracing/baggage/init.js create mode 100644 packages/integration-tests/suites/tracing/baggage/subject.js create mode 100644 packages/integration-tests/suites/tracing/baggage/template.html create mode 100644 packages/integration-tests/suites/tracing/baggage/test.ts diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 86e1896acf48..29d7b9a5addf 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -119,11 +119,11 @@ export function createEventEnvelope( if (eventType === 'transaction') { const baggage = dropUndefinedKeys( createBaggage({ - environment: event.environment && event.environment, + environment: event.environment, release: event.release, transaction: event.transaction, userid: event.user && event.user.id, - // TODO user.segment currently doesn't exist explicitly in interface User (just as a record key) + // user.segment currently doesn't exist explicitly in interface User (just as a record key) usersegment: event.user && event.user.segment, } as BaggageObj), ); diff --git a/packages/integration-tests/suites/tracing/baggage/init.js b/packages/integration-tests/suites/tracing/baggage/init.js new file mode 100644 index 000000000000..822c48a054ea --- /dev/null +++ b/packages/integration-tests/suites/tracing/baggage/init.js @@ -0,0 +1,14 @@ +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()], + release: '1.0.0', + environment: 'production', + tracesSampleRate: 1, +}); + +Sentry.configureScope(scope => scope.setUser({ id: 'user123', segment: 'segmentB' })); diff --git a/packages/integration-tests/suites/tracing/baggage/subject.js b/packages/integration-tests/suites/tracing/baggage/subject.js new file mode 100644 index 000000000000..58cc6d0f07d1 --- /dev/null +++ b/packages/integration-tests/suites/tracing/baggage/subject.js @@ -0,0 +1,4 @@ +document.getElementById('start-transaction').addEventListener('click', () => { + window.transaction = Sentry.startTransaction({ name: 'test-transaction' }); + Sentry.getCurrentHub().configureScope(scope => scope.setSpan(window.transaction)); +}); diff --git a/packages/integration-tests/suites/tracing/baggage/template.html b/packages/integration-tests/suites/tracing/baggage/template.html new file mode 100644 index 000000000000..d88fc9c0d1c9 --- /dev/null +++ b/packages/integration-tests/suites/tracing/baggage/template.html @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/packages/integration-tests/suites/tracing/baggage/test.ts b/packages/integration-tests/suites/tracing/baggage/test.ts new file mode 100644 index 000000000000..305deaa57279 --- /dev/null +++ b/packages/integration-tests/suites/tracing/baggage/test.ts @@ -0,0 +1,21 @@ +import { expect } from '@playwright/test'; +import { EventEnvelopeHeaders } from '@sentry/types'; + +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; + +sentryTest('should send baggage data in transaction envelope header', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadTransaction = await getFirstSentryEnvelopeRequest(page, url); + expect(pageloadTransaction).toBeDefined(); + + await page.click('#start-transaction'); + + const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); + + expect(envHeader.baggage).toBeDefined(); + expect(envHeader.baggage).toEqual( + 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=test-transaction,sentry-userid=user123,sentry-usersegment=segmentB', + ); +}); diff --git a/packages/integration-tests/utils/helpers.ts b/packages/integration-tests/utils/helpers.ts index 8f6e06b97d5a..34b512ec9e01 100644 --- a/packages/integration-tests/utils/helpers.ts +++ b/packages/integration-tests/utils/helpers.ts @@ -1,5 +1,5 @@ import { Page, Request } from '@playwright/test'; -import { Event } from '@sentry/types'; +import { Event, EventEnvelopeHeaders } from '@sentry/types'; const envelopeUrlRegex = /\.sentry\.io\/api\/\d+\/envelope\//; @@ -11,6 +11,14 @@ const envelopeRequestParser = (request: Request | null): Event => { return envelope.split('\n').map(line => JSON.parse(line))[2]; }; +export const envelopeHeaderRequestParser = (request: Request | null): EventEnvelopeHeaders => { + // https://develop.sentry.dev/sdk/envelopes/ + const envelope = request?.postData() || ''; + + // First row of the envelop is the event payload. + return envelope.split('\n').map(line => JSON.parse(line))[0]; +}; + /** * Run script at the given path inside the test environment. * @@ -122,10 +130,11 @@ async function getMultipleSentryEnvelopeRequests( url?: string; timeout?: number; }, + requestParser: (req: Request) => T = envelopeRequestParser as (req: Request) => T, ): Promise { // TODO: This is not currently checking the type of envelope, just casting for now. // We can update this to include optional type-guarding when we have types for Envelope. - return getMultipleRequests(page, count, envelopeUrlRegex, envelopeRequestParser, options) as Promise; + return getMultipleRequests(page, count, envelopeUrlRegex, requestParser, options) as Promise; } /** @@ -136,8 +145,12 @@ async function getMultipleSentryEnvelopeRequests( * @param {string} [url] * @return {*} {Promise} */ -async function getFirstSentryEnvelopeRequest(page: Page, url?: string): Promise { - return (await getMultipleSentryEnvelopeRequests(page, 1, { url }))[0]; +async function getFirstSentryEnvelopeRequest( + page: Page, + url?: string, + requestParser: (req: Request) => T = envelopeRequestParser as (req: Request) => T, +): Promise { + return (await getMultipleSentryEnvelopeRequests(page, 1, { url }, requestParser))[0]; } /** From 7ff814c5ad2daea3c579c3c8ccf3a66733110c70 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 May 2022 11:08:48 +0200 Subject: [PATCH 06/11] fix linter errors --- packages/core/src/envelope.ts | 2 +- packages/core/test/lib/envelope.test.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 29d7b9a5addf..04f9e577b6c9 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -15,8 +15,8 @@ import { BaggageObj, createBaggage, createEnvelope, - dsnToString, dropUndefinedKeys, + dsnToString, serializeBaggage, } from '@sentry/utils'; diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index 4afed96059b1..a5c8cb8ae06a 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -1,11 +1,12 @@ -import { createEventEnvelope } from '../../src/envelope'; import { DsnComponents, Event } from '@sentry/types'; +import { createEventEnvelope } from '../../src/envelope'; + const testDsn: DsnComponents = { protocol: 'https', projectId: 'abc', host: 'testry.io' }; describe('createEventEnvelope', () => { describe('baggage header', () => { - it(`doesn't add baggage header if event is not a transaction`, () => { + it("doesn't add baggage header if event is not a transaction", () => { const event: Event = {}; const envelopeHeaders = createEventEnvelope(event, testDsn)[0]; @@ -13,7 +14,7 @@ describe('createEventEnvelope', () => { expect(envelopeHeaders.baggage).toBeUndefined(); }); - it(`doesn't add baggage header if no baggage data is available`, () => { + it("doesn't add baggage header if no baggage data is available", () => { const event: Event = { type: 'transaction', }; From 263599b19e98e3c6a46181c3621f3891282d3047 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 May 2022 14:08:19 +0200 Subject: [PATCH 07/11] simplify integration test --- packages/core/src/envelope.ts | 5 ++--- .../integration-tests/suites/tracing/baggage/init.js | 8 +++++--- .../suites/tracing/baggage/subject.js | 4 ---- .../suites/tracing/baggage/template.html | 8 -------- .../integration-tests/suites/tracing/baggage/test.ts | 11 ++++------- 5 files changed, 11 insertions(+), 25 deletions(-) delete mode 100644 packages/integration-tests/suites/tracing/baggage/subject.js delete mode 100644 packages/integration-tests/suites/tracing/baggage/template.html diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 04f9e577b6c9..f06c98824a4d 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -117,8 +117,8 @@ export function createEventEnvelope( }; if (eventType === 'transaction') { - const baggage = dropUndefinedKeys( - createBaggage({ + const baggage = createBaggage( + dropUndefinedKeys({ environment: event.environment, release: event.release, transaction: event.transaction, @@ -127,7 +127,6 @@ export function createEventEnvelope( usersegment: event.user && event.user.segment, } as BaggageObj), ); - envelopeHeaders = { ...envelopeHeaders, ...(Object.keys(baggage[0]).length > 0 && { baggage: serializeBaggage(baggage) }), diff --git a/packages/integration-tests/suites/tracing/baggage/init.js b/packages/integration-tests/suites/tracing/baggage/init.js index 822c48a054ea..5cd0764d4da5 100644 --- a/packages/integration-tests/suites/tracing/baggage/init.js +++ b/packages/integration-tests/suites/tracing/baggage/init.js @@ -5,10 +5,12 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [new Integrations.BrowserTracing()], - release: '1.0.0', + integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] })], environment: 'production', tracesSampleRate: 1, }); -Sentry.configureScope(scope => scope.setUser({ id: 'user123', segment: 'segmentB' })); +Sentry.configureScope(scope => { + scope.setUser({ id: 'user123', segment: 'segmentB' }); + scope.setTransactionName('testTransactionBaggage'); +}); diff --git a/packages/integration-tests/suites/tracing/baggage/subject.js b/packages/integration-tests/suites/tracing/baggage/subject.js deleted file mode 100644 index 58cc6d0f07d1..000000000000 --- a/packages/integration-tests/suites/tracing/baggage/subject.js +++ /dev/null @@ -1,4 +0,0 @@ -document.getElementById('start-transaction').addEventListener('click', () => { - window.transaction = Sentry.startTransaction({ name: 'test-transaction' }); - Sentry.getCurrentHub().configureScope(scope => scope.setSpan(window.transaction)); -}); diff --git a/packages/integration-tests/suites/tracing/baggage/template.html b/packages/integration-tests/suites/tracing/baggage/template.html deleted file mode 100644 index d88fc9c0d1c9..000000000000 --- a/packages/integration-tests/suites/tracing/baggage/template.html +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - diff --git a/packages/integration-tests/suites/tracing/baggage/test.ts b/packages/integration-tests/suites/tracing/baggage/test.ts index 305deaa57279..9847d2e54430 100644 --- a/packages/integration-tests/suites/tracing/baggage/test.ts +++ b/packages/integration-tests/suites/tracing/baggage/test.ts @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import { EventEnvelopeHeaders } from '@sentry/types'; +import { Event, EventEnvelopeHeaders } from '@sentry/types'; import { sentryTest } from '../../../utils/fixtures'; import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; @@ -7,15 +7,12 @@ import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../. sentryTest('should send baggage data in transaction envelope header', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); - const pageloadTransaction = await getFirstSentryEnvelopeRequest(page, url); - expect(pageloadTransaction).toBeDefined(); - - await page.click('#start-transaction'); - const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); + const event = await getFirstSentryEnvelopeRequest(page, url); + expect(event.type).toBe('transaction'); expect(envHeader.baggage).toBeDefined(); expect(envHeader.baggage).toEqual( - 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=test-transaction,sentry-userid=user123,sentry-usersegment=segmentB', + 'sentry-environment=production,sentry-transaction=testTransactionBaggage,sentry-userid=user123,sentry-usersegment=segmentB', ); }); From 8bcf2a6d99a84dbf0393fa4e48756cf7c5ac918e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 May 2022 14:31:34 +0200 Subject: [PATCH 08/11] fix integration test in CI (hopefully) --- packages/integration-tests/suites/tracing/baggage/test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/integration-tests/suites/tracing/baggage/test.ts b/packages/integration-tests/suites/tracing/baggage/test.ts index 9847d2e54430..5617e0d57309 100644 --- a/packages/integration-tests/suites/tracing/baggage/test.ts +++ b/packages/integration-tests/suites/tracing/baggage/test.ts @@ -8,9 +8,7 @@ sentryTest('should send baggage data in transaction envelope header', async ({ g const url = await getLocalTestPath({ testDir: __dirname }); const envHeader = await getFirstSentryEnvelopeRequest(page, url, envelopeHeaderRequestParser); - const event = await getFirstSentryEnvelopeRequest(page, url); - expect(event.type).toBe('transaction'); expect(envHeader.baggage).toBeDefined(); expect(envHeader.baggage).toEqual( 'sentry-environment=production,sentry-transaction=testTransactionBaggage,sentry-userid=user123,sentry-usersegment=segmentB', From 77d03fc278376338ad4cd6a4b16f0cc92f6f921c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 18 May 2022 10:09:27 +0200 Subject: [PATCH 09/11] extract baggage emptyness check to function --- packages/core/src/envelope.ts | 3 ++- packages/utils/src/baggage.ts | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index f06c98824a4d..1f20c2e45a51 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -11,6 +11,7 @@ import { SessionEnvelope, SessionItem, } from '@sentry/types'; +import { isBaggageEmpty } from '@sentry/utils'; import { BaggageObj, createBaggage, @@ -129,7 +130,7 @@ export function createEventEnvelope( ); envelopeHeaders = { ...envelopeHeaders, - ...(Object.keys(baggage[0]).length > 0 && { baggage: serializeBaggage(baggage) }), + ...(isBaggageEmpty(baggage) && { baggage: serializeBaggage(baggage) }), }; } diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index 8c683a1aec7d..433f1180d609 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -47,6 +47,11 @@ export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: baggage[0][key] = value; } +/** Check if the baggage object (i.e. the first element in the tuple) is empty */ +export function isBaggageEmpty(baggage: Baggage): boolean { + return Object.keys(baggage[0]).length > 0; +} + /** Serialize a baggage object */ export function serializeBaggage(baggage: Baggage): string { return Object.keys(baggage[0]).reduce((prev, key: keyof BaggageObj) => { From 5739252c8f204c57ee0159ad3f1fbe5e82b65187 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 18 May 2022 11:00:16 +0200 Subject: [PATCH 10/11] extract event envelope header creation to function --- packages/core/src/envelope.ts | 51 ++++++++++++++++------------- packages/utils/src/baggage.ts | 2 +- packages/utils/test/baggage.test.ts | 18 +++++++++- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 1f20c2e45a51..884289ab602a 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -11,13 +11,14 @@ import { SessionEnvelope, SessionItem, } from '@sentry/types'; -import { isBaggageEmpty } from '@sentry/utils'; import { + Baggage, BaggageObj, createBaggage, createEnvelope, dropUndefinedKeys, dsnToString, + isBaggageEmpty, serializeBaggage, } from '@sentry/utils'; @@ -110,15 +111,27 @@ export function createEventEnvelope( // TODO: This is NOT part of the hack - DO NOT DELETE delete event.sdkProcessingMetadata; - let envelopeHeaders: EventEnvelopeHeaders = { - event_id: event.event_id as string, - sent_at: new Date().toISOString(), - ...(sdkInfo && { sdk: sdkInfo }), - ...(!!tunnel && { dsn: dsnToString(dsn) }), - }; + const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn); + + const eventItem: EventItem = [ + { + type: eventType, + sample_rates: [{ id: samplingMethod, rate: sampleRate }], + }, + event, + ]; + return createEnvelope(envelopeHeaders, [eventItem]); +} - if (eventType === 'transaction') { - const baggage = createBaggage( +function createEventEnvelopeHeaders( + event: Event, + sdkInfo: SdkInfo | undefined, + tunnel: string | undefined, + dsn: DsnComponents, +): EventEnvelopeHeaders { + const baggage = + event.type === 'transaction' && + createBaggage( dropUndefinedKeys({ environment: event.environment, release: event.release, @@ -128,18 +141,12 @@ export function createEventEnvelope( usersegment: event.user && event.user.segment, } as BaggageObj), ); - envelopeHeaders = { - ...envelopeHeaders, - ...(isBaggageEmpty(baggage) && { baggage: serializeBaggage(baggage) }), - }; - } - const eventItem: EventItem = [ - { - type: eventType, - sample_rates: [{ id: samplingMethod, rate: sampleRate }], - }, - event, - ]; - return createEnvelope(envelopeHeaders, [eventItem]); + return { + event_id: event.event_id as string, + sent_at: new Date().toISOString(), + ...(sdkInfo && { sdk: sdkInfo }), + ...(!!tunnel && { dsn: dsnToString(dsn) }), + ...(baggage && !isBaggageEmpty(baggage) && { baggage: serializeBaggage(baggage) }), + }; } diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index 433f1180d609..f8bec69439b3 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -49,7 +49,7 @@ export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: /** Check if the baggage object (i.e. the first element in the tuple) is empty */ export function isBaggageEmpty(baggage: Baggage): boolean { - return Object.keys(baggage[0]).length > 0; + return Object.keys(baggage[0]).length === 0; } /** Serialize a baggage object */ diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts index 08d4213f2a99..e1c91a87f5ee 100644 --- a/packages/utils/test/baggage.test.ts +++ b/packages/utils/test/baggage.test.ts @@ -1,4 +1,11 @@ -import { createBaggage, getBaggageValue, parseBaggageString, serializeBaggage, setBaggageValue } from '../src/baggage'; +import { + createBaggage, + getBaggageValue, + isBaggageEmpty, + parseBaggageString, + serializeBaggage, + setBaggageValue, +} from '../src/baggage'; describe('Baggage', () => { describe('createBaggage', () => { @@ -89,4 +96,13 @@ describe('Baggage', () => { expect(parseBaggageString(baggageString)).toEqual(baggage); }); }); + + describe('isBaggageEmpty', () => { + it.each([ + ['returns true if the modifyable part of baggage is empty', createBaggage({}), true], + ['returns false if the modifyable part of baggage is not empty', createBaggage({ release: '10.0.2' }), false], + ])('%s', (_: string, baggage, outcome) => { + expect(isBaggageEmpty(baggage)).toEqual(outcome); + }); + }); }); From abe18b757fd7f329a6e62effce8a662d8da05fd0 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 18 May 2022 11:17:55 +0200 Subject: [PATCH 11/11] remove unused import --- packages/core/src/envelope.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 884289ab602a..149d98d5242e 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -12,7 +12,6 @@ import { SessionItem, } from '@sentry/types'; import { - Baggage, BaggageObj, createBaggage, createEnvelope,