-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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]; | ||||||
|
||||||
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 */ | ||||||
AbhiPrasad marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
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; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: |
||||||
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; | ||||||
AbhiPrasad marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
}, baggage[1]); | ||||||
} | ||||||
|
||||||
/** Parse a baggage header to a string */ | ||||||
export function parseBaggageString(baggageString: string): Baggage { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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}`]; | ||||||
} | ||||||
}, | ||||||
[{}, ''], | ||||||
); | ||||||
} |
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
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!