Skip to content

Commit

Permalink
fix: make TraceState immutable (#1597)
Browse files Browse the repository at this point in the history
  • Loading branch information
Flarna authored Oct 22, 2020
1 parent 956604e commit cd8e77b
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 24 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;
}
}
24 changes: 12 additions & 12 deletions packages/opentelemetry-core/test/trace/tracestate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,31 @@ describe('TraceState', () => {
assert.deepStrictEqual(state.serialize(), 'a=1,b=2');
});

it('must replace keys and move them to the front', () => {
const state = new TraceState('a=1,b=2');
state.set('b', '3');
it('must create a new TraceState and move updated keys to the front', () => {
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');
it('must create a new TraceState and add new keys to the front', () => {
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');
it('must create a new TraceState and unset the entries', () => {
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 cd8e77b

Please sign in to comment.