From 83343ef741d8a6ea444c07e797643d6928623be7 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 5 Feb 2021 09:31:07 -0500 Subject: [PATCH] refactor!: specification compliant baggage (#1876) --- .../opentelemetry-api/src/baggage/Baggage.ts | 50 +++++- .../src/baggage/{EntryValue.ts => Entry.ts} | 31 ++-- .../opentelemetry-api/src/baggage/index.ts | 56 ++++++ .../src/baggage/internal/baggage.ts | 63 +++++++ .../src/baggage/internal/symbol.ts | 20 +++ packages/opentelemetry-api/src/index.ts | 3 +- .../test/baggage/Baggage.test.ts | 161 ++++++++++++++++++ .../src/baggage/propagation/HttpBaggage.ts | 39 +++-- .../test/baggage/HttpBaggage.test.ts | 155 ++++++++++++----- .../src/shim.ts | 16 +- .../test/Shim.test.ts | 8 +- 11 files changed, 508 insertions(+), 94 deletions(-) rename packages/opentelemetry-api/src/baggage/{EntryValue.ts => Entry.ts} (51%) create mode 100644 packages/opentelemetry-api/src/baggage/index.ts create mode 100644 packages/opentelemetry-api/src/baggage/internal/baggage.ts create mode 100644 packages/opentelemetry-api/src/baggage/internal/symbol.ts create mode 100644 packages/opentelemetry-api/test/baggage/Baggage.test.ts diff --git a/packages/opentelemetry-api/src/baggage/Baggage.ts b/packages/opentelemetry-api/src/baggage/Baggage.ts index f14371e777..2876f5bd47 100644 --- a/packages/opentelemetry-api/src/baggage/Baggage.ts +++ b/packages/opentelemetry-api/src/baggage/Baggage.ts @@ -14,16 +14,50 @@ * limitations under the License. */ -import { BaggageEntryValue } from './EntryValue'; +import { BaggageEntry } from './Entry'; /** - * Baggage represents collection of entries. Each key of - * Baggage is associated with exactly one value. Baggage - * is serializable, to facilitate propagating it not only inside the process - * but also across process boundaries. Baggage is used to annotate - * telemetry with the name:value pair Entry. Those values can be used to add - * dimension to the metric or additional contest properties to logs and traces. + * Baggage represents collection of key-value pairs with optional metadata. + * Each key of Baggage is associated with exactly one value. + * Baggage may be used to annotate and enrich telemetry data. */ export interface Baggage { - [entryKey: string]: BaggageEntryValue; + /** + * Get an entry from Baggage if it exists + * + * @param key The key which identifies the BaggageEntry + */ + getEntry(key: string): BaggageEntry | undefined; + + /** + * Get a list of all entries in the Baggage + */ + getAllEntries(): [string, BaggageEntry][]; + + /** + * Returns a new baggage with the entries from the current bag and the specified entry + * + * @param key string which identifies the baggage entry + * @param entry BaggageEntry for the given key + */ + setEntry(key: string, entry: BaggageEntry): Baggage; + + /** + * Returns a new baggage with the entries from the current bag except the removed entry + * + * @param key key identifying the entry to be removed + */ + removeEntry(key: string): Baggage; + + /** + * Returns a new baggage with the entries from the current bag except the removed entries + * + * @param key keys identifying the entries to be removed + */ + removeEntries(...key: string[]): Baggage; + + /** + * Returns a new baggage with no entries + */ + clear(): Baggage; } diff --git a/packages/opentelemetry-api/src/baggage/EntryValue.ts b/packages/opentelemetry-api/src/baggage/Entry.ts similarity index 51% rename from packages/opentelemetry-api/src/baggage/EntryValue.ts rename to packages/opentelemetry-api/src/baggage/Entry.ts index bafc7c37b0..d249c85cec 100644 --- a/packages/opentelemetry-api/src/baggage/EntryValue.ts +++ b/packages/opentelemetry-api/src/baggage/Entry.ts @@ -13,28 +13,23 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export interface BaggageEntryValue { - /** `String` value of the `BaggageEntryValue`. */ + +import { baggageEntryMetadataSymbol } from './internal/symbol'; + +export interface BaggageEntry { + /** `String` value of the `BaggageEntry`. */ value: string; /** - * ttl is an integer that represents number of hops an entry can - * propagate. + * Metadata is an optional string property defined by the W3C baggage specification. + * It currently has no special meaning defined by the specification. */ - ttl?: BaggageEntryTtl; + metadata?: BaggageEntryMetadata; } /** - * BaggageEntryTtl is an integer that represents number of hops an entry can propagate. - * - * For now, ONLY special values (0 and -1) are supported. + * Serializable Metadata defined by the W3C baggage specification. + * It currently has no special meaning defined by the OpenTelemetry or W3C. */ -export enum BaggageEntryTtl { - /** - * NO_PROPAGATION is considered to have local context and is used within the - * process it created. - */ - NO_PROPAGATION = 0, - - /** UNLIMITED_PROPAGATION can propagate unlimited hops. */ - UNLIMITED_PROPAGATION = -1, -} +export type BaggageEntryMetadata = { toString(): string } & { + __TYPE__: typeof baggageEntryMetadataSymbol; +}; diff --git a/packages/opentelemetry-api/src/baggage/index.ts b/packages/opentelemetry-api/src/baggage/index.ts new file mode 100644 index 0000000000..ba0a297c5d --- /dev/null +++ b/packages/opentelemetry-api/src/baggage/index.ts @@ -0,0 +1,56 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Baggage } from './Baggage'; +import { BaggageEntry, BaggageEntryMetadata } from './Entry'; +import { BaggageImpl } from './internal/baggage'; +import { baggageEntryMetadataSymbol } from './internal/symbol'; + +export * from './Baggage'; +export * from './Entry'; + +/** + * Create a new Baggage with optional entries + * + * @param entries An array of baggage entries the new baggage should contain + */ +export function createBaggage( + entries: Record = {} +): Baggage { + return new BaggageImpl(new Map(Object.entries(entries))); +} + +/** + * Create a serializable BaggageEntryMetadata object from a string. + * + * @param str string metadata. Format is currently not defined by the spec and has no special meaning. + * + */ +export function baggageEntryMetadataFromString( + str: string +): BaggageEntryMetadata { + if (typeof str !== 'string') { + // @TODO log diagnostic + str = ''; + } + + return { + __TYPE__: baggageEntryMetadataSymbol, + toString() { + return str; + }, + }; +} diff --git a/packages/opentelemetry-api/src/baggage/internal/baggage.ts b/packages/opentelemetry-api/src/baggage/internal/baggage.ts new file mode 100644 index 0000000000..f9331f411a --- /dev/null +++ b/packages/opentelemetry-api/src/baggage/internal/baggage.ts @@ -0,0 +1,63 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Baggage } from '../Baggage'; +import type { BaggageEntry } from '../Entry'; + +export class BaggageImpl implements Baggage { + private _entries: Map; + + constructor(entries?: Map) { + this._entries = entries ? new Map(entries) : new Map(); + } + + getEntry(key: string): BaggageEntry | undefined { + const entry = this._entries.get(key); + if (!entry) { + return undefined; + } + + return Object.assign({}, entry); + } + + getAllEntries(): [string, BaggageEntry][] { + return Array.from(this._entries.entries()).map(([k, v]) => [k, v]); + } + + setEntry(key: string, entry: BaggageEntry): BaggageImpl { + const newBaggage = new BaggageImpl(this._entries); + newBaggage._entries.set(key, entry); + return newBaggage; + } + + removeEntry(key: string): BaggageImpl { + const newBaggage = new BaggageImpl(this._entries); + newBaggage._entries.delete(key); + return newBaggage; + } + + removeEntries(...keys: string[]): BaggageImpl { + const newBaggage = new BaggageImpl(this._entries); + for (const key of keys) { + newBaggage._entries.delete(key); + } + return newBaggage; + } + + clear(): BaggageImpl { + return new BaggageImpl(); + } +} diff --git a/packages/opentelemetry-api/src/baggage/internal/symbol.ts b/packages/opentelemetry-api/src/baggage/internal/symbol.ts new file mode 100644 index 0000000000..f4213926c9 --- /dev/null +++ b/packages/opentelemetry-api/src/baggage/internal/symbol.ts @@ -0,0 +1,20 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Symbol used to make BaggageEntryMetadata an opaque type + */ +export const baggageEntryMetadataSymbol = Symbol('BaggageEntryMetadata'); diff --git a/packages/opentelemetry-api/src/index.ts b/packages/opentelemetry-api/src/index.ts index f1dcf8211a..4c6b64ffd6 100644 --- a/packages/opentelemetry-api/src/index.ts +++ b/packages/opentelemetry-api/src/index.ts @@ -20,8 +20,7 @@ export * from './common/Time'; export * from './context/context'; export * from './context/propagation/TextMapPropagator'; export * from './context/propagation/NoopTextMapPropagator'; -export * from './baggage/Baggage'; -export * from './baggage/EntryValue'; +export * from './baggage'; export * from './trace/attributes'; export * from './trace/Event'; export * from './trace/link_context'; diff --git a/packages/opentelemetry-api/test/baggage/Baggage.test.ts b/packages/opentelemetry-api/test/baggage/Baggage.test.ts new file mode 100644 index 0000000000..45eb59d7b0 --- /dev/null +++ b/packages/opentelemetry-api/test/baggage/Baggage.test.ts @@ -0,0 +1,161 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert'; +import { + createBaggage, + setBaggage, + getBaggage, + ROOT_CONTEXT, + baggageEntryMetadataFromString, +} from '../../src'; + +describe('Baggage', () => { + describe('create', () => { + it('should create an empty bag', () => { + const bag = createBaggage(); + + assert.deepStrictEqual(bag.getAllEntries(), []); + }); + + it('should create a bag with entries', () => { + const meta = baggageEntryMetadataFromString('opaque string'); + const bag = createBaggage({ + key1: { value: 'value1' }, + key2: { value: 'value2', metadata: meta }, + }); + + assert.deepStrictEqual(bag.getAllEntries(), [ + ['key1', { value: 'value1' }], + ['key2', { value: 'value2', metadata: meta }], + ]); + }); + }); + + describe('get', () => { + it('should not allow modification of returned entries', () => { + const bag = createBaggage().setEntry('key', { value: 'value' }); + + const entry = bag.getEntry('key'); + assert.ok(entry); + entry.value = 'mutated'; + + assert.strictEqual(bag.getEntry('key')?.value, 'value'); + }); + }); + + describe('set', () => { + it('should create a new bag when an entry is added', () => { + const bag = createBaggage(); + + const bag2 = bag.setEntry('key', { value: 'value' }); + + assert.notStrictEqual(bag, bag2); + assert.deepStrictEqual(bag.getAllEntries(), []); + assert.deepStrictEqual(bag2.getAllEntries(), [ + ['key', { value: 'value' }], + ]); + }); + }); + + describe('remove', () => { + it('should create a new bag when an entry is removed', () => { + const bag = createBaggage({ + key: { value: 'value' }, + }); + + const bag2 = bag.removeEntry('key'); + + assert.deepStrictEqual(bag.getAllEntries(), [ + ['key', { value: 'value' }], + ]); + + assert.deepStrictEqual(bag2.getAllEntries(), []); + }); + + it('should create an empty bag multiple keys are removed', () => { + const bag = createBaggage({ + key: { value: 'value' }, + key1: { value: 'value1' }, + key2: { value: 'value2' }, + }); + + const bag2 = bag.removeEntries('key', 'key1'); + + assert.deepStrictEqual(bag.getAllEntries(), [ + ['key', { value: 'value' }], + ['key1', { value: 'value1' }], + ['key2', { value: 'value2' }], + ]); + + assert.deepStrictEqual(bag2.getAllEntries(), [ + ['key2', { value: 'value2' }], + ]); + }); + + it('should create an empty bag when it cleared', () => { + const bag = createBaggage({ + key: { value: 'value' }, + key1: { value: 'value1' }, + }); + + const bag2 = bag.clear(); + + assert.deepStrictEqual(bag.getAllEntries(), [ + ['key', { value: 'value' }], + ['key1', { value: 'value1' }], + ]); + + assert.deepStrictEqual(bag2.getAllEntries(), []); + }); + }); + + describe('context', () => { + it('should set and get a baggage from a context', () => { + const bag = createBaggage(); + + const ctx = setBaggage(ROOT_CONTEXT, bag); + + assert.strictEqual(bag, getBaggage(ctx)); + }); + }); + + describe('metadata', () => { + it('should create an opaque object which returns the string unchanged', () => { + const meta = baggageEntryMetadataFromString('this is a string'); + + assert.strictEqual(meta.toString(), 'this is a string'); + }); + + it('should return an empty string if input is invalid', () => { + //@ts-expect-error only accepts string values + const meta = baggageEntryMetadataFromString(1); + + assert.strictEqual(meta.toString(), ''); + }); + + it('should retain metadata', () => { + const bag = createBaggage({ + key: { + value: 'value', + metadata: baggageEntryMetadataFromString('meta'), + }, + }); + + assert.deepStrictEqual(bag.getEntry('key')?.metadata?.toString(), 'meta'); + }); + }); +}); diff --git a/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts b/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts index a576d5224d..8999af8245 100644 --- a/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts +++ b/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts @@ -17,11 +17,14 @@ import { Baggage, Context, + BaggageEntry, getBaggage, setBaggage, TextMapGetter, TextMapPropagator, TextMapSetter, + createBaggage, + baggageEntryMetadataFromString, } from '@opentelemetry/api'; const KEY_PAIR_SEPARATOR = '='; @@ -36,10 +39,6 @@ export const MAX_NAME_VALUE_PAIRS = 180; export const MAX_PER_NAME_VALUE_PAIRS = 4096; // Maximum total length of all name-value pairs allowed by w3c spec export const MAX_TOTAL_LENGTH = 8192; -type KeyPair = { - key: string; - value: string; -}; /** * Propagates {@link Baggage} through Context format propagation. @@ -70,16 +69,18 @@ export class HttpBaggage implements TextMapPropagator { } private _getKeyPairs(baggage: Baggage): string[] { - return Object.keys(baggage).map( - (key: string) => - `${encodeURIComponent(key)}=${encodeURIComponent(baggage[key].value)}` - ); + return baggage + .getAllEntries() + .map( + ([key, value]) => + `${encodeURIComponent(key)}=${encodeURIComponent(value.value)}` + ); } extract(context: Context, carrier: unknown, getter: TextMapGetter): Context { const headerValue: string = getter.get(carrier, BAGGAGE_HEADER) as string; if (!headerValue) return context; - const baggage: Baggage = {}; + const baggage: Record = {}; if (headerValue.length == 0) { return context; } @@ -87,16 +88,20 @@ export class HttpBaggage implements TextMapPropagator { pairs.forEach(entry => { const keyPair = this._parsePairKeyValue(entry); if (keyPair) { - baggage[keyPair.key] = { value: keyPair.value }; + const entry: BaggageEntry = { value: keyPair.value }; + if (keyPair.metadata) { + entry.metadata = keyPair.metadata; + } + baggage[keyPair.key] = entry; } }); if (Object.entries(baggage).length === 0) { return context; } - return setBaggage(context, baggage); + return setBaggage(context, createBaggage(baggage)); } - private _parsePairKeyValue(entry: string): KeyPair | undefined { + private _parsePairKeyValue(entry: string) { const valueProps = entry.split(PROPERTIES_SEPARATOR); if (valueProps.length <= 0) return; const keyPairPart = valueProps.shift(); @@ -104,12 +109,14 @@ export class HttpBaggage implements TextMapPropagator { const keyPair = keyPairPart.split(KEY_PAIR_SEPARATOR); if (keyPair.length != 2) return; const key = decodeURIComponent(keyPair[0].trim()); - let value = decodeURIComponent(keyPair[1].trim()); + const value = decodeURIComponent(keyPair[1].trim()); + let metadata; if (valueProps.length > 0) { - value = - value + PROPERTIES_SEPARATOR + valueProps.join(PROPERTIES_SEPARATOR); + metadata = baggageEntryMetadataFromString( + valueProps.join(PROPERTIES_SEPARATOR) + ); } - return { key, value }; + return { key, value, metadata }; } fields(): string[] { diff --git a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts index 3d1ff9b33e..e7231cac22 100644 --- a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts +++ b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts @@ -15,18 +15,19 @@ */ import { + Baggage, + BaggageEntry, + createBaggage, defaultTextMapGetter, defaultTextMapSetter, - Baggage, - setBaggage, getBaggage, + setBaggage, } from '@opentelemetry/api'; import { ROOT_CONTEXT } from '@opentelemetry/context-base'; import * as assert from 'assert'; import { - HttpBaggage, BAGGAGE_HEADER, - MAX_PER_NAME_VALUE_PAIRS, + HttpBaggage, } from '../../src/baggage/propagation/HttpBaggage'; describe('HttpBaggage', () => { @@ -40,11 +41,11 @@ describe('HttpBaggage', () => { describe('.inject()', () => { it('should set baggage header', () => { - const baggage: Baggage = { + const baggage = createBaggage({ key1: { value: 'd4cda95b652f4a1592b449d5929fda1b' }, key3: { value: 'c88815a7-0fa9-4d95-a1f1-cdccce3c5c2a' }, 'with/slash': { value: 'with spaces' }, - }; + }); httpTraceContext.inject( setBaggage(ROOT_CONTEXT, baggage), @@ -58,14 +59,10 @@ describe('HttpBaggage', () => { }); it('should skip long key-value pairs', () => { - const baggage: Baggage = { + const baggage = createBaggage({ key1: { value: 'd4cda95b' }, key3: { value: 'c88815a7' }, - }; - - // Generate long value 2*MAX_PER_NAME_VALUE_PAIRS - const value = '1a'.repeat(MAX_PER_NAME_VALUE_PAIRS); - baggage['longPair'] = { value }; + }); httpTraceContext.inject( setBaggage(ROOT_CONTEXT, baggage), @@ -78,32 +75,104 @@ describe('HttpBaggage', () => { ); }); - it('should skip all keys that surpassed the max limit of the header', () => { - const baggage: Baggage = {}; + it('should skip all entries whose length exceeds the W3C standard limit of 4096 bytes', () => { + const longKey = Array(96).fill('k').join(''); + const shortKey = Array(95).fill('k').join(''); + const value = Array(4000).fill('v').join(''); - const zeroPad = (num: number, places: number) => - String(num).padStart(places, '0'); + let baggage = createBaggage({ + aa: { value: 'shortvalue' }, + [shortKey]: { value: value }, + }); - // key=value with same size , 1024 => 8 keys - for (let i = 0; i < 9; ++i) { - const index = zeroPad(i, 510); - baggage[`k${index}`] = { value: `${index}` }; - } + httpTraceContext.inject( + setBaggage(ROOT_CONTEXT, baggage), + carrier, + defaultTextMapSetter + ); - // Build expected - let expected = ''; - for (let i = 0; i < 8; ++i) { - const index = zeroPad(i, 510); - expected += `k${index}=${index},`; - } - expected = expected.slice(0, -1); + let header = carrier[BAGGAGE_HEADER]; + assert.ok(typeof header === 'string'); + assert.deepStrictEqual(header, `aa=shortvalue,${shortKey}=${value}`); + + baggage = createBaggage({ + aa: { value: 'shortvalue' }, + [longKey]: { value: value }, + }); + + carrier = {}; + httpTraceContext.inject( + setBaggage(ROOT_CONTEXT, baggage), + carrier, + defaultTextMapSetter + ); + + header = carrier[BAGGAGE_HEADER]; + assert.ok(typeof header === 'string'); + assert.deepStrictEqual(header, 'aa=shortvalue'); + }); + + it('should not exceed the W3C standard header length limit of 8192 bytes', () => { + const longKey0 = Array(48).fill('0').join(''); + const longKey1 = Array(49).fill('1').join(''); + const longValue = Array(4000).fill('v').join(''); + + let baggage = createBaggage({ + [longKey0]: { value: longValue }, + [longKey1]: { value: longValue }, + aa: { value: Array(88).fill('v').join('') }, + }); + + httpTraceContext.inject( + setBaggage(ROOT_CONTEXT, baggage), + carrier, + defaultTextMapSetter + ); + + let header = carrier[BAGGAGE_HEADER]; + assert.ok(typeof header === 'string'); + assert.deepStrictEqual(header.length, 8192); + assert.deepStrictEqual(header.split(',').length, 3); + + baggage = createBaggage({ + [longKey0]: { value: longValue }, + [longKey1]: { value: longValue }, + aa: { value: Array(89).fill('v').join('') }, + }); + + carrier = {}; + httpTraceContext.inject( + setBaggage(ROOT_CONTEXT, baggage), + carrier, + defaultTextMapSetter + ); + + header = carrier[BAGGAGE_HEADER]; + assert.ok(typeof header === 'string'); + assert.deepStrictEqual(header.length, 8100); + assert.deepStrictEqual(header.split(',').length, 2); + }); + + it('should not exceed the W3C standard header entry limit of 180 entries', () => { + const entries: Record = {}; + + Array(200) + .fill(0) + .forEach((_, i) => { + entries[`${i}`] = { value: 'v' }; + }); + + const baggage = createBaggage(entries); httpTraceContext.inject( setBaggage(ROOT_CONTEXT, baggage), carrier, defaultTextMapSetter ); - assert.deepStrictEqual(carrier[BAGGAGE_HEADER], expected); + + const header = carrier[BAGGAGE_HEADER]; + assert.ok(typeof header === 'string'); + assert.strictEqual(header.split(',').length, 180); }); }); @@ -115,12 +184,12 @@ describe('HttpBaggage', () => { httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) ); - const expected: Baggage = { + const expected = createBaggage({ key1: { value: 'd4cda95b' }, key3: { value: 'c88815a7' }, keyn: { value: 'valn' }, keym: { value: 'valm' }, - }; + }); assert.deepStrictEqual(extractedBaggage, expected); }); }); @@ -143,15 +212,19 @@ describe('HttpBaggage', () => { it('returns keys with their properties', () => { carrier[BAGGAGE_HEADER] = 'key1=d4cda95b,key3=c88815a7;prop1=value1'; - const expected: Baggage = { - key1: { value: 'd4cda95b' }, - key3: { value: 'c88815a7;prop1=value1' }, - }; + const bag = getBaggage( + httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) + ); + + assert.ok(bag); + const entries = bag.getAllEntries(); + + assert.strictEqual(entries.length, 2); + assert.deepStrictEqual(bag.getEntry('key1')!.value, 'd4cda95b'); + assert.deepStrictEqual(bag.getEntry('key3')!.value, 'c88815a7'); assert.deepStrictEqual( - getBaggage( - httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) - ), - expected + bag.getEntry('key3')!.metadata?.toString(), + 'prop1=value1' ); }); @@ -181,11 +254,11 @@ describe('HttpBaggage', () => { }, mixInvalidAndValidKeys: { header: 'key1==value,key2=value2', - baggage: { + baggage: createBaggage({ key2: { value: 'value2', }, - }, + }), }, }; Object.getOwnPropertyNames(testCases).forEach(testCase => { diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts index 9e79ae86d8..61faf9d620 100644 --- a/packages/opentelemetry-shim-opentracing/src/shim.ts +++ b/packages/opentelemetry-shim-opentracing/src/shim.ts @@ -16,7 +16,11 @@ import * as api from '@opentelemetry/api'; import * as opentracing from 'opentracing'; -import { SpanAttributes, SpanAttributeValue } from '@opentelemetry/api'; +import { + createBaggage, + SpanAttributes, + SpanAttributeValue, +} from '@opentelemetry/api'; function translateReferences(references: opentracing.Reference[]): api.Link[] { const links: api.Link[] = []; @@ -103,13 +107,11 @@ export class SpanContextShim extends opentracing.SpanContext { } getBaggageItem(key: string): string | undefined { - return this._baggage[key]?.value; + return this._baggage.getEntry(key)?.value; } setBaggageItem(key: string, value: string) { - this._baggage = Object.assign({}, this._baggage, { - [key]: { value }, - }); + this._baggage = this._baggage.setEntry(key, { value }); } } @@ -138,7 +140,7 @@ export class TracerShim extends opentracing.Tracer { getContextWithParent(options) ); - let baggage: api.Baggage = {}; + let baggage: api.Baggage = createBaggage(); if (options.childOf instanceof SpanShim) { const shimContext = options.childOf.context() as SpanContextShim; baggage = shimContext.getBaggage(); @@ -200,7 +202,7 @@ export class TracerShim extends opentracing.Tracer { if (!spanContext) { return null; } - return new SpanContextShim(spanContext, baggage || {}); + return new SpanContextShim(spanContext, baggage || createBaggage()); } case opentracing.FORMAT_BINARY: { // @todo: Implement binary format diff --git a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts index ba0b3b461d..b571150013 100644 --- a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts +++ b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts @@ -24,7 +24,11 @@ import { CompositePropagator, HttpBaggage, } from '@opentelemetry/core'; -import { INVALID_SPAN_CONTEXT, propagation } from '@opentelemetry/api'; +import { + createBaggage, + INVALID_SPAN_CONTEXT, + propagation, +} from '@opentelemetry/api'; import { performance } from 'perf_hooks'; describe('OpenTracing Shim', () => { @@ -159,7 +163,7 @@ describe('OpenTracing Shim', () => { describe('SpanContextShim', () => { it('returns the correct context', () => { - const shim = new SpanContextShim(INVALID_SPAN_CONTEXT, {}); + const shim = new SpanContextShim(INVALID_SPAN_CONTEXT, createBaggage()); assert.strictEqual(shim.getSpanContext(), INVALID_SPAN_CONTEXT); assert.strictEqual(shim.toTraceId(), INVALID_SPAN_CONTEXT.traceId); assert.strictEqual(shim.toSpanId(), INVALID_SPAN_CONTEXT.spanId);