From c7f0099fd9925e21f1efcccd1b4c35fc69501b1c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 10 May 2022 17:01:38 -0400 Subject: [PATCH 1/5] feat(utils): Introduce Baggage API --- packages/utils/src/baggage.ts | 54 +++++++++++++++++++++++++++++ packages/utils/test/baggage.test.ts | 25 +++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 packages/utils/src/baggage.ts create mode 100644 packages/utils/test/baggage.test.ts diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts new file mode 100644 index 000000000000..43b31d1f6234 --- /dev/null +++ b/packages/utils/src/baggage.ts @@ -0,0 +1,54 @@ +export type AllowedBaggageKeys = 'environment' | 'release'; // TODO: Add remaining allowed baggage keys | 'transaction' | 'userid' | 'usersegment'; +export type Baggage = Partial & Record>; + +export const BAGGAGE_HEADER_NAME = 'baggage'; + +/** + * 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: Baggage): Baggage { + return { ...initItems }; +} + +/** Add a value to baggage */ +export function getBaggageValue(baggage: Baggage, key: keyof Baggage): Baggage[keyof Baggage] { + return baggage[key]; +} + +/** Add a value to baggage */ +export function setBaggageValue(baggage: Baggage, key: keyof Baggage, value: Baggage[keyof Baggage]): void { + baggage[key] = value; +} + +/** remove a value from baggage */ +// TODO: Is this even useful? +// export function removeBaggageValue(baggage: Baggage, key: keyof Baggage): void { +// // eslint-disable-next-line @typescript-eslint/no-dynamic-delete +// delete baggage[key]; +// } + +/** Serialize a baggage object */ +export function serializeBaggage(baggage: Baggage): string { + return Object.keys(baggage).reduce((prev, key) => { + const newVal = `${prev};${key}=${baggage[key as keyof Baggage]}`; + return newVal.length > MAX_BAGGAGE_STRING_LENGTH ? prev : newVal; + }, ''); +} + +/** Parse a baggage header to a string */ +export function parseBaggageString(baggageString: string): Baggage { + // TODO: Should we check validity with regex? How much should we check for malformed baggage keys? + // Perhaps we shouldn't worry about this being used in the frontend, so bundle size isn't that much of a concern + return baggageString.split(';').reduce((prevBaggage, curr) => { + const [key, val] = curr.split('='); + return { + ...prevBaggage, + [key]: val, + }; + }, {} as Baggage); +} diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts new file mode 100644 index 000000000000..a625d6dd2e76 --- /dev/null +++ b/packages/utils/test/baggage.test.ts @@ -0,0 +1,25 @@ +import { createBaggage, getBaggageValue } 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) => { + expect(createBaggage(input)).toEqual(output); + }); + }); + + describe('getBaggageValue', () => { + it.each([ + ['gets a baggage item', { environment: 'production', anyKey: 'anyValue' }, 'environment', 'production'], + ['finds undefined items', {}, 'environment', undefined], + ])('%s', (_: string, baggage, key, value) => { + expect(getBaggageValue(baggage, key)).toEqual(value); + }); + }); +}); From 0a2811dba19be8275c8be32b4fd4f8ebd7d38187 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 12 May 2022 11:50:15 -0400 Subject: [PATCH 2/5] add tests --- packages/utils/src/baggage.ts | 61 +++++++++++++---------- packages/utils/test/baggage.test.ts | 77 +++++++++++++++++++++++++++-- 2 files changed, 107 insertions(+), 31 deletions(-) diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index 43b31d1f6234..3f34e2c952e6 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -1,8 +1,15 @@ export type AllowedBaggageKeys = 'environment' | 'release'; // TODO: Add remaining allowed baggage keys | 'transaction' | 'userid' | 'usersegment'; -export type Baggage = Partial & Record>; +export type BaggageObj = Partial & Record>; +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-/; + +// baggage = sentry-environment=prod;my-info=true; + /** * Max length of a serialized baggage string * @@ -11,44 +18,46 @@ export const BAGGAGE_HEADER_NAME = 'baggage'; export const MAX_BAGGAGE_STRING_LENGTH = 8192; /** Create an instance of Baggage */ -export function createBaggage(initItems: Baggage): Baggage { - return { ...initItems }; +export function createBaggage(initItems: BaggageObj, baggageString: string = ''): Baggage { + return [{ ...initItems }, baggageString]; } /** Add a value to baggage */ -export function getBaggageValue(baggage: Baggage, key: keyof Baggage): Baggage[keyof Baggage] { - return baggage[key]; +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 Baggage, value: Baggage[keyof Baggage]): void { - baggage[key] = value; +export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: BaggageObj[keyof BaggageObj]): void { + baggage[0][key] = value; } -/** remove a value from baggage */ -// TODO: Is this even useful? -// export function removeBaggageValue(baggage: Baggage, key: keyof Baggage): void { -// // eslint-disable-next-line @typescript-eslint/no-dynamic-delete -// delete baggage[key]; -// } - /** Serialize a baggage object */ export function serializeBaggage(baggage: Baggage): string { - return Object.keys(baggage).reduce((prev, key) => { - const newVal = `${prev};${key}=${baggage[key as keyof Baggage]}`; + return Object.keys(baggage[0]).reduce((prev, key) => { + const baggageEntry = `${SENTRY_BAGGAGE_KEY_PREFIX}${key}=${baggage[0][key as keyof BaggageObj]}`; + 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 { - // TODO: Should we check validity with regex? How much should we check for malformed baggage keys? - // Perhaps we shouldn't worry about this being used in the frontend, so bundle size isn't that much of a concern - return baggageString.split(';').reduce((prevBaggage, curr) => { - const [key, val] = curr.split('='); - return { - ...prevBaggage, - [key]: val, - }; - }, {} as Baggage); + return baggageString.split(',').reduce( + ([baggageObj, baggageString], curr) => { + const [key, val] = curr.split('='); + if (SENTRY_BAGGAGE_KEY_PREFIX_REGEX.test(key)) { + return [ + { + ...baggageObj, + [key.split('-')[1]]: val, + }, + baggageString, + ]; + } else { + return [baggageObj, baggageString === '' ? curr : `${baggageString},${curr}`]; + } + }, + [{}, ''], + ); } diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts index a625d6dd2e76..08d4213f2a99 100644 --- a/packages/utils/test/baggage.test.ts +++ b/packages/utils/test/baggage.test.ts @@ -1,13 +1,13 @@ -import { createBaggage, getBaggageValue } from '../src/baggage'; +import { createBaggage, getBaggageValue, parseBaggageString, serializeBaggage, setBaggageValue } from '../src/baggage'; describe('Baggage', () => { describe('createBaggage', () => { it.each([ - ['creates an empty baggage instance', {}, {}], + ['creates an empty baggage instance', {}, [{}, '']], [ 'creates a baggage instance with initial values', { environment: 'production', anyKey: 'anyValue' }, - { environment: 'production', anyKey: 'anyValue' }, + [{ environment: 'production', anyKey: 'anyValue' }, ''], ], ])('%s', (_: string, input, output) => { expect(createBaggage(input)).toEqual(output); @@ -16,10 +16,77 @@ describe('Baggage', () => { describe('getBaggageValue', () => { it.each([ - ['gets a baggage item', { environment: 'production', anyKey: 'anyValue' }, 'environment', 'production'], - ['finds undefined items', {}, 'environment', undefined], + [ + '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); + }); + }); }); From 0068cb54cfe921021ea0cdb16d0edca51be96314 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 12 May 2022 14:55:38 -0400 Subject: [PATCH 3/5] description --- packages/utils/src/baggage.ts | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index 3f34e2c952e6..bd7324f81e55 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -1,5 +1,19 @@ export type AllowedBaggageKeys = 'environment' | 'release'; // TODO: Add remaining allowed baggage keys | 'transaction' | 'userid' | 'usersegment'; export type BaggageObj = Partial & Record>; + +/** + * 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, seperating 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 propogation time. + */ export type Baggage = [BaggageObj, string]; export const BAGGAGE_HEADER_NAME = 'baggage'; @@ -8,8 +22,6 @@ export const SENTRY_BAGGAGE_KEY_PREFIX = 'sentry-'; export const SENTRY_BAGGAGE_KEY_PREFIX_REGEX = /^sentry-/; -// baggage = sentry-environment=prod;my-info=true; - /** * Max length of a serialized baggage string * @@ -35,7 +47,8 @@ export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: /** Serialize a baggage object */ export function serializeBaggage(baggage: Baggage): string { return Object.keys(baggage[0]).reduce((prev, key) => { - const baggageEntry = `${SENTRY_BAGGAGE_KEY_PREFIX}${key}=${baggage[0][key as keyof BaggageObj]}`; + const val = baggage[0][key as keyof BaggageObj] as string; + 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]); @@ -47,10 +60,11 @@ export function parseBaggageString(baggageString: string): Baggage { ([baggageObj, baggageString], curr) => { const [key, val] = curr.split('='); if (SENTRY_BAGGAGE_KEY_PREFIX_REGEX.test(key)) { + const baggageKey = decodeURIComponent(key.split('-')[1]); return [ { ...baggageObj, - [key.split('-')[1]]: val, + [baggageKey]: decodeURIComponent(val), }, baggageString, ]; From 8310ca752dd008a8ef57eca11070e39dcb7028ac Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 12 May 2022 14:58:23 -0400 Subject: [PATCH 4/5] spelling --- packages/utils/src/baggage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index bd7324f81e55..94ecc75db55b 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -8,11 +8,11 @@ export type BaggageObj = Partial & Record Date: Mon, 16 May 2022 10:48:37 -0400 Subject: [PATCH 5/5] address PR review --- packages/utils/src/baggage.ts | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index 94ecc75db55b..db402608a22b 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -1,3 +1,6 @@ +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 BaggageObj = Partial & Record>; @@ -34,7 +37,7 @@ export function createBaggage(initItems: BaggageObj, baggageString: string = '') return [{ ...initItems }, baggageString]; } -/** Add a value to baggage */ +/** Get a value from baggage */ export function getBaggageValue(baggage: Baggage, key: keyof BaggageObj): BaggageObj[keyof BaggageObj] { return baggage[0][key]; } @@ -46,17 +49,23 @@ export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, 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; + return Object.keys(baggage[0]).reduce((prev, key: keyof BaggageObj) => { + const val = baggage[0][key] as string; 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; + if (newVal.length > MAX_BAGGAGE_STRING_LENGTH) { + IS_DEBUG_BUILD && + logger.warn(`Not adding key: ${key} with val: ${val} to baggage due to exceeding baggage size limits.`); + return prev; + } else { + return newVal; + } }, baggage[1]); } /** Parse a baggage header to a string */ -export function parseBaggageString(baggageString: string): Baggage { - return baggageString.split(',').reduce( +export function parseBaggageString(inputBaggageString: string): Baggage { + return inputBaggageString.split(',').reduce( ([baggageObj, baggageString], curr) => { const [key, val] = curr.split('='); if (SENTRY_BAGGAGE_KEY_PREFIX_REGEX.test(key)) {