Skip to content

feat(utils): Introduce Baggage API #5066

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 5 commits into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions packages/utils/src/baggage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
export type AllowedBaggageKeys = 'environment' | 'release'; // TODO: Add remaining allowed baggage keys | 'transaction' | 'userid' | 'usersegment';
export type BaggageObj = Partial<Record<AllowedBaggageKeys, string> & Record<string, string>>;

/**
* The baggage data structure represents key,value pairs based on the baggage
* spec: https://www.w3.org/TR/baggage
*
* It is expected that users interact with baggage using the helpers methods:
* `createBaggage`, `getBaggageValue`, and `setBaggageValue`.
*
* Internally, the baggage data structure is a tuple of length 2, separating baggage values
* based on if they are related to Sentry or not. If the baggage values are
* set/used by sentry, they will be stored in an object to be easily accessed.
* If they are not, they are kept as a string to be only accessed when serialized
* at baggage propagation time.
*/
export type Baggage = [BaggageObj, string];
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion the readability of all of the functions in this file (and also their type definitions) suffer a lot from this being a tuple. I'd much rather have an interface with descriptive keys. But feel free to overrule me on this one!


export const BAGGAGE_HEADER_NAME = 'baggage';

export const SENTRY_BAGGAGE_KEY_PREFIX = 'sentry-';

export const SENTRY_BAGGAGE_KEY_PREFIX_REGEX = /^sentry-/;

/**
* Max length of a serialized baggage string
*
* https://www.w3.org/TR/baggage/#limits
*/
export const MAX_BAGGAGE_STRING_LENGTH = 8192;

/** Create an instance of Baggage */
export function createBaggage(initItems: BaggageObj, baggageString: string = ''): Baggage {
return [{ ...initItems }, baggageString];
}

/** Add a value to baggage */
export function getBaggageValue(baggage: Baggage, key: keyof BaggageObj): BaggageObj[keyof BaggageObj] {
return baggage[0][key];
}

/** Add a value to baggage */
export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: BaggageObj[keyof BaggageObj]): void {
baggage[0][key] = value;
}

/** Serialize a baggage object */
export function serializeBaggage(baggage: Baggage): string {
return Object.keys(baggage[0]).reduce((prev, key) => {
const val = baggage[0][key as keyof BaggageObj] as string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: as keyof BaggageObj is not needed here

const baggageEntry = `${SENTRY_BAGGAGE_KEY_PREFIX}${encodeURIComponent(key)}=${encodeURIComponent(val)}`;
const newVal = prev === '' ? baggageEntry : `${prev},${baggageEntry}`;
return newVal.length > MAX_BAGGAGE_STRING_LENGTH ? prev : newVal;
}, baggage[1]);
}

/** Parse a baggage header to a string */
export function parseBaggageString(baggageString: string): Baggage {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export function parseBaggageString(baggageString: string): Baggage {
export function parseBaggageString(inputBaggageString: string): Baggage {

Nit: Can be anything but there bagageString is in the same scope twice.

return baggageString.split(',').reduce(
([baggageObj, baggageString], curr) => {
const [key, val] = curr.split('=');
if (SENTRY_BAGGAGE_KEY_PREFIX_REGEX.test(key)) {
const baggageKey = decodeURIComponent(key.split('-')[1]);
return [
{
...baggageObj,
[baggageKey]: decodeURIComponent(val),
},
baggageString,
];
} else {
return [baggageObj, baggageString === '' ? curr : `${baggageString},${curr}`];
}
},
[{}, ''],
);
}
92 changes: 92 additions & 0 deletions packages/utils/test/baggage.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { createBaggage, getBaggageValue, parseBaggageString, serializeBaggage, setBaggageValue } from '../src/baggage';

describe('Baggage', () => {
describe('createBaggage', () => {
it.each([
['creates an empty baggage instance', {}, [{}, '']],
[
'creates a baggage instance with initial values',
{ environment: 'production', anyKey: 'anyValue' },
[{ environment: 'production', anyKey: 'anyValue' }, ''],
],
])('%s', (_: string, input, output) => {
Comment on lines +5 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very high up on the cleverness scale 😄 What do you think about splitting this up into separate tests for readability?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a big fan of table driven tests, and I believe they lend themselves well to maintainable and extensible unit tests. Picked it up from the golang ecosystem: https://dave.cheney.net/2019/05/07/prefer-table-driven-tests.

It removes the boilerplate, and makes it easy to understand the that the setup + usage of the function is the exact same over the various scenarios. If the setup ever needs to change, just create a new test block!

expect(createBaggage(input)).toEqual(output);
});
});

describe('getBaggageValue', () => {
it.each([
[
'gets a baggage item',
createBaggage({ environment: 'production', anyKey: 'anyValue' }),
'environment',
'production',
],
['finds undefined items', createBaggage({}), 'environment', undefined],
])('%s', (_: string, baggage, key, value) => {
expect(getBaggageValue(baggage, key)).toEqual(value);
});
});

describe('setBaggageValue', () => {
it.each([
['sets a baggage item', createBaggage({}), 'environment', 'production'],
['overwrites a baggage item', createBaggage({ environment: 'development' }), 'environment', 'production'],
])('%s', (_: string, baggage, key, value) => {
setBaggageValue(baggage, key, value);
expect(getBaggageValue(baggage, key)).toEqual(value);
});
});

describe('serializeBaggage', () => {
it.each([
['serializes empty baggage', createBaggage({}), ''],
[
'serializes baggage with a single value',
createBaggage({ environment: 'production' }),
'sentry-environment=production',
],
[
'serializes baggage with multiple values',
createBaggage({ environment: 'production', release: '10.0.2' }),
'sentry-environment=production,sentry-release=10.0.2',
],
[
'keeps non-sentry prefixed baggage items',
createBaggage(
{ environment: 'production', release: '10.0.2' },
'userId=alice,serverNode=DF%2028,isProduction=false',
),
'userId=alice,serverNode=DF%2028,isProduction=false,sentry-environment=production,sentry-release=10.0.2',
],
[
'can only use non-sentry prefixed baggage items',
createBaggage({}, 'userId=alice,serverNode=DF%2028,isProduction=false'),
'userId=alice,serverNode=DF%2028,isProduction=false',
],
])('%s', (_: string, baggage, serializedBaggage) => {
expect(serializeBaggage(baggage)).toEqual(serializedBaggage);
});
});

describe('parseBaggageString', () => {
it.each([
['parses an empty string', '', createBaggage({})],
[
'parses sentry values into baggage',
'sentry-environment=production,sentry-release=10.0.2',
createBaggage({ environment: 'production', release: '10.0.2' }),
],
[
'parses arbitrary baggage headers',
'userId=alice,serverNode=DF%2028,isProduction=false,sentry-environment=production,sentry-release=10.0.2',
createBaggage(
{ environment: 'production', release: '10.0.2' },
'userId=alice,serverNode=DF%2028,isProduction=false',
),
],
])('%s', (_: string, baggageString, baggage) => {
expect(parseBaggageString(baggageString)).toEqual(baggage);
});
});
});