Skip to content

feat(core): Send Baggage in Envelope Header #5104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 18, 2022
2 changes: 1 addition & 1 deletion packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
throw new SentryError('`beforeSend` returned `null`, will not send event.');
}

const session = scope && scope.getSession && scope.getSession();
const session = scope && scope.getSession();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't have anything to do with baggage but it's a no longer necessary check I stumbled across. See this conversation for context

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, why not use scope?.getSession() notation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question!

It's because we try to optimize our codebase around targeting es6. Optional Chaining is a newer JS feature, ES2020 - "ES11" as per: https://gist.github.com/rajaramtt/7df3702a04c644b0b62c9a64f48f3dbf#64-optional-chaining. This means that it'll have to be down-compiled to es6 compatible code when we generate the final assets to distribute on npm and the CDN (this is for max browser + node version support).

Something like so:

if (hey?.me) {
 console.log('me'); 
}

ends up getting down-compiled to:

if (hey !== null && hey !== void 0 && hey.me) {
  console.log('me');
}

See the babel demo.

which is way more bytes, which is bad for bundle size.

We made a decision as a result to not use any optional chaining in code that might be used in browser environments. See some examples on bundle wins we made here: https://github.com/getsentry/sentry-javascript/pulls?q=is%3Apr+optional+chaining+is%3Aclosed+milestone%3A%22Tree+shaking+%2F+Bundle+Size%22

if (!isTransaction && session) {
this._updateSessionFromEvent(session, processedEvent);
}
Expand Down
47 changes: 40 additions & 7 deletions packages/core/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
DsnComponents,
Event,
EventEnvelope,
EventEnvelopeHeaders,
EventItem,
SdkInfo,
SdkMetadata,
Expand All @@ -10,7 +11,15 @@ import {
SessionEnvelope,
SessionItem,
} from '@sentry/types';
import { createEnvelope, dsnToString } from '@sentry/utils';
import {
BaggageObj,
createBaggage,
createEnvelope,
dropUndefinedKeys,
dsnToString,
isBaggageEmpty,
serializeBaggage,
} from '@sentry/utils';

/** Extract sdk info from from the API metadata */
function getSdkMetadataForEnvelopeHeader(metadata?: SdkMetadata): SdkInfo | undefined {
Expand Down Expand Up @@ -101,12 +110,8 @@ export function createEventEnvelope(
// TODO: This is NOT part of the hack - DO NOT DELETE
delete event.sdkProcessingMetadata;

const envelopeHeaders = {
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,
Expand All @@ -116,3 +121,31 @@ export function createEventEnvelope(
];
return createEnvelope<EventEnvelope>(envelopeHeaders, [eventItem]);
}

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,
transaction: event.transaction,
userid: event.user && event.user.id,
// user.segment currently doesn't exist explicitly in interface User (just as a record key)
usersegment: event.user && event.user.segment,
} as BaggageObj),
);

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) }),
};
}
54 changes: 54 additions & 0 deletions packages/core/test/lib/envelope.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
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", () => {
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);
});
});
});
16 changes: 16 additions & 0 deletions packages/integration-tests/suites/tracing/baggage/init.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
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,
});

Sentry.configureScope(scope => {
scope.setUser({ id: 'user123', segment: 'segmentB' });
scope.setTransactionName('testTransactionBaggage');
});
16 changes: 16 additions & 0 deletions packages/integration-tests/suites/tracing/baggage/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { expect } from '@playwright/test';
import { Event, 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 envHeader = await getFirstSentryEnvelopeRequest<EventEnvelopeHeaders>(page, url, envelopeHeaderRequestParser);

expect(envHeader.baggage).toBeDefined();
expect(envHeader.baggage).toEqual(
'sentry-environment=production,sentry-transaction=testTransactionBaggage,sentry-userid=user123,sentry-usersegment=segmentB',
);
});
21 changes: 17 additions & 4 deletions packages/integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
@@ -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\//;

Expand All @@ -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.
*
Expand Down Expand Up @@ -122,10 +130,11 @@ async function getMultipleSentryEnvelopeRequests<T>(
url?: string;
timeout?: number;
},
requestParser: (req: Request) => T = envelopeRequestParser as (req: Request) => T,
): Promise<T[]> {
// 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<T[]>;
return getMultipleRequests(page, count, envelopeUrlRegex, requestParser, options) as Promise<T[]>;
}

/**
Expand All @@ -136,8 +145,12 @@ async function getMultipleSentryEnvelopeRequests<T>(
* @param {string} [url]
* @return {*} {Promise<T>}
*/
async function getFirstSentryEnvelopeRequest<T>(page: Page, url?: string): Promise<T> {
return (await getMultipleSentryEnvelopeRequests<T>(page, 1, { url }))[0];
async function getFirstSentryEnvelopeRequest<T>(
page: Page,
url?: string,
requestParser: (req: Request) => T = envelopeRequestParser as (req: Request) => T,
): Promise<T> {
return (await getMultipleSentryEnvelopeRequests<T>(page, 1, { url }, requestParser))[0];
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export type SessionItem =
| BaseEnvelopeItem<SessionAggregatesItemHeaders, SessionAggregates>;
export type ClientReportItem = BaseEnvelopeItem<ClientReportItemHeaders, ClientReport>;

type EventEnvelopeHeaders = { event_id: string; sent_at: string };
export type EventEnvelopeHeaders = { event_id: string; sent_at: string; baggage?: string };
type SessionEnvelopeHeaders = { sent_at: string };
type ClientReportEnvelopeHeaders = BaseEnvelopeHeaders;

Expand Down
1 change: 1 addition & 0 deletions packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export type {
EnvelopeItemType,
EnvelopeItem,
EventEnvelope,
EventEnvelopeHeaders,
EventItem,
SessionEnvelope,
SessionItem,
Expand Down
7 changes: 6 additions & 1 deletion packages/utils/src/baggage.ts
Original file line number Diff line number Diff line change
@@ -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<AllowedBaggageKeys, string> & Record<string, string>>;

/**
Expand Down Expand Up @@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions packages/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ export * from './env';
export * from './envelope';
export * from './clientreport';
export * from './ratelimit';
export * from './baggage';
18 changes: 17 additions & 1 deletion packages/utils/test/baggage.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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);
});
});
});