Skip to content

Commit

Permalink
fix: make TraceState immutable
Browse files Browse the repository at this point in the history
Spec requires that TraceState is immutable.

Adapt TraceState API to return a TraceState for set/unset.
Adapt implementation of set/unset to clone the TraceState before
mutating and return the cloned TraceState.
  • Loading branch information
Flarna committed Oct 15, 2020
1 parent 1c27690 commit 5e412ac
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 21 deletions.
16 changes: 9 additions & 7 deletions packages/opentelemetry-api/src/trace/trace_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,25 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export interface TraceState {
/**
* Adds or updates the TraceState that has the given `key` if it is
* present. The new State will always be added in the front of the
* list of states.
* Create a new TraceState which inherits from this TraceState and has the
* given key set.
* The new entry will always be added in the front of the list of states.
*
* @param key key of the TraceState entry.
* @param value value of the TraceState entry.
*/
set(key: string, value: string): void;
set(key: string, value: string): TraceState;

/**
* Removes the TraceState Entry that has the given `key` if it is present.
* Return a new TraceState which inherits from this TraceState but does not
* contain the given key.
*
* @param key the key for the TraceState Entry to be removed.
* @param key the key for the TraceState entry to be removed.
*/
unset(key: string): void;
unset(key: string): TraceState;

/**
* Returns the value to which the specified key is mapped, or `undefined` if
Expand Down
22 changes: 17 additions & 5 deletions packages/opentelemetry-core/src/trace/TraceState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,21 @@ export class TraceState implements api.TraceState {
if (rawTraceState) this._parse(rawTraceState);
}

set(key: string, value: string): void {
set(key: string, value: string): TraceState {
// TODO: Benchmark the different approaches(map vs list) and
// use the faster one.
if (this._internalState.has(key)) this._internalState.delete(key);
this._internalState.set(key, value);
const traceState = this._clone();
if (traceState._internalState.has(key)) {
traceState._internalState.delete(key);
}
traceState._internalState.set(key, value);
return traceState;
}

unset(key: string): void {
this._internalState.delete(key);
unset(key: string): TraceState {
const traceState = this._clone();
traceState._internalState.delete(key);
return traceState;
}

get(key: string): string | undefined {
Expand Down Expand Up @@ -95,4 +101,10 @@ export class TraceState implements api.TraceState {
private _keys(): string[] {
return Array.from(this._internalState.keys()).reverse();
}

private _clone(): TraceState {
const traceState = new TraceState();
traceState._internalState = new Map(this._internalState);
return traceState;
}
}
18 changes: 9 additions & 9 deletions packages/opentelemetry-core/test/trace/tracestate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,30 @@ describe('TraceState', () => {
});

it('must replace keys and move them to the front', () => {
const state = new TraceState('a=1,b=2');
state.set('b', '3');
const orgState = new TraceState('a=1,b=2');
const state = orgState.set('b', '3');
assert.deepStrictEqual(orgState.serialize(), 'a=1,b=2');
assert.deepStrictEqual(state.serialize(), 'b=3,a=1');
});

it('must add new keys to the front', () => {
const state = new TraceState();
state.set('vendorname1', 'opaqueValue1');
let state = new TraceState().set('vendorname1', 'opaqueValue1');
assert.deepStrictEqual(state.serialize(), 'vendorname1=opaqueValue1');

state.set('vendorname2', 'opaqueValue2');
state = state.set('vendorname2', 'opaqueValue2');
assert.deepStrictEqual(
state.serialize(),
'vendorname2=opaqueValue2,vendorname1=opaqueValue1'
);
});

it('must unset the entries', () => {
const state = new TraceState('c=4,b=3,a=1');
state.unset('b');
const orgState = new TraceState('c=4,b=3,a=1');
let state = orgState.unset('b');
assert.deepStrictEqual(state.serialize(), 'c=4,a=1');
state.unset('c');
state.unset('A');
state = state.unset('c').unset('A');
assert.deepStrictEqual(state.serialize(), 'a=1');
assert.strictEqual(orgState.serialize(), 'c=4,b=3,a=1');
});
});

Expand Down

0 comments on commit 5e412ac

Please sign in to comment.