From fcbb29e27f41d90e99ed85159c021cfa700d8fc4 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 27 Jan 2021 15:18:00 -0500 Subject: [PATCH 1/7] refactor!: specification compliant baggage --- .../opentelemetry-api/src/baggage/Baggage.ts | 43 +++++++-- .../src/baggage/{EntryValue.ts => Entry.ts} | 30 +++---- .../opentelemetry-api/src/baggage/index.ts | 47 ++++++++++ .../src/baggage/internal/baggage.ts | 47 ++++++++++ packages/opentelemetry-api/src/index.ts | 3 +- .../src/baggage/propagation/HttpBaggage.ts | 41 +++++---- .../test/baggage/HttpBaggage.test.ts | 90 ++++++++++--------- .../src/shim.ts | 12 ++- .../test/Shim.test.ts | 8 +- 9 files changed, 223 insertions(+), 98 deletions(-) rename packages/opentelemetry-api/src/baggage/{EntryValue.ts => Entry.ts} (53%) create mode 100644 packages/opentelemetry-api/src/baggage/index.ts create mode 100644 packages/opentelemetry-api/src/baggage/internal/baggage.ts diff --git a/packages/opentelemetry-api/src/baggage/Baggage.ts b/packages/opentelemetry-api/src/baggage/Baggage.ts index 1ab2a9b670e..24dadf021a3 100644 --- a/packages/opentelemetry-api/src/baggage/Baggage.ts +++ b/packages/opentelemetry-api/src/baggage/Baggage.ts @@ -14,16 +14,43 @@ * limitations under the License. */ -import { EntryValue } from './EntryValue'; +import { BaggageEntry, BaggageEntryMetadata } 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]: EntryValue; + /** + * 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(): BaggageEntry[]; + + /** + * Create a new Baggage from this baggage with a new entry. + * + * @param key string which identifies the baggage entry + * @param value string value of the baggage + * @param metadata optional entry metadata + */ + setEntry( + key: string, + value: string, + metadata?: BaggageEntryMetadata + ): Baggage; + + /** + * Create a new baggage containing all entries from this except the removed entry + * + * @param key key identifying the entry to be removed + */ + removeEntry(key: string): Baggage; } diff --git a/packages/opentelemetry-api/src/baggage/EntryValue.ts b/packages/opentelemetry-api/src/baggage/Entry.ts similarity index 53% rename from packages/opentelemetry-api/src/baggage/EntryValue.ts rename to packages/opentelemetry-api/src/baggage/Entry.ts index 59ec73cc2a3..0f7bde5d2a2 100644 --- a/packages/opentelemetry-api/src/baggage/EntryValue.ts +++ b/packages/opentelemetry-api/src/baggage/Entry.ts @@ -13,28 +13,24 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export interface EntryValue { - /** `String` value of the `EntryValue`. */ + +export interface BaggageEntry { + /** `String` key of the `BaggageEntry` */ + key: string; + + /** `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?: EntryTtl; + metadata?: BaggageEntryMetadata; } /** - * EntryTtl 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 EntryTtl { - /** - * 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 interface BaggageEntryMetadata { + toString(): string; } diff --git a/packages/opentelemetry-api/src/baggage/index.ts b/packages/opentelemetry-api/src/baggage/index.ts new file mode 100644 index 00000000000..5cc950a3543 --- /dev/null +++ b/packages/opentelemetry-api/src/baggage/index.ts @@ -0,0 +1,47 @@ +/* + * 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'; + +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: BaggageEntry[] = []): Baggage { + return new BaggageImpl(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 { + return { + toString: function () { + 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 00000000000..23c394fc8e4 --- /dev/null +++ b/packages/opentelemetry-api/src/baggage/internal/baggage.ts @@ -0,0 +1,47 @@ +/* + * 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 { + constructor(private _entries: BaggageEntry[]) {} + + getEntry(key: string): BaggageEntry | undefined { + const entry = this._entries.find(e => e.key === key); + if (!entry) { + return undefined; + } + + return Object.assign(Object.create(null), entry); + } + + getAllEntries(): BaggageEntry[] { + return this._entries.map(e => Object.assign(Object.create(null), e)); + } + + setEntry(key: string, value: string, metadata?: string): BaggageImpl { + const newBaggage = this.removeEntry(key); + newBaggage._entries.push( + Object.assign(Object.create(null), { key, value, metadata }) + ); + return newBaggage; + } + + removeEntry(key: string): BaggageImpl { + return new BaggageImpl(this._entries.filter(e => e.key !== key)); + } +} diff --git a/packages/opentelemetry-api/src/index.ts b/packages/opentelemetry-api/src/index.ts index f1dcf8211a4..4c6b64ffd64 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-core/src/baggage/propagation/HttpBaggage.ts b/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts index a576d5224dd..d49fa3ce2af 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,33 +69,35 @@ 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( + entry => + `${encodeURIComponent(entry.key)}=${encodeURIComponent(entry.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 entries: BaggageEntry[] = []; if (headerValue.length == 0) { return context; } const pairs = headerValue.split(ITEMS_SEPARATOR); pairs.forEach(entry => { - const keyPair = this._parsePairKeyValue(entry); - if (keyPair) { - baggage[keyPair.key] = { value: keyPair.value }; + const parsedEntry = this._parsePairKeyValue(entry); + if (parsedEntry) { + entries.push(parsedEntry); } }); - if (Object.entries(baggage).length === 0) { + if (entries.length === 0) { return context; } - return setBaggage(context, baggage); + return setBaggage(context, createBaggage(entries)); } - private _parsePairKeyValue(entry: string): KeyPair | undefined { + private _parsePairKeyValue(entry: string): BaggageEntry | undefined { const valueProps = entry.split(PROPERTIES_SEPARATOR); if (valueProps.length <= 0) return; const keyPairPart = valueProps.shift(); @@ -104,12 +105,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 3d1ff9b33ea..8a1420832b8 100644 --- a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts +++ b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts @@ -15,17 +15,18 @@ */ import { + Baggage, + 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, + HttpBaggage, MAX_PER_NAME_VALUE_PAIRS, } from '../../src/baggage/propagation/HttpBaggage'; @@ -40,11 +41,11 @@ describe('HttpBaggage', () => { describe('.inject()', () => { it('should set baggage header', () => { - const baggage: Baggage = { - key1: { value: 'd4cda95b652f4a1592b449d5929fda1b' }, - key3: { value: 'c88815a7-0fa9-4d95-a1f1-cdccce3c5c2a' }, - 'with/slash': { value: 'with spaces' }, - }; + const baggage: Baggage = createBaggage([ + { key: 'key1', value: 'd4cda95b652f4a1592b449d5929fda1b' }, + { key: 'key3', value: 'c88815a7-0fa9-4d95-a1f1-cdccce3c5c2a' }, + { key: 'with/slash', value: 'with spaces' }, + ]); httpTraceContext.inject( setBaggage(ROOT_CONTEXT, baggage), @@ -58,14 +59,11 @@ describe('HttpBaggage', () => { }); it('should skip long key-value pairs', () => { - const baggage: Baggage = { - 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 }; + const baggage = createBaggage([ + { key: 'key1', value: 'd4cda95b' }, + { key: 'key3', value: 'c88815a7' }, + { key: 'longPair', value: '1a'.repeat(MAX_PER_NAME_VALUE_PAIRS) }, + ]); httpTraceContext.inject( setBaggage(ROOT_CONTEXT, baggage), @@ -79,16 +77,17 @@ describe('HttpBaggage', () => { }); it('should skip all keys that surpassed the max limit of the header', () => { - const baggage: Baggage = {}; - const zeroPad = (num: number, places: number) => String(num).padStart(places, '0'); - // 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}` }; - } + const baggage = createBaggage( + Array(9) + .fill(0) + .map((_, i) => ({ + key: `k${zeroPad(i, 510)}`, + value: String(zeroPad(i, 510)), + })) + ); // Build expected let expected = ''; @@ -115,13 +114,16 @@ describe('HttpBaggage', () => { httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) ); - const expected: Baggage = { - key1: { value: 'd4cda95b' }, - key3: { value: 'c88815a7' }, - keyn: { value: 'valn' }, - keym: { value: 'valm' }, - }; - assert.deepStrictEqual(extractedBaggage, expected); + assert.deepStrictEqual( + extractedBaggage?.getEntry('key1')!.value, + 'd4cda95b' + ); + assert.deepStrictEqual( + extractedBaggage?.getEntry('key3')!.value, + 'c88815a7' + ); + assert.deepStrictEqual(extractedBaggage?.getEntry('keyn')!.value, 'valn'); + assert.deepStrictEqual(extractedBaggage?.getEntry('keym')!.value, 'valm'); }); }); @@ -143,15 +145,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 +187,9 @@ describe('HttpBaggage', () => { }, mixInvalidAndValidKeys: { header: 'key1==value,key2=value2', - baggage: { - key2: { - value: 'value2', - }, - }, + baggage: createBaggage([ + { key: 'key2', value: 'value2', metadata: undefined }, + ]), }, }; Object.getOwnPropertyNames(testCases).forEach(testCase => { diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts index c6b41c9a9b8..887101c2dd4 100644 --- a/packages/opentelemetry-shim-opentracing/src/shim.ts +++ b/packages/opentelemetry-shim-opentracing/src/shim.ts @@ -16,7 +16,7 @@ import * as api from '@opentelemetry/api'; import * as opentracing from 'opentracing'; -import { Attributes, AttributeValue } from '@opentelemetry/api'; +import { Attributes, AttributeValue, createBaggage } from '@opentelemetry/api'; function translateReferences(references: opentracing.Reference[]): api.Link[] { const links: api.Link[] = []; @@ -103,13 +103,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 +136,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 +198,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 ba0b3b461d1..b5711500137 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); From 63b6a6e0cec39a224e043730e5f0143bc34652db Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 28 Jan 2021 15:35:29 -0500 Subject: [PATCH 2/7] chore: use map in baggage --- .../opentelemetry-api/src/baggage/Baggage.ts | 13 ++-- .../opentelemetry-api/src/baggage/Entry.ts | 3 - .../opentelemetry-api/src/baggage/index.ts | 6 +- .../src/baggage/internal/baggage.ts | 24 ++++--- .../src/baggage/propagation/HttpBaggage.ts | 22 ++++--- .../test/baggage/HttpBaggage.test.ts | 63 +++++++++---------- .../src/shim.ts | 2 +- 7 files changed, 67 insertions(+), 66 deletions(-) diff --git a/packages/opentelemetry-api/src/baggage/Baggage.ts b/packages/opentelemetry-api/src/baggage/Baggage.ts index 24dadf021a3..d2353d1cd0b 100644 --- a/packages/opentelemetry-api/src/baggage/Baggage.ts +++ b/packages/opentelemetry-api/src/baggage/Baggage.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { BaggageEntry, BaggageEntryMetadata } from './Entry'; +import { BaggageEntry } from './Entry'; /** * Baggage represents collection of key-value pairs with optional metadata. @@ -32,20 +32,15 @@ export interface Baggage { /** * Get a list of all entries in the Baggage */ - getAllEntries(): BaggageEntry[]; + getAllEntries(): [string, BaggageEntry][]; /** * Create a new Baggage from this baggage with a new entry. * * @param key string which identifies the baggage entry - * @param value string value of the baggage - * @param metadata optional entry metadata + * @param entry BaggageEntry for the given key */ - setEntry( - key: string, - value: string, - metadata?: BaggageEntryMetadata - ): Baggage; + setEntry(key: string, entry: BaggageEntry): Baggage; /** * Create a new baggage containing all entries from this except the removed entry diff --git a/packages/opentelemetry-api/src/baggage/Entry.ts b/packages/opentelemetry-api/src/baggage/Entry.ts index 0f7bde5d2a2..ec494053201 100644 --- a/packages/opentelemetry-api/src/baggage/Entry.ts +++ b/packages/opentelemetry-api/src/baggage/Entry.ts @@ -15,9 +15,6 @@ */ export interface BaggageEntry { - /** `String` key of the `BaggageEntry` */ - key: string; - /** `String` value of the `BaggageEntry`. */ value: string; /** diff --git a/packages/opentelemetry-api/src/baggage/index.ts b/packages/opentelemetry-api/src/baggage/index.ts index 5cc950a3543..aa320fd28ea 100644 --- a/packages/opentelemetry-api/src/baggage/index.ts +++ b/packages/opentelemetry-api/src/baggage/index.ts @@ -26,8 +26,10 @@ export * from './Entry'; * * @param entries An array of baggage entries the new baggage should contain */ -export function createBaggage(entries: BaggageEntry[] = []): Baggage { - return new BaggageImpl(entries); +export function createBaggage( + entries: Record = {} +): Baggage { + return new BaggageImpl(new Map(Object.entries(entries))); } /** diff --git a/packages/opentelemetry-api/src/baggage/internal/baggage.ts b/packages/opentelemetry-api/src/baggage/internal/baggage.ts index 23c394fc8e4..56fe62b2360 100644 --- a/packages/opentelemetry-api/src/baggage/internal/baggage.ts +++ b/packages/opentelemetry-api/src/baggage/internal/baggage.ts @@ -18,10 +18,14 @@ import type { Baggage } from '../Baggage'; import type { BaggageEntry } from '../Entry'; export class BaggageImpl implements Baggage { - constructor(private _entries: BaggageEntry[]) {} + private _entries: Map; + + constructor(entries: Map) { + this._entries = new Map(entries); + } getEntry(key: string): BaggageEntry | undefined { - const entry = this._entries.find(e => e.key === key); + const entry = this._entries.get(key); if (!entry) { return undefined; } @@ -29,19 +33,19 @@ export class BaggageImpl implements Baggage { return Object.assign(Object.create(null), entry); } - getAllEntries(): BaggageEntry[] { - return this._entries.map(e => Object.assign(Object.create(null), e)); + getAllEntries(): [string, BaggageEntry][] { + return Array.from(this._entries.entries()).map(([k, v]) => [k, v]); } - setEntry(key: string, value: string, metadata?: string): BaggageImpl { - const newBaggage = this.removeEntry(key); - newBaggage._entries.push( - Object.assign(Object.create(null), { key, value, metadata }) - ); + setEntry(key: string, entry: BaggageEntry): BaggageImpl { + const newBaggage = new BaggageImpl(this._entries); + newBaggage._entries.set(key, entry); return newBaggage; } removeEntry(key: string): BaggageImpl { - return new BaggageImpl(this._entries.filter(e => e.key !== key)); + const newBaggage = new BaggageImpl(this._entries); + newBaggage._entries.delete(key); + return newBaggage; } } diff --git a/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts b/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts index d49fa3ce2af..8999af82455 100644 --- a/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts +++ b/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts @@ -72,32 +72,36 @@ export class HttpBaggage implements TextMapPropagator { return baggage .getAllEntries() .map( - entry => - `${encodeURIComponent(entry.key)}=${encodeURIComponent(entry.value)}` + ([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 entries: BaggageEntry[] = []; + const baggage: Record = {}; if (headerValue.length == 0) { return context; } const pairs = headerValue.split(ITEMS_SEPARATOR); pairs.forEach(entry => { - const parsedEntry = this._parsePairKeyValue(entry); - if (parsedEntry) { - entries.push(parsedEntry); + const keyPair = this._parsePairKeyValue(entry); + if (keyPair) { + const entry: BaggageEntry = { value: keyPair.value }; + if (keyPair.metadata) { + entry.metadata = keyPair.metadata; + } + baggage[keyPair.key] = entry; } }); - if (entries.length === 0) { + if (Object.entries(baggage).length === 0) { return context; } - return setBaggage(context, createBaggage(entries)); + return setBaggage(context, createBaggage(baggage)); } - private _parsePairKeyValue(entry: string): BaggageEntry | undefined { + private _parsePairKeyValue(entry: string) { const valueProps = entry.split(PROPERTIES_SEPARATOR); if (valueProps.length <= 0) return; const keyPairPart = valueProps.shift(); diff --git a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts index 8a1420832b8..42209a262a4 100644 --- a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts +++ b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts @@ -16,6 +16,7 @@ import { Baggage, + BaggageEntry, createBaggage, defaultTextMapGetter, defaultTextMapSetter, @@ -27,7 +28,6 @@ import * as assert from 'assert'; import { BAGGAGE_HEADER, HttpBaggage, - MAX_PER_NAME_VALUE_PAIRS, } from '../../src/baggage/propagation/HttpBaggage'; describe('HttpBaggage', () => { @@ -41,11 +41,11 @@ describe('HttpBaggage', () => { describe('.inject()', () => { it('should set baggage header', () => { - const baggage: Baggage = createBaggage([ - { key: 'key1', value: 'd4cda95b652f4a1592b449d5929fda1b' }, - { key: 'key3', value: 'c88815a7-0fa9-4d95-a1f1-cdccce3c5c2a' }, - { key: 'with/slash', value: 'with spaces' }, - ]); + const baggage = createBaggage({ + key1: { value: 'd4cda95b652f4a1592b449d5929fda1b' }, + key3: { value: 'c88815a7-0fa9-4d95-a1f1-cdccce3c5c2a' }, + 'with/slash': { value: 'with spaces' }, + }); httpTraceContext.inject( setBaggage(ROOT_CONTEXT, baggage), @@ -59,11 +59,10 @@ describe('HttpBaggage', () => { }); it('should skip long key-value pairs', () => { - const baggage = createBaggage([ - { key: 'key1', value: 'd4cda95b' }, - { key: 'key3', value: 'c88815a7' }, - { key: 'longPair', value: '1a'.repeat(MAX_PER_NAME_VALUE_PAIRS) }, - ]); + const baggage = createBaggage({ + key1: { value: 'd4cda95b' }, + key3: { value: 'c88815a7' }, + }); httpTraceContext.inject( setBaggage(ROOT_CONTEXT, baggage), @@ -80,14 +79,15 @@ describe('HttpBaggage', () => { const zeroPad = (num: number, places: number) => String(num).padStart(places, '0'); - const baggage = createBaggage( - Array(9) - .fill(0) - .map((_, i) => ({ - key: `k${zeroPad(i, 510)}`, - value: String(zeroPad(i, 510)), - })) - ); + const bag: Record = {}; + + // key=value with same size , 1024 => 8 keys + for (let i = 0; i < 9; ++i) { + const index = zeroPad(i, 510); + bag[`k${index}`] = { value: `${index}` }; + } + + const baggage = createBaggage(bag); // Build expected let expected = ''; @@ -114,16 +114,13 @@ describe('HttpBaggage', () => { httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) ); - assert.deepStrictEqual( - extractedBaggage?.getEntry('key1')!.value, - 'd4cda95b' - ); - assert.deepStrictEqual( - extractedBaggage?.getEntry('key3')!.value, - 'c88815a7' - ); - assert.deepStrictEqual(extractedBaggage?.getEntry('keyn')!.value, 'valn'); - assert.deepStrictEqual(extractedBaggage?.getEntry('keym')!.value, 'valm'); + const expected = createBaggage({ + key1: { value: 'd4cda95b' }, + key3: { value: 'c88815a7' }, + keyn: { value: 'valn' }, + keym: { value: 'valm' }, + }); + assert.deepStrictEqual(extractedBaggage, expected); }); }); @@ -187,9 +184,11 @@ describe('HttpBaggage', () => { }, mixInvalidAndValidKeys: { header: 'key1==value,key2=value2', - baggage: createBaggage([ - { key: 'key2', value: 'value2', metadata: undefined }, - ]), + 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 887101c2dd4..ea602bd5dc3 100644 --- a/packages/opentelemetry-shim-opentracing/src/shim.ts +++ b/packages/opentelemetry-shim-opentracing/src/shim.ts @@ -107,7 +107,7 @@ export class SpanContextShim extends opentracing.SpanContext { } setBaggageItem(key: string, value: string) { - this._baggage = this._baggage.setEntry(key, value); + this._baggage = this._baggage.setEntry(key, { value }); } } From 747262feb1b9a31d55f1417f2e323cf47f25d5c6 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 28 Jan 2021 16:37:09 -0500 Subject: [PATCH 3/7] test: add baggage tests --- .../opentelemetry-api/src/baggage/Entry.ts | 8 +- .../opentelemetry-api/src/baggage/index.ts | 9 +- .../src/baggage/internal/baggage.ts | 2 +- .../src/baggage/internal/symbol.ts | 20 +++ .../test/baggage/Baggage.test.ts | 125 ++++++++++++++++++ 5 files changed, 159 insertions(+), 5 deletions(-) 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/Entry.ts b/packages/opentelemetry-api/src/baggage/Entry.ts index ec494053201..d249c85cecd 100644 --- a/packages/opentelemetry-api/src/baggage/Entry.ts +++ b/packages/opentelemetry-api/src/baggage/Entry.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import { baggageEntryMetadataSymbol } from './internal/symbol'; + export interface BaggageEntry { /** `String` value of the `BaggageEntry`. */ value: string; @@ -28,6 +30,6 @@ export interface BaggageEntry { * Serializable Metadata defined by the W3C baggage specification. * It currently has no special meaning defined by the OpenTelemetry or W3C. */ -export interface BaggageEntryMetadata { - toString(): string; -} +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 index aa320fd28ea..f4a3dbb5cab 100644 --- a/packages/opentelemetry-api/src/baggage/index.ts +++ b/packages/opentelemetry-api/src/baggage/index.ts @@ -17,6 +17,7 @@ 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'; @@ -41,8 +42,14 @@ export function createBaggage( export function baggageEntryMetadataFromString( str: string ): BaggageEntryMetadata { + if (typeof str !== 'string') { + // TODO log diagnostic + str = ''; + } + return { - toString: function () { + __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 index 56fe62b2360..978baae4087 100644 --- a/packages/opentelemetry-api/src/baggage/internal/baggage.ts +++ b/packages/opentelemetry-api/src/baggage/internal/baggage.ts @@ -30,7 +30,7 @@ export class BaggageImpl implements Baggage { return undefined; } - return Object.assign(Object.create(null), entry); + return Object.assign({}, entry); } getAllEntries(): [string, BaggageEntry][] { 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 00000000000..f4213926c98 --- /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/test/baggage/Baggage.test.ts b/packages/opentelemetry-api/test/baggage/Baggage.test.ts new file mode 100644 index 00000000000..1c7dad8ea5c --- /dev/null +++ b/packages/opentelemetry-api/test/baggage/Baggage.test.ts @@ -0,0 +1,125 @@ +/* + * 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 deleted', () => { + const bag = createBaggage({ + key: { value: 'value' }, + }); + + const bag2 = bag.removeEntry('key'); + + assert.deepStrictEqual(bag.getAllEntries(), [ + ['key', { value: 'value' }], + ]); + + 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 + 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'); + }); + }); +}); From 9a29a537b137d0bd2c6b63b50387fbc223c06127 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 2 Feb 2021 13:58:57 -0500 Subject: [PATCH 4/7] chore: review comments --- .../opentelemetry-api/src/baggage/Baggage.ts | 16 ++++++++++++++-- packages/opentelemetry-api/src/baggage/index.ts | 2 +- .../src/baggage/internal/baggage.ts | 16 ++++++++++++++-- .../test/baggage/Baggage.test.ts | 4 ++-- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-api/src/baggage/Baggage.ts b/packages/opentelemetry-api/src/baggage/Baggage.ts index d2353d1cd0b..2876f5bd47b 100644 --- a/packages/opentelemetry-api/src/baggage/Baggage.ts +++ b/packages/opentelemetry-api/src/baggage/Baggage.ts @@ -35,7 +35,7 @@ export interface Baggage { getAllEntries(): [string, BaggageEntry][]; /** - * Create a new Baggage from this baggage with a new entry. + * 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 @@ -43,9 +43,21 @@ export interface Baggage { setEntry(key: string, entry: BaggageEntry): Baggage; /** - * Create a new baggage containing all entries from this except the removed entry + * 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/index.ts b/packages/opentelemetry-api/src/baggage/index.ts index f4a3dbb5cab..ba0a297c5d4 100644 --- a/packages/opentelemetry-api/src/baggage/index.ts +++ b/packages/opentelemetry-api/src/baggage/index.ts @@ -43,7 +43,7 @@ export function baggageEntryMetadataFromString( str: string ): BaggageEntryMetadata { if (typeof str !== 'string') { - // TODO log diagnostic + // @TODO log diagnostic str = ''; } diff --git a/packages/opentelemetry-api/src/baggage/internal/baggage.ts b/packages/opentelemetry-api/src/baggage/internal/baggage.ts index 978baae4087..f9331f411ab 100644 --- a/packages/opentelemetry-api/src/baggage/internal/baggage.ts +++ b/packages/opentelemetry-api/src/baggage/internal/baggage.ts @@ -20,8 +20,8 @@ import type { BaggageEntry } from '../Entry'; export class BaggageImpl implements Baggage { private _entries: Map; - constructor(entries: Map) { - this._entries = new Map(entries); + constructor(entries?: Map) { + this._entries = entries ? new Map(entries) : new Map(); } getEntry(key: string): BaggageEntry | undefined { @@ -48,4 +48,16 @@ export class BaggageImpl implements Baggage { 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/test/baggage/Baggage.test.ts b/packages/opentelemetry-api/test/baggage/Baggage.test.ts index 1c7dad8ea5c..c6fce539330 100644 --- a/packages/opentelemetry-api/test/baggage/Baggage.test.ts +++ b/packages/opentelemetry-api/test/baggage/Baggage.test.ts @@ -31,7 +31,7 @@ describe('Baggage', () => { assert.deepStrictEqual(bag.getAllEntries(), []); }); - it('should create a bag with entries', () => { + it('should create a bag with entries', () => { const meta = baggageEntryMetadataFromString('opaque string'); const bag = createBaggage({ key1: { value: 'value1' }, @@ -72,7 +72,7 @@ describe('Baggage', () => { }); describe('remove', () => { - it('should create a new bag when an entry is deleted', () => { + it('should create a new bag when an entry is removed', () => { const bag = createBaggage({ key: { value: 'value' }, }); From b1f53e430d8a1b63dcdfea2af2a7c2ba72b27485 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 2 Feb 2021 13:59:21 -0500 Subject: [PATCH 5/7] chore: baggage propagation test improvements --- .../test/baggage/HttpBaggage.test.ts | 87 +++++++++++++++---- 1 file changed, 69 insertions(+), 18 deletions(-) diff --git a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts index 42209a262a4..be294099342 100644 --- a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts +++ b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts @@ -75,34 +75,85 @@ describe('HttpBaggage', () => { ); }); - it('should skip all keys that surpassed the max limit of the header', () => { - const zeroPad = (num: number, places: number) => - String(num).padStart(places, '0'); + 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 bag: Record = {}; + const baggage = createBaggage({ + [shortKey]: { value: value }, + [longKey]: { value: value }, + }); - // key=value with same size , 1024 => 8 keys - for (let i = 0; i < 9; ++i) { - const index = zeroPad(i, 510); - bag[`k${index}`] = { value: `${index}` }; - } + httpTraceContext.inject( + setBaggage(ROOT_CONTEXT, baggage), + carrier, + defaultTextMapSetter + ); - const baggage = createBaggage(bag); + assert.deepStrictEqual(carrier[BAGGAGE_HEADER], `${shortKey}=${value}`); + }); - // 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); + 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 ); - assert.deepStrictEqual(carrier[BAGGAGE_HEADER], expected); + + 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('') }, + }); + + 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 + ); + + const header = carrier[BAGGAGE_HEADER]; + assert.ok(typeof header === 'string'); + assert.strictEqual(header.split(',').length, 180); }); }); From 9821a68115f679288ba97e84b99aba3cde8dc881 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 2 Feb 2021 14:07:42 -0500 Subject: [PATCH 6/7] chore: improve test for baggage key length --- .../test/baggage/HttpBaggage.test.ts | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts index be294099342..e7231cac22b 100644 --- a/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts +++ b/packages/opentelemetry-core/test/baggage/HttpBaggage.test.ts @@ -80,18 +80,36 @@ describe('HttpBaggage', () => { const shortKey = Array(95).fill('k').join(''); const value = Array(4000).fill('v').join(''); - const baggage = createBaggage({ + let baggage = createBaggage({ + aa: { value: 'shortvalue' }, [shortKey]: { value: value }, + }); + + httpTraceContext.inject( + setBaggage(ROOT_CONTEXT, baggage), + carrier, + defaultTextMapSetter + ); + + 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 ); - assert.deepStrictEqual(carrier[BAGGAGE_HEADER], `${shortKey}=${value}`); + 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', () => { @@ -122,6 +140,7 @@ describe('HttpBaggage', () => { aa: { value: Array(89).fill('v').join('') }, }); + carrier = {}; httpTraceContext.inject( setBaggage(ROOT_CONTEXT, baggage), carrier, From a188f9f1f8fd7ecafdac0aa93fd4b163d39c1d4f Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 2 Feb 2021 14:11:05 -0500 Subject: [PATCH 7/7] chore: add tests for clear and removeEntries --- .../test/baggage/Baggage.test.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/opentelemetry-api/test/baggage/Baggage.test.ts b/packages/opentelemetry-api/test/baggage/Baggage.test.ts index c6fce539330..811c0e6e4c3 100644 --- a/packages/opentelemetry-api/test/baggage/Baggage.test.ts +++ b/packages/opentelemetry-api/test/baggage/Baggage.test.ts @@ -85,6 +85,42 @@ describe('Baggage', () => { 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', () => {