Skip to content

Commit 0a0c5fe

Browse files
committed
fix: make TraceState immutable
Spec requires that TraceState is immutable. Adapt TraceState API to return a TraceState for set/unset. Adapt implemenation of set/unset to clone the TraceState before mutating and return the cloned TraceState.
1 parent 1c27690 commit 0a0c5fe

File tree

3 files changed

+35
-21
lines changed

3 files changed

+35
-21
lines changed

packages/opentelemetry-api/src/trace/trace_state.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,25 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
1617
export interface TraceState {
1718
/**
18-
* Adds or updates the TraceState that has the given `key` if it is
19-
* present. The new State will always be added in the front of the
20-
* list of states.
19+
* Create a new TraceState which inherits from this TraceState and has the
20+
* given key set.
21+
* The new entry will always be added in the front of the list of states.
2122
*
2223
* @param key key of the TraceState entry.
2324
* @param value value of the TraceState entry.
2425
*/
25-
set(key: string, value: string): void;
26+
set(key: string, value: string): TraceState;
2627

2728
/**
28-
* Removes the TraceState Entry that has the given `key` if it is present.
29+
* Return a new TraceState which inherits from this TraceState but does not
30+
* contain the given key.
2931
*
30-
* @param key the key for the TraceState Entry to be removed.
32+
* @param key the key for the TraceState entry to be removed.
3133
*/
32-
unset(key: string): void;
34+
unset(key: string): TraceState;
3335

3436
/**
3537
* Returns the value to which the specified key is mapped, or `undefined` if

packages/opentelemetry-core/src/trace/TraceState.ts

+17-5
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,21 @@ export class TraceState implements api.TraceState {
3838
if (rawTraceState) this._parse(rawTraceState);
3939
}
4040

41-
set(key: string, value: string): void {
41+
set(key: string, value: string): TraceState {
4242
// TODO: Benchmark the different approaches(map vs list) and
4343
// use the faster one.
44-
if (this._internalState.has(key)) this._internalState.delete(key);
45-
this._internalState.set(key, value);
44+
const traceState = this._clone();
45+
if (traceState._internalState.has(key)) {
46+
traceState._internalState.delete(key);
47+
}
48+
traceState._internalState.set(key, value);
49+
return traceState;
4650
}
4751

48-
unset(key: string): void {
49-
this._internalState.delete(key);
52+
unset(key: string): TraceState {
53+
const traceState = this._clone();
54+
traceState._internalState.delete(key);
55+
return traceState;
5056
}
5157

5258
get(key: string): string | undefined {
@@ -95,4 +101,10 @@ export class TraceState implements api.TraceState {
95101
private _keys(): string[] {
96102
return Array.from(this._internalState.keys()).reverse();
97103
}
104+
105+
private _clone(): TraceState {
106+
const traceState = new TraceState();
107+
traceState._internalState = new Map(this._internalState);
108+
return traceState;
109+
}
98110
}

packages/opentelemetry-core/test/trace/tracestate.test.ts

+9-9
Original file line numberDiff line numberDiff line change
@@ -25,30 +25,30 @@ describe('TraceState', () => {
2525
});
2626

2727
it('must replace keys and move them to the front', () => {
28-
const state = new TraceState('a=1,b=2');
29-
state.set('b', '3');
28+
const orgState = new TraceState('a=1,b=2');
29+
const state = orgState.set('b', '3');
30+
assert.deepStrictEqual(orgState.serialize(), 'a=1,b=2');
3031
assert.deepStrictEqual(state.serialize(), 'b=3,a=1');
3132
});
3233

3334
it('must add new keys to the front', () => {
34-
const state = new TraceState();
35-
state.set('vendorname1', 'opaqueValue1');
35+
let state = new TraceState().set('vendorname1', 'opaqueValue1');
3636
assert.deepStrictEqual(state.serialize(), 'vendorname1=opaqueValue1');
3737

38-
state.set('vendorname2', 'opaqueValue2');
38+
state = state.set('vendorname2', 'opaqueValue2');
3939
assert.deepStrictEqual(
4040
state.serialize(),
4141
'vendorname2=opaqueValue2,vendorname1=opaqueValue1'
4242
);
4343
});
4444

4545
it('must unset the entries', () => {
46-
const state = new TraceState('c=4,b=3,a=1');
47-
state.unset('b');
46+
const orgState = new TraceState('c=4,b=3,a=1');
47+
let state = orgState.unset('b');
4848
assert.deepStrictEqual(state.serialize(), 'c=4,a=1');
49-
state.unset('c');
50-
state.unset('A');
49+
state = state.unset('c').unset('A');
5150
assert.deepStrictEqual(state.serialize(), 'a=1');
51+
assert.strictEqual(orgState.serialize(), 'c=4,b=3,a=1');
5252
});
5353
});
5454

0 commit comments

Comments
 (0)