fix: make TraceState immutable (#1597)

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

@@ -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
+   * Create a new TraceState which inherits from this TraceState and has the
-   * present. The new State will always be added in the front of the
+   * given key set.
-   * list of states.
+   * 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

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

@@ -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);
+    const traceState = this._clone();
-    this._internalState.set(key, value);
+    if (traceState._internalState.has(key)) {
+      traceState._internalState.delete(key);
+    }
+    traceState._internalState.set(key, value);
+    return traceState;
   }
 
-  unset(key: string): void {
+  unset(key: string): TraceState {
-    this._internalState.delete(key);
+    const traceState = this._clone();
+    traceState._internalState.delete(key);
+    return traceState;
   }
 
   get(key: string): string | undefined {
@@ -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;
+  }
 }

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

@@ -24,31 +24,31 @@ describe('TraceState', () => {
       assert.deepStrictEqual(state.serialize(), 'a=1,b=2');
     });
 
-    it('must replace keys and move them to the front', () => {
+    it('must create a new TraceState and move updated keys to the front', () => {
-      const state = new TraceState('a=1,b=2');
+      const orgState = new TraceState('a=1,b=2');
-      state.set('b', '3');
+      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', () => {
+    it('must create a new TraceState and add new keys to the front', () => {
-      const state = new TraceState();
+      let state = new TraceState().set('vendorname1', 'opaqueValue1');
-      state.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', () => {
+    it('must create a new TraceState and unset the entries', () => {
-      const state = new TraceState('c=4,b=3,a=1');
+      const orgState = new TraceState('c=4,b=3,a=1');
-      state.unset('b');
+      let state = orgState.unset('b');
       assert.deepStrictEqual(state.serialize(), 'c=4,a=1');
-      state.unset('c');
+      state = state.unset('c').unset('A');
-      state.unset('A');
       assert.deepStrictEqual(state.serialize(), 'a=1');
+      assert.strictEqual(orgState.serialize(), 'c=4,b=3,a=1');
     });
   });